Skip to content
Draft
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,7 @@
---
changeKind: feature
packages:
- "@azure-tools/typespec-client-generator-core"
---

Add new `bool-property-name-prefix` lint rule (enabled in the `best-practices:csharp` ruleset) that flags boolean properties and operation parameters whose names do not start with a verb prefix such as `Is`, `Has`, `Can`, `Should`, `Are`, `Was`, `Will`, `Do`, or `Does` followed by an uppercase letter, following the Azure SDK for .NET naming conventions.
Original file line number Diff line number Diff line change
Expand Up @@ -98,5 +98,6 @@ export default {
"@azure-tools/typespec-client-generator-core/require-client-suffix": true,
"@azure-tools/typespec-client-generator-core/property-name-conflict": true,
"@azure-tools/typespec-client-generator-core/no-unnamed-types": false, // Too bad performance https://github.com/Azure/typespec-azure/issues/2803
"@azure-tools/typespec-client-generator-core/bool-property-name-prefix": true,
},
} satisfies LinterRuleSet;
10 changes: 8 additions & 2 deletions packages/typespec-client-generator-core/src/linter.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,17 @@
import { defineLinter } from "@typespec/compiler";
import { boolPropertyNamePrefixRule } from "./rules/bool-property-name-prefix.rule.js";
import { noUnnamedTypesRule } from "./rules/no-unnamed-types.rule.js";
import { propertyNameConflictRule } from "./rules/property-name-conflict.rule.js";
import { requireClientSuffixRule } from "./rules/require-client-suffix.rule.js";

const rules = [requireClientSuffixRule, propertyNameConflictRule, noUnnamedTypesRule];
const rules = [
requireClientSuffixRule,
propertyNameConflictRule,
noUnnamedTypesRule,
boolPropertyNamePrefixRule,
];

const csharpRules = [propertyNameConflictRule];
const csharpRules = [propertyNameConflictRule, boolPropertyNamePrefixRule];

export const $linter = defineLinter({
rules,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
import {
CodeFix,
createRule,
defineCodeFix,
getSourceLocation,
ModelProperty,
paramMessage,
Type,
} from "@typespec/compiler";
import { createTCGCContext } from "../context.js";
import { getLibraryName } from "../public-utils.js";

const ALLOWED_PREFIXES = ["Is", "Has", "Can", "Should", "Are", "Was", "Will", "Does", "Do"];
const SUGGESTED_PREFIXES = ["Is", "Can", "Has"] as const;

function isBooleanType(type: Type): boolean {
if (type.kind !== "Scalar") return false;
let current: Type | undefined = type;
while (current && current.kind === "Scalar") {
if (current.name === "boolean") return true;
current = current.baseScalar;
}
return false;
}

function startsWithAllowedPrefix(name: string): boolean {
for (const prefix of ALLOWED_PREFIXES) {
if (name.length > prefix.length && name.startsWith(prefix)) {
const next = name.charAt(prefix.length);
if (next >= "A" && next <= "Z") {
return true;
}
}
}
return false;
}

function toPascalCase(name: string): string {
if (name.length === 0) return name;
return name.charAt(0).toUpperCase() + name.slice(1);
}

function createClientNameCodeFix(
target: ModelProperty,
prefix: string,
csharpName: string,
): CodeFix {
const newName = `${prefix}${csharpName}`;
return defineCodeFix({
id: `add-clientName-${prefix}`,
label: `Add @clientName("${newName}", "csharp")`,
fix: (fixContext) => {
const location = getSourceLocation(target);
const text = location.file.text;
let lineStart = location.pos;
while (lineStart > 0 && text[lineStart - 1] !== "\n") {
lineStart--;
}
let indentEnd = lineStart;
while (indentEnd < text.length && (text[indentEnd] === " " || text[indentEnd] === "\t")) {
indentEnd++;
}
const indent = text.slice(lineStart, indentEnd);
const updatedLocation = { ...location, pos: lineStart };
return fixContext.prependText(
updatedLocation,
`${indent}@clientName("${newName}", "csharp")\n`,
);
},
});
}

export const boolPropertyNamePrefixRule = createRule({
name: "bool-property-name-prefix",
description:
"Boolean property and parameter names should start with a verb prefix such as Is, Has, Can, Should, etc.",
severity: "warning",
url: "https://azure.github.io/typespec-azure/docs/libraries/typespec-client-generator-core/rules/bool-property-name-prefix",
messages: {
default: paramMessage`Boolean property or parameter '${"name"}' should start with one of the following verb prefixes followed by an uppercase letter: ${"prefixes"}. Consider renaming it (for example to '${"suggestion"}') or use @clientName("${"suggestion"}", "csharp") to rename it for C#.`,
},
create(context) {
const tcgcContext = createTCGCContext(
context.program,
"@azure-tools/typespec-client-generator-core",
{
mutateNamespace: false,
},
);
return {
modelProperty: (property: ModelProperty) => {
if (!isBooleanType(property.type)) return;
// The C# emitter PascalCases property/parameter names, so apply the same
// transformation before checking for an allowed verb prefix.
const csharpName = toPascalCase(getLibraryName(tcgcContext, property, "csharp"));
if (startsWithAllowedPrefix(csharpName)) return;
const suggestion = `Is${csharpName}`;
context.reportDiagnostic({
format: {
name: csharpName,
prefixes: ALLOWED_PREFIXES.join(", "),
suggestion,
},
target: property,
codefixes: SUGGESTED_PREFIXES.map((prefix) =>
createClientNameCodeFix(property, prefix, csharpName),
),
});
},
};
},
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,194 @@
import {
createLinterRuleTester,
LinterRuleTester,
TesterInstance,
} from "@typespec/compiler/testing";
import { beforeEach, describe, it } from "vitest";
import { boolPropertyNamePrefixRule } from "../../src/rules/bool-property-name-prefix.rule.js";
import { SimpleTester } from "../tester.js";

let runner: TesterInstance;
let tester: LinterRuleTester;

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

it("emits warning when a boolean property has no verb prefix", async () => {
await tester
.expect(
`model Foo {
tracked: boolean;
}`,
)
.toEmitDiagnostics({
code: "@azure-tools/typespec-client-generator-core/bool-property-name-prefix",
message: `Boolean property or parameter 'Tracked' should start with one of the following verb prefixes followed by an uppercase letter: Is, Has, Can, Should, Are, Was, Will, Does, Do. Consider renaming it (for example to 'IsTracked') or use @clientName("IsTracked", "csharp") to rename it for C#.`,
});
});

it("is valid when boolean property starts with Is", async () => {
await tester
.expect(
`model Foo {
isTracked: boolean;
}`,
)
.toBeValid();
});

it("is valid when boolean property starts with Has", async () => {
await tester
.expect(
`model Foo {
hasDynamicFieldSchema: boolean;
}`,
)
.toBeValid();
});

it("is valid when boolean property starts with Can", async () => {
await tester
.expect(
`model Foo {
canDelete: boolean;
}`,
)
.toBeValid();
});

it("is valid when boolean property starts with Should", async () => {
await tester
.expect(
`model Foo {
shouldRetry: boolean;
}`,
)
.toBeValid();
});

it("does not emit for non-boolean properties", async () => {
await tester
.expect(
`model Foo {
name: string;
count: int32;
}`,
)
.toBeValid();
});

it("emits warning for properties whose type extends boolean", async () => {
await tester
.expect(
`scalar MyBool extends boolean;
model Foo {
tracked: MyBool;
}`,
)
.toEmitDiagnostics({
code: "@azure-tools/typespec-client-generator-core/bool-property-name-prefix",
message: `Boolean property or parameter 'Tracked' should start with one of the following verb prefixes followed by an uppercase letter: Is, Has, Can, Should, Are, Was, Will, Does, Do. Consider renaming it (for example to 'IsTracked') or use @clientName("IsTracked", "csharp") to rename it for C#.`,
});
});

it(`is valid when @clientName("IsTracked", "csharp") provides csharp-specific name with prefix`, async () => {
await tester
.expect(
`model Foo {
@clientName("IsTracked", "csharp")
tracked: boolean;
}`,
)
.toBeValid();
});

it(`emits warning when @clientName provides csharp name without prefix`, async () => {
await tester
.expect(
`model Foo {
@clientName("renamed", "csharp")
isTracked: boolean;
}`,
)
.toEmitDiagnostics({
code: "@azure-tools/typespec-client-generator-core/bool-property-name-prefix",
message: `Boolean property or parameter 'Renamed' should start with one of the following verb prefixes followed by an uppercase letter: Is, Has, Can, Should, Are, Was, Will, Does, Do. Consider renaming it (for example to 'IsRenamed') or use @clientName("IsRenamed", "csharp") to rename it for C#.`,
});
});

it("emits warning for boolean operation parameter without prefix", async () => {
await tester.expect(`op getData(tracked: boolean): void;`).toEmitDiagnostics({
code: "@azure-tools/typespec-client-generator-core/bool-property-name-prefix",
message: `Boolean property or parameter 'Tracked' should start with one of the following verb prefixes followed by an uppercase letter: Is, Has, Can, Should, Are, Was, Will, Does, Do. Consider renaming it (for example to 'IsTracked') or use @clientName("IsTracked", "csharp") to rename it for C#.`,
});
});

it("is valid for boolean operation parameter with prefix", async () => {
await tester.expect(`op getData(isTracked: boolean): void;`).toBeValid();
});

it("does not match prefix substrings without uppercase boundary", async () => {
// "issue" starts with "is" but next char is lowercase -> still invalid
await tester
.expect(
`model Foo {
issue: boolean;
}`,
)
.toEmitDiagnostics({
code: "@azure-tools/typespec-client-generator-core/bool-property-name-prefix",
});
});

describe("codefix", () => {
it("offers Is prefix codefix", async () => {
await tester
.expect(
`
model Foo {
tracked: boolean;
}`,
)
.applyCodeFix("add-clientName-Is").toEqual(`
model Foo {
@clientName("IsTracked", "csharp")
tracked: boolean;
}`);
});

it("offers Can prefix codefix", async () => {
await tester
.expect(
`
model Foo {
tracked: boolean;
}`,
)
.applyCodeFix("add-clientName-Can").toEqual(`
model Foo {
@clientName("CanTracked", "csharp")
tracked: boolean;
}`);
});

it("offers Has prefix codefix", async () => {
await tester
.expect(
`
model Foo {
tracked: boolean;
}`,
)
.applyCodeFix("add-clientName-Has").toEqual(`
model Foo {
@clientName("HasTracked", "csharp")
tracked: boolean;
}`);
});
});
Loading
Loading