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

[tcgc] add void and never handling for parameter and return types #797

Merged
merged 2 commits into from
May 8, 2024
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
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
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
Loading