From 31b847a39e2355e26923b80f0aa9ee2a93e16743 Mon Sep 17 00:00:00 2001 From: Ravi Chande Date: Wed, 7 Feb 2018 09:12:20 -0800 Subject: [PATCH 1/6] Disable cake diagnostics endpoint for requests that don't specify a file Cake's diagnostics engine calls the generic CodeCheckService, which generates all diagnostics for the solution if called without a specific file path. This results in duplicate errors in C# projects when VS Code requests project level diagnostics. The solution is to only compute diagnostics for specified files. Fixes https://github.com/OmniSharp/omnisharp-vscode/issues/1830 --- .../RequestHandlers/CakeRequestHandler.cs | 7 +- .../Diagnostics/CodeCheckHandler.cs | 11 +++ tests/OmniSharp.Cake.Tests/CodeCheckFacts.cs | 77 +++++++++++++++++++ 3 files changed, 94 insertions(+), 1 deletion(-) create mode 100644 tests/OmniSharp.Cake.Tests/CodeCheckFacts.cs diff --git a/src/OmniSharp.Cake/Services/RequestHandlers/CakeRequestHandler.cs b/src/OmniSharp.Cake/Services/RequestHandlers/CakeRequestHandler.cs index 3bfd8b5a62..b2823dc2a1 100644 --- a/src/OmniSharp.Cake/Services/RequestHandlers/CakeRequestHandler.cs +++ b/src/OmniSharp.Cake/Services/RequestHandlers/CakeRequestHandler.cs @@ -54,11 +54,16 @@ public virtual async Task Handle(TRequest request) request = await TranslateRequestAsync(request); - var response = await service.Handle(request); + var response = await HandleCore(request, service); return await TranslateResponse(response, request); } + public virtual Task HandleCore(TRequest request, IRequestHandler service) + { + return service.Handle(request); + } + protected virtual async Task TranslateRequestAsync(TRequest req) { var request = req as Request; diff --git a/src/OmniSharp.Cake/Services/RequestHandlers/Diagnostics/CodeCheckHandler.cs b/src/OmniSharp.Cake/Services/RequestHandlers/Diagnostics/CodeCheckHandler.cs index 8a857b9422..f9d47b9ff9 100644 --- a/src/OmniSharp.Cake/Services/RequestHandlers/Diagnostics/CodeCheckHandler.cs +++ b/src/OmniSharp.Cake/Services/RequestHandlers/Diagnostics/CodeCheckHandler.cs @@ -1,4 +1,5 @@ using System.Composition; +using System.Threading.Tasks; using OmniSharp.Mef; using OmniSharp.Models; using OmniSharp.Models.CodeCheck; @@ -14,5 +15,15 @@ public CodeCheckHandler( : base(workspace) { } + + public override Task HandleCore(CodeCheckRequest request, IRequestHandler service) + { + if (string.IsNullOrEmpty(request.FileName)) + { + return Task.FromResult(new QuickFixResponse()); + } + + return service.Handle(request); + } } } diff --git a/tests/OmniSharp.Cake.Tests/CodeCheckFacts.cs b/tests/OmniSharp.Cake.Tests/CodeCheckFacts.cs new file mode 100644 index 0000000000..f41941f182 --- /dev/null +++ b/tests/OmniSharp.Cake.Tests/CodeCheckFacts.cs @@ -0,0 +1,77 @@ +using System.Collections.Generic; +using System.IO; +using System.Linq; +using System.Text; +using System.Threading.Tasks; +using Microsoft.Extensions.Logging; +using OmniSharp.Cake.Services.RequestHandlers.Diagnostics; +using OmniSharp.Models; +using OmniSharp.Models.AutoComplete; +using OmniSharp.Models.CodeCheck; +using OmniSharp.Models.UpdateBuffer; +using TestUtility; +using Xunit; +using Xunit.Abstractions; + +namespace OmniSharp.Cake.Tests +{ + public class CodeCheckFacts : CakeSingleRequestHandlerTestFixture + { + private readonly ILogger _logger; + + public CodeCheckFacts(ITestOutputHelper testOutput) : base(testOutput) + { + _logger = LoggerFactory.CreateLogger(); + } + + protected override string EndpointName => OmniSharpEndpoints.CodeCheck; + + [Fact] + public async Task ShouldProvideDiagnosticsIfRequestContainsCakeFileName() + { + const string input = @"zzz"; + + var diagnostics = await FindDiagnostics(input, includeFileName: true); + Assert.NotEmpty(diagnostics.QuickFixes); + } + + [Fact] + public async Task ShouldNotCallCodeCheckServiceIfRequestDoesNotSpecifyFileName() + { + const string input = @"zzz$$"; + + var diagnostics = await FindDiagnostics(input, includeFileName: false); + Assert.Null(diagnostics.QuickFixes); + } + + + private async Task FindDiagnostics(string contents, bool includeFileName) + { + using (var testProject = await TestAssets.Instance.GetTestProjectAsync("CakeProject", shadowCopy : false)) + using (var host = CreateOmniSharpHost(testProject.Directory)) + { + var testFile = new TestFile(Path.Combine(testProject.Directory, "build.cake"), contents); + + var request = new CodeCheckRequest + { + FileName = includeFileName ? testFile.FileName : string.Empty, + }; + + var updateBufferRequest = new UpdateBufferRequest + { + Buffer = testFile.Content.Code, + Column = request.Column, + FileName = testFile.FileName, + Line = request.Line, + FromDisk = false + }; + + await GetUpdateBufferHandler(host).Handle(updateBufferRequest); + + var requestHandler = GetRequestHandler(host); + + return await requestHandler.Handle(request); + } + } + } +} From 582c8efaaa784f0482e95ee0088f7a1943210996 Mon Sep 17 00:00:00 2001 From: Ravi Chande Date: Wed, 7 Feb 2018 09:20:12 -0800 Subject: [PATCH 2/6] Remove extra blanks --- tests/OmniSharp.Cake.Tests/CodeCheckFacts.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/OmniSharp.Cake.Tests/CodeCheckFacts.cs b/tests/OmniSharp.Cake.Tests/CodeCheckFacts.cs index f41941f182..72722f224a 100644 --- a/tests/OmniSharp.Cake.Tests/CodeCheckFacts.cs +++ b/tests/OmniSharp.Cake.Tests/CodeCheckFacts.cs @@ -43,8 +43,6 @@ public async Task ShouldNotCallCodeCheckServiceIfRequestDoesNotSpecifyFileName() var diagnostics = await FindDiagnostics(input, includeFileName: false); Assert.Null(diagnostics.QuickFixes); } - - private async Task FindDiagnostics(string contents, bool includeFileName) { using (var testProject = await TestAssets.Instance.GetTestProjectAsync("CakeProject", shadowCopy : false)) From 9616062ea749ca3af6039c1d4cceb35711b18f55 Mon Sep 17 00:00:00 2001 From: Ravi Chande Date: Wed, 7 Feb 2018 09:20:31 -0800 Subject: [PATCH 3/6] Remove unused usings --- tests/OmniSharp.Cake.Tests/CodeCheckFacts.cs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/tests/OmniSharp.Cake.Tests/CodeCheckFacts.cs b/tests/OmniSharp.Cake.Tests/CodeCheckFacts.cs index 72722f224a..c066300945 100644 --- a/tests/OmniSharp.Cake.Tests/CodeCheckFacts.cs +++ b/tests/OmniSharp.Cake.Tests/CodeCheckFacts.cs @@ -1,12 +1,8 @@ -using System.Collections.Generic; -using System.IO; -using System.Linq; -using System.Text; +using System.IO; using System.Threading.Tasks; using Microsoft.Extensions.Logging; using OmniSharp.Cake.Services.RequestHandlers.Diagnostics; using OmniSharp.Models; -using OmniSharp.Models.AutoComplete; using OmniSharp.Models.CodeCheck; using OmniSharp.Models.UpdateBuffer; using TestUtility; From b5718aea7da9818829ac5c839eaeacf3038631af Mon Sep 17 00:00:00 2001 From: Ravi Chande Date: Wed, 7 Feb 2018 14:04:49 -0800 Subject: [PATCH 4/6] CR feedback: add IsInvalid to code check handler --- .../Services/RequestHandlers/CakeRequestHandler.cs | 9 ++++----- .../RequestHandlers/Diagnostics/CodeCheckHandler.cs | 11 ++--------- tests/OmniSharp.Cake.Tests/CodeCheckFacts.cs | 2 +- 3 files changed, 7 insertions(+), 15 deletions(-) diff --git a/src/OmniSharp.Cake/Services/RequestHandlers/CakeRequestHandler.cs b/src/OmniSharp.Cake/Services/RequestHandlers/CakeRequestHandler.cs index b2823dc2a1..b42a5d272b 100644 --- a/src/OmniSharp.Cake/Services/RequestHandlers/CakeRequestHandler.cs +++ b/src/OmniSharp.Cake/Services/RequestHandlers/CakeRequestHandler.cs @@ -54,15 +54,14 @@ public virtual async Task Handle(TRequest request) request = await TranslateRequestAsync(request); - var response = await HandleCore(request, service); + var response = IsValid(request) + ? await service.Handle(request) + : default(TResponse) return await TranslateResponse(response, request); } - public virtual Task HandleCore(TRequest request, IRequestHandler service) - { - return service.Handle(request); - } + public virtual bool IsValid(TRequest request) => true; protected virtual async Task TranslateRequestAsync(TRequest req) { diff --git a/src/OmniSharp.Cake/Services/RequestHandlers/Diagnostics/CodeCheckHandler.cs b/src/OmniSharp.Cake/Services/RequestHandlers/Diagnostics/CodeCheckHandler.cs index f9d47b9ff9..930f36a6e3 100644 --- a/src/OmniSharp.Cake/Services/RequestHandlers/Diagnostics/CodeCheckHandler.cs +++ b/src/OmniSharp.Cake/Services/RequestHandlers/Diagnostics/CodeCheckHandler.cs @@ -16,14 +16,7 @@ public CodeCheckHandler( { } - public override Task HandleCore(CodeCheckRequest request, IRequestHandler service) - { - if (string.IsNullOrEmpty(request.FileName)) - { - return Task.FromResult(new QuickFixResponse()); - } - - return service.Handle(request); - } + public override bool IsValid(CodeCheckRequest request) => + !string.IsNullOrEmpty(request.FileName); } } diff --git a/tests/OmniSharp.Cake.Tests/CodeCheckFacts.cs b/tests/OmniSharp.Cake.Tests/CodeCheckFacts.cs index c066300945..859f4c0e25 100644 --- a/tests/OmniSharp.Cake.Tests/CodeCheckFacts.cs +++ b/tests/OmniSharp.Cake.Tests/CodeCheckFacts.cs @@ -37,7 +37,7 @@ public async Task ShouldNotCallCodeCheckServiceIfRequestDoesNotSpecifyFileName() const string input = @"zzz$$"; var diagnostics = await FindDiagnostics(input, includeFileName: false); - Assert.Null(diagnostics.QuickFixes); + Assert.Null(diagnostics); } private async Task FindDiagnostics(string contents, bool includeFileName) { From 842cac4b219fdee119ae92a26192f6d1b83fcd89 Mon Sep 17 00:00:00 2001 From: Ravi Chande Date: Wed, 7 Feb 2018 14:11:53 -0800 Subject: [PATCH 5/6] Add ; --- .../Services/RequestHandlers/CakeRequestHandler.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/OmniSharp.Cake/Services/RequestHandlers/CakeRequestHandler.cs b/src/OmniSharp.Cake/Services/RequestHandlers/CakeRequestHandler.cs index b42a5d272b..848afae35a 100644 --- a/src/OmniSharp.Cake/Services/RequestHandlers/CakeRequestHandler.cs +++ b/src/OmniSharp.Cake/Services/RequestHandlers/CakeRequestHandler.cs @@ -56,7 +56,7 @@ public virtual async Task Handle(TRequest request) var response = IsValid(request) ? await service.Handle(request) - : default(TResponse) + : default(TResponse); return await TranslateResponse(response, request); } From d5a1f00ab2b27ca31cf403ca15aac90380aecd24 Mon Sep 17 00:00:00 2001 From: Ravi Chande Date: Mon, 12 Feb 2018 13:03:41 -0800 Subject: [PATCH 6/6] CR feedback + changelog update --- CHANGELOG.md | 5 ++++- tests/OmniSharp.Cake.Tests/CodeCheckFacts.cs | 1 + 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 184f703870..175c78ecbd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,8 +1,11 @@ # Changelog All changes to the project will be documented in this file. +## [1.29.1] - Not Yet Released +* Fixed duplicate diagnostics in C# ([omnisharp-vscode#1830](https://github.com/OmniSharp/omnisharp-vscode/issues/1830), PR: [#1107](https://github.com/OmniSharp/omnisharp-roslyn/pull/1107)) + ## [1.29.0] - 2018-1-29 -* Updated to Roslyn 2.6.1 packages - C# 7.2 support, (PR: [#1055](https://github.com/OmniSharp/omnisharp-roslyn/pull/1055)) +* Updated to Roslyn 2.6.1 packages - C# 7.2 support (PR: [#1055](https://github.com/OmniSharp/omnisharp-roslyn/pull/1055)) * Shipped Language Server Protocol support in box. (PR: [#969](https://github.com/OmniSharp/omnisharp-roslyn/pull/969)) - Additional information and features tracked at [#968](https://github.com/OmniSharp/omnisharp-roslyn/issues/968) * Fixed locating Visual Studio with more than one installation (PR: [#1063](https://github.com/OmniSharp/omnisharp-roslyn/pull/1063)) diff --git a/tests/OmniSharp.Cake.Tests/CodeCheckFacts.cs b/tests/OmniSharp.Cake.Tests/CodeCheckFacts.cs index 859f4c0e25..79d28d7b66 100644 --- a/tests/OmniSharp.Cake.Tests/CodeCheckFacts.cs +++ b/tests/OmniSharp.Cake.Tests/CodeCheckFacts.cs @@ -39,6 +39,7 @@ public async Task ShouldNotCallCodeCheckServiceIfRequestDoesNotSpecifyFileName() var diagnostics = await FindDiagnostics(input, includeFileName: false); Assert.Null(diagnostics); } + private async Task FindDiagnostics(string contents, bool includeFileName) { using (var testProject = await TestAssets.Instance.GetTestProjectAsync("CakeProject", shadowCopy : false))