Skip to content
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
changeKind: feature
packages:
- "@azure-tools/typespec-azure-resource-manager"
- "@azure-tools/typespec-azure-rulesets"
---

Add an ARM linter rule that reports required, default-valued, and create-only properties emitted in PATCH request bodies.
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 @@ -64,6 +64,7 @@ Available ruleSets:
| [`@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/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-body-invalid-property`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/patch-body-invalid-property) | ARM PATCH body properties must not be required, have defaults, or be create-only. |
| [`@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. |
| [`@azure-tools/typespec-azure-resource-manager/resource-name`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/resource-name) | Check the resource name. |
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 @@ -33,6 +33,7 @@ import { noReservedResourcePropertyRule } from "./rules/no-reserved-resource-pro
import { deleteOperationMissingRule } from "./rules/no-resource-delete-operation.js";
import { noResponseBodyRule } from "./rules/no-response-body.js";
import { operationsInterfaceMissingRule } from "./rules/operations-interface-missing.js";
import { patchBodyInvalidPropertyRule } from "./rules/patch-body-invalid-property.js";
import { patchEnvelopePropertiesRules } from "./rules/patch-envelope-properties.js";
import { resourceNameRule } from "./rules/resource-name.js";
import { retryAfterRule } from "./rules/retry-after.js";
Expand Down Expand Up @@ -81,6 +82,7 @@ const rules = [
missingXmsIdentifiersRule,
noResponseBodyRule,
operationsInterfaceMissingRule,
patchBodyInvalidPropertyRule,
patchEnvelopePropertiesRules,
patchOperationsRule,
resourceNameRule,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
ARM PATCH request body properties must be update-safe. A property emitted in an ARM PATCH body must not be required, must not define a default value, and must not be create-only.

## Impact

- **Area:** API

PATCH describes partial updates. Required PATCH body properties, default-valued properties, and create-only properties can make partial updates ambiguous for service authors and SDKs, and can produce ARM OpenAPI that violates PATCH request-body guidance.

The rule checks the effective emitted PATCH payload. Properties omitted from the PATCH payload, such as `never` properties or create-only properties removed by the PATCH visibility transform, are not reported. A top-level emitted property named `identity` is skipped to match ARM PATCH identity envelope behavior.

## ❌ Incorrect

```tsp
@armProviderNamespace
namespace Microsoft.Contoso;

model WidgetPatchBody {
displayName: string;
enabled?: boolean = false;

@visibility(Lifecycle.Create)
createdBy?: string;
}

@route("/widgets/{name}")
@patch
op update(@path name: string, @body body: WidgetPatchBody): void;
```

## ✅ Correct

```tsp
@armProviderNamespace
namespace Microsoft.Contoso;

model WidgetPatchBody {
displayName?: string;
enabled?: boolean;
}

@route("/widgets/{name}")
@patch
op update(@path name: string, @body body: WidgetPatchBody): void;
```

## LintDiff Equivalent

This rule corresponds to the LintDiff rule [PatchBodyParametersSchema](https://github.com/Azure/azure-openapi-validator/blob/main/docs/patch-body-parameters-schema.md).

## Suppression

Do not suppress this rule for ordinary ARM resource PATCH operations. Fix the PATCH model so updateable properties are optional, do not carry defaults, and exclude create-only properties from the emitted PATCH payload.
Original file line number Diff line number Diff line change
@@ -0,0 +1,296 @@
import {
createRule,
fileRef,
getDiscriminator,
getLifecycleVisibilityEnum,
getLocationContext,
getVisibilityForClass,
isNeverType,
isNullType,
paramMessage,
resolveEncodedName,
type DiagnosticTarget,
type Model,
type ModelProperty,
type Operation,
type Program,
type Type,
} from "@typespec/compiler";
import {
createMetadataInfo,
getHttpOperation,
resolveRequestVisibility,
Visibility,
type MetadataInfo,
} from "@typespec/http";
import { resolveProviderNamespace } from "../namespace.js";

export const patchBodyInvalidPropertyRule = createRule({
name: "patch-body-invalid-property",
Comment thread
msyyc marked this conversation as resolved.
Outdated
docs: fileRef.fromPackageRoot("src/rules/patch-body-invalid-property.md"),
severity: "warning",
description: "ARM PATCH body properties must not be required, have defaults, or be create-only.",
url: "https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/patch-body-invalid-property",
messages: {
required: paramMessage`Properties of a PATCH request body must not be required, property:${"propertyName"}.`,
default: paramMessage`Properties of a PATCH request body must not have default value, property:${"propertyName"}.`,
createOnly: paramMessage`Properties of a PATCH request body must not be x-ms-mutability: ["create"], property:${"propertyName"}.`,
},
create(context) {
return {
operation: (operation) => {
const namespace = operation.interface?.namespace ?? operation.namespace;
if (resolveProviderNamespace(context.program, namespace) === undefined) {
return;
}

const [httpOperation] = getHttpOperation(context.program, operation);
if (httpOperation.verb !== "patch") {
return;
}

const patchBody = httpOperation.parameters.body?.type;
if (patchBody === undefined) {
return;
}

for (const violation of findViolations(context.program, patchBody, operation)) {
context.reportDiagnostic({
target: violation.target,
messageId: violation.messageId,
format: {
propertyName: violation.propertyName,
},
});
}
},
};
},
});

type Violation = {
target: DiagnosticTarget;
propertyName: string;
messageId: "required" | "default" | "createOnly";
};

function findViolations(program: Program, patchBody: Type, operation: Operation): Violation[] {
const violations: Violation[] = [];
const metadataInfo = createMetadataInfo(program, {
canonicalVisibility: Visibility.Read,
canShareProperty: (property) => canSharePropertyUsingReadonlyOrXmsMutability(program, property),
});
const visibility = resolveRequestVisibility(program, operation, "patch");
collectNestedViolations(
program,
patchBody,
violations,
[],
new Map(),
operation,
metadataInfo,
visibility,
);
return violations;
}

function collectViolations(
program: Program,
model: Model,
violations: Violation[],
path: string[] = [],
visited: Map<Model, Set<Visibility>> = new Map(),
diagnosticTarget: DiagnosticTarget,
metadataInfo: MetadataInfo,
visibility: Visibility,
) {
const schemaVisibility = metadataInfo.isTransformed(model, visibility)
? visibility
: Visibility.Read;
const visitedVisibilities = visited.get(model);
if (visitedVisibilities?.has(schemaVisibility)) {
return;
}
if (visitedVisibilities === undefined) {
visited.set(model, new Set([schemaVisibility]));
} else {
visitedVisibilities.add(schemaVisibility);
}

const discriminator = getInheritedDiscriminator(program, model);
if (
discriminator !== undefined &&
getModelProperty(model, discriminator.propertyName) === undefined &&
!isTopLevelIdentityProperty([...path, discriminator.propertyName], discriminator.propertyName)
) {
violations.push({
target: getLocationContext(program, model).type === "project" ? model : diagnosticTarget,
propertyName: [...path, discriminator.propertyName].join("."),
messageId: "required",
});
}

for (const property of getModelProperties(model)) {
const jsonName = resolveEncodedName(program, property, "application/json");
const propertyPath = [...path, jsonName];
if (isTopLevelIdentityProperty(propertyPath, jsonName)) {
continue;
}
if (!metadataInfo.isPayloadProperty(property, schemaVisibility)) {
continue;
}
if (isNeverType(property.type)) {
continue;
}
const propertyTarget =
getLocationContext(program, property).type === "project" ? property : diagnosticTarget;

if (
!metadataInfo.isOptional(property, schemaVisibility) ||
property.name === discriminator?.propertyName
) {
violations.push({
target: propertyTarget,
propertyName: propertyPath.join("."),
messageId: "required",
});
}

if (property.defaultValue !== undefined) {
violations.push({
target: propertyTarget,
propertyName: propertyPath.join("."),
messageId: "default",
});
}

if (isCreateOnlyMutability(program, property)) {
violations.push({
target: propertyTarget,
propertyName: propertyPath.join("."),
messageId: "createOnly",
});
}

collectNestedViolations(
program,
property.type,
violations,
propertyPath,
visited,
propertyTarget,
metadataInfo,
schemaVisibility,
);
}
}

function collectNestedViolations(
program: Program,
type: Type,
violations: Violation[],
path: string[],
visited: Map<Model, Set<Visibility>>,
diagnosticTarget: DiagnosticTarget,
metadataInfo: MetadataInfo,
visibility: Visibility,
) {
if (type.kind === "Model") {
collectViolations(
program,
type,
violations,
path,
visited,
diagnosticTarget,
metadataInfo,
visibility,
);
return;
}

if (type.kind === "Union") {
const nonNullVariants = [...type.variants.values()]
.map((variant) => variant.type)
.filter((variant) => !isNullType(variant));
if (nonNullVariants.length === 1) {
collectNestedViolations(
program,
nonNullVariants[0],
violations,
path,
visited,
diagnosticTarget,
metadataInfo,
visibility,
);
}
}
}

function isTopLevelIdentityProperty(propertyPath: string[], jsonName: string): boolean {
return propertyPath.length === 1 && jsonName.toLowerCase() === "identity";
}

function isCreateOnlyMutability(program: Program, property: ModelProperty): boolean {
const lifecycle = getLifecycleVisibilityEnum(program);
const create = lifecycle.members.get("Create");
if (create === undefined) {
return false;
}

const visibility = getVisibilityForClass(program, property, lifecycle);
return visibility.size === 1 && visibility.has(create);
}

function getInheritedDiscriminator(program: Program, model: Model) {
for (let current: Model | undefined = model; current !== undefined; current = current.baseModel) {
const discriminator = getDiscriminator(program, current);
if (discriminator !== undefined) {
return discriminator;
}
}

return undefined;
}

function canSharePropertyUsingReadonlyOrXmsMutability(
program: Program,
property: ModelProperty,
): boolean {
const lifecycle = getLifecycleVisibilityEnum(program);
const visibility = getVisibilityForClass(program, property, lifecycle);
if (visibility.size === lifecycle.members.size) {
return true;
}

return (
visibility.size > 0 &&
[...visibility].every((member) => ["Read", "Create", "Update"].includes(member.name))
);
}

function getModelProperty(model: Model, name: string): ModelProperty | undefined {
for (let current: Model | undefined = model; current !== undefined; current = current.baseModel) {
const property = current.properties.get(name);
if (property !== undefined) {
return property;
}
}

return undefined;
}

function getModelProperties(model: Model): ModelProperty[] {
const properties = new Map<string, ModelProperty>();

for (let current: Model | undefined = model; current !== undefined; current = current.baseModel) {
for (const property of current.properties.values()) {
if (!properties.has(property.name)) {
properties.set(property.name, property);
}
}
}

return [...properties.values()];
}
Loading
Loading