Skip to content

Commit

Permalink
Add rule for RPC-Policy-V1-03: no properties of type object without a…
Browse files Browse the repository at this point in the history
… schema (#1555)

Issue: #1426

---------

Co-authored-by: Timothee Guerin <timothee.guerin@outlook.com>
  • Loading branch information
AlitzelMendez and timotheeguerin authored Oct 4, 2024
1 parent 2e65ae9 commit 22aa5ee
Show file tree
Hide file tree
Showing 9 changed files with 195 additions and 0 deletions.
7 changes: 7 additions & 0 deletions .chronus/changes/almend-Rule-2024-8-17-16-58-23.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
changeKind: feature
packages:
- "@azure-tools/typespec-azure-resource-manager"
---

Add `no-empty-model` rule
7 changes: 7 additions & 0 deletions .chronus/changes/almend-Rule-2024-8-18-10-2-28.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
changeKind: feature
packages:
- "@azure-tools/typespec-azure-rulesets"
---

Add `no-empty-model` rule to ruleset
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 @@ -56,3 +56,4 @@ Available ruleSets:
| `@azure-tools/typespec-azure-resource-manager/resource-name` | Check the resource name. |
| `@azure-tools/typespec-azure-resource-manager/retry-after` | Check if retry-after header appears in response body. |
| [`@azure-tools/typespec-azure-resource-manager/unsupported-type`](/libraries/azure-resource-manager/rules/unsupported-type.md) | Check for unsupported ARM types. |
| [`@azure-tools/typespec-azure-resource-manager/no-empty-model`](/libraries/azure-resource-manager/rules/no-empty-model.md) | ARM Properties with type:object that don't reference a model definition are not allowed. ARM doesn't allow generic type definitions as this leads to bad customer experience. |
51 changes: 51 additions & 0 deletions docs/libraries/azure-resource-manager/rules/no-empty-model.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
---
title: no-empty-model
---

```text title=- Full name-
@azure-tools/typespec-azure-resource-manager/no-empty-model
```

ARM Properties with type:object that don't reference a model definition are not allowed. ARM doesn't allow generic type definitions as this leads to bad customer experience.

#### ❌ Incorrect

```tsp
model Information {
address: {};
}
```

#### ❌ Incorrect

```tsp
model Empty {}
```

#### ✅ Correct

```tsp
model Information {
address: Address;
}
model Address {
street: string;
city: string;
state: string;
country: string;
postalCode: string;
}
```

#### ✅ Correct

```tsp
model Information {
street: string;
city: string;
state: string;
country: string;
postalCode: 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 @@ -60,6 +60,7 @@ Available ruleSets:
| `@azure-tools/typespec-azure-resource-manager/resource-name` | Check the resource name. |
| `@azure-tools/typespec-azure-resource-manager/retry-after` | Check if retry-after header appears in response body. |
| [`@azure-tools/typespec-azure-resource-manager/unsupported-type`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/unsupported-type) | Check for unsupported ARM types. |
| [`@azure-tools/typespec-azure-resource-manager/no-empty-model`](https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/no-empty-model) | ARM Properties with type:object that don't reference a model definition are not allowed. ARM doesn't allow generic type definitions as this leads to bad customer experience. |

## Decorators

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 @@ -23,6 +23,7 @@ import { envelopePropertiesRules } from "./rules/envelope-properties.js";
import { listBySubscriptionRule } from "./rules/list-operation.js";
import { lroLocationHeaderRule } from "./rules/lro-location-header.js";
import { missingXmsIdentifiersRule } from "./rules/missing-x-ms-identifiers.js";
import { noEmptyModel } from "./rules/no-empty-model.js";
import { noResponseBodyRule } from "./rules/no-response-body.js";
import { operationsInterfaceMissingRule } from "./rules/operations-interface-missing.js";
import { patchEnvelopePropertiesRules } from "./rules/patch-envelope-properties.js";
Expand Down Expand Up @@ -61,6 +62,7 @@ const rules = [
resourceNameRule,
retryAfterRule,
unsupportedTypeRule,
noEmptyModel,
];

export const $linter = defineLinter({
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import { createRule, Model } from "@typespec/compiler";

export const noEmptyModel = createRule({
name: "no-empty-model",
severity: "warning",
description:
"ARM Properties with type:object that don't reference a model definition are not allowed. ARM doesn't allow generic type definitions as this leads to bad customer experience.",
url: "https://azure.github.io/typespec-azure/docs/libraries/azure-resource-manager/rules/no-empty-model",
messages: {
default: "Properties with type:object must have definition of a reference model.",
extends:
"The `type:object` model is not defined in the payload. Define the reference model of the object or change the `type` to a primitive data type like string, int, etc",
},
create(context) {
return {
model: (model: Model) => {
if (model.properties.size === 0) {
context.reportDiagnostic({
code: "no-empty-model",
target: model,
});
}
},
};
},
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
import {
BasicTestRunner,
LinterRuleTester,
createLinterRuleTester,
} from "@typespec/compiler/testing";
import { beforeEach, it } from "vitest";
import { noEmptyModel } from "../../src/rules/no-empty-model.js";
import { createAzureResourceManagerTestRunner } from "../test-host.js";

const armDef = `
@armProviderNamespace
@useDependency(Azure.ResourceManager.Versions.v1_0_Preview_1)
namespace Microsoft.Contoso;
`;

let runner: BasicTestRunner;
let tester: LinterRuleTester;

beforeEach(async () => {
runner = await createAzureResourceManagerTestRunner();
tester = createLinterRuleTester(
runner,
noEmptyModel,
"@azure-tools/typespec-azure-resource-manager",
);
});

it("emits diagnostic when a property use type:object that is not defined", async () => {
await tester
.expect(
`
${armDef}
model Foo {
props: {};
}
`,
)
.toEmitDiagnostics({
code: "@azure-tools/typespec-azure-resource-manager/no-empty-model",
message: "Properties with type:object must have definition of a reference model.",
});
});

it("emits diagnostic when model type:object is not defined", async () => {
await tester
.expect(
`
${armDef}
model Foo { }
`,
)
.toEmitDiagnostics({
code: "@azure-tools/typespec-azure-resource-manager/no-empty-model",
message: "Properties with type:object must have definition of a reference model.",
});
});

it("valid when a property use type:object that is defined", async () => {
await tester
.expect(
`
${armDef}
model WidgetProperties {
props: Foo;
}
model Foo{
Name: string;
}
`,
)
.toBeValid();
});

it("valid when a property use type:object known scalar", async () => {
await tester
.expect(
`
${armDef}
model WidgetProperties {
Date: utcDateTime;
}
`,
)
.toBeValid();
});

it("valid when a property use a simple data type ", async () => {
await tester
.expect(
`
${armDef}
model WidgetProperties {
Name: string;
}
`,
)
.toBeValid();
});
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ export default {

// Azure resource manager
"@azure-tools/typespec-azure-resource-manager/arm-no-record": true,
"@azure-tools/typespec-azure-resource-manager/no-empty-model": true,
"@azure-tools/typespec-azure-resource-manager/arm-common-types-version": true,
"@azure-tools/typespec-azure-resource-manager/arm-delete-operation-response-codes": true,
"@azure-tools/typespec-azure-resource-manager/arm-put-operation-response-codes": true,
Expand Down

0 comments on commit 22aa5ee

Please sign in to comment.