diff --git a/.chronus/changes/arm-resourceNamePatternRule-2024-2-4-21-52-56.md b/.chronus/changes/arm-resourceNamePatternRule-2024-2-4-21-52-56.md new file mode 100644 index 0000000000..644a4556d6 --- /dev/null +++ b/.chronus/changes/arm-resourceNamePatternRule-2024-2-4-21-52-56.md @@ -0,0 +1,8 @@ +--- +# Change versionKind to one of: internal, fix, dependencies, feature, deprecation, breaking +changeKind: feature +packages: + - "@azure-tools/typespec-azure-resource-manager" +--- + +ARM: add `arm-resource-name-pattern` rule to allow disabling LintDiff `ResourceNamePattern` rule diff --git a/docs/libraries/azure-resource-manager/reference/linter.md b/docs/libraries/azure-resource-manager/reference/linter.md index ab0a186fc2..ee7e4b98eb 100644 --- a/docs/libraries/azure-resource-manager/reference/linter.md +++ b/docs/libraries/azure-resource-manager/reference/linter.md @@ -36,6 +36,7 @@ Available ruleSets: | `@azure-tools/typespec-azure-resource-manager/arm-resource-invalid-envelope-property` | Check for invalid resource envelope properties. | | `@azure-tools/typespec-azure-resource-manager/arm-resource-invalid-version-format` | Check for valid versions. | | `@azure-tools/typespec-azure-resource-manager/arm-resource-key-invalid-chars` | 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` | [RPC 008]: PUT, GET, PATCH & LIST must return the same resource schema. | | `@azure-tools/typespec-azure-resource-manager/arm-resource-path-segment-invalid-chars` | Arm resource name must contain only alphanumeric characters. | | `@azure-tools/typespec-azure-resource-manager/arm-resource-provisioning-state` | Check for properly configured provisioningState property. | diff --git a/docs/libraries/azure-resource-manager/rules/resource-name-pattern.md b/docs/libraries/azure-resource-manager/rules/resource-name-pattern.md new file mode 100644 index 0000000000..fd03a68d42 --- /dev/null +++ b/docs/libraries/azure-resource-manager/rules/resource-name-pattern.md @@ -0,0 +1,32 @@ +--- +title: resource-name-pattern +--- + +```text title=- Full name- +@azure-tools/typespec-azure-resource-manager/resource-name-pattern +``` + +Resource names must specify a pattern string using `@pattern`, providing a regular expression that the name must match. + +#### ❌ Incorrect + +```tsp +model Employee is ProxyResource<{}> { + @key("employeeName") + @path + @segment("employees") + name: string; +} +``` + +#### ✅ Correct + +```tsp +model Employee is ProxyResource<{}> { + @pattern("^[a-zA-Z0-9-]{3,24}$") + @key("employeeName") + @path + @segment("employees") + name: string; +} +``` diff --git a/packages/typespec-azure-resource-manager/README.md b/packages/typespec-azure-resource-manager/README.md index 0de0314c4d..84c87b7564 100644 --- a/packages/typespec-azure-resource-manager/README.md +++ b/packages/typespec-azure-resource-manager/README.md @@ -40,6 +40,7 @@ Available ruleSets: | `@azure-tools/typespec-azure-resource-manager/arm-resource-invalid-envelope-property` | Check for invalid resource envelope properties. | | `@azure-tools/typespec-azure-resource-manager/arm-resource-invalid-version-format` | Check for valid versions. | | `@azure-tools/typespec-azure-resource-manager/arm-resource-key-invalid-chars` | Arm resource key must contain only alphanumeric characters. | +| [`@azure-tools/typespec-azure-resource-manager/arm-resource-name-pattern`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/resource-name-pattern) | The resource name parameter should be defined with a 'pattern' restriction. | | `@azure-tools/typespec-azure-resource-manager/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` | Arm resource name must contain only alphanumeric characters. | | `@azure-tools/typespec-azure-resource-manager/arm-resource-provisioning-state` | Check for properly configured provisioningState property. | diff --git a/packages/typespec-azure-resource-manager/src/linter.ts b/packages/typespec-azure-resource-manager/src/linter.ts index f9ccdb9447..ac5afaceef 100644 --- a/packages/typespec-azure-resource-manager/src/linter.ts +++ b/packages/typespec-azure-resource-manager/src/linter.ts @@ -11,6 +11,7 @@ import { invalidActionVerbRule } from "./rules/arm-resource-invalid-action-verb. import { armResourceEnvelopeProperties } from "./rules/arm-resource-invalid-envelope-property.js"; import { armResourceInvalidVersionFormatRule } from "./rules/arm-resource-invalid-version-format.js"; import { armResourceKeyInvalidCharsRule } from "./rules/arm-resource-key-invalid-chars.js"; +import { armResourceNamePatternRule } from "./rules/arm-resource-name-pattern.js"; import { armResourceOperationsRule } from "./rules/arm-resource-operation-response.js"; import { patchOperationsRule } from "./rules/arm-resource-patch.js"; import { armResourcePathInvalidCharsRule } from "./rules/arm-resource-path-invalid-chars.js"; @@ -39,6 +40,7 @@ const rules = [ armResourceEnvelopeProperties, armResourceInvalidVersionFormatRule, armResourceKeyInvalidCharsRule, + armResourceNamePatternRule, armResourceOperationsRule, armResourcePathInvalidCharsRule, armResourceProvisioningStateRule, diff --git a/packages/typespec-azure-resource-manager/src/rules/arm-delete-response-codes.ts b/packages/typespec-azure-resource-manager/src/rules/arm-delete-response-codes.ts index 5d30febc31..8edff1c6ed 100644 --- a/packages/typespec-azure-resource-manager/src/rules/arm-delete-response-codes.ts +++ b/packages/typespec-azure-resource-manager/src/rules/arm-delete-response-codes.ts @@ -1,4 +1,4 @@ -import { Model, createRule } from "@typespec/compiler"; +import { Program, createRule } from "@typespec/compiler"; import { getLroMetadata } from "@azure-tools/typespec-azure-core"; import { getArmResources } from "../resource.js"; @@ -17,25 +17,27 @@ export const armDeleteResponseCodesRule = createRule({ }, create(context) { return { - model: (model: Model) => { - const resources = getArmResources(context.program); - const armResource = resources.find((re) => re.typespecType === model); - if (armResource && armResource.operations.lifecycle.delete) { - const deleteOperation = armResource.operations.lifecycle.delete; - const isAsync = getLroMetadata(context.program, deleteOperation.operation) !== undefined; - const httpOp = deleteOperation.httpOperation; - const statusCodes = new Set([...httpOp.responses.map((r) => r.statusCodes.toString())]); - const expected = new Set(["204", "*"]); - expected.add(isAsync ? "202" : "200"); + root: (program: Program) => { + const resources = getArmResources(program); + for (const resource of resources) { + if (resource.operations.lifecycle.delete) { + const deleteOperation = resource.operations.lifecycle.delete; + const isAsync = + getLroMetadata(context.program, deleteOperation.operation) !== undefined; + const httpOp = deleteOperation.httpOperation; + const statusCodes = new Set([...httpOp.responses.map((r) => r.statusCodes.toString())]); + const expected = new Set(["204", "*"]); + expected.add(isAsync ? "202" : "200"); - if ( - statusCodes.size !== expected.size || - ![...statusCodes].every((v) => expected.has(v)) - ) { - context.reportDiagnostic({ - target: deleteOperation.operation, - messageId: isAsync ? "async" : "sync", - }); + if ( + statusCodes.size !== expected.size || + ![...statusCodes].every((v) => expected.has(v)) + ) { + context.reportDiagnostic({ + target: deleteOperation.operation, + messageId: isAsync ? "async" : "sync", + }); + } } } }, diff --git a/packages/typespec-azure-resource-manager/src/rules/arm-resource-name-pattern.ts b/packages/typespec-azure-resource-manager/src/rules/arm-resource-name-pattern.ts new file mode 100644 index 0000000000..18db55953f --- /dev/null +++ b/packages/typespec-azure-resource-manager/src/rules/arm-resource-name-pattern.ts @@ -0,0 +1,74 @@ +import { + DiagnosticTarget, + Program, + SourceLocation, + createRule, + defineCodeFix, + getPattern, + getSourceLocation, +} from "@typespec/compiler"; + +import { getArmResources } from "../resource.js"; + +// TODO: Replace this with a reusable implementation from the compiler package when implemented. +// Issue: https://github.com/microsoft/typespec/issues/3044 +function createPatternCodeFix(diagnosticTarget: DiagnosticTarget) { + return defineCodeFix({ + id: "add-pattern-decorator", + label: "Add `@pattern` decorator to the resource name property with the default ARM pattern.", + fix: (context) => { + const location = getSourceLocation(diagnosticTarget); + const { lineStart, indent } = findLineStartAndIndent(location); + const updatedLocation = { ...location, pos: lineStart }; + return context.prependText(updatedLocation, `${indent}@pattern(/^[a-zA-Z0-9-]{3,24}$/)\n`); + }, + }); +} + +function findLineStartAndIndent(location: SourceLocation): { lineStart: number; indent: string } { + const text = location.file.text; + let pos = location.pos; + let indent = 0; + while (pos > 0 && text[pos - 1] !== "\n") { + if ([" ", "\t", "\n"].includes(text[pos - 1])) { + indent++; + } else { + indent = 0; + } + pos--; + } + return { lineStart: pos, indent: location.file.text.slice(pos, pos + indent) }; +} + +/** + * Verify that a delete operation only + */ +export const armResourceNamePatternRule = createRule({ + name: "arm-resource-name-pattern", + severity: "warning", + url: "https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/resource-name-pattern", + description: "The resource name parameter should be defined with a 'pattern' restriction.", + messages: { + default: `The resource name parameter should be defined with a 'pattern' restriction. Decorate the "name" property in the resource definition using the @pattern decorator, with a regular expression indicating the allowed characters in the resource name.`, + }, + create(context) { + return { + root: (program: Program) => { + const resources = getArmResources(program); + for (const resource of resources) { + // find the name property + const nameProperty = resource.typespecType.properties.get("name"); + if (nameProperty !== undefined) { + const pattern = getPattern(program, nameProperty); + if (pattern === undefined) { + context.reportDiagnostic({ + target: nameProperty, + codefixes: [createPatternCodeFix(nameProperty)], + }); + } + } + } + }, + }; + }, +}); diff --git a/packages/typespec-azure-resource-manager/test/rules/arm-resource-name-pattern.test.ts b/packages/typespec-azure-resource-manager/test/rules/arm-resource-name-pattern.test.ts new file mode 100644 index 0000000000..a5999e3080 --- /dev/null +++ b/packages/typespec-azure-resource-manager/test/rules/arm-resource-name-pattern.test.ts @@ -0,0 +1,116 @@ +import { + BasicTestRunner, + LinterRuleTester, + createLinterRuleTester, +} from "@typespec/compiler/testing"; +import { beforeEach, it } from "vitest"; +import { armResourceNamePatternRule } from "../../src/rules/arm-resource-name-pattern.js"; +import { createAzureResourceManagerTestRunner } from "../test-host.js"; + +let runner: BasicTestRunner; +let tester: LinterRuleTester; + +beforeEach(async () => { + runner = await createAzureResourceManagerTestRunner(); + tester = createLinterRuleTester( + runner, + armResourceNamePatternRule, + "@azure-tools/typespec-azure-resource-manager" + ); +}); + +it("Emits a warning for an ARM resource that doesn't specify `@pattern` on the name", async () => { + await tester + .expect( + ` + @armProviderNamespace + @useDependency(Azure.ResourceManager.Versions.v1_0_Preview_1) + namespace Microsoft.Contoso; + + model Employee is ProxyResource<{}> { + @key("employeeName") + @path + @segment("employees") + name: string; + } + + @parentResource(Employee) + model EmployeeRole is ProxyResource<{}> { + @key("roleName") + @segment("roles") + @path + @visibility("read") + name: string; + } + ` + ) + .toEmitDiagnostics([ + { + code: "@azure-tools/typespec-azure-resource-manager/arm-resource-name-pattern", + }, + { + code: "@azure-tools/typespec-azure-resource-manager/arm-resource-name-pattern", + }, + ]); +}); + +it("Allows codefix when ARM resource name is missing pattern.", async () => { + await tester + .expect( + ` + @armProviderNamespace + @useDependency(Azure.ResourceManager.Versions.v1_0_Preview_1) + namespace Microsoft.Contoso; + + model Employee is ProxyResource<{}> { + @key("employeeName") + @path + @segment("employees") + name: string; + } + ` + ) + .applyCodeFix("add-pattern-decorator").toEqual(` + @armProviderNamespace + @useDependency(Azure.ResourceManager.Versions.v1_0_Preview_1) + namespace Microsoft.Contoso; + + model Employee is ProxyResource<{}> { + @pattern(/^[a-zA-Z0-9-]{3,24}$/) + @key("employeeName") + @path + @segment("employees") + name: string; + } + `); +}); + +it("Does not emit a warning for an ARM resource that specifies `@pattern` on the name", async () => { + await tester + .expect( + ` + @armProviderNamespace + @useDependency(Azure.ResourceManager.Versions.v1_0_Preview_1) + namespace Microsoft.Contoso; + + model Employee is ProxyResource<{}> { + @doc("Name of employee") + @pattern("^[a-zA-Z0-9-]{3,24}$") + @key("employeeName") + @path + @segment("employees") + name: string; + } + + @parentResource(Employee) + model EmployeeRole is ProxyResource<{}> { + @key("roleName") + @segment("roles") + @pattern("^[a-zA-Z0-9-]{3,24}$") + @path + @visibility("read") + name: string; + }` + ) + .toBeValid(); +});