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.

Sign up for GitHub

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

Jump to bottom

[tcgc] make artifical content type parameter optional #425

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

[tcgc] make artifical content type parameter optional #425

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
17a60ba
make manual addition of content type and accept header optional
Mar 14, 2024
88e5cb1
add changeset
Mar 14, 2024
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/content_type_optional-2024-2-14-13-57-49.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
changeKind: fix
packages:
- "@azure-tools/typespec-client-generator-core"
---

when tcgc creates a content type when not defined in tsp, we want to set it as optional
4 changes: 3 additions & 1 deletion packages/typespec-client-generator-core/src/package.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,13 +175,15 @@ function createContentTypeOrAcceptHeader(
};
}
// No need for clientDefaultValue because it's a constant, it only has one value
// Since these are the created accept and content type header parameters, we don't make them required.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the motivation for this change? Since these are synthesized, constant, params, they won't show up in public surface area so there's no way for callers to set them. Maybe I'm missing something.

// If they are defined as required, we won't enter this code and they will be generated as required
return {
type,
nameInClient,
apiVersions: bodyObject.apiVersions,
isApiVersionParam: false,
onClient: false,
optional: false,
optional: true,
};
}

Expand Down
32 changes: 16 additions & 16 deletions packages/typespec-client-generator-core/test/package.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1013,7 +1013,7 @@ describe("typespec-client-generator-core: package", () => {
strictEqual(methodContentTypeParam.clientDefaultValue, undefined);
strictEqual(methodContentTypeParam.type.kind, "constant");
strictEqual(methodContentTypeParam.onClient, false);
strictEqual(methodContentTypeParam.optional, false);
strictEqual(methodContentTypeParam.optional, true);

const serviceOperation = method.operation;
strictEqual(serviceOperation.bodyParams.length, 1);
Expand All @@ -1035,7 +1035,7 @@ describe("typespec-client-generator-core: package", () => {
strictEqual(contentTypeParam.serializedName, "Content-Type");
strictEqual(contentTypeParam.clientDefaultValue, undefined);
strictEqual(contentTypeParam.onClient, false);
strictEqual(contentTypeParam.optional, false);
strictEqual(contentTypeParam.optional, true);

const correspondingContentTypeMethodParams = method.getParameterMapping(contentTypeParam);
strictEqual(correspondingContentTypeMethodParams.length, 1);
Expand Down Expand Up @@ -1074,7 +1074,7 @@ describe("typespec-client-generator-core: package", () => {
strictEqual(methodContentTypeParam.clientDefaultValue, undefined);
strictEqual(methodContentTypeParam.type.kind, "constant");
strictEqual(methodContentTypeParam.onClient, false);
strictEqual(methodContentTypeParam.optional, false);
strictEqual(methodContentTypeParam.optional, true);

const serviceOperation = method.operation;
strictEqual(serviceOperation.bodyParams.length, 1);
Expand All @@ -1096,7 +1096,7 @@ describe("typespec-client-generator-core: package", () => {
strictEqual(contentTypeParam.serializedName, "Content-Type");
strictEqual(contentTypeParam.clientDefaultValue, undefined);
strictEqual(contentTypeParam.onClient, false);
strictEqual(contentTypeParam.optional, false);
strictEqual(contentTypeParam.optional, true);

const correspondingContentTypeMethodParams = method.getParameterMapping(contentTypeParam);
strictEqual(correspondingContentTypeMethodParams.length, 1);
Expand Down Expand Up @@ -1267,7 +1267,7 @@ describe("typespec-client-generator-core: package", () => {
methodParam = method.parameters[1];
strictEqual(methodParam.kind, "method");
strictEqual(methodParam.nameInClient, "contentType");
strictEqual(methodParam.optional, false);
strictEqual(methodParam.optional, true);
strictEqual(methodParam.onClient, false);
strictEqual(methodParam.isApiVersionParam, false);
strictEqual(methodParam.type.kind, "constant");
Expand Down Expand Up @@ -1322,7 +1322,7 @@ describe("typespec-client-generator-core: package", () => {
methodParam = method.parameters[1];
strictEqual(methodParam.kind, "method");
strictEqual(methodParam.nameInClient, "contentType");
strictEqual(methodParam.optional, false);
strictEqual(methodParam.optional, true);
strictEqual(methodParam.onClient, false);
strictEqual(methodParam.isApiVersionParam, false);
strictEqual(methodParam.type.kind, "constant");
Expand Down Expand Up @@ -1672,14 +1672,14 @@ describe("typespec-client-generator-core: package", () => {
)!;
strictEqual(contentTypeOperationParam.clientDefaultValue, undefined);
strictEqual(contentTypeOperationParam.onClient, false);
strictEqual(contentTypeOperationParam.optional, false);
strictEqual(contentTypeOperationParam.optional, true);

const contentTypeMethodParam = method.parameters.find(
(x) => x.nameInClient === "contentType"
)!;
strictEqual(contentTypeMethodParam.clientDefaultValue, undefined);
strictEqual(contentTypeMethodParam.onClient, false);
strictEqual(contentTypeMethodParam.optional, false);
strictEqual(contentTypeMethodParam.optional, true);

strictEqual(method.getParameterMapping(contentTypeOperationParam)[0], contentTypeMethodParam);

Expand Down Expand Up @@ -1775,11 +1775,11 @@ describe("typespec-client-generator-core: package", () => {
(x) => x.nameInClient === "contentType"
)!;
strictEqual(methodContentTypeParam.clientDefaultValue, undefined);
strictEqual(methodContentTypeParam.optional, false);
strictEqual(methodContentTypeParam.optional, true);

const methodAcceptParam = method.parameters.find((x) => x.nameInClient === "accept")!;
strictEqual(methodAcceptParam.clientDefaultValue, undefined);
strictEqual(methodAcceptParam.optional, false);
strictEqual(methodAcceptParam.optional, true);

const serviceOperation = method.operation;

Expand Down Expand Up @@ -1810,11 +1810,11 @@ describe("typespec-client-generator-core: package", () => {
(x) => x.serializedName === "Content-Type"
)!;
strictEqual(operationContentTypeParam.clientDefaultValue, undefined);
strictEqual(operationContentTypeParam.optional, false);
strictEqual(operationContentTypeParam.optional, true);

const operationAcceptParam = headerParams.find((x) => x.serializedName === "Accept")!;
strictEqual(operationAcceptParam.clientDefaultValue, undefined);
strictEqual(operationAcceptParam.optional, false);
strictEqual(operationAcceptParam.optional, true);

const correspondingMethodParams = method
.getParameterMapping(bodyParameter)
Expand Down Expand Up @@ -2111,7 +2111,7 @@ describe("typespec-client-generator-core: package", () => {
const methodAcceptParam = method.parameters.find((x) => x.nameInClient === "accept")!;
strictEqual(methodAcceptParam.clientDefaultValue, undefined);
strictEqual(methodAcceptParam.onClient, false);
strictEqual(methodAcceptParam.optional, false);
strictEqual(methodAcceptParam.optional, true);

const headerParams = method.operation.parameters.filter(
(x): x is SdkHeaderParameter => x.kind === "header"
Expand All @@ -2121,7 +2121,7 @@ describe("typespec-client-generator-core: package", () => {
const operationAcceptParam = headerParams.find((x) => x.serializedName === "Accept")!;
strictEqual(operationAcceptParam.clientDefaultValue, undefined);
strictEqual(operationAcceptParam.onClient, false);
strictEqual(operationAcceptParam.optional, false);
strictEqual(operationAcceptParam.optional, true);
strictEqual(method.getParameterMapping(operationAcceptParam)[0], methodAcceptParam);

strictEqual(
Expand Down Expand Up @@ -2179,7 +2179,7 @@ describe("typespec-client-generator-core: package", () => {
const methodAcceptParam = getStatus.parameters.find((x) => x.nameInClient === "accept")!;
strictEqual(methodAcceptParam.clientDefaultValue, undefined);
strictEqual(methodAcceptParam.onClient, false);
strictEqual(methodAcceptParam.optional, false);
strictEqual(methodAcceptParam.optional, true);

strictEqual(getStatus.operation.parameters.length, 4);

Expand Down Expand Up @@ -2217,7 +2217,7 @@ describe("typespec-client-generator-core: package", () => {
strictEqual(operationAcceptParam.nameInClient, "accept");
strictEqual(operationAcceptParam.clientDefaultValue, undefined);
strictEqual(operationAcceptParam.onClient, false);
strictEqual(operationAcceptParam.optional, false);
strictEqual(operationAcceptParam.optional, true);
strictEqual(getStatus.getParameterMapping(operationAcceptParam)[0], methodAcceptParam);

const widgetModel = sdkPackage.models.find((x) => x.name === "Widget")!;
Expand Down
Loading