Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[tcgc] add linter rulesets framework #1208

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
---
title: "require-client-suffix"
---

```text title="Full name"
@azure-tools/typespec-client-generator-core/require-client-suffix
```

Verify that the generated client's name will have the suffix `Client` in its name

#### ❌ Incorrect

```ts
// main.tsp
namespace MyService;
```

```ts
// client.tsp
import "@azure-tools/typespec-client-generator-core";
import "./main.tsp";

using Azure.ClientGenerator.Core;

namespace MyCustomizations;

@@client(MyService);
```

#### ✅ Ok

Either you can rename the client using input param `{"name": "MyClient"}`, or if you are completely recreating a client namespace in your `client.tsp`, you can rely on that namespace to end in `Client`

```ts
// client.tsp
import "@azure-tools/typespec-client-generator-core";
iscai-msft marked this conversation as resolved.
Show resolved Hide resolved
import "./main.tsp";

using Azure.ClientGenerator.Core;

@client({service: MyService})
namespace MyClient {
iscai-msft marked this conversation as resolved.
Show resolved Hide resolved
}
```
11 changes: 2 additions & 9 deletions packages/typespec-client-generator-core/src/decorators.ts
Original file line number Diff line number Diff line change
Expand Up @@ -157,13 +157,6 @@ export const $client: ClientDecorator = (
explicitService?.kind === "Namespace"
? explicitService
: findClientService(context.program, target) ?? (target as any);
if (!name.endsWith("Client")) {
reportDiagnostic(context.program, {
code: "client-name",
format: { name },
target: context.decoratorTarget,
});
}

if (!isService(context.program, service)) {
reportDiagnostic(context.program, {
Expand Down Expand Up @@ -781,7 +774,7 @@ export const $clientFormat: ClientFormatDecorator = (
setScopedDecoratorData(context, $clientFormat, clientFormatKey, target, format, scope); // eslint-disable-line deprecation/deprecation
} else {
reportDiagnostic(context.program, {
code: "incorrect-client-format",
code: "invalid-client-format",
format: { format, expectedTargetTypes: expectedTargetTypes.join('", "') },
target: context.decoratorTarget,
});
Expand Down Expand Up @@ -929,7 +922,7 @@ export const $access: AccessDecorator = (
) => {
if (typeof value.value !== "string" || (value.value !== "public" && value.value !== "internal")) {
reportDiagnostic(context.program, {
code: "access",
code: "incorrect-access",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isn't it invalid-access in lib.ts that should make the build fail, doesn't it?

format: {},
target: entity,
});
Expand Down
1 change: 1 addition & 0 deletions packages/typespec-client-generator-core/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
export * from "./decorators.js";
export * from "./interfaces.js";
export * from "./lib.js";
export { $linter } from "./linter.js";
export * from "./public-utils.js";
export * from "./types.js";
export { $onValidate } from "./validate.js";
38 changes: 2 additions & 36 deletions packages/typespec-client-generator-core/src/lib.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,25 +3,13 @@ import { createTypeSpecLibrary, paramMessage } from "@typespec/compiler";
export const $lib = createTypeSpecLibrary({
name: "@azure-tools/typespec-client-generator-core",
diagnostics: {
"client-name": {
severity: "warning",
messages: {
default: paramMessage`Client name "${"name"}" must end with Client. Use @client({name: "...Client"}`,
},
},
"client-service": {
severity: "warning",
messages: {
default: paramMessage`Client "${"name"}" is not inside a service namespace. Use @client({service: MyServiceNS}`,
},
},
"unknown-client-format": {
severity: "error",
messages: {
default: paramMessage`Client format "${"format"}" is unknown. Known values are "${"knownValues"}"`,
},
},
"incorrect-client-format": {
"invalid-client-format": {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this actually used?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's used when we're checking the input of @clientFormat. Is this valid?

severity: "error",
messages: {
default: paramMessage`Format "${"format"}" can only apply to "${"expectedTargetTypes"}"`,
Expand All @@ -33,22 +21,7 @@ export const $lib = createTypeSpecLibrary({
default: "Cannot have a union containing only null types.",
},
},
"union-unsupported": {
severity: "error",
messages: {
default:
"Unions cannot be emitted by our language generators unless all options are literals of the same type.",
null: "Unions containing multiple model types cannot be emitted unless the union is between one model type and 'null'.",
},
},
"use-enum-instead": {
severity: "warning",
messages: {
default:
"Use enum instead of union of string or number literals. Falling back to the literal type.",
},
},
access: {
"invalid-access": {
severity: "error",
messages: {
default: `Access decorator value must be "public" or "internal".`,
Expand All @@ -60,13 +33,6 @@ export const $lib = createTypeSpecLibrary({
default: `Usage decorator value must be 2 ("input") or 4 ("output").`,
},
},
"invalid-encode": {
severity: "error",
messages: {
default: "Invalid encoding",
wrongType: paramMessage`Encoding '${"encoding"}' cannot be used on type '${"type"}'`,
},
},
"conflicting-multipart-model-usage": {
severity: "error",
messages: {
Expand Down
7 changes: 7 additions & 0 deletions packages/typespec-client-generator-core/src/linter.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import { defineLinter } from "@typespec/compiler";
import { requireClientSuffixRule } from "./rules/require-client-suffix.js";


export const $linter = defineLinter({
rules: [requireClientSuffixRule],
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
import { createRule, Interface, Namespace, paramMessage } from "@typespec/compiler";
import { getClient } from "../decorators.js";
import { createTCGCContext } from "../internal-utils.js";

export const requireClientSuffixRule = createRule({
name: "require-client-suffix",
description: "Client names should end with 'Client'.",
severity: "warning",
messages: {
default: paramMessage`Client name "${"name"}" must end with Client. Use @client({name: "...Client"}`,
},
create(context) {
const tcgcContext = createTCGCContext(context.program);
return {
namespace: (namespace: Namespace) => {
const sdkClient = getClient(tcgcContext, namespace);
if (sdkClient && !sdkClient.name.endsWith("Client")) {
context.reportDiagnostic({
target: namespace,
format: {
name: sdkClient.name,
},
});
}
},
interface: (interfaceType: Interface) => {
const sdkClient = getClient(tcgcContext, interfaceType);
if (sdkClient && !sdkClient.name.endsWith("Client")) {
context.reportDiagnostic({
target: interfaceType,
format: {
name: sdkClient.name,
},
});
}
},
};
},
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
import {
BasicTestRunner,
createLinterRuleTester,
LinterRuleTester,
} from "@typespec/compiler/testing";
import { beforeEach, describe, it } from "vitest";
import { requireClientSuffixRule } from "../../src/rules/require-client-suffix.js";
import { createTcgcTestRunner } from "../test-host.js";

describe("typespec-client-generator-core: client names end with 'Client'", () => {
iscai-msft marked this conversation as resolved.
Show resolved Hide resolved
let runner: BasicTestRunner;
let tester: LinterRuleTester;

beforeEach(async () => {
runner = await createTcgcTestRunner();
tester = createLinterRuleTester(
runner,
requireClientSuffixRule,
"@azure-tools/typespec-client-generator-core"
);
});

it("namespace", async () => {
await tester
.expect(
`
@service
namespace MyService;

namespace MyCustomizations {
@@client(MyService);
}
`
)
.toEmitDiagnostics([
{
code: "@azure-tools/typespec-client-generator-core/require-client-suffix",
severity: "warning",
message: `Client name "MyNamespace" must end with Client. Use @client({name: "...Client"}`,
},
]);
});
});
13 changes: 13 additions & 0 deletions packages/typespec-client-generator-core/test/test-host.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,19 @@ export interface CreateSdkTestRunnerOptions extends SdkEmitterOptions {
packageName?: string;
}

export async function createTcgcTestRunner(): Promise<SdkTestRunner> {
const host = await createSdkTestHost();
const autoUsings = [
"Azure.ClientGenerator.Core",
"TypeSpec.Rest",
"TypeSpec.Http",
"TypeSpec.Versioning",
];
return createTestWrapper(host, {
autoUsings: autoUsings,
}) as SdkTestRunner;
}

export async function createSdkTestRunner(
options: CreateSdkTestRunnerOptions = {},
sdkContextOption?: CreateSdkContextOptions
Expand Down
Loading