From 922ccf5b5b9d0c182c4a3dde71477e3c247d0c56 Mon Sep 17 00:00:00 2001 From: filipw Date: Tue, 9 Feb 2021 20:41:16 +0100 Subject: [PATCH 1/5] do not crash on find usages when symbol is null --- .../Services/Navigation/FindUsagesService.cs | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/OmniSharp.Roslyn.CSharp/Services/Navigation/FindUsagesService.cs b/src/OmniSharp.Roslyn.CSharp/Services/Navigation/FindUsagesService.cs index 6ac9f32c5c..94a93383b1 100644 --- a/src/OmniSharp.Roslyn.CSharp/Services/Navigation/FindUsagesService.cs +++ b/src/OmniSharp.Roslyn.CSharp/Services/Navigation/FindUsagesService.cs @@ -1,3 +1,4 @@ +using System; using System.Collections.Generic; using System.Collections.Immutable; using System.Composition; @@ -6,6 +7,7 @@ using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.FindSymbols; using Microsoft.CodeAnalysis.Text; +using Microsoft.Extensions.Logging; using OmniSharp.Helpers; using OmniSharp.Mef; using OmniSharp.Models; @@ -17,11 +19,14 @@ namespace OmniSharp.Roslyn.CSharp.Services.Navigation public class FindUsagesService : IRequestHandler { private readonly OmniSharpWorkspace _workspace; + private readonly ILogger _logger; [ImportingConstructor] - public FindUsagesService(OmniSharpWorkspace workspace) + public FindUsagesService(OmniSharpWorkspace workspace, ILoggerFactory loggerFactory) { _workspace = workspace; + _logger = loggerFactory.CreateLogger(); + } public async Task Handle(FindUsagesRequest request) @@ -37,6 +42,12 @@ public async Task Handle(FindUsagesRequest request) var sourceText = await document.GetTextAsync(); var position = sourceText.Lines.GetPosition(new LinePosition(request.Line, request.Column)); var symbol = await SymbolFinder.FindSymbolAtPositionAsync(semanticModel, position, _workspace); + if (symbol is null) + { + _logger.LogWarning($"No symbol found. File: {request.FileName}, Line: {request.Line}, Column: {request.Column}."); + return new QuickFixResponse(); + } + var definition = await SymbolFinder.FindSourceDefinitionAsync(symbol, _workspace.CurrentSolution); var usages = request.OnlyThisFile ? await SymbolFinder.FindReferencesAsync(definition ?? symbol, _workspace.CurrentSolution, ImmutableHashSet.Create(document)) @@ -60,6 +71,7 @@ public async Task Handle(FindUsagesRequest request) .OrderBy(q => q.FileName) .ThenBy(q => q.Line) .ThenBy(q => q.Column)); + } } } From b5de373b56b5c07e742f4774c4e6f5dacccbc848 Mon Sep 17 00:00:00 2001 From: filipw Date: Tue, 9 Feb 2021 20:41:30 +0100 Subject: [PATCH 2/5] added a test for find usages crash --- .../FindReferencesFacts.cs | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/tests/OmniSharp.Roslyn.CSharp.Tests/FindReferencesFacts.cs b/tests/OmniSharp.Roslyn.CSharp.Tests/FindReferencesFacts.cs index 658268ad41..f155158410 100644 --- a/tests/OmniSharp.Roslyn.CSharp.Tests/FindReferencesFacts.cs +++ b/tests/OmniSharp.Roslyn.CSharp.Tests/FindReferencesFacts.cs @@ -19,6 +19,12 @@ public FindReferencesFacts(ITestOutputHelper output, SharedOmniSharpHostFixture protected override string EndpointName => OmniSharpEndpoints.FindUsages; + // [Fact] + // public void Foo(string event) + // { + + // } + [Fact] public async Task CanFindReferencesOfLocalVariable() { @@ -53,6 +59,28 @@ public Foo(string $$s) Assert.Equal(2, usages.QuickFixes.Count()); } + [Fact] + public async Task DoesNotCrashOnInvalidReference() + { + const string code = @" + public class Foo + { + public Foo(string $$event) + { + var prop = event + 'abc'; + } + }"; + + + var exception = await Record.ExceptionAsync(async () => + { + var usages = await FindUsagesAsync(code); + Assert.Empty(usages.QuickFixes); + }); + + Assert.Null(exception); + } + [Fact] public async Task CanFindReferencesOfField() { From b0d39359750b6f16ae341e1ce6dd8210f6b787a7 Mon Sep 17 00:00:00 2001 From: filipw Date: Tue, 9 Feb 2021 20:46:25 +0100 Subject: [PATCH 3/5] converted fact into theory --- .../FindReferencesFacts.cs | 44 +++++++++---------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/tests/OmniSharp.Roslyn.CSharp.Tests/FindReferencesFacts.cs b/tests/OmniSharp.Roslyn.CSharp.Tests/FindReferencesFacts.cs index f155158410..798789fdda 100644 --- a/tests/OmniSharp.Roslyn.CSharp.Tests/FindReferencesFacts.cs +++ b/tests/OmniSharp.Roslyn.CSharp.Tests/FindReferencesFacts.cs @@ -59,28 +59,6 @@ public Foo(string $$s) Assert.Equal(2, usages.QuickFixes.Count()); } - [Fact] - public async Task DoesNotCrashOnInvalidReference() - { - const string code = @" - public class Foo - { - public Foo(string $$event) - { - var prop = event + 'abc'; - } - }"; - - - var exception = await Record.ExceptionAsync(async () => - { - var usages = await FindUsagesAsync(code); - Assert.Empty(usages.QuickFixes); - }); - - Assert.Null(exception); - } - [Fact] public async Task CanFindReferencesOfField() { @@ -465,6 +443,28 @@ public Foo Clone() { Assert.Equal("a.cs", usages.QuickFixes.ElementAt(0).FileName); } + [Theory] + [InlineData("public Foo(string $$event)")] + [InlineData("pu$$blic Foo(string s)")] + public async Task DoesNotCrashOnInvalidReference(string methodDefinition) + { + var code = @$" + public class Foo + {{ + {methodDefinition} + {{ + }} + }}"; + + var exception = await Record.ExceptionAsync(async () => + { + var usages = await FindUsagesAsync(code); + Assert.NotNull(usages); + }); + + Assert.Null(exception); + } + private Task FindUsagesAsync(string code, bool excludeDefinition = false) { return FindUsagesAsync(new[] { new TestFile("dummy.cs", code) }, false, excludeDefinition); From dd6f5c76be6ae6df1e44c63114cd463ba507319e Mon Sep 17 00:00:00 2001 From: filipw Date: Wed, 10 Feb 2021 08:52:19 +0100 Subject: [PATCH 4/5] extra logging --- .../Services/Navigation/FindUsagesService.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/OmniSharp.Roslyn.CSharp/Services/Navigation/FindUsagesService.cs b/src/OmniSharp.Roslyn.CSharp/Services/Navigation/FindUsagesService.cs index 94a93383b1..820ca33412 100644 --- a/src/OmniSharp.Roslyn.CSharp/Services/Navigation/FindUsagesService.cs +++ b/src/OmniSharp.Roslyn.CSharp/Services/Navigation/FindUsagesService.cs @@ -35,6 +35,7 @@ public async Task Handle(FindUsagesRequest request) var document = await _workspace.GetDocumentFromFullProjectModelAsync(request.FileName); if (document == null) { + _logger.LogWarning($"No document found. File: {request.FileName}."); return new QuickFixResponse(); } From 2a8c668a3c4059246adf20bc2e7ac22d63cae85e Mon Sep 17 00:00:00 2001 From: Filip W Date: Fri, 12 Feb 2021 16:45:47 +0100 Subject: [PATCH 5/5] removed unnecessary comment --- tests/OmniSharp.Roslyn.CSharp.Tests/FindReferencesFacts.cs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/tests/OmniSharp.Roslyn.CSharp.Tests/FindReferencesFacts.cs b/tests/OmniSharp.Roslyn.CSharp.Tests/FindReferencesFacts.cs index 798789fdda..583680a418 100644 --- a/tests/OmniSharp.Roslyn.CSharp.Tests/FindReferencesFacts.cs +++ b/tests/OmniSharp.Roslyn.CSharp.Tests/FindReferencesFacts.cs @@ -19,12 +19,6 @@ public FindReferencesFacts(ITestOutputHelper output, SharedOmniSharpHostFixture protected override string EndpointName => OmniSharpEndpoints.FindUsages; - // [Fact] - // public void Foo(string event) - // { - - // } - [Fact] public async Task CanFindReferencesOfLocalVariable() {