From bc7c31580087dc91f6d42c5d2cd240dc1fc96afc Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 5 May 2026 02:25:56 +0000 Subject: [PATCH 01/14] Add arm-resource-required-operations rule and extend arm-resource-patch Agent-Logs-Url: https://github.com/Azure/typespec-azure/sessions/a7d5b431-f91c-48a7-8277-5f09cc7a4018 Co-authored-by: markcowl <1054056+markcowl@users.noreply.github.com> --- ...rce-required-operations-2026-5-5-2-15-0.md | 8 + .../src/linter.ts | 2 + .../src/rules/arm-resource-patch.ts | 234 ++++++++++++++++-- .../rules/arm-resource-required-operations.ts | 190 ++++++++++++++ .../src/rules/utils.ts | 42 ++++ .../arm-resource-required-operations.test.ts | 198 +++++++++++++++ .../test/rules/patch-operations.test.ts | 106 ++++++++ .../src/rulesets/resource-manager.ts | 1 + .../reference/linter.md | 1 + .../rules/arm-resource-patch.md | 9 + .../rules/arm-resource-required-operations.md | 70 ++++++ .../rules/no-resource-delete-operation.md | 6 + 12 files changed, 844 insertions(+), 23 deletions(-) create mode 100644 .chronus/changes/arm-resource-required-operations-2026-5-5-2-15-0.md create mode 100644 packages/typespec-azure-resource-manager/src/rules/arm-resource-required-operations.ts create mode 100644 packages/typespec-azure-resource-manager/test/rules/arm-resource-required-operations.test.ts create mode 100644 website/src/content/docs/docs/libraries/azure-resource-manager/rules/arm-resource-required-operations.md diff --git a/.chronus/changes/arm-resource-required-operations-2026-5-5-2-15-0.md b/.chronus/changes/arm-resource-required-operations-2026-5-5-2-15-0.md new file mode 100644 index 0000000000..85f4d94630 --- /dev/null +++ b/.chronus/changes/arm-resource-required-operations-2026-5-5-2-15-0.md @@ -0,0 +1,8 @@ +--- +changeKind: feature +packages: + - "@azure-tools/typespec-azure-resource-manager" + - "@azure-tools/typespec-azure-rulesets" +--- + +Add new `arm-resource-required-operations` linting rule that ensures every ARM resource declares the complete set of required lifecycle and list operations (singleton-aware; supersedes `no-resource-delete-operation`). Extend `arm-resource-patch` with `requiredInPatch`, `notUpdateableInPatch`, `missingMergePatch`, and `nonMergePatchContentType` diagnostics, with codefixes for making PATCH properties optional, removing non-updateable properties, and adding `@patch(#{ implicitOptionality: true })`. diff --git a/packages/typespec-azure-resource-manager/src/linter.ts b/packages/typespec-azure-resource-manager/src/linter.ts index 4d597f3e8c..2a94e96f8d 100644 --- a/packages/typespec-azure-resource-manager/src/linter.ts +++ b/packages/typespec-azure-resource-manager/src/linter.ts @@ -18,6 +18,7 @@ import { armResourceOperationsRule } from "./rules/arm-resource-operation-respon import { patchOperationsRule } from "./rules/arm-resource-patch.js"; import { armResourcePathInvalidCharsRule } from "./rules/arm-resource-path-invalid-chars.js"; import { armResourceProvisioningStateRule } from "./rules/arm-resource-provisioning-state-rule.js"; +import { armResourceRequiredOperationsRule } from "./rules/arm-resource-required-operations.js"; import { beyondNestingRule } from "./rules/beyond-nesting-levels.js"; import { coreOperationsRule } from "./rules/core-operations.js"; import { envelopePropertiesRules } from "./rules/envelope-properties.js"; @@ -49,6 +50,7 @@ const rules = [ armResourceOperationsRule, armResourcePathInvalidCharsRule, armResourceProvisioningStateRule, + armResourceRequiredOperationsRule, armCustomResourceNoKey, armCustomResourceUsageDiscourage, beyondNestingRule, diff --git a/packages/typespec-azure-resource-manager/src/rules/arm-resource-patch.ts b/packages/typespec-azure-resource-manager/src/rules/arm-resource-patch.ts index fbbcccc311..1b19b7c580 100644 --- a/packages/typespec-azure-resource-manager/src/rules/arm-resource-patch.ts +++ b/packages/typespec-azure-resource-manager/src/rules/arm-resource-patch.ts @@ -1,19 +1,29 @@ import { + CodeFix, DiagnosticMessages, LinterRuleContext, Model, ModelProperty, Operation, Program, + createAddDecoratorCodeFix, createRule, + defineCodeFix, getEffectiveModelType, + getLifecycleVisibilityEnum, getProperty, + getSourceLocation, + getVisibilityForClass, isErrorType, isType, paramMessage, } from "@typespec/compiler"; +import { SyntaxKind } from "@typespec/compiler/ast"; import { + getContentTypes, + getHeaderFieldName, getOperationVerb, + getPatchOptions, isBody, isBodyRoot, isHeader, @@ -22,7 +32,7 @@ import { } from "@typespec/http"; import { getArmResource } from "../resource.js"; -import { getSourceModel, isInternalTypeSpec } from "./utils.js"; +import { getSourceModel, getSourceProperty, isInternalTypeSpec } from "./utils.js"; export const patchOperationsRule = createRule({ name: "arm-resource-patch", @@ -33,6 +43,10 @@ export const patchOperationsRule = createRule({ default: "The request body of a PATCH must be a model with a subset of resource properties", missingTags: "Resource PATCH must contain the 'tags' property.", modelSuperset: paramMessage`Resource PATCH models must be a subset of the resource type. The following properties: [${"name"}] do not exist in resource Model '${"resourceModel"}'.`, + notUpdateableInPatch: paramMessage`Property '${"propertyName"}' is in the PATCH body but is not visible for the Update lifecycle phase on the resource; it cannot be updated and must be removed from the PATCH model.`, + requiredInPatch: paramMessage`Property '${"propertyName"}' is required in the PATCH body. PATCH body properties must all be optional so partial updates work.`, + missingMergePatch: paramMessage`PATCH operation '${"operationName"}' must use '@patch(#{implicitOptionality: true})' or wrap its body in 'MergePatchUpdate<>' so that the generated wire format is application/merge-patch+json.`, + nonMergePatchContentType: paramMessage`PATCH operation '${"operationName"}' specifies a content-type other than 'application/merge-patch+json'.`, }, create(context) { return { @@ -44,6 +58,7 @@ export const patchOperationsRule = createRule({ if (resourceType) { checkPatchModel(context, operation, resourceType); } + checkMergePatchSupport(context, operation); } } }, @@ -61,7 +76,9 @@ function checkPatchModel( context.reportDiagnostic({ target: operation, }); - } else if ( + return; + } + if ( resourceType.properties.has("tags") && !patchModel.some((p) => p.name === "tags" && p.type.kind === "Model") ) { @@ -69,32 +86,203 @@ function checkPatchModel( messageId: "missingTags", target: operation, }); - } else { - const resourceProperties = resourceType.properties.get("properties"); - const badProperties: ModelProperty[] = []; - for (const property of patchModel) { - const sourceModel = getSourceModel(property); - if (sourceModel === undefined || !getArmResource(context.program, sourceModel)) { - if ( - !getProperty(resourceType, property.name) && - (resourceProperties === undefined || - resourceProperties.type.kind !== "Model" || - !getProperty(resourceProperties.type, property.name)) - ) { - badProperties.push(property); - } + return; + } + const resourceProperties = resourceType.properties.get("properties"); + const badProperties: ModelProperty[] = []; + for (const property of patchModel) { + const sourceModel = getSourceModel(property); + if (sourceModel === undefined || !getArmResource(context.program, sourceModel)) { + if ( + !getProperty(resourceType, property.name) && + (resourceProperties === undefined || + resourceProperties.type.kind !== "Model" || + !getProperty(resourceProperties.type, property.name)) + ) { + badProperties.push(property); } } - if (badProperties.length > 0) + } + if (badProperties.length > 0) { + context.reportDiagnostic({ + messageId: "modelSuperset", + format: { + name: badProperties.flatMap((t) => t.name).join(", "), + resourceModel: resourceType.name, + }, + target: operation, + }); + } + + // Per-property checks: required-in-patch and not-updateable-in-patch. + checkPatchModelProperties(context, patchModel); +} + +function checkPatchModelProperties( + context: LinterRuleContext, + patchModel: ModelProperty[], +) { + const program = context.program; + const lifecycleEnum = getLifecycleVisibilityEnum(program); + const updateModifier = lifecycleEnum.members.get("Update"); + for (const property of patchModel) { + // Skip properties that come from an ARM resource model itself — those + // are produced by the ARM templates which already handle visibility and + // optionality correctly on the wire. + const sourceModel = getSourceModel(property); + if (sourceModel !== undefined && getArmResource(program, sourceModel)) { + continue; + } + + if (!property.optional) { context.reportDiagnostic({ - messageId: "modelSuperset", - format: { - name: badProperties.flatMap((t) => t.name).join(", "), - resourceModel: resourceType.name, - }, - target: operation, + messageId: "requiredInPatch", + format: { propertyName: property.name }, + target: property, + codefixes: [createMakeOptionalCodeFix(property)], }); + } + + // Visibility check: trace back to the original resource property and + // confirm it has the Update lifecycle visibility modifier. + const sourceProperty = getSourceProperty(property); + if (updateModifier !== undefined && sourceProperty !== undefined) { + const visibility = getVisibilityForClass(program, sourceProperty, lifecycleEnum); + if (visibility.size > 0 && !visibility.has(updateModifier)) { + context.reportDiagnostic({ + messageId: "notUpdateableInPatch", + format: { propertyName: property.name }, + target: property, + codefixes: [createRemovePropertyCodeFix(property)], + }); + } + } + } +} + +function checkMergePatchSupport( + context: LinterRuleContext, + operation: Operation, +) { + const program = context.program; + if (operationUsesMergePatch(program, operation)) { + // Even when implicit optionality is enabled the user may still have + // explicitly set a non-merge-patch content-type. That's caught below. + } else { + context.reportDiagnostic({ + messageId: "missingMergePatch", + format: { operationName: operation.name }, + target: operation, + codefixes: [ + createAddDecoratorCodeFix(operation, "patch", ["#{ implicitOptionality: true }"]), + ], + }); } + + const contentTypeProperty = findContentTypeProperty(program, operation); + if (contentTypeProperty !== undefined) { + const [contentTypes] = getContentTypes(contentTypeProperty); + if (contentTypes.length > 0) { + const allValid = contentTypes.every( + (ct) => ct === "application/merge-patch+json" || ct === "application/json", + ); + if (!allValid) { + context.reportDiagnostic({ + messageId: "nonMergePatchContentType", + format: { operationName: operation.name }, + target: contentTypeProperty, + }); + } + } + } +} + +function operationUsesMergePatch(program: Program, operation: Operation): boolean { + // 1. Direct @patch options on this operation or anywhere in its source chain. + let current: Operation | undefined = operation; + while (current !== undefined) { + const options = getPatchOptions(program, current); + if (options?.implicitOptionality === true) return true; + current = current.sourceOperation; + } + + // 2. Body type built using MergePatchUpdate<>. + const body = getBodyType(program, operation); + if (body && bodyUsesMergePatchTemplate(body)) return true; + + return false; +} + +function bodyUsesMergePatchTemplate(body: Model): boolean { + let m: Model | undefined = body; + const seen = new Set(); + while (m && !seen.has(m)) { + seen.add(m); + if (m.name === "MergePatchUpdate" || m.name === "MergePatchCreateOrUpdate") { + return true; + } + if (m.templateMapper !== undefined) { + for (const arg of m.templateMapper.args) { + if (isType(arg) && arg.kind === "Model") { + if (arg.name === "MergePatchUpdate" || arg.name === "MergePatchCreateOrUpdate") { + return true; + } + } + } + } + m = m.sourceModel ?? m.sourceModels[0]?.model; + } + return false; +} + +function getBodyType(program: Program, operation: Operation): Model | undefined { + const params = operation.parameters; + for (const property of params.properties.values()) { + if (isBody(program, property) || isBodyRoot(program, property)) { + if (property.type.kind === "Model") return property.type; + return undefined; + } + } + return undefined; +} + +function findContentTypeProperty( + program: Program, + operation: Operation, +): ModelProperty | undefined { + for (const property of operation.parameters.properties.values()) { + if (isHeader(program, property)) { + const headerName = getHeaderFieldName(program, property); + if (headerName?.toLowerCase() === "content-type") return property; + } + } + return undefined; +} + +function createMakeOptionalCodeFix(property: ModelProperty): CodeFix { + return defineCodeFix({ + id: "patch-property-make-optional", + label: `Make property '${property.name}' optional`, + fix: (context) => { + const node = property.node; + if (!node || node.kind !== SyntaxKind.ModelProperty) return; + const idLocation = getSourceLocation(node.id); + // appendText inserts at end position of node — append "?" right after the id token. + return context.appendText(idLocation, "?"); + }, + }); +} + +function createRemovePropertyCodeFix(property: ModelProperty): CodeFix { + return defineCodeFix({ + id: "patch-property-remove", + label: `Remove non-updateable property '${property.name}' from PATCH model`, + fix: (context) => { + const node = property.node; + if (!node || node.kind !== SyntaxKind.ModelProperty) return; + return context.replaceText(getSourceLocation(node), ""); + }, + }); } function getResourceModel(program: Program, operation: Operation): Model | undefined { diff --git a/packages/typespec-azure-resource-manager/src/rules/arm-resource-required-operations.ts b/packages/typespec-azure-resource-manager/src/rules/arm-resource-required-operations.ts new file mode 100644 index 0000000000..7dfa3f9b6d --- /dev/null +++ b/packages/typespec-azure-resource-manager/src/rules/arm-resource-required-operations.ts @@ -0,0 +1,190 @@ +import { + CodeFix, + createRule, + defineCodeFix, + getSourceLocation, + Interface, + Model, + paramMessage, + Program, +} from "@typespec/compiler"; +import { ArmResourceDetails, getArmResources, isSingletonResource } from "../resource.js"; +import { getInterface, getListCategories, isInternalTypeSpec } from "./utils.js"; + +type RequiredOperation = + | "read" + | "createOrUpdate" + | "delete" + | "list-by-resource-group" + | "list-by-subscription" + | "list-by-parent"; + +/** + * Verify that an ARM resource declares the complete set of required + * lifecycle and list operations for its kind. This rule is singleton-aware + * and is intended to supersede `no-resource-delete-operation`. + */ +export const armResourceRequiredOperationsRule = createRule({ + name: "arm-resource-required-operations", + severity: "warning", + url: "https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/arm-resource-required-operations", + description: + "Tracked, proxy, and extension ARM resources must define the complete set of required operations.", + messages: { + default: paramMessage`Resource '${"name"}' is missing required operations: [${"operations"}].`, + missingList: paramMessage`Tracked resource '${"name"}' must have a list-by-resource-group (and list-by-subscription) operation.`, + missingListByParent: paramMessage`Nested resource '${"name"}' must have a list-by-parent operation.`, + missingGet: paramMessage`Resource '${"name"}' must have a GET (read) operation.`, + missingCreateOrUpdate: paramMessage`Resource '${"name"}' must have a PUT (createOrUpdate) operation.`, + missingDelete: paramMessage`Resource '${"name"}' must have a delete operation.`, + }, + create(context) { + return { + model: (model: Model) => { + if (isInternalTypeSpec(context.program, model)) return; + const armResource = getArmResources(context.program).find((r) => r.typespecType === model); + if (!armResource) return; + // Skip @armVirtualResource and custom resources entirely. + if (armResource.kind === "Virtual" || armResource.kind === "Custom") return; + + const required = getRequiredOperationsForResource(context.program, armResource); + if (required.length === 0) return; + const present = getPresentOperations(armResource); + const missing = required.filter((op) => !present.has(op)); + if (missing.length === 0) return; + + const resourceInterface = getInterface(armResource); + const target = resourceInterface ?? model; + + if (missing.length === 1) { + const messageId = singleMessageId(missing[0]); + context.reportDiagnostic({ + messageId, + target, + format: { name: armResource.name }, + codefixes: buildCodefixes(missing, armResource.name, resourceInterface), + }); + return; + } + context.reportDiagnostic({ + target, + format: { name: armResource.name, operations: missing.join(", ") }, + codefixes: buildCodefixes(missing, armResource.name, resourceInterface), + }); + }, + }; + }, +}); + +function getRequiredOperationsForResource( + program: Program, + armResource: ArmResourceDetails, +): RequiredOperation[] { + const kind = armResource.kind; + if (kind !== "Tracked" && kind !== "Proxy" && kind !== "Extension") { + return []; + } + + const isSingleton = isSingletonResource(program, armResource.typespecType); + + if (kind === "Tracked") { + if (isSingleton) { + return ["read", "createOrUpdate"]; + } + return ["read", "createOrUpdate", "delete", "list-by-resource-group", "list-by-subscription"]; + } + + // Proxy / Extension + if (isSingleton) { + return ["read", "createOrUpdate"]; + } + return ["read", "createOrUpdate", "delete", "list-by-parent"]; +} + +function getPresentOperations(armResource: ArmResourceDetails): Set { + const present = new Set(); + const lifecycle = armResource.operations.lifecycle; + if (lifecycle?.read) present.add("read"); + if (lifecycle?.createOrUpdate) present.add("createOrUpdate"); + if (lifecycle?.delete) present.add("delete"); + const cats = getListCategories(armResource); + if (cats.bySubscription) present.add("list-by-subscription"); + if (cats.byResourceGroup) present.add("list-by-resource-group"); + if (cats.byParent) present.add("list-by-parent"); + return present; +} + +function singleMessageId( + op: RequiredOperation, +): + | "missingGet" + | "missingCreateOrUpdate" + | "missingDelete" + | "missingList" + | "missingListByParent" { + switch (op) { + case "read": + return "missingGet"; + case "createOrUpdate": + return "missingCreateOrUpdate"; + case "delete": + return "missingDelete"; + case "list-by-parent": + return "missingListByParent"; + case "list-by-resource-group": + case "list-by-subscription": + return "missingList"; + } +} + +function operationTemplate(op: RequiredOperation, resourceName: string): string { + switch (op) { + case "read": + return `read is ArmResourceRead<${resourceName}>;`; + case "createOrUpdate": + return `createOrUpdate is ArmResourceCreateOrReplaceAsync<${resourceName}>;`; + case "delete": + return `delete is ArmResourceDeleteWithoutOkAsync<${resourceName}>;`; + case "list-by-resource-group": + return `listByResourceGroup is ArmResourceListByParent<${resourceName}>;`; + case "list-by-subscription": + return `listBySubscription is ArmListBySubscription<${resourceName}>;`; + case "list-by-parent": + return `listByParent is ArmResourceListByParent<${resourceName}>;`; + } +} + +function buildCodefixes( + missing: RequiredOperation[], + resourceName: string, + resourceInterface: Interface | undefined, +): CodeFix[] | undefined { + if (!resourceInterface || !resourceInterface.node) return undefined; + const node = resourceInterface.node; + const fixes: CodeFix[] = []; + for (const op of missing) { + fixes.push(makeAddOperationFix(op, resourceName, node)); + } + return fixes; +} + +function makeAddOperationFix( + op: RequiredOperation, + resourceName: string, + node: NonNullable, +): CodeFix { + return defineCodeFix({ + id: `add-${op}-operation`, + label: `Add ${op} operation declaration`, + fix: (context) => { + const sourceLocation = getSourceLocation(node); + const file = sourceLocation.file; + // Insert just before the closing `}` of the interface body. + const insertPos = node.bodyRange.end - 1; + return context.prependText( + { file, pos: insertPos }, + ` ${operationTemplate(op, resourceName)}\n`, + ); + }, + }); +} diff --git a/packages/typespec-azure-resource-manager/src/rules/utils.ts b/packages/typespec-azure-resource-manager/src/rules/utils.ts index 6d457e02b9..5f7b9b3158 100644 --- a/packages/typespec-azure-resource-manager/src/rules/utils.ts +++ b/packages/typespec-azure-resource-manager/src/rules/utils.ts @@ -15,6 +15,48 @@ import { getResourceOperation } from "@typespec/rest"; import { ArmResourceOperation } from "../operations.js"; import { ArmResourceDetails, getArmResourceKind } from "../resource.js"; +/** Categorization of list operations for an ARM resource. */ +export interface ListCategories { + /** True if the resource has a list-by-subscription operation. */ + bySubscription: boolean; + /** True if the resource has a list-by-resource-group operation. */ + byResourceGroup: boolean; + /** True if the resource has a list-by-parent (nested) operation. */ + byParent: boolean; +} + +/** + * Examine the list operations registered on the given ARM resource and + * categorize each by its path prefix. The categorization rules mirror RPC + * §2.3 conventions: + * - A path containing more than one `/providers/` segment is considered a + * list-by-parent (nested) operation. + * - A path containing `/resourceGroups/{` is a list-by-resource-group. + * - A path containing `/subscriptions/{` (without `/resourceGroups/{`) is a + * list-by-subscription. + */ +export function getListCategories(armResource: ArmResourceDetails): ListCategories { + const categories: ListCategories = { + bySubscription: false, + byResourceGroup: false, + byParent: false, + }; + const lists = armResource.operations.lists; + if (!lists) return categories; + for (const op of Object.values(lists)) { + const path = op.path ?? ""; + const providersCount = (path.match(/\/providers\//g) ?? []).length; + if (providersCount > 1) { + categories.byParent = true; + } else if (/\/resourceGroups\/\{/.test(path)) { + categories.byResourceGroup = true; + } else if (/\/subscriptions\/\{/.test(path)) { + categories.bySubscription = true; + } + } + return categories; +} + /** * *@param target diff --git a/packages/typespec-azure-resource-manager/test/rules/arm-resource-required-operations.test.ts b/packages/typespec-azure-resource-manager/test/rules/arm-resource-required-operations.test.ts new file mode 100644 index 0000000000..13f8588c66 --- /dev/null +++ b/packages/typespec-azure-resource-manager/test/rules/arm-resource-required-operations.test.ts @@ -0,0 +1,198 @@ +import { Tester } from "#test/tester.js"; +import { + LinterRuleTester, + TesterInstance, + createLinterRuleTester, +} from "@typespec/compiler/testing"; +import { beforeEach, it } from "vitest"; + +import { armResourceRequiredOperationsRule } from "../../src/rules/arm-resource-required-operations.js"; + +let runner: TesterInstance; +let tester: LinterRuleTester; + +beforeEach(async () => { + runner = await Tester.createInstance(); + tester = createLinterRuleTester( + runner, + armResourceRequiredOperationsRule, + "@azure-tools/typespec-azure-resource-manager", + ); +}); + +it("is valid when tracked resource has the complete set of required operations", async () => { + await tester + .expect( + ` + @armProviderNamespace + namespace Microsoft.Foo; + + model Foo is TrackedResource<{}> { + @key @path @segment("foos") name: string; + } + + @armResourceOperations + interface FooOperations { + read is ArmResourceRead; + createOrUpdate is ArmResourceCreateOrReplaceAsync; + delete is ArmResourceDeleteWithoutOkAsync; + listByResourceGroup is ArmResourceListByParent; + listBySubscription is ArmListBySubscription; + } + `, + ) + .toBeValid(); +}); + +it("emits missingDelete when only the delete operation is missing", async () => { + await tester + .expect( + ` + @armProviderNamespace + namespace Microsoft.Foo; + + model Foo is TrackedResource<{}> { + @key @path @segment("foos") name: string; + } + + @armResourceOperations + interface FooOperations { + read is ArmResourceRead; + createOrUpdate is ArmResourceCreateOrReplaceAsync; + listByResourceGroup is ArmResourceListByParent; + listBySubscription is ArmListBySubscription; + } + `, + ) + .toEmitDiagnostics({ + code: "@azure-tools/typespec-azure-resource-manager/arm-resource-required-operations", + message: `Resource 'Foo' must have a delete operation.`, + }); +}); + +it("emits missingGet when only read is missing", async () => { + await tester + .expect( + ` + @armProviderNamespace + namespace Microsoft.Foo; + + model Foo is TrackedResource<{}> { + @key @path @segment("foos") name: string; + } + + @armResourceOperations + interface FooOperations { + createOrUpdate is ArmResourceCreateOrReplaceAsync; + delete is ArmResourceDeleteWithoutOkAsync; + listByResourceGroup is ArmResourceListByParent; + listBySubscription is ArmListBySubscription; + } + `, + ) + .toEmitDiagnostics({ + code: "@azure-tools/typespec-azure-resource-manager/arm-resource-required-operations", + message: `Resource 'Foo' must have a GET (read) operation.`, + }); +}); + +it("emits a single default diagnostic listing all missing operations when multiple are missing", async () => { + await tester + .expect( + ` + @armProviderNamespace + namespace Microsoft.Foo; + + model Foo is TrackedResource<{}> { + @key @path @segment("foos") name: string; + } + + @armResourceOperations + interface FooOperations { + createOrUpdate is ArmResourceCreateOrReplaceAsync; + } + `, + ) + .toEmitDiagnostics({ + code: "@azure-tools/typespec-azure-resource-manager/arm-resource-required-operations", + message: + "Resource 'Foo' is missing required operations: [read, delete, list-by-resource-group, list-by-subscription].", + }); +}); + +it("emits missingListByParent for a nested proxy resource", async () => { + await tester + .expect( + ` + @armProviderNamespace + namespace Microsoft.Foo; + + model Foo is TrackedResource<{}> { + @key @path @segment("foos") fooName: string; + } + + @parentResource(Foo) + model Bar is ProxyResource<{}> { + @key @path @segment("bars") barName: string; + } + + @armResourceOperations + interface FooOperations { + read is ArmResourceRead; + createOrUpdate is ArmResourceCreateOrReplaceAsync; + delete is ArmResourceDeleteWithoutOkAsync; + listByResourceGroup is ArmResourceListByParent; + listBySubscription is ArmListBySubscription; + } + + @armResourceOperations + interface BarOperations { + read is ArmResourceRead; + createOrUpdate is ArmResourceCreateOrReplaceAsync; + delete is ArmResourceDeleteWithoutOkAsync; + } + `, + ) + .toEmitDiagnostics({ + code: "@azure-tools/typespec-azure-resource-manager/arm-resource-required-operations", + message: `Nested resource 'Bar' must have a list-by-parent operation.`, + }); +}); + +it("does not emit missingDelete or missingList for a singleton tracked resource", async () => { + await tester + .expect( + ` + @armProviderNamespace + namespace Microsoft.Foo; + + @singleton + model Foo is TrackedResource<{}> { + @key @path @segment("foos") name: string; + } + + @armResourceOperations + interface FooOperations { + read is ArmResourceRead; + createOrUpdate is ArmResourceCreateOrReplaceAsync; + } + `, + ) + .toBeValid(); +}); + +it("skips @armVirtualResource models", async () => { + await tester + .expect( + ` + @armProviderNamespace + namespace Microsoft.Foo; + + @armVirtualResource + model VirtualFoo { + @key @path @segment("virtualFoos") name: string; + } + `, + ) + .toBeValid(); +}); diff --git a/packages/typespec-azure-resource-manager/test/rules/patch-operations.test.ts b/packages/typespec-azure-resource-manager/test/rules/patch-operations.test.ts index 491f23e785..b94e74957d 100644 --- a/packages/typespec-azure-resource-manager/test/rules/patch-operations.test.ts +++ b/packages/typespec-azure-resource-manager/test/rules/patch-operations.test.ts @@ -174,3 +174,109 @@ it("emit diagnostic when there is no request body", async () => { message: "The request body of a PATCH must be a model with a subset of resource properties", }); }); + +it("emits requiredInPatch when a PATCH body property is required", async () => { + await tester + .expect( + ` + @armProviderNamespace + namespace Microsoft.Foo; + + model FooResource is TrackedResource { + @key("foo") @segment("foo") @path + name: string; + } + + model MyPatch { + tags?: Record; + displayName: string; + } + + @armResourceOperations + #suppress "deprecated" "test" + interface FooResources + extends ResourceRead, ResourceCreate, ResourceDelete { + @armResourceUpdate(FooResource) + @patch(#{implicitOptionality: true}) myFooUpdate(...ResourceInstanceParameters, @bodyRoot body: MyPatch) : ArmResponse | ErrorResponse; + } + + enum ResourceState { Succeeded, Canceled, Failed } + model FooProperties { + displayName?: string; + provisioningState: ResourceState; + } + `, + ) + .toEmitDiagnostics({ + code: "@azure-tools/typespec-azure-resource-manager/arm-resource-patch", + message: + "Property 'displayName' is required in the PATCH body. PATCH body properties must all be optional so partial updates work.", + }); +}); + +it("emits missingMergePatch when PATCH operation is not derived from an ARM template and has no implicit optionality", async () => { + await tester + .expect( + ` + @armProviderNamespace + namespace Microsoft.Foo; + + model FooResource is TrackedResource { + @key("foo") @segment("foo") @path + name: string; + } + + model MyPatch { + tags?: Record; + } + + @armResourceOperations + #suppress "deprecated" "test" + interface FooResources + extends ResourceRead, ResourceCreate, ResourceDelete { + @autoRoute + @armResourceUpdate(FooResource) + @patch(#{implicitOptionality: false}) + myFooUpdate(...ResourceInstanceParameters, @bodyRoot body: MyPatch) : ArmResponse | ErrorResponse; + } + + enum ResourceState { Succeeded, Canceled, Failed } + model FooProperties { + displayName?: string; + provisioningState: ResourceState; + } + `, + ) + .toEmitDiagnostics({ + code: "@azure-tools/typespec-azure-resource-manager/arm-resource-patch", + message: + "PATCH operation 'myFooUpdate' must use '@patch(#{implicitOptionality: true})' or wrap its body in 'MergePatchUpdate<>' so that the generated wire format is application/merge-patch+json.", + }); +}); + +it("does not emit missingMergePatch for a PATCH operation derived from ArmResourcePatchAsync", async () => { + await tester + .expect( + ` + @armProviderNamespace + namespace Microsoft.Foo; + + model FooResource is TrackedResource { + @key("foo") @segment("foo") @path + name: string; + } + + @armResourceOperations + interface FooResources { + update is ArmResourcePatchAsync; + } + + enum ResourceState { Succeeded, Canceled, Failed } + model FooProperties { + displayName?: string; + provisioningState: ResourceState; + } + `, + ) + .toBeValid(); +}); diff --git a/packages/typespec-azure-rulesets/src/rulesets/resource-manager.ts b/packages/typespec-azure-rulesets/src/rulesets/resource-manager.ts index 66046d52a5..9dd414d5a1 100644 --- a/packages/typespec-azure-rulesets/src/rulesets/resource-manager.ts +++ b/packages/typespec-azure-rulesets/src/rulesets/resource-manager.ts @@ -77,6 +77,7 @@ export default { "@azure-tools/typespec-azure-resource-manager/beyond-nesting-levels": true, "@azure-tools/typespec-azure-resource-manager/arm-resource-operation": true, "@azure-tools/typespec-azure-resource-manager/no-resource-delete-operation": true, + "@azure-tools/typespec-azure-resource-manager/arm-resource-required-operations": true, "@azure-tools/typespec-azure-resource-manager/empty-updateable-properties": true, "@azure-tools/typespec-azure-resource-manager/arm-resource-interface-requires-decorator": true, "@azure-tools/typespec-azure-resource-manager/arm-resource-invalid-action-verb": true, 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..9a2ec890ab 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 @@ -41,6 +41,7 @@ Available ruleSets: | [`@azure-tools/typespec-azure-resource-manager/beyond-nesting-levels`](/libraries/azure-resource-manager/rules/beyond-nesting-levels.md) | Tracked Resources must use 3 or fewer levels of nesting. | | [`@azure-tools/typespec-azure-resource-manager/arm-resource-operation`](/libraries/azure-resource-manager/rules/arm-resource-operation.md) | Validate ARM Resource operations. | | [`@azure-tools/typespec-azure-resource-manager/no-resource-delete-operation`](/libraries/azure-resource-manager/rules/no-resource-delete-operation.md) | Check for resources that must have a delete operation. | +| [`@azure-tools/typespec-azure-resource-manager/arm-resource-required-operations`](/libraries/azure-resource-manager/rules/arm-resource-required-operations.md) | ARM resources must declare the complete set of required lifecycle and list operations. Singleton-aware; supersedes `no-resource-delete-operation`. | | [`@azure-tools/typespec-azure-resource-manager/empty-updateable-properties`](/libraries/azure-resource-manager/rules/empty-updateable-properties.md) | Should have updateable properties. | | [`@azure-tools/typespec-azure-resource-manager/arm-resource-interface-requires-decorator`](/libraries/azure-resource-manager/rules/arm-resource-interface-requires-decorator.md) | Each resource interface must have an @armResourceOperations decorator. | | [`@azure-tools/typespec-azure-resource-manager/arm-resource-invalid-action-verb`](/libraries/azure-resource-manager/rules/arm-resource-invalid-action-verb.md) | Actions must be HTTP Post or Get operations. | diff --git a/website/src/content/docs/docs/libraries/azure-resource-manager/rules/arm-resource-patch.md b/website/src/content/docs/docs/libraries/azure-resource-manager/rules/arm-resource-patch.md index 86a7c8aefb..6e9e08e338 100644 --- a/website/src/content/docs/docs/libraries/azure-resource-manager/rules/arm-resource-patch.md +++ b/website/src/content/docs/docs/libraries/azure-resource-manager/rules/arm-resource-patch.md @@ -8,6 +8,15 @@ title: "arm-resource-patch" Validate ARM PATCH operations. The request body of a PATCH must be a model with a subset of the resource properties. The PATCH body must not contain properties that do not exist on the resource. +This rule additionally validates the following: + +- **Optional properties** (`requiredInPatch`): every property in a custom PATCH body model must be optional, so partial updates work as expected. +- **Visibility** (`notUpdateableInPatch`): properties present in a custom PATCH body model that map to resource properties without the `Update` lifecycle visibility cannot be updated and must be removed. +- **Merge-patch wire format** (`missingMergePatch`): every PATCH operation must use `@patch(#{ implicitOptionality: true })` (or be derived from one of the ARM `*PatchAsync`/`*PatchSync` templates) so that the generated wire format is `application/merge-patch+json`. +- **Content type** (`nonMergePatchContentType`): the explicit content-type on a PATCH operation must be `application/merge-patch+json` (or `application/json`, which is rewritten by the emitter under implicit optionality). + +See also [`patch-envelope`](./patch-envelope.md), which validates the envelope properties surrounding the PATCH body. + #### ❌ Incorrect ```tsp diff --git a/website/src/content/docs/docs/libraries/azure-resource-manager/rules/arm-resource-required-operations.md b/website/src/content/docs/docs/libraries/azure-resource-manager/rules/arm-resource-required-operations.md new file mode 100644 index 0000000000..843b546150 --- /dev/null +++ b/website/src/content/docs/docs/libraries/azure-resource-manager/rules/arm-resource-required-operations.md @@ -0,0 +1,70 @@ +--- +title: arm-resource-required-operations +--- + +```text title=- Full name- +@azure-tools/typespec-azure-resource-manager/arm-resource-required-operations +``` + +ARM resources must declare the complete set of required lifecycle and list +operations defined by the [ARM RPC contract][rpc] (sections 2.2 and 2.3). + +The required set depends on the resource kind: + +| Resource kind | Required operations | +| ----------------------------------------- | ------------------------------------------------------------------------------------ | +| Tracked | `read`, `createOrUpdate`, `delete`, `list-by-resource-group`, `list-by-subscription` | +| Proxy / Extension (top-level) | `read`, `createOrUpdate`, `delete` | +| Proxy / Extension (nested under a parent) | `read`, `createOrUpdate`, `delete`, `list-by-parent` | +| Singleton (any kind) | `read`, `createOrUpdate` only — no `delete`, no `list` | + +When more than one operation is missing, a single diagnostic is emitted that +lists every missing operation. When only one is missing, a more specific +message ID is used so editors and tooling can present a clearer hint. + +This rule is singleton-aware and supersedes +[`no-resource-delete-operation`](./no-resource-delete-operation.md). Both rules +are currently registered for backwards compatibility. + +#### ❌ Incorrect — tracked resource missing the delete and list operations + +```tsp +@armResourceOperations +interface EmployeeOperations { + read is ArmResourceRead; + createOrUpdate is ArmResourceCreateOrReplaceAsync; +} +``` + +#### ✅ Correct — complete operation set for a tracked resource + +```tsp +@armResourceOperations +interface EmployeeOperations { + read is ArmResourceRead; + createOrUpdate is ArmResourceCreateOrReplaceAsync; + delete is ArmResourceDeleteWithoutOkAsync; + listByResourceGroup is ArmResourceListByParent; + listBySubscription is ArmListBySubscription; +} +``` + +#### ✅ Correct — singleton resource (no delete or list required) + +```tsp +@singleton +model Settings is ProxyResource { + @key + @path + @segment("settings") + name: string; +} + +@armResourceOperations +interface SettingsOperations { + read is ArmResourceRead; + createOrUpdate is ArmResourceCreateOrReplaceAsync; +} +``` + +[rpc]: https://github.com/cloud-and-ai-microsoft/resource-provider-contract diff --git a/website/src/content/docs/docs/libraries/azure-resource-manager/rules/no-resource-delete-operation.md b/website/src/content/docs/docs/libraries/azure-resource-manager/rules/no-resource-delete-operation.md index f97e564bce..cbf2b19cf4 100644 --- a/website/src/content/docs/docs/libraries/azure-resource-manager/rules/no-resource-delete-operation.md +++ b/website/src/content/docs/docs/libraries/azure-resource-manager/rules/no-resource-delete-operation.md @@ -8,6 +8,12 @@ title: no-resource-delete-operation Every ARM resource that provides a create operation must also provide a delete operation. +> **See also:** This rule has been superseded by +> [`arm-resource-required-operations`](./arm-resource-required-operations.md), +> which is singleton-aware and additionally enforces presence of `read`, +> `createOrUpdate`, and the appropriate `list` operations. Both rules are +> currently registered for backwards compatibility. + #### ❌ Incorrect ```tsp From fa7e218cd56ea4f7a5be19a73e0526113ab5fe9a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 5 May 2026 18:10:26 +0000 Subject: [PATCH 02/14] Remove Rule 2 changes (arm-resource-patch extension) Agent-Logs-Url: https://github.com/Azure/typespec-azure/sessions/ebe47644-6824-4c23-869d-b9d738f7d9a0 Co-authored-by: markcowl <1054056+markcowl@users.noreply.github.com> --- ...rce-required-operations-2026-5-5-2-15-0.md | 2 +- .../src/rules/arm-resource-patch.ts | 234 ++---------------- .../test/rules/patch-operations.test.ts | 106 -------- .../rules/arm-resource-patch.md | 9 - 4 files changed, 24 insertions(+), 327 deletions(-) diff --git a/.chronus/changes/arm-resource-required-operations-2026-5-5-2-15-0.md b/.chronus/changes/arm-resource-required-operations-2026-5-5-2-15-0.md index 85f4d94630..6331d4f8d7 100644 --- a/.chronus/changes/arm-resource-required-operations-2026-5-5-2-15-0.md +++ b/.chronus/changes/arm-resource-required-operations-2026-5-5-2-15-0.md @@ -5,4 +5,4 @@ packages: - "@azure-tools/typespec-azure-rulesets" --- -Add new `arm-resource-required-operations` linting rule that ensures every ARM resource declares the complete set of required lifecycle and list operations (singleton-aware; supersedes `no-resource-delete-operation`). Extend `arm-resource-patch` with `requiredInPatch`, `notUpdateableInPatch`, `missingMergePatch`, and `nonMergePatchContentType` diagnostics, with codefixes for making PATCH properties optional, removing non-updateable properties, and adding `@patch(#{ implicitOptionality: true })`. +Add new `arm-resource-required-operations` linting rule that ensures every ARM resource declares the complete set of required lifecycle and list operations (singleton-aware; supersedes `no-resource-delete-operation`). diff --git a/packages/typespec-azure-resource-manager/src/rules/arm-resource-patch.ts b/packages/typespec-azure-resource-manager/src/rules/arm-resource-patch.ts index 1b19b7c580..fbbcccc311 100644 --- a/packages/typespec-azure-resource-manager/src/rules/arm-resource-patch.ts +++ b/packages/typespec-azure-resource-manager/src/rules/arm-resource-patch.ts @@ -1,29 +1,19 @@ import { - CodeFix, DiagnosticMessages, LinterRuleContext, Model, ModelProperty, Operation, Program, - createAddDecoratorCodeFix, createRule, - defineCodeFix, getEffectiveModelType, - getLifecycleVisibilityEnum, getProperty, - getSourceLocation, - getVisibilityForClass, isErrorType, isType, paramMessage, } from "@typespec/compiler"; -import { SyntaxKind } from "@typespec/compiler/ast"; import { - getContentTypes, - getHeaderFieldName, getOperationVerb, - getPatchOptions, isBody, isBodyRoot, isHeader, @@ -32,7 +22,7 @@ import { } from "@typespec/http"; import { getArmResource } from "../resource.js"; -import { getSourceModel, getSourceProperty, isInternalTypeSpec } from "./utils.js"; +import { getSourceModel, isInternalTypeSpec } from "./utils.js"; export const patchOperationsRule = createRule({ name: "arm-resource-patch", @@ -43,10 +33,6 @@ export const patchOperationsRule = createRule({ default: "The request body of a PATCH must be a model with a subset of resource properties", missingTags: "Resource PATCH must contain the 'tags' property.", modelSuperset: paramMessage`Resource PATCH models must be a subset of the resource type. The following properties: [${"name"}] do not exist in resource Model '${"resourceModel"}'.`, - notUpdateableInPatch: paramMessage`Property '${"propertyName"}' is in the PATCH body but is not visible for the Update lifecycle phase on the resource; it cannot be updated and must be removed from the PATCH model.`, - requiredInPatch: paramMessage`Property '${"propertyName"}' is required in the PATCH body. PATCH body properties must all be optional so partial updates work.`, - missingMergePatch: paramMessage`PATCH operation '${"operationName"}' must use '@patch(#{implicitOptionality: true})' or wrap its body in 'MergePatchUpdate<>' so that the generated wire format is application/merge-patch+json.`, - nonMergePatchContentType: paramMessage`PATCH operation '${"operationName"}' specifies a content-type other than 'application/merge-patch+json'.`, }, create(context) { return { @@ -58,7 +44,6 @@ export const patchOperationsRule = createRule({ if (resourceType) { checkPatchModel(context, operation, resourceType); } - checkMergePatchSupport(context, operation); } } }, @@ -76,9 +61,7 @@ function checkPatchModel( context.reportDiagnostic({ target: operation, }); - return; - } - if ( + } else if ( resourceType.properties.has("tags") && !patchModel.some((p) => p.name === "tags" && p.type.kind === "Model") ) { @@ -86,203 +69,32 @@ function checkPatchModel( messageId: "missingTags", target: operation, }); - return; - } - const resourceProperties = resourceType.properties.get("properties"); - const badProperties: ModelProperty[] = []; - for (const property of patchModel) { - const sourceModel = getSourceModel(property); - if (sourceModel === undefined || !getArmResource(context.program, sourceModel)) { - if ( - !getProperty(resourceType, property.name) && - (resourceProperties === undefined || - resourceProperties.type.kind !== "Model" || - !getProperty(resourceProperties.type, property.name)) - ) { - badProperties.push(property); - } - } - } - if (badProperties.length > 0) { - context.reportDiagnostic({ - messageId: "modelSuperset", - format: { - name: badProperties.flatMap((t) => t.name).join(", "), - resourceModel: resourceType.name, - }, - target: operation, - }); - } - - // Per-property checks: required-in-patch and not-updateable-in-patch. - checkPatchModelProperties(context, patchModel); -} - -function checkPatchModelProperties( - context: LinterRuleContext, - patchModel: ModelProperty[], -) { - const program = context.program; - const lifecycleEnum = getLifecycleVisibilityEnum(program); - const updateModifier = lifecycleEnum.members.get("Update"); - for (const property of patchModel) { - // Skip properties that come from an ARM resource model itself — those - // are produced by the ARM templates which already handle visibility and - // optionality correctly on the wire. - const sourceModel = getSourceModel(property); - if (sourceModel !== undefined && getArmResource(program, sourceModel)) { - continue; - } - - if (!property.optional) { - context.reportDiagnostic({ - messageId: "requiredInPatch", - format: { propertyName: property.name }, - target: property, - codefixes: [createMakeOptionalCodeFix(property)], - }); - } - - // Visibility check: trace back to the original resource property and - // confirm it has the Update lifecycle visibility modifier. - const sourceProperty = getSourceProperty(property); - if (updateModifier !== undefined && sourceProperty !== undefined) { - const visibility = getVisibilityForClass(program, sourceProperty, lifecycleEnum); - if (visibility.size > 0 && !visibility.has(updateModifier)) { - context.reportDiagnostic({ - messageId: "notUpdateableInPatch", - format: { propertyName: property.name }, - target: property, - codefixes: [createRemovePropertyCodeFix(property)], - }); - } - } - } -} - -function checkMergePatchSupport( - context: LinterRuleContext, - operation: Operation, -) { - const program = context.program; - if (operationUsesMergePatch(program, operation)) { - // Even when implicit optionality is enabled the user may still have - // explicitly set a non-merge-patch content-type. That's caught below. } else { - context.reportDiagnostic({ - messageId: "missingMergePatch", - format: { operationName: operation.name }, - target: operation, - codefixes: [ - createAddDecoratorCodeFix(operation, "patch", ["#{ implicitOptionality: true }"]), - ], - }); - } - - const contentTypeProperty = findContentTypeProperty(program, operation); - if (contentTypeProperty !== undefined) { - const [contentTypes] = getContentTypes(contentTypeProperty); - if (contentTypes.length > 0) { - const allValid = contentTypes.every( - (ct) => ct === "application/merge-patch+json" || ct === "application/json", - ); - if (!allValid) { - context.reportDiagnostic({ - messageId: "nonMergePatchContentType", - format: { operationName: operation.name }, - target: contentTypeProperty, - }); - } - } - } -} - -function operationUsesMergePatch(program: Program, operation: Operation): boolean { - // 1. Direct @patch options on this operation or anywhere in its source chain. - let current: Operation | undefined = operation; - while (current !== undefined) { - const options = getPatchOptions(program, current); - if (options?.implicitOptionality === true) return true; - current = current.sourceOperation; - } - - // 2. Body type built using MergePatchUpdate<>. - const body = getBodyType(program, operation); - if (body && bodyUsesMergePatchTemplate(body)) return true; - - return false; -} - -function bodyUsesMergePatchTemplate(body: Model): boolean { - let m: Model | undefined = body; - const seen = new Set(); - while (m && !seen.has(m)) { - seen.add(m); - if (m.name === "MergePatchUpdate" || m.name === "MergePatchCreateOrUpdate") { - return true; - } - if (m.templateMapper !== undefined) { - for (const arg of m.templateMapper.args) { - if (isType(arg) && arg.kind === "Model") { - if (arg.name === "MergePatchUpdate" || arg.name === "MergePatchCreateOrUpdate") { - return true; - } + const resourceProperties = resourceType.properties.get("properties"); + const badProperties: ModelProperty[] = []; + for (const property of patchModel) { + const sourceModel = getSourceModel(property); + if (sourceModel === undefined || !getArmResource(context.program, sourceModel)) { + if ( + !getProperty(resourceType, property.name) && + (resourceProperties === undefined || + resourceProperties.type.kind !== "Model" || + !getProperty(resourceProperties.type, property.name)) + ) { + badProperties.push(property); } } } - m = m.sourceModel ?? m.sourceModels[0]?.model; - } - return false; -} - -function getBodyType(program: Program, operation: Operation): Model | undefined { - const params = operation.parameters; - for (const property of params.properties.values()) { - if (isBody(program, property) || isBodyRoot(program, property)) { - if (property.type.kind === "Model") return property.type; - return undefined; - } - } - return undefined; -} - -function findContentTypeProperty( - program: Program, - operation: Operation, -): ModelProperty | undefined { - for (const property of operation.parameters.properties.values()) { - if (isHeader(program, property)) { - const headerName = getHeaderFieldName(program, property); - if (headerName?.toLowerCase() === "content-type") return property; - } + if (badProperties.length > 0) + context.reportDiagnostic({ + messageId: "modelSuperset", + format: { + name: badProperties.flatMap((t) => t.name).join(", "), + resourceModel: resourceType.name, + }, + target: operation, + }); } - return undefined; -} - -function createMakeOptionalCodeFix(property: ModelProperty): CodeFix { - return defineCodeFix({ - id: "patch-property-make-optional", - label: `Make property '${property.name}' optional`, - fix: (context) => { - const node = property.node; - if (!node || node.kind !== SyntaxKind.ModelProperty) return; - const idLocation = getSourceLocation(node.id); - // appendText inserts at end position of node — append "?" right after the id token. - return context.appendText(idLocation, "?"); - }, - }); -} - -function createRemovePropertyCodeFix(property: ModelProperty): CodeFix { - return defineCodeFix({ - id: "patch-property-remove", - label: `Remove non-updateable property '${property.name}' from PATCH model`, - fix: (context) => { - const node = property.node; - if (!node || node.kind !== SyntaxKind.ModelProperty) return; - return context.replaceText(getSourceLocation(node), ""); - }, - }); } function getResourceModel(program: Program, operation: Operation): Model | undefined { diff --git a/packages/typespec-azure-resource-manager/test/rules/patch-operations.test.ts b/packages/typespec-azure-resource-manager/test/rules/patch-operations.test.ts index 911cf6a17b..8e53d205d7 100644 --- a/packages/typespec-azure-resource-manager/test/rules/patch-operations.test.ts +++ b/packages/typespec-azure-resource-manager/test/rules/patch-operations.test.ts @@ -177,109 +177,3 @@ it("emit diagnostic when there is no request body", async () => { message: "The request body of a PATCH must be a model with a subset of resource properties", }); }); - -it("emits requiredInPatch when a PATCH body property is required", async () => { - await tester - .expect( - ` - @armProviderNamespace - namespace Microsoft.Foo; - - model FooResource is TrackedResource { - @key("foo") @segment("foo") @path - name: string; - } - - model MyPatch { - tags?: Record; - displayName: string; - } - - @armResourceOperations - #suppress "deprecated" "test" - interface FooResources - extends ResourceRead, ResourceCreate, ResourceDelete { - @armResourceUpdate(FooResource) - @patch(#{implicitOptionality: true}) myFooUpdate(...ResourceInstanceParameters, @bodyRoot body: MyPatch) : ArmResponse | ErrorResponse; - } - - enum ResourceState { Succeeded, Canceled, Failed } - model FooProperties { - displayName?: string; - provisioningState: ResourceState; - } - `, - ) - .toEmitDiagnostics({ - code: "@azure-tools/typespec-azure-resource-manager/arm-resource-patch", - message: - "Property 'displayName' is required in the PATCH body. PATCH body properties must all be optional so partial updates work.", - }); -}); - -it("emits missingMergePatch when PATCH operation is not derived from an ARM template and has no implicit optionality", async () => { - await tester - .expect( - ` - @armProviderNamespace - namespace Microsoft.Foo; - - model FooResource is TrackedResource { - @key("foo") @segment("foo") @path - name: string; - } - - model MyPatch { - tags?: Record; - } - - @armResourceOperations - #suppress "deprecated" "test" - interface FooResources - extends ResourceRead, ResourceCreate, ResourceDelete { - @autoRoute - @armResourceUpdate(FooResource) - @patch(#{implicitOptionality: false}) - myFooUpdate(...ResourceInstanceParameters, @bodyRoot body: MyPatch) : ArmResponse | ErrorResponse; - } - - enum ResourceState { Succeeded, Canceled, Failed } - model FooProperties { - displayName?: string; - provisioningState: ResourceState; - } - `, - ) - .toEmitDiagnostics({ - code: "@azure-tools/typespec-azure-resource-manager/arm-resource-patch", - message: - "PATCH operation 'myFooUpdate' must use '@patch(#{implicitOptionality: true})' or wrap its body in 'MergePatchUpdate<>' so that the generated wire format is application/merge-patch+json.", - }); -}); - -it("does not emit missingMergePatch for a PATCH operation derived from ArmResourcePatchAsync", async () => { - await tester - .expect( - ` - @armProviderNamespace - namespace Microsoft.Foo; - - model FooResource is TrackedResource { - @key("foo") @segment("foo") @path - name: string; - } - - @armResourceOperations - interface FooResources { - update is ArmResourcePatchAsync; - } - - enum ResourceState { Succeeded, Canceled, Failed } - model FooProperties { - displayName?: string; - provisioningState: ResourceState; - } - `, - ) - .toBeValid(); -}); diff --git a/website/src/content/docs/docs/libraries/azure-resource-manager/rules/arm-resource-patch.md b/website/src/content/docs/docs/libraries/azure-resource-manager/rules/arm-resource-patch.md index 6e9e08e338..86a7c8aefb 100644 --- a/website/src/content/docs/docs/libraries/azure-resource-manager/rules/arm-resource-patch.md +++ b/website/src/content/docs/docs/libraries/azure-resource-manager/rules/arm-resource-patch.md @@ -8,15 +8,6 @@ title: "arm-resource-patch" Validate ARM PATCH operations. The request body of a PATCH must be a model with a subset of the resource properties. The PATCH body must not contain properties that do not exist on the resource. -This rule additionally validates the following: - -- **Optional properties** (`requiredInPatch`): every property in a custom PATCH body model must be optional, so partial updates work as expected. -- **Visibility** (`notUpdateableInPatch`): properties present in a custom PATCH body model that map to resource properties without the `Update` lifecycle visibility cannot be updated and must be removed. -- **Merge-patch wire format** (`missingMergePatch`): every PATCH operation must use `@patch(#{ implicitOptionality: true })` (or be derived from one of the ARM `*PatchAsync`/`*PatchSync` templates) so that the generated wire format is `application/merge-patch+json`. -- **Content type** (`nonMergePatchContentType`): the explicit content-type on a PATCH operation must be `application/merge-patch+json` (or `application/json`, which is rewritten by the emitter under implicit optionality). - -See also [`patch-envelope`](./patch-envelope.md), which validates the envelope properties surrounding the PATCH body. - #### ❌ Incorrect ```tsp From 5b963c97bd4f21b4eb4eea0d3c6af5b0acd682eb Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 5 May 2026 21:28:32 +0000 Subject: [PATCH 03/14] Use resolveArmResources, unify list-by-parent semantics, add list-by-rg test Agent-Logs-Url: https://github.com/Azure/typespec-azure/sessions/6338dc59-3d54-4c13-8b8c-cc154adc2ff5 Co-authored-by: markcowl <1054056+markcowl@users.noreply.github.com> --- .../rules/arm-resource-required-operations.ts | 137 +++++++++++------- .../src/rules/utils.ts | 42 ------ .../arm-resource-required-operations.test.ts | 56 ++++++- .../rules/arm-resource-required-operations.md | 16 +- 4 files changed, 146 insertions(+), 105 deletions(-) diff --git a/packages/typespec-azure-resource-manager/src/rules/arm-resource-required-operations.ts b/packages/typespec-azure-resource-manager/src/rules/arm-resource-required-operations.ts index 7dfa3f9b6d..0ebde6b8d1 100644 --- a/packages/typespec-azure-resource-manager/src/rules/arm-resource-required-operations.ts +++ b/packages/typespec-azure-resource-manager/src/rules/arm-resource-required-operations.ts @@ -8,16 +8,15 @@ import { paramMessage, Program, } from "@typespec/compiler"; -import { ArmResourceDetails, getArmResources, isSingletonResource } from "../resource.js"; -import { getInterface, getListCategories, isInternalTypeSpec } from "./utils.js"; +import { resolveArmResources, ResolvedResource } from "../resource.js"; +import { isInternalTypeSpec } from "./utils.js"; type RequiredOperation = | "read" | "createOrUpdate" | "delete" - | "list-by-resource-group" - | "list-by-subscription" - | "list-by-parent"; + | "list-by-parent" + | "list-by-subscription"; /** * Verify that an ARM resource declares the complete set of required @@ -32,28 +31,45 @@ export const armResourceRequiredOperationsRule = createRule({ "Tracked, proxy, and extension ARM resources must define the complete set of required operations.", messages: { default: paramMessage`Resource '${"name"}' is missing required operations: [${"operations"}].`, - missingList: paramMessage`Tracked resource '${"name"}' must have a list-by-resource-group (and list-by-subscription) operation.`, - missingListByParent: paramMessage`Nested resource '${"name"}' must have a list-by-parent operation.`, + missingListBySubscription: paramMessage`Tracked resource '${"name"}' must have a list-by-subscription operation.`, + missingListByParent: paramMessage`Resource '${"name"}' must have a list-by-parent operation (list-by-resource-group satisfies this for tracked resources).`, missingGet: paramMessage`Resource '${"name"}' must have a GET (read) operation.`, missingCreateOrUpdate: paramMessage`Resource '${"name"}' must have a PUT (createOrUpdate) operation.`, missingDelete: paramMessage`Resource '${"name"}' must have a delete operation.`, }, create(context) { + // Resolve resources once for the program and reuse for every model visit. + let resourcesByModel: Map | undefined; + const getResources = (program: Program): Map => { + if (resourcesByModel !== undefined) return resourcesByModel; + resourcesByModel = new Map(); + const provider = resolveArmResources(program); + for (const resource of provider.resources ?? []) { + const list = resourcesByModel.get(resource.type); + if (list) list.push(resource); + else resourcesByModel.set(resource.type, [resource]); + } + return resourcesByModel; + }; + return { model: (model: Model) => { if (isInternalTypeSpec(context.program, model)) return; - const armResource = getArmResources(context.program).find((r) => r.typespecType === model); - if (!armResource) return; - // Skip @armVirtualResource and custom resources entirely. - if (armResource.kind === "Virtual" || armResource.kind === "Custom") return; + const resources = getResources(context.program).get(model); + if (!resources || resources.length === 0) return; + // A resource model can have multiple resolved entries (e.g., distinct + // operation paths). Treat the union of their operations as the + // operations declared for this model. + const armResource = resources[0]; + if (armResource.kind === "Other") return; - const required = getRequiredOperationsForResource(context.program, armResource); + const required = getRequiredOperationsForResource(armResource); if (required.length === 0) return; - const present = getPresentOperations(armResource); + const present = getPresentOperations(resources); const missing = required.filter((op) => !present.has(op)); if (missing.length === 0) return; - const resourceInterface = getInterface(armResource); + const resourceInterface = getResolvedResourceInterface(resources); const target = resourceInterface ?? model; if (missing.length === 1) { @@ -61,67 +77,81 @@ export const armResourceRequiredOperationsRule = createRule({ context.reportDiagnostic({ messageId, target, - format: { name: armResource.name }, - codefixes: buildCodefixes(missing, armResource.name, resourceInterface), + format: { name: armResource.resourceName }, + codefixes: buildCodefixes(missing, armResource.resourceName, resourceInterface), }); return; } context.reportDiagnostic({ target, - format: { name: armResource.name, operations: missing.join(", ") }, - codefixes: buildCodefixes(missing, armResource.name, resourceInterface), + format: { name: armResource.resourceName, operations: missing.join(", ") }, + codefixes: buildCodefixes(missing, armResource.resourceName, resourceInterface), }); }, }; }, }); -function getRequiredOperationsForResource( - program: Program, - armResource: ArmResourceDetails, -): RequiredOperation[] { - const kind = armResource.kind; - if (kind !== "Tracked" && kind !== "Proxy" && kind !== "Extension") { - return []; - } - - const isSingleton = isSingletonResource(program, armResource.typespecType); - - if (kind === "Tracked") { - if (isSingleton) { - return ["read", "createOrUpdate"]; - } - return ["read", "createOrUpdate", "delete", "list-by-resource-group", "list-by-subscription"]; - } - - // Proxy / Extension +function getRequiredOperationsForResource(resource: ResolvedResource): RequiredOperation[] { + if (resource.kind === "Other") return []; + const isSingleton = resource.singleton !== undefined; if (isSingleton) { return ["read", "createOrUpdate"]; } - return ["read", "createOrUpdate", "delete", "list-by-parent"]; + // Every non-singleton resource requires read, createOrUpdate, delete, and a + // list-by-parent operation. For tracked resources at resource-group scope, + // list-by-resource-group satisfies the list-by-parent requirement. + const required: RequiredOperation[] = ["read", "createOrUpdate", "delete", "list-by-parent"]; + if (resource.kind === "Tracked") { + required.push("list-by-subscription"); + } + return required; } -function getPresentOperations(armResource: ArmResourceDetails): Set { +function getPresentOperations(resources: ResolvedResource[]): Set { const present = new Set(); - const lifecycle = armResource.operations.lifecycle; - if (lifecycle?.read) present.add("read"); - if (lifecycle?.createOrUpdate) present.add("createOrUpdate"); - if (lifecycle?.delete) present.add("delete"); - const cats = getListCategories(armResource); - if (cats.bySubscription) present.add("list-by-subscription"); - if (cats.byResourceGroup) present.add("list-by-resource-group"); - if (cats.byParent) present.add("list-by-parent"); + for (const resource of resources) { + const lifecycle = resource.operations.lifecycle; + if (lifecycle.read?.length) present.add("read"); + if (lifecycle.createOrUpdate?.length) present.add("createOrUpdate"); + if (lifecycle.delete?.length) present.add("delete"); + for (const op of resource.operations.lists ?? []) { + const path = op.path ?? ""; + const providersCount = (path.match(/\/providers\//g) ?? []).length; + if (providersCount > 1) { + // Nested list-by-parent. + present.add("list-by-parent"); + } else if (/\/resourceGroups\/\{/.test(path)) { + // List-by-resource-group satisfies list-by-parent for tracked resources. + present.add("list-by-parent"); + } else if (/\/subscriptions\/\{/.test(path)) { + present.add("list-by-subscription"); + } + } + } return present; } +function getResolvedResourceInterface(resources: ResolvedResource[]): Interface | undefined { + for (const resource of resources) { + for (const ops of Object.values(resource.operations.lifecycle)) { + if (!Array.isArray(ops)) continue; + for (const op of ops) { + if (op.operation.interface) return op.operation.interface; + } + } + } + return undefined; +} + function singleMessageId( op: RequiredOperation, ): | "missingGet" | "missingCreateOrUpdate" | "missingDelete" - | "missingList" - | "missingListByParent" { + | "missingListByParent" + | "missingListBySubscription" { switch (op) { case "read": return "missingGet"; @@ -131,9 +161,8 @@ function singleMessageId( return "missingDelete"; case "list-by-parent": return "missingListByParent"; - case "list-by-resource-group": case "list-by-subscription": - return "missingList"; + return "missingListBySubscription"; } } @@ -145,12 +174,10 @@ function operationTemplate(op: RequiredOperation, resourceName: string): string return `createOrUpdate is ArmResourceCreateOrReplaceAsync<${resourceName}>;`; case "delete": return `delete is ArmResourceDeleteWithoutOkAsync<${resourceName}>;`; - case "list-by-resource-group": - return `listByResourceGroup is ArmResourceListByParent<${resourceName}>;`; - case "list-by-subscription": - return `listBySubscription is ArmListBySubscription<${resourceName}>;`; case "list-by-parent": return `listByParent is ArmResourceListByParent<${resourceName}>;`; + case "list-by-subscription": + return `listBySubscription is ArmListBySubscription<${resourceName}>;`; } } diff --git a/packages/typespec-azure-resource-manager/src/rules/utils.ts b/packages/typespec-azure-resource-manager/src/rules/utils.ts index 5f7b9b3158..6d457e02b9 100644 --- a/packages/typespec-azure-resource-manager/src/rules/utils.ts +++ b/packages/typespec-azure-resource-manager/src/rules/utils.ts @@ -15,48 +15,6 @@ import { getResourceOperation } from "@typespec/rest"; import { ArmResourceOperation } from "../operations.js"; import { ArmResourceDetails, getArmResourceKind } from "../resource.js"; -/** Categorization of list operations for an ARM resource. */ -export interface ListCategories { - /** True if the resource has a list-by-subscription operation. */ - bySubscription: boolean; - /** True if the resource has a list-by-resource-group operation. */ - byResourceGroup: boolean; - /** True if the resource has a list-by-parent (nested) operation. */ - byParent: boolean; -} - -/** - * Examine the list operations registered on the given ARM resource and - * categorize each by its path prefix. The categorization rules mirror RPC - * §2.3 conventions: - * - A path containing more than one `/providers/` segment is considered a - * list-by-parent (nested) operation. - * - A path containing `/resourceGroups/{` is a list-by-resource-group. - * - A path containing `/subscriptions/{` (without `/resourceGroups/{`) is a - * list-by-subscription. - */ -export function getListCategories(armResource: ArmResourceDetails): ListCategories { - const categories: ListCategories = { - bySubscription: false, - byResourceGroup: false, - byParent: false, - }; - const lists = armResource.operations.lists; - if (!lists) return categories; - for (const op of Object.values(lists)) { - const path = op.path ?? ""; - const providersCount = (path.match(/\/providers\//g) ?? []).length; - if (providersCount > 1) { - categories.byParent = true; - } else if (/\/resourceGroups\/\{/.test(path)) { - categories.byResourceGroup = true; - } else if (/\/subscriptions\/\{/.test(path)) { - categories.bySubscription = true; - } - } - return categories; -} - /** * *@param target diff --git a/packages/typespec-azure-resource-manager/test/rules/arm-resource-required-operations.test.ts b/packages/typespec-azure-resource-manager/test/rules/arm-resource-required-operations.test.ts index 13f8588c66..39f4af4f7c 100644 --- a/packages/typespec-azure-resource-manager/test/rules/arm-resource-required-operations.test.ts +++ b/packages/typespec-azure-resource-manager/test/rules/arm-resource-required-operations.test.ts @@ -116,7 +116,59 @@ it("emits a single default diagnostic listing all missing operations when multip .toEmitDiagnostics({ code: "@azure-tools/typespec-azure-resource-manager/arm-resource-required-operations", message: - "Resource 'Foo' is missing required operations: [read, delete, list-by-resource-group, list-by-subscription].", + "Resource 'Foo' is missing required operations: [read, delete, list-by-parent, list-by-subscription].", + }); +}); + +it("emits missingListByParent for a tracked resource without a list-by-resource-group operation", async () => { + await tester + .expect( + ` + @armProviderNamespace + namespace Microsoft.Foo; + + model Foo is TrackedResource<{}> { + @key @path @segment("foos") name: string; + } + + @armResourceOperations + interface FooOperations { + read is ArmResourceRead; + createOrUpdate is ArmResourceCreateOrReplaceAsync; + delete is ArmResourceDeleteWithoutOkAsync; + listBySubscription is ArmListBySubscription; + } + `, + ) + .toEmitDiagnostics({ + code: "@azure-tools/typespec-azure-resource-manager/arm-resource-required-operations", + message: `Resource 'Foo' must have a list-by-parent operation (list-by-resource-group satisfies this for tracked resources).`, + }); +}); + +it("emits missingListBySubscription for a tracked resource without a list-by-subscription operation", async () => { + await tester + .expect( + ` + @armProviderNamespace + namespace Microsoft.Foo; + + model Foo is TrackedResource<{}> { + @key @path @segment("foos") name: string; + } + + @armResourceOperations + interface FooOperations { + read is ArmResourceRead; + createOrUpdate is ArmResourceCreateOrReplaceAsync; + delete is ArmResourceDeleteWithoutOkAsync; + listByResourceGroup is ArmResourceListByParent; + } + `, + ) + .toEmitDiagnostics({ + code: "@azure-tools/typespec-azure-resource-manager/arm-resource-required-operations", + message: `Tracked resource 'Foo' must have a list-by-subscription operation.`, }); }); @@ -155,7 +207,7 @@ it("emits missingListByParent for a nested proxy resource", async () => { ) .toEmitDiagnostics({ code: "@azure-tools/typespec-azure-resource-manager/arm-resource-required-operations", - message: `Nested resource 'Bar' must have a list-by-parent operation.`, + message: `Resource 'Bar' must have a list-by-parent operation (list-by-resource-group satisfies this for tracked resources).`, }); }); diff --git a/website/src/content/docs/docs/libraries/azure-resource-manager/rules/arm-resource-required-operations.md b/website/src/content/docs/docs/libraries/azure-resource-manager/rules/arm-resource-required-operations.md index 843b546150..02707dd550 100644 --- a/website/src/content/docs/docs/libraries/azure-resource-manager/rules/arm-resource-required-operations.md +++ b/website/src/content/docs/docs/libraries/azure-resource-manager/rules/arm-resource-required-operations.md @@ -11,12 +11,16 @@ operations defined by the [ARM RPC contract][rpc] (sections 2.2 and 2.3). The required set depends on the resource kind: -| Resource kind | Required operations | -| ----------------------------------------- | ------------------------------------------------------------------------------------ | -| Tracked | `read`, `createOrUpdate`, `delete`, `list-by-resource-group`, `list-by-subscription` | -| Proxy / Extension (top-level) | `read`, `createOrUpdate`, `delete` | -| Proxy / Extension (nested under a parent) | `read`, `createOrUpdate`, `delete`, `list-by-parent` | -| Singleton (any kind) | `read`, `createOrUpdate` only — no `delete`, no `list` | +| Resource kind | Required operations | +| -------------------- | -------------------------------------------------------------------------------------------------------------------- | +| Tracked | `read`, `createOrUpdate`, `delete`, `list-by-parent` (satisfied by `list-by-resource-group`), `list-by-subscription` | +| Proxy / Extension | `read`, `createOrUpdate`, `delete`, `list-by-parent` | +| Singleton (any kind) | `read`, `createOrUpdate` only — no `delete`, no `list` | + +For tracked resources, a `list-by-resource-group` operation satisfies the +`list-by-parent` requirement (the resource group is the parent in that scope). +For nested proxy or extension resources, `list-by-parent` refers to a list +operation under the parent resource's path. When more than one operation is missing, a single diagnostic is emitted that lists every missing operation. When only one is missing, a more specific From 38144d78a32d3bdaabf04f396807085f3d9f4f33 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 6 May 2026 02:39:44 +0000 Subject: [PATCH 04/14] Iterate ResolvedResources via root listener; aggregate per-model Agent-Logs-Url: https://github.com/Azure/typespec-azure/sessions/4c07b4b6-5c3e-40c7-b572-022659ae1a4e Co-authored-by: markcowl <1054056+markcowl@users.noreply.github.com> --- .../rules/arm-resource-required-operations.ts | 125 ++++++++++-------- 1 file changed, 73 insertions(+), 52 deletions(-) diff --git a/packages/typespec-azure-resource-manager/src/rules/arm-resource-required-operations.ts b/packages/typespec-azure-resource-manager/src/rules/arm-resource-required-operations.ts index 0ebde6b8d1..6790952673 100644 --- a/packages/typespec-azure-resource-manager/src/rules/arm-resource-required-operations.ts +++ b/packages/typespec-azure-resource-manager/src/rules/arm-resource-required-operations.ts @@ -4,6 +4,7 @@ import { defineCodeFix, getSourceLocation, Interface, + LinterRuleContext, Model, paramMessage, Program, @@ -18,6 +19,15 @@ type RequiredOperation = | "list-by-parent" | "list-by-subscription"; +type RequiredOperationsMessages = { + default: ReturnType; + missingListBySubscription: ReturnType; + missingListByParent: ReturnType; + missingGet: ReturnType; + missingCreateOrUpdate: ReturnType; + missingDelete: ReturnType; +}; + /** * Verify that an ARM resource declares the complete set of required * lifecycle and list operations for its kind. This rule is singleton-aware @@ -38,62 +48,73 @@ export const armResourceRequiredOperationsRule = createRule({ missingDelete: paramMessage`Resource '${"name"}' must have a delete operation.`, }, create(context) { - // Resolve resources once for the program and reuse for every model visit. - let resourcesByModel: Map | undefined; - const getResources = (program: Program): Map => { - if (resourcesByModel !== undefined) return resourcesByModel; - resourcesByModel = new Map(); - const provider = resolveArmResources(program); - for (const resource of provider.resources ?? []) { - const list = resourcesByModel.get(resource.type); - if (list) list.push(resource); - else resourcesByModel.set(resource.type, [resource]); - } - return resourcesByModel; - }; - return { - model: (model: Model) => { - if (isInternalTypeSpec(context.program, model)) return; - const resources = getResources(context.program).get(model); - if (!resources || resources.length === 0) return; - // A resource model can have multiple resolved entries (e.g., distinct - // operation paths). Treat the union of their operations as the - // operations declared for this model. - const armResource = resources[0]; - if (armResource.kind === "Other") return; - - const required = getRequiredOperationsForResource(armResource); - if (required.length === 0) return; - const present = getPresentOperations(resources); - const missing = required.filter((op) => !present.has(op)); - if (missing.length === 0) return; - - const resourceInterface = getResolvedResourceInterface(resources); - const target = resourceInterface ?? model; - - if (missing.length === 1) { - const messageId = singleMessageId(missing[0]); - context.reportDiagnostic({ - messageId, - target, - format: { name: armResource.resourceName }, - codefixes: buildCodefixes(missing, armResource.resourceName, resourceInterface), - }); - return; + // Iterate every resolved ARM resource once for the program. We use + // `resolveArmResources` (which handles both standard and legacy resource + // operations) and read kind / singleton / operations directly from the + // returned ResolvedResource entries. + // + // A single declared resource can produce multiple ResolvedResource + // entries (e.g., when ARM operation templates emit additional paths + // for createOrUpdate under a nested resource), so entries are grouped + // by model identity and a single diagnostic is reported per logical + // resource using the union of their operations. + root: (program: Program) => { + const provider = resolveArmResources(program); + const groups = new Map(); + for (const resource of provider.resources ?? []) { + if (resource.kind === "Other") continue; + if (isInternalTypeSpec(program, resource.type)) continue; + const list = groups.get(resource.type); + if (list) list.push(resource); + else groups.set(resource.type, [resource]); + } + for (const entries of groups.values()) { + checkResource(context, entries); } - context.reportDiagnostic({ - target, - format: { name: armResource.resourceName, operations: missing.join(", ") }, - codefixes: buildCodefixes(missing, armResource.resourceName, resourceInterface), - }); }, }; }, }); +function checkResource( + context: LinterRuleContext, + entries: ResolvedResource[], +): void { + // Use the entry with the shortest `resourceType.types` as the canonical + // representation of the resource (it has the natural depth declared by the + // user). Operations are aggregated across all entries. + const canonical = entries.reduce((best, cur) => + cur.resourceType.types.length < best.resourceType.types.length ? cur : best, + ); + + const required = getRequiredOperationsForResource(canonical); + if (required.length === 0) return; + const present = getPresentOperations(entries); + const missing = required.filter((op) => !present.has(op)); + if (missing.length === 0) return; + + const resourceInterface = getResolvedResourceInterface(entries); + const target = resourceInterface ?? canonical.type; + const name = canonical.resourceName; + + if (missing.length === 1) { + context.reportDiagnostic({ + messageId: singleMessageId(missing[0]), + target, + format: { name }, + codefixes: buildCodefixes(missing, name, resourceInterface), + }); + return; + } + context.reportDiagnostic({ + target, + format: { name, operations: missing.join(", ") }, + codefixes: buildCodefixes(missing, name, resourceInterface), + }); +} + function getRequiredOperationsForResource(resource: ResolvedResource): RequiredOperation[] { - if (resource.kind === "Other") return []; const isSingleton = resource.singleton !== undefined; if (isSingleton) { return ["read", "createOrUpdate"]; @@ -108,9 +129,9 @@ function getRequiredOperationsForResource(resource: ResolvedResource): RequiredO return required; } -function getPresentOperations(resources: ResolvedResource[]): Set { +function getPresentOperations(entries: ResolvedResource[]): Set { const present = new Set(); - for (const resource of resources) { + for (const resource of entries) { const lifecycle = resource.operations.lifecycle; if (lifecycle.read?.length) present.add("read"); if (lifecycle.createOrUpdate?.length) present.add("createOrUpdate"); @@ -132,8 +153,8 @@ function getPresentOperations(resources: ResolvedResource[]): Set Date: Thu, 7 May 2026 01:17:46 +0000 Subject: [PATCH 05/14] Merge remote-tracking branch 'origin/main' into copilot/add-arm-resource-required-operations-rule Agent-Logs-Url: https://github.com/Azure/typespec-azure/sessions/f98c08fb-bae8-4462-a33e-47862ea7bfed Co-authored-by: markcowl <1054056+markcowl@users.noreply.github.com> --- ...-version-regression-2026-05-06-02-30-00.md | 7 ++ core | 2 +- .../src/public-utils.ts | 40 +++++++-- .../test/public-utils/is-api-version.test.ts | 32 +++++++ .../libraries/azure-core/reference/linter.md | 86 +++++++++---------- .../reference/linter.md | 74 ++++++++-------- .../reference/linter.md | 10 +-- 7 files changed, 160 insertions(+), 91 deletions(-) create mode 100644 .chronus/changes/fix-tcgc-server-param-api-version-regression-2026-05-06-02-30-00.md diff --git a/.chronus/changes/fix-tcgc-server-param-api-version-regression-2026-05-06-02-30-00.md b/.chronus/changes/fix-tcgc-server-param-api-version-regression-2026-05-06-02-30-00.md new file mode 100644 index 0000000000..c70a2f0043 --- /dev/null +++ b/.chronus/changes/fix-tcgc-server-param-api-version-regression-2026-05-06-02-30-00.md @@ -0,0 +1,7 @@ +--- +changeKind: fix +packages: + - "@azure-tools/typespec-client-generator-core" +--- + +Fix regression introduced in PR #4341: a server URL template parameter (declared in `@server`) named `apiVersion`/`api-version` with a plain `string` type in a versioned service is now correctly recognized as `isApiVersionParam`. diff --git a/core b/core index c1f77ce90b..46fbbc0006 160000 --- a/core +++ b/core @@ -1 +1 @@ -Subproject commit c1f77ce90b3559065276f6b1dbd6a3fc0c990236 +Subproject commit 46fbbc0006e83e5bdcab8523bd8c6e48968830c7 diff --git a/packages/typespec-client-generator-core/src/public-utils.ts b/packages/typespec-client-generator-core/src/public-utils.ts index 7bbc110fa9..4466d125a1 100644 --- a/packages/typespec-client-generator-core/src/public-utils.ts +++ b/packages/typespec-client-generator-core/src/public-utils.ts @@ -20,7 +20,14 @@ import { isService, resolveEncodedName, } from "@typespec/compiler"; -import { HttpOperation, Visibility, getHttpOperation, isMetadata, isVisible } from "@typespec/http"; +import { + HttpOperation, + Visibility, + getHttpOperation, + getServers, + isMetadata, + isVisible, +} from "@typespec/http"; import { getOperationId } from "@typespec/openapi"; import { Version, getVersions } from "@typespec/versioning"; import { pascalCase } from "change-case"; @@ -96,10 +103,10 @@ export function isApiVersion(context: TCGCContext, type: ModelProperty): boolean return true; } // otherwise, only consider name-based matching for http metadata parameters - // (header/query/path/cookie/statusCode). A regular body model property whose - // name happens to be `apiVersion`/`api-version` should not be treated as an - // api version parameter. - if (!isMetadata(context.program, type)) { + // (header/query/path/cookie/statusCode) or server URL template parameters. + // A regular body model property whose name happens to be `apiVersion`/`api-version` + // should not be treated as an api version parameter. + if (!isMetadata(context.program, type) && !isServerUrlTemplateParam(context, type)) { return false; } return ( @@ -108,6 +115,29 @@ export function isApiVersion(context: TCGCContext, type: ModelProperty): boolean ); } +/** + * Return whether a model property is a server URL template parameter (i.e., a + * path-segment variable declared in the `@server` decorator's parameter model). + * These parameters are not annotated with HTTP metadata decorators, but they + * represent URL template variables and should still be eligible for API-version + * name matching. + */ +function isServerUrlTemplateParam(context: TCGCContext, type: ModelProperty): boolean { + for (const ns of listAllServiceNamespaces(context)) { + const servers = getServers(context.program, ns); + if (servers) { + for (const server of servers) { + for (const param of server.parameters.values()) { + if (param === type) { + return true; + } + } + } + } + } + return false; +} + /** * If the given type is an anonymous model, returns a named model with same shape. * The finding logic will ignore all the properties of header/query/path/status-code metadata, diff --git a/packages/typespec-client-generator-core/test/public-utils/is-api-version.test.ts b/packages/typespec-client-generator-core/test/public-utils/is-api-version.test.ts index 941adcb95a..2a8c4882b7 100644 --- a/packages/typespec-client-generator-core/test/public-utils/is-api-version.test.ts +++ b/packages/typespec-client-generator-core/test/public-utils/is-api-version.test.ts @@ -99,3 +99,35 @@ it("api version in host param with versioning", async () => { ok(hostParam && isApiVersion(context, hostParam)); }); + +it("api version in host param named api-version with plain string type in versioned service", async () => { + // Regression test: server URL template param named `apiVersion` with plain string type + // (not the version enum) in a versioned service must still be recognized as an API + // version parameter. This was broken by PR #4341 which added an isMetadata guard that + // excluded server URL template params because they carry no HTTP metadata annotations. + // See https://github.com/Azure/typespec-azure/blob/main/packages/azure-http-specs/specs/resiliency/srv-driven/old.tsp + const { program } = await SimpleTester.compile(` + @service + @versioned(Versions) + @server( + "{endpoint}/resiliency/service-driven/client:v1/service:{serviceDeploymentVersion}/api-version:{apiVersion}", + "Testserver endpoint", + { + endpoint: url, + serviceDeploymentVersion: string, + apiVersion: string, + } + ) + namespace Resiliency.ServiceDriven; + + enum Versions { + v1, + } + `); + const context = await createSdkContextForTester(program); + const serviceNamespace = listAllServiceNamespaces(context)[0]; + const server = getServers(context.program, serviceNamespace)?.[0]; + const apiVersionParam = server?.parameters.get("apiVersion"); + + ok(apiVersionParam && isApiVersion(context, apiVersionParam)); +}); diff --git a/website/src/content/docs/docs/libraries/azure-core/reference/linter.md b/website/src/content/docs/docs/libraries/azure-core/reference/linter.md index 53670bd4d7..ad75ef5146 100644 --- a/website/src/content/docs/docs/libraries/azure-core/reference/linter.md +++ b/website/src/content/docs/docs/libraries/azure-core/reference/linter.md @@ -21,46 +21,46 @@ Available ruleSets: ## Rules -| Name | Description | -| ---------------------------------------------------------------------------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------- | -| [`@azure-tools/typespec-azure-core/operation-missing-api-version`](/libraries/azure-core/rules/operation-missing-api-version.md) | Operations need an api version parameter. | -| [`@azure-tools/typespec-azure-core/auth-required`](/libraries/azure-core/rules/auth-required.md) | Enforce service authentication. | -| [`@azure-tools/typespec-azure-core/request-body-problem`](/libraries/azure-core/rules/request-body-problem.md) | Request body should not be of raw array type. | -| [`@azure-tools/typespec-azure-core/byos`](/libraries/azure-core/rules/byos.md) | Use the BYOS pattern recommended for Azure Services. | -| [`@azure-tools/typespec-azure-core/casing-style`](/libraries/azure-core/rules/casing-style.md) | Ensure proper casing style. | -| [`@azure-tools/typespec-azure-core/composition-over-inheritance`](/libraries/azure-core/rules/composition-over-inheritance.md) | Check that if a model is used in an operation and has derived models that it has a discriminator or recommend to use composition via spread or `is`. | -| [`@azure-tools/typespec-azure-core/known-encoding`](/libraries/azure-core/rules/known-encoding.md) | Check for supported encodings. | -| [`@azure-tools/typespec-azure-core/long-running-polling-operation-required`](/libraries/azure-core/rules/long-running-polling-operation-required.md) | Long-running operations should have a linked polling operation. | -| [`@azure-tools/typespec-azure-core/no-case-mismatch`](/libraries/azure-core/rules/no-case-mismatch.md) | Validate that no two types have the same name with different casing. | -| [`@azure-tools/typespec-azure-core/no-closed-literal-union`](/libraries/azure-core/rules/no-closed-literal-union.md) | Unions of literals should include the base scalar type to mark them as open enum. | -| [`@azure-tools/typespec-azure-core/no-enum`](/libraries/azure-core/rules/no-enum.md) | Azure services should not use enums. | -| [`@azure-tools/typespec-azure-core/no-error-status-codes`](/libraries/azure-core/rules/no-error-status-codes.md) | Recommend using the error response defined by Azure REST API guidelines. | -| [`@azure-tools/typespec-azure-core/no-explicit-routes-resource-ops`](/libraries/azure-core/rules/no-explicit-routes-resource-ops.md) | The @route decorator should not be used on standard resource operation signatures. | -| [`@azure-tools/typespec-azure-core/non-breaking-versioning`](/libraries/azure-core/rules/non-breaking-versioning.md) | Check that only backward compatible versioning change are done to a service. | -| [`@azure-tools/typespec-azure-core/no-generic-numeric`](/libraries/azure-core/rules/no-generic-numeric.md) | Don't use generic types. Use more specific types instead. | -| [`@azure-tools/typespec-azure-core/no-nullable`](/libraries/azure-core/rules/no-nullable.md) | Use `?` for optional properties. | -| [`@azure-tools/typespec-azure-core/no-offsetdatetime`](/libraries/azure-core/rules/no-offsetdatetime.md) | Prefer using `utcDateTime` when representing a datetime unless an offset is necessary. | -| [`@azure-tools/typespec-azure-core/no-response-body`](/libraries/azure-core/rules/no-response-body.md) | Ensure that the body is set correctly for the response type. | -| [`@azure-tools/typespec-azure-core/no-rpc-path-params`](/libraries/azure-core/rules/no-rpc-path-params.md) | Operations defined using RpcOperation should not have path parameters. | -| [`@azure-tools/typespec-azure-core/no-openapi`](/libraries/azure-core/rules/no-openapi.md) | Azure specs should not be using decorators from @typespec/openapi or @azure-tools/typespec-autorest | -| [`@azure-tools/typespec-azure-core/no-unnamed-union`](/libraries/azure-core/rules/no-unnamed-union.md) | Azure services should not define a union expression but create a declaration. | -| [`@azure-tools/typespec-azure-core/no-header-explode`](/libraries/azure-core/rules/no-header-explode.md) | It is recommended to serialize header parameter without explode: true | -| [`@azure-tools/typespec-azure-core/no-format`](/libraries/azure-core/rules/prevent-format.md) | Azure services should not use the `@format` decorator. | -| [`@azure-tools/typespec-azure-core/no-multiple-discriminator`](/libraries/azure-core/rules/no-multiple-discriminator.md) | Classes should have at most one discriminator. | -| [`@azure-tools/typespec-azure-core/no-rest-library-interfaces`](/libraries/azure-core/rules/no-rest-library-interfaces.md) | Resource interfaces from the TypeSpec.Rest.Resource library are incompatible with Azure.Core. | -| [`@azure-tools/typespec-azure-core/no-unknown`](/libraries/azure-core/rules/no-unknown.md) | Azure services must not have properties of type `unknown`. | -| [`@azure-tools/typespec-azure-core/bad-record-type`](/libraries/azure-core/rules/bad-record-type.md) | Identify bad record definitions. | -| [`@azure-tools/typespec-azure-core/documentation-required`](/libraries/azure-core/rules/documentation-required.md) | Require documentation over enums, models, and operations. | -| [`@azure-tools/typespec-azure-core/key-visibility-required`](/libraries/azure-core/rules/key-visibility-required.md) | Key properties need to have a Lifecycle visibility setting. | -| [`@azure-tools/typespec-azure-core/response-schema-problem`](/libraries/azure-core/rules/response-schema-problem.md) | Warn about operations having multiple non-error response schemas. | -| [`@azure-tools/typespec-azure-core/rpc-operation-request-body`](/libraries/azure-core/rules/rpc-operation-request-body.md) | Warning for RPC body problems. | -| [`@azure-tools/typespec-azure-core/spread-discriminated-model`](/libraries/azure-core/rules/spread-discriminated-model.md) | Check a model with a discriminator has not been used in composition. | -| [`@azure-tools/typespec-azure-core/use-standard-names`](/libraries/azure-core/rules/use-standard-names.md) | Use recommended names for operations. | -| [`@azure-tools/typespec-azure-core/use-standard-operations`](/libraries/azure-core/rules/use-standard-operations.md) | Operations should be defined using a signature from the Azure.Core namespace. | -| [`@azure-tools/typespec-azure-core/no-string-discriminator`](/libraries/azure-core/rules/no-string-discriminator.md) | Azure services discriminated models should define the discriminated property as an extensible union. | -| [`@azure-tools/typespec-azure-core/require-versioned`](/libraries/azure-core/rules/require-versioned.md) | Azure services should use the versioning library. | -| [`@azure-tools/typespec-azure-core/friendly-name`](/libraries/azure-core/rules/friendly-name.md) | Ensures that @friendlyName is used as intended. | -| [`@azure-tools/typespec-azure-core/no-private-usage`](/libraries/azure-core/rules/no-private-usage.md) | Verify that elements inside Private namespace are not referenced. | -| [`@azure-tools/typespec-azure-core/no-legacy-usage`](/libraries/azure-core/rules/no-legacy-usage.md) | Linter warning against using elements from the Legacy namespace | -| [`@azure-tools/typespec-azure-core/no-query-explode`](/libraries/azure-core/rules/no-query-explode.md) | It is recommended to serialize query parameter without explode: true | -| [`@azure-tools/typespec-azure-core/no-route-parameter-name-mismatch`](/libraries/azure-core/rules/no-route-parameter-name-mismatch.md) | Ensure that operations with the same path use consistent path parameter names. | +| Name | Description | +| --------------------------------------------------------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------- | +| [`@azure-tools/typespec-azure-core/operation-missing-api-version`](../rules/operation-missing-api-version.md) | Operations need an api version parameter. | +| [`@azure-tools/typespec-azure-core/auth-required`](../rules/auth-required.md) | Enforce service authentication. | +| [`@azure-tools/typespec-azure-core/request-body-problem`](../rules/request-body-problem.md) | Request body should not be of raw array type. | +| [`@azure-tools/typespec-azure-core/byos`](../rules/byos.md) | Use the BYOS pattern recommended for Azure Services. | +| [`@azure-tools/typespec-azure-core/casing-style`](../rules/casing-style.md) | Ensure proper casing style. | +| [`@azure-tools/typespec-azure-core/composition-over-inheritance`](../rules/composition-over-inheritance.md) | Check that if a model is used in an operation and has derived models that it has a discriminator or recommend to use composition via spread or `is`. | +| [`@azure-tools/typespec-azure-core/known-encoding`](../rules/known-encoding.md) | Check for supported encodings. | +| [`@azure-tools/typespec-azure-core/long-running-polling-operation-required`](../rules/long-running-polling-operation-required.md) | Long-running operations should have a linked polling operation. | +| [`@azure-tools/typespec-azure-core/no-case-mismatch`](../rules/no-case-mismatch.md) | Validate that no two types have the same name with different casing. | +| [`@azure-tools/typespec-azure-core/no-closed-literal-union`](../rules/no-closed-literal-union.md) | Unions of literals should include the base scalar type to mark them as open enum. | +| [`@azure-tools/typespec-azure-core/no-enum`](../rules/no-enum.md) | Azure services should not use enums. | +| [`@azure-tools/typespec-azure-core/no-error-status-codes`](../rules/no-error-status-codes.md) | Recommend using the error response defined by Azure REST API guidelines. | +| [`@azure-tools/typespec-azure-core/no-explicit-routes-resource-ops`](../rules/no-explicit-routes-resource-ops.md) | The @route decorator should not be used on standard resource operation signatures. | +| [`@azure-tools/typespec-azure-core/non-breaking-versioning`](../rules/non-breaking-versioning.md) | Check that only backward compatible versioning change are done to a service. | +| [`@azure-tools/typespec-azure-core/no-generic-numeric`](../rules/no-generic-numeric.md) | Don't use generic types. Use more specific types instead. | +| [`@azure-tools/typespec-azure-core/no-nullable`](../rules/no-nullable.md) | Use `?` for optional properties. | +| [`@azure-tools/typespec-azure-core/no-offsetdatetime`](../rules/no-offsetdatetime.md) | Prefer using `utcDateTime` when representing a datetime unless an offset is necessary. | +| [`@azure-tools/typespec-azure-core/no-response-body`](../rules/no-response-body.md) | Ensure that the body is set correctly for the response type. | +| [`@azure-tools/typespec-azure-core/no-rpc-path-params`](../rules/no-rpc-path-params.md) | Operations defined using RpcOperation should not have path parameters. | +| [`@azure-tools/typespec-azure-core/no-openapi`](../rules/no-openapi.md) | Azure specs should not be using decorators from @typespec/openapi or @azure-tools/typespec-autorest | +| [`@azure-tools/typespec-azure-core/no-unnamed-union`](../rules/no-unnamed-union.md) | Azure services should not define a union expression but create a declaration. | +| [`@azure-tools/typespec-azure-core/no-header-explode`](../rules/no-header-explode.md) | It is recommended to serialize header parameter without explode: true | +| [`@azure-tools/typespec-azure-core/no-format`](../rules/no-format.md) | Azure services should not use the `@format` decorator. | +| [`@azure-tools/typespec-azure-core/no-multiple-discriminator`](../rules/no-multiple-discriminator.md) | Classes should have at most one discriminator. | +| [`@azure-tools/typespec-azure-core/no-rest-library-interfaces`](../rules/no-rest-library-interfaces.md) | Resource interfaces from the TypeSpec.Rest.Resource library are incompatible with Azure.Core. | +| [`@azure-tools/typespec-azure-core/no-unknown`](../rules/no-unknown.md) | Azure services must not have properties of type `unknown`. | +| [`@azure-tools/typespec-azure-core/bad-record-type`](../rules/bad-record-type.md) | Identify bad record definitions. | +| [`@azure-tools/typespec-azure-core/documentation-required`](../rules/documentation-required.md) | Require documentation over enums, models, and operations. | +| [`@azure-tools/typespec-azure-core/key-visibility-required`](../rules/key-visibility-required.md) | Key properties need to have a Lifecycle visibility setting. | +| [`@azure-tools/typespec-azure-core/response-schema-problem`](../rules/response-schema-problem.md) | Warn about operations having multiple non-error response schemas. | +| [`@azure-tools/typespec-azure-core/rpc-operation-request-body`](../rules/rpc-operation-request-body.md) | Warning for RPC body problems. | +| [`@azure-tools/typespec-azure-core/spread-discriminated-model`](../rules/spread-discriminated-model.md) | Check a model with a discriminator has not been used in composition. | +| [`@azure-tools/typespec-azure-core/use-standard-names`](../rules/use-standard-names.md) | Use recommended names for operations. | +| [`@azure-tools/typespec-azure-core/use-standard-operations`](../rules/use-standard-operations.md) | Operations should be defined using a signature from the Azure.Core namespace. | +| [`@azure-tools/typespec-azure-core/no-string-discriminator`](../rules/no-string-discriminator.md) | Azure services discriminated models should define the discriminated property as an extensible union. | +| [`@azure-tools/typespec-azure-core/require-versioned`](../rules/require-versioned.md) | Azure services should use the versioning library. | +| [`@azure-tools/typespec-azure-core/friendly-name`](../rules/friendly-name.md) | Ensures that @friendlyName is used as intended. | +| [`@azure-tools/typespec-azure-core/no-private-usage`](../rules/no-private-usage.md) | Verify that elements inside Private namespace are not referenced. | +| [`@azure-tools/typespec-azure-core/no-legacy-usage`](../rules/no-legacy-usage.md) | Linter warning against using elements from the Legacy namespace | +| [`@azure-tools/typespec-azure-core/no-query-explode`](../rules/no-query-explode.md) | It is recommended to serialize query parameter without explode: true | +| [`@azure-tools/typespec-azure-core/no-route-parameter-name-mismatch`](../rules/no-route-parameter-name-mismatch.md) | Ensure that operations with the same path use consistent path parameter names. | 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 9a2ec890ab..5b46d320f9 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 @@ -20,40 +20,40 @@ Available ruleSets: ## Rules -| 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-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. | -| [`@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`](/libraries/azure-resource-manager/rules/arm-resource-action-no-segment.md) | `@armResourceAction` should not be used with `@segment`. | -| [`@azure-tools/typespec-azure-resource-manager/arm-resource-duplicate-property`](/libraries/azure-resource-manager/rules/arm-resource-duplicate-property.md) | Warn about duplicate properties in resources. | -| [`@azure-tools/typespec-azure-resource-manager/arm-resource-invalid-envelope-property`](/libraries/azure-resource-manager/rules/arm-resource-invalid-envelope-property.md) | Check for invalid resource envelope properties. | -| [`@azure-tools/typespec-azure-resource-manager/arm-resource-invalid-version-format`](/libraries/azure-resource-manager/rules/arm-resource-invalid-version-format.md) | Check for valid versions. | -| [`@azure-tools/typespec-azure-resource-manager/arm-resource-key-invalid-chars`](/libraries/azure-resource-manager/rules/arm-resource-key-invalid-chars.md) | Arm resource key must contain only alphanumeric characters. | -| [`@azure-tools/typespec-azure-resource-manager/arm-resource-name-pattern`](/libraries/azure-resource-manager/rules/resource-name-pattern.md) | The resource name parameter should be defined with a 'pattern' restriction. | -| [`@azure-tools/typespec-azure-resource-manager/arm-resource-operation-response`](/libraries/azure-resource-manager/rules/arm-resource-operation-response.md) | [RPC 008]: PUT, GET, PATCH & LIST must return the same resource schema. | -| [`@azure-tools/typespec-azure-resource-manager/arm-resource-path-segment-invalid-chars`](/libraries/azure-resource-manager/rules/arm-resource-path-segment-invalid-chars.md) | Arm resource name must contain only alphanumeric characters. | -| [`@azure-tools/typespec-azure-resource-manager/arm-resource-provisioning-state`](/libraries/azure-resource-manager/rules/arm-resource-provisioning-state.md) | Check for properly configured provisioningState property. | -| [`@azure-tools/typespec-azure-resource-manager/arm-custom-resource-no-key`](/libraries/azure-resource-manager/rules/arm-custom-resource-no-key.md) | Validate that custom resource contains a key property. | -| [`@azure-tools/typespec-azure-resource-manager/arm-custom-resource-usage-discourage`](/libraries/azure-resource-manager/rules/arm-custom-resource-usage-discourage.md) | Verify the usage of @customAzureResource decorator. | -| [`@azure-tools/typespec-azure-resource-manager/beyond-nesting-levels`](/libraries/azure-resource-manager/rules/beyond-nesting-levels.md) | Tracked Resources must use 3 or fewer levels of nesting. | -| [`@azure-tools/typespec-azure-resource-manager/arm-resource-operation`](/libraries/azure-resource-manager/rules/arm-resource-operation.md) | Validate ARM Resource operations. | -| [`@azure-tools/typespec-azure-resource-manager/no-resource-delete-operation`](/libraries/azure-resource-manager/rules/no-resource-delete-operation.md) | Check for resources that must have a delete operation. | -| [`@azure-tools/typespec-azure-resource-manager/arm-resource-required-operations`](/libraries/azure-resource-manager/rules/arm-resource-required-operations.md) | ARM resources must declare the complete set of required lifecycle and list operations. Singleton-aware; supersedes `no-resource-delete-operation`. | -| [`@azure-tools/typespec-azure-resource-manager/empty-updateable-properties`](/libraries/azure-resource-manager/rules/empty-updateable-properties.md) | Should have updateable properties. | -| [`@azure-tools/typespec-azure-resource-manager/arm-resource-interface-requires-decorator`](/libraries/azure-resource-manager/rules/arm-resource-interface-requires-decorator.md) | Each resource interface must have an @armResourceOperations decorator. | -| [`@azure-tools/typespec-azure-resource-manager/arm-resource-invalid-action-verb`](/libraries/azure-resource-manager/rules/arm-resource-invalid-action-verb.md) | Actions must be HTTP Post or Get operations. | -| [`@azure-tools/typespec-azure-resource-manager/improper-subscription-list-operation`](/libraries/azure-resource-manager/rules/improper-subscription-list-operation.md) | Tenant and Extension resources should not define a list by subscription operation. | -| [`@azure-tools/typespec-azure-resource-manager/lro-location-header`](/libraries/azure-resource-manager/rules/lro-location-header.md) | A 202 response should include a Location response header. | -| [`@azure-tools/typespec-azure-resource-manager/missing-x-ms-identifiers`](/libraries/azure-resource-manager/rules/missing-x-ms-identifiers.md) | Array properties should describe their identifying properties with x-ms-identifiers. Decorate the property with @OpenAPI.extension("x-ms-identifiers", #[id-prop]) where "id-prop" is a list of the names of identifying properties in the item type. | -| [`@azure-tools/typespec-azure-resource-manager/no-response-body`](/libraries/azure-resource-manager/rules/no-response-body.md) | Check that the body is empty for 202 and 204 responses, and not empty for other success (2xx) responses. | -| [`@azure-tools/typespec-azure-resource-manager/missing-operations-endpoint`](/libraries/azure-resource-manager/rules/missing-operations-endpoint.md) | Check for missing Operations interface. | -| [`@azure-tools/typespec-azure-resource-manager/patch-envelope`](/libraries/azure-resource-manager/rules/patch-envelope.md) | Patch envelope properties should match the resource properties. | -| [`@azure-tools/typespec-azure-resource-manager/arm-resource-patch`](/libraries/azure-resource-manager/rules/arm-resource-patch.md) | Validate ARM PATCH operations. | -| [`@azure-tools/typespec-azure-resource-manager/resource-name`](/libraries/azure-resource-manager/rules/resource-name.md) | Check the resource name. | -| [`@azure-tools/typespec-azure-resource-manager/retry-after`](/libraries/azure-resource-manager/rules/retry-after.md) | Check if retry-after header appears in response body. | -| [`@azure-tools/typespec-azure-resource-manager/unsupported-type`](/libraries/azure-resource-manager/rules/unsupported-type.md) | Check for unsupported ARM types. | -| [`@azure-tools/typespec-azure-resource-manager/secret-prop`](/libraries/azure-resource-manager/rules/secret-prop.md) | RPC-v1-13: Check that property with names indicating sensitive information(e.g. contains auth, password, token, secret, etc.) are marked with @secret decorator. | -| [`@azure-tools/typespec-azure-resource-manager/no-empty-model`](/libraries/azure-resource-manager/rules/no-empty-model.md) | ARM Properties with type:object that don't reference a model definition are not allowed. ARM doesn't allow generic type definitions as this leads to bad customer experience. | +| Name | Description | +| ------------------------------------------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| [`@azure-tools/typespec-azure-resource-manager/arm-no-record`](../rules/arm-no-record.md) | Don't use Record types for ARM resources. | +| [`@azure-tools/typespec-azure-resource-manager/arm-common-types-version`](../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`](../rules/arm-delete-operation-response-codes.md) | Ensure delete operations have the appropriate status codes. | +| [`@azure-tools/typespec-azure-resource-manager/arm-put-operation-response-codes`](../rules/arm-put-operation-response-codes.md) | Ensure put operations have the appropriate status codes. | +| [`@azure-tools/typespec-azure-resource-manager/arm-post-operation-response-codes`](../rules/arm-post-operation-response-codes.md) | Ensure post operations have the appropriate status codes. | +| [`@azure-tools/typespec-azure-resource-manager/arm-resource-action-no-segment`](../rules/arm-resource-action-no-segment.md) | `@armResourceAction` should not be used with `@segment`. | +| [`@azure-tools/typespec-azure-resource-manager/arm-resource-duplicate-property`](../rules/arm-resource-duplicate-property.md) | Warn about duplicate properties in resources. | +| [`@azure-tools/typespec-azure-resource-manager/arm-resource-invalid-envelope-property`](../rules/arm-resource-invalid-envelope-property.md) | Check for invalid resource envelope properties. | +| [`@azure-tools/typespec-azure-resource-manager/arm-resource-invalid-version-format`](../rules/arm-resource-invalid-version-format.md) | Check for valid versions. | +| [`@azure-tools/typespec-azure-resource-manager/arm-resource-key-invalid-chars`](../rules/arm-resource-key-invalid-chars.md) | Arm resource key must contain only alphanumeric characters. | +| [`@azure-tools/typespec-azure-resource-manager/arm-resource-name-pattern`](../rules/arm-resource-name-pattern.md) | The resource name parameter should be defined with a 'pattern' restriction. | +| [`@azure-tools/typespec-azure-resource-manager/arm-resource-operation-response`](../rules/arm-resource-operation-response.md) | [RPC 008]: PUT, GET, PATCH & LIST must return the same resource schema. | +| [`@azure-tools/typespec-azure-resource-manager/arm-resource-path-segment-invalid-chars`](../rules/arm-resource-path-segment-invalid-chars.md) | Arm resource name must contain only alphanumeric characters. | +| [`@azure-tools/typespec-azure-resource-manager/arm-resource-provisioning-state`](../rules/arm-resource-provisioning-state.md) | Check for properly configured provisioningState property. | +| [`@azure-tools/typespec-azure-resource-manager/arm-custom-resource-no-key`](../rules/arm-custom-resource-no-key.md) | Validate that custom resource contains a key property. | +| [`@azure-tools/typespec-azure-resource-manager/arm-custom-resource-usage-discourage`](../rules/arm-custom-resource-usage-discourage.md) | Verify the usage of @customAzureResource decorator. | +| [`@azure-tools/typespec-azure-resource-manager/beyond-nesting-levels`](../rules/beyond-nesting-levels.md) | Tracked Resources must use 3 or fewer levels of nesting. | +| [`@azure-tools/typespec-azure-resource-manager/arm-resource-operation`](../rules/arm-resource-operation.md) | Validate ARM Resource operations. | +| [`@azure-tools/typespec-azure-resource-manager/no-resource-delete-operation`](../rules/no-resource-delete-operation.md) | Check for resources that must have a delete operation. | +| [`@azure-tools/typespec-azure-resource-manager/arm-resource-required-operations`](../rules/arm-resource-required-operations.md) | ARM resources must declare the complete set of required lifecycle and list operations. Singleton-aware; supersedes `no-resource-delete-operation`. | +| [`@azure-tools/typespec-azure-resource-manager/empty-updateable-properties`](../rules/empty-updateable-properties.md) | Should have updateable properties. | +| [`@azure-tools/typespec-azure-resource-manager/arm-resource-interface-requires-decorator`](../rules/arm-resource-interface-requires-decorator.md) | Each resource interface must have an @armResourceOperations decorator. | +| [`@azure-tools/typespec-azure-resource-manager/arm-resource-invalid-action-verb`](../rules/arm-resource-invalid-action-verb.md) | Actions must be HTTP Post or Get operations. | +| [`@azure-tools/typespec-azure-resource-manager/improper-subscription-list-operation`](../rules/improper-subscription-list-operation.md) | Tenant and Extension resources should not define a list by subscription operation. | +| [`@azure-tools/typespec-azure-resource-manager/lro-location-header`](../rules/lro-location-header.md) | A 202 response should include a Location response header. | +| [`@azure-tools/typespec-azure-resource-manager/missing-x-ms-identifiers`](../rules/missing-x-ms-identifiers.md) | Array properties should describe their identifying properties with x-ms-identifiers. Decorate the property with @OpenAPI.extension("x-ms-identifiers", #[id-prop]) where "id-prop" is a list of the names of identifying properties in the item type. | +| [`@azure-tools/typespec-azure-resource-manager/no-response-body`](../rules/no-response-body.md) | Check that the body is empty for 202 and 204 responses, and not empty for other success (2xx) responses. | +| [`@azure-tools/typespec-azure-resource-manager/missing-operations-endpoint`](../rules/missing-operations-endpoint.md) | Check for missing Operations interface. | +| [`@azure-tools/typespec-azure-resource-manager/patch-envelope`](../rules/patch-envelope.md) | Patch envelope properties should match the resource properties. | +| [`@azure-tools/typespec-azure-resource-manager/arm-resource-patch`](../rules/arm-resource-patch.md) | Validate ARM PATCH operations. | +| [`@azure-tools/typespec-azure-resource-manager/resource-name`](../rules/resource-name.md) | Check the resource name. | +| [`@azure-tools/typespec-azure-resource-manager/retry-after`](../rules/retry-after.md) | Check if retry-after header appears in response body. | +| [`@azure-tools/typespec-azure-resource-manager/unsupported-type`](../rules/unsupported-type.md) | Check for unsupported ARM types. | +| [`@azure-tools/typespec-azure-resource-manager/secret-prop`](../rules/secret-prop.md) | RPC-v1-13: Check that property with names indicating sensitive information(e.g. contains auth, password, token, secret, etc.) are marked with @secret decorator. | +| [`@azure-tools/typespec-azure-resource-manager/no-empty-model`](../rules/no-empty-model.md) | ARM Properties with type:object that don't reference a model definition are not allowed. ARM doesn't allow generic type definitions as this leads to bad customer experience. | diff --git a/website/src/content/docs/docs/libraries/typespec-client-generator-core/reference/linter.md b/website/src/content/docs/docs/libraries/typespec-client-generator-core/reference/linter.md index 0d4e67d51c..354b28d0ac 100644 --- a/website/src/content/docs/docs/libraries/typespec-client-generator-core/reference/linter.md +++ b/website/src/content/docs/docs/libraries/typespec-client-generator-core/reference/linter.md @@ -21,8 +21,8 @@ Available ruleSets: ## Rules -| Name | Description | -| ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------- | -| [`@azure-tools/typespec-client-generator-core/require-client-suffix`](https://azure.github.io/typespec-azure/docs/libraries/typespec-client-generator-core/rules/require-client-suffix) | Client names should end with 'Client'. | -| [`@azure-tools/typespec-client-generator-core/property-name-conflict`](https://azure.github.io/typespec-azure/docs/libraries/typespec-client-generator-core/rules/property-name-conflict) | Avoid naming conflicts between a property and a model of the same name. | -| [`@azure-tools/typespec-client-generator-core/no-unnamed-types`](https://azure.github.io/typespec-azure/docs/libraries/typespec-client-generator-core/rules/no-unnamed-types) | Requires types to be named rather than defined anonymously or inline. | +| Name | Description | +| ---------------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------- | +| [`@azure-tools/typespec-client-generator-core/require-client-suffix`](../rules/require-client-suffix.md) | Client names should end with 'Client'. | +| [`@azure-tools/typespec-client-generator-core/property-name-conflict`](../rules/property-name-conflict.md) | Avoid naming conflicts between a property and a model of the same name. | +| [`@azure-tools/typespec-client-generator-core/no-unnamed-types`](../rules/no-unnamed-types.md) | Requires types to be named rather than defined anonymously or inline. | From 545ef741fbb3fb34ac2fce8c9e048c1301c67a6d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 7 May 2026 19:47:00 +0000 Subject: [PATCH 06/14] Merge main; refine list-categorization heuristic; update samples; add tests Agent-Logs-Url: https://github.com/Azure/typespec-azure/sessions/9ea77a12-d557-4aaa-8261-a34a7f9c8251 Co-authored-by: markcowl <1054056+markcowl@users.noreply.github.com> --- .../resource-manager/legacy/nsp/main.tsp | 1 + .../legacy/optional-body/main.tsp | 1 + .../legacy/polymorphic-resource/extension.tsp | 5 ++ .../legacy/polymorphic-resource/proxy.tsp | 1 + .../legacy/rename-operations/main.tsp | 1 + .../resource-types/nsp/main.tsp | 1 + .../2025-01-01-preview/openapi.json | 34 +++++++++ .../2025-01-01-preview/openapi.json | 34 +++++++++ .../rules/arm-resource-required-operations.ts | 38 ++++++++-- .../arm-resource-required-operations.test.ts | 73 +++++++++++++++++++ 10 files changed, 181 insertions(+), 8 deletions(-) diff --git a/packages/samples/specs/resource-manager/legacy/nsp/main.tsp b/packages/samples/specs/resource-manager/legacy/nsp/main.tsp index 6648c0a745..a5acd95e2f 100644 --- a/packages/samples/specs/resource-manager/legacy/nsp/main.tsp +++ b/packages/samples/specs/resource-manager/legacy/nsp/main.tsp @@ -70,6 +70,7 @@ interface Operations extends Azure.ResourceManager.Operations {} model NetworkSecurityPerimeterConfiguration is Azure.ResourceManager.NspConfiguration; alias NspConfigurationOperations = Azure.ResourceManager.NspConfigurations; +#suppress "@azure-tools/typespec-azure-resource-manager/arm-resource-required-operations" "Network security perimeter configuration resources are read-only by design." @armResourceOperations interface Employees { get is ArmResourceRead; diff --git a/packages/samples/specs/resource-manager/legacy/optional-body/main.tsp b/packages/samples/specs/resource-manager/legacy/optional-body/main.tsp index cecd27f2d7..0c3a13a2cc 100644 --- a/packages/samples/specs/resource-manager/legacy/optional-body/main.tsp +++ b/packages/samples/specs/resource-manager/legacy/optional-body/main.tsp @@ -43,4 +43,5 @@ interface Widgets { >; delete is ArmResourceDeleteWithoutOkAsync; list is ArmResourceListByParent; + listBySubscription is ArmListBySubscription; } diff --git a/packages/samples/specs/resource-manager/legacy/polymorphic-resource/extension.tsp b/packages/samples/specs/resource-manager/legacy/polymorphic-resource/extension.tsp index 747e28dc13..1f92362486 100644 --- a/packages/samples/specs/resource-manager/legacy/polymorphic-resource/extension.tsp +++ b/packages/samples/specs/resource-manager/legacy/polymorphic-resource/extension.tsp @@ -45,16 +45,21 @@ model Expense extends Cost { properties: ExpenseProperties; } +#suppress "@azure-tools/typespec-azure-resource-manager/arm-resource-required-operations" "This sample demonstrates read-only polymorphic extension resources; createOrUpdate and delete are intentionally omitted." @armResourceOperations interface Costs { get is Extension.Read; list is Extension.ListByTarget; } +#suppress "@azure-tools/typespec-azure-resource-manager/arm-resource-required-operations" "This sample demonstrates read-only polymorphic extension resources; createOrUpdate and delete are intentionally omitted." @armResourceOperations interface Scopes extends Costs {} +#suppress "@azure-tools/typespec-azure-resource-manager/arm-resource-required-operations" "This sample demonstrates read-only polymorphic extension resources; createOrUpdate and delete are intentionally omitted." @armResourceOperations interface Tenants extends Costs {} +#suppress "@azure-tools/typespec-azure-resource-manager/arm-resource-required-operations" "This sample demonstrates read-only polymorphic extension resources; createOrUpdate and delete are intentionally omitted." @armResourceOperations interface Subscriptions extends Costs {} +#suppress "@azure-tools/typespec-azure-resource-manager/arm-resource-required-operations" "This sample demonstrates read-only polymorphic extension resources; createOrUpdate and delete are intentionally omitted." @armResourceOperations interface ManagementGroups extends Costs {} diff --git a/packages/samples/specs/resource-manager/legacy/polymorphic-resource/proxy.tsp b/packages/samples/specs/resource-manager/legacy/polymorphic-resource/proxy.tsp index dc0a06bc18..407e352adc 100644 --- a/packages/samples/specs/resource-manager/legacy/polymorphic-resource/proxy.tsp +++ b/packages/samples/specs/resource-manager/legacy/polymorphic-resource/proxy.tsp @@ -49,6 +49,7 @@ model Office extends Room { properties: OfficeProperties; } +#suppress "@azure-tools/typespec-azure-resource-manager/arm-resource-required-operations" "This sample demonstrates a read-only polymorphic proxy resource; createOrUpdate and delete are intentionally omitted." @armResourceOperations interface Rooms { get is ArmResourceRead; diff --git a/packages/samples/specs/resource-manager/legacy/rename-operations/main.tsp b/packages/samples/specs/resource-manager/legacy/rename-operations/main.tsp index 98c0d272ba..4081084f76 100644 --- a/packages/samples/specs/resource-manager/legacy/rename-operations/main.tsp +++ b/packages/samples/specs/resource-manager/legacy/rename-operations/main.tsp @@ -56,4 +56,5 @@ interface Widgets { @Azure.ResourceManager.Legacy.renamePathParameter("widgetName", "resourceGroupName") delete is ArmResourceDeleteWithoutOkAsync; list is ArmResourceListByParent; + listBySubscription is ArmListBySubscription; } diff --git a/packages/samples/specs/resource-manager/resource-types/nsp/main.tsp b/packages/samples/specs/resource-manager/resource-types/nsp/main.tsp index 44de563eb7..3bb67ace8c 100644 --- a/packages/samples/specs/resource-manager/resource-types/nsp/main.tsp +++ b/packages/samples/specs/resource-manager/resource-types/nsp/main.tsp @@ -70,6 +70,7 @@ interface Operations extends Azure.ResourceManager.Operations {} model NetworkSecurityPerimeterConfiguration is Azure.ResourceManager.NspConfiguration; alias NspConfigurationOperations = Azure.ResourceManager.NspConfigurations; +#suppress "@azure-tools/typespec-azure-resource-manager/arm-resource-required-operations" "Network security perimeter configuration resources are read-only by design." @armResourceOperations interface Employees { get is ArmResourceRead; diff --git a/packages/samples/test/output/azure/resource-manager/legacy/optional-body/@azure-tools/typespec-autorest/2025-01-01-preview/openapi.json b/packages/samples/test/output/azure/resource-manager/legacy/optional-body/@azure-tools/typespec-autorest/2025-01-01-preview/openapi.json index 1f44d2e741..a0ed566e32 100644 --- a/packages/samples/test/output/azure/resource-manager/legacy/optional-body/@azure-tools/typespec-autorest/2025-01-01-preview/openapi.json +++ b/packages/samples/test/output/azure/resource-manager/legacy/optional-body/@azure-tools/typespec-autorest/2025-01-01-preview/openapi.json @@ -77,6 +77,40 @@ } } }, + "/subscriptions/{subscriptionId}/providers/Microsoft.OptionalBody/widgets": { + "get": { + "operationId": "Widgets_ListBySubscription", + "tags": [ + "Widgets" + ], + "description": "List Widget resources by subscription ID", + "parameters": [ + { + "$ref": "../../../../../../../../../specs/resource-manager/common-types/v5/types.json#/parameters/ApiVersionParameter" + }, + { + "$ref": "../../../../../../../../../specs/resource-manager/common-types/v5/types.json#/parameters/SubscriptionIdParameter" + } + ], + "responses": { + "200": { + "description": "Azure operation completed successfully.", + "schema": { + "$ref": "#/definitions/WidgetListResult" + } + }, + "default": { + "description": "An unexpected error response.", + "schema": { + "$ref": "../../../../../../../../../specs/resource-manager/common-types/v5/types.json#/definitions/ErrorResponse" + } + } + }, + "x-ms-pageable": { + "nextLinkName": "nextLink" + } + } + }, "/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.OptionalBody/widgets": { "get": { "operationId": "Widgets_List", diff --git a/packages/samples/test/output/azure/resource-manager/legacy/rename-operations/@azure-tools/typespec-autorest/2025-01-01-preview/openapi.json b/packages/samples/test/output/azure/resource-manager/legacy/rename-operations/@azure-tools/typespec-autorest/2025-01-01-preview/openapi.json index 2777179b92..fee650c9c5 100644 --- a/packages/samples/test/output/azure/resource-manager/legacy/rename-operations/@azure-tools/typespec-autorest/2025-01-01-preview/openapi.json +++ b/packages/samples/test/output/azure/resource-manager/legacy/rename-operations/@azure-tools/typespec-autorest/2025-01-01-preview/openapi.json @@ -77,6 +77,40 @@ } } }, + "/subscriptions/{subscriptionId}/providers/Microsoft.RenamedOperations/widgets": { + "get": { + "operationId": "Widgets_ListBySubscription", + "tags": [ + "Widgets" + ], + "description": "List Widget resources by subscription ID", + "parameters": [ + { + "$ref": "../../../../../../../../../specs/resource-manager/common-types/v5/types.json#/parameters/ApiVersionParameter" + }, + { + "$ref": "../../../../../../../../../specs/resource-manager/common-types/v5/types.json#/parameters/SubscriptionIdParameter" + } + ], + "responses": { + "200": { + "description": "Azure operation completed successfully.", + "schema": { + "$ref": "#/definitions/WidgetListResult" + } + }, + "default": { + "description": "An unexpected error response.", + "schema": { + "$ref": "../../../../../../../../../specs/resource-manager/common-types/v5/types.json#/definitions/ErrorResponse" + } + } + }, + "x-ms-pageable": { + "nextLinkName": "nextLink" + } + } + }, "/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.RenamedOperations/widgets": { "get": { "operationId": "Widgets_List", diff --git a/packages/typespec-azure-resource-manager/src/rules/arm-resource-required-operations.ts b/packages/typespec-azure-resource-manager/src/rules/arm-resource-required-operations.ts index 6790952673..a5216e258a 100644 --- a/packages/typespec-azure-resource-manager/src/rules/arm-resource-required-operations.ts +++ b/packages/typespec-azure-resource-manager/src/rules/arm-resource-required-operations.ts @@ -123,14 +123,28 @@ function getRequiredOperationsForResource(resource: ResolvedResource): RequiredO // list-by-parent operation. For tracked resources at resource-group scope, // list-by-resource-group satisfies the list-by-parent requirement. const required: RequiredOperation[] = ["read", "createOrUpdate", "delete", "list-by-parent"]; - if (resource.kind === "Tracked") { + // list-by-subscription is required only for top-level resource-group-scoped + // tracked resources (the standard Azure RP pattern). Nested resources + // (parented under another resource) and tracked resources at non-RG scope + // (tenant, subscription, location) do not require a list-by-subscription. + if (resource.kind === "Tracked" && isTopLevelResourceGroupScoped(resource)) { required.push("list-by-subscription"); } return required; } +function isTopLevelResourceGroupScoped(resource: ResolvedResource): boolean { + if (resource.parent !== undefined) return false; + const path = resource.resourceInstancePath ?? ""; + return /\/resourceGroups\/\{/.test(path); +} + function getPresentOperations(entries: ResolvedResource[]): Set { const present = new Set(); + // Determine the resource kind from the canonical-equivalent: if any entry is + // tracked, the resource is tracked. (All entries for the same logical model + // share the same kind in practice.) + const isTracked = entries.some((e) => e.kind === "Tracked"); for (const resource of entries) { const lifecycle = resource.operations.lifecycle; if (lifecycle.read?.length) present.add("read"); @@ -138,15 +152,23 @@ function getPresentOperations(entries: ResolvedResource[]): Set 1) { - // Nested list-by-parent. - present.add("list-by-parent"); - } else if (/\/resourceGroups\/\{/.test(path)) { - // List-by-resource-group satisfies list-by-parent for tracked resources. + if (!isTracked) { + // For non-tracked resources (Proxy / Extension), any list operation + // satisfies the list-by-parent requirement regardless of scope. present.add("list-by-parent"); - } else if (/\/subscriptions\/\{/.test(path)) { + continue; + } + // Tracked resources: list-by-subscription is a list rooted at + // subscription scope (has /subscriptions/{...} but not + // /resourceGroups/{...} and no nested /providers/). Everything else + // (resource-group, location, parent, etc.) counts as list-by-parent. + const providersCount = (path.match(/\/providers\//g) ?? []).length; + const hasSubscription = /\/subscriptions\/\{/.test(path); + const hasResourceGroup = /\/resourceGroups\/\{/.test(path); + if (hasSubscription && !hasResourceGroup && providersCount <= 1) { present.add("list-by-subscription"); + } else { + present.add("list-by-parent"); } } } diff --git a/packages/typespec-azure-resource-manager/test/rules/arm-resource-required-operations.test.ts b/packages/typespec-azure-resource-manager/test/rules/arm-resource-required-operations.test.ts index 39f4af4f7c..574685a5eb 100644 --- a/packages/typespec-azure-resource-manager/test/rules/arm-resource-required-operations.test.ts +++ b/packages/typespec-azure-resource-manager/test/rules/arm-resource-required-operations.test.ts @@ -233,6 +233,79 @@ it("does not emit missingDelete or missingList for a singleton tracked resource" .toBeValid(); }); +it("emits missingCreateOrUpdate when only createOrUpdate is missing", async () => { + await tester + .expect( + ` + @armProviderNamespace + namespace Microsoft.Foo; + + model Foo is TrackedResource<{}> { + @key @path @segment("foos") name: string; + } + + @armResourceOperations + interface FooOperations { + read is ArmResourceRead; + delete is ArmResourceDeleteWithoutOkAsync; + listByResourceGroup is ArmResourceListByParent; + listBySubscription is ArmListBySubscription; + } + `, + ) + .toEmitDiagnostics({ + code: "@azure-tools/typespec-azure-resource-manager/arm-resource-required-operations", + message: `Resource 'Foo' must have a PUT (createOrUpdate) operation.`, + }); +}); + +it("emits missingGet for a singleton tracked resource missing read", async () => { + await tester + .expect( + ` + @armProviderNamespace + namespace Microsoft.Foo; + + @singleton + model Foo is TrackedResource<{}> { + @key @path @segment("foos") name: string; + } + + @armResourceOperations + interface FooOperations { + createOrUpdate is ArmResourceCreateOrReplaceAsync; + } + `, + ) + .toEmitDiagnostics({ + code: "@azure-tools/typespec-azure-resource-manager/arm-resource-required-operations", + message: `Resource 'Foo' must have a GET (read) operation.`, + }); +}); + +it("is valid when an extension resource has read, createOrUpdate, delete, and a list operation", async () => { + await tester + .expect( + ` + @armProviderNamespace + namespace Microsoft.Foo; + + model Foo is ExtensionResource<{}> { + @key @path @segment("foos") name: string; + } + + @armResourceOperations + interface FooOperations { + read is ArmResourceRead; + createOrUpdate is ArmResourceCreateOrReplaceAsync; + delete is ArmResourceDeleteWithoutOkAsync; + listByParent is ArmResourceListByParent; + } + `, + ) + .toBeValid(); +}); + it("skips @armVirtualResource models", async () => { await tester .expect( From df78d9102bdde354d4e0e0f69e66514e8cfbfe8b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 7 May 2026 22:10:56 +0000 Subject: [PATCH 07/14] Exempt NSPs, PrivateLinks, and PrivateEndpointConnections from required-operations rule Agent-Logs-Url: https://github.com/Azure/typespec-azure/sessions/708a01b8-ca47-4974-ac3a-c4b2d16abbe4 Co-authored-by: markcowl <1054056+markcowl@users.noreply.github.com> --- .../resource-manager/legacy/nsp/main.tsp | 1 - .../resource-types/nsp/main.tsp | 1 - .../rules/arm-resource-required-operations.ts | 40 +++++++ .../arm-resource-required-operations.test.ts | 105 ++++++++++++++++++ 4 files changed, 145 insertions(+), 2 deletions(-) diff --git a/packages/samples/specs/resource-manager/legacy/nsp/main.tsp b/packages/samples/specs/resource-manager/legacy/nsp/main.tsp index a5acd95e2f..6648c0a745 100644 --- a/packages/samples/specs/resource-manager/legacy/nsp/main.tsp +++ b/packages/samples/specs/resource-manager/legacy/nsp/main.tsp @@ -70,7 +70,6 @@ interface Operations extends Azure.ResourceManager.Operations {} model NetworkSecurityPerimeterConfiguration is Azure.ResourceManager.NspConfiguration; alias NspConfigurationOperations = Azure.ResourceManager.NspConfigurations; -#suppress "@azure-tools/typespec-azure-resource-manager/arm-resource-required-operations" "Network security perimeter configuration resources are read-only by design." @armResourceOperations interface Employees { get is ArmResourceRead; diff --git a/packages/samples/specs/resource-manager/resource-types/nsp/main.tsp b/packages/samples/specs/resource-manager/resource-types/nsp/main.tsp index 3bb67ace8c..44de563eb7 100644 --- a/packages/samples/specs/resource-manager/resource-types/nsp/main.tsp +++ b/packages/samples/specs/resource-manager/resource-types/nsp/main.tsp @@ -70,7 +70,6 @@ interface Operations extends Azure.ResourceManager.Operations {} model NetworkSecurityPerimeterConfiguration is Azure.ResourceManager.NspConfiguration; alias NspConfigurationOperations = Azure.ResourceManager.NspConfigurations; -#suppress "@azure-tools/typespec-azure-resource-manager/arm-resource-required-operations" "Network security perimeter configuration resources are read-only by design." @armResourceOperations interface Employees { get is ArmResourceRead; diff --git a/packages/typespec-azure-resource-manager/src/rules/arm-resource-required-operations.ts b/packages/typespec-azure-resource-manager/src/rules/arm-resource-required-operations.ts index a5216e258a..ba1590e05a 100644 --- a/packages/typespec-azure-resource-manager/src/rules/arm-resource-required-operations.ts +++ b/packages/typespec-azure-resource-manager/src/rules/arm-resource-required-operations.ts @@ -2,6 +2,7 @@ import { CodeFix, createRule, defineCodeFix, + getNamespaceFullName, getSourceLocation, Interface, LinterRuleContext, @@ -65,6 +66,10 @@ export const armResourceRequiredOperationsRule = createRule({ for (const resource of provider.resources ?? []) { if (resource.kind === "Other") continue; if (isInternalTypeSpec(program, resource.type)) continue; + // Network Security Perimeter configurations, Private Links, and + // Private Endpoint Connections have their own well-defined operation + // shapes and are exempt from this rule. + if (isExemptCommonTypeResource(resource.type)) continue; const list = groups.get(resource.type); if (list) list.push(resource); else groups.set(resource.type, [resource]); @@ -258,3 +263,38 @@ function makeAddOperationFix( }, }); } + +/** + * Common-types resource models that have their own well-defined operation + * shapes and are therefore exempt from the required-operations rule. These + * are the canonical models in `Azure.ResourceManager.CommonTypes`. + */ +const EXEMPT_COMMON_TYPE_RESOURCES = new Set([ + "PrivateLinkResource", + "PrivateEndpointConnection", + "NetworkSecurityPerimeterConfiguration", +]); +const EXEMPT_COMMON_TYPE_NAMESPACE = "Azure.ResourceManager.CommonTypes"; + +/** + * Returns true if the given model derives from one of the exempt common-type + * ARM resource models (NSP configurations, Private Links, Private Endpoint + * Connections). Walks both `sourceModel` (`is`) and `baseModel` (`extends`) + * chains. + */ +function isExemptCommonTypeResource(model: Model): boolean { + const visited = new Set(); + let current: Model | undefined = model; + while (current && !visited.has(current)) { + visited.add(current); + if ( + EXEMPT_COMMON_TYPE_RESOURCES.has(current.name) && + current.namespace !== undefined && + getNamespaceFullName(current.namespace) === EXEMPT_COMMON_TYPE_NAMESPACE + ) { + return true; + } + current = current.sourceModel ?? current.baseModel; + } + return false; +} diff --git a/packages/typespec-azure-resource-manager/test/rules/arm-resource-required-operations.test.ts b/packages/typespec-azure-resource-manager/test/rules/arm-resource-required-operations.test.ts index 574685a5eb..56c1af4610 100644 --- a/packages/typespec-azure-resource-manager/test/rules/arm-resource-required-operations.test.ts +++ b/packages/typespec-azure-resource-manager/test/rules/arm-resource-required-operations.test.ts @@ -321,3 +321,108 @@ it("skips @armVirtualResource models", async () => { ) .toBeValid(); }); + +it("exempts NetworkSecurityPerimeterConfiguration resources", async () => { + await tester + .expect( + ` + @armProviderNamespace + @versioned(Versions) + namespace Microsoft.Foo; + + enum Versions { + @armCommonTypesVersion(Azure.ResourceManager.CommonTypes.Versions.v5) + v1, + } + + interface Operations extends Azure.ResourceManager.Operations {} + + model Employee is TrackedResource<{}> { + @key @path @segment("employees") name: string; + } + + model EmployeeNspConfig is Azure.ResourceManager.NspConfiguration; + alias EmployeeNspOps = Azure.ResourceManager.NspConfigurations; + + @armResourceOperations + interface Employees { + get is ArmResourceRead; + createOrUpdate is ArmResourceCreateOrReplaceAsync; + delete is ArmResourceDeleteWithoutOkAsync; + listByResourceGroup is ArmResourceListByParent; + listBySubscription is ArmListBySubscription; + /** Get NSP config */ + getNsp is EmployeeNspOps.Read; + /** List NSP configs */ + listNsp is EmployeeNspOps.ListByParent; + } + `, + ) + .toBeValid(); +}); + +it("exempts PrivateLink resources", async () => { + await tester + .expect( + ` + @armProviderNamespace + namespace Microsoft.Foo; + + interface Operations extends Azure.ResourceManager.Operations {} + + model Employee is TrackedResource<{}> { + @key @path @segment("employees") name: string; + } + + model EmployeePrivateLink is Azure.ResourceManager.PrivateLink; + alias EmployeePLOps = Azure.ResourceManager.PrivateLinks; + + @armResourceOperations + interface Employees { + get is ArmResourceRead; + createOrUpdate is ArmResourceCreateOrReplaceAsync; + delete is ArmResourceDeleteWithoutOkAsync; + listByResourceGroup is ArmResourceListByParent; + listBySubscription is ArmListBySubscription; + /** Get private link */ + getPrivateLink is EmployeePLOps.Read; + /** List private links */ + listPrivateLinks is EmployeePLOps.ListByParent; + } + `, + ) + .toBeValid(); +}); + +it("exempts PrivateEndpointConnection resources", async () => { + await tester + .expect( + ` + @armProviderNamespace + namespace Microsoft.Foo; + + interface Operations extends Azure.ResourceManager.Operations {} + + model Employee is TrackedResource<{}> { + @key @path @segment("employees") name: string; + } + + model EmployeePrivateEndpoint is Azure.ResourceManager.PrivateEndpointConnectionResource; + alias EmployeePEOps = Azure.ResourceManager.PrivateEndpoints; + + @armResourceOperations + interface Employees { + get is ArmResourceRead; + createOrUpdate is ArmResourceCreateOrReplaceAsync; + delete is ArmResourceDeleteWithoutOkAsync; + listByResourceGroup is ArmResourceListByParent; + listBySubscription is ArmListBySubscription; + /** Get private endpoint connection */ + getPE is EmployeePEOps.Read; + /** List private endpoint connections */ + listPE is EmployeePEOps.ListByParent; + } + `, + ) + .toBeValid(); +}); From ae3107e88de13ab7633090235c0ecaae46bce706 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 8 May 2026 03:06:39 +0000 Subject: [PATCH 08/14] Regenerate ARM linter docs after rule registration Agent-Logs-Url: https://github.com/Azure/typespec-azure/sessions/3e09d1cd-d919-45c2-a987-9e7c6b7b00d3 Co-authored-by: markcowl <1054056+markcowl@users.noreply.github.com> --- packages/typespec-azure-resource-manager/README.md | 1 + .../docs/libraries/azure-resource-manager/reference/linter.md | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/typespec-azure-resource-manager/README.md b/packages/typespec-azure-resource-manager/README.md index e0d06fdb2e..159acfc89e 100644 --- a/packages/typespec-azure-resource-manager/README.md +++ b/packages/typespec-azure-resource-manager/README.md @@ -42,6 +42,7 @@ Available ruleSets: | [`@azure-tools/typespec-azure-resource-manager/arm-resource-operation-response`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/arm-resource-operation-response) | [RPC 008]: PUT, GET, PATCH & LIST must return the same resource schema. | | [`@azure-tools/typespec-azure-resource-manager/arm-resource-path-segment-invalid-chars`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/arm-resource-path-segment-invalid-chars) | Arm resource name must contain only alphanumeric characters. | | [`@azure-tools/typespec-azure-resource-manager/arm-resource-provisioning-state`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/arm-resource-provisioning-state) | Check for properly configured provisioningState property. | +| [`@azure-tools/typespec-azure-resource-manager/arm-resource-required-operations`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/arm-resource-required-operations) | Tracked, proxy, and extension ARM resources must define the complete set of required operations. | | [`@azure-tools/typespec-azure-resource-manager/arm-custom-resource-no-key`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/arm-custom-resource-no-key) | Validate that custom resource contains a key property. | | [`@azure-tools/typespec-azure-resource-manager/arm-custom-resource-usage-discourage`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/arm-custom-resource-usage-discourage) | Verify the usage of @customAzureResource decorator. | | [`@azure-tools/typespec-azure-resource-manager/beyond-nesting-levels`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/beyond-nesting-levels) | Tracked Resources must use 3 or fewer levels of nesting. | 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 5b46d320f9..a0ec4a30ec 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 @@ -36,12 +36,12 @@ Available ruleSets: | [`@azure-tools/typespec-azure-resource-manager/arm-resource-operation-response`](../rules/arm-resource-operation-response.md) | [RPC 008]: PUT, GET, PATCH & LIST must return the same resource schema. | | [`@azure-tools/typespec-azure-resource-manager/arm-resource-path-segment-invalid-chars`](../rules/arm-resource-path-segment-invalid-chars.md) | Arm resource name must contain only alphanumeric characters. | | [`@azure-tools/typespec-azure-resource-manager/arm-resource-provisioning-state`](../rules/arm-resource-provisioning-state.md) | Check for properly configured provisioningState property. | +| [`@azure-tools/typespec-azure-resource-manager/arm-resource-required-operations`](../rules/arm-resource-required-operations.md) | Tracked, proxy, and extension ARM resources must define the complete set of required operations. | | [`@azure-tools/typespec-azure-resource-manager/arm-custom-resource-no-key`](../rules/arm-custom-resource-no-key.md) | Validate that custom resource contains a key property. | | [`@azure-tools/typespec-azure-resource-manager/arm-custom-resource-usage-discourage`](../rules/arm-custom-resource-usage-discourage.md) | Verify the usage of @customAzureResource decorator. | | [`@azure-tools/typespec-azure-resource-manager/beyond-nesting-levels`](../rules/beyond-nesting-levels.md) | Tracked Resources must use 3 or fewer levels of nesting. | | [`@azure-tools/typespec-azure-resource-manager/arm-resource-operation`](../rules/arm-resource-operation.md) | Validate ARM Resource operations. | | [`@azure-tools/typespec-azure-resource-manager/no-resource-delete-operation`](../rules/no-resource-delete-operation.md) | Check for resources that must have a delete operation. | -| [`@azure-tools/typespec-azure-resource-manager/arm-resource-required-operations`](../rules/arm-resource-required-operations.md) | ARM resources must declare the complete set of required lifecycle and list operations. Singleton-aware; supersedes `no-resource-delete-operation`. | | [`@azure-tools/typespec-azure-resource-manager/empty-updateable-properties`](../rules/empty-updateable-properties.md) | Should have updateable properties. | | [`@azure-tools/typespec-azure-resource-manager/arm-resource-interface-requires-decorator`](../rules/arm-resource-interface-requires-decorator.md) | Each resource interface must have an @armResourceOperations decorator. | | [`@azure-tools/typespec-azure-resource-manager/arm-resource-invalid-action-verb`](../rules/arm-resource-invalid-action-verb.md) | Actions must be HTTP Post or Get operations. | From 822a5e061a5a303583bdb2e0e67ffdd9412c4c3c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 8 May 2026 23:01:35 +0000 Subject: [PATCH 09/14] Merge main; resolve conflicts and regenerate ARM linter docs Agent-Logs-Url: https://github.com/Azure/typespec-azure/sessions/2d658f0a-adde-4a23-a7d1-ad8ed557edc5 Co-authored-by: markcowl <1054056+markcowl@users.noreply.github.com> --- ...on-progression-rule-2026-04-28-23-52-25.md | 8 + ...peration-id-warning-2026-05-07-18-02-05.md | 7 + ...st-property-example-2026-05-07-15-04-16.md | 7 + core | 2 +- packages/benchmark/src/cli.ts | 3 - packages/benchmark/src/format-comment.ts | 44 +++-- packages/typespec-autorest/src/lib.ts | 6 + packages/typespec-autorest/src/openapi.ts | 42 ++++- .../src/openapi2-document.ts | 3 + .../typespec-autorest/test/models.test.ts | 14 ++ .../test/openapi-output.test.ts | 64 +++++++ .../typespec-azure-resource-manager/README.md | 1 + .../src/linter.ts | 2 + .../src/rules/version-progression.ts | 75 ++++++++ .../test/rules/version-progression.test.ts | 167 ++++++++++++++++++ .../src/rulesets/resource-manager.ts | 1 + .../benchmarks/benchmark-dashboard.tsx | 115 ++---------- .../howtos/Versioning/01-about-versioning.md | 2 + .../reference/linter.md | 1 + .../rules/version-progression.md | 70 ++++++++ 20 files changed, 502 insertions(+), 132 deletions(-) create mode 100644 .chronus/changes/add-version-progression-rule-2026-04-28-23-52-25.md create mode 100644 .chronus/changes/autorest-duplicate-operation-id-warning-2026-05-07-18-02-05.md create mode 100644 .chronus/changes/fix-autorest-property-example-2026-05-07-15-04-16.md create mode 100644 packages/typespec-azure-resource-manager/src/rules/version-progression.ts create mode 100644 packages/typespec-azure-resource-manager/test/rules/version-progression.test.ts create mode 100644 website/src/content/docs/docs/libraries/azure-resource-manager/rules/version-progression.md diff --git a/.chronus/changes/add-version-progression-rule-2026-04-28-23-52-25.md b/.chronus/changes/add-version-progression-rule-2026-04-28-23-52-25.md new file mode 100644 index 0000000000..b3227d6de5 --- /dev/null +++ b/.chronus/changes/add-version-progression-rule-2026-04-28-23-52-25.md @@ -0,0 +1,8 @@ +--- +changeKind: feature +packages: + - "@azure-tools/typespec-azure-resource-manager" + - "@azure-tools/typespec-azure-rulesets" +--- + +Add new `version-progression` linter rule that validates ARM service versions all use unique dates and are declared in strictly increasing chronological order. Two api-versions sharing the same `YYYY-MM-DD` date (for example, `2026-04-28` and `2026-04-28-preview`) are not allowed. diff --git a/.chronus/changes/autorest-duplicate-operation-id-warning-2026-05-07-18-02-05.md b/.chronus/changes/autorest-duplicate-operation-id-warning-2026-05-07-18-02-05.md new file mode 100644 index 0000000000..8b4391c9a7 --- /dev/null +++ b/.chronus/changes/autorest-duplicate-operation-id-warning-2026-05-07-18-02-05.md @@ -0,0 +1,7 @@ +--- +changeKind: fix +packages: + - "@azure-tools/typespec-autorest" +--- + +Add an autorest emitter warning when multiple operations resolve to the same OpenAPI `operationId`, and report the warning on each conflicting operation. diff --git a/.chronus/changes/fix-autorest-property-example-2026-05-07-15-04-16.md b/.chronus/changes/fix-autorest-property-example-2026-05-07-15-04-16.md new file mode 100644 index 0000000000..222ec662aa --- /dev/null +++ b/.chronus/changes/fix-autorest-property-example-2026-05-07-15-04-16.md @@ -0,0 +1,7 @@ +--- +changeKind: fix +packages: + - "@azure-tools/typespec-autorest" +--- + +Emit intrinsic `@TypeSpec.example(...)` on model properties in the autorest OpenAPI2 emitter so property `example` values are preserved in generated definitions. diff --git a/core b/core index 3e578c2cec..0aa3dd239d 160000 --- a/core +++ b/core @@ -1 +1 @@ -Subproject commit 3e578c2cec441296a56753dcf087f9e8a32039d7 +Subproject commit 0aa3dd239dac4f0cb7083168f830a84eb55a865c diff --git a/packages/benchmark/src/cli.ts b/packages/benchmark/src/cli.ts index 48ba67537e..d26384eadd 100644 --- a/packages/benchmark/src/cli.ts +++ b/packages/benchmark/src/cli.ts @@ -43,7 +43,6 @@ Compare options: --output Output file (default: stdout) --format Output format: "console" or "markdown" (default: console) --detailed Show per-rule/per-emitter-step breakdown - --changes-only Only show metrics with notable changes Generate-history options: --dir Read results from a directory instead of the benchmark-data git branch @@ -122,7 +121,6 @@ async function compareCommand(args: Record): Promise { const threshold = args["threshold"] ? parseFloat(args["threshold"]) : undefined; const format = args["format"] ?? "console"; - const changesOnly = args["changes-only"] === "true"; const outputFile = args["output"]; const baseline = await loadJson(baselineFile); @@ -133,7 +131,6 @@ async function compareCommand(args: Record): Promise { if (format === "markdown") { output = formatPrComment(comparisons, baseline.commit, current.commit, { threshold, - changesOnly, }); } else { output = formatConsoleSummary(comparisons, threshold); diff --git a/packages/benchmark/src/format-comment.ts b/packages/benchmark/src/format-comment.ts index aa620aaecf..a774c70e70 100644 --- a/packages/benchmark/src/format-comment.ts +++ b/packages/benchmark/src/format-comment.ts @@ -145,8 +145,6 @@ const LEGEND = export interface FormatOptions { /** Change threshold for highlighting (default: 5%). */ threshold?: number; - /** Only show metrics with notable changes. */ - changesOnly?: boolean; } /** Format comparison results as a GitHub PR comment markdown. */ @@ -157,30 +155,26 @@ export function formatPrComment( options: FormatOptions = {}, ): string { const threshold = options.threshold ?? DEFAULT_THRESHOLD; - const changesOnly = options.changesOnly ?? false; const lines: string[] = []; lines.push("## ⚡ Benchmark Results\n"); - lines.push( - `Comparing [\`${currentCommit.slice(0, 7)}\`] against baseline [\`${baselineCommit.slice(0, 7)}\`]\n`, - ); // Average metrics across all specs const averaged = averageComparisonMetrics(comparisons); + const regressions = averaged.filter((m) => m.percentChange >= threshold); - let metrics = averaged; - if (changesOnly) { - metrics = metrics.filter((m) => Math.abs(m.percentChange) >= threshold); - } - - if (metrics.length === 0) { - lines.push("_No notable changes._\n"); + // Top-level summary: show regressions prominently, otherwise a simple ok message + if (regressions.length === 0) { + lines.push("✅ No performance regressions detected.\n"); } else { + lines.push( + `⚠️ **${regressions.length} metric(s) regressed** above the +${threshold}% threshold:\n`, + ); lines.push("| Metric | Baseline | Current | Change |"); lines.push("|--------|----------|---------|--------|"); - for (const m of metrics) { - const indicator = changeIndicator(m.percentChange, threshold); - const changeStr = `${formatPercent(m.percentChange)} ${indicator}`.trim(); + for (const m of regressions) { + const changeStr = + `${formatPercent(m.percentChange)} ${changeIndicator(m.percentChange, threshold)}`.trim(); const th = thresholdsFor(m.label); lines.push( `| ${displayLabel(m.label)} | ${formatMsColored(m.baseline, th)} | ${formatMsColored(m.current, th)} | ${changeStr} |`, @@ -189,9 +183,27 @@ export function formatPrComment( lines.push(""); } + // Full details collapsed const specNames = comparisons.map((c) => c.specName).join(", "); + lines.push("
"); + lines.push( + `Full details – comparing ${currentCommit.slice(0, 7)} vs baseline ${baselineCommit.slice(0, 7)}\n`, + ); + lines.push("| Metric | Baseline | Current | Change |"); + lines.push("|--------|----------|---------|--------|"); + for (const m of averaged) { + const changeStr = + `${formatPercent(m.percentChange)} ${changeIndicator(m.percentChange, threshold)}`.trim(); + const th = thresholdsFor(m.label); + lines.push( + `| ${displayLabel(m.label)} | ${formatMsColored(m.baseline, th)} | ${formatMsColored(m.current, th)} | ${changeStr} |`, + ); + } + lines.push(""); lines.push(`> Averaged across ${comparisons.length} specs (${specNames}).`); lines.push(`> Threshold: changes > ±${threshold}% are highlighted.`); + lines.push(LEGEND); + lines.push("
"); return lines.join("\n"); } diff --git a/packages/typespec-autorest/src/lib.ts b/packages/typespec-autorest/src/lib.ts index de1af15318..3dc1dc96e0 100644 --- a/packages/typespec-autorest/src/lib.ts +++ b/packages/typespec-autorest/src/lib.ts @@ -288,6 +288,12 @@ export const $lib = createTypeSpecLibrary({ default: paramMessage`Example file ${"filename"} uses duplicate title '${"title"}' for operationId '${"operationId"}'`, }, }, + "duplicate-operation-id": { + severity: "warning", + messages: { + default: paramMessage`Operation ID '${"operationId"}' is duplicated across operations. OpenAPI requires operationId values to be globally unique.`, + }, + }, "invalid-schema": { severity: "error", messages: { diff --git a/packages/typespec-autorest/src/openapi.ts b/packages/typespec-autorest/src/openapi.ts index 5b14d319b8..658d82854b 100644 --- a/packages/typespec-autorest/src/openapi.ts +++ b/packages/typespec-autorest/src/openapi.ts @@ -81,6 +81,7 @@ import { getRelativePathFromDirectory, getRootLength, getSummary, + getExamples as getTypeSpecExamples, getVisibilityForClass, ignoreDiagnostics, interpolatePath, @@ -109,7 +110,7 @@ import { } from "@typespec/compiler"; import { SyntaxKind } from "@typespec/compiler/ast"; import { $ } from "@typespec/compiler/typekit"; -import { TwoLevelMap } from "@typespec/compiler/utils"; +import { DuplicateTracker, TwoLevelMap } from "@typespec/compiler/utils"; import { AuthenticationOptionReference, AuthenticationReference, @@ -152,7 +153,7 @@ import { } from "@typespec/openapi"; import { getVersionsForEnum } from "@typespec/versioning"; import { AutorestOpenAPISchema } from "./autorest-openapi-schema.js"; -import { getExamples, getRef } from "./decorators.js"; +import { getExamples as getAutorestExamples, getRef } from "./decorators.js"; import { sortWithJsonSchema } from "./json-schema-sorter/sorter.js"; import { createDiagnostic, reportDiagnostic } from "./lib.js"; import { @@ -596,7 +597,7 @@ export async function getOpenAPIForService( currentEndpoint.deprecated = true; } - const examples = getExamples(program, op); + const examples = getAutorestExamples(program, op); if (examples) { currentEndpoint["x-ms-examples"] = examples.reduce( (acc, example) => ({ ...acc, [example.title]: { $ref: example.pathOrUri } }), @@ -2280,6 +2281,11 @@ export async function getOpenAPIForService( newTarget.uniqueItems = true; } + const examples = getTypeSpecExamples(program, typespecType); + if (typespecType.kind === "ModelProperty" && examples.length > 0) { + newTarget.example = serializeValueAsJson(program, examples[0].value, typespecType); + } + if (isSecret(program, typespecType)) { newTarget.format = "password"; newTarget["x-ms-secret"] = true; @@ -2943,6 +2949,7 @@ export function createDefaultDocumentProxy( const tags = new Set(); const definitions = new Map(); const parameters: Map = new Map(); + const operationIds = new DuplicateTracker(); let examples: Map> = new Map(); let operationIdsWithExamples: Set = new Set(); return { @@ -2980,6 +2987,7 @@ export function createDefaultDocumentProxy( } const resolvedOp = pathItem[op.verb]!; resolvedOp.operationId = resolveOperationId(context, op.operation); + operationIds.track(resolvedOp.operationId, op.operation); return resolvedOp; }, addTag(tag: string, op: Operation) { @@ -3015,6 +3023,7 @@ export function createDefaultDocumentProxy( operationIdsWithExamples = exampleIds; }, resolveDocuments(context: AutorestEmitterContext) { + reportDuplicateOperationIds(program, operationIds); root.definitions = {}; for (const [name, schema] of definitions) { root.definitions[name] = schema; @@ -3079,14 +3088,14 @@ function createFeatureDocumentProxy( return createDefaultDocumentProxy(program, service, options, version); const root: Map = new Map(); const operationFeatures: Map> = new Map(); + const operationIds = new Map>(); let examples: Map> = new Map(); let operationIdsWithExamples: Set = new Set(); for (const featureName of features.keys()) { const featureOptions = features.get(featureName)!; - root.set( - featureName.toLowerCase(), - initializeOpenAPIDocumentItem(program, service, featureOptions, version), - ); + const featureKey = featureName.toLowerCase(); + root.set(featureKey, initializeOpenAPIDocumentItem(program, service, featureOptions, version)); + operationIds.set(featureKey, new DuplicateTracker()); } const defaultFeature = [...root.entries()].filter( ([key, _]) => key.toLowerCase() === "common", @@ -3133,6 +3142,7 @@ function createFeatureDocumentProxy( } const resolvedOp = pathItem[op.verb]!; const opId = resolveOperationId(context, op.operation); + operationIds.get(options.featureName.toLowerCase())?.track(opId, op.operation); addFeatureOperation(opId, options.featureName); resolvedOp.operationId = opId; return resolvedOp; @@ -3182,6 +3192,9 @@ function createFeatureDocumentProxy( }, resolveDocuments(context: AutorestEmitterContext) { const docs: AutorestEmitterResult[] = []; + for (const tracker of operationIds.values()) { + reportDuplicateOperationIds(program, tracker); + } for (const [featureName, featureItem] of root.entries()) { const exampleIds = operationFeatures.get(featureName) || new Set(); const featureExamples = [...exampleIds] @@ -3261,6 +3274,21 @@ function createFeatureDocumentProxy( } } +function reportDuplicateOperationIds( + program: Program, + duplicateTracker: DuplicateTracker, +) { + for (const [operationId, duplicates] of duplicateTracker.entries()) { + for (const duplicate of duplicates) { + reportDiagnostic(program, { + code: "duplicate-operation-id", + format: { operationId }, + target: duplicate, + }); + } + } +} + function initializeOpenAPIDocumentItem( program: Program, service: Service, diff --git a/packages/typespec-autorest/src/openapi2-document.ts b/packages/typespec-autorest/src/openapi2-document.ts index d004b9fa40..6f2aa78d0a 100644 --- a/packages/typespec-autorest/src/openapi2-document.ts +++ b/packages/typespec-autorest/src/openapi2-document.ts @@ -223,6 +223,9 @@ export type OpenAPI2Schema = Extensions & { * "default" has no meaning for required parameters.) See https://tools.ietf.org/html/draft-fge-json-schema-validation-00#section-6.2. Unlike JSON Schema this value MUST conform to the defined type for this parameter. */ default?: string | boolean | number | Record; + /** Example value represented as JSON. */ + example?: unknown; + /** * the maximum value for the property * diff --git a/packages/typespec-autorest/test/models.test.ts b/packages/typespec-autorest/test/models.test.ts index 816a3d4491..2e783e02c8 100644 --- a/packages/typespec-autorest/test/models.test.ts +++ b/packages/typespec-autorest/test/models.test.ts @@ -62,6 +62,20 @@ describe("typespec-autorest: model definitions", () => { strictEqual(res.defs.Foo.properties.y.title, "YProp"); }); + it("emits property-level @example", async () => { + const res = await oapiForModel( + "Test", + ` + model Test { + @TypeSpec.example("core.windows.net") + abcd: string; + }; + `, + ); + + expect(res.defs.Test.properties.abcd.example).toEqual("core.windows.net"); + }); + it("uses json name specified via @encodedName", async () => { const res = await oapiForModel( "Foo", diff --git a/packages/typespec-autorest/test/openapi-output.test.ts b/packages/typespec-autorest/test/openapi-output.test.ts index 9fb5a1ff1c..3a4b0eff24 100644 --- a/packages/typespec-autorest/test/openapi-output.test.ts +++ b/packages/typespec-autorest/test/openapi-output.test.ts @@ -587,6 +587,70 @@ describe("typespec-autorest: operations", () => { // Original operation in the custom namespace should still use namespace prefix strictEqual(res.paths["/custom-op"].get.operationId, "CustomNamespace_CustomOperation"); }); + + it("emits warning on each operation when operationId is duplicated", async () => { + const diagnostics = await diagnoseOpenApiFor(` + @service namespace MyService; + + namespace A { + @route("/foo") + @operationId("Shared_List") + op list(): string; + } + + namespace B { + @route("/bar") + @operationId("Shared_List") + op list(): string; + } + `); + const duplicateMessage = + "Operation ID 'Shared_List' is duplicated across operations. OpenAPI requires operationId values to be globally unique."; + + expectDiagnostics(diagnostics, [ + { + code: "@azure-tools/typespec-autorest/duplicate-operation-id", + message: duplicateMessage, + }, + { + code: "@azure-tools/typespec-autorest/duplicate-operation-id", + message: duplicateMessage, + }, + ]); + }); + + it("emits warning when interfaces and operations with same names in different namespaces collide", async () => { + const diagnostics = await diagnoseOpenApiFor(` + @service namespace MyService; + + namespace A { + interface Shared { + @route("/a/list") + op list(): string; + } + } + + namespace B { + interface Shared { + @route("/b/list") + op list(): string; + } + } + `); + const duplicateMessage = + "Operation ID 'Shared_List' is duplicated across operations. OpenAPI requires operationId values to be globally unique."; + + expectDiagnostics(diagnostics, [ + { + code: "@azure-tools/typespec-autorest/duplicate-operation-id", + message: duplicateMessage, + }, + { + code: "@azure-tools/typespec-autorest/duplicate-operation-id", + message: duplicateMessage, + }, + ]); + }); }); describe("typespec-autorest: request", () => { diff --git a/packages/typespec-azure-resource-manager/README.md b/packages/typespec-azure-resource-manager/README.md index 159acfc89e..1e4f6973b8 100644 --- a/packages/typespec-azure-resource-manager/README.md +++ b/packages/typespec-azure-resource-manager/README.md @@ -43,6 +43,7 @@ Available ruleSets: | [`@azure-tools/typespec-azure-resource-manager/arm-resource-path-segment-invalid-chars`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/arm-resource-path-segment-invalid-chars) | Arm resource name must contain only alphanumeric characters. | | [`@azure-tools/typespec-azure-resource-manager/arm-resource-provisioning-state`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/arm-resource-provisioning-state) | Check for properly configured provisioningState property. | | [`@azure-tools/typespec-azure-resource-manager/arm-resource-required-operations`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/arm-resource-required-operations) | Tracked, proxy, and extension ARM resources must define the complete set of required operations. | +| [`@azure-tools/typespec-azure-resource-manager/version-progression`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/version-progression) | Validate that ARM service versions all use unique dates and are declared in strictly increasing chronological order. | | [`@azure-tools/typespec-azure-resource-manager/arm-custom-resource-no-key`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/arm-custom-resource-no-key) | Validate that custom resource contains a key property. | | [`@azure-tools/typespec-azure-resource-manager/arm-custom-resource-usage-discourage`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/arm-custom-resource-usage-discourage) | Verify the usage of @customAzureResource decorator. | | [`@azure-tools/typespec-azure-resource-manager/beyond-nesting-levels`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/beyond-nesting-levels) | Tracked Resources must use 3 or fewer levels of nesting. | diff --git a/packages/typespec-azure-resource-manager/src/linter.ts b/packages/typespec-azure-resource-manager/src/linter.ts index 2a94e96f8d..bc65def6ca 100644 --- a/packages/typespec-azure-resource-manager/src/linter.ts +++ b/packages/typespec-azure-resource-manager/src/linter.ts @@ -34,6 +34,7 @@ import { resourceNameRule } from "./rules/resource-name.js"; import { retryAfterRule } from "./rules/retry-after.js"; import { secretProprule } from "./rules/secret-prop.js"; import { unsupportedTypeRule } from "./rules/unsupported-type.js"; +import { versionProgressionRule } from "./rules/version-progression.js"; const rules = [ armNoRecordRule, @@ -51,6 +52,7 @@ const rules = [ armResourcePathInvalidCharsRule, armResourceProvisioningStateRule, armResourceRequiredOperationsRule, + versionProgressionRule, armCustomResourceNoKey, armCustomResourceUsageDiscourage, beyondNestingRule, diff --git a/packages/typespec-azure-resource-manager/src/rules/version-progression.ts b/packages/typespec-azure-resource-manager/src/rules/version-progression.ts new file mode 100644 index 0000000000..b6a6f62f80 --- /dev/null +++ b/packages/typespec-azure-resource-manager/src/rules/version-progression.ts @@ -0,0 +1,75 @@ +import { EnumMember, Namespace, createRule, paramMessage } from "@typespec/compiler"; + +import { getVersion } from "@typespec/versioning"; + +interface ParsedVersion { + enumMember: EnumMember; + raw: string; + date: string; +} + +/** + * Parse a version string of the form "YYYY-MM-DD[-suffix[.N]]" and return its + * date component. Returns undefined if the string does not match the expected + * format (the `arm-resource-invalid-version-format` rule is responsible for + * reporting that). + */ +function parseVersionDate(value: string): string | undefined { + const match = value.match(/^(\d{4}-\d{2}-\d{2})(?:-[A-Za-z]+(?:\.\d+)?)?$/); + return match ? match[1] : undefined; +} + +/** + * Validates that the versions declared on an ARM service namespace use a + * unique date and are listed in strictly increasing chronological order. Two + * api-versions sharing the same date (for example, `2026-04-28` and + * `2026-04-28-preview`) are not allowed — every version's `YYYY-MM-DD` date + * must be different from every other version's date in the same `Versions` + * enum. + */ +export const versionProgressionRule = createRule({ + name: "version-progression", + severity: "warning", + description: + "Validate that ARM service versions all use unique dates and are declared in strictly increasing chronological order.", + url: "https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/version-progression", + messages: { + notMonotonic: paramMessage`Version '${"version"}' is declared after '${"previous"}' but is not chronologically later. ARM versions must be declared in strictly increasing chronological order by date.`, + duplicateDate: paramMessage`Version '${"version"}' has the same date as '${"previous"}'. Every ARM api-version must use a unique date — preview and stable versions cannot share the same 'YYYY-MM-DD'.`, + }, + create(context) { + return { + namespace: (namespace: Namespace) => { + const map = getVersion(context.program, namespace); + if (map === undefined) return; + + const parsed: ParsedVersion[] = []; + for (const version of map.getVersions()) { + const value = version.enumMember.value ?? version.enumMember.name; + if (typeof value !== "string") continue; + const date = parseVersionDate(value); + if (date === undefined) continue; + parsed.push({ enumMember: version.enumMember, raw: value, date }); + } + + for (let i = 1; i < parsed.length; i++) { + const current = parsed[i]; + const previous = parsed[i - 1]; + if (current.date === previous.date) { + context.reportDiagnostic({ + messageId: "duplicateDate", + format: { version: current.raw, previous: previous.raw }, + target: current.enumMember, + }); + } else if (current.date < previous.date) { + context.reportDiagnostic({ + messageId: "notMonotonic", + format: { version: current.raw, previous: previous.raw }, + target: current.enumMember, + }); + } + } + }, + }; + }, +}); diff --git a/packages/typespec-azure-resource-manager/test/rules/version-progression.test.ts b/packages/typespec-azure-resource-manager/test/rules/version-progression.test.ts new file mode 100644 index 0000000000..1bd101ae9a --- /dev/null +++ b/packages/typespec-azure-resource-manager/test/rules/version-progression.test.ts @@ -0,0 +1,167 @@ +import { Tester } from "#test/tester.js"; +import { + LinterRuleTester, + TesterInstance, + createLinterRuleTester, +} from "@typespec/compiler/testing"; +import { beforeEach, it } from "vitest"; + +import { versionProgressionRule } from "../../src/rules/version-progression.js"; + +let runner: TesterInstance; +let tester: LinterRuleTester; + +beforeEach(async () => { + runner = await Tester.createInstance(); + tester = createLinterRuleTester( + runner, + versionProgressionRule, + "@azure-tools/typespec-azure-resource-manager", + ); +}); + +it("is valid for chronologically increasing versions with unique dates", async () => { + await tester + .expect( + ` + @versioned(Versions) + @armProviderNamespace + namespace Microsoft.Foo; + + enum Versions { + v2024_01_01_preview: "2024-01-01-preview", + v2024_03_01: "2024-03-01", + v2024_06_01: "2024-06-01", + } + `, + ) + .toBeValid(); +}); + +it("is valid when there is only a single version", async () => { + await tester + .expect( + ` + @versioned(Versions) + @armProviderNamespace + namespace Microsoft.Foo; + + enum Versions { + v2024_01_01: "2024-01-01", + } + `, + ) + .toBeValid(); +}); + +it("is valid when the namespace is not versioned", async () => { + await tester + .expect( + ` + @armProviderNamespace + namespace Microsoft.Foo; + `, + ) + .toBeValid(); +}); + +it("emits diagnostic when versions are not in chronological order", async () => { + await tester + .expect( + ` + @versioned(Versions) + @armProviderNamespace + namespace Microsoft.Foo; + + enum Versions { + v2024_06_01: "2024-06-01", + v2024_01_01: "2024-01-01", + } + `, + ) + .toEmitDiagnostics({ + code: "@azure-tools/typespec-azure-resource-manager/version-progression", + message: + "Version '2024-01-01' is declared after '2024-06-01' but is not chronologically later. ARM versions must be declared in strictly increasing chronological order by date.", + }); +}); + +it("emits diagnostic when a stable version shares a date with a preview version", async () => { + await tester + .expect( + ` + @versioned(Versions) + @armProviderNamespace + namespace Microsoft.Foo; + + enum Versions { + v2026_04_28_preview: "2026-04-28-preview", + v2026_04_28: "2026-04-28", + } + `, + ) + .toEmitDiagnostics({ + code: "@azure-tools/typespec-azure-resource-manager/version-progression", + message: + "Version '2026-04-28' has the same date as '2026-04-28-preview'. Every ARM api-version must use a unique date — preview and stable versions cannot share the same 'YYYY-MM-DD'.", + }); +}); + +it("emits diagnostic when a preview version follows the stable version with the same date", async () => { + await tester + .expect( + ` + @versioned(Versions) + @armProviderNamespace + namespace Microsoft.Foo; + + enum Versions { + v2024_01_01: "2024-01-01", + v2024_01_01_preview: "2024-01-01-preview", + } + `, + ) + .toEmitDiagnostics({ + code: "@azure-tools/typespec-azure-resource-manager/version-progression", + message: + "Version '2024-01-01-preview' has the same date as '2024-01-01'. Every ARM api-version must use a unique date — preview and stable versions cannot share the same 'YYYY-MM-DD'.", + }); +}); + +it("emits diagnostic when two preview versions share a date", async () => { + await tester + .expect( + ` + @versioned(Versions) + @armProviderNamespace + namespace Microsoft.Foo; + + enum Versions { + v2024_01_01_alpha_1: "2024-01-01-alpha.1", + v2024_01_01_alpha_2: "2024-01-01-alpha.2", + } + `, + ) + .toEmitDiagnostics({ + code: "@azure-tools/typespec-azure-resource-manager/version-progression", + message: + "Version '2024-01-01-alpha.2' has the same date as '2024-01-01-alpha.1'. Every ARM api-version must use a unique date — preview and stable versions cannot share the same 'YYYY-MM-DD'.", + }); +}); + +it("ignores malformed version strings (handled by arm-resource-invalid-version-format)", async () => { + await tester + .expect( + ` + @versioned(Versions) + @armProviderNamespace + namespace Microsoft.Foo; + + enum Versions { + v1: "not-a-date", + v2: "2024-01-01", + } + `, + ) + .toBeValid(); +}); diff --git a/packages/typespec-azure-rulesets/src/rulesets/resource-manager.ts b/packages/typespec-azure-rulesets/src/rulesets/resource-manager.ts index 9dd414d5a1..e835dc1f14 100644 --- a/packages/typespec-azure-rulesets/src/rulesets/resource-manager.ts +++ b/packages/typespec-azure-rulesets/src/rulesets/resource-manager.ts @@ -66,6 +66,7 @@ export default { "@azure-tools/typespec-azure-resource-manager/arm-resource-duplicate-property": true, "@azure-tools/typespec-azure-resource-manager/arm-resource-invalid-envelope-property": true, "@azure-tools/typespec-azure-resource-manager/arm-resource-invalid-version-format": true, + "@azure-tools/typespec-azure-resource-manager/version-progression": true, "@azure-tools/typespec-azure-resource-manager/arm-resource-key-invalid-chars": true, "@azure-tools/typespec-azure-resource-manager/arm-resource-name-pattern": true, "@azure-tools/typespec-azure-resource-manager/arm-custom-resource-no-key": true, diff --git a/website/src/components/benchmarks/benchmark-dashboard.tsx b/website/src/components/benchmarks/benchmark-dashboard.tsx index 0ee24130cb..8b440582c2 100644 --- a/website/src/components/benchmarks/benchmark-dashboard.tsx +++ b/website/src/components/benchmarks/benchmark-dashboard.tsx @@ -6,7 +6,6 @@ import { LinearScale, LineElement, PointElement, - TimeScale, Title, Tooltip, type ChartOptions, @@ -15,102 +14,9 @@ import { import { useEffect, useMemo, useState } from "react"; import { Line } from "react-chartjs-2"; -// Minimal Chart.js date adapter using Temporal -import { _adapters } from "chart.js"; - -const MILLISECONDS_PER_UNIT: Record = { - millisecond: 1, - second: 1000, - minute: 60_000, - hour: 3_600_000, - day: 86_400_000, - week: 604_800_000, - month: 2_592_000_000, - quarter: 7_776_000_000, - year: 31_536_000_000, -}; - -_adapters._date.override({ - formats() { - return { - datetime: "yyyy-MM-dd HH:mm", - millisecond: "HH:mm:ss.SSS", - second: "HH:mm:ss", - minute: "HH:mm", - hour: "HH:mm", - day: "MMM d", - week: "MMM d", - month: "MMM yyyy", - quarter: "QQ yyyy", - year: "yyyy", - }; - }, - parse(value: unknown) { - if (value === null || value === undefined) return null; - if (typeof value === "number") return value; - if (typeof value === "string") return new Date(value).getTime(); - if (value instanceof Date) return value.getTime(); - return null; - }, - format(timestamp: number, fmt: string) { - const d = new Date(timestamp); - // Simple format substitution - return fmt - .replace("yyyy", String(d.getFullYear())) - .replace("MMM", d.toLocaleString("en", { month: "short" })) - .replace("MM", String(d.getMonth() + 1).padStart(2, "0")) - .replace("dd", String(d.getDate()).padStart(2, "0")) - .replace(/\bd\b/, String(d.getDate())) - .replace("HH", String(d.getHours()).padStart(2, "0")) - .replace("mm", String(d.getMinutes()).padStart(2, "0")) - .replace("ss", String(d.getSeconds()).padStart(2, "0")) - .replace("SSS", String(d.getMilliseconds()).padStart(3, "0")) - .replace("QQ", `Q${Math.floor(d.getMonth() / 3) + 1}`); - }, - add(timestamp: number, amount: number, unit: string) { - return timestamp + amount * (MILLISECONDS_PER_UNIT[unit] ?? 0); - }, - diff(a: number, b: number, unit: string) { - return (a - b) / (MILLISECONDS_PER_UNIT[unit] ?? 1); - }, - startOf(timestamp: number, unit: string) { - const d = new Date(timestamp); - if (unit === "year") return new Date(d.getFullYear(), 0, 1).getTime(); - if (unit === "quarter") - return new Date(d.getFullYear(), Math.floor(d.getMonth() / 3) * 3, 1).getTime(); - if (unit === "month") return new Date(d.getFullYear(), d.getMonth(), 1).getTime(); - if (unit === "week") { - const day = d.getDay(); - return new Date(d.getFullYear(), d.getMonth(), d.getDate() - day).getTime(); - } - if (unit === "day") return new Date(d.getFullYear(), d.getMonth(), d.getDate()).getTime(); - if (unit === "hour") - return new Date(d.getFullYear(), d.getMonth(), d.getDate(), d.getHours()).getTime(); - if (unit === "minute") - return new Date( - d.getFullYear(), - d.getMonth(), - d.getDate(), - d.getHours(), - d.getMinutes(), - ).getTime(); - if (unit === "second") - return new Date( - d.getFullYear(), - d.getMonth(), - d.getDate(), - d.getHours(), - d.getMinutes(), - d.getSeconds(), - ).getTime(); - return timestamp; - }, -}); - ChartJS.register( CategoryScale, LinearScale, - TimeScale, PointElement, LineElement, Title, @@ -216,13 +122,12 @@ function BenchmarkChart({ data, category }: { data: HistoryData; category: Metri const colors = useMemo(() => seriesColors(metricLabels.length), [metricLabels.length]); const isDense = metricLabels.length >= 8; + const commitLabels = useMemo(() => data.entries.map((e) => e.commit.slice(0, 7)), [data]); + const chartData = useMemo(() => { const datasets = metricLabels.map((label, i) => ({ label: shortLabel(label), - data: data.entries.map((e) => ({ - x: new Date(e.timestamp).getTime(), - y: e.metrics[label] ?? null, - })), + data: data.entries.map((e) => e.metrics[label] ?? null), borderColor: colors[i], backgroundColor: colors[i], borderWidth: 1.5, @@ -230,8 +135,8 @@ function BenchmarkChart({ data, category }: { data: HistoryData; category: Metri pointHoverRadius: 5, tension: 0.2, })); - return { datasets }; - }, [data, metricLabels, colors]); + return { labels: commitLabels, datasets }; + }, [data, metricLabels, colors, commitLabels]); const options: ChartOptions<"line"> = useMemo( () => ({ @@ -267,12 +172,12 @@ function BenchmarkChart({ data, category }: { data: HistoryData; category: Metri }, scales: { x: { - type: "time", - time: { - unit: "day", - tooltipFormat: "MMM d, yyyy", + type: "category", + title: { display: true, text: "Commit" }, + ticks: { + maxRotation: 45, + minRotation: 45, }, - title: { display: true, text: "Date" }, }, y: { beginAtZero: true, diff --git a/website/src/content/docs/docs/howtos/Versioning/01-about-versioning.md b/website/src/content/docs/docs/howtos/Versioning/01-about-versioning.md index 74b1594dc4..123c25736f 100644 --- a/website/src/content/docs/docs/howtos/Versioning/01-about-versioning.md +++ b/website/src/content/docs/docs/howtos/Versioning/01-about-versioning.md @@ -15,6 +15,8 @@ Additionally, in Azure, preview APIs have a limited lifespan and limited support - Only one version may be marked with the `@previewVersion` decorator. - Mark all changes from the latest stable with appropriate versioning decorators, using `Versions.` as the version argument (where `` is the name of the last enum value) +The [`@azure-tools/typespec-azure-resource-manager/version-progression`](../../libraries/azure-resource-manager/rules/version-progression.md) linter rule helps enforce that every api-version uses a unique `YYYY-MM-DD` date and that versions are declared in strictly increasing chronological order from top to bottom of the `Versions` enum. + ## Common Version Transition Scenarios These documents add specific guidance about how to follow these guidelines for specific version changes 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 a0ec4a30ec..4a19074240 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 @@ -37,6 +37,7 @@ Available ruleSets: | [`@azure-tools/typespec-azure-resource-manager/arm-resource-path-segment-invalid-chars`](../rules/arm-resource-path-segment-invalid-chars.md) | Arm resource name must contain only alphanumeric characters. | | [`@azure-tools/typespec-azure-resource-manager/arm-resource-provisioning-state`](../rules/arm-resource-provisioning-state.md) | Check for properly configured provisioningState property. | | [`@azure-tools/typespec-azure-resource-manager/arm-resource-required-operations`](../rules/arm-resource-required-operations.md) | Tracked, proxy, and extension ARM resources must define the complete set of required operations. | +| [`@azure-tools/typespec-azure-resource-manager/version-progression`](../rules/version-progression.md) | Validate that ARM service versions all use unique dates and are declared in strictly increasing chronological order. | | [`@azure-tools/typespec-azure-resource-manager/arm-custom-resource-no-key`](../rules/arm-custom-resource-no-key.md) | Validate that custom resource contains a key property. | | [`@azure-tools/typespec-azure-resource-manager/arm-custom-resource-usage-discourage`](../rules/arm-custom-resource-usage-discourage.md) | Verify the usage of @customAzureResource decorator. | | [`@azure-tools/typespec-azure-resource-manager/beyond-nesting-levels`](../rules/beyond-nesting-levels.md) | Tracked Resources must use 3 or fewer levels of nesting. | diff --git a/website/src/content/docs/docs/libraries/azure-resource-manager/rules/version-progression.md b/website/src/content/docs/docs/libraries/azure-resource-manager/rules/version-progression.md new file mode 100644 index 0000000000..bb71d8209a --- /dev/null +++ b/website/src/content/docs/docs/libraries/azure-resource-manager/rules/version-progression.md @@ -0,0 +1,70 @@ +--- +title: version-progression +--- + +```text title="Full name" +@azure-tools/typespec-azure-resource-manager/version-progression +``` + +ARM service api-versions must: + +1. Use a **unique date** — every entry's `YYYY-MM-DD` must differ from every other entry's date in the same `Versions` enum. A preview version and a stable version cannot share the same date (for example, `2026-04-28` and `2026-04-28-preview` together is not allowed). +2. Be declared in **strictly increasing chronological order** from top to bottom. + +This rule complements [`arm-resource-invalid-version-format`](./arm-resource-invalid-version-format.md), which validates the format of each individual version string. `arm-version-progression` does not flag malformed version strings — those are reported by the format rule. + +#### ❌ Incorrect + +A preview and a stable version share the same date: + +```tsp +@versioned(Versions) +@armProviderNamespace +namespace Microsoft.Contoso; + +enum Versions { + v2026_04_28: "2026-04-28", + v2026_04_28_preview: "2026-04-28-preview", +} +``` + +Versions are declared out of chronological order: + +```tsp +@versioned(Versions) +@armProviderNamespace +namespace Microsoft.Contoso; + +enum Versions { + v2024_06_01: "2024-06-01", + v2024_01_01: "2024-01-01", +} +``` + +#### ✅ Correct + +```tsp +@versioned(Versions) +@armProviderNamespace +namespace Microsoft.Contoso; + +enum Versions { + v2024_01_01: "2024-01-01", + v2024_03_01: "2024-03-01", + v2024_06_01_preview: "2024-06-01-preview", +} +``` + +A stable version followed by a preview that is chronologically later (one day after) is also valid — previews must come chronologically after the most recent stable version: + +```tsp +@versioned(Versions) +@armProviderNamespace +namespace Microsoft.Contoso; + +enum Versions { + v2024_03_01: "2024-03-01", + v2024_06_01: "2024-06-01", + v2024_06_02_preview: "2024-06-02-preview", +} +``` From aebb3fad9910d26bb0b641021af5b570ffb8bb76 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 13 May 2026 00:21:07 +0000 Subject: [PATCH 10/14] Narrow rule: tracked resources keep full set; others only need read (and delete if put present) Agent-Logs-Url: https://github.com/Azure/typespec-azure/sessions/42b7ce08-198a-4a01-9260-473a33e46175 Co-authored-by: markcowl <1054056+markcowl@users.noreply.github.com> --- .../legacy/polymorphic-resource/extension.tsp | 5 - .../legacy/polymorphic-resource/proxy.tsp | 1 - .../typespec-azure-resource-manager/README.md | 2 +- .../rules/arm-resource-required-operations.ts | 45 ++++++--- .../arm-resource-required-operations.test.ts | 99 ++++++++++++++++++- .../reference/linter.md | 2 +- .../rules/arm-resource-required-operations.md | 18 ++-- 7 files changed, 134 insertions(+), 38 deletions(-) diff --git a/packages/samples/specs/resource-manager/legacy/polymorphic-resource/extension.tsp b/packages/samples/specs/resource-manager/legacy/polymorphic-resource/extension.tsp index 1f92362486..747e28dc13 100644 --- a/packages/samples/specs/resource-manager/legacy/polymorphic-resource/extension.tsp +++ b/packages/samples/specs/resource-manager/legacy/polymorphic-resource/extension.tsp @@ -45,21 +45,16 @@ model Expense extends Cost { properties: ExpenseProperties; } -#suppress "@azure-tools/typespec-azure-resource-manager/arm-resource-required-operations" "This sample demonstrates read-only polymorphic extension resources; createOrUpdate and delete are intentionally omitted." @armResourceOperations interface Costs { get is Extension.Read; list is Extension.ListByTarget; } -#suppress "@azure-tools/typespec-azure-resource-manager/arm-resource-required-operations" "This sample demonstrates read-only polymorphic extension resources; createOrUpdate and delete are intentionally omitted." @armResourceOperations interface Scopes extends Costs {} -#suppress "@azure-tools/typespec-azure-resource-manager/arm-resource-required-operations" "This sample demonstrates read-only polymorphic extension resources; createOrUpdate and delete are intentionally omitted." @armResourceOperations interface Tenants extends Costs {} -#suppress "@azure-tools/typespec-azure-resource-manager/arm-resource-required-operations" "This sample demonstrates read-only polymorphic extension resources; createOrUpdate and delete are intentionally omitted." @armResourceOperations interface Subscriptions extends Costs {} -#suppress "@azure-tools/typespec-azure-resource-manager/arm-resource-required-operations" "This sample demonstrates read-only polymorphic extension resources; createOrUpdate and delete are intentionally omitted." @armResourceOperations interface ManagementGroups extends Costs {} diff --git a/packages/samples/specs/resource-manager/legacy/polymorphic-resource/proxy.tsp b/packages/samples/specs/resource-manager/legacy/polymorphic-resource/proxy.tsp index 407e352adc..dc0a06bc18 100644 --- a/packages/samples/specs/resource-manager/legacy/polymorphic-resource/proxy.tsp +++ b/packages/samples/specs/resource-manager/legacy/polymorphic-resource/proxy.tsp @@ -49,7 +49,6 @@ model Office extends Room { properties: OfficeProperties; } -#suppress "@azure-tools/typespec-azure-resource-manager/arm-resource-required-operations" "This sample demonstrates a read-only polymorphic proxy resource; createOrUpdate and delete are intentionally omitted." @armResourceOperations interface Rooms { get is ArmResourceRead; diff --git a/packages/typespec-azure-resource-manager/README.md b/packages/typespec-azure-resource-manager/README.md index 99ac0fcc46..964eac7b4d 100644 --- a/packages/typespec-azure-resource-manager/README.md +++ b/packages/typespec-azure-resource-manager/README.md @@ -43,7 +43,7 @@ Available ruleSets: | [`@azure-tools/typespec-azure-resource-manager/arm-resource-operation-response`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/arm-resource-operation-response) | [RPC 008]: PUT, GET, PATCH & LIST must return the same resource schema. | | [`@azure-tools/typespec-azure-resource-manager/arm-resource-path-segment-invalid-chars`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/arm-resource-path-segment-invalid-chars) | Arm resource name must contain only alphanumeric characters. | | [`@azure-tools/typespec-azure-resource-manager/arm-resource-provisioning-state`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/arm-resource-provisioning-state) | Check for properly configured provisioningState property. | -| [`@azure-tools/typespec-azure-resource-manager/arm-resource-required-operations`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/arm-resource-required-operations) | Tracked, proxy, and extension ARM resources must define the complete set of required operations. | +| [`@azure-tools/typespec-azure-resource-manager/arm-resource-required-operations`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/arm-resource-required-operations) | ARM resources must define their required operations: tracked resources need the full lifecycle and list set, other resources need a read, and any resource defining createOrUpdate must also define delete. | | [`@azure-tools/typespec-azure-resource-manager/version-progression`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/version-progression) | Validate that ARM service versions all use unique dates and are declared in strictly increasing chronological order. | | [`@azure-tools/typespec-azure-resource-manager/arm-custom-resource-no-key`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/arm-custom-resource-no-key) | Validate that custom resource contains a key property. | | [`@azure-tools/typespec-azure-resource-manager/arm-custom-resource-usage-discourage`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/arm-custom-resource-usage-discourage) | Verify the usage of @customAzureResource decorator. | diff --git a/packages/typespec-azure-resource-manager/src/rules/arm-resource-required-operations.ts b/packages/typespec-azure-resource-manager/src/rules/arm-resource-required-operations.ts index ba1590e05a..9ccdbd6cb7 100644 --- a/packages/typespec-azure-resource-manager/src/rules/arm-resource-required-operations.ts +++ b/packages/typespec-azure-resource-manager/src/rules/arm-resource-required-operations.ts @@ -39,7 +39,7 @@ export const armResourceRequiredOperationsRule = createRule({ severity: "warning", url: "https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/arm-resource-required-operations", description: - "Tracked, proxy, and extension ARM resources must define the complete set of required operations.", + "ARM resources must define their required operations: tracked resources need the full lifecycle and list set, other resources need a read, and any resource defining createOrUpdate must also define delete.", messages: { default: paramMessage`Resource '${"name"}' is missing required operations: [${"operations"}].`, missingListBySubscription: paramMessage`Tracked resource '${"name"}' must have a list-by-subscription operation.`, @@ -93,9 +93,9 @@ function checkResource( cur.resourceType.types.length < best.resourceType.types.length ? cur : best, ); - const required = getRequiredOperationsForResource(canonical); - if (required.length === 0) return; const present = getPresentOperations(entries); + const required = getRequiredOperationsForResource(canonical, present); + if (required.length === 0) return; const missing = required.filter((op) => !present.has(op)); if (missing.length === 0) return; @@ -119,21 +119,34 @@ function checkResource( }); } -function getRequiredOperationsForResource(resource: ResolvedResource): RequiredOperation[] { +function getRequiredOperationsForResource( + resource: ResolvedResource, + present: Set, +): RequiredOperation[] { const isSingleton = resource.singleton !== undefined; - if (isSingleton) { - return ["read", "createOrUpdate"]; + if (resource.kind === "Tracked") { + if (isSingleton) { + return ["read", "createOrUpdate"]; + } + // Tracked non-singleton resources require the full set of lifecycle and + // list operations. For resources at resource-group scope, + // list-by-resource-group satisfies the list-by-parent requirement. + const required: RequiredOperation[] = ["read", "createOrUpdate", "delete", "list-by-parent"]; + // list-by-subscription is required only for top-level resource-group-scoped + // tracked resources (the standard Azure RP pattern). Nested tracked + // resources and tracked resources at non-RG scope (tenant, subscription, + // location) do not require a list-by-subscription. + if (isTopLevelResourceGroupScoped(resource)) { + required.push("list-by-subscription"); + } + return required; } - // Every non-singleton resource requires read, createOrUpdate, delete, and a - // list-by-parent operation. For tracked resources at resource-group scope, - // list-by-resource-group satisfies the list-by-parent requirement. - const required: RequiredOperation[] = ["read", "createOrUpdate", "delete", "list-by-parent"]; - // list-by-subscription is required only for top-level resource-group-scoped - // tracked resources (the standard Azure RP pattern). Nested resources - // (parented under another resource) and tracked resources at non-RG scope - // (tenant, subscription, location) do not require a list-by-subscription. - if (resource.kind === "Tracked" && isTopLevelResourceGroupScoped(resource)) { - required.push("list-by-subscription"); + // Non-tracked resources (Proxy / Extension) only require a read operation. + // Additionally, any resource that defines a createOrUpdate (PUT) operation + // must also define a delete operation. + const required: RequiredOperation[] = ["read"]; + if (present.has("createOrUpdate")) { + required.push("delete"); } return required; } diff --git a/packages/typespec-azure-resource-manager/test/rules/arm-resource-required-operations.test.ts b/packages/typespec-azure-resource-manager/test/rules/arm-resource-required-operations.test.ts index 56c1af4610..c092212876 100644 --- a/packages/typespec-azure-resource-manager/test/rules/arm-resource-required-operations.test.ts +++ b/packages/typespec-azure-resource-manager/test/rules/arm-resource-required-operations.test.ts @@ -172,7 +172,41 @@ it("emits missingListBySubscription for a tracked resource without a list-by-sub }); }); -it("emits missingListByParent for a nested proxy resource", async () => { +it("is valid when a nested proxy resource has only a read operation", async () => { + await tester + .expect( + ` + @armProviderNamespace + namespace Microsoft.Foo; + + model Foo is TrackedResource<{}> { + @key @path @segment("foos") fooName: string; + } + + @parentResource(Foo) + model Bar is ProxyResource<{}> { + @key @path @segment("bars") barName: string; + } + + @armResourceOperations + interface FooOperations { + read is ArmResourceRead; + createOrUpdate is ArmResourceCreateOrReplaceAsync; + delete is ArmResourceDeleteWithoutOkAsync; + listByResourceGroup is ArmResourceListByParent; + listBySubscription is ArmListBySubscription; + } + + @armResourceOperations + interface BarOperations { + read is ArmResourceRead; + } + `, + ) + .toBeValid(); +}); + +it("emits missingDelete for a proxy resource that defines createOrUpdate but no delete", async () => { await tester .expect( ` @@ -201,13 +235,49 @@ it("emits missingListByParent for a nested proxy resource", async () => { interface BarOperations { read is ArmResourceRead; createOrUpdate is ArmResourceCreateOrReplaceAsync; - delete is ArmResourceDeleteWithoutOkAsync; } `, ) .toEmitDiagnostics({ code: "@azure-tools/typespec-azure-resource-manager/arm-resource-required-operations", - message: `Resource 'Bar' must have a list-by-parent operation (list-by-resource-group satisfies this for tracked resources).`, + message: `Resource 'Bar' must have a delete operation.`, + }); +}); + +it("emits missingGet for a proxy resource missing read", async () => { + await tester + .expect( + ` + @armProviderNamespace + namespace Microsoft.Foo; + + model Foo is TrackedResource<{}> { + @key @path @segment("foos") fooName: string; + } + + @parentResource(Foo) + model Bar is ProxyResource<{}> { + @key @path @segment("bars") barName: string; + } + + @armResourceOperations + interface FooOperations { + read is ArmResourceRead; + createOrUpdate is ArmResourceCreateOrReplaceAsync; + delete is ArmResourceDeleteWithoutOkAsync; + listByResourceGroup is ArmResourceListByParent; + listBySubscription is ArmListBySubscription; + } + + @armResourceOperations + interface BarOperations { + listByParent is ArmResourceListByParent; + } + `, + ) + .toEmitDiagnostics({ + code: "@azure-tools/typespec-azure-resource-manager/arm-resource-required-operations", + message: `Resource 'Bar' must have a GET (read) operation.`, }); }); @@ -283,7 +353,27 @@ it("emits missingGet for a singleton tracked resource missing read", async () => }); }); -it("is valid when an extension resource has read, createOrUpdate, delete, and a list operation", async () => { +it("is valid when an extension resource has only a read operation", async () => { + await tester + .expect( + ` + @armProviderNamespace + namespace Microsoft.Foo; + + model Foo is ExtensionResource<{}> { + @key @path @segment("foos") name: string; + } + + @armResourceOperations + interface FooOperations { + read is ArmResourceRead; + } + `, + ) + .toBeValid(); +}); + +it("is valid when an extension resource has read, createOrUpdate, and delete", async () => { await tester .expect( ` @@ -299,7 +389,6 @@ it("is valid when an extension resource has read, createOrUpdate, delete, and a read is ArmResourceRead; createOrUpdate is ArmResourceCreateOrReplaceAsync; delete is ArmResourceDeleteWithoutOkAsync; - listByParent is ArmResourceListByParent; } `, ) 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 44aba52060..39599c8d9d 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 @@ -37,7 +37,7 @@ Available ruleSets: | [`@azure-tools/typespec-azure-resource-manager/arm-resource-operation-response`](../rules/arm-resource-operation-response.md) | [RPC 008]: PUT, GET, PATCH & LIST must return the same resource schema. | | [`@azure-tools/typespec-azure-resource-manager/arm-resource-path-segment-invalid-chars`](../rules/arm-resource-path-segment-invalid-chars.md) | Arm resource name must contain only alphanumeric characters. | | [`@azure-tools/typespec-azure-resource-manager/arm-resource-provisioning-state`](../rules/arm-resource-provisioning-state.md) | Check for properly configured provisioningState property. | -| [`@azure-tools/typespec-azure-resource-manager/arm-resource-required-operations`](../rules/arm-resource-required-operations.md) | Tracked, proxy, and extension ARM resources must define the complete set of required operations. | +| [`@azure-tools/typespec-azure-resource-manager/arm-resource-required-operations`](../rules/arm-resource-required-operations.md) | ARM resources must define their required operations: tracked resources need the full lifecycle and list set, other resources need a read, and any resource defining createOrUpdate must also define delete. | | [`@azure-tools/typespec-azure-resource-manager/version-progression`](../rules/version-progression.md) | Validate that ARM service versions all use unique dates and are declared in strictly increasing chronological order. | | [`@azure-tools/typespec-azure-resource-manager/arm-custom-resource-no-key`](../rules/arm-custom-resource-no-key.md) | Validate that custom resource contains a key property. | | [`@azure-tools/typespec-azure-resource-manager/arm-custom-resource-usage-discourage`](../rules/arm-custom-resource-usage-discourage.md) | Verify the usage of @customAzureResource decorator. | diff --git a/website/src/content/docs/docs/libraries/azure-resource-manager/rules/arm-resource-required-operations.md b/website/src/content/docs/docs/libraries/azure-resource-manager/rules/arm-resource-required-operations.md index 02707dd550..76c5362c42 100644 --- a/website/src/content/docs/docs/libraries/azure-resource-manager/rules/arm-resource-required-operations.md +++ b/website/src/content/docs/docs/libraries/azure-resource-manager/rules/arm-resource-required-operations.md @@ -6,21 +6,21 @@ title: arm-resource-required-operations @azure-tools/typespec-azure-resource-manager/arm-resource-required-operations ``` -ARM resources must declare the complete set of required lifecycle and list -operations defined by the [ARM RPC contract][rpc] (sections 2.2 and 2.3). +ARM resources must declare their required lifecycle and list operations as +defined by the [ARM RPC contract][rpc] (sections 2.2 and 2.3). The required set depends on the resource kind: -| Resource kind | Required operations | -| -------------------- | -------------------------------------------------------------------------------------------------------------------- | -| Tracked | `read`, `createOrUpdate`, `delete`, `list-by-parent` (satisfied by `list-by-resource-group`), `list-by-subscription` | -| Proxy / Extension | `read`, `createOrUpdate`, `delete`, `list-by-parent` | -| Singleton (any kind) | `read`, `createOrUpdate` only — no `delete`, no `list` | +| Resource kind | Required operations | +| ----------------- | -------------------------------------------------------------------------------------------------------------------- | +| Tracked | `read`, `createOrUpdate`, `delete`, `list-by-parent` (satisfied by `list-by-resource-group`), `list-by-subscription` | +| Tracked singleton | `read`, `createOrUpdate` only | +| Proxy / Extension | `read` (and `delete` if `createOrUpdate` is also defined) | For tracked resources, a `list-by-resource-group` operation satisfies the `list-by-parent` requirement (the resource group is the parent in that scope). -For nested proxy or extension resources, `list-by-parent` refers to a list -operation under the parent resource's path. +`list-by-subscription` is required only for top-level resource-group-scoped +tracked resources. When more than one operation is missing, a single diagnostic is emitted that lists every missing operation. When only one is missing, a more specific From 2fc6eb922601e21ad0d82b1a44704c98206cd2f0 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 13 May 2026 17:53:37 +0000 Subject: [PATCH 11/14] Drop PUT-requires-delete check from new rule; defer to no-resource-delete-operation Agent-Logs-Url: https://github.com/Azure/typespec-azure/sessions/f6f7a756-775c-47a1-bffc-8970e27398f3 Co-authored-by: markcowl <1054056+markcowl@users.noreply.github.com> --- .../rules/arm-resource-required-operations.ts | 17 +++++------------ .../arm-resource-required-operations.test.ts | 7 ++----- .../rules/arm-resource-required-operations.md | 8 ++++---- .../rules/no-resource-delete-operation.md | 6 ------ 4 files changed, 11 insertions(+), 27 deletions(-) diff --git a/packages/typespec-azure-resource-manager/src/rules/arm-resource-required-operations.ts b/packages/typespec-azure-resource-manager/src/rules/arm-resource-required-operations.ts index 9ccdbd6cb7..636053894f 100644 --- a/packages/typespec-azure-resource-manager/src/rules/arm-resource-required-operations.ts +++ b/packages/typespec-azure-resource-manager/src/rules/arm-resource-required-operations.ts @@ -94,7 +94,7 @@ function checkResource( ); const present = getPresentOperations(entries); - const required = getRequiredOperationsForResource(canonical, present); + const required = getRequiredOperationsForResource(canonical); if (required.length === 0) return; const missing = required.filter((op) => !present.has(op)); if (missing.length === 0) return; @@ -119,10 +119,7 @@ function checkResource( }); } -function getRequiredOperationsForResource( - resource: ResolvedResource, - present: Set, -): RequiredOperation[] { +function getRequiredOperationsForResource(resource: ResolvedResource): RequiredOperation[] { const isSingleton = resource.singleton !== undefined; if (resource.kind === "Tracked") { if (isSingleton) { @@ -142,13 +139,9 @@ function getRequiredOperationsForResource( return required; } // Non-tracked resources (Proxy / Extension) only require a read operation. - // Additionally, any resource that defines a createOrUpdate (PUT) operation - // must also define a delete operation. - const required: RequiredOperation[] = ["read"]; - if (present.has("createOrUpdate")) { - required.push("delete"); - } - return required; + // The "createOrUpdate without delete" condition is enforced separately by + // the `no-resource-delete-operation` rule. + return ["read"]; } function isTopLevelResourceGroupScoped(resource: ResolvedResource): boolean { diff --git a/packages/typespec-azure-resource-manager/test/rules/arm-resource-required-operations.test.ts b/packages/typespec-azure-resource-manager/test/rules/arm-resource-required-operations.test.ts index c092212876..ca9e78ad83 100644 --- a/packages/typespec-azure-resource-manager/test/rules/arm-resource-required-operations.test.ts +++ b/packages/typespec-azure-resource-manager/test/rules/arm-resource-required-operations.test.ts @@ -206,7 +206,7 @@ it("is valid when a nested proxy resource has only a read operation", async () = .toBeValid(); }); -it("emits missingDelete for a proxy resource that defines createOrUpdate but no delete", async () => { +it("is valid for a proxy resource that defines createOrUpdate but no delete (covered by no-resource-delete-operation)", async () => { await tester .expect( ` @@ -238,10 +238,7 @@ it("emits missingDelete for a proxy resource that defines createOrUpdate but no } `, ) - .toEmitDiagnostics({ - code: "@azure-tools/typespec-azure-resource-manager/arm-resource-required-operations", - message: `Resource 'Bar' must have a delete operation.`, - }); + .toBeValid(); }); it("emits missingGet for a proxy resource missing read", async () => { diff --git a/website/src/content/docs/docs/libraries/azure-resource-manager/rules/arm-resource-required-operations.md b/website/src/content/docs/docs/libraries/azure-resource-manager/rules/arm-resource-required-operations.md index 76c5362c42..a06cd2db30 100644 --- a/website/src/content/docs/docs/libraries/azure-resource-manager/rules/arm-resource-required-operations.md +++ b/website/src/content/docs/docs/libraries/azure-resource-manager/rules/arm-resource-required-operations.md @@ -15,7 +15,7 @@ The required set depends on the resource kind: | ----------------- | -------------------------------------------------------------------------------------------------------------------- | | Tracked | `read`, `createOrUpdate`, `delete`, `list-by-parent` (satisfied by `list-by-resource-group`), `list-by-subscription` | | Tracked singleton | `read`, `createOrUpdate` only | -| Proxy / Extension | `read` (and `delete` if `createOrUpdate` is also defined) | +| Proxy / Extension | `read` | For tracked resources, a `list-by-resource-group` operation satisfies the `list-by-parent` requirement (the resource group is the parent in that scope). @@ -26,9 +26,9 @@ When more than one operation is missing, a single diagnostic is emitted that lists every missing operation. When only one is missing, a more specific message ID is used so editors and tooling can present a clearer hint. -This rule is singleton-aware and supersedes -[`no-resource-delete-operation`](./no-resource-delete-operation.md). Both rules -are currently registered for backwards compatibility. +The requirement that a resource defining `createOrUpdate` must also define +`delete` is enforced by the separate +[`no-resource-delete-operation`](./no-resource-delete-operation.md) rule. #### ❌ Incorrect — tracked resource missing the delete and list operations diff --git a/website/src/content/docs/docs/libraries/azure-resource-manager/rules/no-resource-delete-operation.md b/website/src/content/docs/docs/libraries/azure-resource-manager/rules/no-resource-delete-operation.md index cbf2b19cf4..f97e564bce 100644 --- a/website/src/content/docs/docs/libraries/azure-resource-manager/rules/no-resource-delete-operation.md +++ b/website/src/content/docs/docs/libraries/azure-resource-manager/rules/no-resource-delete-operation.md @@ -8,12 +8,6 @@ title: no-resource-delete-operation Every ARM resource that provides a create operation must also provide a delete operation. -> **See also:** This rule has been superseded by -> [`arm-resource-required-operations`](./arm-resource-required-operations.md), -> which is singleton-aware and additionally enforces presence of `read`, -> `createOrUpdate`, and the appropriate `list` operations. Both rules are -> currently registered for backwards compatibility. - #### ❌ Incorrect ```tsp From b6ee6dbaf256ce338d27f67259308a0b6ff7b446 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 14 May 2026 21:04:24 +0000 Subject: [PATCH 12/14] Apply suggested rule changes: no model grouping, target interface, update description and changelog Agent-Logs-Url: https://github.com/Azure/typespec-azure/sessions/35057102-f07f-45ec-a141-adc327a4aabf Co-authored-by: markcowl <1054056+markcowl@users.noreply.github.com> --- ...rce-required-operations-2026-5-5-2-15-0.md | 2 +- .../rules/arm-resource-required-operations.ts | 141 ++++++++++-------- 2 files changed, 76 insertions(+), 67 deletions(-) diff --git a/.chronus/changes/arm-resource-required-operations-2026-5-5-2-15-0.md b/.chronus/changes/arm-resource-required-operations-2026-5-5-2-15-0.md index 6331d4f8d7..6980f256b2 100644 --- a/.chronus/changes/arm-resource-required-operations-2026-5-5-2-15-0.md +++ b/.chronus/changes/arm-resource-required-operations-2026-5-5-2-15-0.md @@ -5,4 +5,4 @@ packages: - "@azure-tools/typespec-azure-rulesets" --- -Add new `arm-resource-required-operations` linting rule that ensures every ARM resource declares the complete set of required lifecycle and list operations (singleton-aware; supersedes `no-resource-delete-operation`). +Add new `arm-resource-required-operations` linting rule that ensures every ARM resource declares the complete set of required lifecycle and list operations (singleton-aware; complements`no-resource-delete-operation`). diff --git a/packages/typespec-azure-resource-manager/src/rules/arm-resource-required-operations.ts b/packages/typespec-azure-resource-manager/src/rules/arm-resource-required-operations.ts index 636053894f..878159fd25 100644 --- a/packages/typespec-azure-resource-manager/src/rules/arm-resource-required-operations.ts +++ b/packages/typespec-azure-resource-manager/src/rules/arm-resource-required-operations.ts @@ -32,14 +32,14 @@ type RequiredOperationsMessages = { /** * Verify that an ARM resource declares the complete set of required * lifecycle and list operations for its kind. This rule is singleton-aware - * and is intended to supersede `no-resource-delete-operation`. + * and complements `no-resource-delete-operation`. */ export const armResourceRequiredOperationsRule = createRule({ name: "arm-resource-required-operations", severity: "warning", url: "https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/arm-resource-required-operations", description: - "ARM resources must define their required operations: tracked resources need the full lifecycle and list set, other resources need a read, and any resource defining createOrUpdate must also define delete.", + "ARM resources must define their required operations: tracked resources need the full lifecycle and list set, other resources need a read operation.", messages: { default: paramMessage`Resource '${"name"}' is missing required operations: [${"operations"}].`, missingListBySubscription: paramMessage`Tracked resource '${"name"}' must have a list-by-subscription operation.`, @@ -53,29 +53,33 @@ export const armResourceRequiredOperationsRule = createRule({ // Iterate every resolved ARM resource once for the program. We use // `resolveArmResources` (which handles both standard and legacy resource // operations) and read kind / singleton / operations directly from the - // returned ResolvedResource entries. - // - // A single declared resource can produce multiple ResolvedResource - // entries (e.g., when ARM operation templates emit additional paths - // for createOrUpdate under a nested resource), so entries are grouped - // by model identity and a single diagnostic is reported per logical - // resource using the union of their operations. + // returned ResolvedResource entries. A single resource model may + // represent multiple actual resources, so each ResolvedResource is + // validated individually and its diagnostic is targeted at the + // interface containing the resource's operations. root: (program: Program) => { const provider = resolveArmResources(program); - const groups = new Map(); + // Collect operation sets contributed by resolver path-doubling + // artifacts (entries where a model appears as its own ancestor), so + // they can be merged into the canonical entry's present-operation set + // without producing duplicate diagnostics for the same logical + // resource. + const extraPresentByModel = new Map>(); + for (const resource of provider.resources ?? []) { + if (!isSelfAncestor(resource)) continue; + const set = extraPresentByModel.get(resource.type) ?? new Set(); + for (const op of getPresentOperations(resource)) set.add(op); + extraPresentByModel.set(resource.type, set); + } for (const resource of provider.resources ?? []) { if (resource.kind === "Other") continue; if (isInternalTypeSpec(program, resource.type)) continue; + if (isSelfAncestor(resource)) continue; // Network Security Perimeter configurations, Private Links, and // Private Endpoint Connections have their own well-defined operation // shapes and are exempt from this rule. if (isExemptCommonTypeResource(resource.type)) continue; - const list = groups.get(resource.type); - if (list) list.push(resource); - else groups.set(resource.type, [resource]); - } - for (const entries of groups.values()) { - checkResource(context, entries); + checkResource(context, resource, extraPresentByModel.get(resource.type)); } }, }; @@ -84,24 +88,19 @@ export const armResourceRequiredOperationsRule = createRule({ function checkResource( context: LinterRuleContext, - entries: ResolvedResource[], + resource: ResolvedResource, + extraPresent: Set | undefined, ): void { - // Use the entry with the shortest `resourceType.types` as the canonical - // representation of the resource (it has the natural depth declared by the - // user). Operations are aggregated across all entries. - const canonical = entries.reduce((best, cur) => - cur.resourceType.types.length < best.resourceType.types.length ? cur : best, - ); - - const present = getPresentOperations(entries); - const required = getRequiredOperationsForResource(canonical); + const required = getRequiredOperationsForResource(resource); if (required.length === 0) return; + const present = getPresentOperations(resource); + if (extraPresent) for (const op of extraPresent) present.add(op); const missing = required.filter((op) => !present.has(op)); if (missing.length === 0) return; - const resourceInterface = getResolvedResourceInterface(entries); - const target = resourceInterface ?? canonical.type; - const name = canonical.resourceName; + const resourceInterface = getResolvedResourceInterface(resource); + const target = resourceInterface ?? resource.type; + const name = resource.resourceName; if (missing.length === 1) { context.reportDiagnostic({ @@ -150,51 +149,61 @@ function isTopLevelResourceGroupScoped(resource: ResolvedResource): boolean { return /\/resourceGroups\/\{/.test(path); } -function getPresentOperations(entries: ResolvedResource[]): Set { +function isSelfAncestor(resource: ResolvedResource): boolean { + const visited = new Set(); + let current = resource.parent; + while (current && !visited.has(current)) { + if (current.type === resource.type) return true; + visited.add(current); + current = current.parent; + } + return false; +} + +function getPresentOperations(resource: ResolvedResource): Set { const present = new Set(); - // Determine the resource kind from the canonical-equivalent: if any entry is - // tracked, the resource is tracked. (All entries for the same logical model - // share the same kind in practice.) - const isTracked = entries.some((e) => e.kind === "Tracked"); - for (const resource of entries) { - const lifecycle = resource.operations.lifecycle; - if (lifecycle.read?.length) present.add("read"); - if (lifecycle.createOrUpdate?.length) present.add("createOrUpdate"); - if (lifecycle.delete?.length) present.add("delete"); - for (const op of resource.operations.lists ?? []) { - const path = op.path ?? ""; - if (!isTracked) { - // For non-tracked resources (Proxy / Extension), any list operation - // satisfies the list-by-parent requirement regardless of scope. - present.add("list-by-parent"); - continue; - } - // Tracked resources: list-by-subscription is a list rooted at - // subscription scope (has /subscriptions/{...} but not - // /resourceGroups/{...} and no nested /providers/). Everything else - // (resource-group, location, parent, etc.) counts as list-by-parent. - const providersCount = (path.match(/\/providers\//g) ?? []).length; - const hasSubscription = /\/subscriptions\/\{/.test(path); - const hasResourceGroup = /\/resourceGroups\/\{/.test(path); - if (hasSubscription && !hasResourceGroup && providersCount <= 1) { - present.add("list-by-subscription"); - } else { - present.add("list-by-parent"); - } + const isTracked = resource.kind === "Tracked"; + const lifecycle = resource.operations.lifecycle; + if (lifecycle.read?.length) present.add("read"); + if (lifecycle.createOrUpdate?.length) present.add("createOrUpdate"); + if (lifecycle.delete?.length) present.add("delete"); + for (const op of resource.operations.lists ?? []) { + const path = op.path ?? ""; + if (!isTracked) { + // For non-tracked resources (Proxy / Extension), any list operation + // satisfies the list-by-parent requirement regardless of scope. + present.add("list-by-parent"); + continue; + } + // Tracked resources: list-by-subscription is a list rooted at + // subscription scope (has /subscriptions/{...} but not + // /resourceGroups/{...} and no nested /providers/). Everything else + // (resource-group, location, parent, etc.) counts as list-by-parent. + const providersCount = (path.match(/\/providers\//g) ?? []).length; + const hasSubscription = /\/subscriptions\/\{/.test(path); + const hasResourceGroup = /\/resourceGroups\/\{/.test(path); + if (hasSubscription && !hasResourceGroup && providersCount <= 1) { + present.add("list-by-subscription"); + } else { + present.add("list-by-parent"); } } return present; } -function getResolvedResourceInterface(entries: ResolvedResource[]): Interface | undefined { - for (const resource of entries) { - for (const ops of Object.values(resource.operations.lifecycle)) { - if (!Array.isArray(ops)) continue; - for (const op of ops) { - if (op.operation.interface) return op.operation.interface; - } +function getResolvedResourceInterface(resource: ResolvedResource): Interface | undefined { + for (const ops of Object.values(resource.operations.lifecycle)) { + if (!Array.isArray(ops)) continue; + for (const op of ops) { + if (op.operation.interface) return op.operation.interface; } } + for (const op of resource.operations.lists ?? []) { + if (op.operation.interface) return op.operation.interface; + } + for (const op of resource.operations.actions ?? []) { + if (op.operation.interface) return op.operation.interface; + } return undefined; } From aaefb2e5c6db13f823b7247a30319d373f4c1219 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 20 May 2026 00:59:16 +0000 Subject: [PATCH 13/14] Remove model-keyed extraPresent merging; key by specific canonical ancestor entry Agent-Logs-Url: https://github.com/Azure/typespec-azure/sessions/b37c44f0-bedf-4b7b-a57b-a4f7ec574919 Co-authored-by: markcowl <1054056+markcowl@users.noreply.github.com> --- .../rules/arm-resource-required-operations.ts | 47 ++++++++++------ .../arm-resource-required-operations.test.ts | 53 +++++++++++++++++++ 2 files changed, 83 insertions(+), 17 deletions(-) diff --git a/packages/typespec-azure-resource-manager/src/rules/arm-resource-required-operations.ts b/packages/typespec-azure-resource-manager/src/rules/arm-resource-required-operations.ts index 878159fd25..8b07c05d94 100644 --- a/packages/typespec-azure-resource-manager/src/rules/arm-resource-required-operations.ts +++ b/packages/typespec-azure-resource-manager/src/rules/arm-resource-required-operations.ts @@ -54,32 +54,38 @@ export const armResourceRequiredOperationsRule = createRule({ // `resolveArmResources` (which handles both standard and legacy resource // operations) and read kind / singleton / operations directly from the // returned ResolvedResource entries. A single resource model may - // represent multiple actual resources, so each ResolvedResource is - // validated individually and its diagnostic is targeted at the - // interface containing the resource's operations. + // represent multiple actual resources (for example, an extension + // resource declared at multiple scopes), so each ResolvedResource is + // validated independently with its own required-operations check, and + // its diagnostic is targeted at the interface containing the + // resource's operations. root: (program: Program) => { const provider = resolveArmResources(program); - // Collect operation sets contributed by resolver path-doubling - // artifacts (entries where a model appears as its own ancestor), so - // they can be merged into the canonical entry's present-operation set - // without producing duplicate diagnostics for the same logical - // resource. - const extraPresentByModel = new Map>(); + // Path-doubling artifacts: the resolver sometimes emits an extra + // ResolvedResource for a model whose parent chain loops back to a + // canonical entry of the same model (typically attributing the + // `createOrUpdate` operation to that artifact when the resource has + // a nested child). These are not distinct resources, so collect + // their operations and merge into the specific canonical ancestor + // entry they belong to (not by Model, which would incorrectly bleed + // operations across genuine multi-scope resources). + const extraPresent = new Map>(); for (const resource of provider.resources ?? []) { - if (!isSelfAncestor(resource)) continue; - const set = extraPresentByModel.get(resource.type) ?? new Set(); + const canonical = findSelfAncestor(resource); + if (!canonical) continue; + const set = extraPresent.get(canonical) ?? new Set(); for (const op of getPresentOperations(resource)) set.add(op); - extraPresentByModel.set(resource.type, set); + extraPresent.set(canonical, set); } for (const resource of provider.resources ?? []) { if (resource.kind === "Other") continue; if (isInternalTypeSpec(program, resource.type)) continue; - if (isSelfAncestor(resource)) continue; + if (findSelfAncestor(resource)) continue; // Network Security Perimeter configurations, Private Links, and // Private Endpoint Connections have their own well-defined operation // shapes and are exempt from this rule. if (isExemptCommonTypeResource(resource.type)) continue; - checkResource(context, resource, extraPresentByModel.get(resource.type)); + checkResource(context, resource, extraPresent.get(resource)); } }, }; @@ -149,15 +155,22 @@ function isTopLevelResourceGroupScoped(resource: ResolvedResource): boolean { return /\/resourceGroups\/\{/.test(path); } -function isSelfAncestor(resource: ResolvedResource): boolean { +/** + * Returns the closest ancestor ResolvedResource sharing the same model type + * as the given resource, or undefined if none exists. Used to detect the + * resolver's path-doubling artifacts (where the model appears as its own + * ancestor) and to merge their operations into the specific canonical + * ancestor entry they belong to. + */ +function findSelfAncestor(resource: ResolvedResource): ResolvedResource | undefined { const visited = new Set(); let current = resource.parent; while (current && !visited.has(current)) { - if (current.type === resource.type) return true; + if (current.type === resource.type) return current; visited.add(current); current = current.parent; } - return false; + return undefined; } function getPresentOperations(resource: ResolvedResource): Set { diff --git a/packages/typespec-azure-resource-manager/test/rules/arm-resource-required-operations.test.ts b/packages/typespec-azure-resource-manager/test/rules/arm-resource-required-operations.test.ts index ca9e78ad83..aeb14a8911 100644 --- a/packages/typespec-azure-resource-manager/test/rules/arm-resource-required-operations.test.ts +++ b/packages/typespec-azure-resource-manager/test/rules/arm-resource-required-operations.test.ts @@ -392,6 +392,59 @@ it("is valid when an extension resource has read, createOrUpdate, and delete", a .toBeValid(); }); +it("is valid when an extension resource declared at multiple scopes has read at every scope", async () => { + await tester + .expect( + ` + @armProviderNamespace + namespace Microsoft.Foo; + + model Employee is ExtensionResource<{}> { + @key @path @segment("employees") name: string; + } + + @armResourceOperations + interface AtSubscription { + read is Extension.Read; + } + + @armResourceOperations + interface AtTenant { + read is Extension.Read; + } + `, + ) + .toBeValid(); +}); + +it("emits missingGet for the scope that lacks read when an extension resource is declared at multiple scopes", async () => { + await tester + .expect( + ` + @armProviderNamespace + namespace Microsoft.Foo; + + model Employee is ExtensionResource<{}> { + @key @path @segment("employees") name: string; + } + + @armResourceOperations + interface AtSubscription { + read is Extension.Read; + } + + @armResourceOperations + interface AtTenant { + list is Extension.ListByTarget; + } + `, + ) + .toEmitDiagnostics({ + code: "@azure-tools/typespec-azure-resource-manager/arm-resource-required-operations", + message: `Resource 'TenantEmployee' must have a GET (read) operation.`, + }); +}); + it("skips @armVirtualResource models", async () => { await tester .expect( From d2ab95e851fb338f24aef8ecfd7b527be25eb7ff Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 27 May 2026 01:56:00 +0000 Subject: [PATCH 14/14] Suppress arm-resource-required-operations on legacy static-routes sample Co-authored-by: markcowl <1054056+markcowl@users.noreply.github.com> --- .../samples/specs/resource-manager/legacy/static-routes/main.tsp | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/samples/specs/resource-manager/legacy/static-routes/main.tsp b/packages/samples/specs/resource-manager/legacy/static-routes/main.tsp index 1ee47938c4..e4c12222e3 100644 --- a/packages/samples/specs/resource-manager/legacy/static-routes/main.tsp +++ b/packages/samples/specs/resource-manager/legacy/static-routes/main.tsp @@ -117,6 +117,7 @@ alias BaseParams2 = { }; /** Subscription operations */ +#suppress "@azure-tools/typespec-azure-resource-manager/arm-resource-required-operations" "Legacy static-routes sample uses custom operation templates not recognized by the standard required-operations check." @armResourceOperations(#{ allowStaticRoutes: true }) interface EmployeeSubscriptions { get is EmplOps.Read;