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

Feature: Code fixes #2888

Merged
merged 32 commits into from
Mar 5, 2024
Merged
Show file tree
Hide file tree
Changes from 24 commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
ef78a77
Code fixes initial implementation
timotheeguerin Feb 5, 2024
790db01
Apply code fix working
timotheeguerin Feb 5, 2024
6c7d341
better way
timotheeguerin Feb 6, 2024
0ff335e
Basics for language server
timotheeguerin Feb 6, 2024
7193e59
Comment out for now
timotheeguerin Feb 6, 2024
cf6849b
Add quick fix for number -> float64
timotheeguerin Feb 6, 2024
2c88430
alllow on linters
timotheeguerin Feb 12, 2024
524833e
Merge with main
timotheeguerin Feb 12, 2024
40bc8dd
Allow passing codefixes in linter
timotheeguerin Feb 13, 2024
87ca94b
Apply code fix and linter test helper
timotheeguerin Feb 13, 2024
c53efcb
Add codefix testing api
timotheeguerin Feb 14, 2024
cc7d429
Merge branch 'main' of https://github.com/microsoft/typespec into fea…
timotheeguerin Feb 14, 2024
08ddf25
update types and add docs
timotheeguerin Feb 14, 2024
f1df1eb
revert
timotheeguerin Feb 14, 2024
097034a
.
timotheeguerin Feb 14, 2024
050da68
Create feature-code-fixes-2024-1-14-18-23-37.md
timotheeguerin Feb 14, 2024
339294d
add
timotheeguerin Feb 14, 2024
d51dd25
code fixes test
timotheeguerin Feb 14, 2024
daaeb8f
Create feature-code-fixes-2024-1-14-19-34-9.md
timotheeguerin Feb 14, 2024
13a0146
fix
timotheeguerin Feb 14, 2024
dd49d89
Merge branch 'main' of https://github.com/Microsoft/typespec into fea…
timotheeguerin Feb 15, 2024
ebaebb1
Merge branch 'main' into feature/code-fixes
timotheeguerin Feb 15, 2024
d0ab146
Merge branch 'feature/code-fixes' of https://github.com/timotheegueri…
timotheeguerin Feb 15, 2024
8f909bf
format
timotheeguerin Feb 15, 2024
79b8780
Update docs/extending-typespec/codefixes.md
timotheeguerin Mar 1, 2024
da0e355
Update docs/extending-typespec/codefixes.md
timotheeguerin Mar 1, 2024
48e54f0
Update docs/extending-typespec/linters.md
timotheeguerin Mar 1, 2024
99b7e8a
Merge branch 'main' of https://github.com/Microsoft/typespec into fea…
timotheeguerin Mar 1, 2024
477e519
Merge branch 'main' into feature/code-fixes
timotheeguerin Mar 1, 2024
acfa7f8
Fix
timotheeguerin Mar 1, 2024
9362c7f
fix
timotheeguerin Mar 1, 2024
8c89a48
Merge branch 'main' of https://github.com/Microsoft/typespec into fea…
timotheeguerin Mar 5, 2024
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
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 needs to be declared separately from being reported but doing so allows you to test it.
timotheeguerin marked this conversation as resolved.
Show resolved Hide resolved

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 to do patch operation on the source code. The fix function should return the list of changes to apply to the source code.
timotheeguerin marked this conversation as resolved.
Show resolved Hide resolved

```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 provide an API to easily test a codefix. This is a different approach to the standalone codefix tester which is more targeted at testing codefixes in isolation.
timotheeguerin marked this conversation as resolved.
Show resolved Hide resolved

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,5 +1,6 @@
import { $docFromComment, getIndexer } from "../lib/decorators.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 @@ -33,6 +34,7 @@ import {
AugmentDecoratorStatementNode,
BooleanLiteral,
BooleanLiteralNode,
CodeFix,
DecoratedType,
Decorator,
DecoratorApplication,
Expand Down Expand Up @@ -2314,12 +2316,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 type {
CodeFix,
CodeFixContext,
CodeFixEdit,
CompilerHost,
FilePos,
InsertTextCodeFixEdit,
ReplaceTextCodeFixEdit,
SourceFile,
SourceLocation,
} from "./types.js";
import { isArray } from "./util.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);
},
});
}
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
Expand Up @@ -6,6 +6,7 @@ import type {
DiagnosticMessages,
DiagnosticReport,
} from "./types.js";
import { mutate } from "./util.js";

/**
* Create a new diagnostics creator.
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 { CharCode } from "./charcode.js";
import { formatLog } from "./logger/index.js";
import type { Program } from "./program.js";
import {
CodeFix,
Diagnostic,
DiagnosticResult,
DiagnosticTarget,
Expand Down Expand Up @@ -386,3 +387,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
Loading