From 1726967db913b055dacf2ce652e23c55bcc3f232 Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Mon, 5 Aug 2024 13:22:03 -0700 Subject: [PATCH] No private usage linter rule (#1193) fix [#977](https://github.com/Azure/typespec-azure/issues/977) Add new linter rule to prevent using items from Private namespace from an external library. --- ...-private-linter-rule-2024-6-17-15-10-51.md | 7 ++ ...-private-linter-rule-2024-6-17-15-11-17.md | 7 ++ docs/libraries/azure-core/reference/linter.md | 1 + .../azure-core/rules/no-private-usage.md | 32 +++++++ packages/typespec-azure-core/README.md | 1 + packages/typespec-azure-core/src/linter.ts | 2 + .../src/rules/no-private-usage.ts | 94 +++++++++++++++++++ .../test/rules/no-private-usage.test.ts | 75 +++++++++++++++ .../src/rulesets/data-plane.ts | 1 + .../src/rulesets/resource-manager.ts | 1 + 10 files changed, 221 insertions(+) create mode 100644 .chronus/changes/no-private-linter-rule-2024-6-17-15-10-51.md create mode 100644 .chronus/changes/no-private-linter-rule-2024-6-17-15-11-17.md create mode 100644 docs/libraries/azure-core/rules/no-private-usage.md create mode 100644 packages/typespec-azure-core/src/rules/no-private-usage.ts create mode 100644 packages/typespec-azure-core/test/rules/no-private-usage.test.ts diff --git a/.chronus/changes/no-private-linter-rule-2024-6-17-15-10-51.md b/.chronus/changes/no-private-linter-rule-2024-6-17-15-10-51.md new file mode 100644 index 0000000000..445c400491 --- /dev/null +++ b/.chronus/changes/no-private-linter-rule-2024-6-17-15-10-51.md @@ -0,0 +1,7 @@ +--- +changeKind: feature +packages: + - "@azure-tools/typespec-azure-core" +--- + +Add new linter rule to prevent using items from Private namespace from an external library. \ No newline at end of file diff --git a/.chronus/changes/no-private-linter-rule-2024-6-17-15-11-17.md b/.chronus/changes/no-private-linter-rule-2024-6-17-15-11-17.md new file mode 100644 index 0000000000..604c13d971 --- /dev/null +++ b/.chronus/changes/no-private-linter-rule-2024-6-17-15-11-17.md @@ -0,0 +1,7 @@ +--- +changeKind: feature +packages: + - "@azure-tools/typespec-azure-rulesets" +--- + +Add new `no-private-usage` linter rule to `data-plane` and `resource-manager` rulesets \ No newline at end of file diff --git a/docs/libraries/azure-core/reference/linter.md b/docs/libraries/azure-core/reference/linter.md index f3471b65e8..e9f6e2d7e3 100644 --- a/docs/libraries/azure-core/reference/linter.md +++ b/docs/libraries/azure-core/reference/linter.md @@ -62,3 +62,4 @@ Available ruleSets: | `@azure-tools/typespec-azure-core/use-standard-operations` | Operations should be defined using a signature from the Azure.Core namespace. | | [`@azure-tools/typespec-azure-core/no-string-discriminator`](/libraries/azure-core/rules/no-string-discriminator.md) | Azure services discriminated models should define the discriminated property as an extensible union. | | `@azure-tools/typespec-azure-core/friendly-name` | Ensures that @friendlyName is used as intended. | +| [`@azure-tools/typespec-azure-core/no-private-usage`](/libraries/azure-core/rules/no-private-usage.md) | Verify that elements inside Private namespace are not referenced. | diff --git a/docs/libraries/azure-core/rules/no-private-usage.md b/docs/libraries/azure-core/rules/no-private-usage.md new file mode 100644 index 0000000000..6a9c8a641e --- /dev/null +++ b/docs/libraries/azure-core/rules/no-private-usage.md @@ -0,0 +1,32 @@ +--- +title: "no-private-usage" +--- + +```text title="Full name" +@azure-tools/typespec-azure-core/no-private-usage +``` + +Verify that a spec is not referencing items from another library using a private namespace. + +#### ❌ Incorrect + +```ts +@Azure.Core.Foundations.Private.embeddingVector(string) +model Foo {} +``` + +#### ✅ Ok + +Using items from a private namespace within the same library is allowed. + +```ts +namespace MyService; + +@MyService.Private.myPrivateDecorator +model Foo {} + + +namespace Private { + extern dec myPrivateDecorator(target); +} +``` diff --git a/packages/typespec-azure-core/README.md b/packages/typespec-azure-core/README.md index a4e42fbd6d..6566a4b50a 100644 --- a/packages/typespec-azure-core/README.md +++ b/packages/typespec-azure-core/README.md @@ -66,6 +66,7 @@ Available ruleSets: | `@azure-tools/typespec-azure-core/use-standard-operations` | Operations should be defined using a signature from the Azure.Core namespace. | | [`@azure-tools/typespec-azure-core/no-string-discriminator`](https://azure.github.io/typespec-azure/docs/libraries/azure-core/rules/no-string-discriminator) | Azure services discriminated models should define the discriminated property as an extensible union. | | `@azure-tools/typespec-azure-core/friendly-name` | Ensures that @friendlyName is used as intended. | +| [`@azure-tools/typespec-azure-core/no-private-usage`](https://azure.github.io/typespec-azure/docs/libraries/azure-core/rules/no-private-usage) | Verify that elements inside Private namespace are not referenced. | ## Decorators diff --git a/packages/typespec-azure-core/src/linter.ts b/packages/typespec-azure-core/src/linter.ts index 179a84a2bf..367ec660bb 100644 --- a/packages/typespec-azure-core/src/linter.ts +++ b/packages/typespec-azure-core/src/linter.ts @@ -15,6 +15,7 @@ import { noGenericNumericRule } from "./rules/no-generic-numeric.js"; import { noNullableRule } from "./rules/no-nullable.js"; import { noOffsetDateTimeRule } from "./rules/no-offsetdatetime.js"; import { operationIdRule } from "./rules/no-operation-id.js"; +import { noPrivateUsage } from "./rules/no-private-usage.js"; import { noResponseBodyRule } from "./rules/no-response-body.js"; import { noRpcPathParamsRule } from "./rules/no-rpc-path-params.js"; import { noStringDiscriminatorRule } from "./rules/no-string-discriminator.js"; @@ -71,6 +72,7 @@ const rules = [ useStandardOperations, noStringDiscriminatorRule, friendlyNameRule, + noPrivateUsage, ]; export const $linter = defineLinter({ diff --git a/packages/typespec-azure-core/src/rules/no-private-usage.ts b/packages/typespec-azure-core/src/rules/no-private-usage.ts new file mode 100644 index 0000000000..49b9015bea --- /dev/null +++ b/packages/typespec-azure-core/src/rules/no-private-usage.ts @@ -0,0 +1,94 @@ +import { + createRule, + DecoratedType, + DiagnosticTarget, + getLocationContext, + getTypeName, + Namespace, + paramMessage, + Type, +} from "@typespec/compiler"; + +export const noPrivateUsage = createRule({ + name: "no-private-usage", + description: "Verify that elements inside Private namespace are not referenced.", + severity: "warning", + url: "https://azure.github.io/typespec-azure/docs/libraries/azure-core/rules/no-private-usage", + messages: { + default: paramMessage`Referencing elements inside Private namespace "${"ns"}" is not allowed.`, + }, + create(context) { + function checkReference(origin: Type, type: Type, target: DiagnosticTarget) { + if (getLocationContext(context.program, origin).type !== "project") { + return; + } + if (getLocationContext(context.program, type).type === "project") { + return; + } + if (isInPrivateNamespace(type)) { + context.reportDiagnostic({ + target, + format: { ns: getTypeName(type.namespace) }, + }); + } + } + + function checkDecorators(type: Type & DecoratedType) { + if (getLocationContext(context.program, type).type !== "project") { + return; + } + for (const decorator of type.decorators) { + if ( + decorator.definition && + isInPrivateNamespace(decorator.definition) && + getLocationContext(context.program, decorator.definition).type !== "project" + ) { + context.reportDiagnostic({ + target: decorator.node ?? type, + format: { ns: getTypeName(decorator.definition.namespace) }, + }); + } + } + } + return { + model: (model) => { + checkDecorators(model); + model.baseModel && checkReference(model, model.baseModel, model); + }, + modelProperty: (prop) => { + checkDecorators(prop); + checkReference(prop, prop.type, prop); + }, + unionVariant: (variant) => { + checkDecorators(variant); + checkReference(variant, variant.type, variant); + }, + operation: (type) => { + checkDecorators(type); + }, + interface: (type) => { + checkDecorators(type); + }, + enum: (type) => { + checkDecorators(type); + }, + union: (type) => { + checkDecorators(type); + }, + }; + }, +}); + +function isInPrivateNamespace(type: Type): type is Type & { namespace: Namespace } { + if (!("namespace" in type)) { + return false; + } + let current = type; + while (current.namespace) { + if (current.namespace?.name === "Private") { + return true; + } + current = current.namespace; + } + return false; +} diff --git a/packages/typespec-azure-core/test/rules/no-private-usage.test.ts b/packages/typespec-azure-core/test/rules/no-private-usage.test.ts new file mode 100644 index 0000000000..8744206cdf --- /dev/null +++ b/packages/typespec-azure-core/test/rules/no-private-usage.test.ts @@ -0,0 +1,75 @@ +import { + BasicTestRunner, + LinterRuleTester, + createLinterRuleTester, +} from "@typespec/compiler/testing"; +import { beforeEach, it } from "vitest"; +import { noPrivateUsage } from "../../src/rules/no-private-usage.js"; +import { createAzureCoreTestRunner } from "../test-host.js"; + +let runner: BasicTestRunner; +let tester: LinterRuleTester; + +beforeEach(async () => { + runner = await createAzureCoreTestRunner({ omitServiceNamespace: true }); + tester = createLinterRuleTester(runner, noPrivateUsage, "@azure-tools/typespec-azure-core"); +}); + +it("emits a warning diagnostic if using type from Azure.Core.Foundations.Private", async () => { + await tester + .expect( + ` + @useDependency(Azure.Core.Versions.v1_0_Preview_2) + namespace MyService { + model Foo { + bar: Azure.Core.Foundations.Private.ArmResourceIdentifierConfigOptions + } + } + ` + ) + .toEmitDiagnostics([ + { + code: "@azure-tools/typespec-azure-core/no-private-usage", + message: + 'Referencing elements inside Private namespace "Azure.Core.Foundations.Private" is not allowed.', + }, + ]); +}); + +it("emits a warning diagnostic if using decorators from Azure.Core.Foundations.Private", async () => { + await tester + .expect( + ` + @useDependency(Azure.Core.Versions.v1_0_Preview_2) + namespace MyService { + @Azure.Core.Foundations.Private.embeddingVector(string) + model Foo {} + } + ` + ) + .toEmitDiagnostics([ + { + code: "@azure-tools/typespec-azure-core/no-private-usage", + message: + 'Referencing elements inside Private namespace "Azure.Core.Foundations.Private" is not allowed.', + }, + ]); +}); + +it("ok using item from Private namespace in the project", async () => { + await tester + .expect( + ` + namespace MyService { + model Foo { + bar: MyLib.Private.Bar; + } + } + + namespace MyLib.Private { + model Bar {} + } + ` + ) + .toBeValid(); +}); diff --git a/packages/typespec-azure-rulesets/src/rulesets/data-plane.ts b/packages/typespec-azure-rulesets/src/rulesets/data-plane.ts index 37ceff4744..854553bfbc 100644 --- a/packages/typespec-azure-rulesets/src/rulesets/data-plane.ts +++ b/packages/typespec-azure-rulesets/src/rulesets/data-plane.ts @@ -37,6 +37,7 @@ export default { "@azure-tools/typespec-azure-core/use-standard-names": true, "@azure-tools/typespec-azure-core/use-standard-operations": true, "@azure-tools/typespec-azure-core/no-string-discriminator": true, + "@azure-tools/typespec-azure-core/no-private-usage": true, "@azure-tools/typespec-azure-core/friendly-name": true, // Azure core rules enabled via an optional rulesets diff --git a/packages/typespec-azure-rulesets/src/rulesets/resource-manager.ts b/packages/typespec-azure-rulesets/src/rulesets/resource-manager.ts index e1a63c3cb9..a046682609 100644 --- a/packages/typespec-azure-rulesets/src/rulesets/resource-manager.ts +++ b/packages/typespec-azure-rulesets/src/rulesets/resource-manager.ts @@ -37,6 +37,7 @@ export default { "@azure-tools/typespec-azure-core/use-standard-names": true, "@azure-tools/typespec-azure-core/use-standard-operations": true, "@azure-tools/typespec-azure-core/no-string-discriminator": true, + "@azure-tools/typespec-azure-core/no-private-usage": true, "@azure-tools/typespec-azure-core/friendly-name": true, // Azure core not enabled - Arm has its own conflicting rule