From add86c69c02071fb556a467fcea6f12e2e253bde Mon Sep 17 00:00:00 2001 From: Kalita Alexey Date: Sat, 18 Feb 2017 13:24:32 +0300 Subject: [PATCH 1/2] Fixed the problem. --- src/components/cargo/diagnostic_publisher.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/cargo/diagnostic_publisher.ts b/src/components/cargo/diagnostic_publisher.ts index 59bd7e5..aa90e64 100644 --- a/src/components/cargo/diagnostic_publisher.ts +++ b/src/components/cargo/diagnostic_publisher.ts @@ -28,7 +28,7 @@ export class DiagnosticPublisher { if (oneFileDiagnostics === undefined) { this.diagnostics.set(filePathUri, [diagnostic]); } else if (this.isUniqueDiagnostic(diagnostic, oneFileDiagnostics)) { - oneFileDiagnostics.push(diagnostic); + this.diagnostics.set(filePathUri, oneFileDiagnostics.concat([diagnostic])); } } From 27b30e0049d3d9dfc653e043aac3ca75831bd293 Mon Sep 17 00:00:00 2001 From: Kalita Alexey Date: Sat, 18 Feb 2017 16:43:58 +0300 Subject: [PATCH 2/2] Splitted DiagnosticPublisher into functions. Added tests for them --- src/components/cargo/diagnostic_publisher.ts | 50 ------ src/components/cargo/diagnostic_utils.ts | 55 +++++++ .../cargo/output_channel_task_manager.ts | 17 ++- .../components/cargo/diagnostic_utils.test.ts | 142 ++++++++++++++++++ 4 files changed, 206 insertions(+), 58 deletions(-) delete mode 100644 src/components/cargo/diagnostic_publisher.ts create mode 100644 src/components/cargo/diagnostic_utils.ts create mode 100644 test/components/cargo/diagnostic_utils.test.ts diff --git a/src/components/cargo/diagnostic_publisher.ts b/src/components/cargo/diagnostic_publisher.ts deleted file mode 100644 index aa90e64..0000000 --- a/src/components/cargo/diagnostic_publisher.ts +++ /dev/null @@ -1,50 +0,0 @@ -import { isAbsolute, join } from 'path'; - -import { Diagnostic, DiagnosticCollection, Uri, languages } from 'vscode'; - -import { FileDiagnostic } from './file_diagnostic'; - -export class DiagnosticPublisher { - private diagnostics: DiagnosticCollection; - - public constructor() { - this.diagnostics = languages.createDiagnosticCollection('rust'); - } - - public clearDiagnostics(): void { - this.diagnostics.clear(); - } - - /** - * Publishes a diagnostic if the diagnostic wasn't published yet - */ - public publishDiagnostic(fileDiagnostic: FileDiagnostic, cwd: string): void { - const { diagnostic, filePath } = fileDiagnostic; - const absoluteFilePath = isAbsolute(filePath) ? filePath : join(cwd, filePath); - const filePathUri = Uri.file(absoluteFilePath); - - const oneFileDiagnostics = this.diagnostics.get(filePathUri); - - if (oneFileDiagnostics === undefined) { - this.diagnostics.set(filePathUri, [diagnostic]); - } else if (this.isUniqueDiagnostic(diagnostic, oneFileDiagnostics)) { - this.diagnostics.set(filePathUri, oneFileDiagnostics.concat([diagnostic])); - } - } - - private isUniqueDiagnostic(diagnostic: Diagnostic, diagnostics: Diagnostic[]): boolean { - const foundDiagnostic = diagnostics.find(uniqueDiagnostic => { - if (!diagnostic.range.isEqual(uniqueDiagnostic.range)) { - return false; - } - - if (diagnostic.message !== uniqueDiagnostic.message) { - return false; - } - - return true; - }); - - return foundDiagnostic === undefined; - } -} diff --git a/src/components/cargo/diagnostic_utils.ts b/src/components/cargo/diagnostic_utils.ts new file mode 100644 index 0000000..9f6bf28 --- /dev/null +++ b/src/components/cargo/diagnostic_utils.ts @@ -0,0 +1,55 @@ +import { isAbsolute, join } from 'path'; + +import { Diagnostic, DiagnosticCollection, Uri } from 'vscode'; + +import { FileDiagnostic } from './file_diagnostic'; + +/** + * The path of a diagnostic must be absolute. + * The function prepends the path of the project to the path of the diagnostic. + * @param diagnosticPath The path of the diagnostic + * @param projectPath The path of the project + */ +export function normalizeDiagnosticPath(diagnosticPath: string, projectPath: string): string { + if (isAbsolute(diagnosticPath)) { + return diagnosticPath; + } else { + return join(projectPath, diagnosticPath); + } +} + +/** + * Adds the diagnostic to the diagnostics only if the diagnostic isn't in the diagnostics. + * @param diagnostic The diagnostic to add + * @param diagnostics The collection of diagnostics to take the diagnostic + */ +export function addUniqueDiagnostic(diagnostic: FileDiagnostic, diagnostics: DiagnosticCollection): void { + const uri = Uri.file(diagnostic.filePath); + + const fileDiagnostics = diagnostics.get(uri); + + if (fileDiagnostics === undefined) { + // No diagnostics for the file + // The diagnostic is unique + diagnostics.set(uri, [diagnostic.diagnostic]); + } else if (isUniqueDiagnostic(diagnostic.diagnostic, fileDiagnostics)) { + const newFileDiagnostics = fileDiagnostics.concat([diagnostic.diagnostic]); + diagnostics.set(uri, newFileDiagnostics); + } +} + +export function isUniqueDiagnostic(diagnostic: Diagnostic, diagnostics: Diagnostic[]): boolean { + const foundDiagnostic = diagnostics.find(uniqueDiagnostic => { + if (!diagnostic.range.isEqual(uniqueDiagnostic.range)) { + return false; + } + + if (diagnostic.message !== uniqueDiagnostic.message) { + return false; + } + + return true; + }); + + return foundDiagnostic === undefined; +} diff --git a/src/components/cargo/output_channel_task_manager.ts b/src/components/cargo/output_channel_task_manager.ts index d7e7fb5..5b08df7 100644 --- a/src/components/cargo/output_channel_task_manager.ts +++ b/src/components/cargo/output_channel_task_manager.ts @@ -1,4 +1,4 @@ -import { window } from 'vscode'; +import { DiagnosticCollection, languages, window } from 'vscode'; import { ConfigurationManager } from '../configuration/configuration_manager'; @@ -6,7 +6,7 @@ import ChildLogger from '../logging/child_logger'; import { DiagnosticParser } from './diagnostic_parser'; -import { DiagnosticPublisher } from './diagnostic_publisher'; +import { normalizeDiagnosticPath, addUniqueDiagnostic } from './diagnostic_utils'; import { OutputChannelWrapper } from './output_channel_wrapper'; @@ -23,9 +23,9 @@ export class OutputChannelTaskManager { private runningTask: Task | undefined; - private diagnosticParser: DiagnosticParser; + private diagnostics: DiagnosticCollection; - private diagnosticPublisher: DiagnosticPublisher; + private diagnosticParser: DiagnosticParser; private statusBarItem: OutputChannelTaskStatusBarItem; @@ -40,9 +40,9 @@ export class OutputChannelTaskManager { this.logger = logger; - this.diagnosticParser = new DiagnosticParser(); + this.diagnostics = languages.createDiagnosticCollection('rust'); - this.diagnosticPublisher = new DiagnosticPublisher(); + this.diagnosticParser = new DiagnosticParser(); this.statusBarItem = new OutputChannelTaskStatusBarItem(stopCommandName); } @@ -84,7 +84,7 @@ export class OutputChannelTaskManager { this.channel.clear(); this.channel.append(`Started cargo ${args.join(' ')}\n\n`); - this.diagnosticPublisher.clearDiagnostics(); + this.diagnostics.clear(); }); this.runningTask.setLineReceivedInStdout(line => { @@ -92,7 +92,8 @@ export class OutputChannelTaskManager { const fileDiagnostics = this.diagnosticParser.parseLine(line); for (const fileDiagnostic of fileDiagnostics) { - this.diagnosticPublisher.publishDiagnostic(fileDiagnostic, cwd); + fileDiagnostic.filePath = normalizeDiagnosticPath(fileDiagnostic.filePath, cwd); + addUniqueDiagnostic(fileDiagnostic, this.diagnostics); } } else { this.channel.append(`${line}\n`); diff --git a/test/components/cargo/diagnostic_utils.test.ts b/test/components/cargo/diagnostic_utils.test.ts new file mode 100644 index 0000000..ab26578 --- /dev/null +++ b/test/components/cargo/diagnostic_utils.test.ts @@ -0,0 +1,142 @@ +import * as assert from 'assert'; + +import { Diagnostic, Range, Uri, languages } from 'vscode'; + +import { addUniqueDiagnostic, isUniqueDiagnostic, normalizeDiagnosticPath } from '../../../src/components/cargo/diagnostic_utils'; + +import { FileDiagnostic } from '../../../src/components/cargo/file_diagnostic'; + +suite('Diagnostic Utils Tests', () => { + suite('normalizeDiagnosticPath', () => { + test('It works for a relative path', () => { + if (process.platform === 'win32') { + assert.equal(normalizeDiagnosticPath('src\\main.rs', 'C:\\Project'), 'C:\\Project\\src\\main.rs'); + } else { + assert.equal(normalizeDiagnosticPath('src/main.rs', '/project'), '/project/src/main.rs'); + } + }); + + test('It works for an absolute path', () => { + if (process.platform === 'win32') { + assert.equal(normalizeDiagnosticPath('C:\\Library\\src\\lib.rs', 'C:\\Project'), 'C:\\Library\\src\\lib.rs'); + } else { + assert.equal(normalizeDiagnosticPath('/library/src/lib.rs', '/project'), '/library/src/lib.rs'); + } + }); + }); + + suite('isUniqueDiagnostic', () => { + test('It returns true for empty diagnostics', () => { + const result = isUniqueDiagnostic(new Diagnostic(new Range(0, 0, 0, 0), '', undefined), []); + + assert.equal(result, true); + }); + + test('It returns true is the diagnostics do not contain any similar diagnostic', () => { + const diagnostics = [ + new Diagnostic(new Range(0, 0, 0, 0), '', undefined) + ]; + + const result = isUniqueDiagnostic(new Diagnostic(new Range(1, 2, 3, 4), 'Hello', undefined), diagnostics); + + assert.equal(result, true); + }); + + test('It returns true is the diagnostics contain a diagnostic with same range, but different message', () => { + const diagnostics = [ + new Diagnostic(new Range(0, 0, 0, 0), '', undefined) + ]; + + const result = isUniqueDiagnostic(new Diagnostic(new Range(0, 0, 0, 0), 'Hello', undefined), diagnostics); + + assert.equal(result, true); + }); + + test('It returns true is the diagnostics contain a diagnostic with same message, but different range', () => { + const diagnostics = [ + new Diagnostic(new Range(0, 0, 0, 0), 'Hello', undefined) + ]; + + const result = isUniqueDiagnostic(new Diagnostic(new Range(1, 2, 3, 4), 'Hello', undefined), diagnostics); + + assert.equal(result, true); + }); + + test('It returns false is the diagnostics contain a diagnostic with the same message and range', () => { + const diagnostics = [ + new Diagnostic(new Range(1, 2, 3, 4), 'Hello', undefined) + ]; + + const result = isUniqueDiagnostic(new Diagnostic(new Range(1, 2, 3, 4), 'Hello', undefined), diagnostics); + + assert.equal(result, false); + }); + }); + + test('addUniqueDiagnostic adds the diagnostic to the empty diagnostics', () => { + const diagnostic: FileDiagnostic = { + filePath: '/1', + diagnostic: new Diagnostic(new Range(1, 2, 3, 4), 'Hello', undefined) + }; + + const diagnostics = languages.createDiagnosticCollection('rust'); + + addUniqueDiagnostic(diagnostic, diagnostics); + + const fileDiagnostics = diagnostics.get(Uri.file('/1')); + + if (fileDiagnostics === undefined) { + assert.notEqual(fileDiagnostics, undefined); + } else { + assert.equal(fileDiagnostics.length, 1); + } + }); + + suite('addUniqueDiagnostic', () => { + test('It adds the diagnostic to the diagnostics which do not contain any similar diagnostic', () => { + const diagnostic: FileDiagnostic = { + filePath: '/1', + diagnostic: new Diagnostic(new Range(1, 2, 3, 4), 'Hello', undefined) + }; + + const diagnostics = languages.createDiagnosticCollection('rust'); + diagnostics.set(Uri.file('/1'), [ + new Diagnostic(new Range(2, 3, 3, 4), 'Hello', undefined), + new Diagnostic(new Range(1, 2, 3, 4), 'Hell', undefined) + ]); + + addUniqueDiagnostic(diagnostic, diagnostics); + + const fileDiagnostics = diagnostics.get(Uri.file('/1')); + + if (fileDiagnostics === undefined) { + assert.notEqual(fileDiagnostics, undefined); + } else { + assert.equal(fileDiagnostics.length, 3); + } + }); + + test('It does not add the diagnostic to the diagnostics which contain any similar diagnostic', () => { + const diagnostic: FileDiagnostic = { + filePath: '/1', + diagnostic: new Diagnostic(new Range(1, 2, 3, 4), 'Hello', undefined) + }; + + const diagnostics = languages.createDiagnosticCollection('rust'); + diagnostics.set(Uri.file('/1'), [ + new Diagnostic(new Range(1, 2, 3, 4), 'Hello', undefined), + new Diagnostic(new Range(1, 2, 3, 4), 'Hell', undefined) + ]); + + addUniqueDiagnostic(diagnostic, diagnostics); + + const fileDiagnostics = diagnostics.get(Uri.file('/1')); + + if (fileDiagnostics === undefined) { + assert.notEqual(fileDiagnostics, undefined); + } else { + assert.equal(fileDiagnostics.length, 2); + } + }); + }); +});