[tcgc] default .access to "public" (#925)

fixes #926 --------- Co-authored-by: iscai-msft <[email protected]> Co-authored-by: Chenjie Shi <[email protected]>
Azure · May 31, 2024 · 884d68b · 884d68b
1 parent 9bd912d
commit 884d68b
Show file tree

Hide file tree

Showing 8 changed files with 102 additions and 111 deletions.
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
@@ -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`
diff --git 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
@@ -488,7 +488,7 @@ They each extend from the shared `SdkMethodBase`
 interface SdkMethodBase<TServiceOperation extends SdkServiceOperation> {
   __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";

diff --git 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
@@ -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
@@ -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",
       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
@@ -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
@@ -3134,7 +3134,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);
@@ -3146,7 +3146,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(`