Skip to content

Commit

Permalink
ARM: add arm-resource-name-pattern rule to allow disabling LintDiff…
Browse files Browse the repository at this point in the history
… `ResourceNamePattern` rule (#359)

Closes Azure/typespec-azure-pr#3903

**REST API Specs** (6 violations)
Azure/azure-rest-api-specs#28085

**OpenAPI Validator PR**
Azure/azure-openapi-validator#669

---------

Co-authored-by: Mark Cowlishaw <[email protected]>
  • Loading branch information
tjprescott and markcowl authored Mar 28, 2024
1 parent 909ed30 commit 8d9d99a
Show file tree
Hide file tree
Showing 8 changed files with 255 additions and 19 deletions.
Original file line number Diff line number Diff line change
@@ -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
1 change: 1 addition & 0 deletions docs/libraries/azure-resource-manager/reference/linter.md
Original file line number Diff line number Diff line change
Expand Up @@ -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. |
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
}
```
1 change: 1 addition & 0 deletions packages/typespec-azure-resource-manager/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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. |
Expand Down
2 changes: 2 additions & 0 deletions packages/typespec-azure-resource-manager/src/linter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -39,6 +40,7 @@ const rules = [
armResourceEnvelopeProperties,
armResourceInvalidVersionFormatRule,
armResourceKeyInvalidCharsRule,
armResourceNamePatternRule,
armResourceOperationsRule,
armResourcePathInvalidCharsRule,
armResourceProvisioningStateRule,
Expand Down
Original file line number Diff line number Diff line change
@@ -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";
Expand All @@ -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",
});
}
}
}
},
Expand Down
Original file line number Diff line number Diff line change
@@ -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)],
});
}
}
}
},
};
},
});
Original file line number Diff line number Diff line change
@@ -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();
});

0 comments on commit 8d9d99a

Please sign in to comment.