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] Fix naming logic for anonymous model wrapped by HttpPart #1491

Merged
merged 11 commits into from
Sep 13, 2024
7 changes: 7 additions & 0 deletions .chronus/changes/allow-reserved-2024-8-11-16-40-29.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
changeKind: feature
packages:
- "@azure-tools/typespec-azure-resource-manager"
---

`x-ms-skip-url-encoding` should be replaced with `allowReserved`
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
changeKind: fix
packages:
- "@azure-tools/typespec-client-generator-core"
---

Fix naming logic for anonymous model wrapped by `HttpPart`
2 changes: 1 addition & 1 deletion core
Submodule core updated 28 files
+7 −1 packages/http-client-csharp/generator/Microsoft.Generator.CSharp.ClientModel/src/ClientModelPlugin.cs
+1 −1 packages/http-client-csharp/generator/Microsoft.Generator.CSharp/src/PostProcessing/GeneratedCodeWorkspace.cs
+31 −2 packages/http-client-csharp/generator/Microsoft.Generator.CSharp/src/Primitives/CSharpType.cs
+67 −24 packages/http-client-csharp/generator/Microsoft.Generator.CSharp/src/Providers/NamedTypeSymbolProvider.cs
+20 −0 packages/http-client-csharp/generator/Microsoft.Generator.CSharp/test/Primitives/CSharpTypeTests.cs
+54 −0 ...nt-csharp/generator/Microsoft.Generator.CSharp/test/Providers/NamedTypeSymbolProviders/CompilationHelper.cs
+93 −0 ...lient-csharp/generator/Microsoft.Generator.CSharp/test/Providers/NamedTypeSymbolProviders/InterfaceTests.cs
+2 −21 ...enerator/Microsoft.Generator.CSharp/test/Providers/NamedTypeSymbolProviders/NamedTypeSymbolProviderTests.cs
+110 −0 ...-client-csharp/generator/Microsoft.Generator.CSharp/test/Providers/NamedTypeSymbolProviders/XmlDocsTests.cs
+10 −10 packages/http-client-java/generator/http-client-generator-test/package.json
+287 −0 ...ttp-client-java/generator/http-client-generator-test/src/main/java/com/cadl/naming/NamingClientBuilder.java
+273 −0 ...t-java/generator/http-client-generator-test/src/main/java/com/cadl/naming/implementation/NamingOpsImpl.java
+3 −3 ...nt-java/generator/http-client-generator-test/src/main/java/com/cadl/naming/implementation/package-info.java
+11 −0 ...ttp-client-java/generator/http-client-generator-test/src/main/java/com/cadl/naming/models/package-info.java
+11 −0 packages/http-client-java/generator/http-client-generator-test/src/main/java/com/cadl/naming/package-info.java
+6 −8 ...tor/http-client-generator-test/src/main/java/com/parameters/bodyoptionality/BodyOptionalityAsyncClient.java
+6 −8 ...enerator/http-client-generator-test/src/main/java/com/parameters/bodyoptionality/BodyOptionalityClient.java
+11 −16 ...t-generator-test/src/main/java/com/parameters/bodyoptionality/implementation/BodyOptionalityClientImpl.java
+0 −83 ...erator-test/src/main/java/com/parameters/bodyoptionality/implementation/models/RequiredImplicitRequest.java
+6 −7 ...-client-java/generator/http-client-generator-test/src/main/java/com/parameters/spread/ModelAsyncClient.java
+6 −8 .../http-client-java/generator/http-client-generator-test/src/main/java/com/parameters/spread/ModelClient.java
+11 −14 ...ava/generator/http-client-generator-test/src/main/java/com/parameters/spread/implementation/ModelsImpl.java
+0 −83 ...t-generator-test/src/main/java/com/parameters/spread/implementation/models/SpreadAsRequestBodyRequest1.java
+0 −1 ...r/http-client-generator-test/src/main/resources/META-INF/parameters-bodyoptionality_apiview_properties.json
+0 −1 .../generator/http-client-generator-test/src/main/resources/META-INF/parameters-spread_apiview_properties.json
+2 −0 packages/http-client-java/generator/http-client-generator-test/src/main/resources/cadl-naming.properties
+393 −850 packages/http-client-java/package-lock.json
+27 −27 packages/http-client-java/package.json
2 changes: 1 addition & 1 deletion packages/e2e-tests/cadl-ranch-specs/package.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"name": "@azure-tools/typespec-e2e-cadl-ranch-specs",
"dependencies": {
"@azure-tools/cadl-ranch-specs": "0.37.1"
"@azure-tools/cadl-ranch-specs": "0.37.2"
},
"type": "module",
"private": true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -537,10 +537,9 @@ model ManagementGroupNameParameter {
@added(Versions.v4)
model ScopeParameter {
/** The scope at which the operation is performed. */
@path
@path(#{ allowReserved: true })
@segment("scope")
@minLength(1)
@extension("x-ms-skip-url-encoding", true)
scope: string;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,9 +83,8 @@ model ArmLocationResource<BaseType extends
*/
@doc("The default resourceUri parameter type.")
model ResourceUriParameter {
@path
@path(#{ allowReserved: true })
@doc("The fully qualified Azure Resource manager identifier of the resource.")
@extension("x-ms-skip-url-encoding", true)
@resourceParameterBaseFor([ResourceHome.Extension])
resourceUri: string;
}
Expand Down
33 changes: 22 additions & 11 deletions packages/typespec-client-generator-core/src/public-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import {
listServices,
resolveEncodedName,
} from "@typespec/compiler";
import { HttpOperation, getHttpOperation, isMetadata } from "@typespec/http";
import { HttpOperation, getHttpOperation, getHttpPart, isMetadata } from "@typespec/http";
import { Version, getVersions } from "@typespec/versioning";
import { pascalCase } from "change-case";
import pluralize from "pluralize";
Expand Down Expand Up @@ -470,17 +470,28 @@ function getContextPath(
if (currentType === expectedType) {
result.push({ name: displayName, type: currentType });
return true;
} else if (
currentType.kind === "Model" &&
currentType.indexer &&
currentType.properties.size === 0 &&
((currentType.indexer.key.name === "string" && currentType.name === "Record") ||
currentType.indexer.key.name === "integer")
) {
// handle array or dict
const dictOrArrayItemType: Type = currentType.indexer.value;
return dfsModelProperties(expectedType, dictOrArrayItemType, pluralize.singular(displayName));
} else if (currentType.kind === "Model") {
// Peel off HttpPart<MyRealType> to get "MyRealType"
const typeWrappedByHttpPart = getHttpPart(context.program, currentType);
if (typeWrappedByHttpPart) {
return dfsModelProperties(expectedType, typeWrappedByHttpPart.type, displayName);
}

if (
currentType.indexer &&
currentType.properties.size === 0 &&
((currentType.indexer.key.name === "string" && currentType.name === "Record") ||
currentType.indexer.key.name === "integer")
) {
// handle array or dict
const dictOrArrayItemType: Type = currentType.indexer.value;
return dfsModelProperties(
expectedType,
dictOrArrayItemType,
pluralize.singular(displayName)
);
}

msyyc marked this conversation as resolved.
Show resolved Hide resolved
// handle model
result.push({ name: displayName, type: currentType });
for (const property of currentType.properties.values()) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { AzureCoreTestLibrary } from "@azure-tools/typespec-azure-core/testing";
import { ok, strictEqual } from "assert";
import { beforeEach, describe, it } from "vitest";
import { afterEach, beforeEach, describe, it } from "vitest";
import { SdkTestRunner, createSdkTestRunner } from "../test-host.js";

describe("typespec-client-generator-core: array types", () => {
Expand All @@ -9,7 +9,16 @@ describe("typespec-client-generator-core: array types", () => {
beforeEach(async () => {
runner = await createSdkTestRunner({ emitterName: "@azure-tools/typespec-java" });
});

afterEach(async () => {
for (const modelsOrEnums of [
runner.context.sdkPackage.models,
runner.context.sdkPackage.enums,
]) {
for (const item of modelsOrEnums) {
msyyc marked this conversation as resolved.
Show resolved Hide resolved
ok(item.name !== "");
}
}
});
it("use model is to represent array", async () => {
await runner.compile(`
@service({})
Expand Down Expand Up @@ -40,13 +49,13 @@ describe("typespec-client-generator-core: array types", () => {
});

it("EmbeddingVector from azure-core", async () => {
const runnerWithCore = await createSdkTestRunner({
runner = await createSdkTestRunner({
librariesToAdd: [AzureCoreTestLibrary],
autoUsings: ["Azure.Core"],
"filter-out-core-models": false,
emitterName: "@azure-tools/typespec-java",
});
await runnerWithCore.compileWithBuiltInAzureCoreService(`
await runner.compileWithBuiltInAzureCoreService(`
@service({})
namespace TestClient {
model ModelWithEmbeddingVector {
Expand All @@ -56,7 +65,7 @@ describe("typespec-client-generator-core: array types", () => {
op get(): ModelWithEmbeddingVector;
}
`);
const models = runnerWithCore.context.sdkPackage.models;
const models = runner.context.sdkPackage.models;
strictEqual(models.length, 1);
const model = models[0];
const property = model.properties[0];
Expand All @@ -67,13 +76,13 @@ describe("typespec-client-generator-core: array types", () => {
});

it("alias of EmbeddingVector", async () => {
const runnerWithCore = await createSdkTestRunner({
runner = await createSdkTestRunner({
librariesToAdd: [AzureCoreTestLibrary],
autoUsings: ["Azure.Core"],
"filter-out-core-models": false,
emitterName: "@azure-tools/typespec-java",
});
await runnerWithCore.compileWithBuiltInAzureCoreService(`
await runner.compileWithBuiltInAzureCoreService(`
@service({})
namespace TestClient {
alias MyEmbeddingVector = EmbeddingVector<int32>;
Expand All @@ -85,7 +94,7 @@ describe("typespec-client-generator-core: array types", () => {
op get(): ModelWithEmbeddingVector;
}
`);
const models = runnerWithCore.context.sdkPackage.models;
const models = runner.context.sdkPackage.models;
strictEqual(models.length, 1);
const model = models[0];
const property = model.properties[0];
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { deepStrictEqual, ok, strictEqual } from "assert";
import { beforeEach, describe, it } from "vitest";
import { afterEach, beforeEach, describe, it } from "vitest";
import { isReadOnly } from "../../src/types.js";
import { SdkTestRunner, createSdkTestRunner } from "../test-host.js";
import { getSdkBodyModelPropertyTypeHelper } from "./utils.js";
Expand All @@ -10,7 +10,16 @@ describe("typespec-client-generator-core: body model property types", () => {
beforeEach(async () => {
runner = await createSdkTestRunner({ emitterName: "@azure-tools/typespec-java" });
});

afterEach(async () => {
for (const modelsOrEnums of [
runner.context.sdkPackage.models,
runner.context.sdkPackage.enums,
]) {
for (const item of modelsOrEnums) {
ok(item.name !== "");
}
}
});
it("required", async function () {
await runner.compileWithBuiltInService(`
@usage(Usage.input | Usage.output)
Expand Down Expand Up @@ -134,12 +143,12 @@ describe("typespec-client-generator-core: body model property types", () => {
strictEqual(values[1].kind, "int32");
});
it("versioning", async function () {
const runnerWithVersion = await createSdkTestRunner({
runner = await createSdkTestRunner({
"api-version": "all",
emitterName: "@azure-tools/typespec-python",
});

await runnerWithVersion.compile(`
await runner.compile(`
@versioned(Versions)
@service({title: "Widget Service"})
namespace DemoService;
Expand All @@ -164,7 +173,7 @@ describe("typespec-client-generator-core: body model property types", () => {
removedProp: string;
}
`);
const sdkModel = runnerWithVersion.context.sdkPackage.models.find((x) => x.kind === "model");
const sdkModel = runner.context.sdkPackage.models.find((x) => x.kind === "model");
ok(sdkModel);
strictEqual(sdkModel.kind, "model");

Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { AzureCoreTestLibrary } from "@azure-tools/typespec-azure-core/testing";
import { expectDiagnostics } from "@typespec/compiler/testing";
import { ok, strictEqual } from "assert";
import { beforeEach, describe, it } from "vitest";
import { afterEach, beforeEach, describe, it } from "vitest";
import { SdkBuiltInType } from "../../src/interfaces.js";
import { getAllModels } from "../../src/types.js";
import { SdkTestRunner, createSdkTestRunner } from "../test-host.js";
Expand All @@ -13,7 +13,16 @@ describe("typespec-client-generator-core: built-in types", () => {
beforeEach(async () => {
runner = await createSdkTestRunner({ emitterName: "@azure-tools/typespec-java" });
});

afterEach(async () => {
for (const modelsOrEnums of [
runner.context.sdkPackage.models,
runner.context.sdkPackage.enums,
]) {
for (const item of modelsOrEnums) {
ok(item.name !== "");
}
}
});
it("string", async function () {
await runner.compileWithBuiltInService(
`
Expand Down Expand Up @@ -204,12 +213,12 @@ describe("typespec-client-generator-core: built-in types", () => {
});

it("armId from Core", async function () {
const runnerWithCore = await createSdkTestRunner({
runner = await createSdkTestRunner({
librariesToAdd: [AzureCoreTestLibrary],
autoUsings: ["Azure.Core"],
emitterName: "@azure-tools/typespec-java",
});
await runnerWithCore.compileWithBuiltInAzureCoreService(
await runner.compileWithBuiltInAzureCoreService(
`
@usage(Usage.input | Usage.output)
model Test {
Expand All @@ -221,7 +230,7 @@ describe("typespec-client-generator-core: built-in types", () => {
}
`
);
const models = runnerWithCore.context.sdkPackage.models;
const models = runner.context.sdkPackage.models;
const type = models[0].properties[0].type;
strictEqual(type.kind, "string");
strictEqual(type.name, "armResourceIdentifier");
Expand All @@ -230,12 +239,12 @@ describe("typespec-client-generator-core: built-in types", () => {
});

it("format", async function () {
const runnerWithCore = await createSdkTestRunner({
runner = await createSdkTestRunner({
librariesToAdd: [AzureCoreTestLibrary],
autoUsings: ["Azure.Core"],
emitterName: "@azure-tools/typespec-java",
});
await runnerWithCore.compileWithBuiltInAzureCoreService(
await runner.compileWithBuiltInAzureCoreService(
`
@usage(Usage.input | Usage.output)
model Test {
Expand All @@ -246,7 +255,7 @@ describe("typespec-client-generator-core: built-in types", () => {
}
`
);
const models = runnerWithCore.context.sdkPackage.models;
const models = runner.context.sdkPackage.models;
for (const property of models[0].properties) {
strictEqual(property.kind, "property");
strictEqual(
Expand All @@ -257,13 +266,13 @@ describe("typespec-client-generator-core: built-in types", () => {
});

it("etag from core", async () => {
const runnerWithCore = await createSdkTestRunner({
runner = await createSdkTestRunner({
librariesToAdd: [AzureCoreTestLibrary],
autoUsings: ["Azure.Core"],
"filter-out-core-models": false,
emitterName: "@azure-tools/typespec-java",
});
await runnerWithCore.compileWithBuiltInAzureCoreService(`
await runner.compileWithBuiltInAzureCoreService(`
@resource("users")
@doc("Details about a user.")
model User {
Expand All @@ -278,7 +287,7 @@ describe("typespec-client-generator-core: built-in types", () => {
@doc("Gets status.")
op getStatus is GetResourceOperationStatus<User>;
`);
const userModel = runnerWithCore.context.sdkPackage.models.find(
const userModel = runner.context.sdkPackage.models.find(
(x) => x.kind === "model" && x.name === "User"
);
ok(userModel);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { ok, strictEqual } from "assert";
import { beforeEach, describe, it } from "vitest";
import { afterEach, beforeEach, describe, it } from "vitest";
import { SdkBasicServiceMethod, SdkBuiltInType, SdkHttpOperation } from "../../src/interfaces.js";
import { SdkTestRunner, createSdkTestRunner } from "../test-host.js";

Expand All @@ -9,7 +9,16 @@ describe("typespec-client-generator-core: bytes types", () => {
beforeEach(async () => {
runner = await createSdkTestRunner({ emitterName: "@azure-tools/typespec-java" });
});

afterEach(async () => {
for (const modelsOrEnums of [
runner.context.sdkPackage.models,
runner.context.sdkPackage.enums,
]) {
for (const item of modelsOrEnums) {
ok(item.name !== "");
}
}
});
describe("bytes SdkMethodParameter", () => {
it("should use service operation parameter encoding", async () => {
await runner.compile(`
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { strictEqual } from "assert";
import { beforeEach, describe, it } from "vitest";
import { ok, strictEqual } from "assert";
import { afterEach, beforeEach, describe, it } from "vitest";
import { SdkTestRunner, createSdkTestRunner } from "../test-host.js";
import { getSdkTypeHelper } from "./utils.js";

Expand All @@ -9,7 +9,16 @@ describe("typespec-client-generator-core: constant types", () => {
beforeEach(async () => {
runner = await createSdkTestRunner({ emitterName: "@azure-tools/typespec-java" });
});

afterEach(async () => {
for (const modelsOrEnums of [
runner.context.sdkPackage.models,
runner.context.sdkPackage.enums,
]) {
for (const item of modelsOrEnums) {
ok(item.name !== "");
}
}
});
it("string", async function () {
await runner.compileWithBuiltInService(`
@usage(Usage.input | Usage.output)
Expand Down
Loading
Loading