From b23548f3d521d5e37efffe0642b5f590ad26c3d9 Mon Sep 17 00:00:00 2001 From: iscai-msft Date: Tue, 28 May 2024 12:21:28 -0400 Subject: [PATCH 1/4] default access to public --- .../src/decorators.ts | 6 +- .../src/interfaces.ts | 6 +- .../src/types.ts | 8 +- .../test/decorators.test.ts | 172 ++++++++---------- .../test/types.test.ts | 4 +- 5 files changed, 89 insertions(+), 107 deletions(-) diff --git a/packages/typespec-client-generator-core/src/decorators.ts b/packages/typespec-client-generator-core/src/decorators.ts index a0583d8b4f..20886a50c5 100644 --- a/packages/typespec-client-generator-core/src/decorators.ts +++ b/packages/typespec-client-generator-core/src/decorators.ts @@ -892,10 +892,10 @@ export function getAccessOverride( export function getAccess( context: TCGCContext, entity: Model | Enum | Operation | Union -): AccessFlags | undefined { +): AccessFlags { const override = getScopedDecoratorData(context, accessKey, entity); if (override || entity.kind === "Operation") { - return override; + return override || "public"; } switch (entity.kind) { @@ -908,7 +908,7 @@ export function getAccess( if (type.kind === "enum" || type.kind === "model") { return type.access; } - return undefined; + return "public"; } } diff --git a/packages/typespec-client-generator-core/src/interfaces.ts b/packages/typespec-client-generator-core/src/interfaces.ts index d6f2550405..8e211f1809 100644 --- a/packages/typespec-client-generator-core/src/interfaces.ts +++ b/packages/typespec-client-generator-core/src/interfaces.ts @@ -254,7 +254,7 @@ export interface SdkEnumType extends SdkTypeBase { isFixed: boolean; isFlags: boolean; usage: UsageFlags; - access?: AccessFlags; + access: AccessFlags; crossLanguageDefinitionId: string; apiVersions: string[]; isUnionAsEnum: boolean; @@ -297,7 +297,7 @@ export interface SdkModelType extends SdkTypeBase { */ isError: boolean; isGeneratedName: boolean; - access?: AccessFlags; + access: AccessFlags; usage: UsageFlags; additionalProperties?: SdkType; additionalPropertiesNullable?: boolean; @@ -470,7 +470,7 @@ export type SdkServiceParameter = SdkHttpParameter; interface SdkMethodBase { __raw?: Operation; name: string; - access: AccessFlags | undefined; + access: AccessFlags; parameters: SdkParameter[]; apiVersions: string[]; description?: string; diff --git a/packages/typespec-client-generator-core/src/types.ts b/packages/typespec-client-generator-core/src/types.ts index 34e2bdc55e..c3aacb85be 100644 --- a/packages/typespec-client-generator-core/src/types.ts +++ b/packages/typespec-client-generator-core/src/types.ts @@ -557,7 +557,7 @@ export function getSdkModelWithDiagnostics( details: docWrapper.details, properties: [], additionalProperties: undefined, // going to set additional properties in the next few lines when we look at base model - access: undefined, // dummy value since we need to update models map before we can set this + access: "public", // dummy value since we need to update models map before we can set this usage: UsageFlags.None, // dummy value since we need to update models map before we can set this crossLanguageDefinitionId: getCrossLanguageDefinitionId(type, name), apiVersions: getAvailableApiVersions(context, type, type.namespace), @@ -683,7 +683,7 @@ export function getSdkEnum(context: TCGCContext, type: Enum, operation?: Operati isFixed: true, // enums are always fixed after we switch to use union to represent extensible enum isFlags: false, usage: UsageFlags.None, // We will add usage as we loop through the operations - access: undefined, // Dummy value until we update models map + access: "public", // Dummy value until we update models map crossLanguageDefinitionId: getCrossLanguageDefinitionId(type), apiVersions: getAvailableApiVersions(context, type, type.namespace), isUnionAsEnum: false, @@ -739,7 +739,7 @@ function getSdkUnionEnum(context: TCGCContext, type: UnionEnum, operation?: Oper isFixed: !type.open, isFlags: false, usage: UsageFlags.None, // We will add usage as we loop through the operations - access: undefined, // Dummy value until we update models map + access: "public", // Dummy value until we update models map crossLanguageDefinitionId: getCrossLanguageDefinitionId(union, name), apiVersions: getAvailableApiVersions(context, type.union, type.union.namespace), isUnionAsEnum: true, @@ -777,7 +777,7 @@ function getKnownValuesEnum( isFixed: false, isFlags: false, usage: UsageFlags.None, // We will add usage as we loop through the operations - access: undefined, // Dummy value until we update models map + access: "public", // Dummy value until we update models map crossLanguageDefinitionId: getCrossLanguageDefinitionId(type), apiVersions: getAvailableApiVersions(context, type, type.namespace), isUnionAsEnum: false, diff --git a/packages/typespec-client-generator-core/test/decorators.test.ts b/packages/typespec-client-generator-core/test/decorators.test.ts index f864be7928..0b340b1ad0 100644 --- a/packages/typespec-client-generator-core/test/decorators.test.ts +++ b/packages/typespec-client-generator-core/test/decorators.test.ts @@ -1433,17 +1433,22 @@ describe("typespec-client-generator-core: decorators", () => { }); it("emitter different scope from decorator", async () => { - const { func } = (await runner.compile(` - @test - @access(Access.internal, "csharp") - op func( - @query("createdAt") - createdAt: utcDateTime; - ): void; - `)) as { func: Operation }; + const code = ` + @test + @access(Access.internal, "csharp") + op func( + @query("createdAt") + createdAt: utcDateTime; + ): void; + `; + const { func } = (await runner.compile(code)) as { func: Operation }; + strictEqual(getAccess(runner.context, func), "public"); - const actual = getAccess(runner.context, func); - strictEqual(actual, undefined); + const runnerWithCsharp = await createSdkTestRunner({ + emitterName: "@azure-tools/typespec-csharp", + }); + const { func: funcCsharp } = (await runnerWithCsharp.compile(code)) as { func: Operation }; + strictEqual(getAccess(runnerWithCsharp.context, funcCsharp), "internal"); }); it("emitter first in decorator scope list", async () => { @@ -1479,18 +1484,23 @@ describe("typespec-client-generator-core: decorators", () => { }); it("emitter excluded from decorator scope list", async () => { - const { func } = (await runner.compile(` - @test - @access(Access.internal, "java") - @access(Access.internal, "csharp") - op func( - @query("createdAt") - createdAt: utcDateTime; - ): void; - `)) as { func: Operation }; + const code = ` + @test + @access(Access.internal, "java") + @access(Access.internal, "csharp") + op func( + @query("createdAt") + createdAt: utcDateTime; + ): void; + `; + const { func } = (await runner.compile(code)) as { func: Operation }; - const actual = getAccess(runner.context, func); - strictEqual(actual, undefined); + strictEqual(getAccess(runner.context, func), "public"); + const runnerWithJava = await createSdkTestRunner({ + emitterName: "@azure-tools/typespec-java", + }); + const { func: funcJava } = (await runnerWithJava.compile(code)) as { func: Operation }; + strictEqual(getAccess(runnerWithJava.context, funcJava), "internal"); }); it("duplicate-decorator diagnostic for first non-scoped decorator then scoped decorator", async () => { @@ -1563,10 +1573,8 @@ describe("typespec-client-generator-core: decorators", () => { op test(): void; `)) as { test: Operation; Test: Model }; - let actual = getAccess(runner.context, test); - strictEqual(actual, undefined); - actual = getAccess(runner.context, Test); - strictEqual(actual, undefined); + strictEqual(getAccess(runner.context, test), "public"); + strictEqual(getAccess(runner.context, Test), "public"); }); it("model access calculated by operation", async () => { @@ -1630,10 +1638,8 @@ describe("typespec-client-generator-core: decorators", () => { } `)) as { Test: Model; func: Operation }; - let actual = getAccess(runner.context, Test); - strictEqual(actual, "internal"); - actual = getAccess(runner.context, func); - strictEqual(actual, undefined); + strictEqual(getAccess(runner.context, Test), "internal"); + strictEqual(getAccess(runner.context, func), "public"); }); it("access propagation", async () => { @@ -1762,29 +1768,18 @@ describe("typespec-client-generator-core: decorators", () => { func5: Operation; }; - const func1Actual = getAccess(runner.context, func1); - strictEqual(func1Actual, "internal"); - const func2Actual = getAccess(runner.context, func2); - strictEqual(func2Actual, "internal"); - const func3Actual = getAccess(runner.context, func3); - strictEqual(func3Actual, undefined); - const func4Actual = getAccess(runner.context, func4); - strictEqual(func4Actual, "internal"); - const func5Actual = getAccess(runner.context, func5); - strictEqual(func5Actual, undefined); - - const test1Actual = getAccess(runner.context, Test1); - strictEqual(test1Actual, "internal"); - const test2Actual = getAccess(runner.context, Test2); - strictEqual(test2Actual, "internal"); - const test3Actual = getAccess(runner.context, Test3); - strictEqual(test3Actual, undefined); - const test4Actual = getAccess(runner.context, Test4); - strictEqual(test4Actual, "internal"); - const test5Actual = getAccess(runner.context, Test5); - strictEqual(test5Actual, "internal"); - const test6Actual = getAccess(runner.context, Test6); - strictEqual(test6Actual, undefined); + strictEqual(getAccess(runner.context, func1), "internal"); + strictEqual(getAccess(runner.context, func2), "internal"); + strictEqual(getAccess(runner.context, func3), "public"); + strictEqual(getAccess(runner.context, func4), "internal"); + strictEqual(getAccess(runner.context, func5), "public"); + + strictEqual(getAccess(runner.context, Test1), "internal"); + strictEqual(getAccess(runner.context, Test2), "internal"); + strictEqual(getAccess(runner.context, Test3), "public"); + strictEqual(getAccess(runner.context, Test4), "internal"); + strictEqual(getAccess(runner.context, Test5), "internal"); + strictEqual(getAccess(runner.context, Test6), "public"); }); it("access propagation for properties, base models and sub models", async () => { @@ -1915,21 +1910,21 @@ describe("typespec-client-generator-core: decorators", () => { }; strictEqual(getAccess(runner.context, func1), "internal"); - strictEqual(getAccess(runner.context, func2), undefined); + strictEqual(getAccess(runner.context, func2), "public"); strictEqual(getAccess(runner.context, func3), "internal"); - strictEqual(getAccess(runner.context, func4), undefined); - - strictEqual(getAccess(runner.context, Fish), undefined); - strictEqual(getAccess(runner.context, Salmon), undefined); - strictEqual(getAccess(runner.context, Origin), undefined); - strictEqual(getAccess(runner.context, BaseModel), undefined); - strictEqual(getAccess(runner.context, ModelA), undefined); - strictEqual(getAccess(runner.context, ModelB), undefined); - strictEqual(getAccess(runner.context, ModelC), undefined); - strictEqual(getAccess(runner.context, ModelD), undefined); - strictEqual(getAccess(runner.context, ModelE), undefined); - strictEqual(getAccess(runner.context, ModelF), undefined); - strictEqual(getAccess(runner.context, EnumA), undefined); + strictEqual(getAccess(runner.context, func4), "public"); + + strictEqual(getAccess(runner.context, Fish), "public"); + strictEqual(getAccess(runner.context, Salmon), "public"); + strictEqual(getAccess(runner.context, Origin), "public"); + strictEqual(getAccess(runner.context, BaseModel), "public"); + strictEqual(getAccess(runner.context, ModelA), "public"); + strictEqual(getAccess(runner.context, ModelB), "public"); + strictEqual(getAccess(runner.context, ModelC), "public"); + strictEqual(getAccess(runner.context, ModelD), "public"); + strictEqual(getAccess(runner.context, ModelE), "public"); + strictEqual(getAccess(runner.context, ModelF), "public"); + strictEqual(getAccess(runner.context, EnumA), "public"); strictEqual(runner.context.operationModelsMap?.get(func1)?.size, 3); strictEqual(runner.context.operationModelsMap?.get(func2)?.size, 3); @@ -2037,33 +2032,20 @@ describe("typespec-client-generator-core: decorators", () => { func8: Operation; }; - const func1Actual = getAccess(runner.context, func1); - strictEqual(func1Actual, "internal"); - const func2Actual = getAccess(runner.context, func2); - strictEqual(func2Actual, undefined); - const func3Actual = getAccess(runner.context, func3); - strictEqual(func3Actual, "public"); - const func4Actual = getAccess(runner.context, func4); - strictEqual(func4Actual, "internal"); - const func5Actual = getAccess(runner.context, func5); - strictEqual(func5Actual, undefined); - const func6Actual = getAccess(runner.context, func6); - strictEqual(func6Actual, "internal"); - const func7Actual = getAccess(runner.context, func7); - strictEqual(func7Actual, undefined); - const func8Actual = getAccess(runner.context, func8); - strictEqual(func8Actual, "public"); - - const test1Actual = getAccess(runner.context, Test1); - strictEqual(test1Actual, "internal"); - const test2Actual = getAccess(runner.context, Test2); - strictEqual(test2Actual, undefined); - const test3Actual = getAccess(runner.context, Test3); - strictEqual(test3Actual, "public"); - const test4Actual = getAccess(runner.context, Test4); - strictEqual(test4Actual, undefined); - const test5Actual = getAccess(runner.context, Test5); - strictEqual(test5Actual, "public"); + strictEqual(getAccess(runner.context, func1), "internal"); + strictEqual(getAccess(runner.context, func2), "public"); + strictEqual(getAccess(runner.context, func3), "public"); + strictEqual(getAccess(runner.context, func4), "internal"); + strictEqual(getAccess(runner.context, func5), "public"); + strictEqual(getAccess(runner.context, func6), "internal"); + strictEqual(getAccess(runner.context, func7), "public"); + strictEqual(getAccess(runner.context, func8), "public"); + + strictEqual(getAccess(runner.context, Test1), "internal"); + strictEqual(getAccess(runner.context, Test2), "public"); + strictEqual(getAccess(runner.context, Test3), "public"); + strictEqual(getAccess(runner.context, Test4), "public"); + strictEqual(getAccess(runner.context, Test5), "public"); }); it("access propagation with nullable", async () => { @@ -2098,8 +2080,8 @@ describe("typespec-client-generator-core: decorators", () => { ); const models = runner.context.experimental_sdkPackage.models; strictEqual(models.length, 2); - strictEqual(models[0].access, undefined); - strictEqual(models[1].access, undefined); + strictEqual(models[0].access, "public"); + strictEqual(models[1].access, "public"); }); }); diff --git a/packages/typespec-client-generator-core/test/types.test.ts b/packages/typespec-client-generator-core/test/types.test.ts index 4184d66916..a143f6ac38 100644 --- a/packages/typespec-client-generator-core/test/types.test.ts +++ b/packages/typespec-client-generator-core/test/types.test.ts @@ -3084,7 +3084,7 @@ describe("typespec-client-generator-core: types", () => { const Test3 = models.find((x) => x.name === "Test3"); ok(Test3); - strictEqual(Test3.access, undefined); + strictEqual(Test3.access, "public"); const Test4 = models.find((x) => x.name === "Test4"); ok(Test4); @@ -3096,7 +3096,7 @@ describe("typespec-client-generator-core: types", () => { const Test6 = models.find((x) => x.name === "Test6"); ok(Test6); - strictEqual(Test6.access, undefined); + strictEqual(Test6.access, "public"); }); it("additionalProperties of same type", async () => { await runner.compileWithBuiltInService(` From cd22823f6d6a95d3d8b16b85c14ed5d19d48bf81 Mon Sep 17 00:00:00 2001 From: iscai-msft Date: Tue, 28 May 2024 12:23:13 -0400 Subject: [PATCH 2/4] add changeset --- .../changes/default_access_public-2024-4-28-12-23-9.md | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 .chronus/changes/default_access_public-2024-4-28-12-23-9.md diff --git a/.chronus/changes/default_access_public-2024-4-28-12-23-9.md b/.chronus/changes/default_access_public-2024-4-28-12-23-9.md new file mode 100644 index 0000000000..baea596af0 --- /dev/null +++ b/.chronus/changes/default_access_public-2024-4-28-12-23-9.md @@ -0,0 +1,7 @@ +--- +changeKind: breaking +packages: + - "@azure-tools/typespec-client-generator-core" +--- + +change default of `.access` on a model or enum to `"public"` instead of `undefined` \ No newline at end of file From 0539cb1a01dbadf8351943bde998219015fdf953 Mon Sep 17 00:00:00 2001 From: iscai-msft Date: Tue, 28 May 2024 12:31:25 -0400 Subject: [PATCH 3/4] update docs --- docs/howtos/DataPlane Generation - DPG/03convenient.mdx | 2 +- docs/howtos/DataPlane Generation - DPG/07tcgcTypes.mdx | 8 +++++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/docs/howtos/DataPlane Generation - DPG/03convenient.mdx b/docs/howtos/DataPlane Generation - DPG/03convenient.mdx index 80c29249ab..0b377a8de7 100644 --- a/docs/howtos/DataPlane Generation - DPG/03convenient.mdx +++ b/docs/howtos/DataPlane Generation - DPG/03convenient.mdx @@ -189,7 +189,7 @@ import "@azure-tools/typespec-client-generator-core"; using Azure.ClientGenerator.Core; -@@access(PetStoreNamespace.GetModel, Access.internal); +@@access(PetStoreNamespace.GetModel, "internal"); ``` The two possible value for the `Access` enum are `internal` and `public`. diff --git a/docs/howtos/DataPlane Generation - DPG/07tcgcTypes.mdx b/docs/howtos/DataPlane Generation - DPG/07tcgcTypes.mdx index 5f1d855b58..1f383e22c3 100644 --- a/docs/howtos/DataPlane Generation - DPG/07tcgcTypes.mdx +++ b/docs/howtos/DataPlane Generation - DPG/07tcgcTypes.mdx @@ -488,7 +488,7 @@ They each extend from the shared `SdkMethodBase` interface SdkMethodBase { __raw?: Operation; name: string; - access: AccessFlags | undefined; + access: AccessFlags; parameters: SdkParameter[]; apiVersions: string[]; description?: string; @@ -1360,7 +1360,7 @@ export interface SdkModelType extends SdkTypeBase { generatedName: string; description?: string; details?: string; - access?: AccessFlags; + access: AccessFlags; usage: UsageFlags; additionalProperties?: SdkType; discriminatorValue?: string; @@ -1387,7 +1387,9 @@ export enum UsageFlags { ### AccessFlags -We have the `.access` property on a model etc. as optional, so emitters can choose what to default to. +We default the value of `.access` property on model, enum, and method types to be `"public"`. So if the `@access` decorator isn't explicitly applied to one of these definitions, its value will be `"public"`. + +If you want to know if a tsp author explicitly set the value with an `@access` decorator, you can call `getAccessOverride` ```ts export type AccessFlags = "internal" | "public"; From cdcc61d1c90722f83306df092234a6731d57b2d7 Mon Sep 17 00:00:00 2001 From: iscai-msft Date: Tue, 28 May 2024 16:00:01 -0400 Subject: [PATCH 4/4] remove comment --- packages/typespec-client-generator-core/src/types.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/typespec-client-generator-core/src/types.ts b/packages/typespec-client-generator-core/src/types.ts index c3aacb85be..5f89c960d7 100644 --- a/packages/typespec-client-generator-core/src/types.ts +++ b/packages/typespec-client-generator-core/src/types.ts @@ -557,7 +557,7 @@ export function getSdkModelWithDiagnostics( details: docWrapper.details, properties: [], additionalProperties: undefined, // going to set additional properties in the next few lines when we look at base model - access: "public", // dummy value since we need to update models map before we can set this + access: "public", usage: UsageFlags.None, // dummy value since we need to update models map before we can set this crossLanguageDefinitionId: getCrossLanguageDefinitionId(type, name), apiVersions: getAvailableApiVersions(context, type, type.namespace),