Extensions: Add ReduceExtensionMember API #80659
Open
+747
−28
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This PR modifies public API files. Please follow the instructions at https://github.com/dotnet/roslyn/blob/main/docs/contributing/API%20Review%20Process.md for ensuring all public APIs are reviewed before merging. |
bd1e5af to
cadd91d
Compare
cadd91d to
a151018
Compare
AlekseyTs
reviewed
Oct 16, 2025
| IMethodSymbol? ReduceExtensionMethod(ITypeSymbol receiverType); | ||
|
|
||
| /// <summary> | ||
| /// If this is a (new) extension method that can be applied to a receiver of the given type, |
AlekseyTs
reviewed
Oct 16, 2025
| if (_underlying.GetIsNewExtensionMember() && SourceMemberContainerTypeSymbol.IsAllowedExtensionMember(_underlying)) | ||
| { | ||
| var csharpReceiver = receiverType.EnsureCSharpSymbolOrNull(nameof(receiverType)); | ||
| return (IMethodSymbol?)SourceNamedTypeSymbol.GetCompatibleSubstitutedMember(compilation: null, _underlying, csharpReceiver, wasExtensionFullyInferred: out _).GetPublicSymbol(); |
There was a problem hiding this comment.
I don't think so. See description in OP: "For lookup, we do want to return members that can't be fully inferred."
AlekseyTs
reviewed
Oct 16, 2025
| if (_underlying.GetIsNewExtensionMember() && SourceMemberContainerTypeSymbol.IsAllowedExtensionMember(_underlying)) | ||
| { | ||
| var csharpReceiver = receiverType.EnsureCSharpSymbolOrNull(nameof(receiverType)); | ||
| return (IPropertySymbol?)SourceNamedTypeSymbol.GetCompatibleSubstitutedMember(compilation: null, _underlying, csharpReceiver, wasExtensionFullyInferred: out _).GetPublicSymbol(); |
AlekseyTs
reviewed
Oct 16, 2025
|
|
||
| INamedTypeSymbol systemObject = comp.GetSpecialType(SpecialType.System_Object).GetPublicSymbol(); | ||
|
|
||
| AssertEx.Equal("void E.<G>$66F77D1E46F965A5B22D4932892FA78B<T>.M()", method.ToTestDisplayString()); |
AlekseyTs
reviewed
Oct 16, 2025
| } | ||
|
|
||
| [Fact] | ||
| public void ReduceExtensionMember_12() |
There was a problem hiding this comment.
That's correct, this is testing the internal API which has a different name, but is morally equivalent. I don't think this warrants a different test name. An alternative would be to rename the internal API to use the same name as the public one.
AlekseyTs
reviewed
Oct 16, 2025
| } | ||
|
|
||
| [Fact] | ||
| public void ReduceExtensionMember_13() |
|
Done with review pass (Commit 1) #Closed |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #80273 (pending API review to decide whether to have an API on ISymbol or split as implemented here)
There is a small change in behavior when dealing with extensions that aren't fully inferred. Previously,
GetCompatibleSubstitutedMemberdropped them. Now, it's up to the caller to decide.The
LookupSymbols_WasNotFullyInferredtest illustrates such a change. For lookup, we do want to return members that can't be fully inferred.Relates to test plan #76130