diff --git a/.chronus/changes/promote-guid-usage-2026-08-28.md b/.chronus/changes/promote-guid-usage-2026-08-28.md new file mode 100644 index 0000000000..29f1e215a9 --- /dev/null +++ b/.chronus/changes/promote-guid-usage-2026-08-28.md @@ -0,0 +1,7 @@ +--- +changeKind: feature +packages: + - "@azure-tools/typespec-azure-resource-manager" +--- + +Add the `no-uuid` ARM lint rule, migrated from the Swagger `GuidUsage` validator rule. diff --git a/.chronus/changes/register-no-uuid-ruleset.md b/.chronus/changes/register-no-uuid-ruleset.md new file mode 100644 index 0000000000..5fcda6743c --- /dev/null +++ b/.chronus/changes/register-no-uuid-ruleset.md @@ -0,0 +1,7 @@ +--- +changeKind: internal +packages: + - "@azure-tools/typespec-azure-rulesets" +--- + +Register the ARM `no-uuid` lint rule as disabled in the resource manager ruleset. diff --git a/packages/typespec-azure-resource-manager/README.md b/packages/typespec-azure-resource-manager/README.md index 7db74ab137..3be49d0830 100644 --- a/packages/typespec-azure-resource-manager/README.md +++ b/packages/typespec-azure-resource-manager/README.md @@ -65,6 +65,7 @@ Available ruleSets: | [`@azure-tools/typespec-azure-resource-manager/lro-location-header`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/lro-location-header) | A 202 response should include a Location response header. | | [`@azure-tools/typespec-azure-resource-manager/missing-x-ms-identifiers`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/missing-x-ms-identifiers) | 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`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/no-response-body) | 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/no-uuid`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/no-uuid) | ARM APIs should avoid UUID-typed schemas unless they have explicit Azure API review approval. | | [`@azure-tools/typespec-azure-resource-manager/missing-operations-endpoint`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/missing-operations-endpoint) | Check for missing Operations interface. | | [`@azure-tools/typespec-azure-resource-manager/patch-envelope`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/patch-envelope) | Patch envelope properties should match the resource properties. | | [`@azure-tools/typespec-azure-resource-manager/arm-resource-patch`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/arm-resource-patch) | Validate ARM PATCH operations. | diff --git a/packages/typespec-azure-resource-manager/src/linter.ts b/packages/typespec-azure-resource-manager/src/linter.ts index 2e5ed94ff2..c5a2653438 100644 --- a/packages/typespec-azure-resource-manager/src/linter.ts +++ b/packages/typespec-azure-resource-manager/src/linter.ts @@ -34,6 +34,7 @@ import { noQueryInPointOpRule } from "./rules/no-query-in-point-op.js"; import { noReservedResourcePropertyRule } from "./rules/no-reserved-resource-property.js"; import { deleteOperationMissingRule } from "./rules/no-resource-delete-operation.js"; import { noResponseBodyRule } from "./rules/no-response-body.js"; +import { noUuidRule } from "./rules/no-uuid.js"; import { operationsInterfaceMissingRule } from "./rules/operations-interface-missing.js"; import { patchEnvelopePropertiesRules } from "./rules/patch-envelope-properties.js"; import { resourceNameRule } from "./rules/resource-name.js"; @@ -85,6 +86,7 @@ const rules = [ lroLocationHeaderRule, missingXmsIdentifiersRule, noResponseBodyRule, + noUuidRule, operationsInterfaceMissingRule, patchEnvelopePropertiesRules, patchOperationsRule, diff --git a/packages/typespec-azure-resource-manager/src/rules/no-uuid.md b/packages/typespec-azure-resource-manager/src/rules/no-uuid.md new file mode 100644 index 0000000000..cb40046b8f --- /dev/null +++ b/packages/typespec-azure-resource-manager/src/rules/no-uuid.md @@ -0,0 +1,37 @@ +Avoid UUID-typed schemas in Azure Resource Manager APIs unless their use has explicit Azure API review approval. + +## Impact + +- **Area:** API, SDK + +UUIDs are difficult for customers to create, recognize, and troubleshoot. Prefer stable, human-readable identifiers that follow the resource's naming constraints. UUID wire types also become language-specific UUID types in generated SDKs, which can make an API harder to use consistently across languages. + +The rule checks UUID model properties, HTTP parameters, request and response bodies, response headers, custom scalar aliases, and container types. It also checks UUID formats applied directly with `@format("uuid")`. + +## Incorrect + +```tsp +@armProviderNamespace +namespace Microsoft.Contoso; + +model WidgetProperties { + id: Azure.Core.uuid; +} +``` + +## Correct + +```tsp +@armProviderNamespace +namespace Microsoft.Contoso; + +model WidgetProperties { + id: string; +} +``` + +If a UUID is required, obtain Azure API review approval and suppress the rule at the authored declaration with the approval context. + +## LintDiff Equivalent + +This rule corresponds to the Swagger validator rule [GuidUsage](https://github.com/Azure/azure-openapi-validator/blob/6243cb01c16c7535cd3b8df6f45fbeb3c095ed7f/docs/guid-usage.md). diff --git a/packages/typespec-azure-resource-manager/src/rules/no-uuid.ts b/packages/typespec-azure-resource-manager/src/rules/no-uuid.ts new file mode 100644 index 0000000000..1452e8cd79 --- /dev/null +++ b/packages/typespec-azure-resource-manager/src/rules/no-uuid.ts @@ -0,0 +1,190 @@ +import { + type ArrayModelType, + type Model, + type ModelProperty, + type Namespace, + type Operation, + type Program, + type RecordModelType, + type Scalar, + type Type, + createRule, + fileRef, + getFormat, + getLocationContext, + isArrayModelType, + isRecordModelType, +} from "@typespec/compiler"; +import { $ } from "@typespec/compiler/typekit"; +import { getAllHttpServices } from "@typespec/http"; + +import { getArmProviderNamespace } from "../namespace.js"; +import { getArmResources } from "../resource.js"; + +export const noUuidRule = createRule({ + name: "no-uuid", + docs: fileRef.fromPackageRoot("src/rules/no-uuid.md"), + description: + "ARM APIs should avoid UUID-typed schemas unless they have explicit Azure API review approval.", + severity: "warning", + url: "https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/no-uuid", + messages: { + default: + "UUID usage is not recommended. If UUIDs are required in your service, get sign-off from the Azure API review board.", + }, + create(context) { + const reportedTargets = new Set(); + const uuidScalar = $(context.program).type.resolve("Azure.Core.uuid", "Scalar"); + const [services] = getAllHttpServices(context.program); + const armServices = services.filter((service) => + getArmProviderNamespace(context.program, service.namespace), + ); + const resourceKeyByOperation = getResourceKeyByOperation(context.program); + + return { + modelProperty: (property) => { + if (!isInArmService(property.model?.namespace, armServices)) { + return; + } + + if ( + getFormat(context.program, property) === "uuid" || + containsUuid(context.program, uuidScalar, property.type) + ) { + reportTarget(context, property, reportedTargets); + } + }, + operation: (operation) => { + const namespace = operation.interface?.namespace ?? operation.namespace; + if ( + isInArmService(namespace, armServices) && + containsUuid(context.program, uuidScalar, operation.returnType) + ) { + reportTarget(context, operation, reportedTargets); + } + }, + root: () => { + for (const service of armServices) { + for (const httpOperation of service.operations) { + const operation = httpOperation.operation; + const resourceKey = resourceKeyByOperation.get(operation); + if (resourceKey === undefined) { + continue; + } + + const parameter = httpOperation.parameters.parameters.find( + (parameter) => parameter.param.name === resourceKey, + ); + if ( + parameter !== undefined && + (getFormat(context.program, parameter.param) === "uuid" || + containsUuid(context.program, uuidScalar, parameter.param.type)) + ) { + reportTarget(context, operation, reportedTargets); + } + } + } + }, + }; + }, +}); + +function getResourceKeyByOperation(program: Program): Map { + const result = new Map(); + for (const resource of getArmResources(program)) { + if (resource.keyName === undefined) { + continue; + } + + const operations = [ + ...Object.values(resource.operations.lifecycle), + ...Object.values(resource.operations.lists), + ...Object.values(resource.operations.actions), + ]; + for (const operation of operations) { + if (operation !== undefined) { + result.set(operation.operation, resource.keyName); + } + } + } + return result; +} + +function isWithinNamespace(namespace: Namespace, ancestor: Namespace): boolean { + for (let current: Namespace | undefined = namespace; current; current = current.namespace) { + if (current === ancestor) { + return true; + } + } + return false; +} + +function isInArmService( + namespace: Namespace | undefined, + services: readonly { namespace: Namespace }[], +): boolean { + return ( + namespace !== undefined && + services.some((service) => isWithinNamespace(namespace, service.namespace)) + ); +} + +function containsUuid( + program: Program, + uuidScalar: Scalar | undefined, + type: Type, + seen = new Set(), +): boolean { + if (seen.has(type)) { + return false; + } + + seen.add(type); + + switch (type.kind) { + case "Scalar": + return ( + type === uuidScalar || + getFormat(program, type) === "uuid" || + (type.baseScalar !== undefined && containsUuid(program, uuidScalar, type.baseScalar, seen)) + ); + case "Model": + if (isContainerModel(type)) { + return containsUuid(program, uuidScalar, type.indexer.value, seen); + } + if (getLocationContext(program, type).type === "project") { + return false; + } + // Project model properties are visited by the linter. Recurse only through library wrappers + // such as ArmResponse, whose instantiated payload property cannot be reported directly. + return [...type.properties.values()].some( + (property) => + getLocationContext(program, property).type !== "project" && + (getFormat(program, property) === "uuid" || + containsUuid(program, uuidScalar, property.type, new Set(seen))), + ); + case "Tuple": + return type.values.some((value) => containsUuid(program, uuidScalar, value, new Set(seen))); + case "Union": + return [...type.variants.values()].some((variant) => + containsUuid(program, uuidScalar, variant.type, new Set(seen)), + ); + default: + return false; + } +} + +function reportTarget( + context: Parameters[0], + target: ModelProperty | Operation, + reportedTargets: Set, +): void { + if (!reportedTargets.has(target)) { + reportedTargets.add(target); + context.reportDiagnostic({ target }); + } +} + +function isContainerModel(model: Model): model is ArrayModelType | RecordModelType { + return isArrayModelType(model) || isRecordModelType(model); +} diff --git a/packages/typespec-azure-resource-manager/test/rules/no-uuid.test.ts b/packages/typespec-azure-resource-manager/test/rules/no-uuid.test.ts new file mode 100644 index 0000000000..e8c07ed46a --- /dev/null +++ b/packages/typespec-azure-resource-manager/test/rules/no-uuid.test.ts @@ -0,0 +1,294 @@ +import { Tester } from "#test/tester.js"; +import { + type LinterRuleTester, + type TesterInstance, + createLinterRuleTester, +} from "@typespec/compiler/testing"; +import { beforeEach, it } from "vitest"; + +import { noUuidRule } from "../../src/rules/no-uuid.js"; + +let tester: LinterRuleTester; + +beforeEach(async () => { + const runner: TesterInstance = await Tester.createInstance(); + tester = createLinterRuleTester( + runner, + noUuidRule, + "@azure-tools/typespec-azure-resource-manager", + ); +}); + +function inArmService(code: string): string { + return ` + @armProviderNamespace + @service(#{ title: "Test service" }) + namespace Microsoft.Test { + ${code} + } + `; +} + +function versionedArmService(resourceName: string): string { + return ` + @armProviderNamespace + @service(#{ title: "Test service" }) + @versioned(Versions) + @armCommonTypesVersion(Azure.ResourceManager.CommonTypes.Versions.v6) + namespace Microsoft.Test; + + enum Versions { + @useDependency(Azure.ResourceManager.CommonTypes.Versions.v6) + v2024_01_01: "2024-01-01", + } + + model Widget is TrackedResource { + ...ResourceNameParameter< + Resource = Widget, + KeyName = "widgetName", + SegmentName = "widgets", + ${resourceName} + NamePattern = "" + >; + } + + model WidgetProperties { + @visibility(Lifecycle.Read) + provisioningState?: ResourceProvisioningState; + } + + interface Operations extends Azure.ResourceManager.Operations {} + + @armResourceOperations + interface Widgets { + get is ArmResourceRead; + createOrUpdate is ArmResourceCreateOrReplaceAsync; + update is ArmResourcePatchAsync; + delete is ArmResourceDeleteWithoutOkAsync; + listByResourceGroup is ArmResourceListByParent; + } + `; +} + +const diagnostic = { + code: "@azure-tools/typespec-azure-resource-manager/no-uuid", +}; + +it("reports a UUID-typed model property", async () => { + await tester + .expect( + inArmService(` + model WidgetProperties { + id: Azure.Core.uuid; + } + `), + ) + .toEmitDiagnostics(diagnostic); +}); + +it("reports a UUID-typed query parameter", async () => { + await tester + .expect( + inArmService(` + @route("/widgets") + interface Widgets { + @get read(@query requestId: Azure.Core.uuid): string; + } + `), + ) + .toEmitDiagnostics(diagnostic); +}); + +it("reports a UUID-typed resource name template parameter", async () => { + await tester + .expect(versionedArmService("Type = Azure.Core.uuid,")) + .toEmitDiagnostics([diagnostic, diagnostic, diagnostic, diagnostic]); +}); + +it("reports a UUID-formatted resource name template parameter", async () => { + await tester + .expect( + `${versionedArmService("")} + @@format(Widget.name, "uuid"); + `, + ) + .toEmitDiagnostics([diagnostic, diagnostic, diagnostic, diagnostic]); +}); + +it("reports a direct UUID request body", async () => { + await tester + .expect( + inArmService(` + @route("/widgets") + interface Widgets { + @post create(@body body: Azure.Core.uuid): string; + } + `), + ) + .toEmitDiagnostics(diagnostic); +}); + +it("reports a direct UUID response body", async () => { + await tester + .expect( + inArmService(` + @route("/widgets") + interface Widgets { + @get read(): Azure.Core.uuid; + } + `), + ) + .toEmitDiagnostics(diagnostic); +}); + +it("reports a UUID response body through an ARM response template", async () => { + await tester + .expect( + inArmService(` + @route("/widgets") + interface Widgets { + @get read(): ArmResponse; + } + `), + ) + .toEmitDiagnostics(diagnostic); +}); + +it("reports a custom scalar derived from UUID", async () => { + await tester + .expect( + inArmService(` + scalar WidgetId extends Azure.Core.uuid; + + model WidgetProperties { + id: WidgetId; + } + `), + ) + .toEmitDiagnostics(diagnostic); +}); + +it("reports a custom scalar with an explicit UUID format", async () => { + await tester + .expect( + inArmService(` + @format("uuid") + scalar WidgetId extends string; + + model WidgetProperties { + id: WidgetId; + } + `), + ) + .toEmitDiagnostics(diagnostic); +}); + +it("reports a property-level UUID format", async () => { + await tester + .expect( + inArmService(` + model IdentifierProperties { + id: string; + } + + model WidgetProperties { + ...IdentifierProperties; + } + + @@format(WidgetProperties.id, "uuid"); + `), + ) + .toEmitDiagnostics(diagnostic); +}); + +it("reports an array property containing UUID values", async () => { + await tester + .expect( + inArmService(` + model WidgetProperties { + relatedIds: Azure.Core.uuid[]; + } + `), + ) + .toEmitDiagnostics(diagnostic); +}); + +it("allows non-UUID shapes and UUIDs in client-only namespaces", async () => { + await tester + .expect( + ` + ${inArmService(` + model WidgetProperties { + id: string; + relatedIds: string[]; + } + `)} + + namespace Azure.ResourceManager.Test.Models { + model ClientOnlyModel { + id: Azure.Core.uuid; + } + } + `, + ) + .toBeValid(); +}); + +it("reports a UUID property on an unreferenced named model", async () => { + await tester + .expect( + inArmService(` + model UnreferencedModel { + id: Azure.Core.uuid; + } + `), + ) + .toEmitDiagnostics(diagnostic); +}); + +it("reports UUID values nested in records, tuples, and unions", async () => { + await tester + .expect( + inArmService(` + model WidgetProperties { + recordIds: Record; + tupleIds: [Azure.Core.uuid]; + unionId: Azure.Core.uuid | string; + } + `), + ) + .toEmitDiagnostics([diagnostic, diagnostic, diagnostic]); +}); + +it("reports a UUID-typed response header", async () => { + await tester + .expect( + inArmService(` + @route("/widgets") + interface Widgets { + @get read(): { + @header requestId: Azure.Core.uuid; + @body body: string; + }; + } + `), + ) + .toEmitDiagnostics(diagnostic); +}); + +it("deduplicates a property reached through declarations and HTTP payloads", async () => { + await tester + .expect( + inArmService(` + model Widget { + id: Azure.Core.uuid; + } + + @route("/widgets") + interface Widgets { + @get read(): Widget; + } + `), + ) + .toEmitDiagnostics(diagnostic); +}); diff --git a/packages/typespec-azure-rulesets/src/rulesets/resource-manager.ts b/packages/typespec-azure-rulesets/src/rulesets/resource-manager.ts index ce215cf72b..89f00aea39 100644 --- a/packages/typespec-azure-rulesets/src/rulesets/resource-manager.ts +++ b/packages/typespec-azure-rulesets/src/rulesets/resource-manager.ts @@ -98,6 +98,7 @@ export default { "@azure-tools/typespec-azure-resource-manager/lro-location-header": true, "@azure-tools/typespec-azure-resource-manager/missing-x-ms-identifiers": true, "@azure-tools/typespec-azure-resource-manager/no-response-body": true, + "@azure-tools/typespec-azure-resource-manager/no-uuid": false, "@azure-tools/typespec-azure-resource-manager/missing-operations-endpoint": true, "@azure-tools/typespec-azure-resource-manager/patch-envelope": true, "@azure-tools/typespec-azure-resource-manager/arm-resource-patch": 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 cf666f029a..6e6519522b 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 @@ -59,6 +59,7 @@ Available ruleSets: | [`@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/no-uuid`](../rules/no-uuid.md) | ARM APIs should avoid UUID-typed schemas unless they have explicit Azure API review approval. | | [`@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. |