From db9cf77cd7f730d23d2785dd7f2c46c8c24803ec Mon Sep 17 00:00:00 2001 From: filipw Date: Sat, 8 Feb 2020 08:33:46 +0100 Subject: [PATCH 1/7] fixed out of bounds in LocationExtensions --- .../Helpers/LocationExtensions.cs | 35 +++++++++++++++++-- 1 file changed, 32 insertions(+), 3 deletions(-) diff --git a/src/OmniSharp.Roslyn.CSharp/Helpers/LocationExtensions.cs b/src/OmniSharp.Roslyn.CSharp/Helpers/LocationExtensions.cs index bbc1e8e082..8a7e7579c0 100644 --- a/src/OmniSharp.Roslyn.CSharp/Helpers/LocationExtensions.cs +++ b/src/OmniSharp.Roslyn.CSharp/Helpers/LocationExtensions.cs @@ -3,6 +3,7 @@ using System.Linq; using System.Threading.Tasks; using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Text; using OmniSharp.Models; namespace OmniSharp.Helpers @@ -21,13 +22,41 @@ public static QuickFix GetQuickFix(this Location location, OmniSharpWorkspace wo var documents = workspace.GetDocuments(path); var line = lineSpan.StartLinePosition.Line; - var text = location.SourceTree.GetText().Lines[line].ToString(); + var endLine = lineSpan.EndLinePosition.Line; + + string text; + SourceText sourceText = null; + + // if we have a mapped linespan and we found a corresponding document, pick that one + // otherwise use the SourceText of current location + if (lineSpan.HasMappedPath && documents != null && documents.Any()) + { + documents.First().TryGetText(out sourceText); + } + + if (sourceText == null) + { + sourceText = location.SourceTree.GetText(); + } + + // bounds check in case the mapping is incorrect, since user can put whatever line they want + if (sourceText.Lines.Count > line) + { + text = sourceText.Lines[line].ToString(); + } + else + { + var fallBackLineSpan = location.GetLineSpan(); + + // in case we fall out of bounds, we shouldn't crash, fallback to text from the unmapped span and the current file + text = location.SourceTree.GetText().Lines[fallBackLineSpan.StartLinePosition.Line].ToString(); + } return new QuickFix { - Text = text.Trim(), + Text = text, FileName = path, - Line = line, + Line = lineSpan.StartLinePosition.Line, Column = lineSpan.HasMappedPath ? 0 : lineSpan.StartLinePosition.Character, // when a #line directive maps into a separate file, assume columns (0,0) EndLine = lineSpan.EndLinePosition.Line, EndColumn = lineSpan.HasMappedPath ? 0 : lineSpan.EndLinePosition.Character, From 92689ee6d8dd1121e3f596b4426b93c75ede3bc6 Mon Sep 17 00:00:00 2001 From: filipw Date: Sat, 8 Feb 2020 08:33:58 +0100 Subject: [PATCH 2/7] extra tests in FindReferencesFacts --- .../FindReferencesFacts.cs | 43 ++++++++++++++++++- 1 file changed, 42 insertions(+), 1 deletion(-) diff --git a/tests/OmniSharp.Roslyn.CSharp.Tests/FindReferencesFacts.cs b/tests/OmniSharp.Roslyn.CSharp.Tests/FindReferencesFacts.cs index 962e16eb00..1c52921a6c 100644 --- a/tests/OmniSharp.Roslyn.CSharp.Tests/FindReferencesFacts.cs +++ b/tests/OmniSharp.Roslyn.CSharp.Tests/FindReferencesFacts.cs @@ -175,7 +175,48 @@ public FooConsumer() var usages = await FindUsagesAsync(testFiles, onlyThisFile: false); Assert.Equal(2, usages.QuickFixes.Count()); - var mappedResult = usages.QuickFixes.FirstOrDefault(x => x.Line == 0 && x.FileName == "b.cs"); + var mappedResult = usages.QuickFixes.FirstOrDefault(x => x.Line == 0 && x.FileName == "b.cs" && x.Text == "// hello"); + var regularResult = usages.QuickFixes.FirstOrDefault(x => x.Line == 3 && x.FileName == "a.cs"); + Assert.NotNull(mappedResult); + Assert.NotNull(regularResult); + + // regular result has regular postition + Assert.Equal(32, regularResult.Column); + Assert.Equal(35, regularResult.EndColumn); + + // mapped result has column 0,0 + Assert.Equal(0, mappedResult.Column); + Assert.Equal(0, mappedResult.EndColumn); + } + + [Fact] + public async Task DoesNotFallOutOfBoundsWithIncorrectMapping() + { + var testFiles = new[] + { + new TestFile("a.cs", @" + public class Foo + { + public void b$$ar() { } + } + + public class FooConsumer + { + public FooConsumer() + { +#line 100 ""b.cs"" +new Foo().bar(); +#line default + } + }"), + new TestFile("b.cs", + @"// hello") + }; + + var usages = await FindUsagesAsync(testFiles, onlyThisFile: false); + Assert.Equal(2, usages.QuickFixes.Count()); + + var mappedResult = usages.QuickFixes.FirstOrDefault(x => x.Line == 99 && x.FileName == "b.cs" && x.Text == "new Foo().bar();"); var regularResult = usages.QuickFixes.FirstOrDefault(x => x.Line == 3 && x.FileName == "a.cs"); Assert.NotNull(mappedResult); Assert.NotNull(regularResult); From a8a04443654008b2fb010c6b641a61e907786556 Mon Sep 17 00:00:00 2001 From: filipw Date: Sat, 8 Feb 2020 16:45:05 +0100 Subject: [PATCH 3/7] use local functions --- .../Helpers/LocationExtensions.cs | 76 +++++++++++-------- 1 file changed, 43 insertions(+), 33 deletions(-) diff --git a/src/OmniSharp.Roslyn.CSharp/Helpers/LocationExtensions.cs b/src/OmniSharp.Roslyn.CSharp/Helpers/LocationExtensions.cs index 8a7e7579c0..9dfed2d967 100644 --- a/src/OmniSharp.Roslyn.CSharp/Helpers/LocationExtensions.cs +++ b/src/OmniSharp.Roslyn.CSharp/Helpers/LocationExtensions.cs @@ -1,4 +1,5 @@ using System; +using System.Collections.Generic; using System.IO; using System.Linq; using System.Threading.Tasks; @@ -18,50 +19,59 @@ public static QuickFix GetQuickFix(this Location location, OmniSharpWorkspace wo var lineSpan = Path.GetExtension(location.SourceTree.FilePath).Equals(".cake", StringComparison.OrdinalIgnoreCase) ? location.GetLineSpan() : location.GetMappedLineSpan(); - var path = lineSpan.Path; - var documents = workspace.GetDocuments(path); - var line = lineSpan.StartLinePosition.Line; - var endLine = lineSpan.EndLinePosition.Line; - - string text; - SourceText sourceText = null; - - // if we have a mapped linespan and we found a corresponding document, pick that one - // otherwise use the SourceText of current location - if (lineSpan.HasMappedPath && documents != null && documents.Any()) - { - documents.First().TryGetText(out sourceText); - } - - if (sourceText == null) - { - sourceText = location.SourceTree.GetText(); - } - - // bounds check in case the mapping is incorrect, since user can put whatever line they want - if (sourceText.Lines.Count > line) - { - text = sourceText.Lines[line].ToString(); - } - else - { - var fallBackLineSpan = location.GetLineSpan(); - - // in case we fall out of bounds, we shouldn't crash, fallback to text from the unmapped span and the current file - text = location.SourceTree.GetText().Lines[fallBackLineSpan.StartLinePosition.Line].ToString(); - } + var documents = workspace.GetDocuments(lineSpan.Path); + var sourceText = GetSourceText(location, documents, lineSpan.HasMappedPath); + var text = GetText(location, sourceText, lineSpan.StartLinePosition.Line); return new QuickFix { Text = text, - FileName = path, + FileName = lineSpan.Path, Line = lineSpan.StartLinePosition.Line, Column = lineSpan.HasMappedPath ? 0 : lineSpan.StartLinePosition.Character, // when a #line directive maps into a separate file, assume columns (0,0) EndLine = lineSpan.EndLinePosition.Line, EndColumn = lineSpan.HasMappedPath ? 0 : lineSpan.EndLinePosition.Character, Projects = documents.Select(document => document.Project.Name).ToArray() }; + + static SourceText GetSourceText(Location location, IEnumerable documents, bool hasMappedPath) + { + SourceText sourceText = null; + + // if we have a mapped linespan and we found a corresponding document, pick that one + // otherwise use the SourceText of current location + if (hasMappedPath && documents != null && documents.Any()) + { + documents.First().TryGetText(out sourceText); + } + + if (sourceText == null) + { + sourceText = location.SourceTree.GetText(); + } + + return sourceText; + } + + static string GetText(Location location, SourceText sourceText, int startLine) + { + string text; + // bounds check in case the mapping is incorrect, since user can put whatever line they want + if (sourceText.Lines.Count > startLine) + { + text = sourceText.Lines[startLine].ToString(); + } + else + { + var fallBackLineSpan = location.GetLineSpan(); + + // in case we fall out of bounds, we shouldn't crash, fallback to text from the unmapped span and the current file + text = location.SourceTree.GetText().Lines[fallBackLineSpan.StartLinePosition.Line].ToString(); + } + + return text; + } } } } From 26a6dd6b7da5d4e3fc163c3c0962aa412d54e8f9 Mon Sep 17 00:00:00 2001 From: filipw Date: Sat, 8 Feb 2020 17:04:16 +0100 Subject: [PATCH 4/7] improved tests --- .../Helpers/LocationExtensions.cs | 38 ++++----- .../FindReferencesFacts.cs | 83 +++++++------------ 2 files changed, 43 insertions(+), 78 deletions(-) diff --git a/src/OmniSharp.Roslyn.CSharp/Helpers/LocationExtensions.cs b/src/OmniSharp.Roslyn.CSharp/Helpers/LocationExtensions.cs index 9dfed2d967..a9023329b1 100644 --- a/src/OmniSharp.Roslyn.CSharp/Helpers/LocationExtensions.cs +++ b/src/OmniSharp.Roslyn.CSharp/Helpers/LocationExtensions.cs @@ -21,8 +21,8 @@ public static QuickFix GetQuickFix(this Location location, OmniSharpWorkspace wo : location.GetMappedLineSpan(); var documents = workspace.GetDocuments(lineSpan.Path); - var sourceText = GetSourceText(location, documents, lineSpan.HasMappedPath); - var text = GetText(location, sourceText, lineSpan.StartLinePosition.Line); + (var sourceText, var isMappedDocument) = GetSourceText(location, documents, lineSpan.HasMappedPath); + var text = GetText(location, sourceText, lineSpan.StartLinePosition.Line, isMappedDocument); return new QuickFix { @@ -35,42 +35,32 @@ public static QuickFix GetQuickFix(this Location location, OmniSharpWorkspace wo Projects = documents.Select(document => document.Project.Name).ToArray() }; - static SourceText GetSourceText(Location location, IEnumerable documents, bool hasMappedPath) + static (SourceText sourceText, bool isMappedDocument) GetSourceText(Location location, IEnumerable documents, bool hasMappedPath) { - SourceText sourceText = null; - // if we have a mapped linespan and we found a corresponding document, pick that one // otherwise use the SourceText of current location if (hasMappedPath && documents != null && documents.Any()) { - documents.First().TryGetText(out sourceText); - } - - if (sourceText == null) - { - sourceText = location.SourceTree.GetText(); + if (documents.First().TryGetText(out SourceText sourceText)) + { + return (sourceText, true); + } } - return sourceText; + return (location.SourceTree.GetText(), false); } - static string GetText(Location location, SourceText sourceText, int startLine) + static string GetText(Location location, SourceText sourceText, int startLine, bool isMappedDocument) { - string text; // bounds check in case the mapping is incorrect, since user can put whatever line they want - if (sourceText.Lines.Count > startLine) + if (isMappedDocument && sourceText.Lines.Count > startLine) { - text = sourceText.Lines[startLine].ToString(); - } - else - { - var fallBackLineSpan = location.GetLineSpan(); - - // in case we fall out of bounds, we shouldn't crash, fallback to text from the unmapped span and the current file - text = location.SourceTree.GetText().Lines[fallBackLineSpan.StartLinePosition.Line].ToString(); + return sourceText.Lines[startLine].ToString(); } - return text; + // in case we fall out of bounds, we shouldn't crash, fallback to text from the unmapped span and the current file + var fallBackLineSpan = location.GetLineSpan(); + return location.SourceTree.GetText().Lines[fallBackLineSpan.StartLinePosition.Line].ToString(); } } } diff --git a/tests/OmniSharp.Roslyn.CSharp.Tests/FindReferencesFacts.cs b/tests/OmniSharp.Roslyn.CSharp.Tests/FindReferencesFacts.cs index 1c52921a6c..09013dd94d 100644 --- a/tests/OmniSharp.Roslyn.CSharp.Tests/FindReferencesFacts.cs +++ b/tests/OmniSharp.Roslyn.CSharp.Tests/FindReferencesFacts.cs @@ -1,4 +1,5 @@ -using System.Linq; +using System.Collections.Generic; +using System.Linq; using System.Threading.Tasks; using OmniSharp.Models; using OmniSharp.Models.FindUsages; @@ -148,82 +149,56 @@ public FooConsumer() Assert.Equal(35, regularResult.EndColumn); } - [Fact] - public async Task CanFindReferencesWithLineMappingAcrossFiles() + [Theory] + [InlineData(1, "// hello", true)] // everything correct + [InlineData(100, "new Foo().bar();", true)] // file exists in workspace but mapping incorrect + [InlineData(1, "new Foo().bar();", false)] // file doesn't exist in workspace but mapping correct + public async Task CanFindReferencesWithLineMappingAcrossFiles(int mappingLine, string expectedMappingText, bool mappedFileExistsInWorkspace) { - var testFiles = new[] + var testFiles = new List() { new TestFile("a.cs", @" public class Foo { - public void b$$ar() { } +public void b$$ar() { } } public class FooConsumer { public FooConsumer() { -#line 1 ""b.cs"" - new Foo().bar(); +#line "+mappingLine+@" ""b.cs"" +new Foo().bar(); #line default } }"), - new TestFile("b.cs", - @"// hello") + }; - var usages = await FindUsagesAsync(testFiles, onlyThisFile: false); - Assert.Equal(2, usages.QuickFixes.Count()); - - var mappedResult = usages.QuickFixes.FirstOrDefault(x => x.Line == 0 && x.FileName == "b.cs" && x.Text == "// hello"); - var regularResult = usages.QuickFixes.FirstOrDefault(x => x.Line == 3 && x.FileName == "a.cs"); - Assert.NotNull(mappedResult); - Assert.NotNull(regularResult); - - // regular result has regular postition - Assert.Equal(32, regularResult.Column); - Assert.Equal(35, regularResult.EndColumn); + if (mappedFileExistsInWorkspace) + { + testFiles.Add(new TestFile("b.cs", + @"// hello")); + } - // mapped result has column 0,0 - Assert.Equal(0, mappedResult.Column); - Assert.Equal(0, mappedResult.EndColumn); - } + var usages = await FindUsagesAsync(testFiles.ToArray(), onlyThisFile: false); + Assert.Equal(2, usages.QuickFixes.Count()); - [Fact] - public async Task DoesNotFallOutOfBoundsWithIncorrectMapping() - { - var testFiles = new[] - { - new TestFile("a.cs", @" - public class Foo - { - public void b$$ar() { } - } + var regularResult = usages.QuickFixes.ElementAt(0); + var mappedResult = usages.QuickFixes.ElementAt(1); - public class FooConsumer - { - public FooConsumer() - { -#line 100 ""b.cs"" -new Foo().bar(); -#line default - } - }"), - new TestFile("b.cs", - @"// hello") - }; + Assert.Equal("a.cs", regularResult.FileName); + Assert.Equal("b.cs", mappedResult.FileName); - var usages = await FindUsagesAsync(testFiles, onlyThisFile: false); - Assert.Equal(2, usages.QuickFixes.Count()); + Assert.Equal(3, regularResult.Line); + Assert.Equal(mappingLine-1, mappedResult.Line); - var mappedResult = usages.QuickFixes.FirstOrDefault(x => x.Line == 99 && x.FileName == "b.cs" && x.Text == "new Foo().bar();"); - var regularResult = usages.QuickFixes.FirstOrDefault(x => x.Line == 3 && x.FileName == "a.cs"); - Assert.NotNull(mappedResult); - Assert.NotNull(regularResult); + Assert.Equal("public void bar() { }", regularResult.Text); + Assert.Equal(expectedMappingText, mappedResult.Text); // regular result has regular postition - Assert.Equal(32, regularResult.Column); - Assert.Equal(35, regularResult.EndColumn); + Assert.Equal(12, regularResult.Column); + Assert.Equal(15, regularResult.EndColumn); // mapped result has column 0,0 Assert.Equal(0, mappedResult.Column); From f23b414bc0890e203aecb793edf4584f3f7f0293 Mon Sep 17 00:00:00 2001 From: filipw Date: Mon, 10 Feb 2020 16:10:53 +0100 Subject: [PATCH 5/7] cleaned up the logic a bit and added more tests --- .../Helpers/LocationExtensions.cs | 24 ++++++----- .../FindReferencesFacts.cs | 40 +++++++++++++------ 2 files changed, 42 insertions(+), 22 deletions(-) diff --git a/src/OmniSharp.Roslyn.CSharp/Helpers/LocationExtensions.cs b/src/OmniSharp.Roslyn.CSharp/Helpers/LocationExtensions.cs index a9023329b1..03d85adaa9 100644 --- a/src/OmniSharp.Roslyn.CSharp/Helpers/LocationExtensions.cs +++ b/src/OmniSharp.Roslyn.CSharp/Helpers/LocationExtensions.cs @@ -21,8 +21,8 @@ public static QuickFix GetQuickFix(this Location location, OmniSharpWorkspace wo : location.GetMappedLineSpan(); var documents = workspace.GetDocuments(lineSpan.Path); - (var sourceText, var isMappedDocument) = GetSourceText(location, documents, lineSpan.HasMappedPath); - var text = GetText(location, sourceText, lineSpan.StartLinePosition.Line, isMappedDocument); + var sourceText = GetSourceText(location, documents, lineSpan.HasMappedPath); + var text = GetText(location, sourceText, lineSpan.StartLinePosition.Line); return new QuickFix { @@ -35,25 +35,31 @@ public static QuickFix GetQuickFix(this Location location, OmniSharpWorkspace wo Projects = documents.Select(document => document.Project.Name).ToArray() }; - static (SourceText sourceText, bool isMappedDocument) GetSourceText(Location location, IEnumerable documents, bool hasMappedPath) + static SourceText GetSourceText(Location location, IEnumerable documents, bool hasMappedPath) { // if we have a mapped linespan and we found a corresponding document, pick that one // otherwise use the SourceText of current location - if (hasMappedPath && documents != null && documents.Any()) + if (hasMappedPath) { - if (documents.First().TryGetText(out SourceText sourceText)) + if (documents != null && documents.Any() && documents.First().TryGetText(out SourceText sourceText)) { - return (sourceText, true); + // we have a mapped document that exists in workspace + return sourceText; } + + // we have a mapped document that doesn't exist in workspace + // in that case we have to always fall back to original linespan + return null; } - return (location.SourceTree.GetText(), false); + // unmapped document so just continue with current SourceText + return location.SourceTree.GetText(); } - static string GetText(Location location, SourceText sourceText, int startLine, bool isMappedDocument) + static string GetText(Location location, SourceText sourceText, int startLine) { // bounds check in case the mapping is incorrect, since user can put whatever line they want - if (isMappedDocument && sourceText.Lines.Count > startLine) + if (sourceText != null && sourceText.Lines.Count > startLine) { return sourceText.Lines[startLine].ToString(); } diff --git a/tests/OmniSharp.Roslyn.CSharp.Tests/FindReferencesFacts.cs b/tests/OmniSharp.Roslyn.CSharp.Tests/FindReferencesFacts.cs index 09013dd94d..d438fde1ae 100644 --- a/tests/OmniSharp.Roslyn.CSharp.Tests/FindReferencesFacts.cs +++ b/tests/OmniSharp.Roslyn.CSharp.Tests/FindReferencesFacts.cs @@ -117,21 +117,23 @@ public FooConsumer() Assert.Equal(2, usages.QuickFixes.Count()); } - [Fact] - public async Task CanFindReferencesWithLineMapping() + [Theory] + [InlineData(9, "public FooConsumer()")] + [InlineData(100, "new Foo().bar();")] + public async Task CanFindReferencesWithLineMapping(int mappingLine, string expectedMappingText) { - const string code = @" + var code = @" public class Foo { - public void b$$ar() { } +public void b$$ar() { } } public class FooConsumer { - public FooConsumer() +public FooConsumer() { -#line 1 - new Foo().bar(); +#line "+mappingLine+@" +new Foo().bar(); #line default } }"; @@ -139,14 +141,26 @@ public FooConsumer() var usages = await FindUsagesAsync(code); Assert.Equal(2, usages.QuickFixes.Count()); - var mappedResult = usages.QuickFixes.FirstOrDefault(x => x.Line == 0); - var regularResult = usages.QuickFixes.FirstOrDefault(x => x.Line == 3); - Assert.NotNull(mappedResult); - Assert.NotNull(regularResult); + var quickFixes = usages.QuickFixes.OrderBy(x => x.Line); + var regularResult = quickFixes.ElementAt(0); + var mappedResult = quickFixes.ElementAt(1); + + Assert.Equal("dummy.cs", regularResult.FileName); + Assert.Equal("dummy.cs", mappedResult.FileName); + + Assert.Equal("public void bar() { }", regularResult.Text); + Assert.Equal(expectedMappingText, mappedResult.Text); + + Assert.Equal(3, regularResult.Line); + Assert.Equal(mappingLine-1, mappedResult.Line); // regular result has regular postition - Assert.Equal(32, regularResult.Column); - Assert.Equal(35, regularResult.EndColumn); + Assert.Equal(12, regularResult.Column); + Assert.Equal(15, regularResult.EndColumn); + + // mapped result has column 0,0 + Assert.Equal(10, mappedResult.Column); + Assert.Equal(13, mappedResult.EndColumn); } [Theory] From 1231d50e4a4a9f286a5848f8f78b44efd4ed4785 Mon Sep 17 00:00:00 2001 From: filipw Date: Mon, 10 Feb 2020 20:14:54 +0100 Subject: [PATCH 6/7] code review changes --- .../Helpers/LocationExtensions.cs | 11 +-- .../FindReferencesFacts.cs | 71 +++++++++++++++---- 2 files changed, 65 insertions(+), 17 deletions(-) diff --git a/src/OmniSharp.Roslyn.CSharp/Helpers/LocationExtensions.cs b/src/OmniSharp.Roslyn.CSharp/Helpers/LocationExtensions.cs index 03d85adaa9..b2fd902cb7 100644 --- a/src/OmniSharp.Roslyn.CSharp/Helpers/LocationExtensions.cs +++ b/src/OmniSharp.Roslyn.CSharp/Helpers/LocationExtensions.cs @@ -22,11 +22,11 @@ public static QuickFix GetQuickFix(this Location location, OmniSharpWorkspace wo var documents = workspace.GetDocuments(lineSpan.Path); var sourceText = GetSourceText(location, documents, lineSpan.HasMappedPath); - var text = GetText(location, sourceText, lineSpan.StartLinePosition.Line); + var text = GetLineText(location, sourceText, lineSpan.StartLinePosition.Line); return new QuickFix { - Text = text, + Text = text.Trim(), FileName = lineSpan.Path, Line = lineSpan.StartLinePosition.Line, Column = lineSpan.HasMappedPath ? 0 : lineSpan.StartLinePosition.Character, // when a #line directive maps into a separate file, assume columns (0,0) @@ -41,10 +41,11 @@ static SourceText GetSourceText(Location location, IEnumerable documen // otherwise use the SourceText of current location if (hasMappedPath) { - if (documents != null && documents.Any() && documents.First().TryGetText(out SourceText sourceText)) + SourceText source = null; + if (documents != null && documents.Any() && documents.FirstOrDefault(d => d.TryGetText(out source)) != null) { // we have a mapped document that exists in workspace - return sourceText; + return source; } // we have a mapped document that doesn't exist in workspace @@ -56,7 +57,7 @@ static SourceText GetSourceText(Location location, IEnumerable documen return location.SourceTree.GetText(); } - static string GetText(Location location, SourceText sourceText, int startLine) + static string GetLineText(Location location, SourceText sourceText, int startLine) { // bounds check in case the mapping is incorrect, since user can put whatever line they want if (sourceText != null && sourceText.Lines.Count > startLine) diff --git a/tests/OmniSharp.Roslyn.CSharp.Tests/FindReferencesFacts.cs b/tests/OmniSharp.Roslyn.CSharp.Tests/FindReferencesFacts.cs index d438fde1ae..658268ad41 100644 --- a/tests/OmniSharp.Roslyn.CSharp.Tests/FindReferencesFacts.cs +++ b/tests/OmniSharp.Roslyn.CSharp.Tests/FindReferencesFacts.cs @@ -125,15 +125,15 @@ public async Task CanFindReferencesWithLineMapping(int mappingLine, string expec var code = @" public class Foo { -public void b$$ar() { } + public void b$$ar() { } } public class FooConsumer { -public FooConsumer() + public FooConsumer() { -#line "+mappingLine+@" -new Foo().bar(); +#line " + mappingLine+ @" + new Foo().bar(); #line default } }"; @@ -155,12 +155,12 @@ public FooConsumer() Assert.Equal(mappingLine-1, mappedResult.Line); // regular result has regular postition - Assert.Equal(12, regularResult.Column); - Assert.Equal(15, regularResult.EndColumn); + Assert.Equal(32, regularResult.Column); + Assert.Equal(35, regularResult.EndColumn); // mapped result has column 0,0 - Assert.Equal(10, mappedResult.Column); - Assert.Equal(13, mappedResult.EndColumn); + Assert.Equal(34, mappedResult.Column); + Assert.Equal(37, mappedResult.EndColumn); } [Theory] @@ -174,7 +174,7 @@ public async Task CanFindReferencesWithLineMappingAcrossFiles(int mappingLine, s new TestFile("a.cs", @" public class Foo { -public void b$$ar() { } + public void b$$ar() { } } public class FooConsumer @@ -182,7 +182,7 @@ public class FooConsumer public FooConsumer() { #line "+mappingLine+@" ""b.cs"" -new Foo().bar(); + new Foo().bar(); #line default } }"), @@ -211,14 +211,61 @@ public FooConsumer() Assert.Equal(expectedMappingText, mappedResult.Text); // regular result has regular postition - Assert.Equal(12, regularResult.Column); - Assert.Equal(15, regularResult.EndColumn); + Assert.Equal(32, regularResult.Column); + Assert.Equal(35, regularResult.EndColumn); // mapped result has column 0,0 Assert.Equal(0, mappedResult.Column); Assert.Equal(0, mappedResult.EndColumn); } + [Fact] + public async Task CanFindReferencesWithNegativeLineMapping() + { + var testFiles = new List() + { + new TestFile("a.cs", @" + public class Foo + { + public void b$$ar() { } + } + + public class FooConsumer + { + public FooConsumer() + { +#line -10 ""b.cs"" + new Foo().bar(); +#line default + } + }"), + + }; + + testFiles.Add(new TestFile("b.cs", + @"// hello")); + + var usages = await FindUsagesAsync(testFiles.ToArray(), onlyThisFile: false); + Assert.Equal(2, usages.QuickFixes.Count()); + + var regularResult = usages.QuickFixes.ElementAt(0); + var mappedResult = usages.QuickFixes.ElementAt(1); + + Assert.Equal("a.cs", regularResult.FileName); + Assert.Equal("a.cs", mappedResult.FileName); + + Assert.Equal(3, regularResult.Line); + Assert.Equal(11, mappedResult.Line); + + Assert.Equal("public void bar() { }", regularResult.Text); + Assert.Equal("new Foo().bar();", mappedResult.Text); + + Assert.Equal(32, regularResult.Column); + Assert.Equal(35, regularResult.EndColumn); + Assert.Equal(34, mappedResult.Column); + Assert.Equal(37, mappedResult.EndColumn); + } + [Fact] public async Task ExcludesMethodDefinition() { From e502657ce008157d5a69bc19bf48d20a310c2f87 Mon Sep 17 00:00:00 2001 From: Filip W Date: Wed, 12 Feb 2020 18:41:52 +0100 Subject: [PATCH 7/7] removed any() --- src/OmniSharp.Roslyn.CSharp/Helpers/LocationExtensions.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/OmniSharp.Roslyn.CSharp/Helpers/LocationExtensions.cs b/src/OmniSharp.Roslyn.CSharp/Helpers/LocationExtensions.cs index b2fd902cb7..ef15b41265 100644 --- a/src/OmniSharp.Roslyn.CSharp/Helpers/LocationExtensions.cs +++ b/src/OmniSharp.Roslyn.CSharp/Helpers/LocationExtensions.cs @@ -42,7 +42,7 @@ static SourceText GetSourceText(Location location, IEnumerable documen if (hasMappedPath) { SourceText source = null; - if (documents != null && documents.Any() && documents.FirstOrDefault(d => d.TryGetText(out source)) != null) + if (documents != null && documents.FirstOrDefault(d => d.TryGetText(out source)) != null) { // we have a mapped document that exists in workspace return source;