Skip to content
Open
Show file tree
Hide file tree
Changes from all 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 new linting rule `arm-no-replace-inherited-props` that warns when a model redefines a property that is already defined in one of its base models. The 'name' property of an ARM resource and properties redefined as part of a model marked with `@discriminator` are not flagged by this rule.
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 @@ -30,6 +30,7 @@ Available ruleSets:
| ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| [`@azure-tools/typespec-azure-resource-manager/arm-no-record`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/arm-no-record) | Don't use Record types for ARM resources. |
| [`@azure-tools/typespec-azure-resource-manager/arm-no-path-casing-conflicts`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/arm-no-path-casing-conflicts) | Operation paths must be unique when compared case-insensitively. |
| [`@azure-tools/typespec-azure-resource-manager/arm-no-replace-inherited-props`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/arm-no-replace-inherited-props) | Disallow redefining properties already defined in a base type. |
| [`@azure-tools/typespec-azure-resource-manager/arm-common-types-version`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/arm-common-types-version) | Specify the ARM common-types version using @armCommonTypesVersion. |
| [`@azure-tools/typespec-azure-resource-manager/arm-delete-operation-response-codes`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/delete-operation-response-codes) | Ensure delete operations have the appropriate status codes. |
| [`@azure-tools/typespec-azure-resource-manager/arm-put-operation-response-codes`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/put-operation-response-codes) | Ensure put operations have the appropriate status codes. |
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 @@ -5,6 +5,7 @@ import { armCustomResourceUsageDiscourage } from "./rules/arm-custom-resource-us
import { armDeleteResponseCodesRule } from "./rules/arm-delete-response-codes.js";
import { armNoPathCasingConflictsRule } from "./rules/arm-no-path-casing-conflicts.js";
import { armNoRecordRule } from "./rules/arm-no-record.js";
import { armNoReplaceInheritedPropsRule } from "./rules/arm-no-replace-inherited-props.js";
import { armPostResponseCodesRule } from "./rules/arm-post-response-codes.js";
import { armPutResponseCodesRule } from "./rules/arm-put-response-codes.js";
import { armResourceActionNoSegmentRule } from "./rules/arm-resource-action-no-segment.js";
Expand Down Expand Up @@ -39,6 +40,7 @@ import { versionProgressionRule } from "./rules/version-progression.js";
const rules = [
armNoRecordRule,
armNoPathCasingConflictsRule,
armNoReplaceInheritedPropsRule,
armCommonTypesVersionRule,
armDeleteResponseCodesRule,
armPutResponseCodesRule,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,135 @@
import { getUnionAsEnum } from "@azure-tools/typespec-azure-core";
import {
createRule,
ignoreDiagnostics,
Model,
paramMessage,
Scalar,
Type,
} from "@typespec/compiler";

type ScalarFamily = "string" | "numeric" | "boolean";

/**
* Determine if `type` is "logically" a value of one of the well-known scalar
* families (string, numeric, boolean). Walks unions, enums, scalar-extends
* chains, union variants, and literals.
*
* Returns the standard scalar family name if `type` reduces to a single
* family, or `undefined` otherwise (e.g. model types, mixed unions, custom
* non-numeric/non-string scalars such as utcDateTime, etc.).
*/
function getScalarFamily(type: Type): ScalarFamily | undefined {
switch (type.kind) {
case "Scalar": {
// Walk up baseScalar chain to the root scalar.
let root: Scalar = type;
while (root.baseScalar !== undefined) {
root = root.baseScalar;
}
switch (root.name) {
case "string":
return "string";
case "boolean":
return "boolean";
case "numeric":
return "numeric";
default:
return undefined;
}
}
case "String":
case "StringTemplate":
return "string";
case "Number":
return "numeric";
case "Boolean":
return "boolean";
case "Enum": {
const families = new Set<ScalarFamily>();
for (const member of type.members.values()) {
if (typeof member.value === "number") {
families.add("numeric");
} else {
// Members with string values or no explicit value are string-typed.
families.add("string");
}
}
return families.size === 1 ? [...families][0] : undefined;
}
case "EnumMember":
return typeof type.value === "number" ? "numeric" : "string";
case "UnionVariant":
return getScalarFamily(type.type);
case "Union": {
Comment thread
markcowl marked this conversation as resolved.
// Use the azure-core helper to classify the union as a string- or
// number-typed open/closed enum.
const unionEnum = ignoreDiagnostics(getUnionAsEnum(type));
if (unionEnum === undefined) return undefined;
return unionEnum.kind === "string" ? "string" : "numeric";
}
default:
return undefined;
}
}

export const armNoReplaceInheritedPropsRule = createRule({
name: "arm-no-replace-inherited-props",
severity: "warning",
description: "Disallow redefining properties already defined in a base type.",
url: "https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/arm-no-replace-inherited-props",
messages: {
default: paramMessage`The property '${"propertyName"}' is also defined in the base model. Redefining inherited properties can cause problems with OpenAPI tooling and some language representations of the models.`,
},
create(context) {
return {
Comment thread
markcowl marked this conversation as resolved.
model: (model: Model) => {
if (model.baseModel === undefined) return;

for (const property of model.properties.values()) {
// Walk the chain of base models looking for a property with the
// same name.
let inherited: typeof property | undefined;
let current: Model | undefined = model.baseModel;
while (current !== undefined) {
const found = current.properties.get(property.name);
if (found !== undefined) {
inherited = found;
break;
}
current = current.baseModel;
}

if (inherited === undefined) continue;

// Allow overriding with the exact same type (identity). Aliases in
// TypeSpec resolve to the same Type instance, and template
// instantiations are cached, so this naturally handles cases such
// as redefining `systemData: SystemData` (alias) where the parent
// uses `systemData: Foundations.SystemData`, or `tags: Record<string>`
// cloned via `model X is TrackedResource<...>`.
if (property.type === inherited.type) continue;

// Allow overriding when both the inherited property and the override
// resolve to the same scalar family (e.g. both are "string"
// -- including string scalars, string-derived scalars, string
// literals, string enums, and open or closed string unions).
const overrideFamily = getScalarFamily(property.type);
const inheritedFamily = getScalarFamily(inherited.type);
if (
overrideFamily !== undefined &&
inheritedFamily !== undefined &&
overrideFamily === inheritedFamily
) {
continue;
}

context.reportDiagnostic({
format: { propertyName: property.name },
target: property,
});
}
},
};
},
});
Loading
Loading