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.

Sign up for GitHub

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

Jump to bottom

Do not return nulls when getting documents by path #2233

Merged
merged 4 commits into from
Sep 11, 2021
Merged

Do not return nulls when getting documents by path #2233

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
451ea48
Do not return nulls when getting documents by path
JoeRobich Sep 8, 2021
eae3c08
Update MappedLocation to better simulate Razor scenario
JoeRobich Sep 9, 2021
dcbef8d
Fix Cake Completion tests
JoeRobich Sep 9, 2021
d79627d
Add test for misc Workspace scenario
JoeRobich Sep 10, 2021
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
14 changes: 10 additions & 4 deletions src/OmniSharp.Roslyn/OmniSharpWorkspace.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,12 +62,12 @@ public OmniSharpWorkspace(HostServicesAggregator aggregator, ILoggerFactory logg

private void OnDirectoryRemoved(string path, FileChangeType changeType)
{
if(changeType == FileChangeType.DirectoryDelete)
if (changeType == FileChangeType.DirectoryDelete)
{
var docs = CurrentSolution.Projects.SelectMany(x => x.Documents)
.Where(x => x.FilePath.StartsWith(path + Path.DirectorySeparatorChar, StringComparison.OrdinalIgnoreCase));

foreach(var doc in docs)
foreach (var doc in docs)
{
OnDocumentRemoved(doc.Id);
}
Expand Down Expand Up @@ -340,7 +340,8 @@ public IEnumerable<Document> GetDocuments(string filePath)
{
return CurrentSolution
.GetDocumentIdsWithFilePath(filePath)
.Select(id => CurrentSolution.GetDocument(id));
.Select(id => CurrentSolution.GetDocument(id))
.OfType<Document>();
Copy link
Member Author

Choose a reason for hiding this comment

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

Since the .cshtml files will have valid DocumentIds but not be part of any Project's Documents null can be returned, the OfType will filter out the null values.

}

public Document GetDocument(string filePath)
Expand Down Expand Up @@ -542,8 +543,13 @@ public void SetAnalyzerReferences(ProjectId id, ImmutableArray<AnalyzerFileRefer

public void AddAdditionalDocument(ProjectId projectId, string filePath)
{
var documentId = DocumentId.CreateNewId(projectId);
var loader = new OmniSharpTextLoader(filePath);
AddAdditionalDocument(projectId, filePath, loader);
}

public void AddAdditionalDocument(ProjectId projectId, string filePath, TextLoader loader)
{
var documentId = DocumentId.CreateNewId(projectId);
var documentInfo = DocumentInfo.Create(documentId, Path.GetFileName(filePath), filePath: filePath, loader: loader);
OnAdditionalDocumentAdded(documentInfo);
}
Expand Down
102 changes: 82 additions & 20 deletions tests/OmniSharp.Roslyn.CSharp.Tests/FindReferencesFacts.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
using System.IO;
using System.Linq;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis;
using OmniSharp.Models;
using OmniSharp.Models.FindUsages;
using OmniSharp.Roslyn.CSharp.Services.Navigation;
Expand Down Expand Up @@ -419,31 +420,87 @@ public Foo Clone() {
}

[Fact]
public async Task MappedLocationFileNameProperlyRooted()
public async Task MappedLocationFileNameProperlyRootedInAdditionalDocuments()
{
var relativeFile = ".\\pages\\test.xaml";
var folderPath = Directory.GetCurrentDirectory();
var relativeFile = ".\\Index.cshtml";
var mappedFilePath = Path.GetFullPath(Path.Combine(folderPath, relativeFile));

var testFiles = new[]
{
new TestFile("a.cs",
@"public class Bar : F$$oo {}"),
new TestFile("b.cs", $@"
#line 23 ""{relativeFile}""
public class Foo {{ }}
new TestFile("Constants.cs", @"
public static class Constants
{
public const string My$$Text = ""Hello World"";
}"),
new TestFile("Index.cshtml.cs", @"
using Microsoft.AspNetCore.Mvc.RazorPages;

public class IndexModel : PageModel
{
public IndexModel()
{
}

public void OnGet()
{

}
}"),
new TestFile("Index.cshtml_virtual.cs", $@"
#line 1 ""{relativeFile}""
Constants.MyText
#line default
#line hidden")
#line hidden"),
new TestFile(mappedFilePath, "<p>@Constants.MyText</p>")
};

var usages = await FindUsagesAsync(testFiles, onlyThisFile: false);
var usages = await FindUsagesAsync(testFiles, onlyThisFile: false, folderPath: folderPath);

Assert.DoesNotContain(usages.QuickFixes, location => location.FileName.EndsWith("b.cs"));
Assert.DoesNotContain(usages.QuickFixes, location => location.FileName.EndsWith("Index.cshtml_virtual.cs"));
Assert.DoesNotContain(usages.QuickFixes, location => location.FileName.Equals(relativeFile));

var project = SharedOmniSharpTestHost.Workspace.CurrentSolution.Projects.Single();
var projectFolder = Path.GetDirectoryName(project.FilePath);
var mappedFilePath = Path.GetFullPath(Path.Combine(projectFolder, relativeFile));
var quickFix = Assert.Single(usages.QuickFixes, location => location.FileName.Equals(mappedFilePath));
Assert.Empty(quickFix.Projects);
}

[Fact]
public async Task MappedLocationFileNameProperlyRootedInMiscellaneousWorkspace()
{
var folderPath = Directory.GetCurrentDirectory();
var relativeFile = ".\\Index.cshtml.cs";
var mappedFilePath = Path.GetFullPath(Path.Combine(folderPath, relativeFile));

Assert.Single(usages.QuickFixes, location => location.FileName.Equals(mappedFilePath));
var testFiles = new[]
{
new TestFile("Constants.cs", @"
public static class Constants
{
public const string My$$Text = ""Hello World"";
}"),
new TestFile("Index.cshtml_virtual.cs", $@"
#line 1 ""{relativeFile}""
Constants.MyText
#line default
#line hidden")
};

var miscFile = new TestFile(mappedFilePath, "// Constants.MyText;");

SharedOmniSharpTestHost.AddFilesToWorkspace(folderPath, testFiles);
SharedOmniSharpTestHost.Workspace.TryAddMiscellaneousDocument(
miscFile.FileName,
TextLoader.From(TextAndVersion.Create(miscFile.Content.Text, VersionStamp.Create())),
LanguageNames.CSharp);

var testFile = testFiles.Single(tf => tf.Content.HasPosition);
var usages = await FindUsagesAsync(testFile, onlyThisFile: false);

Assert.DoesNotContain(usages.QuickFixes, location => location.FileName.EndsWith("Index.cshtml_virtual.cs"));
Assert.DoesNotContain(usages.QuickFixes, location => location.FileName.Equals(relativeFile));

var quickFix = Assert.Single(usages.QuickFixes, location => location.FileName.Equals(mappedFilePath));
Assert.Empty(quickFix.Projects);
}

[Fact]
Expand Down Expand Up @@ -493,20 +550,25 @@ private Task<QuickFixResponse> FindUsagesAsync(string code, bool excludeDefiniti
return FindUsagesAsync(new[] { new TestFile("dummy.cs", code) }, false, excludeDefinition);
}

private async Task<QuickFixResponse> FindUsagesAsync(TestFile[] testFiles, bool onlyThisFile, bool excludeDefinition = false)
private Task<QuickFixResponse> FindUsagesAsync(TestFile[] testFiles, bool onlyThisFile, bool excludeDefinition = false, string folderPath = null)
{
SharedOmniSharpTestHost.AddFilesToWorkspace(folderPath, testFiles);
var testFile = testFiles.Single(tf => tf.Content.HasPosition);
return FindUsagesAsync(testFile, onlyThisFile, excludeDefinition);
}

private async Task<QuickFixResponse> FindUsagesAsync(TestFile testFile, bool onlyThisFile, bool excludeDefinition = false)
{
SharedOmniSharpTestHost.AddFilesToWorkspace(testFiles);
var file = testFiles.Single(tf => tf.Content.HasPosition);
var point = file.Content.GetPointFromPosition();
var point = testFile.Content.GetPointFromPosition();

var requestHandler = GetRequestHandler(SharedOmniSharpTestHost);

var request = new FindUsagesRequest
{
Line = point.Line,
Column = point.Offset,
FileName = file.FileName,
Buffer = file.Content.Code,
FileName = testFile.FileName,
Buffer = testFile.Content.Code,
OnlyThisFile = onlyThisFile,
ExcludeDefinition = excludeDefinition
};
Expand Down
1 change: 1 addition & 0 deletions tests/OmniSharp.Roslyn.CSharp.Tests/SourceGeneratorFacts.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ class GeneratedCode
"project.csproj",
new[] { "netcoreapp3.1" },
new[] { testFile },
otherFiles: null,
ImmutableArray.Create<AnalyzerReference>(reference));

var point = testFile.Content.GetPointFromPosition();
Expand Down
5 changes: 4 additions & 1 deletion tests/TestUtility/OmniSharpTestHost.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,10 @@ public IEnumerable<ProjectId> AddFilesToWorkspace(string folderPath, params Test
Workspace,
Path.Combine(folderPath, "project.csproj"),
new[] { "net472" },
testFiles.Where(f => f.FileName.EndsWith(".cs", StringComparison.OrdinalIgnoreCase)).ToArray());
testFiles.Where(f => f.FileName.EndsWith(".cs", StringComparison.OrdinalIgnoreCase)).ToArray(),
testFiles.Where(f => !f.FileName.EndsWith(".cs", StringComparison.OrdinalIgnoreCase)
&& !f.FileName.EndsWith(".csx", StringComparison.OrdinalIgnoreCase)
&& !f.FileName.EndsWith(".cake", StringComparison.OrdinalIgnoreCase)).ToArray());

foreach (var csxFile in testFiles.Where(f => f.FileName.EndsWith(".csx", StringComparison.OrdinalIgnoreCase)))
{
Expand Down
9 changes: 8 additions & 1 deletion tests/TestUtility/TestHelpers.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,10 @@ public static void AddCsxProjectToWorkspace(OmniSharpWorkspace workspace, TestFi
workspace.AddDocument(documentInfo);
}

public static IEnumerable<ProjectId> AddProjectToWorkspace(OmniSharpWorkspace workspace, string filePath, string[] frameworks, TestFile[] testFiles, ImmutableArray<AnalyzerReference> analyzerRefs = default)
public static IEnumerable<ProjectId> AddProjectToWorkspace(OmniSharpWorkspace workspace, string filePath, string[] frameworks, TestFile[] testFiles, TestFile[] otherFiles = null, ImmutableArray<AnalyzerReference> analyzerRefs = default)
{
otherFiles ??= Array.Empty<TestFile>();

var versionStamp = VersionStamp.Create();
var references = GetReferences();
frameworks = frameworks ?? new[] { string.Empty };
Expand Down Expand Up @@ -84,6 +86,11 @@ public static IEnumerable<ProjectId> AddProjectToWorkspace(OmniSharpWorkspace wo
workspace.AddDocument(projectInfo.Id, testFile.FileName, TextLoader.From(TextAndVersion.Create(testFile.Content.Text, versionStamp)), SourceCodeKind.Regular);
}

foreach (var otherFile in otherFiles)
{
workspace.AddAdditionalDocument(projectInfo.Id, otherFile.FileName, TextLoader.From(TextAndVersion.Create(otherFile.Content.Text, versionStamp)));
}

projectsIds.Add(projectInfo.Id);
}

Expand Down