Conversation
|
Azure Pipelines: Successfully started running 2 pipeline(s). There may be pipelines that require an authorized user to comment /azp run to run. |
|
@dotnet-policy-service agree |
| { | ||
| return | ||
| x.MetadataName == y.MetadataName && | ||
| IsPartialEventDefinitionPart(x) == IsPartialEventDefinitionPart(y) && |
There was a problem hiding this comment.
i would not make these changes. see my recomendation in the issue about an appropriate fix here.
specifically, make all the partial handling logic in SymEquivalenceComparer an option (that find-refs can opt-out of). So from it's perspective, partial is irrelvant, which i think is entirely fine for Find-Refs.
we can also remove the partial-cascading logic from Find-Refs as well.
this then doesn't impact any other features, whihc may want strict correctness in symbol equivalence.
There was a problem hiding this comment.
Done. These changes are dropped: the partial checks are now behind a distinguishPartialParts option that only FAR turns off, and FAR no longer cascades between partial parts. Details are in the updated PR description.
A project that sees another project through a retargeting assembly binds uses of partial members to retargeting symbols, which by design expose no partial parts (see the partial properties and partial events API reviews, dotnet#73411 and dotnet#77203). SymbolEquivalenceComparer required the partial-part flags to match, so Find References never matched those uses to the source definition part and dropped every use in the referencing project. Add a distinguishPartialParts option to SymbolEquivalenceComparer, on by default, and turn it off in SymbolFinder.OriginalSymbolsMatch. Every other user of the comparer keeps the strict comparison. With the parts no longer distinguished, a search of either part finds the uses of both, and searching both would report each reference twice. Find References now treats the parts of a partial member the way it treats the copies of a symbol in linked files: as the same symbol. Both parts are reported in one symbol group, and when cascading only the definition part is searched, so interface and overridden members are reached from either part. A parameter or type parameter named differently in the two parts (CS8826) is still searched in both parts, as its uses are spelled with the other name. This replaces every cascade between partial parts in the finders. Implement Explicitly searched without cascading and used only the first ReferencedSymbol. A symbol group reports one ReferencedSymbol per member, and only the starting symbol is searched, so for a member declared in a linked file it could pick an empty copy and leave the call sites unchanged. It now uses the locations from every ReferencedSymbol. Fixes dotnet#82744 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
720f1ef to
6194a6e
Compare
Enumerable.Order doesn't exist on .NET Framework, so use OrderBy. In EditorFeatures.UnitTests, CSharpCompilation.GetTypeByMetadataName returns the compiler's internal symbol, so cast the compilation to Compilation as the other tests in that file do. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Under OLDER_ROSLYN the event helpers return false without reading any instance state, so CA1822 asked for them to be static. Read the distinguishPartialParts flag outside the #if so both builds use it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Fixes #82744
When a project sees another project through a retargeting assembly, uses of partial members bind to retargeting symbols. This happens with a transitive package version skew, with a netstandard2.0 library referenced from netX, and with a Web SDK project whose ASP.NET Core framework reference resolves assemblies that the library's netfx facades forward to. Retargeting symbols expose no
PartialDefinitionPart/PartialImplementationPart, by design (#73411, #77203).SymbolEquivalenceComparerrequired the partial-part flags to match, so Find References, and everything built on it, dropped every use in the referencing project.Following the review, the partial handling in
SymbolEquivalenceCompareris now an option (distinguishPartialParts, on by default). OnlySymbolFinder.OriginalSymbolsMatchturns it off, so every other feature keeps the strict comparison.Once the parts aren't distinguished, a search of either part finds the uses of both, and searching both would report every reference twice. So Find References now treats the parts of a partial member the way it already treats the copies of a symbol in linked files: as the same symbol.
FindSameSymbolsAsync(the linked copies, plus each copy's other partial part) replacesFindLinkedSymbolsAsyncin the two places the engine used it: building theSymbolGroup, and expanding the search set. Both declarations stay definitions, and the UI shows them as one entry.SymbolFinder.FindReferencesAsyncreturns the sameReferencedSymbols as before.The one visible change in results: uses of a same-name parameter inside the implementation body are now attributed to the definition part's parameter. The set of locations is unchanged.
This also fixes a bug in Implement Explicitly that already existed for linked files. It searched without cascading and took
references.FirstOrDefault(). A symbol group reports oneReferencedSymbolper member, but a non-cascading search only searches the starting symbol, so for a member declared in a linked file (for example, in a multi-targeted project), the firstReferencedSymbolcould be an empty copy, and the call sites were left unchanged. In a local repro it picked the empty copy in 3 of 5 runs. It now takes the locations from everyReferencedSymbol, which matters for partial members too now that they're grouped.Tests, in Workspaces.UnitTests unless noted:
FindReferences_PartialMemberThroughRetargetingReferencecovers methods, properties and events, searched from either part, through a real project reference (Lib on netstandard2.0, App on netcoreapp). It checks that the cross-project use is attributed to the same definitions as a same-project use.FindReferences_PartialMethodCascadesToInterfaceMemberFromEitherPartandFindReferences_PartialMethodParametercover the hierarchy, a renamed parameter and duplicate references.SymbolEquivalenceComparerTests.TestPartialParts(EditorFeatures.UnitTests) covers the option'sEqualsandGetHashCode.Tested locally on macOS (net10.0): Workspaces, CSharp.Workspaces, LanguageServer.Protocol, Features and CSharp.Features unit tests pass. The EditorFeatures test projects, which hold the Test2 Find References tests and the Implement Explicitly tests, are net472/WPF and weren't run locally.
🤖 Generated with Claude Code
Microsoft Reviewers: Open in CodeFlow