From abc0784e57610a0cceca63301489918015418df6 Mon Sep 17 00:00:00 2001 From: Ryan Cumming Date: Thu, 27 Jun 2019 21:30:23 +1000 Subject: Fix `cargo watch` code action filtering There are two issues with the implementation of `provideCodeActions` introduced in #1439: 1. We're returning the code action based on the file its diagnostic is in; not the file the suggested fix is in. I'm not sure how often fixes are suggested cross-file but it's something we should handle. 2. We're not filtering code actions based on the passed range. The means if there is any suggestion in a file we'll show an action for every line of the file. I naively thought that VS Code would filter for us but that was wrong. Unfortunately the VS Code `CodeAction` object is very complex - it can handle edits across multiple files, run commands, etc. This makes it complex to check them for equality or see if any of their edits intersects with a specified range. To make it easier to work with suggestions this introduces a `SuggestedFix` model object and a `SuggestFixCollection` code action provider. This is a layer between the raw Rust JSON and VS Code's `CodeAction`s. I was reluctant to introduce another layer of abstraction here but my attempt to work directly with VS Code's model objects was worse. --- editors/code/src/commands/cargo_watch.ts | 67 ++---- editors/code/src/test/rust_diagnostics.test.ts | 162 -------------- .../src/test/utils/diagnotics/SuggestedFix.test.ts | 133 ++++++++++++ .../diagnotics/SuggestedFixCollection.test.ts | 125 +++++++++++ .../code/src/test/utils/diagnotics/rust.test.ts | 173 +++++++++++++++ .../code/src/test/utils/diagnotics/vscode.test.ts | 98 +++++++++ editors/code/src/test/vscode_diagnostics.test.ts | 182 ---------------- editors/code/src/utils/diagnostics/SuggestedFix.ts | 67 ++++++ .../utils/diagnostics/SuggestedFixCollection.ts | 74 +++++++ editors/code/src/utils/diagnostics/rust.ts | 235 ++++++++++++++++++++ editors/code/src/utils/diagnostics/vscode.ts | 14 ++ editors/code/src/utils/rust_diagnostics.ts | 236 --------------------- editors/code/src/utils/vscode_diagnostics.ts | 73 ------- 13 files changed, 940 insertions(+), 699 deletions(-) delete mode 100644 editors/code/src/test/rust_diagnostics.test.ts create mode 100644 editors/code/src/test/utils/diagnotics/SuggestedFix.test.ts create mode 100644 editors/code/src/test/utils/diagnotics/SuggestedFixCollection.test.ts create mode 100644 editors/code/src/test/utils/diagnotics/rust.test.ts create mode 100644 editors/code/src/test/utils/diagnotics/vscode.test.ts delete mode 100644 editors/code/src/test/vscode_diagnostics.test.ts create mode 100644 editors/code/src/utils/diagnostics/SuggestedFix.ts create mode 100644 editors/code/src/utils/diagnostics/SuggestedFixCollection.ts create mode 100644 editors/code/src/utils/diagnostics/rust.ts create mode 100644 editors/code/src/utils/diagnostics/vscode.ts delete mode 100644 editors/code/src/utils/rust_diagnostics.ts delete mode 100644 editors/code/src/utils/vscode_diagnostics.ts diff --git a/editors/code/src/commands/cargo_watch.ts b/editors/code/src/commands/cargo_watch.ts index 1ec5f8d5f..4c3c10c8b 100644 --- a/editors/code/src/commands/cargo_watch.ts +++ b/editors/code/src/commands/cargo_watch.ts @@ -5,16 +5,15 @@ import * as vscode from 'vscode'; import { Server } from '../server'; import { terminate } from '../utils/processes'; +import { LineBuffer } from './line_buffer'; +import { StatusDisplay } from './watch_status'; + import { mapRustDiagnosticToVsCode, RustDiagnostic -} from '../utils/rust_diagnostics'; -import { - areCodeActionsEqual, - areDiagnosticsEqual -} from '../utils/vscode_diagnostics'; -import { LineBuffer } from './line_buffer'; -import { StatusDisplay } from './watch_status'; +} from '../utils/diagnostics/rust'; +import SuggestedFixCollection from '../utils/diagnostics/SuggestedFixCollection'; +import { areDiagnosticsEqual } from '../utils/diagnostics/vscode'; export function registerCargoWatchProvider( subscriptions: vscode.Disposable[] @@ -42,16 +41,13 @@ export function registerCargoWatchProvider( return provider; } -export class CargoWatchProvider - implements vscode.Disposable, vscode.CodeActionProvider { +export class CargoWatchProvider implements vscode.Disposable { private readonly diagnosticCollection: vscode.DiagnosticCollection; private readonly statusDisplay: StatusDisplay; private readonly outputChannel: vscode.OutputChannel; - private codeActions: { - [fileUri: string]: vscode.CodeAction[]; - }; - private readonly codeActionDispose: vscode.Disposable; + private suggestedFixCollection: SuggestedFixCollection; + private codeActionDispose: vscode.Disposable; private cargoProcess?: child_process.ChildProcess; @@ -66,13 +62,14 @@ export class CargoWatchProvider 'Cargo Watch Trace' ); - // Register code actions for rustc's suggested fixes - this.codeActions = {}; + // Track `rustc`'s suggested fixes so we can convert them to code actions + this.suggestedFixCollection = new SuggestedFixCollection(); this.codeActionDispose = vscode.languages.registerCodeActionsProvider( [{ scheme: 'file', language: 'rust' }], - this, + this.suggestedFixCollection, { - providedCodeActionKinds: [vscode.CodeActionKind.QuickFix] + providedCodeActionKinds: + SuggestedFixCollection.PROVIDED_CODE_ACTION_KINDS } ); } @@ -156,13 +153,6 @@ export class CargoWatchProvider this.codeActionDispose.dispose(); } - public provideCodeActions( - document: vscode.TextDocument - ): vscode.ProviderResult> { - const documentActions = this.codeActions[document.uri.toString()]; - return documentActions || []; - } - private logInfo(line: string) { if (Server.config.cargoWatchOptions.trace === 'verbose') { this.outputChannel.append(line); @@ -181,7 +171,7 @@ export class CargoWatchProvider private parseLine(line: string) { if (line.startsWith('[Running')) { this.diagnosticCollection.clear(); - this.codeActions = {}; + this.suggestedFixCollection.clear(); this.statusDisplay.show(); } @@ -225,7 +215,7 @@ export class CargoWatchProvider return; } - const { location, diagnostic, codeActions } = mapResult; + const { location, diagnostic, suggestedFixes } = mapResult; const fileUri = location.uri; const diagnostics: vscode.Diagnostic[] = [ @@ -236,7 +226,6 @@ export class CargoWatchProvider const isDuplicate = diagnostics.some(d => areDiagnosticsEqual(d, diagnostic) ); - if (isDuplicate) { return; } @@ -244,29 +233,15 @@ export class CargoWatchProvider diagnostics.push(diagnostic); this.diagnosticCollection!.set(fileUri, diagnostics); - if (codeActions.length) { - const fileUriString = fileUri.toString(); - const existingActions = this.codeActions[fileUriString] || []; - - for (const newAction of codeActions) { - const existingAction = existingActions.find(existing => - areCodeActionsEqual(existing, newAction) + if (suggestedFixes.length) { + for (const suggestedFix of suggestedFixes) { + this.suggestedFixCollection.addSuggestedFixForDiagnostic( + suggestedFix, + diagnostic ); - - if (existingAction) { - if (!existingAction.diagnostics) { - existingAction.diagnostics = []; - } - // This action also applies to this diagnostic - existingAction.diagnostics.push(diagnostic); - } else { - newAction.diagnostics = [diagnostic]; - existingActions.push(newAction); - } } // Have VsCode query us for the code actions - this.codeActions[fileUriString] = existingActions; vscode.commands.executeCommand( 'vscode.executeCodeActionProvider', fileUri, diff --git a/editors/code/src/test/rust_diagnostics.test.ts b/editors/code/src/test/rust_diagnostics.test.ts deleted file mode 100644 index f27c58fe2..000000000 --- a/editors/code/src/test/rust_diagnostics.test.ts +++ /dev/null @@ -1,162 +0,0 @@ -import * as assert from 'assert'; -import * as fs from 'fs'; -import * as vscode from 'vscode'; - -import { - MappedRustDiagnostic, - mapRustDiagnosticToVsCode, - RustDiagnostic -} from '../utils/rust_diagnostics'; - -function loadDiagnosticFixture(name: string): RustDiagnostic { - const jsonText = fs - .readFileSync( - // We're actually in our JavaScript output directory, climb out - `${__dirname}/../../src/test/fixtures/rust-diagnostics/${name}.json` - ) - .toString(); - - return JSON.parse(jsonText); -} - -function mapFixtureToVsCode(name: string): MappedRustDiagnostic { - const rd = loadDiagnosticFixture(name); - const mapResult = mapRustDiagnosticToVsCode(rd); - - if (!mapResult) { - return assert.fail('Mapping unexpectedly failed'); - } - return mapResult; -} - -describe('mapRustDiagnosticToVsCode', () => { - it('should map an incompatible type for trait error', () => { - const { diagnostic, codeActions } = mapFixtureToVsCode('error/E0053'); - - assert.strictEqual( - diagnostic.severity, - vscode.DiagnosticSeverity.Error - ); - assert.strictEqual(diagnostic.source, 'rustc'); - assert.strictEqual( - diagnostic.message, - [ - `method \`next\` has an incompatible type for trait`, - `expected type \`fn(&mut ty::list_iter::ListIterator<'list, M>) -> std::option::Option<&ty::Ref>\``, - ` found type \`fn(&ty::list_iter::ListIterator<'list, M>) -> std::option::Option<&'list ty::Ref>\`` - ].join('\n') - ); - assert.strictEqual(diagnostic.code, 'E0053'); - assert.strictEqual(diagnostic.tags, undefined); - - // No related information - assert.deepStrictEqual(diagnostic.relatedInformation, []); - - // There are no code actions available - assert.strictEqual(codeActions.length, 0); - }); - - it('should map an unused variable warning', () => { - const { diagnostic, codeActions } = mapFixtureToVsCode( - 'warning/unused_variables' - ); - - assert.strictEqual( - diagnostic.severity, - vscode.DiagnosticSeverity.Warning - ); - assert.strictEqual( - diagnostic.message, - [ - 'unused variable: `foo`', - '#[warn(unused_variables)] on by default' - ].join('\n') - ); - assert.strictEqual(diagnostic.code, 'unused_variables'); - assert.strictEqual(diagnostic.source, 'rustc'); - assert.deepStrictEqual(diagnostic.tags, [ - vscode.DiagnosticTag.Unnecessary - ]); - - // No related information - assert.deepStrictEqual(diagnostic.relatedInformation, []); - - // One code action available to prefix the variable - assert.strictEqual(codeActions.length, 1); - const [codeAction] = codeActions; - assert.strictEqual( - codeAction.title, - 'consider prefixing with an underscore: `_foo`' - ); - assert(codeAction.isPreferred); - }); - - it('should map a wrong number of parameters error', () => { - const { diagnostic, codeActions } = mapFixtureToVsCode('error/E0061'); - - assert.strictEqual( - diagnostic.severity, - vscode.DiagnosticSeverity.Error - ); - assert.strictEqual( - diagnostic.message, - 'this function takes 2 parameters but 3 parameters were supplied' - ); - assert.strictEqual(diagnostic.code, 'E0061'); - assert.strictEqual(diagnostic.source, 'rustc'); - assert.strictEqual(diagnostic.tags, undefined); - - // One related information for the original definition - const relatedInformation = diagnostic.relatedInformation; - if (!relatedInformation) { - return assert.fail('Related information unexpectedly undefined'); - } - assert.strictEqual(relatedInformation.length, 1); - const [related] = relatedInformation; - assert.strictEqual(related.message, 'defined here'); - - // There are no actions available - assert.strictEqual(codeActions.length, 0); - }); - - it('should map a Clippy copy pass by ref warning', () => { - const { diagnostic, codeActions } = mapFixtureToVsCode( - 'clippy/trivially_copy_pass_by_ref' - ); - - assert.strictEqual( - diagnostic.severity, - vscode.DiagnosticSeverity.Warning - ); - assert.strictEqual(diagnostic.source, 'clippy'); - assert.strictEqual( - diagnostic.message, - [ - 'this argument is passed by reference, but would be more efficient if passed by value', - '#[warn(clippy::trivially_copy_pass_by_ref)] implied by #[warn(clippy::all)]', - 'for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#trivially_copy_pass_by_ref' - ].join('\n') - ); - assert.strictEqual(diagnostic.code, 'trivially_copy_pass_by_ref'); - assert.strictEqual(diagnostic.tags, undefined); - - // One related information for the lint definition - const relatedInformation = diagnostic.relatedInformation; - if (!relatedInformation) { - return assert.fail('Related information unexpectedly undefined'); - } - assert.strictEqual(relatedInformation.length, 1); - const [related] = relatedInformation; - assert.strictEqual(related.message, 'lint level defined here'); - - // One code action available to pass by value - assert.strictEqual(codeActions.length, 1); - const [codeAction] = codeActions; - assert.strictEqual( - codeAction.title, - 'consider passing by value instead: `self`' - ); - // Clippy does not mark this as machine applicable - assert.strictEqual(codeAction.isPreferred, false); - }); -}); diff --git a/editors/code/src/test/utils/diagnotics/SuggestedFix.test.ts b/editors/code/src/test/utils/diagnotics/SuggestedFix.test.ts new file mode 100644 index 000000000..6c7f436f3 --- /dev/null +++ b/editors/code/src/test/utils/diagnotics/SuggestedFix.test.ts @@ -0,0 +1,133 @@ +import * as assert from 'assert'; +import * as vscode from 'vscode'; + +import { SuggestionApplicability } from '../../../utils/diagnostics/rust'; +import SuggestedFix from '../../../utils/diagnostics/SuggestedFix'; + +const location1 = new vscode.Location( + vscode.Uri.file('/file/1'), + new vscode.Range(new vscode.Position(1, 2), new vscode.Position(3, 4)) +); + +const location2 = new vscode.Location( + vscode.Uri.file('/file/2'), + new vscode.Range(new vscode.Position(5, 6), new vscode.Position(7, 8)) +); + +describe('SuggestedFix', () => { + describe('isEqual', () => { + it('should treat identical instances as equal', () => { + const suggestion1 = new SuggestedFix( + 'Replace me!', + location1, + 'With this!' + ); + + const suggestion2 = new SuggestedFix( + 'Replace me!', + location1, + 'With this!' + ); + + assert(suggestion1.isEqual(suggestion2)); + }); + + it('should treat instances with different titles as inequal', () => { + const suggestion1 = new SuggestedFix( + 'Replace me!', + location1, + 'With this!' + ); + + const suggestion2 = new SuggestedFix( + 'Not the same title!', + location1, + 'With this!' + ); + + assert(!suggestion1.isEqual(suggestion2)); + }); + + it('should treat instances with different replacements as inequal', () => { + const suggestion1 = new SuggestedFix( + 'Replace me!', + location1, + 'With this!' + ); + + const suggestion2 = new SuggestedFix( + 'Replace me!', + location1, + 'With something else!' + ); + + assert(!suggestion1.isEqual(suggestion2)); + }); + + it('should treat instances with different locations as inequal', () => { + const suggestion1 = new SuggestedFix( + 'Replace me!', + location1, + 'With this!' + ); + + const suggestion2 = new SuggestedFix( + 'Replace me!', + location2, + 'With this!' + ); + + assert(!suggestion1.isEqual(suggestion2)); + }); + + it('should treat instances with different applicability as inequal', () => { + const suggestion1 = new SuggestedFix( + 'Replace me!', + location1, + 'With this!', + SuggestionApplicability.MachineApplicable + ); + + const suggestion2 = new SuggestedFix( + 'Replace me!', + location2, + 'With this!', + SuggestionApplicability.HasPlaceholders + ); + + assert(!suggestion1.isEqual(suggestion2)); + }); + }); + + describe('toCodeAction', () => { + it('should map a simple suggestion', () => { + const suggestion = new SuggestedFix( + 'Replace me!', + location1, + 'With this!' + ); + + const codeAction = suggestion.toCodeAction(); + assert.strictEqual(codeAction.kind, vscode.CodeActionKind.QuickFix); + assert.strictEqual(codeAction.title, 'Replace me!'); + assert.strictEqual(codeAction.isPreferred, false); + + const edit = codeAction.edit; + if (!edit) { + return assert.fail('Code Action edit unexpectedly missing'); + } + + const editEntries = edit.entries(); + assert.strictEqual(editEntries.length, 1); + + const [[editUri, textEdits]] = editEntries; + assert.strictEqual(editUri.toString(), location1.uri.toString()); + + assert.strictEqual(textEdits.length, 1); + const [textEdit] = textEdits; + + assert(textEdit.range.isEqual(location1.range)); + assert.strictEqual(textEdit.newText, 'With this!'); + }); + }); +}); diff --git a/editors/code/src/test/utils/diagnotics/SuggestedFixCollection.test.ts b/editors/code/src/test/utils/diagnotics/SuggestedFixCollection.test.ts new file mode 100644 index 000000000..f0328893e --- /dev/null +++ b/editors/code/src/test/utils/diagnotics/SuggestedFixCollection.test.ts @@ -0,0 +1,125 @@ +import * as assert from 'assert'; +import * as vscode from 'vscode'; + +import SuggestedFix from '../../../utils/diagnostics/SuggestedFix'; +import SuggestedFixCollection from '../../../utils/diagnostics/SuggestedFixCollection'; + +const uri1 = vscode.Uri.file('/file/1'); +const uri2 = vscode.Uri.file('/file/2'); + +const mockDocument1 = ({ + uri: uri1 +} as unknown) as vscode.TextDocument; + +const mockDocument2 = ({ + uri: uri2 +} as unknown) as vscode.TextDocument; + +const range1 = new vscode.Range( + new vscode.Position(1, 2), + new vscode.Position(3, 4) +); +const range2 = new vscode.Range( + new vscode.Position(5, 6), + new vscode.Position(7, 8) +); + +const diagnostic1 = new vscode.Diagnostic(range1, 'First diagnostic'); +const diagnostic2 = new vscode.Diagnostic(range2, 'Second diagnostic'); + +// This is a mutable object so return a fresh instance every time +function suggestion1(): SuggestedFix { + return new SuggestedFix( + 'Replace me!', + new vscode.Location(uri1, range1), + 'With this!' + ); +} + +describe('SuggestedFixCollection', () => { + it('should add a suggestion then return it as a code action', () => { + const suggestedFixes = new SuggestedFixCollection(); + suggestedFixes.addSuggestedFixForDiagnostic(suggestion1(), diagnostic1); + + // Specify the document and range that exactly matches + const codeActions = suggestedFixes.provideCodeActions( + mockDocument1, + range1 + ); + + assert.strictEqual(codeActions.length, 1); + const [codeAction] = codeActions; + assert.strictEqual(codeAction.title, suggestion1().title); + + const { diagnostics } = codeAction; + if (!diagnostics) { + return assert.fail('Diagnostics unexpectedly missing'); + } + + assert.strictEqual(diagnostics.length, 1); + assert.strictEqual(diagnostics[0], diagnostic1); + }); + + it('should not return code actions for different ranges', () => { + const suggestedFixes = new SuggestedFixCollection(); + suggestedFixes.addSuggestedFixForDiagnostic(suggestion1(), diagnostic1); + + const codeActions = suggestedFixes.provideCodeActions( + mockDocument1, + range2 + ); + + assert(!codeActions || codeActions.length === 0); + }); + + it('should not return code actions for different documents', () => { + const suggestedFixes = new SuggestedFixCollection(); + suggestedFixes.addSuggestedFixForDiagnostic(suggestion1(), diagnostic1); + + const codeActions = suggestedFixes.provideCodeActions( + mockDocument2, + range1 + ); + + assert(!codeActions || codeActions.length === 0); + }); + + it('should not return code actions that have been cleared', () => { + const suggestedFixes = new SuggestedFixCollection(); + suggestedFixes.addSuggestedFixForDiagnostic(suggestion1(), diagnostic1); + suggestedFixes.clear(); + + const codeActions = suggestedFixes.provideCodeActions( + mockDocument1, + range1 + ); + + assert(!codeActions || codeActions.length === 0); + }); + + it('should merge identical suggestions together', () => { + const suggestedFixes = new SuggestedFixCollection(); + + // Add the same suggestion for two diagnostics + suggestedFixes.addSuggestedFixForDiagnostic(suggestion1(), diagnostic1); + suggestedFixes.addSuggestedFixForDiagnostic(suggestion1(), diagnostic2); + + const codeActions = suggestedFixes.provideCodeActions( + mockDocument1, + range1 + ); + + assert.strictEqual(codeActions.length, 1); + const [codeAction] = codeActions; + const { diagnostics } = codeAction; + + if (!diagnostics) { + return assert.fail('Diagnostics unexpectedly missing'); + } + + // We should be associated with both diagnostics + assert.strictEqual(diagnostics.length, 2); + assert.strictEqual(diagnostics[0], diagnostic1); + assert.strictEqual(diagnostics[1], diagnostic2); + }); +}); diff --git a/editors/code/src/test/utils/diagnotics/rust.test.ts b/editors/code/src/test/utils/diagnotics/rust.test.ts new file mode 100644 index 000000000..b555a4819 --- /dev/null +++ b/editors/code/src/test/utils/diagnotics/rust.test.ts @@ -0,0 +1,173 @@ +import * as assert from 'assert'; +import * as fs from 'fs'; +import * as vscode from 'vscode'; + +import { + MappedRustDiagnostic, + mapRustDiagnosticToVsCode, + RustDiagnostic, + SuggestionApplicability +} from '../../../utils/diagnostics/rust'; + +function loadDiagnosticFixture(name: string): RustDiagnostic { + const jsonText = fs + .readFileSync( + // We're actually in our JavaScript output directory, climb out + `${__dirname}/../../../../src/test/fixtures/rust-diagnostics/${name}.json` + ) + .toString(); + + return JSON.parse(jsonText); +} + +function mapFixtureToVsCode(name: string): MappedRustDiagnostic { + const rd = loadDiagnosticFixture(name); + const mapResult = mapRustDiagnosticToVsCode(rd); + + if (!mapResult) { + return assert.fail('Mapping unexpectedly failed'); + } + return mapResult; +} + +describe('mapRustDiagnosticToVsCode', () => { + it('should map an incompatible type for trait error', () => { + const { diagnostic, suggestedFixes } = mapFixtureToVsCode( + 'error/E0053' + ); + + assert.strictEqual( + diagnostic.severity, + vscode.DiagnosticSeverity.Error + ); + assert.strictEqual(diagnostic.source, 'rustc'); + assert.strictEqual( + diagnostic.message, + [ + `method \`next\` has an incompatible type for trait`, + `expected type \`fn(&mut ty::list_iter::ListIterator<'list, M>) -> std::option::Option<&ty::Ref>\``, + ` found type \`fn(&ty::list_iter::ListIterator<'list, M>) -> std::option::Option<&'list ty::Ref>\`` + ].join('\n') + ); + assert.strictEqual(diagnostic.code, 'E0053'); + assert.strictEqual(diagnostic.tags, undefined); + + // No related information + assert.deepStrictEqual(diagnostic.relatedInformation, []); + + // There are no suggested fixes + assert.strictEqual(suggestedFixes.length, 0); + }); + + it('should map an unused variable warning', () => { + const { diagnostic, suggestedFixes } = mapFixtureToVsCode( + 'warning/unused_variables' + ); + + assert.strictEqual( + diagnostic.severity, + vscode.DiagnosticSeverity.Warning + ); + assert.strictEqual( + diagnostic.message, + [ + 'unused variable: `foo`', + '#[warn(unused_variables)] on by default' + ].join('\n') + ); + assert.strictEqual(diagnostic.code, 'unused_variables'); + assert.strictEqual(diagnostic.source, 'rustc'); + assert.deepStrictEqual(diagnostic.tags, [ + vscode.DiagnosticTag.Unnecessary + ]); + + // No related information + assert.deepStrictEqual(diagnostic.relatedInformation, []); + + // One suggested fix available to prefix the variable + assert.strictEqual(suggestedFixes.length, 1); + const [suggestedFix] = suggestedFixes; + assert.strictEqual( + suggestedFix.title, + 'consider prefixing with an underscore: `_foo`' + ); + assert.strictEqual( + suggestedFix.applicability, + SuggestionApplicability.MachineApplicable + ); + }); + + it('should map a wrong number of parameters error', () => { + const { diagnostic, suggestedFixes } = mapFixtureToVsCode( + 'error/E0061' + ); + + assert.strictEqual( + diagnostic.severity, + vscode.DiagnosticSeverity.Error + ); + assert.strictEqual( + diagnostic.message, + 'this function takes 2 parameters but 3 parameters were supplied' + ); + assert.strictEqual(diagnostic.code, 'E0061'); + assert.strictEqual(diagnostic.source, 'rustc'); + assert.strictEqual(diagnostic.tags, undefined); + + // One related information for the original definition + const relatedInformation = diagnostic.relatedInformation; + if (!relatedInformation) { + return assert.fail('Related information unexpectedly undefined'); + } + assert.strictEqual(relatedInformation.length, 1); + const [related] = relatedInformation; + assert.strictEqual(related.message, 'defined here'); + + // There are no suggested fixes + assert.strictEqual(suggestedFixes.length, 0); + }); + + it('should map a Clippy copy pass by ref warning', () => { + const { diagnostic, suggestedFixes } = mapFixtureToVsCode( + 'clippy/trivially_copy_pass_by_ref' + ); + + assert.strictEqual( + diagnostic.severity, + vscode.DiagnosticSeverity.Warning + ); + assert.strictEqual(diagnostic.source, 'clippy'); + assert.strictEqual( + diagnostic.message, + [ + 'this argument is passed by reference, but would be more efficient if passed by value', + '#[warn(clippy::trivially_copy_pass_by_ref)] implied by #[warn(clippy::all)]', + 'for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#trivially_copy_pass_by_ref' + ].join('\n') + ); + assert.strictEqual(diagnostic.code, 'trivially_copy_pass_by_ref'); + assert.strictEqual(diagnostic.tags, undefined); + + // One related information for the lint definition + const relatedInformation = diagnostic.relatedInformation; + if (!relatedInformation) { + return assert.fail('Related information unexpectedly undefined'); + } + assert.strictEqual(relatedInformation.length, 1); + const [related] = relatedInformation; + assert.strictEqual(related.message, 'lint level defined here'); + + // One suggested fix to pass by value + assert.strictEqual(suggestedFixes.length, 1); + const [suggestedFix] = suggestedFixes; + assert.strictEqual( + suggestedFix.title, + 'consider passing by value instead: `self`' + ); + // Clippy does not mark this with any applicability + assert.strictEqual( + suggestedFix.applicability, + SuggestionApplicability.Unspecified + ); + }); +}); diff --git a/editors/code/src/test/utils/diagnotics/vscode.test.ts b/editors/code/src/test/utils/diagnotics/vscode.test.ts new file mode 100644 index 000000000..542dec1f5 --- /dev/null +++ b/editors/code/src/test/utils/diagnotics/vscode.test.ts @@ -0,0 +1,98 @@ +import * as assert from 'assert'; +import * as vscode from 'vscode'; + +import { areDiagnosticsEqual } from '../../../utils/diagnostics/vscode'; + +const range1 = new vscode.Range( + new vscode.Position(1, 2), + new vscode.Position(3, 4) +); + +const range2 = new vscode.Range( + new vscode.Position(5, 6), + new vscode.Position(7, 8) +); + +describe('areDiagnosticsEqual', () => { + it('should treat identical diagnostics as equal', () => { + const diagnostic1 = new vscode.Diagnostic( + range1, + 'Hello, world!', + vscode.DiagnosticSeverity.Error + ); + + const diagnostic2 = new vscode.Diagnostic( + range1, + 'Hello, world!', + vscode.DiagnosticSeverity.Error + ); + + assert(areDiagnosticsEqual(diagnostic1, diagnostic2)); + }); + + it('should treat diagnostics with different sources as inequal', () => { + const diagnostic1 = new vscode.Diagnostic( + range1, + 'Hello, world!', + vscode.DiagnosticSeverity.Error + ); + diagnostic1.source = 'rustc'; + + const diagnostic2 = new vscode.Diagnostic( + range1, + 'Hello, world!', + vscode.DiagnosticSeverity.Error + ); + diagnostic2.source = 'clippy'; + + assert(!areDiagnosticsEqual(diagnostic1, diagnostic2)); + }); + + it('should treat diagnostics with different ranges as inequal', () => { + const diagnostic1 = new vscode.Diagnostic( + range1, + 'Hello, world!', + vscode.DiagnosticSeverity.Error + ); + + const diagnostic2 = new vscode.Diagnostic( + range2, + 'Hello, world!', + vscode.DiagnosticSeverity.Error + ); + + assert(!areDiagnosticsEqual(diagnostic1, diagnostic2)); + }); + + it('should treat diagnostics with different messages as inequal', () => { + const diagnostic1 = new vscode.Diagnostic( + range1, + 'Hello, world!', + vscode.DiagnosticSeverity.Error + ); + + const diagnostic2 = new vscode.Diagnostic( + range1, + 'Goodbye!, world!', + vscode.DiagnosticSeverity.Error + ); + + assert(!areDiagnosticsEqual(diagnostic1, diagnostic2)); + }); + + it('should treat diagnostics with different severities as inequal', () => { + const diagnostic1 = new vscode.Diagnostic( + range1, + 'Hello, world!', + vscode.DiagnosticSeverity.Warning + ); + + const diagnostic2 = new vscode.Diagnostic( + range1, + 'Hello, world!', + vscode.DiagnosticSeverity.Error + ); + + assert(!areDiagnosticsEqual(diagnostic1, diagnostic2)); + }); +}); diff --git a/editors/code/src/test/vscode_diagnostics.test.ts b/editors/code/src/test/vscode_diagnostics.test.ts deleted file mode 100644 index 9c5d812fa..000000000 --- a/editors/code/src/test/vscode_diagnostics.test.ts +++ /dev/null @@ -1,182 +0,0 @@ -import * as assert from 'assert'; -import * as vscode from 'vscode'; - -import { - areCodeActionsEqual, - areDiagnosticsEqual -} from '../utils/vscode_diagnostics'; - -const uri = vscode.Uri.file('/file/1'); - -const range1 = new vscode.Range( - new vscode.Position(1, 2), - new vscode.Position(3, 4) -); - -const range2 = new vscode.Range( - new vscode.Position(5, 6), - new vscode.Position(7, 8) -); - -describe('areDiagnosticsEqual', () => { - it('should treat identical diagnostics as equal', () => { - const diagnostic1 = new vscode.Diagnostic( - range1, - 'Hello, world!', - vscode.DiagnosticSeverity.Error - ); - - const diagnostic2 = new vscode.Diagnostic( - range1, - 'Hello, world!', - vscode.DiagnosticSeverity.Error - ); - - assert(areDiagnosticsEqual(diagnostic1, diagnostic2)); - }); - - it('should treat diagnostics with different sources as inequal', () => { - const diagnostic1 = new vscode.Diagnostic( - range1, - 'Hello, world!', - vscode.DiagnosticSeverity.Error - ); - diagnostic1.source = 'rustc'; - - const diagnostic2 = new vscode.Diagnostic( - range1, - 'Hello, world!', - vscode.DiagnosticSeverity.Error - ); - diagnostic2.source = 'clippy'; - - assert(!areDiagnosticsEqual(diagnostic1, diagnostic2)); - }); - - it('should treat diagnostics with different ranges as inequal', () => { - const diagnostic1 = new vscode.Diagnostic( - range1, - 'Hello, world!', - vscode.DiagnosticSeverity.Error - ); - - const diagnostic2 = new vscode.Diagnostic( - range2, - 'Hello, world!', - vscode.DiagnosticSeverity.Error - ); - - assert(!areDiagnosticsEqual(diagnostic1, diagnostic2)); - }); - - it('should treat diagnostics with different messages as inequal', () => { - const diagnostic1 = new vscode.Diagnostic( - range1, - 'Hello, world!', - vscode.DiagnosticSeverity.Error - ); - - const diagnostic2 = new vscode.Diagnostic( - range1, - 'Goodbye!, world!', - vscode.DiagnosticSeverity.Error - ); - - assert(!areDiagnosticsEqual(diagnostic1, diagnostic2)); - }); - - it('should treat diagnostics with different severities as inequal', () => { - const diagnostic1 = new vscode.Diagnostic( - range1, - 'Hello, world!', - vscode.DiagnosticSeverity.Warning - ); - - const diagnostic2 = new vscode.Diagnostic( - range1, - 'Hello, world!', - vscode.DiagnosticSeverity.Error - ); - - assert(!areDiagnosticsEqual(diagnostic1, diagnostic2)); - }); -}); - -describe('areCodeActionsEqual', () => { - it('should treat identical actions as equal', () => { - const codeAction1 = new vscode.CodeAction( - 'Fix me!', - vscode.CodeActionKind.QuickFix - ); - - const codeAction2 = new vscode.CodeAction( - 'Fix me!', - vscode.CodeActionKind.QuickFix - ); - - const edit = new vscode.WorkspaceEdit(); - edit.replace(uri, range1, 'Replace with this'); - codeAction1.edit = edit; - codeAction2.edit = edit; - - assert(areCodeActionsEqual(codeAction1, codeAction2)); - }); - - it('should treat actions with different types as inequal', () => { - const codeAction1 = new vscode.CodeAction( - 'Fix me!', - vscode.CodeActionKind.Refactor - ); - - const codeAction2 = new vscode.CodeAction( - 'Fix me!', - vscode.CodeActionKind.QuickFix - ); - - const edit = new vscode.WorkspaceEdit(); - edit.replace(uri, range1, 'Replace with this'); - codeAction1.edit = edit; - codeAction2.edit = edit; - - assert(!areCodeActionsEqual(codeAction1, codeAction2)); - }); - - it('should treat actions with different titles as inequal', () => { - const codeAction1 = new vscode.CodeAction( - 'Fix me!', - vscode.CodeActionKind.Refactor - ); - - const codeAction2 = new vscode.CodeAction( - 'Do something different!', - vscode.CodeActionKind.Refactor - ); - - const edit = new vscode.WorkspaceEdit(); - edit.replace(uri, range1, 'Replace with this'); - codeAction1.edit = edit; - codeAction2.edit = edit; - - assert(!areCodeActionsEqual(codeAction1, codeAction2)); - }); - - it('should treat actions with different edits as inequal', () => { - const codeAction1 = new vscode.CodeAction( - 'Fix me!', - vscode.CodeActionKind.Refactor - ); - const edit1 = new vscode.WorkspaceEdit(); - edit1.replace(uri, range1, 'Replace with this'); - codeAction1.edit = edit1; - - const codeAction2 = new vscode.CodeAction( - 'Fix me!', - vscode.CodeActionKind.Refactor - ); - const edit2 = new vscode.WorkspaceEdit(); - edit2.replace(uri, range1, 'Replace with this other thing'); - codeAction2.edit = edit2; - - assert(!areCodeActionsEqual(codeAction1, codeAction2)); - }); -}); diff --git a/editors/code/src/utils/diagnostics/SuggestedFix.ts b/editors/code/src/utils/diagnostics/SuggestedFix.ts new file mode 100644 index 000000000..b1be2a225 --- /dev/null +++ b/editors/code/src/utils/diagnostics/SuggestedFix.ts @@ -0,0 +1,67 @@ +import * as vscode from 'vscode'; + +import { SuggestionApplicability } from './rust'; + +/** + * Model object for text replacements suggested by the Rust compiler + * + * This is an intermediate form between the raw `rustc` JSON and a + * `vscode.CodeAction`. It's optimised for the use-cases of + * `SuggestedFixCollection`. + */ +export default class SuggestedFix { + public readonly title: string; + public readonly location: vscode.Location; + public readonly replacement: string; + public readonly applicability: SuggestionApplicability; + + /** + * Diagnostics this suggested fix could resolve + */ + public diagnostics: vscode.Diagnostic[]; + + constructor( + title: string, + location: vscode.Location, + replacement: string, + applicability: SuggestionApplicability = SuggestionApplicability.Unspecified + ) { + this.title = title; + this.location = location; + this.replacement = replacement; + this.applicability = applicability; + this.diagnostics = []; + } + + /** + * Determines if this suggested fix is equivalent to another instance + */ + public isEqual(other: SuggestedFix): boolean { + return ( + this.title === other.title && + this.location.range.isEqual(other.location.range) && + this.replacement === other.replacement && + this.applicability === other.applicability + ); + } + + /** + * Converts this suggested fix to a VS Code Quick Fix code action + */ + public toCodeAction(): vscode.CodeAction { + const codeAction = new vscode.CodeAction( + this.title, + vscode.CodeActionKind.QuickFix + ); + + const edit = new vscode.WorkspaceEdit(); + edit.replace(this.location.uri, this.location.range, this.replacement); + codeAction.edit = edit; + + codeAction.isPreferred = + this.applicability === SuggestionApplicability.MachineApplicable; + + codeAction.diagnostics = [...this.diagnostics]; + return codeAction; + } +} diff --git a/editors/code/src/utils/diagnostics/SuggestedFixCollection.ts b/editors/code/src/utils/diagnostics/SuggestedFixCollection.ts new file mode 100644 index 000000000..3b0bf7468 --- /dev/null +++ b/editors/code/src/utils/diagnostics/SuggestedFixCollection.ts @@ -0,0 +1,74 @@ +import * as vscode from 'vscode'; +import SuggestedFix from './SuggestedFix'; + +/** + * Collection of suggested fixes across multiple documents + * + * This stores `SuggestedFix` model objects and returns them via the + * `vscode.CodeActionProvider` interface. + */ +export default class SuggestedFixCollection + implements vscode.CodeActionProvider { + public static PROVIDED_CODE_ACTION_KINDS = [vscode.CodeActionKind.QuickFix]; + + private suggestedFixes: Map; + + constructor() { + this.suggestedFixes = new Map(); + } + + /** + * Clears all suggested fixes across all documents + */ + public clear(): void { + this.suggestedFixes = new Map(); + } + + /** + * Adds a suggested fix for the given diagnostic + * + * Some suggested fixes will appear in multiple diagnostics. For example, + * forgetting a `mut` on a variable will suggest changing the delaration on + * every mutable usage site. If the suggested fix has already been added + * this method will instead associate the existing fix with the new + * diagnostic. + */ + public addSuggestedFixForDiagnostic( + suggestedFix: SuggestedFix, + diagnostic: vscode.Diagnostic + ): void { + const fileUriString = suggestedFix.location.uri.toString(); + const fileSuggestions = this.suggestedFixes.get(fileUriString) || []; + + const existingSuggestion = fileSuggestions.find(s => + s.isEqual(suggestedFix) + ); + + if (existingSuggestion) { + // The existing suggestion also applies to this new diagnostic + existingSuggestion.diagnostics.push(diagnostic); + } else { + // We haven't seen this suggestion before + suggestedFix.diagnostics.push(diagnostic); + fileSuggestions.push(suggestedFix); + } + + this.suggestedFixes.set(fileUriString, fileSuggestions); + } + + /** + * Filters suggested fixes by their document and range and converts them to + * code actions + */ + public provideCodeActions( + document: vscode.TextDocument, + range: vscode.Range + ): vscode.CodeAction[] { + const documentUriString = document.uri.toString(); + + const suggestedFixes = this.suggestedFixes.get(documentUriString); + return (suggestedFixes || []) + .filter(({ location }) => location.range.intersection(range)) + .map(suggestedEdit => suggestedEdit.toCodeAction()); + } +} diff --git a/editors/code/src/utils/diagnostics/rust.ts b/editors/code/src/utils/diagnostics/rust.ts new file mode 100644 index 000000000..d16576eb1 --- /dev/null +++ b/editors/code/src/utils/diagnostics/rust.ts @@ -0,0 +1,235 @@ +import * as path from 'path'; +import * as vscode from 'vscode'; + +import SuggestedFix from './SuggestedFix'; + +export enum SuggestionApplicability { + MachineApplicable = 'MachineApplicable', + HasPlaceholders = 'HasPlaceholders', + MaybeIncorrect = 'MaybeIncorrect', + Unspecified = 'Unspecified' +} + +// Reference: +// https://github.com/rust-lang/rust/blob/master/src/libsyntax/json.rs +export interface RustDiagnosticSpan { + line_start: number; + line_end: number; + column_start: number; + column_end: number; + is_primary: boolean; + file_name: string; + label?: string; + suggested_replacement?: string; + suggestion_applicability?: SuggestionApplicability; +} + +export interface RustDiagnostic { + spans: RustDiagnosticSpan[]; + rendered: string; + message: string; + level: string; + code?: { + code: string; + }; + children: RustDiagnostic[]; +} + +export interface MappedRustDiagnostic { + location: vscode.Location; + diagnostic: vscode.Diagnostic; + suggestedFixes: SuggestedFix[]; +} + +interface MappedRustChildDiagnostic { + related?: vscode.DiagnosticRelatedInformation; + suggestedFix?: SuggestedFix; + messageLine?: string; +} + +/** + * Converts a Rust level string to a VsCode severity + */ +function mapLevelToSeverity(s: string): vscode.DiagnosticSeverity { + if (s === 'error') { + return vscode.DiagnosticSeverity.Error; + } + if (s.startsWith('warn')) { + return vscode.DiagnosticSeverity.Warning; + } + return vscode.DiagnosticSeverity.Information; +} + +/** + * Converts a Rust span to a VsCode location + */ +function mapSpanToLocation(span: RustDiagnosticSpan): vscode.Location { + const fileName = path.join(vscode.workspace.rootPath!, span.file_name); + const fileUri = vscode.Uri.file(fileName); + + const range = new vscode.Range( + new vscode.Position(span.line_start - 1, span.column_start - 1), + new vscode.Position(span.line_end - 1, span.column_end - 1) + ); + + return new vscode.Location(fileUri, range); +} + +/** + * Converts a secondary Rust span to a VsCode related information + * + * If the span is unlabelled this will return `undefined`. + */ +function mapSecondarySpanToRelated( + span: RustDiagnosticSpan +): vscode.DiagnosticRelatedInformation | undefined { + if (!span.label) { + // Nothing to label this with + return; + } + + const location = mapSpanToLocation(span); + return new vscode.DiagnosticRelatedInformation(location, span.label); +} + +/** + * Determines if diagnostic is related to unused code + */ +function isUnusedOrUnnecessary(rd: RustDiagnostic): boolean { + if (!rd.code) { + return false; + } + + return [ + 'dead_code', + 'unknown_lints', + 'unused_attributes', + 'unused_imports', + 'unused_macros', + 'unused_variables' + ].includes(rd.code.code); +} + +/** + * Converts a Rust child diagnostic to a VsCode related information + * + * This can have three outcomes: + * + * 1. If this is no primary span this will return a `noteLine` + * 2. If there is a primary span with a suggested replacement it will return a + * `codeAction`. + * 3. If there is a primary span without a suggested replacement it will return + * a `related`. + */ +function mapRustChildDiagnostic(rd: RustDiagnostic): MappedRustChildDiagnostic { + const span = rd.spans.find(s => s.is_primary); + + if (!span) { + // `rustc` uses these spanless children as a way to print multi-line + // messages + return { messageLine: rd.message }; + } + + // If we have a primary span use its location, otherwise use the parent + const location = mapSpanToLocation(span); + + // We need to distinguish `null` from an empty string + if (span && typeof span.suggested_replacement === 'string') { + // Include our replacement in the title unless it's empty + const title = span.suggested_replacement + ? `${rd.message}: \`${span.suggested_replacement}\`` + : rd.message; + + return { + suggestedFix: new SuggestedFix( + title, + location, + span.suggested_replacement, + span.suggestion_applicability + ) + }; + } else { + const related = new vscode.DiagnosticRelatedInformation( + location, + rd.message + ); + + return { related }; + } +} + +/** + * Converts a Rust root diagnostic to VsCode form + * + * This flattens the Rust diagnostic by: + * + * 1. Creating a `vscode.Diagnostic` with the root message and primary span. + * 2. Adding any labelled secondary spans to `relatedInformation` + * 3. Categorising child diagnostics as either `SuggestedFix`es, + * `relatedInformation` or additional message lines. + * + * If the diagnostic has no primary span this will return `undefined` + */ +export function mapRustDiagnosticToVsCode( + rd: RustDiagnostic +): MappedRustDiagnostic | undefined { + const primarySpan = rd.spans.find(s => s.is_primary); + if (!primarySpan) { + return; + } + + const location = mapSpanToLocation(primarySpan); + const secondarySpans = rd.spans.filter(s => !s.is_primary); + + const severity = mapLevelToSeverity(rd.level); + + const vd = new vscode.Diagnostic(location.range, rd.message, severity); + + let source = 'rustc'; + let code = rd.code && rd.code.code; + if (code) { + // See if this is an RFC #2103 scoped lint (e.g. from Clippy) + const scopedCode = code.split('::'); + if (scopedCode.length === 2) { + [source, code] = scopedCode; + } + } + + vd.source = source; + vd.code = code; + vd.relatedInformation = []; + + for (const secondarySpan of secondarySpans) { + const related = mapSecondarySpanToRelated(secondarySpan); + if (related) { + vd.relatedInformation.push(related); + } + } + + const suggestedFixes = []; + for (const child of rd.children) { + const { related, suggestedFix, messageLine } = mapRustChildDiagnostic( + child + ); + + if (related) { + vd.relatedInformation.push(related); + } + if (suggestedFix) { + suggestedFixes.push(suggestedFix); + } + if (messageLine) { + vd.message += `\n${messageLine}`; + } + } + + if (isUnusedOrUnnecessary(rd)) { + vd.tags = [vscode.DiagnosticTag.Unnecessary]; + } + + return { + location, + diagnostic: vd, + suggestedFixes + }; +} diff --git a/editors/code/src/utils/diagnostics/vscode.ts b/editors/code/src/utils/diagnostics/vscode.ts new file mode 100644 index 000000000..d8b85b720 --- /dev/null +++ b/editors/code/src/utils/diagnostics/vscode.ts @@ -0,0 +1,14 @@ +import * as vscode from 'vscode'; + +/** Compares two `vscode.Diagnostic`s for equality */ +export function areDiagnosticsEqual( + left: vscode.Diagnostic, + right: vscode.Diagnostic +): boolean { + return ( + left.source === right.source && + left.severity === right.severity && + left.range.isEqual(right.range) && + left.message === right.message + ); +} diff --git a/editors/code/src/utils/rust_diagnostics.ts b/editors/code/src/utils/rust_diagnostics.ts deleted file mode 100644 index 3c524cb37..000000000 --- a/editors/code/src/utils/rust_diagnostics.ts +++ /dev/null @@ -1,236 +0,0 @@ -import * as path from 'path'; -import * as vscode from 'vscode'; - -// Reference: -// https://github.com/rust-lang/rust/blob/master/src/libsyntax/json.rs -export interface RustDiagnosticSpan { - line_start: number; - line_end: number; - column_start: number; - column_end: number; - is_primary: boolean; - file_name: string; - label?: string; - suggested_replacement?: string; - suggestion_applicability?: - | 'MachineApplicable' - | 'HasPlaceholders' - | 'MaybeIncorrect' - | 'Unspecified'; -} - -export interface RustDiagnostic { - spans: RustDiagnosticSpan[]; - rendered: string; - message: string; - level: string; - code?: { - code: string; - }; - children: RustDiagnostic[]; -} - -export interface MappedRustDiagnostic { - location: vscode.Location; - diagnostic: vscode.Diagnostic; - codeActions: vscode.CodeAction[]; -} - -interface MappedRustChildDiagnostic { - related?: vscode.DiagnosticRelatedInformation; - codeAction?: vscode.CodeAction; - messageLine?: string; -} - -/** - * Converts a Rust level string to a VsCode severity - */ -function mapLevelToSeverity(s: string): vscode.DiagnosticSeverity { - if (s === 'error') { - return vscode.DiagnosticSeverity.Error; - } - if (s.startsWith('warn')) { - return vscode.DiagnosticSeverity.Warning; - } - return vscode.DiagnosticSeverity.Information; -} - -/** - * Converts a Rust span to a VsCode location - */ -function mapSpanToLocation(span: RustDiagnosticSpan): vscode.Location { - const fileName = path.join(vscode.workspace.rootPath!, span.file_name); - const fileUri = vscode.Uri.file(fileName); - - const range = new vscode.Range( - new vscode.Position(span.line_start - 1, span.column_start - 1), - new vscode.Position(span.line_end - 1, span.column_end - 1) - ); - - return new vscode.Location(fileUri, range); -} - -/** - * Converts a secondary Rust span to a VsCode related information - * - * If the span is unlabelled this will return `undefined`. - */ -function mapSecondarySpanToRelated( - span: RustDiagnosticSpan -): vscode.DiagnosticRelatedInformation | undefined { - if (!span.label) { - // Nothing to label this with - return; - } - - const location = mapSpanToLocation(span); - return new vscode.DiagnosticRelatedInformation(location, span.label); -} - -/** - * Determines if diagnostic is related to unused code - */ -function isUnusedOrUnnecessary(rd: RustDiagnostic): boolean { - if (!rd.code) { - return false; - } - - return [ - 'dead_code', - 'unknown_lints', - 'unused_attributes', - 'unused_imports', - 'unused_macros', - 'unused_variables' - ].includes(rd.code.code); -} - -/** - * Converts a Rust child diagnostic to a VsCode related information - * - * This can have three outcomes: - * - * 1. If this is no primary span this will return a `noteLine` - * 2. If there is a primary span with a suggested replacement it will return a - * `codeAction`. - * 3. If there is a primary span without a suggested replacement it will return - * a `related`. - */ -function mapRustChildDiagnostic(rd: RustDiagnostic): MappedRustChildDiagnostic { - const span = rd.spans.find(s => s.is_primary); - - if (!span) { - // `rustc` uses these spanless children as a way to print multi-line - // messages - return { messageLine: rd.message }; - } - - // If we have a primary span use its location, otherwise use the parent - const location = mapSpanToLocation(span); - - // We need to distinguish `null` from an empty string - if (span && typeof span.suggested_replacement === 'string') { - const edit = new vscode.WorkspaceEdit(); - edit.replace(location.uri, location.range, span.suggested_replacement); - - // Include our replacement in the label unless it's empty - const title = span.suggested_replacement - ? `${rd.message}: \`${span.suggested_replacement}\`` - : rd.message; - - const codeAction = new vscode.CodeAction( - title, - vscode.CodeActionKind.QuickFix - ); - - codeAction.edit = edit; - codeAction.isPreferred = - span.suggestion_applicability === 'MachineApplicable'; - - return { codeAction }; - } else { - const related = new vscode.DiagnosticRelatedInformation( - location, - rd.message - ); - - return { related }; - } -} - -/** - * Converts a Rust root diagnostic to VsCode form - * - * This flattens the Rust diagnostic by: - * - * 1. Creating a `vscode.Diagnostic` with the root message and primary span. - * 2. Adding any labelled secondary spans to `relatedInformation` - * 3. Categorising child diagnostics as either Quick Fix actions, - * `relatedInformation` or additional message lines. - * - * If the diagnostic has no primary span this will return `undefined` - */ -export function mapRustDiagnosticToVsCode( - rd: RustDiagnostic -): MappedRustDiagnostic | undefined { - const codeActions = []; - - const primarySpan = rd.spans.find(s => s.is_primary); - if (!primarySpan) { - return; - } - - const location = mapSpanToLocation(primarySpan); - const secondarySpans = rd.spans.filter(s => !s.is_primary); - - const severity = mapLevelToSeverity(rd.level); - - const vd = new vscode.Diagnostic(location.range, rd.message, severity); - - let source = 'rustc'; - let code = rd.code && rd.code.code; - if (code) { - // See if this is an RFC #2103 scoped lint (e.g. from Clippy) - const scopedCode = code.split('::'); - if (scopedCode.length === 2) { - [source, code] = scopedCode; - } - } - - vd.source = source; - vd.code = code; - vd.relatedInformation = []; - - for (const secondarySpan of secondarySpans) { - const related = mapSecondarySpanToRelated(secondarySpan); - if (related) { - vd.relatedInformation.push(related); - } - } - - for (const child of rd.children) { - const { related, codeAction, messageLine } = mapRustChildDiagnostic( - child - ); - - if (related) { - vd.relatedInformation.push(related); - } - if (codeAction) { - codeActions.push(codeAction); - } - if (messageLine) { - vd.message += `\n${messageLine}`; - } - } - - if (isUnusedOrUnnecessary(rd)) { - vd.tags = [vscode.DiagnosticTag.Unnecessary]; - } - - return { - location, - diagnostic: vd, - codeActions - }; -} diff --git a/editors/code/src/utils/vscode_diagnostics.ts b/editors/code/src/utils/vscode_diagnostics.ts deleted file mode 100644 index 9d763c8d6..000000000 --- a/editors/code/src/utils/vscode_diagnostics.ts +++ /dev/null @@ -1,73 +0,0 @@ -import * as vscode from 'vscode'; - -/** Compares two `vscode.Diagnostic`s for equality */ -export function areDiagnosticsEqual( - left: vscode.Diagnostic, - right: vscode.Diagnostic -): boolean { - return ( - left.source === right.source && - left.severity === right.severity && - left.range.isEqual(right.range) && - left.message === right.message - ); -} - -/** Compares two `vscode.TextEdit`s for equality */ -function areTextEditsEqual( - left: vscode.TextEdit, - right: vscode.TextEdit -): boolean { - if (!left.range.isEqual(right.range)) { - return false; - } - - if (left.newText !== right.newText) { - return false; - } - - return true; -} - -/** Compares two `vscode.CodeAction`s for equality */ -export function areCodeActionsEqual( - left: vscode.CodeAction, - right: vscode.CodeAction -): boolean { - if ( - left.kind !== right.kind || - left.title !== right.title || - !left.edit || - !right.edit - ) { - return false; - } - - const leftEditEntries = left.edit.entries(); - const rightEditEntries = right.edit.entries(); - - if (leftEditEntries.length !== rightEditEntries.length) { - return false; - } - - for (let i = 0; i < leftEditEntries.length; i++) { - const [leftUri, leftEdits] = leftEditEntries[i]; - const [rightUri, rightEdits] = rightEditEntries[i]; - - if (leftUri.toString() !== rightUri.toString()) { - return false; - } - - if (leftEdits.length !== rightEdits.length) { - return false; - } - - for (let j = 0; j < leftEdits.length; j++) { - if (!areTextEditsEqual(leftEdits[j], rightEdits[j])) { - return false; - } - } - } - - return true; -} -- cgit v1.2.3 From 50c6ab709e38b63fb1e3ee5db40426ef131cbd71 Mon Sep 17 00:00:00 2001 From: Ryan Cumming Date: Sat, 29 Jun 2019 19:46:20 +1000 Subject: Comment on the key of `suggestedFixes` This isn't immediately obvious without looking at the users of the map --- editors/code/src/utils/diagnostics/SuggestedFixCollection.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/editors/code/src/utils/diagnostics/SuggestedFixCollection.ts b/editors/code/src/utils/diagnostics/SuggestedFixCollection.ts index 3b0bf7468..132ce12f8 100644 --- a/editors/code/src/utils/diagnostics/SuggestedFixCollection.ts +++ b/editors/code/src/utils/diagnostics/SuggestedFixCollection.ts @@ -11,6 +11,9 @@ export default class SuggestedFixCollection implements vscode.CodeActionProvider { public static PROVIDED_CODE_ACTION_KINDS = [vscode.CodeActionKind.QuickFix]; + /** + * Map of document URI strings to suggested fixes + */ private suggestedFixes: Map; constructor() { -- cgit v1.2.3