Skip to content

Commit

Permalink
[tcgc] add void and never handling for parameter and return types (#797)
Browse files Browse the repository at this point in the history
resolve: #795
  • Loading branch information
tadelesh authored May 8, 2024
1 parent d598d3e commit b87ace0
Show file tree
Hide file tree
Showing 7 changed files with 83 additions and 17 deletions.
7 changes: 7 additions & 0 deletions .chronus/changes/fix_void-2024-4-8-13-39-22.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
changeKind: fix
packages:
- "@azure-tools/typespec-client-generator-core"
---

add void and never handling for parameter and return types
29 changes: 17 additions & 12 deletions packages/typespec-client-generator-core/src/http.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ import {
getLocationOfOperation,
isAcceptHeader,
isContentTypeHeader,
isNeverOrVoidType,
isNullable,
isSubscriptionId,
} from "./internal-utils.js";
Expand Down Expand Up @@ -108,6 +109,7 @@ function getSdkHttpParameters(
bodyParam: undefined,
};
retval.parameters = httpOperation.parameters.parameters
.filter((x) => !isNeverOrVoidType(x.param.type))
.map((x) =>
diagnostics.pipe(getSdkHttpParameter(context, x.param, httpOperation.operation, x.type))
)
Expand All @@ -124,7 +126,7 @@ function getSdkHttpParameters(
const correspondingMethodParams: SdkModelPropertyType[] = [];
if (tspBody) {
// if there's a param on the body, we can just rely on getSdkHttpParameter
if (tspBody.parameter) {
if (tspBody.parameter && !isNeverOrVoidType(tspBody.parameter.type)) {
const getParamResponse = diagnostics.pipe(
getSdkHttpParameter(context, tspBody.parameter, httpOperation.operation, "body")
);
Expand All @@ -143,7 +145,7 @@ function getSdkHttpParameters(
return diagnostics.wrap(retval);
}
retval.bodyParam = getParamResponse;
} else {
} else if (!isNeverOrVoidType(tspBody.type)) {
const type = diagnostics.pipe(
getClientTypeWithDiagnostics(context, tspBody.type, httpOperation.operation)
);
Expand All @@ -170,15 +172,17 @@ function getSdkHttpParameters(
correspondingMethodParams,
};
}
addContentTypeInfoToBodyParam(context, httpOperation, retval.bodyParam);
retval.bodyParam.correspondingMethodParams = diagnostics.pipe(
getCorrespondingMethodParams(
context,
httpOperation.operation,
methodParameters,
retval.bodyParam
)
);
if (retval.bodyParam) {
addContentTypeInfoToBodyParam(context, httpOperation, retval.bodyParam);
retval.bodyParam.correspondingMethodParams = diagnostics.pipe(
getCorrespondingMethodParams(
context,
httpOperation.operation,
methodParameters,
retval.bodyParam
)
);
}
}
if (retval.bodyParam && !headerParams.some((h) => isContentTypeHeader(h))) {
// if we have a body param and no content type header, we add one
Expand Down Expand Up @@ -380,6 +384,7 @@ function getSdkHttpResponseAndExceptions(

for (const innerResponse of response.responses) {
for (const header of Object.values(innerResponse.headers || [])) {
if (isNeverOrVoidType(header.type)) continue;
const clientType = diagnostics.pipe(getClientTypeWithDiagnostics(context, header.type));
const defaultContentType = innerResponse.body?.contentTypes.includes("application/json")
? "application/json"
Expand All @@ -395,7 +400,7 @@ function getSdkHttpResponseAndExceptions(
nullable: isNullable(header.type),
});
}
if (innerResponse.body) {
if (innerResponse.body && !isNeverOrVoidType(innerResponse.body.type)) {
if (body && body !== innerResponse.body.type) {
diagnostics.add(
createDiagnostic({
Expand Down
6 changes: 6 additions & 0 deletions packages/typespec-client-generator-core/src/internal-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@ import {
getNamespaceFullName,
getSummary,
ignoreDiagnostics,
isNeverType,
isNullType,
isVoidType,
} from "@typespec/compiler";
import { HttpOperation, HttpStatusCodeRange } from "@typespec/http";
import { getAddedOnVersions, getRemovedOnVersions, getVersions } from "@typespec/versioning";
Expand Down Expand Up @@ -407,3 +409,7 @@ export function getLocationOfOperation(operation: Operation): Namespace | Interf
// have to check interface first, because interfaces are more granular than namespaces
return (operation.interface || operation.namespace)!;
}

export function isNeverOrVoidType(type: Type): boolean {
return isNeverType(type) || isVoidType(type);
}
6 changes: 4 additions & 2 deletions packages/typespec-client-generator-core/src/package.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ import {
getHashForType,
getLocationOfOperation,
getSdkTypeBaseHelper,
isNeverOrVoidType,
isNullable,
updateWithApiVersionInformation,
} from "./internal-utils.js";
Expand Down Expand Up @@ -241,14 +242,15 @@ function getSdkBasicServiceMethod<
const parameters = httpOperation.parameters;
// path/query/header parameters
for (const param of parameters.parameters) {
if (isNeverOrVoidType(param.param.type)) continue;
methodParameters.push(diagnostics.pipe(getSdkMethodParameter(context, param.param, operation)));
}
// body parameters
if (parameters.body?.parameter) {
if (parameters.body?.parameter && !isNeverOrVoidType(parameters.body.parameter.type)) {
methodParameters.push(
diagnostics.pipe(getSdkMethodParameter(context, parameters.body?.parameter, operation))
);
} else if (parameters.body) {
} else if (parameters.body && !isNeverOrVoidType(parameters.body.type)) {
if (parameters.body.type.kind === "Model") {
const type = getEffectivePayloadType(context, parameters.body.type);
// spread case
Expand Down
9 changes: 6 additions & 3 deletions packages/typespec-client-generator-core/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ import {
isAzureCoreModel,
isMultipartFormData,
isMultipartOperation,
isNeverOrVoidType,
isNullable,
updateWithApiVersionInformation,
} from "./internal-utils.js";
Expand Down Expand Up @@ -1044,7 +1045,7 @@ function addPropertiesToModelType(
for (const property of type.properties.values()) {
if (
isStatusCode(context.program, property) ||
isNeverType(property.type) ||
isNeverOrVoidType(property.type) ||
sdkType.kind !== "model"
) {
continue;
Expand Down Expand Up @@ -1240,6 +1241,7 @@ function updateTypesFromOperation(
const httpOperation = getHttpOperationWithCache(context, operation);
const generateConvenient = shouldGenerateConvenient(context, operation);
for (const param of operation.parameters.properties.values()) {
if (isNeverOrVoidType(param.type)) continue;
const paramTypes = diagnostics.pipe(checkAndGetClientType(context, param.type, operation));
if (generateConvenient) {
paramTypes.forEach((paramType) => {
Expand All @@ -1248,6 +1250,7 @@ function updateTypesFromOperation(
}
}
for (const param of httpOperation.parameters.parameters) {
if (isNeverOrVoidType(param.param.type)) continue;
const paramTypes = diagnostics.pipe(
checkAndGetClientType(context, param.param.type, operation)
);
Expand All @@ -1258,7 +1261,7 @@ function updateTypesFromOperation(
}
}
const httpBody = httpOperation.parameters.body;
if (httpBody) {
if (httpBody && !isNeverOrVoidType(httpBody.type)) {
const bodies = diagnostics.pipe(checkAndGetClientType(context, httpBody.type, operation));
if (generateConvenient) {
bodies.forEach((body) => {
Expand All @@ -1282,7 +1285,7 @@ function updateTypesFromOperation(
}
for (const response of httpOperation.responses) {
for (const innerResponse of response.responses) {
if (innerResponse.body?.type) {
if (innerResponse.body?.type && !isNeverOrVoidType(innerResponse.body.type)) {
const responseBodies = diagnostics.pipe(
checkAndGetClientType(context, innerResponse.body.type, operation)
);
Expand Down
23 changes: 23 additions & 0 deletions packages/typespec-client-generator-core/test/package.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1679,6 +1679,29 @@ describe("typespec-client-generator-core: package", () => {
["apiVersion", "generationOptions", "contentType", "accept"]
);
});

it("never void parameter or response", async () => {
await runner.compileWithBuiltInService(`
op TestTemplate<
headerType,
queryType,
bodyType,
responseHeaderType,
responseBodyType
>(@header h: headerType, @query q: queryType, @body b: bodyType): {
@header h: responseHeaderType;
@body b: responseBodyType;
};
op test is TestTemplate<void, void, void, void, void>;
`);
const sdkPackage = runner.context.experimental_sdkPackage;
const method = getServiceMethodOfClient(sdkPackage);
strictEqual(method.parameters.length, 0);
strictEqual(method.response.type, undefined);
strictEqual(method.operation.parameters.length, 0);
strictEqual(method.operation.responses.get(200)?.headers.length, 0);
strictEqual(method.operation.responses.get(200)?.type, undefined);
});
});

describe("Responses", () => {
Expand Down
20 changes: 20 additions & 0 deletions packages/typespec-client-generator-core/test/types.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3282,6 +3282,26 @@ describe("typespec-client-generator-core: types", () => {
["ValidResponse"]
);
});

it("never or void property", async () => {
await runner.compileAndDiagnose(`
@service({})
@test namespace MyService {
@test
@usage(Usage.input | Usage.output)
@access(Access.public)
model Test{
prop1: never;
prop2: void;
}
}
`);

const models = runner.context.experimental_sdkPackage.models;
strictEqual(models.length, 1);
strictEqual(models[0].name, "Test");
strictEqual(models[0].properties.length, 0);
});
});

describe("SdkArrayType", () => {
Expand Down

0 comments on commit b87ace0

Please sign in to comment.