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

validation for highlighting range #2055

Merged
merged 3 commits into from
Jan 5, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
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
2 changes: 2 additions & 0 deletions src/OmniSharp.Abstractions/Models/v2/Range.cs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ public bool Contains(int line, int column)
return true;
}

public bool IsValid() => Start != null && Start.Line > -1 && Start.Column > -1 && End != null && End.Line > -1 && End.Column > -1;

public override bool Equals(object obj)
=> Equals(obj as Range);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Classification;
using Microsoft.CodeAnalysis.Text;
using Microsoft.Extensions.Logging;
using OmniSharp.Mef;
using OmniSharp.Models.SemanticHighlight;

Expand All @@ -13,10 +14,14 @@ namespace OmniSharp.Roslyn.CSharp.Services.SemanticHighlight
[OmniSharpHandler(OmniSharpEndpoints.V2.Highlight, LanguageNames.CSharp)]
public class SemanticHighlightService : IRequestHandler<SemanticHighlightRequest, SemanticHighlightResponse>
{
private readonly OmniSharpWorkspace _workspace;
private readonly ILogger<SemanticHighlightService> _logger;

[ImportingConstructor]
public SemanticHighlightService(OmniSharpWorkspace workspace)
public SemanticHighlightService(OmniSharpWorkspace workspace, ILoggerFactory loggerFactory)
{
_workspace = workspace;
_logger = loggerFactory.CreateLogger<SemanticHighlightService>();
}

public async Task<SemanticHighlightResponse> Handle(SemanticHighlightRequest request)
Expand All @@ -33,9 +38,17 @@ public async Task<SemanticHighlightResponse> Handle(SemanticHighlightRequest req
TextSpan textSpan;
if (request.Range is object)
{
var start = text.Lines.GetPosition(new LinePosition(request.Range.Start.Line, request.Range.Start.Column));
var end = text.Lines.GetPosition(new LinePosition(request.Range.End.Line, request.Range.End.Column));
textSpan = new TextSpan(start, end - start);
if (request.Range.IsValid())
{
var start = text.Lines.GetPosition(new LinePosition(request.Range.Start.Line, request.Range.Start.Column));
var end = text.Lines.GetPosition(new LinePosition(request.Range.End.Line, request.Range.End.Column));
textSpan = new TextSpan(start, end - start);
}
else
{
_logger.LogWarning($"Supplied highlight range {request.Range} in document {document.FilePath} is not valid.");
continue;
}
}
else
{
Expand Down Expand Up @@ -162,7 +175,5 @@ class ClassifiedResult
{
[ClassificationTypeNames.StaticSymbol] = SemanticHighlightModifier.Static,
};

private readonly OmniSharpWorkspace _workspace;
}
}
16 changes: 16 additions & 0 deletions tests/OmniSharp.Roslyn.CSharp.Tests/SemanticHighlightFacts.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,22 @@ public SemanticHighlightFacts(ITestOutputHelper output, SharedOmniSharpHostFixtu

protected override string EndpointName => OmniSharpEndpoints.V2.Highlight;

[Fact]
public async Task InvalidPositionDoesNotThrow()
{
var testFile = new TestFile("a.cs", @"
namespace N1
{
class C1 { int n = true; }
}
");

var line = -1;
var highlights = await GetSemanticHighlightsForLineAsync(testFile, line);

Assert.Empty(highlights);
}

[Fact]
public async Task SemanticHighlightSingleLine()
{
Expand Down