Skip to content

Commit

Permalink
Feature: Code fixes (microsoft#2888)
Browse files Browse the repository at this point in the history
closes microsoft#615

## Code fixes added

### Suppress
![Kapture 2024-02-05 at 15 16
22](https://github.com/microsoft/typespec/assets/1031227/644014a3-9352-4bd4-b1b8-0d314c627405)

### `number` -> `float64` typo fix

![Kapture 2024-02-06 at 09 50
28](https://github.com/microsoft/typespec/assets/1031227/65b2e9aa-c510-440f-a1c6-7851611b65a2)

### Enum to extensible enum in typespec-azure

Azure/typespec-azure#258

---------

Co-authored-by: Mark Cowlishaw <[email protected]>
  • Loading branch information
timotheeguerin and markcowl authored Mar 5, 2024
1 parent abba29c commit 6a9c62a
Show file tree
Hide file tree
Showing 32 changed files with 936 additions and 61 deletions.
8 changes: 8 additions & 0 deletions .chronus/changes/feature-code-fixes-2024-1-14-18-23-37.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
# Change versionKind to one of: internal, fix, dependencies, feature, deprecation, breaking
changeKind: feature
packages:
- "@typespec/compiler"
---

Add support for codefixes
8 changes: 8 additions & 0 deletions .chronus/changes/feature-code-fixes-2024-1-14-19-34-9.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
# Change versionKind to one of: internal, fix, dependencies, feature, deprecation, breaking
changeKind: fix
packages:
- "@typespec/playground"
---

Refactor of the mapping between Language Server and monaco API
78 changes: 78 additions & 0 deletions docs/extending-typespec/codefixes.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
---
title: Code fixes
---

## Define a code fix

A code fix can be defined using the `defineCodeFix` function which is just here to help with typing. It doesn't need to be declared separately from being reported but doing so allows you to test it.

A codefix takes 3 arguments:

- `id`: A unique identifier for the code fix.
- `label`: A human-readable label for the code fix.
- `fix`: The implementation of the codefix. Takes in a context that allows patch operations on the source code. The fix function should return the list of changes to apply to the source code.

```ts
import { defineCodeFix, getSourceLocation, type IdentifierNode } from "@typespec/compiler";

export function createChangeIdentifierCodeFix(node: IdentifierNode, newIdentifier: string) {
return defineCodeFix({
id: "change-identifier",
label: `Change ${node.sv} to ${newIdentifier}`,
fix: (context) => {
const location = getSourceLocation(node);
return context.replaceText(location, newIdentifier);
},
});
}
```

## Connect a codefix to a diagnostic

When reporting diagnostics, you can pass `codefixes` to the `reportDiagnostic`/`createDiagnostic` functions. It is an array of codefixes that can be used to fix the diagnostic.

```ts
reportDiagnostic({
code: "invalid-identifier",
target: node,
codefixes: [createChangeIdentifierCodeFix(node, "string")],
});
```

## Test a diagnostic

[See here for testing a codefix inside a linter rule](./linters.md#test-a-codefix)

Testing a codefix is done by using the `expectCodeFixOnAst` function from the `@typespec/compiler/testing` package. It takes in the source code and a function that returns the codefix to apply.
It takes the input source code with a cursor defined by `` which will be used to resolve the node where the codefix should be applied. The callback function will receive that resolved node and is expected to return the codefix to test.

:::note
When using multi-line strings (with `\``) in typescript there is no de-indenting done so you will need to make sure the input and expected result are aligned to the left.
:::

```ts
import { strictEqual } from "assert";
import { createChangeIdentifierCodeFix } from "./change-identifier.codefix.js";
import { SyntaxKind } from "@typespec/compiler";
import { expectCodeFixOnAst } from "@typespec/compiler/testing";

describe("CodeFix: change-identifier", () => {
it("it change identifier", async () => {
await expectCodeFixOnAst(
`
model Foo {
a: ┆number;
}
`,
(node) => {
strictEqual(node.kind, SyntaxKind.Identifier);
return createChangeIdentifierCodeFix(node, "int32");
}
).toChangeTo(`
model Foo {
a: int32;
}
`);
});
});
```
47 changes: 47 additions & 0 deletions docs/extending-typespec/linters.md
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,30 @@ export const requiredDocRule = createLinterRule({
});
```

#### Provide a codefix

[See codefixes](./codefixes.md) for more details on how codefixes work in the TypeSpec ecosystem.

In the same way you can provide a codefix on any reported diagnostic, you can pass codefixes to the `reportDiagnostic` function.

```ts
context.reportDiagnostic({
messageId: "models",
target: model,
codefixes: [
defineCodeFix({
id: "add-model-suffix",
description: "Add 'Model' suffix to model name",
apply: (program) => {
program.update(model, {
name: `${model.name}Model`,
});
},
}),
],
});
```

#### Don'ts

- ❌ Do not call `program.reportDiagnostic` or your library `reportDiagnostic` helper directly in a linter rule
Expand Down Expand Up @@ -158,3 +182,26 @@ describe("required-doc rule", () => {
});
});
```

### Testing linter with codefixes

The linter rule tester provides an API to easily test a codefix. This is a different approach from the standalone codefix tester, which is more targeted at testing codefixes in isolation.

This can be done with calling `applyCodeFix` with the fix id. It will expect a single diagnostic to be emitted with a codefix with the given id.
Then calling `toEqual` with the expected code after the codefix is applied.

:::note
When using multi-line strings (with `\``) in typescript there is no de-indenting done so you will need to make sure the input and expected result are aligned to the left.
:::

```ts
await tester
.expect(
`
model Foo {}
`
)
.applyCodeFix("add-model-suffix").toEqual(`
model FooModel {}
`);
```
18 changes: 17 additions & 1 deletion packages/compiler/src/core/checker.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { $docFromComment, getIndexer, isArrayModelType } from "../lib/decorators.js";
import { MultiKeyMap, Mutable, createRekeyableMap, isArray, mutate } from "../utils/misc.js";
import { createSymbol, createSymbolTable } from "./binder.js";
import { createChangeIdentifierCodeFix } from "./compiler-code-fixes/change-identifier.codefix.js";
import { getDeprecationDetails, markDeprecated } from "./deprecation.js";
import { ProjectionError, compilerAssert, reportDeprecated } from "./diagnostics.js";
import { validateInheritanceDiscriminatedUnions } from "./helpers/discriminator-utils.js";
Expand Down Expand Up @@ -34,6 +35,7 @@ import {
AugmentDecoratorStatementNode,
BooleanLiteral,
BooleanLiteralNode,
CodeFix,
DecoratedType,
Decorator,
DecoratorApplication,
Expand Down Expand Up @@ -2337,12 +2339,26 @@ export function createChecker(program: Program): Checker {

if (mapper === undefined) {
reportCheckerDiagnostic(
createDiagnostic({ code: "unknown-identifier", format: { id: node.sv }, target: node })
createDiagnostic({
code: "unknown-identifier",
format: { id: node.sv },
target: node,
codefixes: getCodefixesForUnknownIdentifier(node),
})
);
}
return undefined;
}

function getCodefixesForUnknownIdentifier(node: IdentifierNode): CodeFix[] | undefined {
switch (node.sv) {
case "number":
return [createChangeIdentifierCodeFix(node, "float64")];
default:
return undefined;
}
}

function resolveTypeReferenceSym(
node: TypeReferenceNode | MemberExpressionNode | IdentifierNode,
mapper: TypeMapper | undefined,
Expand Down
97 changes: 97 additions & 0 deletions packages/compiler/src/core/code-fixes.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
import { isArray } from "../utils/misc.js";
import type {
CodeFix,
CodeFixContext,
CodeFixEdit,
CompilerHost,
FilePos,
InsertTextCodeFixEdit,
ReplaceTextCodeFixEdit,
SourceFile,
SourceLocation,
} from "./types.js";

export async function resolveCodeFix(codeFix: CodeFix): Promise<CodeFixEdit[]> {
const context = createCodeFixContext();
const values = await codeFix.fix(context);
const textEdit = values === undefined ? [] : isArray(values) ? values : [values];
return textEdit;
}

export async function applyCodeFix(host: CompilerHost, codeFix: CodeFix) {
const edits = await resolveCodeFix(codeFix);
await applyCodeFixEdits(host, edits);
}

async function applyCodeFixEdits(host: CompilerHost, edits: CodeFixEdit[]) {
const perFile = new Map<string, [SourceFile, CodeFixEdit[]]>();

for (const edit of edits) {
const file = edit.file;
if (!perFile.has(file.path)) {
perFile.set(file.path, [file, []]);
}
perFile.get(file.path)![1].push(edit);
}

for (const [file, edits] of perFile.values()) {
const newContent = applyCodeFixEditsOnText(file.text, edits);
await host.writeFile(file.path, newContent);
}
}

function applyCodeFixEditsOnText(content: string, edits: CodeFixEdit[]): string {
const segments = [];
let last = 0;
for (const edit of edits) {
switch (edit.kind) {
case "insert-text":
segments.push(content.slice(last, edit.pos));
segments.push(edit.text);
last = edit.pos;
break;
case "replace-text":
segments.push(content.slice(last, edit.pos));
segments.push(edit.text);
last = edit.end;
}
}
segments.push(content.slice(last));
return segments.join("");
}

function createCodeFixContext(): CodeFixContext {
return {
prependText,
appendText,
replaceText,
};

function prependText(node: SourceLocation | FilePos, text: string): InsertTextCodeFixEdit {
return {
kind: "insert-text",
pos: node.pos,
text,
file: node.file,
};
}

function appendText(node: SourceLocation | FilePos, text: string): InsertTextCodeFixEdit {
return {
kind: "insert-text",
pos: "end" in node ? node.end : node.pos,
text,
file: node.file,
};
}

function replaceText(node: SourceLocation, text: string): ReplaceTextCodeFixEdit {
return {
kind: "replace-text",
pos: node.pos,
end: node.end,
file: node.file,
text,
};
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import { defineCodeFix, getSourceLocation } from "../diagnostics.js";
import type { IdentifierNode } from "../types.js";

export function createChangeIdentifierCodeFix(node: IdentifierNode, newIdentifier: string) {
return defineCodeFix({
id: "change-identifier",
label: `Change ${node.sv} to ${newIdentifier}`,
fix: (context) => {
const location = getSourceLocation(node);
return context.replaceText(location, newIdentifier);
},
});
}
31 changes: 31 additions & 0 deletions packages/compiler/src/core/compiler-code-fixes/suppress.codefix.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import { isWhiteSpace } from "../charcode.js";
import { defineCodeFix, getSourceLocation } from "../diagnostics.js";
import type { DiagnosticTarget, SourceLocation } from "../types.js";

export function createSuppressCodeFix(diagnosticTarget: DiagnosticTarget, warningCode: string) {
return defineCodeFix({
id: "suppress",
label: `Suppress warning: "${warningCode}"`,
fix: (context) => {
const location = getSourceLocation(diagnosticTarget);
const { lineStart, indent } = findLineStartAndIndent(location);
const updatedLocation = { ...location, pos: lineStart };
return context.prependText(updatedLocation, `${indent}#suppress "${warningCode}" ""\n`);
},
});
}

function findLineStartAndIndent(location: SourceLocation): { lineStart: number; indent: string } {
const text = location.file.text;
let pos = location.pos;
let indent = 0;
while (pos > 0 && text[pos - 1] !== "\n") {
if (isWhiteSpace(text.charCodeAt(pos - 1))) {
indent++;
} else {
indent = 0;
}
pos--;
}
return { lineStart: pos, indent: location.file.text.slice(pos, pos + indent) };
}
7 changes: 6 additions & 1 deletion packages/compiler/src/core/diagnostic-creator.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { mutate } from "../utils/misc.js";
import type { Program } from "./program.js";
import type {
Diagnostic,
Expand Down Expand Up @@ -50,12 +51,16 @@ export function createDiagnosticCreator<T extends { [code: string]: DiagnosticMe

const messageStr = typeof message === "string" ? message : message((diagnostic as any).format);

return {
const result: Diagnostic = {
code: libraryName ? `${libraryName}/${String(diagnostic.code)}` : diagnostic.code.toString(),
severity: diagnosticDef.severity,
message: messageStr,
target: diagnostic.target,
};
if (diagnostic.codefixes) {
mutate(result).codefixes = diagnostic.codefixes;
}
return result;
}

function reportDiagnostic<C extends keyof T, M extends keyof T[C] = "default">(
Expand Down
5 changes: 5 additions & 0 deletions packages/compiler/src/core/diagnostics.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { formatLog } from "./logger/index.js";
import type { Program } from "./program.js";
import { createSourceFile } from "./source-file.js";
import {
CodeFix,
Diagnostic,
DiagnosticResult,
DiagnosticTarget,
Expand Down Expand Up @@ -300,3 +301,7 @@ export function createDiagnosticCollector(): DiagnosticCollector {
export function ignoreDiagnostics<T>(result: DiagnosticResult<T>): T {
return result[0];
}

export function defineCodeFix(fix: CodeFix): CodeFix {
return fix;
}
1 change: 1 addition & 0 deletions packages/compiler/src/core/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ export {
export * from "./module-resolver.js";
export { NodeHost } from "./node-host.js";
export * from "./options.js";
export { getPositionBeforeTrivia } from "./parser-utils.js";
export * from "./parser.js";
export * from "./path-utils.js";
export * from "./program.js";
Expand Down
1 change: 1 addition & 0 deletions packages/compiler/src/core/linter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,7 @@ export function createLinterRuleContext<N extends string, DM extends DiagnosticM
severity: rule.severity,
message: messageStr,
target: diag.target,
codefixes: diag.codefixes,
};
}

Expand Down
Loading

0 comments on commit 6a9c62a

Please sign in to comment.