Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disable cake diagnostics endpoint for requests that don't specify a file #1107

Merged
merged 6 commits into from
Feb 12, 2018
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
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 dotnet/vscode-csharp#1830
Ravi Chande committed Feb 7, 2018
commit 31b847a39e2355e26923b80f0aa9ee2a93e16743
Original file line number Diff line number Diff line change
@@ -54,11 +54,16 @@ public virtual async Task<TResponse> 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<TResponse> HandleCore(TRequest request, IRequestHandler<TRequest, TResponse> service)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it necessary to introduce this? Task<TResponse> Handle(TRequest request) is already virtual.

{
return service.Handle(request);
}

protected virtual async Task<TRequest> TranslateRequestAsync(TRequest req)
{
var request = req as Request;
Original file line number Diff line number Diff line change
@@ -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<QuickFixResponse> HandleCore(CodeCheckRequest request, IRequestHandler<CodeCheckRequest, QuickFixResponse> service)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Override Task<TResponse> Handle(TRequest request) instead.

{
if (string.IsNullOrEmpty(request.FileName))
{
return Task.FromResult(new QuickFixResponse());
}

return service.Handle(request);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Override Task Handle(TRequest request) and call base.Handle(request) instead?

}
}
}
77 changes: 77 additions & 0 deletions tests/OmniSharp.Cake.Tests/CodeCheckFacts.cs
Original file line number Diff line number Diff line change
@@ -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<CodeCheckHandler>
{
private readonly ILogger _logger;

public CodeCheckFacts(ITestOutputHelper testOutput) : base(testOutput)
{
_logger = LoggerFactory.CreateLogger<AutoCompleteFacts>();
}

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<QuickFixResponse> FindDiagnostics(string contents, bool includeFileName)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

blank line above

{
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);
}
}
}
}