From c723c31280b654dc95c9fa917329338bd6847c82 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 6 May 2026 02:13:45 +0000 Subject: [PATCH 1/6] Initial plan From 4bfcb9d19d2da3cb30ee63c133950102dc4cb479 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 6 May 2026 02:28:21 +0000 Subject: [PATCH 2/6] Add arm-no-replace-inherited-props linting rule Agent-Logs-Url: https://github.com/Azure/typespec-azure/sessions/65193dd6-7c37-41ac-849e-a574be60d7c0 Co-authored-by: markcowl <1054056+markcowl@users.noreply.github.com> --- ...ce-inherited-props-rule-2026-5-6-2-25-0.md | 7 + .../typespec-azure-resource-manager/README.md | 1 + .../src/linter.ts | 2 + .../rules/arm-no-replace-inherited-props.ts | 59 +++++++ .../arm-no-replace-inherited-props.test.ts | 154 ++++++++++++++++++ .../reference/linter.md | 1 + .../rules/arm-no-replace-inherited-props.md | 61 +++++++ 7 files changed, 285 insertions(+) create mode 100644 .chronus/changes/add-arm-no-replace-inherited-props-rule-2026-5-6-2-25-0.md create mode 100644 packages/typespec-azure-resource-manager/src/rules/arm-no-replace-inherited-props.ts create mode 100644 packages/typespec-azure-resource-manager/test/rules/arm-no-replace-inherited-props.test.ts create mode 100644 website/src/content/docs/docs/libraries/azure-resource-manager/rules/arm-no-replace-inherited-props.md diff --git a/.chronus/changes/add-arm-no-replace-inherited-props-rule-2026-5-6-2-25-0.md b/.chronus/changes/add-arm-no-replace-inherited-props-rule-2026-5-6-2-25-0.md new file mode 100644 index 0000000000..8f1a4264cf --- /dev/null +++ b/.chronus/changes/add-arm-no-replace-inherited-props-rule-2026-5-6-2-25-0.md @@ -0,0 +1,7 @@ +--- +changeKind: feature +packages: + - "@azure-tools/typespec-azure-resource-manager" +--- + +Add new linting rule `arm-no-replace-inherited-props` that warns when a model redefines a property that is already defined in one of its base models. The 'name' property of an ARM resource and properties redefined as part of a model marked with `@discriminator` are not flagged by this rule. diff --git a/packages/typespec-azure-resource-manager/README.md b/packages/typespec-azure-resource-manager/README.md index e0d06fdb2e..55a3726d50 100644 --- a/packages/typespec-azure-resource-manager/README.md +++ b/packages/typespec-azure-resource-manager/README.md @@ -29,6 +29,7 @@ Available ruleSets: | Name | Description | | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | [`@azure-tools/typespec-azure-resource-manager/arm-no-record`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/arm-no-record) | Don't use Record types for ARM resources. | +| [`@azure-tools/typespec-azure-resource-manager/arm-no-replace-inherited-props`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/arm-no-replace-inherited-props) | Disallow redefining properties already defined in a base type. | | [`@azure-tools/typespec-azure-resource-manager/arm-common-types-version`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/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. | diff --git a/packages/typespec-azure-resource-manager/src/linter.ts b/packages/typespec-azure-resource-manager/src/linter.ts index 4d597f3e8c..f9e0708d9e 100644 --- a/packages/typespec-azure-resource-manager/src/linter.ts +++ b/packages/typespec-azure-resource-manager/src/linter.ts @@ -4,6 +4,7 @@ import { armCustomResourceNoKey } from "./rules/arm-custom-resource-no-key.js"; import { armCustomResourceUsageDiscourage } from "./rules/arm-custom-resource-usage-discourage.js"; import { armDeleteResponseCodesRule } from "./rules/arm-delete-response-codes.js"; import { armNoRecordRule } from "./rules/arm-no-record.js"; +import { armNoReplaceInheritedPropsRule } from "./rules/arm-no-replace-inherited-props.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"; @@ -36,6 +37,7 @@ import { unsupportedTypeRule } from "./rules/unsupported-type.js"; const rules = [ armNoRecordRule, + armNoReplaceInheritedPropsRule, armCommonTypesVersionRule, armDeleteResponseCodesRule, armPutResponseCodesRule, diff --git a/packages/typespec-azure-resource-manager/src/rules/arm-no-replace-inherited-props.ts b/packages/typespec-azure-resource-manager/src/rules/arm-no-replace-inherited-props.ts new file mode 100644 index 0000000000..cbfe8e61c9 --- /dev/null +++ b/packages/typespec-azure-resource-manager/src/rules/arm-no-replace-inherited-props.ts @@ -0,0 +1,59 @@ +import { createRule, getDiscriminator, Model, paramMessage } from "@typespec/compiler"; + +import { getArmResource } from "../resource.js"; + +export const armNoReplaceInheritedPropsRule = createRule({ + name: "arm-no-replace-inherited-props", + severity: "warning", + description: "Disallow redefining properties already defined in a base type.", + url: "https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/arm-no-replace-inherited-props", + messages: { + default: paramMessage`The property '${"propertyName"}' is also defined in the base model. Redefining inherited properties can cause problems with OpenAPI tooling and some language representations of the models.`, + }, + create(context) { + return { + model: (model: Model) => { + if (model.baseModel === undefined) return; + + // Collect discriminator property names from any ancestor model with `@discriminator`. + const discriminatorProps = new Set(); + let ancestor: Model | undefined = model.baseModel; + while (ancestor !== undefined) { + const discriminator = getDiscriminator(context.program, ancestor); + if (discriminator !== undefined) { + discriminatorProps.add(discriminator.propertyName); + } + ancestor = ancestor.baseModel; + } + + const isArmResource = getArmResource(context.program, model) !== undefined; + + for (const property of model.properties.values()) { + // Skip the 'name' property of any ARM resource. + if (isArmResource && property.name === "name") continue; + + // Skip discriminator properties redefined in derived models. + if (discriminatorProps.has(property.name)) continue; + + // Look up the chain of base models for a property with the same name. + let baseHasProperty = false; + let current: Model | undefined = model.baseModel; + while (current !== undefined) { + if (current.properties.has(property.name)) { + baseHasProperty = true; + break; + } + current = current.baseModel; + } + + if (baseHasProperty) { + context.reportDiagnostic({ + format: { propertyName: property.name }, + target: property, + }); + } + } + }, + }; + }, +}); diff --git a/packages/typespec-azure-resource-manager/test/rules/arm-no-replace-inherited-props.test.ts b/packages/typespec-azure-resource-manager/test/rules/arm-no-replace-inherited-props.test.ts new file mode 100644 index 0000000000..bbeb2a63b4 --- /dev/null +++ b/packages/typespec-azure-resource-manager/test/rules/arm-no-replace-inherited-props.test.ts @@ -0,0 +1,154 @@ +import { Tester } from "#test/tester.js"; +import { + createLinterRuleTester, + LinterRuleTester, + TesterInstance, +} from "@typespec/compiler/testing"; +import { beforeEach, it } from "vitest"; + +import { armNoReplaceInheritedPropsRule } from "../../src/rules/arm-no-replace-inherited-props.js"; + +let runner: TesterInstance; +let tester: LinterRuleTester; + +beforeEach(async () => { + runner = await Tester.createInstance(); + tester = createLinterRuleTester( + runner, + armNoReplaceInheritedPropsRule, + "@azure-tools/typespec-azure-resource-manager", + ); +}); + +it("succeeds when child model does not redefine inherited properties", async () => { + await tester + .expect( + ` + @armProviderNamespace namespace MyService; + + model Base { + id: string; + } + + model Child extends Base { + otherProp: string; + } + `, + ) + .toBeValid(); +}); + +it("emits warning when child model redefines an inherited property", async () => { + await tester + .expect( + ` + @armProviderNamespace namespace MyService; + + model Base { + id: string; + commonProp: string; + } + + model Child extends Base { + commonProp: string; + } + `, + ) + .toEmitDiagnostics([ + { + code: "@azure-tools/typespec-azure-resource-manager/arm-no-replace-inherited-props", + message: + "The property 'commonProp' is also defined in the base model. Redefining inherited properties can cause problems with OpenAPI tooling and some language representations of the models.", + }, + ]); +}); + +it("emits warning when redefining a property from an indirect ancestor", async () => { + await tester + .expect( + ` + @armProviderNamespace namespace MyService; + + model GrandParent { + sharedProp: string; + } + + model Parent extends GrandParent { + midProp: string; + } + + model Child extends Parent { + sharedProp: string; + } + `, + ) + .toEmitDiagnostics([ + { + code: "@azure-tools/typespec-azure-resource-manager/arm-no-replace-inherited-props", + message: + "The property 'sharedProp' is also defined in the base model. Redefining inherited properties can cause problems with OpenAPI tooling and some language representations of the models.", + }, + ]); +}); + +it("does not warn when redefining the 'name' property of an ARM resource", async () => { + await tester + .expect( + ` + @armProviderNamespace namespace MyService; + + model FooResource is TrackedResource<{}> { + @key @segment("foo") name: string; + } + `, + ) + .toBeValid(); +}); + +it("does not warn when redefining a discriminator property in a derived model", async () => { + await tester + .expect( + ` + @armProviderNamespace namespace MyService; + + @discriminator("kind") + model Pet { + kind: string; + weight: int32; + } + + model Cat extends Pet { + kind: "cat"; + meow: boolean; + } + `, + ) + .toBeValid(); +}); + +it("warns on non-discriminator inherited properties even when base has @discriminator", async () => { + await tester + .expect( + ` + @armProviderNamespace namespace MyService; + + @discriminator("kind") + model Pet { + kind: string; + weight: int32; + } + + model Cat extends Pet { + kind: "cat"; + weight: int32; + } + `, + ) + .toEmitDiagnostics([ + { + code: "@azure-tools/typespec-azure-resource-manager/arm-no-replace-inherited-props", + message: + "The property 'weight' is also defined in the base model. Redefining inherited properties can cause problems with OpenAPI tooling and some language representations of the models.", + }, + ]); +}); diff --git a/website/src/content/docs/docs/libraries/azure-resource-manager/reference/linter.md b/website/src/content/docs/docs/libraries/azure-resource-manager/reference/linter.md index 4c2b926af8..29a3aa626b 100644 --- a/website/src/content/docs/docs/libraries/azure-resource-manager/reference/linter.md +++ b/website/src/content/docs/docs/libraries/azure-resource-manager/reference/linter.md @@ -23,6 +23,7 @@ Available ruleSets: | Name | Description | | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | [`@azure-tools/typespec-azure-resource-manager/arm-no-record`](/libraries/azure-resource-manager/rules/arm-no-record.md) | Don't use Record types for ARM resources. | +| [`@azure-tools/typespec-azure-resource-manager/arm-no-replace-inherited-props`](/libraries/azure-resource-manager/rules/arm-no-replace-inherited-props.md) | Disallow redefining properties already defined in a base type. | | [`@azure-tools/typespec-azure-resource-manager/arm-common-types-version`](/libraries/azure-resource-manager/rules/arm-common-types-version.md) | 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. | diff --git a/website/src/content/docs/docs/libraries/azure-resource-manager/rules/arm-no-replace-inherited-props.md b/website/src/content/docs/docs/libraries/azure-resource-manager/rules/arm-no-replace-inherited-props.md new file mode 100644 index 0000000000..6a4b992b2f --- /dev/null +++ b/website/src/content/docs/docs/libraries/azure-resource-manager/rules/arm-no-replace-inherited-props.md @@ -0,0 +1,61 @@ +--- +title: arm-no-replace-inherited-props +--- + +```text title="Full name" +@azure-tools/typespec-azure-resource-manager/arm-no-replace-inherited-props +``` + +Warns when a model redefines a property that is already defined in one of its base models. Repeating inherited properties in a child model is an anti-pattern for Azure APIs and can cause problems with OpenAPI tooling and some language representations of the models. + +The `name` property of an ARM resource and properties redefined as part of a model marked with `@discriminator` are not flagged by this rule. + +#### ❌ Incorrect + +```tsp +@armProviderNamespace +namespace MyService; + +model Base { + id: string; + commonProp: string; +} + +model Child extends Base { + commonProp: string; // duplicate of inherited "commonProp" +} +``` + +#### ✅ Correct + +```tsp +@armProviderNamespace +namespace MyService; + +model Base { + id: string; + commonProp: string; +} + +model Child extends Base { + otherProp: string; +} +``` + +#### ✅ Correct (discriminator property) + +```tsp +@armProviderNamespace +namespace MyService; + +@discriminator("kind") +model Pet { + kind: string; + weight: int32; +} + +model Cat extends Pet { + kind: "cat"; // allowed: 'kind' is a discriminator property + meow: boolean; +} +``` From 6d60f84746eeae296f273b7476a2e4f402a4d2cf Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 7 May 2026 21:59:29 +0000 Subject: [PATCH 3/6] Include typespec-azure-rulesets in changeset Agent-Logs-Url: https://github.com/Azure/typespec-azure/sessions/a463ee62-17e5-45fa-8acd-eb3416b60ccd Co-authored-by: markcowl <1054056+markcowl@users.noreply.github.com> --- .../add-arm-no-replace-inherited-props-rule-2026-5-6-2-25-0.md | 1 + 1 file changed, 1 insertion(+) diff --git a/.chronus/changes/add-arm-no-replace-inherited-props-rule-2026-5-6-2-25-0.md b/.chronus/changes/add-arm-no-replace-inherited-props-rule-2026-5-6-2-25-0.md index 8f1a4264cf..2005e1fc16 100644 --- a/.chronus/changes/add-arm-no-replace-inherited-props-rule-2026-5-6-2-25-0.md +++ b/.chronus/changes/add-arm-no-replace-inherited-props-rule-2026-5-6-2-25-0.md @@ -2,6 +2,7 @@ changeKind: feature packages: - "@azure-tools/typespec-azure-resource-manager" + - "@azure-tools/typespec-azure-rulesets" --- Add new linting rule `arm-no-replace-inherited-props` that warns when a model redefines a property that is already defined in one of its base models. The 'name' property of an ARM resource and properties redefined as part of a model marked with `@discriminator` are not flagged by this rule. From 5180f695a52c1a6ddfd90d8f7845001982308c5e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 13 May 2026 00:07:08 +0000 Subject: [PATCH 4/6] Allow scalar-compatible inherited-property overrides; simplify rule Agent-Logs-Url: https://github.com/Azure/typespec-azure/sessions/081cde0d-41a3-4e99-9080-b24bf0ef7de7 Co-authored-by: markcowl <1054056+markcowl@users.noreply.github.com> --- .../typespec-azure-resource-manager/README.md | 5 +- .../src/linter.ts | 5 +- .../rules/arm-no-replace-inherited-props.ts | 120 ++++++++++++---- .../arm-no-replace-inherited-props.test.ts | 130 ++++++++++++++---- .../rules/arm-no-replace-inherited-props.md | 15 +- 5 files changed, 204 insertions(+), 71 deletions(-) diff --git a/packages/typespec-azure-resource-manager/README.md b/packages/typespec-azure-resource-manager/README.md index 7df87fd5f2..02273882c4 100644 --- a/packages/typespec-azure-resource-manager/README.md +++ b/packages/typespec-azure-resource-manager/README.md @@ -29,11 +29,8 @@ Available ruleSets: | Name | Description | | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | [`@azure-tools/typespec-azure-resource-manager/arm-no-record`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/arm-no-record) | Don't use Record types for ARM resources. | -<<<<<<< HEAD -| [`@azure-tools/typespec-azure-resource-manager/arm-no-replace-inherited-props`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/arm-no-replace-inherited-props) | Disallow redefining properties already defined in a base type. | -======= | [`@azure-tools/typespec-azure-resource-manager/arm-no-path-casing-conflicts`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/arm-no-path-casing-conflicts) | Operation paths must be unique when compared case-insensitively. | ->>>>>>> origin/main +| [`@azure-tools/typespec-azure-resource-manager/arm-no-replace-inherited-props`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/arm-no-replace-inherited-props) | Disallow redefining properties already defined in a base type. | | [`@azure-tools/typespec-azure-resource-manager/arm-common-types-version`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/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. | diff --git a/packages/typespec-azure-resource-manager/src/linter.ts b/packages/typespec-azure-resource-manager/src/linter.ts index 300dcecb5a..2af91fc919 100644 --- a/packages/typespec-azure-resource-manager/src/linter.ts +++ b/packages/typespec-azure-resource-manager/src/linter.ts @@ -39,11 +39,8 @@ import { versionProgressionRule } from "./rules/version-progression.js"; const rules = [ armNoRecordRule, -<<<<<<< HEAD - armNoReplaceInheritedPropsRule, -======= armNoPathCasingConflictsRule, ->>>>>>> origin/main + armNoReplaceInheritedPropsRule, armCommonTypesVersionRule, armDeleteResponseCodesRule, armPutResponseCodesRule, diff --git a/packages/typespec-azure-resource-manager/src/rules/arm-no-replace-inherited-props.ts b/packages/typespec-azure-resource-manager/src/rules/arm-no-replace-inherited-props.ts index cbfe8e61c9..740bcb31f2 100644 --- a/packages/typespec-azure-resource-manager/src/rules/arm-no-replace-inherited-props.ts +++ b/packages/typespec-azure-resource-manager/src/rules/arm-no-replace-inherited-props.ts @@ -1,6 +1,69 @@ -import { createRule, getDiscriminator, Model, paramMessage } from "@typespec/compiler"; +import { createRule, Model, paramMessage, Scalar, Type } from "@typespec/compiler"; -import { getArmResource } from "../resource.js"; +type ScalarFamily = "string" | "numeric" | "boolean"; + +/** + * Determine if `type` is "logically" a value of one of the well-known scalar + * families (string, numeric, boolean). Walks unions, enums, scalar-extends + * chains, and literals. + * + * Returns the standard scalar family name if `type` reduces to a single + * family, or `undefined` otherwise (e.g. model types, mixed unions, custom + * non-numeric/non-string scalars such as utcDateTime, etc.). + */ +function getScalarFamily(type: Type): ScalarFamily | undefined { + switch (type.kind) { + case "Scalar": { + // Walk up baseScalar chain to the root scalar. + let root: Scalar = type; + while (root.baseScalar !== undefined) { + root = root.baseScalar; + } + switch (root.name) { + case "string": + return "string"; + case "boolean": + return "boolean"; + case "numeric": + return "numeric"; + default: + return undefined; + } + } + case "String": + case "StringTemplate": + return "string"; + case "Number": + return "numeric"; + case "Boolean": + return "boolean"; + case "Enum": { + const families = new Set(); + for (const member of type.members.values()) { + if (typeof member.value === "number") { + families.add("numeric"); + } else { + // Members with string values or no explicit value are string-typed. + families.add("string"); + } + } + return families.size === 1 ? [...families][0] : undefined; + } + case "EnumMember": + return typeof type.value === "number" ? "numeric" : "string"; + case "Union": { + const families = new Set(); + for (const variant of type.variants.values()) { + const family = getScalarFamily(variant.type); + if (family === undefined) return undefined; + families.add(family); + } + return families.size === 1 ? [...families][0] : undefined; + } + default: + return undefined; + } +} export const armNoReplaceInheritedPropsRule = createRule({ name: "arm-no-replace-inherited-props", @@ -15,43 +78,40 @@ export const armNoReplaceInheritedPropsRule = createRule({ model: (model: Model) => { if (model.baseModel === undefined) return; - // Collect discriminator property names from any ancestor model with `@discriminator`. - const discriminatorProps = new Set(); - let ancestor: Model | undefined = model.baseModel; - while (ancestor !== undefined) { - const discriminator = getDiscriminator(context.program, ancestor); - if (discriminator !== undefined) { - discriminatorProps.add(discriminator.propertyName); - } - ancestor = ancestor.baseModel; - } - - const isArmResource = getArmResource(context.program, model) !== undefined; - for (const property of model.properties.values()) { - // Skip the 'name' property of any ARM resource. - if (isArmResource && property.name === "name") continue; - - // Skip discriminator properties redefined in derived models. - if (discriminatorProps.has(property.name)) continue; - - // Look up the chain of base models for a property with the same name. - let baseHasProperty = false; + // Walk the chain of base models looking for a property with the + // same name. + let inherited: typeof property | undefined; let current: Model | undefined = model.baseModel; while (current !== undefined) { - if (current.properties.has(property.name)) { - baseHasProperty = true; + const found = current.properties.get(property.name); + if (found !== undefined) { + inherited = found; break; } current = current.baseModel; } - if (baseHasProperty) { - context.reportDiagnostic({ - format: { propertyName: property.name }, - target: property, - }); + if (inherited === undefined) continue; + + // Allow overriding when both the inherited property and the override + // resolve to the same scalar family (e.g. both are "string" + // -- including string scalars, string-derived scalars, string + // literals, string enums, and open or closed string unions). + const overrideFamily = getScalarFamily(property.type); + const inheritedFamily = getScalarFamily(inherited.type); + if ( + overrideFamily !== undefined && + inheritedFamily !== undefined && + overrideFamily === inheritedFamily + ) { + continue; } + + context.reportDiagnostic({ + format: { propertyName: property.name }, + target: property, + }); } }, }; diff --git a/packages/typespec-azure-resource-manager/test/rules/arm-no-replace-inherited-props.test.ts b/packages/typespec-azure-resource-manager/test/rules/arm-no-replace-inherited-props.test.ts index bbeb2a63b4..df9ef3806b 100644 --- a/packages/typespec-azure-resource-manager/test/rules/arm-no-replace-inherited-props.test.ts +++ b/packages/typespec-azure-resource-manager/test/rules/arm-no-replace-inherited-props.test.ts @@ -38,19 +38,22 @@ it("succeeds when child model does not redefine inherited properties", async () .toBeValid(); }); -it("emits warning when child model redefines an inherited property", async () => { +it("warns when child model redefines an inherited model-typed property with the same model", async () => { await tester .expect( ` @armProviderNamespace namespace MyService; + model Inner { + a: string; + } + model Base { - id: string; - commonProp: string; + nested: Inner; } model Child extends Base { - commonProp: string; + nested: Inner; } `, ) @@ -58,19 +61,21 @@ it("emits warning when child model redefines an inherited property", async () => { code: "@azure-tools/typespec-azure-resource-manager/arm-no-replace-inherited-props", message: - "The property 'commonProp' is also defined in the base model. Redefining inherited properties can cause problems with OpenAPI tooling and some language representations of the models.", + "The property 'nested' is also defined in the base model. Redefining inherited properties can cause problems with OpenAPI tooling and some language representations of the models.", }, ]); }); -it("emits warning when redefining a property from an indirect ancestor", async () => { +it("warns when redefining a model-typed property from an indirect ancestor", async () => { await tester .expect( ` @armProviderNamespace namespace MyService; + model Inner { a: string; } + model GrandParent { - sharedProp: string; + sharedProp: Inner; } model Parent extends GrandParent { @@ -78,7 +83,7 @@ it("emits warning when redefining a property from an indirect ancestor", async ( } model Child extends Parent { - sharedProp: string; + sharedProp: Inner; } `, ) @@ -91,42 +96,119 @@ it("emits warning when redefining a property from an indirect ancestor", async ( ]); }); -it("does not warn when redefining the 'name' property of an ARM resource", async () => { +it("allows overriding an inherited string property with another string", async () => { await tester .expect( ` @armProviderNamespace namespace MyService; - model FooResource is TrackedResource<{}> { - @key @segment("foo") name: string; + model Base { + commonProp: string; + } + + model Child extends Base { + commonProp: string; } `, ) .toBeValid(); }); -it("does not warn when redefining a discriminator property in a derived model", async () => { +it("allows overriding an inherited string property with a derived scalar", async () => { await tester .expect( ` @armProviderNamespace namespace MyService; - @discriminator("kind") - model Pet { - kind: string; + scalar myString extends string; + + model Base { + commonProp: string; + } + + model Child extends Base { + commonProp: myString; + } + `, + ) + .toBeValid(); +}); + +it("allows overriding an inherited string property with a closed string union", async () => { + await tester + .expect( + ` + @armProviderNamespace namespace MyService; + + model Base { + commonProp: string; + } + + model Child extends Base { + commonProp: "a" | "b"; + } + `, + ) + .toBeValid(); +}); + +it("allows overriding an inherited string property with an open string union", async () => { + await tester + .expect( + ` + @armProviderNamespace namespace MyService; + + union OpenStrings { + string, + "a", + "b", + } + + model Base { + commonProp: string; + } + + model Child extends Base { + commonProp: OpenStrings; + } + `, + ) + .toBeValid(); +}); + +it("allows overriding an inherited numeric property with the same numeric type", async () => { + await tester + .expect( + ` + @armProviderNamespace namespace MyService; + + model Base { weight: int32; } - model Cat extends Pet { - kind: "cat"; - meow: boolean; + model Child extends Base { + weight: int32; + } + `, + ) + .toBeValid(); +}); + +it("allows overriding the 'name' property of an ARM resource (string is compatible)", async () => { + await tester + .expect( + ` + @armProviderNamespace namespace MyService; + + model FooResource is TrackedResource<{}> { + @key @segment("foo") name: string; } `, ) .toBeValid(); }); -it("warns on non-discriminator inherited properties even when base has @discriminator", async () => { +it("allows overriding a discriminator property with a string literal", async () => { await tester .expect( ` @@ -140,15 +222,9 @@ it("warns on non-discriminator inherited properties even when base has @discrimi model Cat extends Pet { kind: "cat"; - weight: int32; + meow: boolean; } `, ) - .toEmitDiagnostics([ - { - code: "@azure-tools/typespec-azure-resource-manager/arm-no-replace-inherited-props", - message: - "The property 'weight' is also defined in the base model. Redefining inherited properties can cause problems with OpenAPI tooling and some language representations of the models.", - }, - ]); + .toBeValid(); }); diff --git a/website/src/content/docs/docs/libraries/azure-resource-manager/rules/arm-no-replace-inherited-props.md b/website/src/content/docs/docs/libraries/azure-resource-manager/rules/arm-no-replace-inherited-props.md index 6a4b992b2f..9dd443dd33 100644 --- a/website/src/content/docs/docs/libraries/azure-resource-manager/rules/arm-no-replace-inherited-props.md +++ b/website/src/content/docs/docs/libraries/azure-resource-manager/rules/arm-no-replace-inherited-props.md @@ -8,7 +8,7 @@ title: arm-no-replace-inherited-props Warns when a model redefines a property that is already defined in one of its base models. Repeating inherited properties in a child model is an anti-pattern for Azure APIs and can cause problems with OpenAPI tooling and some language representations of the models. -The `name` property of an ARM resource and properties redefined as part of a model marked with `@discriminator` are not flagged by this rule. +Overriding an inherited property is allowed when both the inherited property and the overriding property are "compatible scalars" — that is, both reduce to the same scalar family (`string`, `numeric`, or `boolean`). For example, an inherited property of type `string` may be overridden by a `string`, by a scalar that extends `string`, by a string literal, by a string-valued enum, or by an open or closed string union. #### ❌ Incorrect @@ -16,13 +16,16 @@ The `name` property of an ARM resource and properties redefined as part of a mod @armProviderNamespace namespace MyService; +model Inner { + a: string; +} + model Base { - id: string; - commonProp: string; + nested: Inner; } model Child extends Base { - commonProp: string; // duplicate of inherited "commonProp" + nested: Inner; // duplicate of inherited "nested" } ``` @@ -42,7 +45,7 @@ model Child extends Base { } ``` -#### ✅ Correct (discriminator property) +#### ✅ Correct (compatible scalar override) ```tsp @armProviderNamespace @@ -55,7 +58,7 @@ model Pet { } model Cat extends Pet { - kind: "cat"; // allowed: 'kind' is a discriminator property + kind: "cat"; // allowed: string and string literal are compatible scalars meow: boolean; } ``` From 826590c2a7145c589e3207917c4315801bee49eb Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 14 May 2026 20:25:14 +0000 Subject: [PATCH 5/6] Use getUnionAsEnum helper; add tests for scalar/union/enum variant overrides Agent-Logs-Url: https://github.com/Azure/typespec-azure/sessions/e17694c7-bbb6-4162-961f-041bb6fcf475 Co-authored-by: markcowl <1054056+markcowl@users.noreply.github.com> --- .../rules/arm-no-replace-inherited-props.ts | 26 ++++--- .../arm-no-replace-inherited-props.test.ts | 78 +++++++++++++++++++ 2 files changed, 95 insertions(+), 9 deletions(-) diff --git a/packages/typespec-azure-resource-manager/src/rules/arm-no-replace-inherited-props.ts b/packages/typespec-azure-resource-manager/src/rules/arm-no-replace-inherited-props.ts index 740bcb31f2..ac94b7f307 100644 --- a/packages/typespec-azure-resource-manager/src/rules/arm-no-replace-inherited-props.ts +++ b/packages/typespec-azure-resource-manager/src/rules/arm-no-replace-inherited-props.ts @@ -1,11 +1,19 @@ -import { createRule, Model, paramMessage, Scalar, Type } from "@typespec/compiler"; +import { getUnionAsEnum } from "@azure-tools/typespec-azure-core"; +import { + createRule, + ignoreDiagnostics, + Model, + paramMessage, + Scalar, + Type, +} from "@typespec/compiler"; type ScalarFamily = "string" | "numeric" | "boolean"; /** * Determine if `type` is "logically" a value of one of the well-known scalar * families (string, numeric, boolean). Walks unions, enums, scalar-extends - * chains, and literals. + * chains, union variants, and literals. * * Returns the standard scalar family name if `type` reduces to a single * family, or `undefined` otherwise (e.g. model types, mixed unions, custom @@ -51,14 +59,14 @@ function getScalarFamily(type: Type): ScalarFamily | undefined { } case "EnumMember": return typeof type.value === "number" ? "numeric" : "string"; + case "UnionVariant": + return getScalarFamily(type.type); case "Union": { - const families = new Set(); - for (const variant of type.variants.values()) { - const family = getScalarFamily(variant.type); - if (family === undefined) return undefined; - families.add(family); - } - return families.size === 1 ? [...families][0] : undefined; + // Use the azure-core helper to classify the union as a string- or + // number-typed open/closed enum. + const unionEnum = ignoreDiagnostics(getUnionAsEnum(type)); + if (unionEnum === undefined) return undefined; + return unionEnum.kind === "string" ? "string" : "numeric"; } default: return undefined; diff --git a/packages/typespec-azure-resource-manager/test/rules/arm-no-replace-inherited-props.test.ts b/packages/typespec-azure-resource-manager/test/rules/arm-no-replace-inherited-props.test.ts index df9ef3806b..8eff0272e1 100644 --- a/packages/typespec-azure-resource-manager/test/rules/arm-no-replace-inherited-props.test.ts +++ b/packages/typespec-azure-resource-manager/test/rules/arm-no-replace-inherited-props.test.ts @@ -228,3 +228,81 @@ it("allows overriding a discriminator property with a string literal", async () ) .toBeValid(); }); + +it("allows overriding an inherited string property with a specific string literal value", async () => { + await tester + .expect( + ` + @armProviderNamespace namespace MyService; + + model Parent { + prop: string; + } + + model Child extends Parent { + prop: "foo"; + } + `, + ) + .toBeValid(); +}); + +it("allows overriding an inherited numeric (int32) property with a specific numeric literal value", async () => { + await tester + .expect( + ` + @armProviderNamespace namespace MyService; + + model Parent { + prop: int32; + } + + model Child extends Parent { + prop: 12; + } + `, + ) + .toBeValid(); +}); + +it("allows overriding an inherited anonymous closed string union with one of its variants", async () => { + await tester + .expect( + ` + @armProviderNamespace namespace MyService; + + model Parent { + prop: "foo" | "bar"; + } + + model Child extends Parent { + prop: "foo"; + } + `, + ) + .toBeValid(); +}); + +it("allows overriding an inherited named open string union with one of its named variants", async () => { + await tester + .expect( + ` + @armProviderNamespace namespace MyService; + + union PropType { + Foo: "foo", + Bar: "bar", + string, + } + + model Parent { + prop: PropType; + } + + model Child extends Parent { + prop: PropType.Foo; + } + `, + ) + .toBeValid(); +}); From 2215445ea6addf74cf150338e3bc43f370216003 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 29 May 2026 21:33:41 +0000 Subject: [PATCH 6/6] Allow same-type inherited property overrides (alias, is TrackedResource) Co-authored-by: markcowl <1054056+markcowl@users.noreply.github.com> --- .../rules/arm-no-replace-inherited-props.ts | 8 ++ .../arm-no-replace-inherited-props.test.ts | 97 +++++++++++++++++-- .../rules/arm-no-replace-inherited-props.md | 15 ++- 3 files changed, 108 insertions(+), 12 deletions(-) diff --git a/packages/typespec-azure-resource-manager/src/rules/arm-no-replace-inherited-props.ts b/packages/typespec-azure-resource-manager/src/rules/arm-no-replace-inherited-props.ts index ac94b7f307..d3b3202050 100644 --- a/packages/typespec-azure-resource-manager/src/rules/arm-no-replace-inherited-props.ts +++ b/packages/typespec-azure-resource-manager/src/rules/arm-no-replace-inherited-props.ts @@ -102,6 +102,14 @@ export const armNoReplaceInheritedPropsRule = createRule({ if (inherited === undefined) continue; + // Allow overriding with the exact same type (identity). Aliases in + // TypeSpec resolve to the same Type instance, and template + // instantiations are cached, so this naturally handles cases such + // as redefining `systemData: SystemData` (alias) where the parent + // uses `systemData: Foundations.SystemData`, or `tags: Record` + // cloned via `model X is TrackedResource<...>`. + if (property.type === inherited.type) continue; + // Allow overriding when both the inherited property and the override // resolve to the same scalar family (e.g. both are "string" // -- including string scalars, string-derived scalars, string diff --git a/packages/typespec-azure-resource-manager/test/rules/arm-no-replace-inherited-props.test.ts b/packages/typespec-azure-resource-manager/test/rules/arm-no-replace-inherited-props.test.ts index 8eff0272e1..36879637fd 100644 --- a/packages/typespec-azure-resource-manager/test/rules/arm-no-replace-inherited-props.test.ts +++ b/packages/typespec-azure-resource-manager/test/rules/arm-no-replace-inherited-props.test.ts @@ -38,22 +38,26 @@ it("succeeds when child model does not redefine inherited properties", async () .toBeValid(); }); -it("warns when child model redefines an inherited model-typed property with the same model", async () => { +it("warns when child model redefines an inherited model-typed property with a derived model", async () => { await tester .expect( ` @armProviderNamespace namespace MyService; - model Inner { + model InnerBase { a: string; } + model InnerDerived extends InnerBase { + b: string; + } + model Base { - nested: Inner; + nested: InnerBase; } model Child extends Base { - nested: Inner; + nested: InnerDerived; } `, ) @@ -66,16 +70,17 @@ it("warns when child model redefines an inherited model-typed property with the ]); }); -it("warns when redefining a model-typed property from an indirect ancestor", async () => { +it("warns when redefining a model-typed property from an indirect ancestor with a derived model", async () => { await tester .expect( ` @armProviderNamespace namespace MyService; - model Inner { a: string; } + model InnerBase { a: string; } + model InnerDerived extends InnerBase { b: string; } model GrandParent { - sharedProp: Inner; + sharedProp: InnerBase; } model Parent extends GrandParent { @@ -83,7 +88,7 @@ it("warns when redefining a model-typed property from an indirect ancestor", asy } model Child extends Parent { - sharedProp: Inner; + sharedProp: InnerDerived; } `, ) @@ -96,6 +101,82 @@ it("warns when redefining a model-typed property from an indirect ancestor", asy ]); }); +it("allows overriding an inherited model-typed property with the exact same model type", async () => { + await tester + .expect( + ` + @armProviderNamespace namespace MyService; + + model Inner { a: string; } + + model Base { + nested: Inner; + } + + model Child extends Base { + nested: Inner; + } + `, + ) + .toBeValid(); +}); + +it("allows overriding an inherited model-typed property through an alias that resolves to the same model", async () => { + await tester + .expect( + ` + @armProviderNamespace namespace MyService; + + model Inner { a: string; } + alias InnerAlias = Inner; + + model Base { + nested: Inner; + } + + model Child extends Base { + nested: InnerAlias; + } + `, + ) + .toBeValid(); +}); + +it("allows the implicit 'tags' clone in 'model X is TrackedResource' (no explicit override)", async () => { + await tester + .expect( + ` + @armProviderNamespace + namespace Microsoft.Foo; + + model FooProperties { p: string; } + + model FooResource is TrackedResource { + ...ResourceNameParameter; + } + `, + ) + .toBeValid(); +}); + +it("allows explicitly redefining 'tags' on 'model X is TrackedResource' with the same Record type", async () => { + await tester + .expect( + ` + @armProviderNamespace + namespace Microsoft.Foo; + + model FooProperties { p: string; } + + model FooResource is TrackedResource { + ...ResourceNameParameter; + tags?: Record; + } + `, + ) + .toBeValid(); +}); + it("allows overriding an inherited string property with another string", async () => { await tester .expect( diff --git a/website/src/content/docs/docs/libraries/azure-resource-manager/rules/arm-no-replace-inherited-props.md b/website/src/content/docs/docs/libraries/azure-resource-manager/rules/arm-no-replace-inherited-props.md index 9dd443dd33..683706fb49 100644 --- a/website/src/content/docs/docs/libraries/azure-resource-manager/rules/arm-no-replace-inherited-props.md +++ b/website/src/content/docs/docs/libraries/azure-resource-manager/rules/arm-no-replace-inherited-props.md @@ -8,7 +8,10 @@ title: arm-no-replace-inherited-props Warns when a model redefines a property that is already defined in one of its base models. Repeating inherited properties in a child model is an anti-pattern for Azure APIs and can cause problems with OpenAPI tooling and some language representations of the models. -Overriding an inherited property is allowed when both the inherited property and the overriding property are "compatible scalars" — that is, both reduce to the same scalar family (`string`, `numeric`, or `boolean`). For example, an inherited property of type `string` may be overridden by a `string`, by a scalar that extends `string`, by a string literal, by a string-valued enum, or by an open or closed string union. +Overriding an inherited property is allowed when: + +- The overriding property has the **same type** as the inherited one (by identity). Type aliases resolve to the same underlying type, and template instantiations are cached, so this naturally covers cases such as redefining `systemData: SystemData` (alias) where the parent uses `systemData: Foundations.SystemData`, or the implicit `tags: Record` cloned into a model that uses `is TrackedResource<...>`. +- Both the inherited property and the overriding property are **compatible scalars** — that is, both reduce to the same scalar family (`string`, `numeric`, or `boolean`). For example, an inherited property of type `string` may be overridden by a `string`, by a scalar that extends `string`, by a string literal, by a string-valued enum, or by an open or closed string union. #### ❌ Incorrect @@ -16,16 +19,20 @@ Overriding an inherited property is allowed when both the inherited property and @armProviderNamespace namespace MyService; -model Inner { +model InnerBase { a: string; } +model InnerDerived extends InnerBase { + b: string; +} + model Base { - nested: Inner; + nested: InnerBase; } model Child extends Base { - nested: Inner; // duplicate of inherited "nested" + nested: InnerDerived; // narrows the inherited "nested" to a derived model } ```