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

Find the right symbol when following type forwards #62406

Merged
merged 13 commits into from
Jul 20, 2022
Merged

Find the right symbol when following type forwards #62406

Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
731ef67
Find the right symbol when following type forwards
davidwengier Jul 6, 2022
f199e1f
Update comment
davidwengier Jul 7, 2022
5265792
Use the full namespace to find the type
davidwengier Jul 8, 2022
81d06b3
Reference all DLLs found when following type forwards
davidwengier Jul 8, 2022
9cc43b9
Add tests for more symtol lookup scenarios
davidwengier Jul 8, 2022
67132d1
FIx multi hop test to actually test something
davidwengier Jul 8, 2022
07125d1
Validate type forwards
davidwengier Jul 8, 2022
8583e64
Merge remote-tracking branch 'upstream/main' into FixTypeForwardsInGTD
davidwengier Jul 18, 2022
416f459
Revert "Reference all DLLs found when following type forwards"
davidwengier Jul 18, 2022
927e2c0
Revert "Validate type forwards"
davidwengier Jul 18, 2022
5fd9a3d
Updates after symbol key changes, to make it smarter.
davidwengier Jul 18, 2022
5b458f1
Remove redundant code
davidwengier Jul 18, 2022
8c5fd03
Use a static formatter
davidwengier Jul 18, 2022
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
150 changes: 139 additions & 11 deletions src/EditorFeatures/CSharpTest/PdbSourceDocument/ImplementationAssemblyLookupServiceTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,126 @@ public class C
});
}

[Fact]
public async Task FollowTypeForwards_Namespace()
{
var source = @"
namespace A
{
namespace B
{
public class C
{
public class D
{
// A change
public event System.EventHandler [|E|] { add { } remove { } }
}
}
}
}";
var typeForwardSource = @"
[assembly: System.Runtime.CompilerServices.TypeForwardedTo(typeof(A.B.C))]
";

await RunTestAsync(async path =>
{
MarkupTestFile.GetSpan(source, out var metadataSource, out var expectedSpan);

// Compile reference assembly
var sourceText = SourceText.From(metadataSource, encoding: Encoding.UTF8);
var (project, symbol) = await CompileAndFindSymbolAsync(path, Location.Embedded, Location.Embedded, sourceText, c => c.GetMember("A.B.C.D.E"), buildReferenceAssembly: true);

// Compile implementation assembly to a different DLL
var dllFilePath = Path.Combine(path, "implementation.dll");
var sourceCodePath = Path.Combine(path, "implementation.cs");
var pdbFilePath = Path.Combine(path, "implementation.pdb");
var assemblyName = "implementation";

var workspace = TestWorkspace.Create(@$"
<Workspace>
<Project Language=""{LanguageNames.CSharp}"" CommonReferences=""true"" ReferencesOnDisk=""true"">
</Project>
</Workspace>", composition: GetTestComposition());

var implProject = workspace.CurrentSolution.Projects.First();
CompileTestSource(dllFilePath, sourceCodePath, pdbFilePath, assemblyName, sourceText, implProject, Location.Embedded, Location.Embedded, buildReferenceAssembly: false, windowsPdb: false);

// Compile type forwarding implementation DLL
var typeForwardDllFilePath = Path.Combine(path, "typeforward.dll");
assemblyName = "typeforward";

implProject = implProject.AddMetadataReference(MetadataReference.CreateFromFile(dllFilePath));
sourceText = SourceText.From(typeForwardSource, Encoding.UTF8);
CompileTestSource(typeForwardDllFilePath, sourceCodePath, pdbFilePath, assemblyName, sourceText, implProject, Location.Embedded, Location.Embedded, buildReferenceAssembly: false, windowsPdb: false);

var service = workspace.GetService<IImplementationAssemblyLookupService>();

var foundImplementationFilePath = service.FollowTypeForwards(symbol, typeForwardDllFilePath, new NoDuplicatesLogger());
Assert.Equal(dllFilePath, foundImplementationFilePath);
});
}

[Fact]
public async Task FollowTypeForwards_Generics()
{
var source = @"
namespace A
{
namespace B
{
public class C<T>
{
public class D
{
// A change
public event System.EventHandler [|E|] { add { } remove { } }
}
}
}
}";
var typeForwardSource = @"
[assembly: System.Runtime.CompilerServices.TypeForwardedTo(typeof(A.B.C<>))]
";

await RunTestAsync(async path =>
{
MarkupTestFile.GetSpan(source, out var metadataSource, out var expectedSpan);

// Compile reference assembly
var sourceText = SourceText.From(metadataSource, encoding: Encoding.UTF8);
var (project, symbol) = await CompileAndFindSymbolAsync(path, Location.Embedded, Location.Embedded, sourceText, c => c.GetMember("A.B.C.D.E"), buildReferenceAssembly: true);

// Compile implementation assembly to a different DLL
var dllFilePath = Path.Combine(path, "implementation.dll");
var sourceCodePath = Path.Combine(path, "implementation.cs");
var pdbFilePath = Path.Combine(path, "implementation.pdb");
var assemblyName = "implementation";

var workspace = TestWorkspace.Create(@$"
<Workspace>
<Project Language=""{LanguageNames.CSharp}"" CommonReferences=""true"" ReferencesOnDisk=""true"">
</Project>
</Workspace>", composition: GetTestComposition());

var implProject = workspace.CurrentSolution.Projects.First();
CompileTestSource(dllFilePath, sourceCodePath, pdbFilePath, assemblyName, sourceText, implProject, Location.Embedded, Location.Embedded, buildReferenceAssembly: false, windowsPdb: false);

// Compile type forwarding implementation DLL
var typeForwardDllFilePath = Path.Combine(path, "typeforward.dll");
assemblyName = "typeforward";

implProject = implProject.AddMetadataReference(MetadataReference.CreateFromFile(dllFilePath));
sourceText = SourceText.From(typeForwardSource, Encoding.UTF8);
CompileTestSource(typeForwardDllFilePath, sourceCodePath, pdbFilePath, assemblyName, sourceText, implProject, Location.Embedded, Location.Embedded, buildReferenceAssembly: false, windowsPdb: false);

var service = workspace.GetService<IImplementationAssemblyLookupService>();

var foundImplementationFilePath = service.FollowTypeForwards(symbol, typeForwardDllFilePath, new NoDuplicatesLogger());
Assert.Equal(dllFilePath, foundImplementationFilePath);
});
}

[Fact]
public async Task FollowTypeForwards_NestedType()
{
Expand Down Expand Up @@ -402,28 +522,36 @@ public class C
var typeForwardDllFilePath = Path.Combine(path, "typeforward.dll");
assemblyName = "typeforward";

implProject = implProject.AddMetadataReference(MetadataReference.CreateFromFile(dllFilePath));
sourceText = SourceText.From(typeForwardSource, Encoding.UTF8);
CompileTestSource(typeForwardDllFilePath, sourceCodePath, pdbFilePath, assemblyName, sourceText, implProject, Location.Embedded, Location.Embedded, buildReferenceAssembly: false, windowsPdb: false);
implProject = workspace.CurrentSolution.Projects.First().AddMetadataReference(MetadataReference.CreateFromFile(dllFilePath));
var typeForwardSourceText = SourceText.From(typeForwardSource, Encoding.UTF8);
CompileTestSource(typeForwardDllFilePath, sourceCodePath, pdbFilePath, assemblyName, typeForwardSourceText, implProject, Location.Embedded, Location.Embedded, buildReferenceAssembly: false, windowsPdb: false);

var typeForward2DllFilePath = Path.Combine(path, "typeforward2.dll");
assemblyName = "typeforward2";
// Now compile a new implementation in realimplementation.dll
var realImplementationDllFilePath = Path.Combine(path, "realimplementation.dll");
assemblyName = "realimplementation";

implProject = implProject.AddMetadataReference(MetadataReference.CreateFromFile(typeForwardDllFilePath));
sourceText = SourceText.From(typeForwardSource, Encoding.UTF8);
CompileTestSource(typeForward2DllFilePath, sourceCodePath, pdbFilePath, assemblyName, sourceText, implProject, Location.Embedded, Location.Embedded, buildReferenceAssembly: false, windowsPdb: false);
implProject = workspace.CurrentSolution.Projects.First();
CompileTestSource(realImplementationDllFilePath, sourceCodePath, pdbFilePath, assemblyName, sourceText, implProject, Location.Embedded, Location.Embedded, buildReferenceAssembly: false, windowsPdb: false);

// Now compile a new implementation.dll that typeforwards to realimplementation.dll
assemblyName = "implementation";

implProject = workspace.CurrentSolution.Projects.First().AddMetadataReference(MetadataReference.CreateFromFile(realImplementationDllFilePath));
CompileTestSource(dllFilePath, sourceCodePath, pdbFilePath, assemblyName, typeForwardSourceText, implProject, Location.Embedded, Location.Embedded, buildReferenceAssembly: false, windowsPdb: false);

var service = workspace.GetService<IImplementationAssemblyLookupService>();

Assert.Equal(dllFilePath, service.FollowTypeForwards(symbol, typeForward2DllFilePath, new NoDuplicatesLogger()));
var foundImplementationFilePath = service.FollowTypeForwards(symbol, typeForwardDllFilePath, new NoDuplicatesLogger());
Assert.Equal(realImplementationDllFilePath, foundImplementationFilePath);

// We need the DLLs to exist, in order for some checks to pass correct, but to ensure
// that the file isn't read, we just zero it out.
File.WriteAllBytes(typeForwardDllFilePath, Array.Empty<byte>());
File.WriteAllBytes(typeForward2DllFilePath, Array.Empty<byte>());
File.WriteAllBytes(realImplementationDllFilePath, Array.Empty<byte>());
File.WriteAllBytes(dllFilePath, Array.Empty<byte>());

Assert.Equal(dllFilePath, service.FollowTypeForwards(symbol, typeForward2DllFilePath, new NoDuplicatesLogger()));
foundImplementationFilePath = service.FollowTypeForwards(symbol, typeForwardDllFilePath, new NoDuplicatesLogger());
Assert.Equal(realImplementationDllFilePath, foundImplementationFilePath);
});
}

Expand Down
65 changes: 57 additions & 8 deletions src/EditorFeatures/CSharpTest/PdbSourceDocument/PdbSourceDocumentTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -457,6 +457,50 @@ public class C
{
}
}
";

await RunTestAsync(async path =>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The diff on this is weird. This test is actually old, just with a few bits of code removed. This wasn't testing type forwards at all, but their presence made me think I was correctly testing them.. which I wasn't!

{
MarkupTestFile.GetSpan(source, out var metadataSource, out var expectedSpan);

var packDir = Directory.CreateDirectory(Path.Combine(path, "packs", "MyPack.Ref", "1.0", "ref", "net6.0")).FullName;
var dataDir = Directory.CreateDirectory(Path.Combine(path, "packs", "MyPack.Ref", "1.0", "data")).FullName;
var sharedDir = Directory.CreateDirectory(Path.Combine(path, "shared", "MyPack", "1.0")).FullName;

var sourceText = SourceText.From(metadataSource, Encoding.UTF8);
var (project, symbol) = await CompileAndFindSymbolAsync(packDir, Location.Embedded, Location.Embedded, sourceText, c => c.GetMember("C.M"), buildReferenceAssembly: true);

var workspace = TestWorkspace.Create(@$"
<Workspace>
<Project Language=""{LanguageNames.CSharp}"" CommonReferences=""true"" ReferencesOnDisk=""true"">
</Project>
</Workspace>", composition: GetTestComposition());

var implProject = workspace.CurrentSolution.Projects.First();

// Compile implementation assembly
CompileTestSource(sharedDir, sourceText, project, Location.Embedded, Location.Embedded, buildReferenceAssembly: false, windowsPdb: false);

// Create FrameworkList.xml
File.WriteAllText(Path.Combine(dataDir, "FrameworkList.xml"), """
<FileList FrameworkName="MyPack">
</FileList>
""");

await GenerateFileAndVerifyAsync(project, symbol, Location.Embedded, metadataSource.ToString(), expectedSpan, expectNullResult: false);
});
}

[Fact]
public async Task Net6SdkLayout_TypeForward()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This test is actually entirely new, and correctly tests type forwards, in the manner that the .NET sdk uses them. This test fails without the changes in this PR.

{
var source = @"
public class [|C|]
{
public void M(string d)
{
}
}
";
var typeForwardSource = @"
[assembly: System.Runtime.CompilerServices.TypeForwardedTo(typeof(C))]
Expand All @@ -471,7 +515,7 @@ public class C
var sharedDir = Directory.CreateDirectory(Path.Combine(path, "shared", "MyPack", "1.0")).FullName;

var sourceText = SourceText.From(metadataSource, Encoding.UTF8);
var (project, symbol) = await CompileAndFindSymbolAsync(packDir, Location.Embedded, Location.Embedded, sourceText, c => c.GetMember("C.M"), buildReferenceAssembly: true);
var (project, symbol) = await CompileAndFindSymbolAsync(packDir, Location.Embedded, Location.Embedded, sourceText, c => c.GetMember("C"), buildReferenceAssembly: true);

var workspace = TestWorkspace.Create(@$"
<Workspace>
Expand All @@ -482,15 +526,20 @@ public class C
var implProject = workspace.CurrentSolution.Projects.First();

// Compile implementation assembly
CompileTestSource(sharedDir, sourceText, project, Location.Embedded, Location.Embedded, buildReferenceAssembly: false, windowsPdb: false);
var implementationDllFilePath = Path.Combine(sharedDir, "implementation.dll");
var sourceCodePath = Path.Combine(sharedDir, "implementation.cs");
var pdbFilePath = Path.Combine(sharedDir, "implementation.pdb");
var assemblyName = "implementation";

CompileTestSource(implementationDllFilePath, sourceCodePath, pdbFilePath, assemblyName, sourceText, project, Location.Embedded, Location.Embedded, buildReferenceAssembly: false, windowsPdb: false);

// Compile type forwarding implementation DLL
var typeForwardDllFilePath = Path.Combine(sharedDir, "typeforward.dll");
var sourceCodePath = Path.Combine(sharedDir, "typeforward.cs");
var pdbFilePath = Path.Combine(sharedDir, "typeforward.pdb");
var assemblyName = "typeforward";
// Compile type forwarding implementation DLL, that looks like reference.dll
var typeForwardDllFilePath = Path.Combine(sharedDir, "reference.dll");
sourceCodePath = Path.Combine(sharedDir, "reference.cs");
pdbFilePath = Path.Combine(sharedDir, "reference.pdb");
assemblyName = "reference";

implProject = implProject.AddMetadataReference(MetadataReference.CreateFromFile(GetDllPath(sharedDir)));
implProject = implProject.AddMetadataReference(MetadataReference.CreateFromFile(implementationDllFilePath));
sourceText = SourceText.From(typeForwardSource, Encoding.UTF8);
CompileTestSource(typeForwardDllFilePath, sourceCodePath, pdbFilePath, assemblyName, sourceText, implProject, Location.Embedded, Location.Embedded, buildReferenceAssembly: false, windowsPdb: false);

Expand Down
7 changes: 6 additions & 1 deletion src/Features/Core/Portable/PdbSourceDocument/ImplementationAssemblyLookupService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,11 @@ public bool TryFindImplementationAssemblyPath(string referencedDllPath, [NotNull
// Only the top most containing type in the ExportedType table actually points to an assembly
// so no point looking for nested types.
var typeSymbol = MetadataAsSourceHelpers.GetTopLevelContainingNamedType(symbol);
// We need to generate the namespace name in the same format that is used in metadata, which
// is SymbolDisplayFormat.QualifiedNameOnlyFormat, which this is a copy of.
var namespaceName = typeSymbol.ContainingNamespace.ToDisplayString(new SymbolDisplayFormat(
globalNamespaceStyle: SymbolDisplayGlobalNamespaceStyle.Omitted,
typeQualificationStyle: SymbolDisplayTypeQualificationStyle.NameAndContainingTypesAndNamespaces));
Copy link
Member

Choose a reason for hiding this comment

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

Consider storing the format in a static field.


try
{
Expand All @@ -66,7 +71,7 @@ public bool TryFindImplementationAssemblyPath(string referencedDllPath, [NotNull
{
// If there are no type forwards in this DLL, or not one for this type, then it means
// we've found the right DLL
if (typeForwards?.TryGetValue((typeSymbol.ContainingNamespace.MetadataName, typeSymbol.MetadataName), out var assemblyName) != true)
if (typeForwards?.TryGetValue((namespaceName, typeSymbol.MetadataName), out var assemblyName) != true)
{
return dllPath;
}
Expand Down
4 changes: 2 additions & 2 deletions ...Features/Core/Portable/PdbSourceDocument/PdbSourceDocumentMetadataAsSourceFileProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,11 +117,11 @@ internal sealed class PdbSourceDocumentMetadataAsSourceFileProvider : IMetadataA

var tmpCompilation = compilationFactory
.CreateCompilation("tmp", compilationFactory.GetDefaultCompilationOptions())
.AddReferences(project.MetadataReferences)
.AddReferences(dllReference);

var key = SymbolKey.Create(symbolToFind, cancellationToken);
var newSymbol = key.Resolve(tmpCompilation, ignoreAssemblyKey: true, cancellationToken).Symbol;
var resolution = key.Resolve(tmpCompilation, ignoreAssemblyKey: true, cancellationToken);
var newSymbol = resolution.Symbol;
if (newSymbol is null)
{
_logger?.Log(FeaturesResources.Could_not_find_implementation_of_symbol_0, symbolToFind.MetadataName);
Expand Down