diff --git a/.chronus/changes/cleanup-autorest-diagnostics-2024-2-12-17-55-36.md b/.chronus/changes/cleanup-autorest-diagnostics-2024-2-12-17-55-36.md new file mode 100644 index 0000000000..ec1e6ec1c4 --- /dev/null +++ b/.chronus/changes/cleanup-autorest-diagnostics-2024-2-12-17-55-36.md @@ -0,0 +1,8 @@ +--- +# Change versionKind to one of: internal, fix, dependencies, feature, deprecation, breaking +changeKind: internal +packages: + - "@azure-tools/typespec-autorest" +--- + +Cleanup autorest diagnostics diff --git a/.chronus/changes/enable-no-enum-rule-2024-2-21-16-42-36.md b/.chronus/changes/enable-no-enum-rule-2024-2-21-16-42-36.md new file mode 100644 index 0000000000..3768d3d751 --- /dev/null +++ b/.chronus/changes/enable-no-enum-rule-2024-2-21-16-42-36.md @@ -0,0 +1,8 @@ +--- +# Change versionKind to one of: internal, fix, dependencies, feature, deprecation, breaking +changeKind: feature +packages: + - "@azure-tools/typespec-azure-core" +--- + +Enable `no-enum` rule by default in `all` ruleset diff --git a/.chronus/changes/enum_fixed-2024-2-21-15-54-49.md b/.chronus/changes/enum_fixed-2024-2-21-15-54-49.md new file mode 100644 index 0000000000..df0c8ba8fc --- /dev/null +++ b/.chronus/changes/enum_fixed-2024-2-21-15-54-49.md @@ -0,0 +1,7 @@ +--- +changeKind: breaking +packages: + - "@azure-tools/typespec-client-generator-core" +--- + +enums are always fixed after we switch to use union to represent extensible enum \ No newline at end of file diff --git a/.chronus/changes/fix-no-enum-code-fix-include-name-2024-2-22-8-14-39.md b/.chronus/changes/fix-no-enum-code-fix-include-name-2024-2-22-8-14-39.md new file mode 100644 index 0000000000..9dc60cae6e --- /dev/null +++ b/.chronus/changes/fix-no-enum-code-fix-include-name-2024-2-22-8-14-39.md @@ -0,0 +1,7 @@ +--- +changeKind: fix +packages: + - "@azure-tools/typespec-azure-core" +--- + +`no-enum` rule codefix now convert to named variant when the enum had not values (e.g. `enum E {a, b}`) \ No newline at end of file diff --git a/.chronus/changes/lintdiff-EnablePostRule-2024-2-21-19-18-34.md b/.chronus/changes/lintdiff-EnablePostRule-2024-2-21-19-18-34.md new file mode 100644 index 0000000000..1b68e37a8f --- /dev/null +++ b/.chronus/changes/lintdiff-EnablePostRule-2024-2-21-19-18-34.md @@ -0,0 +1,8 @@ +--- +# Change versionKind to one of: internal, fix, dependencies, feature, deprecation, breaking +changeKind: internal +packages: + - "@azure-tools/typespec-azure-resource-manager" +--- + +Ensure `arm-post-response-codes` rule is in the default ruleset. diff --git a/.chronus/changes/remove-projected-name-property-naming-rule-2024-2-21-15-32-12.md b/.chronus/changes/remove-projected-name-property-naming-rule-2024-2-21-15-32-12.md new file mode 100644 index 0000000000..5ffa4c3623 --- /dev/null +++ b/.chronus/changes/remove-projected-name-property-naming-rule-2024-2-21-15-32-12.md @@ -0,0 +1,8 @@ +--- +# Change versionKind to one of: internal, fix, dependencies, feature, deprecation, breaking +changeKind: fix +packages: + - "@azure-tools/typespec-azure-core" +--- + +Update `property-name-conflict` linter rule to stop looking and recommending `@projectedName` in favor of `@clientName` diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 1cf4891e0f..60a9b0a98d 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -1,4 +1,4 @@ -* @bterlson @daviwil @markcowl @allenjzhang @timotheeguerin +* @bterlson @markcowl @allenjzhang @timotheeguerin /packages/typespec-client-generator-core @lmazuel @m-nash @iscai-msft @srnagar @joheredi diff --git a/core b/core index 68cd9fe906..4e6f137c65 160000 --- a/core +++ b/core @@ -1 +1 @@ -Subproject commit 68cd9fe90681d91a23af2e7cadb5c974657c853d +Subproject commit 4e6f137c65ba502fce97a64d759a6fdd4230953f diff --git a/docs/howtos/DataPlane Generation - DPG/04renaming.mdx b/docs/howtos/DataPlane Generation - DPG/04renaming.mdx index c5611306a5..4a51d7a941 100644 --- a/docs/howtos/DataPlane Generation - DPG/04renaming.mdx +++ b/docs/howtos/DataPlane Generation - DPG/04renaming.mdx @@ -5,6 +5,11 @@ import TabItem from "@theme/TabItem"; This page documents how to customize names for client generations in DPG. For an overview of the setup, please visit the setup page. +:::note +The TypeSpec compiler provides an `@encodedName` decorator that allows changing the name of the property for a given serialization format. +However in Azure we recommend that you define the property name as the value sent on the wire and use the `@clientName` decorator to change the name of the generated property. +::: + ## Default behaviors By default, any language code generator will assume the TYPESPEC name is the client. For clarity, generators do not attempt to do any auto-magic rename. diff --git a/docs/libraries/azure-core/reference/linter.md b/docs/libraries/azure-core/reference/linter.md index 6fadc45891..ed61acf75c 100644 --- a/docs/libraries/azure-core/reference/linter.md +++ b/docs/libraries/azure-core/reference/linter.md @@ -52,7 +52,7 @@ Available ruleSets: | `@azure-tools/typespec-azure-core/no-multiple-discriminator` | Classes should have at most one discriminator. | | `@azure-tools/typespec-azure-core/no-rest-library-interfaces` | Resource interfaces from the TypeSpec.Rest.Resource library are incompatible with Azure.Core. | | `@azure-tools/typespec-azure-core/no-unknown` | Azure services must not have properties of type `unknown`. | -| `@azure-tools/typespec-azure-core/property-name-conflict` | Avoid naming conflicts. | +| `@azure-tools/typespec-azure-core/property-name-conflict` | Avoid naming conflicts between a property and a model of the same name. | | `@azure-tools/typespec-azure-core/bad-record-type` | Identify bad record definitions. | | `@azure-tools/typespec-azure-core/documentation-required` | Require documentation over enums, models, and operations. | | `@azure-tools/typespec-azure-core/key-visibility-required` | Key properties need to have an explicit visibility setting. | diff --git a/docs/libraries/azure-resource-manager/reference/linter.md b/docs/libraries/azure-resource-manager/reference/linter.md index 95c7b46f20..ab0a186fc2 100644 --- a/docs/libraries/azure-resource-manager/reference/linter.md +++ b/docs/libraries/azure-resource-manager/reference/linter.md @@ -30,6 +30,7 @@ Available ruleSets: | `@azure-tools/typespec-azure-resource-manager/arm-common-types-version` | Specify the ARM common-types version using @armCommonTypesVersion. | | [`@azure-tools/typespec-azure-resource-manager/arm-delete-operation-response-codes`](/libraries/azure-resource-manager/rules/delete-operation-response-codes.md) | Ensure delete operations have the appropriate status codes. | | [`@azure-tools/typespec-azure-resource-manager/arm-put-operation-response-codes`](/libraries/azure-resource-manager/rules/put-operation-response-codes.md) | Ensure put operations have the appropriate status codes. | +| [`@azure-tools/typespec-azure-resource-manager/arm-post-operation-response-codes`](/libraries/azure-resource-manager/rules/post-operation-response-codes.md) | Ensure post operations have the appropriate status codes. | | `@azure-tools/typespec-azure-resource-manager/arm-resource-action-no-segment` | `@armResourceAction` should not be used with `@segment`. | | `@azure-tools/typespec-azure-resource-manager/arm-resource-duplicate-property` | Warn about duplicate properties in resources. | | `@azure-tools/typespec-azure-resource-manager/arm-resource-invalid-envelope-property` | Check for invalid resource envelope properties. | diff --git a/packages/typespec-autorest/src/lib.ts b/packages/typespec-autorest/src/lib.ts index 92c647287d..3a4da17ca0 100644 --- a/packages/typespec-autorest/src/lib.ts +++ b/packages/typespec-autorest/src/lib.ts @@ -173,24 +173,6 @@ const EmitterOptionsSchema: JSONSchemaType = { const libDef = { name: "@azure-tools/typespec-autorest", diagnostics: { - "security-service-namespace": { - severity: "error", - messages: { - default: "Cannot add security details to a namespace other than the service namespace.", - }, - }, - "resource-namespace": { - severity: "error", - messages: { - default: "Resource goes on namespace", - }, - }, - "missing-path-param": { - severity: "error", - messages: { - default: paramMessage`Path contains parameter ${"param"} but wasn't found in given parameters`, - }, - }, "duplicate-body-types": { severity: "error", messages: { @@ -242,24 +224,6 @@ const libDef = { default: paramMessage`Invalid type '${"type"}' for a default value`, }, }, - "invalid-property-type-for-collection-format": { - severity: "error", - messages: { - default: "The collectionFormat can only be applied to model property with type 'string[]'.", - }, - }, - "invalid-collection-format": { - severity: "error", - messages: { - default: "The format should be one of 'csv','ssv','tsv','pipes' and 'multi'.", - }, - }, - "non-recommended-collection-format": { - severity: "warning", - messages: { - default: "The recommendation of collection format are 'csv' and 'multi'.", - }, - }, "invalid-multi-collection-format": { severity: "error", messages: { diff --git a/packages/typespec-azure-core/README.md b/packages/typespec-azure-core/README.md index 90385d630e..423b8016b0 100644 --- a/packages/typespec-azure-core/README.md +++ b/packages/typespec-azure-core/README.md @@ -56,7 +56,7 @@ Available ruleSets: | `@azure-tools/typespec-azure-core/no-multiple-discriminator` | Classes should have at most one discriminator. | | `@azure-tools/typespec-azure-core/no-rest-library-interfaces` | Resource interfaces from the TypeSpec.Rest.Resource library are incompatible with Azure.Core. | | `@azure-tools/typespec-azure-core/no-unknown` | Azure services must not have properties of type `unknown`. | -| `@azure-tools/typespec-azure-core/property-name-conflict` | Avoid naming conflicts. | +| `@azure-tools/typespec-azure-core/property-name-conflict` | Avoid naming conflicts between a property and a model of the same name. | | `@azure-tools/typespec-azure-core/bad-record-type` | Identify bad record definitions. | | `@azure-tools/typespec-azure-core/documentation-required` | Require documentation over enums, models, and operations. | | `@azure-tools/typespec-azure-core/key-visibility-required` | Key properties need to have an explicit visibility setting. | diff --git a/packages/typespec-azure-core/src/linter.ts b/packages/typespec-azure-core/src/linter.ts index e93fe4fe7e..898b4a2579 100644 --- a/packages/typespec-azure-core/src/linter.ts +++ b/packages/typespec-azure-core/src/linter.ts @@ -110,7 +110,7 @@ export const $linter = defineLinter({ true, [`@azure-tools/typespec-azure-core/${useStandardNames.name}`]: true, [`@azure-tools/typespec-azure-core/${friendlyNameRule.name}`]: true, - [`@azure-tools/typespec-azure-core/${noEnumRule.name}`]: false, + [`@azure-tools/typespec-azure-core/${noEnumRule.name}`]: true, }, extends: ["@typespec/http/all"], }, diff --git a/packages/typespec-azure-core/src/rules/no-enum.ts b/packages/typespec-azure-core/src/rules/no-enum.ts index 6c04206d00..271e02c4eb 100644 --- a/packages/typespec-azure-core/src/rules/no-enum.ts +++ b/packages/typespec-azure-core/src/rules/no-enum.ts @@ -23,9 +23,11 @@ export const noEnumRule = createRule({ create(context) { return { enum: (en: Enum) => { - if (getVersionsForEnum(context.program, en).length > 0) { + const [_, versions] = getVersionsForEnum(context.program, en); + if (versions !== undefined && versions.getVersions()[0].enumMember.enum === en) { return; } + context.reportDiagnostic({ format: { enumName: en.name }, target: en, @@ -48,7 +50,7 @@ function createEnumToExtensibleUnionCodeFix(en: Enum): CodeFix { ? node.value.value : `"${node.value.value}"` }` - : `"${node.id.sv}"`; + : `${node.id.sv}: "${node.id.sv}"`; } } @@ -131,7 +133,7 @@ function getNodeAnnotations(node: Node): string { } for (let i = endOfTrivia; i < node.end; i++) { - if (source[i] === " ") { + if (source[i] === " " || source[i] === "\n") { endOfTrivia = i + 1; } else { break; diff --git a/packages/typespec-azure-core/src/rules/property-naming.ts b/packages/typespec-azure-core/src/rules/property-naming.ts index c8718f8743..563df24af2 100644 --- a/packages/typespec-azure-core/src/rules/property-naming.ts +++ b/packages/typespec-azure-core/src/rules/property-naming.ts @@ -1,34 +1,20 @@ -import { ModelProperty, createRule, getProjectedNames, paramMessage } from "@typespec/compiler"; +import { ModelProperty, createRule, paramMessage } from "@typespec/compiler"; import { isExcludedCoreType } from "./utils.js"; export const propertyNameRule = createRule({ name: "property-name-conflict", - description: "Avoid naming conflicts.", + description: "Avoid naming conflicts between a property and a model of the same name.", severity: "warning", messages: { - default: paramMessage`Property '${"propertyName"}' having the same name as its enclosing model will cause problems with C# code generation. Consider renaming the property directly or using the @projectedName decorator to rename the property for C#.`, - projectedName: paramMessage`Use of @projectedName on property '${"propertyName"}' results in '${"propertyName"}' having the same name as its enclosing type in C#. Please use a different @projectedName value.`, + default: paramMessage`Property '${"propertyName"}' having the same name as its enclosing model will cause problems with C# code generation. Consider renaming the property directly or using the @clientName("newName", "csharp") decorator to rename the property for C#.`, }, create(context) { return { modelProperty: (property: ModelProperty) => { if (isExcludedCoreType(context.program, property)) return; - const projectedNames = getProjectedNames(context.program, property); const modelName = property.model?.name.toLocaleLowerCase(); const propertyName = property.name.toLocaleLowerCase(); - const csharpProjection = projectedNames?.get("csharp")?.toLocaleLowerCase(); - const clientProjection = projectedNames?.get("client")?.toLocaleLowerCase(); - if ( - csharpProjection === modelName || // csharp projection conflicts with model name - (clientProjection === modelName && !csharpProjection) // client projection conflicts with model name and there is no csharp projection - ) { - context.reportDiagnostic({ - messageId: "projectedName", - format: { propertyName: property.name }, - target: property, - }); - } else if (propertyName === modelName && !(csharpProjection || clientProjection)) { - // warning if the property name conflicts with the model name and there is no csharp or client projected name + if (propertyName === modelName) { context.reportDiagnostic({ format: { propertyName: property.name }, target: property, diff --git a/packages/typespec-azure-core/src/rules/require-docs.ts b/packages/typespec-azure-core/src/rules/require-docs.ts index 5560111d2d..c2c26e1f9d 100644 --- a/packages/typespec-azure-core/src/rules/require-docs.ts +++ b/packages/typespec-azure-core/src/rules/require-docs.ts @@ -12,7 +12,7 @@ export const requireDocumentation = createRule({ description: "Require documentation over enums, models, and operations.", severity: "warning", messages: { - default: paramMessage`The ${"kind"} named '${"name"}' should have a documentation or description, please use decorator @doc to add it.`, + default: paramMessage`The ${"kind"} named '${"name"}' should have a documentation or description, use doc comment /** */ to provide it.`, }, create(context) { return { diff --git a/packages/typespec-azure-core/test/rules/no-enum.test.ts b/packages/typespec-azure-core/test/rules/no-enum.test.ts index 784a630fc7..a0b5edd07c 100644 --- a/packages/typespec-azure-core/test/rules/no-enum.test.ts +++ b/packages/typespec-azure-core/test/rules/no-enum.test.ts @@ -31,6 +31,7 @@ describe("typespec-azure-core: no-enum rule", () => { }, ]); }); + it("allows the version enum", async () => { await tester .expect( @@ -46,6 +47,27 @@ describe("typespec-azure-core: no-enum rule", () => { .toBeValid(); }); + it("emit warning about other enums in versioned service", async () => { + await tester + .expect( + ` + @service + @versioned(Versions) + namespace Foo; + enum Versions { + v1, v2 + } + + enum Bar { a, b} + ` + ) + .toEmitDiagnostics([ + { + code: "@azure-tools/typespec-azure-core/no-enum", + }, + ]); + }); + describe("codefix", () => { it("codefix simple enum", async () => { await tester @@ -60,7 +82,31 @@ describe("typespec-azure-core: no-enum rule", () => { union PetKind { string, - "cat", "dog", + cat: "cat", dog: "dog", + } + `); + }); + + it("keeps new lines", async () => { + await tester + .expect( + ` + enum PetKind { + /** cat doc */ + cat, + /** dog doc */ + dog + } + ` + ) + .applyCodeFix("enum-to-extensible-union").toEqual(` + union PetKind { + string, + + /** cat doc */ + cat: "cat", + /** dog doc */ + dog: "dog", } `); }); @@ -115,14 +161,14 @@ describe("typespec-azure-core: no-enum rule", () => { /** cat */ @doc("cat") #suppress "cat" - "cat", + cat: "cat", // dog /** dog */ @doc("dog") #suppress "dog" - "dog", + dog: "dog", // end } diff --git a/packages/typespec-azure-core/test/rules/prevent-property-name-conflict.test.ts b/packages/typespec-azure-core/test/rules/prevent-property-name-conflict.test.ts deleted file mode 100644 index 42cf55ff9b..0000000000 --- a/packages/typespec-azure-core/test/rules/prevent-property-name-conflict.test.ts +++ /dev/null @@ -1,160 +0,0 @@ -import { - BasicTestRunner, - LinterRuleTester, - createLinterRuleTester, -} from "@typespec/compiler/testing"; -import { beforeEach, describe, it } from "vitest"; -import { propertyNameRule } from "../../src/rules/property-naming.js"; -import { createAzureCoreTestRunner } from "../test-host.js"; - -describe("typespec-azure-core: property name conflict", () => { - let runner: BasicTestRunner; - let tester: LinterRuleTester; - - beforeEach(async () => { - runner = await createAzureCoreTestRunner(); - tester = createLinterRuleTester(runner, propertyNameRule, "@azure-tools/typespec-azure-core"); - }); - - it("emit warning if property name conflicts with model name", async () => { - await tester.expect(`model Foo {foo: string}`).toEmitDiagnostics({ - code: "@azure-tools/typespec-azure-core/property-name-conflict", - message: `Property 'foo' having the same name as its enclosing model will cause problems with C# code generation. Consider renaming the property directly or using the @projectedName decorator to rename the property for C#.`, - }); - }); - - it("is valid if conflict resolved through @projectedName('csharp', ...)", async () => { - await tester - .expect( - `model Foo { - #suppress "deprecated" "for testing" - @projectedName("csharp", "bar") foo: string - }` - ) - .toBeValid(); - }); - - it("is valid if conflict resolved through @projectedName('client', ...)", async () => { - await tester - .expect( - `model Foo { - #suppress "deprecated" "for testing" - @projectedName("client", "bar") foo: string - }` - ) - .toBeValid(); - }); - - it("emit warning if conflict not resolved through @projectedName", async () => { - await tester - .expect( - `model Foo { - #suppress "deprecated" "for testing" - @projectedName("python", "bar") foo: string }` - ) - .toEmitDiagnostics({ - code: "@azure-tools/typespec-azure-core/property-name-conflict", - message: `Property 'foo' having the same name as its enclosing model will cause problems with C# code generation. Consider renaming the property directly or using the @projectedName decorator to rename the property for C#.`, - }); - }); - - it("emit warning if @projectedName('client', ...) introduces conflict", async () => { - await tester - .expect( - `model Foo { - #suppress "deprecated" "for testing" - @projectedName("client", "foo") bar: string; - }` - ) - .toEmitDiagnostics({ - code: "@azure-tools/typespec-azure-core/property-name-conflict", - message: `Use of @projectedName on property 'bar' results in 'bar' having the same name as its enclosing type in C#. Please use a different @projectedName value.`, - }); - }); - - it("emit warning if @projectedName('csharp', ...) introduces conflict", async () => { - await tester - .expect( - `model Foo { - #suppress "deprecated" "for testing" - @projectedName("csharp", "foo") bar: string; - }` - ) - .toEmitDiagnostics({ - code: "@azure-tools/typespec-azure-core/property-name-conflict", - message: `Use of @projectedName on property 'bar' results in 'bar' having the same name as its enclosing type in C#. Please use a different @projectedName value.`, - }); - }); - - it("is valid if @projectedName('client', ...) causes conflict but @projectedName('csharp', ...) resolves it", async () => { - await tester - .expect( - `model Foo { - #suppress "deprecated" "for testing" - @projectedName("client", "foo") @projectedName("csharp", "baz") bar: string; - }` - ) - .toBeValid(); - }); - - it("emit warning if @friendlyName would resolve a conflict (friendlyName is ignored)", async () => { - await tester.expect(`model Foo { @friendlyName("bar") foo: string }`).toEmitDiagnostics({ - code: "@azure-tools/typespec-azure-core/property-name-conflict", - message: `Property 'foo' having the same name as its enclosing model will cause problems with C# code generation. Consider renaming the property directly or using the @projectedName decorator to rename the property for C#.`, - }); - }); - - it("is valid if @friendlyName introduces conflict (friendlyName is ignored)", async () => { - await tester.expect(`model Foo { @friendlyName("foo") bar: string }`).toBeValid(); - }); - - it("emit warning if property name conflicts with model name when using `is`", async () => { - await tester - .expect( - ` - model Base { - foo: string - } - - model Foo is Base {} - ` - ) - .toEmitDiagnostics({ - code: "@azure-tools/typespec-azure-core/property-name-conflict", - message: `Property 'foo' having the same name as its enclosing model will cause problems with C# code generation. Consider renaming the property directly or using the @projectedName decorator to rename the property for C#.`, - }); - }); - - it("is valid if inherited property name conflicts with model name", async () => { - await tester - .expect( - ` - model Base { - foo: string - } - - model Foo extends Base {} - ` - ) - .toBeValid(); - }); - - it("emit warning if spread property name conflicts with model name", async () => { - await tester - .expect( - ` - model Base { - foo: string - } - - model Foo { - ...Base; - } - ` - ) - .toEmitDiagnostics({ - code: "@azure-tools/typespec-azure-core/property-name-conflict", - message: `Property 'foo' having the same name as its enclosing model will cause problems with C# code generation. Consider renaming the property directly or using the @projectedName decorator to rename the property for C#.`, - }); - }); -}); diff --git a/packages/typespec-azure-core/test/rules/property-name-conflict.test.ts b/packages/typespec-azure-core/test/rules/property-name-conflict.test.ts new file mode 100644 index 0000000000..bf6b6b15f2 --- /dev/null +++ b/packages/typespec-azure-core/test/rules/property-name-conflict.test.ts @@ -0,0 +1,159 @@ +import { + BasicTestRunner, + LinterRuleTester, + createLinterRuleTester, +} from "@typespec/compiler/testing"; +import { beforeEach, it } from "vitest"; +import { propertyNameRule } from "../../src/rules/property-naming.js"; +import { createAzureCoreTestRunner } from "../test-host.js"; + +let runner: BasicTestRunner; +let tester: LinterRuleTester; + +beforeEach(async () => { + runner = await createAzureCoreTestRunner(); + tester = createLinterRuleTester(runner, propertyNameRule, "@azure-tools/typespec-azure-core"); +}); + +it("emit warning if property name conflicts with model name", async () => { + await tester.expect(`model Foo {foo: string}`).toEmitDiagnostics({ + code: "@azure-tools/typespec-azure-core/property-name-conflict", + message: `Property 'foo' having the same name as its enclosing model will cause problems with C# code generation. Consider renaming the property directly or using the @clientName("newName", "csharp") decorator to rename the property for C#.`, + }); +}); + +// TODO: reenable when rule is moved to tcgc and can resolve if clientName was set +it.skip(`is valid if conflict resolved through @clientName("newName", "csharp")`, async () => { + await tester + .expect( + `model Foo { + @clientName("bar", "csharp") foo: string + }` + ) + .toBeValid(); +}); + +// TODO: reenable when rule is moved to tcgc and can resolve if clientName was set +it.skip(`is valid if conflict resolved through @clientName("newName")`, async () => { + await tester + .expect( + `model Foo { + @clientName("bar") foo: string + }` + ) + .toBeValid(); +}); + +// TODO: reenable when rule is moved to tcgc and can resolve if clientName was set +it.skip("emit warning if conflict not resolved through @clientName", async () => { + await tester + .expect( + `model Foo { + @clientName("bar", "python") foo: string + }` + ) + .toEmitDiagnostics({ + code: "@azure-tools/typespec-azure-core/property-name-conflict", + message: `Property 'foo' having the same name as its enclosing model will cause problems with C# code generation. Consider renaming the property directly or using the @clientName("newName", "csharp") decorator to rename the property for C#.`, + }); +}); + +// TODO: reenable when rule is moved to tcgc and can resolve if clientName was set +it.skip(`emit warning if @clientName("newName") introduces conflict`, async () => { + await tester + .expect( + `model Foo { + @clientName("foo") bar: string; + }` + ) + .toEmitDiagnostics({ + code: "@azure-tools/typespec-azure-core/property-name-conflict", + message: `Use of @clientName on property 'bar' results in 'bar' having the same name as its enclosing type in C#. Please use a different @clientName("newName", "csharp") value.`, + }); +}); + +// TODO: reenable when rule is moved to tcgc and can resolve if clientName was set +it.skip(`emit warning if @clientName("newName", "csharp") introduces conflict`, async () => { + await tester + .expect( + `model Foo { + @clientName("foo", "csharp") bar: string; + }` + ) + .toEmitDiagnostics({ + code: "@azure-tools/typespec-azure-core/property-name-conflict", + message: `Use of @clientName on property 'bar' results in 'bar' having the same name as its enclosing type in C#. Please use a different @clientName("newName", "csharp") value.`, + }); +}); + +// TODO: reenable when rule is moved to tcgc and can resolve if clientName was set +it.skip(`is valid if @clientName("newName") causes conflict but @clientName("newName", "csharp") resolves it`, async () => { + await tester + .expect( + `model Foo { + @clientName("foo") @"client", ("baz", "csharp") bar: string; + }` + ) + .toBeValid(); +}); + +it("emit warning if @friendlyName would resolve a conflict (friendlyName is ignored)", async () => { + await tester.expect(`model Foo { @friendlyName("bar") foo: string }`).toEmitDiagnostics({ + code: "@azure-tools/typespec-azure-core/property-name-conflict", + message: `Property 'foo' having the same name as its enclosing model will cause problems with C# code generation. Consider renaming the property directly or using the @clientName("newName", "csharp") decorator to rename the property for C#.`, + }); +}); + +it("is valid if @friendlyName introduces conflict (friendlyName is ignored)", async () => { + await tester.expect(`model Foo { @friendlyName("foo") bar: string }`).toBeValid(); +}); + +it("emit warning if property name conflicts with model name when using `is`", async () => { + await tester + .expect( + ` + model Base { + foo: string + } + + model Foo is Base {} + ` + ) + .toEmitDiagnostics({ + code: "@azure-tools/typespec-azure-core/property-name-conflict", + message: `Property 'foo' having the same name as its enclosing model will cause problems with C# code generation. Consider renaming the property directly or using the @clientName("newName", "csharp") decorator to rename the property for C#.`, + }); +}); + +it("is valid if inherited property name conflicts with model name", async () => { + await tester + .expect( + ` + model Base { + foo: string + } + + model Foo extends Base {} + ` + ) + .toBeValid(); +}); + +it("emit warning if spread property name conflicts with model name", async () => { + await tester + .expect( + ` + model Base { + foo: string + } + + model Foo { + ...Base; + } + ` + ) + .toEmitDiagnostics({ + code: "@azure-tools/typespec-azure-core/property-name-conflict", + message: `Property 'foo' having the same name as its enclosing model will cause problems with C# code generation. Consider renaming the property directly or using the @clientName("newName", "csharp") decorator to rename the property for C#.`, + }); +}); diff --git a/packages/typespec-azure-core/test/rules/require-docs.test.ts b/packages/typespec-azure-core/test/rules/require-docs.test.ts index bec13a34f7..6c9c3c48bc 100644 --- a/packages/typespec-azure-core/test/rules/require-docs.test.ts +++ b/packages/typespec-azure-core/test/rules/require-docs.test.ts @@ -36,7 +36,7 @@ describe("typespec-azure-core: documentation-required rule", () => { it("on model", async () => await checkDocRequired( "┆model Foo {}", - "The Model named 'Foo' should have a documentation or description, please use decorator @doc to add it." + "The Model named 'Foo' should have a documentation or description, use doc comment /** */ to provide it." )); it("on model property", async () => @@ -44,25 +44,25 @@ describe("typespec-azure-core: documentation-required rule", () => { `@doc("Abc") model Foo { ┆x: string; }`, - "The ModelProperty named 'x' should have a documentation or description, please use decorator @doc to add it." + "The ModelProperty named 'x' should have a documentation or description, use doc comment /** */ to provide it." )); it("on operation", async () => await checkDocRequired( `┆op read(): void;`, - "The Operation named 'read' should have a documentation or description, please use decorator @doc to add it." + "The Operation named 'read' should have a documentation or description, use doc comment /** */ to provide it." )); it("on property", async () => await checkDocRequired( `@doc("op doc") op read(┆param1: string): void;`, - "The ModelProperty named 'param1' should have a documentation or description, please use decorator @doc to add it." + "The ModelProperty named 'param1' should have a documentation or description, use doc comment /** */ to provide it." )); it("on enum", async () => { await checkDocRequired( "┆enum Foo {}", - "The Enum named 'Foo' should have a documentation or description, please use decorator @doc to add it." + "The Enum named 'Foo' should have a documentation or description, use doc comment /** */ to provide it." ); }); @@ -71,7 +71,7 @@ describe("typespec-azure-core: documentation-required rule", () => { `@doc(".") enum Foo { ┆Bar, }`, - "The EnumMember named 'Bar' should have a documentation or description, please use decorator @doc to add it." + "The EnumMember named 'Bar' should have a documentation or description, use doc comment /** */ to provide it." ); }); }); diff --git a/packages/typespec-azure-resource-manager/README.md b/packages/typespec-azure-resource-manager/README.md index 8a1aa1b87b..0de0314c4d 100644 --- a/packages/typespec-azure-resource-manager/README.md +++ b/packages/typespec-azure-resource-manager/README.md @@ -34,6 +34,7 @@ Available ruleSets: | `@azure-tools/typespec-azure-resource-manager/arm-common-types-version` | Specify the ARM common-types version using @armCommonTypesVersion. | | [`@azure-tools/typespec-azure-resource-manager/arm-delete-operation-response-codes`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/delete-operation-response-codes) | Ensure delete operations have the appropriate status codes. | | [`@azure-tools/typespec-azure-resource-manager/arm-put-operation-response-codes`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/put-operation-response-codes) | Ensure put operations have the appropriate status codes. | +| [`@azure-tools/typespec-azure-resource-manager/arm-post-operation-response-codes`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/post-operation-response-codes) | Ensure post operations have the appropriate status codes. | | `@azure-tools/typespec-azure-resource-manager/arm-resource-action-no-segment` | `@armResourceAction` should not be used with `@segment`. | | `@azure-tools/typespec-azure-resource-manager/arm-resource-duplicate-property` | Warn about duplicate properties in resources. | | `@azure-tools/typespec-azure-resource-manager/arm-resource-invalid-envelope-property` | Check for invalid resource envelope properties. | diff --git a/packages/typespec-azure-resource-manager/src/linter.ts b/packages/typespec-azure-resource-manager/src/linter.ts index b1e7b3b821..f9ccdb9447 100644 --- a/packages/typespec-azure-resource-manager/src/linter.ts +++ b/packages/typespec-azure-resource-manager/src/linter.ts @@ -2,6 +2,7 @@ import { defineLinter } from "@typespec/compiler"; import { armCommonTypesVersionRule } from "./rules/arm-common-types-version.js"; import { armDeleteResponseCodesRule } from "./rules/arm-delete-response-codes.js"; import { armNoRecordRule } from "./rules/arm-no-record.js"; +import { armPostResponseCodesRule } from "./rules/arm-post-response-codes.js"; import { armPutResponseCodesRule } from "./rules/arm-put-response-codes.js"; import { armResourceActionNoSegmentRule } from "./rules/arm-resource-action-no-segment.js"; import { armResourceDuplicatePropertiesRule } from "./rules/arm-resource-duplicate-property.js"; @@ -32,6 +33,7 @@ const rules = [ armCommonTypesVersionRule, armDeleteResponseCodesRule, armPutResponseCodesRule, + armPostResponseCodesRule, armResourceActionNoSegmentRule, armResourceDuplicatePropertiesRule, armResourceEnvelopeProperties, diff --git a/packages/typespec-client-generator-core/src/types.ts b/packages/typespec-client-generator-core/src/types.ts index 4f9b6aca8d..2b4e79de10 100644 --- a/packages/typespec-client-generator-core/src/types.ts +++ b/packages/typespec-client-generator-core/src/types.ts @@ -1,9 +1,4 @@ -import { - UnionEnum, - getLroMetadata, - getUnionAsEnum, - isFixed, -} from "@azure-tools/typespec-azure-core"; +import { UnionEnum, getLroMetadata, getUnionAsEnum } from "@azure-tools/typespec-azure-core"; import { BooleanLiteral, BytesKnownEncoding, @@ -647,7 +642,7 @@ export function getSdkEnum(context: TCGCContext, type: Enum, operation?: Operati details: docWrapper.details, valueType: getSdkEnumValueType(context, type.members.values()), values: [], - isFixed: isFixed(context.program, type), + 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 diff --git a/packages/typespec-client-generator-core/test/types.test.ts b/packages/typespec-client-generator-core/test/types.test.ts index adade352eb..4de3a6fe6e 100644 --- a/packages/typespec-client-generator-core/test/types.test.ts +++ b/packages/typespec-client-generator-core/test/types.test.ts @@ -805,7 +805,7 @@ describe("typespec-client-generator-core: types", () => { strictEqual(runner.context.experimental_sdkPackage.models.length, 1); strictEqual(runner.context.experimental_sdkPackage.enums.length, 1); const sdkType = runner.context.experimental_sdkPackage.enums[0]; - strictEqual(sdkType.isFixed, false); + strictEqual(sdkType.isFixed, true); strictEqual(sdkType.name, "DaysOfWeekExtensibleEnum"); strictEqual(sdkType.valueType.kind, "string"); strictEqual(sdkType.usage & UsageFlags.Versioning, 0); // not a versioning enum @@ -856,7 +856,7 @@ describe("typespec-client-generator-core: types", () => { strictEqual(runner.context.experimental_sdkPackage.models.length, 1); strictEqual(runner.context.experimental_sdkPackage.enums.length, 1); const sdkType = runner.context.experimental_sdkPackage.enums[0]; - strictEqual(sdkType.isFixed, false); + strictEqual(sdkType.isFixed, true); strictEqual(sdkType.name, "Integers"); strictEqual(sdkType.valueType.kind, "int32"); const values = sdkType.values; @@ -890,7 +890,7 @@ describe("typespec-client-generator-core: types", () => { const sdkType = runner.context.experimental_sdkPackage.enums[0]; ok(sdkType); - strictEqual(sdkType.isFixed, false); + strictEqual(sdkType.isFixed, true); strictEqual(sdkType.name, "Floats"); strictEqual(sdkType.valueType.kind, "float32"); const values = sdkType.values;