Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix unknown never type #1685

Merged
merged 8 commits into from
Dec 6, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -588,7 +588,6 @@ export type MicrosoftCognitiveLanguageServiceAnalyzeTextAuthoringClient = Client
export interface OperationStatusOutput {
error?: ErrorModelOutput;
id: string;
result?: never;
status: string;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,6 @@ export interface OperationStatusOutput {
status: string;
/** Error object that describes the error when status is "Failed". */
error?: ErrorModelOutput;
/** The result of the operation. */
result?: never;
}

/** The error object. */
Expand Down
3 changes: 1 addition & 2 deletions packages/cadl-typescript/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ import { listClients } from "@azure-tools/cadl-dpg";

export async function $onEmit(program: Program, options: RLCOptions) {
const clients = listClients(program);
for(const client of clients) {
for (const client of clients) {
const rlcModels = await transformRLCModel(program, options, client);
await emitModels(rlcModels, program);
await emitContentByBuilder(program, buildClientDefinitions, rlcModels);
Expand Down Expand Up @@ -69,5 +69,4 @@ export async function $onEmit(program: Program, options: RLCOptions) {
rlcModels
);
}

}
17 changes: 13 additions & 4 deletions packages/cadl-typescript/src/modelUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,10 +102,15 @@ export function getSchemaForType(
}
return type;
}
if (type.kind === "Intrinsic" && type.name === "unknown") {
return { type: "unknown" };
if (isUnknownType(type)) {
const returnType: any = { type: "unknown" };
if (usage && usage.includes(SchemaContext.Output)) {
returnType.outputTypeName = "any";
returnType.typeName = "unknown";
}
return returnType;
}
if (type.kind === "Intrinsic" && type.name === "never") {
if (isNeverType(type)) {
return { type: "never" };
}
reportDiagnostic(program, {
Expand Down Expand Up @@ -693,6 +698,11 @@ function mapCadlIntrinsicModelToTypeScript(
schema.outputTypeName = `Record<string, ${valueType.name}Output>`;
schema.outputValueTypeName = `${valueType.name}Output`;
}
} else if (isUnknownType(indexer.value!)) {
schema.typeName = `Record<string, ${valueType.type}>`;
if (usage && usage.includes(SchemaContext.Output)) {
schema.outputTypeName = `Record<string, ${valueType.outputTypeName}>`;
}
} else {
schema.typeName = `Record<string, ${valueType.type}>`;
}
Expand Down Expand Up @@ -853,7 +863,6 @@ export function getTypeName(schema: Schema): string {
}

export function getImportedModelName(schema: Schema): string[] | undefined {
// TODO: Handle more type cases
switch (schema.type) {
case "array":
return [(schema as any).items]
Expand Down
31 changes: 20 additions & 11 deletions packages/cadl-typescript/src/operationUtil.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,13 @@
// Licensed under the MIT License.

import { NameType, normalizeName } from "@azure-tools/rlc-common";
import { DecoratedType, ignoreDiagnostics, Model, Program, Type } from "@cadl-lang/compiler";
import {
DecoratedType,
ignoreDiagnostics,
Model,
Program,
Type
} from "@cadl-lang/compiler";
import {
getHttpOperation,
HttpOperation,
Expand All @@ -13,7 +19,12 @@ import {
getPagedResult,
PagedResultMetadata
} from "@azure-tools/cadl-azure-core";
import { Client, listOperationGroups, listOperationsInOperationGroup, OperationGroup } from "@azure-tools/cadl-dpg";
import {
Client,
listOperationGroups,
listOperationsInOperationGroup,
OperationGroup
} from "@azure-tools/cadl-dpg";

export function getNormalizedOperationName(
route: HttpOperation,
Expand All @@ -39,9 +50,7 @@ export function getOperationStatuscode(
}

// FIXME: this is the placeholder function to extract the operationGroupName
export function getOperationGroupName(
operationGroup?: OperationGroup,
) {
export function getOperationGroupName(operationGroup?: OperationGroup) {
return operationGroup?.type.name ?? "";
}

Expand Down Expand Up @@ -97,17 +106,17 @@ function hasDecorator(type: DecoratedType, name: string): boolean {

export function hasPollingOperations(program: Program, client: Client) {
const operationGroups = listOperationGroups(program, client);
for(const operationGroup of operationGroups) {
for (const operationGroup of operationGroups) {
const operations = listOperationsInOperationGroup(program, operationGroup);
for(const op of operations) {
for (const op of operations) {
const route = ignoreDiagnostics(getHttpOperation(program, op));
if (isLongRunningOperation(program, route)) {
return true;
}
}
}
const clientOperations = listOperationsInOperationGroup(program, client);
for(const clientOp of clientOperations) {
for (const clientOp of clientOperations) {
const route = ignoreDiagnostics(getHttpOperation(program, clientOp));
if (isLongRunningOperation(program, route)) {
return true;
Expand All @@ -129,17 +138,17 @@ export function isPagingOperation(program: Program, operation: HttpOperation) {

export function hasPagingOperations(program: Program, client: Client) {
const operationGroups = listOperationGroups(program, client);
for(const operationGroup of operationGroups) {
for (const operationGroup of operationGroups) {
const operations = listOperationsInOperationGroup(program, operationGroup);
for(const op of operations) {
for (const op of operations) {
const route = ignoreDiagnostics(getHttpOperation(program, op));
if (isPagingOperation(program, route)) {
return true;
}
}
}
const clientOperations = listOperationsInOperationGroup(program, client);
for(const clientOp of clientOperations) {
for (const clientOp of clientOperations) {
const route = ignoreDiagnostics(getHttpOperation(program, clientOp));
if (isPagingOperation(program, route)) {
return true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ export interface DurationValuePut204Response extends HttpResponse {
/** The request has succeeded. */
export interface UnknownValueGet200Response extends HttpResponse {
status: "200";
body: unknown[];
body: any[];
}

/** There is no content to send for this request, but the headers may be useful. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ export interface DurationValuePut204Response extends HttpResponse {
/** The request has succeeded. */
export interface UnknownValueGet200Response extends HttpResponse {
status: "200";
body: Record<string, unknown>;
body: Record<string, any>;
}

/** There is no content to send for this request, but the headers may be useful. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,4 @@ export interface DictionaryStringProperty {
}

/** Model with a property never. (This property should not be included). */
export interface NeverProperty {
/** Property */
property: never;
}
export interface NeverProperty {}
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,4 @@ export interface DictionaryStringPropertyOutput {
}

/** Model with a property never. (This property should not be included). */
export interface NeverPropertyOutput {
/** Property */
property: never;
}
export interface NeverPropertyOutput {}
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,10 @@ const testedTypes: TypeDetail[] = [
{
type: "dictionary/string",
defaultValue: { k1: "hello", k2: "world" }
},
{
type: "never",
defaultValue: undefined
}
];
describe("ModelsPropertyTypesClient Rest Client", () => {
Expand Down
48 changes: 34 additions & 14 deletions packages/cadl-typescript/test/unit/generate/models.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -200,16 +200,22 @@ describe("Input/output model type", () => {
await verifyPropertyType(cadlType, typeScriptType);
});

it("should handle never -> never", async () => {
const cadlType = "never";
const typeScriptType = "never";
await verifyPropertyType(cadlType, typeScriptType);
});

it("should handle unknown -> unknown", async () => {
const cadlType = "unknown";
const typeScriptType = "unknown";
await verifyPropertyType(cadlType, typeScriptType);
it("should handle never, its property will be ignored both in Input and Ouput model", async () => {
const cadlDefinition = `
model SimpleModel {
prop1: never;
prop2: never;
}`;
const cadlType = "SimpleModel";
const inputModelName = "SimpleModel";
await verifyPropertyType(cadlType, inputModelName, {
additionalCadlDefinition: cadlDefinition,
outputType: `${inputModelName}Output`,
additionalInputContent: `
export interface ${inputModelName} {}`,
additionalOutputContent: `
export interface ${inputModelName}Output {}`
});
});
});

Expand Down Expand Up @@ -269,10 +275,18 @@ describe("Input/output model type", () => {
await verifyPropertyType(cadlType, typeScriptType);
});

it("should handle unknown[] -> unknown[]", async () => {
it("should handle unknown[] -> input 'unknown[]' output type 'any[]'", async () => {
const cadlType = "unknown[]";
const typeScriptType = "unknown[]";
await verifyPropertyType(cadlType, typeScriptType);
const inputType = "unknown[]";
const outputType = "any[]";
await verifyPropertyType(cadlType, inputType, { outputType });
});

it("should handle unknown -> input 'unknown' output type 'any'", async () => {
const cadlType = "unknown";
const inputType = "unknown";
const outputType = "any";
await verifyPropertyType(cadlType, inputType, { outputType });
});
});
describe("array models generation", () => {
Expand Down Expand Up @@ -313,7 +327,7 @@ describe("Input/output model type", () => {
const inputModelName = typeScriptType;
await verifyPropertyType(cadlType, inputModelName, {
additionalCadlDefinition: cadlDefinition,
outputType: typeScriptType,
outputType: typeScriptType
});
});
});
Expand Down Expand Up @@ -640,6 +654,12 @@ describe("Input/output model type", () => {
it("should handle Record<string> -> Record<string, string>", async () => {
await verifyPropertyType("Record<string>", "Record<string, string>");
});
it("should handle Record<unknown> -> input 'Record<unknown>' output type 'Record<any>'", async () => {
const cadlType = "Record<unknown>";
const inputType = "Record<string, unknown>";
const outputType = "Record<string, any>";
await verifyPropertyType(cadlType, inputType, { outputType });
});
});

describe("Record Model generation", () => {
Expand Down
20 changes: 19 additions & 1 deletion packages/cadl-typescript/test/unit/generate/parameters.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,24 @@ describe("Parameters.ts", () => {
});

describe("Array in body generation", () => {
it("unknown array request generation", async () => {
const parameters = await emitParameterFromCadl(`
@post op read(@body body: unknown[]): void;
`);
assert.ok(parameters);
assertEqualContent(
parameters?.content!,
`
import { RequestParameters } from "@azure-rest/core-client";

export interface ReadBodyParam {
body:unknown[];
}

export type ReadParameters = ReadBodyParam & RequestParameters;
`
);
});
it("string array request generation", async () => {
const parameters = await emitParameterFromCadl(`
@post op read(@body body: string[]): void;
Expand Down Expand Up @@ -491,7 +509,7 @@ describe("Parameters.ts", () => {
prop1: string;
prop2: int32;
}
@post op read(@body body: Record<SimpleModel>): void;
@post op read(@body body: Record<SimpleModel>): SimpleModel;
`);
assert.ok(parameters);
assertEqualContent(
Expand Down
61 changes: 61 additions & 0 deletions packages/cadl-typescript/test/unit/generate/responses.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,67 @@ describe("Responses.ts", () => {
});

describe("body generation", () => {
it("unknown array response generation", async () => {
const parameters = await emitResponsesFromCadl(`
@post op read(): unknown[];
`);
assert.ok(parameters);
assertEqualContent(
parameters?.content!,
`
import { HttpResponse } from "@azure-rest/core-client";

/** The request has succeeded. */
export interface Read200Response extends HttpResponse {
status: "200";
body: any[];
}
`
);
});

it("Record<SimpleModel> response generation", async () => {
const parameters = await emitResponsesFromCadl(`
model SimpleModel {
prop1: string;
prop2: int32;
}
@post op read(@body body: SimpleModel[]): Record<SimpleModel>;
`);
assert.ok(parameters);
assertEqualContent(
parameters?.content!,
`
import { HttpResponse } from "@azure-rest/core-client";
import { SimpleModelOutput } from "./outputModels";

/** The request has succeeded. */
export interface Read200Response extends HttpResponse {
status: "200";
body: Record<string, SimpleModelOutput>;
}
`
);
});

it("should generate Record<unknown> as body property", async () => {
MaryGao marked this conversation as resolved.
Show resolved Hide resolved
const responses = await emitResponsesFromCadl(`
op read(): Record<unknown>;
`);
assert.ok(responses);
assertEqualContent(
responses!.content,
`
import { HttpResponse } from "@azure-rest/core-client";

/** The request has succeeded. */
export interface Read200Response extends HttpResponse {
status: "200";
body: Record<string, any>;
}
`
);
});
it("@header contentType not json or text should set format to binary(finally unit8array)", async () => {
const responses = await emitResponsesFromCadl(`
@get op read(): {@header contentType: "image/png", @body body: bytes};
Expand Down
9 changes: 4 additions & 5 deletions packages/rlc-common/src/buildObjectTypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -351,11 +351,10 @@ function getPropertySignatures(
importedModels: Set<string>
) {
let validProperties = Object.keys(properties);
if (schemaUsage.includes(SchemaContext.Input)) {
validProperties = validProperties.filter((p) => {
return !properties[p].readOnly;
});
}
const readOnlyFilter = (name: string) =>
!(schemaUsage.includes(SchemaContext.Input) && properties[name].readOnly);
const neverFilter = (name: string) => properties[name].type !== "never";
validProperties = validProperties.filter(readOnlyFilter).filter(neverFilter);
return validProperties.map((p) =>
getPropertySignature(
{ ...properties[p], name: p },
Expand Down
Loading