Find references with multiple projects with same assembly name #8096
Conversation
jasonmalinowski
force-pushed
the
find-references-with-multiple-projects-with-same-assembly-name
branch
from
ca02994
to
495a0df
Compare
|
@dotnet-bot, retest prtest/mac/dbg/unit32 please. According to Jenkins, "Mono has a bug that occasionally sits in an infinite loop. Retry the test." |
| @@ -168,7 +169,7 @@ public string OutputFilePath | |||
| IList<AnalyzerReference> analyzerReferences = null) | |||
| { | |||
| _assemblyName = assemblyName; | |||
| _name = assemblyName; | |||
| _name = projectName; | |||
There was a problem hiding this comment.
I woiuldn't mind if _name was renamed to _projectName. Your call.
There was a problem hiding this comment.
I agreed at first, and but this field is exposed through the Name property, and saying Project.Name makes sense. It's the name of the project, after all. :-)
|
I've got some concerns. But i remain open to being convinced. I also wouldn't mind a shot of the UI post this change to see how it looks to the user. Not to reject/accept this change, but merely to brainstorm on how best to make this information clear. |
If there's multiple cursor positions in the test, each is used for a find references operation and the output of each result must match the marked spans. This is useful for linked file scenarios where the results should be same no matter which linked document it comes from.
|
@CyrusNajmabadi: I've answered your questions I hope to your satisfaction. Here's what the screenshot would look like for a line
The fact we don't merge them is unfortunate, but this is consistent with our experience with shared projects and linked files. Making that better should be a separate task item. Note that without my change, we can't even guarantee we'll show you all references in the first place. |
jasonmalinowski
force-pushed
the
find-references-with-multiple-projects-with-same-assembly-name
branch
from
495a0df
to
a94d886
Compare
|
Ew. I hate hte UI. But more is probably better than less in the short term. Can we file a bug about making the UI better? 👍 |
|
Even without this fix the UI is still that terrible. We just omit other references to drive you nuts even further. :-) @Pilchie do we already have something tracking making this better? |
|
@jasonmalinowski I have it in an excel spreadsheet that is waiting to be turned into a backlog item... |
| @@ -150,7 +150,7 @@ public string OutputFilePath | |||
| ParseOptions parseOptions, | |||
| string assemblyName, | |||
| params MetadataReference[] references) | |||
| : this(languageServices, compilationOptions, parseOptions, assemblyName, references, SpecializedCollections.EmptyArray<TestHostDocument>()) | |||
| : this(languageServices, compilationOptions, parseOptions, assemblyName, assemblyName, references, SpecializedCollections.EmptyArray<TestHostDocument>()) | |||
There was a problem hiding this comment.
Named args to explain the assemblyName/assemblyName thing?
|
👍 Woohoo! |
Right now you can specify linked files by assembly name, but that's not helpful if you want to test with two projects with the same assembly name in the first place.
…y name The default case for projects not specifying a name is still to use the assembly name, so this doesn't break existing tests.
This doesn't change the behavior of any tests, but it does make it easier to understand what is going on when a test with linked files fails.
Before this change, if you had two projects that had the same assembly name, you might get weird results from find references. Internally it used assembly name as a cache key for dependent projects, and so as long as the two projects with same assembly name had different dependent projects, we might not actually search all the projects we need to in your solution. Further, if you had the same linked file in both of these projects, we wouldn't search all projects that had it linked. This was because the LinkedFileReferenceFinder would correcty find all the symbols in linked projects, but the engine would then dedup the list calling those equal because once again the assembly names were equal. We make a decision here that when searching for symbols, symbols from source should never be deduplicated across projects that define them, but we will still merge metadata symbols as a single referenced symbol. There is an argument that we should also split up metadata symbols as well, but many consumers might find it very surprising that suddenly they get N referenced symbols in the result if there are N projects in the solution. For now, we'll maintain the result there. Fixes issue dotnet#3351.
FindSourceDefinitionAsync, when presented with a source symbol, would immediately return that symbol. If the source symbol is a retargeted source symbol, we were still returning that symbol instead of returning the "original" definition in the other source assembly. This changes the behavior to now return the original source assembly, a behavior which seems more consistent and allows FindSourceDefinitionAsync to be used in scenarios where we care about IAssemblySymbol identity.
jasonmalinowski
force-pushed
the
find-references-with-multiple-projects-with-same-assembly-name
branch
from
a94d886
to
4c49bf2
Compare
|
@dotnet-bot, retest this please. Jenkins wasn't able to inform GitHub of the final status because of the GitHub outage. |
|
@dotnet-bot, retest this please. Previous build failed due to a build timeout. Bug #8216 filed to track. |
|
@dotnet-bot, retest mac please. Previous build failed with "Mono has a bug that occasionally sits in an infinite loop. Retry the test." |
|
@dotnet-bot, retest eta please. |
|
Everything except prtest/win/tst/eta has passed, which is now good. |
…ltiple-projects-with-same-assembly-name Find references with multiple projects with same assembly name
jasonmalinowski
deleted the
find-references-with-multiple-projects-with-same-assembly-name
branch
If you tried doing find references when you had the same file linked into more than one project with the same assembly name, various things would go wrong. This makes things go right.
The core of this work is in commit 1bc98ab105d788fa7645046d09772afdc1e9920d, with the rest of the commits being general work to bring our test infrastructure up to spec to actually create tests for this scenario.
Review: @dotnet/roslyn-ide