diff --git a/src/features/diagnosticsProvider.ts b/src/features/diagnosticsProvider.ts index 54d4b5c63..1d60ad878 100644 --- a/src/features/diagnosticsProvider.ts +++ b/src/features/diagnosticsProvider.ts @@ -127,12 +127,14 @@ class DiagnosticsProvider extends AbstractSupport { private _documentValidations: { [uri: string]: vscode.CancellationTokenSource } = Object.create(null); private _projectValidation: vscode.CancellationTokenSource; private _diagnostics: vscode.DiagnosticCollection; + private _suppressHiddenDiagnostics: boolean; constructor(server: OmniSharpServer, validationAdvisor: Advisor) { super(server); this._validationAdvisor = validationAdvisor; this._diagnostics = vscode.languages.createDiagnosticCollection('csharp'); + this._suppressHiddenDiagnostics = vscode.workspace.getConfiguration('csharp').get('suppressHiddenDiagnostics', true); let d1 = this._server.onPackageRestore(this._validateProject, this); let d2 = this._server.onProjectChange(this._validateProject, this); @@ -236,7 +238,7 @@ class DiagnosticsProvider extends AbstractSupport { let handle = setTimeout(async () => { try { let value = await serverUtils.codeCheck(this._server, { FileName: document.fileName }, source.token); - let quickFixes = value.QuickFixes.filter(DiagnosticsProvider._shouldInclude); + let quickFixes = value.QuickFixes; // Easy case: If there are no diagnostics in the file, we can clear it quickly. if (quickFixes.length === 0) { if (this._diagnostics.has(document.uri)) { @@ -247,8 +249,9 @@ class DiagnosticsProvider extends AbstractSupport { } // (re)set new diagnostics for this document - let diagnostics = quickFixes.map(DiagnosticsProvider._asDiagnostic); - this._diagnostics.set(document.uri, diagnostics); + let diagnosticsInFile = this._mapQuickFixesAsDiagnosticsInFile(quickFixes); + + this._diagnostics.set(document.uri, diagnosticsInFile.map(x => x.diagnostic)); } catch (error) { return; @@ -259,6 +262,12 @@ class DiagnosticsProvider extends AbstractSupport { this._documentValidations[key] = source; } + private _mapQuickFixesAsDiagnosticsInFile(quickFixes: protocol.QuickFix[]): { diagnostic: vscode.Diagnostic, fileName: string }[] { + return quickFixes + .map(quickFix => this._asDiagnosticInFileIfAny(quickFix)) + .filter(diagnosticInFile => diagnosticInFile !== undefined); + } + private _validateProject(): void { // If we've already started computing for this project, cancel that work. if (this._projectValidation) { @@ -275,25 +284,22 @@ class DiagnosticsProvider extends AbstractSupport { let value = await serverUtils.codeCheck(this._server, { FileName: null }, this._projectValidation.token); let quickFixes = value.QuickFixes - .filter(DiagnosticsProvider._shouldInclude) .sort((a, b) => a.FileName.localeCompare(b.FileName)); let entries: [vscode.Uri, vscode.Diagnostic[]][] = []; let lastEntry: [vscode.Uri, vscode.Diagnostic[]]; - for (let quickFix of quickFixes) { - - let diag = DiagnosticsProvider._asDiagnostic(quickFix); - let uri = vscode.Uri.file(quickFix.FileName); + for (let diagnosticInFile of this._mapQuickFixesAsDiagnosticsInFile(quickFixes)) { + let uri = vscode.Uri.file(diagnosticInFile.fileName); if (lastEntry && lastEntry[0].toString() === uri.toString()) { - lastEntry[1].push(diag); + lastEntry[1].push(diagnosticInFile.diagnostic); } else { // We're replacing all diagnostics in this file. Pushing an entry with undefined for // the diagnostics first ensures that the previous diagnostics for this file are // cleared. Otherwise, new entries will be merged with the old ones. entries.push([uri, undefined]); - lastEntry = [uri, [diag]]; + lastEntry = [uri, [diagnosticInFile.diagnostic]]; entries.push(lastEntry); } } @@ -319,36 +325,59 @@ class DiagnosticsProvider extends AbstractSupport { }); } - private static _shouldInclude(quickFix: protocol.QuickFix): boolean { - const config = vscode.workspace.getConfiguration('csharp'); - if (config.get('suppressHiddenDiagnostics', true)) { - return quickFix.LogLevel.toLowerCase() !== 'hidden'; - } else { - return true; + private _asDiagnosticInFileIfAny(quickFix: protocol.QuickFix): { diagnostic: vscode.Diagnostic, fileName: string } { + let display = this._getDiagnosticDisplay(quickFix, this._asDiagnosticSeverity(quickFix)); + + if (display.severity === "hidden") { + return undefined; } + + let message = `${quickFix.Text} [${quickFix.Projects.map(n => this._asProjectLabel(n)).join(', ')}]`; + + let diagnostic = new vscode.Diagnostic(toRange(quickFix), message, display.severity); + + if (display.isFadeout) { + diagnostic.tags = [vscode.DiagnosticTag.Unnecessary]; + } + + return { diagnostic: diagnostic, fileName: quickFix.FileName }; } - // --- data converter + private _getDiagnosticDisplay(quickFix: protocol.QuickFix, severity: vscode.DiagnosticSeverity | "hidden"): { severity: vscode.DiagnosticSeverity | "hidden", isFadeout: boolean } + { + // CS0162 & CS8019 => Unnused using and unreachable code. + // These hard coded values bring some goodnes of fading even when analyzers are disabled. + let isFadeout = (quickFix.Tags && !!quickFix.Tags.find(x => x.toLowerCase() == 'unnecessary')) || quickFix.Id == "CS0162" || quickFix.Id == "CS8019"; + + if (isFadeout && quickFix.LogLevel.toLowerCase() === 'hidden' || quickFix.LogLevel.toLowerCase() === 'none') { + // Theres no such thing as hidden severity in VSCode, + // however roslyn uses commonly analyzer with hidden to fade out things. + // Without this any of those doesn't fade anything in vscode. + return { severity: vscode.DiagnosticSeverity.Hint , isFadeout }; + } - private static _asDiagnostic(quickFix: protocol.QuickFix): vscode.Diagnostic { - let severity = DiagnosticsProvider._asDiagnosticSeverity(quickFix.LogLevel); - let message = `${quickFix.Text} [${quickFix.Projects.map(n => DiagnosticsProvider._asProjectLabel(n)).join(', ')}]`; - return new vscode.Diagnostic(toRange(quickFix), message, severity); + return { severity: severity, isFadeout }; } - private static _asDiagnosticSeverity(logLevel: string): vscode.DiagnosticSeverity { - switch (logLevel.toLowerCase()) { + private _asDiagnosticSeverity(quickFix: protocol.QuickFix): vscode.DiagnosticSeverity | "hidden" { + switch (quickFix.LogLevel.toLowerCase()) { case 'error': return vscode.DiagnosticSeverity.Error; case 'warning': return vscode.DiagnosticSeverity.Warning; - // info and hidden - default: + case 'info': return vscode.DiagnosticSeverity.Information; + case 'hidden': + if (this._suppressHiddenDiagnostics) { + return "hidden"; + } + return vscode.DiagnosticSeverity.Hint; + default: + return "hidden"; } } - private static _asProjectLabel(projectName: string): string { + private _asProjectLabel(projectName: string): string { const idx = projectName.indexOf('+'); return projectName.substr(idx + 1); } diff --git a/src/omnisharp/protocol.ts b/src/omnisharp/protocol.ts index 4e968a3e3..a34957001 100644 --- a/src/omnisharp/protocol.ts +++ b/src/omnisharp/protocol.ts @@ -201,6 +201,8 @@ export interface QuickFix { EndColumn: number; Text: string; Projects: string[]; + Tags: string[]; + Id: string; } export interface SymbolLocation extends QuickFix { diff --git a/test/integrationTests/codeActionRename.integration.test.ts b/test/integrationTests/codeActionRename.integration.test.ts index 98f4508fa..7685f3dfd 100644 --- a/test/integrationTests/codeActionRename.integration.test.ts +++ b/test/integrationTests/codeActionRename.integration.test.ts @@ -8,16 +8,24 @@ import * as vscode from 'vscode'; import { should, expect } from 'chai'; import { activateCSharpExtension } from './integrationHelpers'; import testAssetWorkspace from './testAssets/testAssetWorkspace'; +import * as path from 'path'; const chai = require('chai'); chai.use(require('chai-arrays')); chai.use(require('chai-fs')); suite(`Code Action Rename ${testAssetWorkspace.description}`, function () { + let fileUri: vscode.Uri; + suiteSetup(async function () { should(); await testAssetWorkspace.restore(); await activateCSharpExtension(); + + let fileName = 'A.cs'; + let projectDirectory = testAssetWorkspace.projects[0].projectDirectoryPath; + let filePath = path.join(projectDirectory, fileName); + fileUri = vscode.Uri.file(filePath) }); suiteTeardown(async () => { @@ -25,7 +33,6 @@ suite(`Code Action Rename ${testAssetWorkspace.description}`, function () { }); test("Code actions can rename and open files", async () => { - let fileUri = await testAssetWorkspace.projects[0].addFileWithContents("test.cs", "class C {}"); await vscode.commands.executeCommand("vscode.open", fileUri); let c = await vscode.commands.executeCommand("vscode.executeCodeActionProvider", fileUri, new vscode.Range(0, 7, 0, 7)) as { command: string, title: string, arguments: string[] }[]; let command = c.find( diff --git a/test/integrationTests/diagnostics.integration.test.ts b/test/integrationTests/diagnostics.integration.test.ts new file mode 100644 index 000000000..a7baf46fa --- /dev/null +++ b/test/integrationTests/diagnostics.integration.test.ts @@ -0,0 +1,60 @@ +/*--------------------------------------------------------------------------------------------- +* Copyright (c) Microsoft Corporation. All rights reserved. +* Licensed under the MIT License. See License.txt in the project root for license information. +*--------------------------------------------------------------------------------------------*/ + +import * as vscode from 'vscode'; +import * as path from 'path'; + +import { should, expect } from 'chai'; +import { activateCSharpExtension } from './integrationHelpers'; +import testAssetWorkspace from './testAssets/testAssetWorkspace'; +import poll from './poll'; + +const chai = require('chai'); +chai.use(require('chai-arrays')); +chai.use(require('chai-fs')); + +suite(`DiagnosticProvider: ${testAssetWorkspace.description}`, function () { + let fileUri: vscode.Uri; + + suiteSetup(async function () { + should(); + + await testAssetWorkspace.restore(); + await activateCSharpExtension(); + + let fileName = 'diagnostics.cs'; + let projectDirectory = testAssetWorkspace.projects[0].projectDirectoryPath; + let filePath = path.join(projectDirectory, fileName); + fileUri = vscode.Uri.file(filePath); + + await vscode.commands.executeCommand("vscode.open", fileUri); + }); + + suiteTeardown(async () => { + await testAssetWorkspace.cleanupWorkspace(); + }); + + test("Returns any diagnostics from file", async function () { + let result = await poll(() => vscode.languages.getDiagnostics(fileUri), 10*1000, 500); + expect(result.length).to.be.greaterThan(0); + }); + + test("Return unnecessary tag in case of unnesessary using", async function () { + let result = await poll(() => vscode.languages.getDiagnostics(fileUri), 15*1000, 500); + + let cs8019 = result.find(x => x.message.includes("CS8019")); + expect(cs8019).to.not.be.undefined; + expect(cs8019.tags).to.include(vscode.DiagnosticTag.Unnecessary); + }); + + test("Return fadeout diagnostics like unused usings based on roslyn analyzers", async function () { + this.skip(); // Remove this once https://github.com/OmniSharp/omnisharp-roslyn/issues/1458 is resolved. + let result = await poll(() => vscode.languages.getDiagnostics(fileUri), 15*1000, 500); + + let ide0005 = result.find(x => x.message.includes("IDE0005")); + expect(ide0005).to.not.be(undefined); + expect(ide0005.tags).to.include(vscode.DiagnosticTag.Unnecessary); + }); +}); diff --git a/test/integrationTests/poll.ts b/test/integrationTests/poll.ts index c26acdfa2..0f0b2ace4 100644 --- a/test/integrationTests/poll.ts +++ b/test/integrationTests/poll.ts @@ -2,23 +2,27 @@ * Copyright (c) Microsoft Corporation. All rights reserved. * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ - -export default async function poll(getValue: () => T, duration: number, step: number): Promise { - while (duration > 0) { + +export default async function poll(getValue: () => T, duration: number, step: number): Promise { + while (duration > 0) { let value = await getValue(); - - if (value) { - return value; - } - - await sleep(step); - - duration -= step; - } - - throw new Error("Polling did not succeed within the alotted duration."); -} - + + if(Array.isArray(value) && value.length > 0) { + return value; + } + + if (!Array.isArray(value) && value) { + return value; + } + + await sleep(step); + + duration -= step; + } + + throw new Error("Polling did not succeed within the alotted duration."); +} + async function sleep(ms = 0) { - return new Promise(r => setTimeout(r, ms)); + return new Promise(r => setTimeout(r, ms)); } \ No newline at end of file diff --git a/test/integrationTests/testAssets/singleCsproj/A.cs b/test/integrationTests/testAssets/singleCsproj/A.cs new file mode 100644 index 000000000..f9822136b --- /dev/null +++ b/test/integrationTests/testAssets/singleCsproj/A.cs @@ -0,0 +1 @@ +class C {} \ No newline at end of file diff --git a/test/integrationTests/testAssets/singleCsproj/diagnostics.cs b/test/integrationTests/testAssets/singleCsproj/diagnostics.cs new file mode 100644 index 000000000..3f932c92e --- /dev/null +++ b/test/integrationTests/testAssets/singleCsproj/diagnostics.cs @@ -0,0 +1,12 @@ +using System.IO; + +namespace Foo +{ + public class FooBar + { + public void FooBarBar() + { + var notUsed = 3; + } + } +} \ No newline at end of file diff --git a/test/integrationTests/testAssets/slnWithCsproj/.vscode/settings.json b/test/integrationTests/testAssets/slnWithCsproj/.vscode/settings.json index c80abf42f..0f37721a6 100644 --- a/test/integrationTests/testAssets/slnWithCsproj/.vscode/settings.json +++ b/test/integrationTests/testAssets/slnWithCsproj/.vscode/settings.json @@ -1,5 +1,5 @@ { "omnisharp.defaultLaunchSolution": "b_SecondInOrder_SlnFile.sln", "omnisharp.path": "latest", - "csharp.maxProjectFileCountForDiagnosticAnalysis": 1000, + "csharp.maxProjectFileCountForDiagnosticAnalysis": 1000 } \ No newline at end of file diff --git a/test/integrationTests/testAssets/slnWithCsproj/src/app/A.cs b/test/integrationTests/testAssets/slnWithCsproj/src/app/A.cs new file mode 100644 index 000000000..f9822136b --- /dev/null +++ b/test/integrationTests/testAssets/slnWithCsproj/src/app/A.cs @@ -0,0 +1 @@ +class C {} \ No newline at end of file diff --git a/test/integrationTests/testAssets/slnWithCsproj/src/app/diagnostics.cs b/test/integrationTests/testAssets/slnWithCsproj/src/app/diagnostics.cs new file mode 100644 index 000000000..3f932c92e --- /dev/null +++ b/test/integrationTests/testAssets/slnWithCsproj/src/app/diagnostics.cs @@ -0,0 +1,12 @@ +using System.IO; + +namespace Foo +{ + public class FooBar + { + public void FooBarBar() + { + var notUsed = 3; + } + } +} \ No newline at end of file