-
Notifications
You must be signed in to change notification settings - Fork 10.4k
Fix FromKeyedServicesAttribute and FromServicesAttribute to support derived types across all generators #63114
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.
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
base: main
Are you sure you want to change the base?
Conversation
Greetings human! You've submitted a PR that modifies code that is shared with https://github.com/dotnet/runtime . Please make sure you synchronize this code with the changes in that repo! |
…erived types - Replace OfType<FromKeyedServicesAttribute>() with IsAssignableFrom checks in BindingInfo.cs - Replace OfType<FromKeyedServicesAttribute>() with IsAssignableFrom checks in RequestDelegateFactory.cs - Update SignalR HubMethodDescriptor pattern matching to handle derived FromKeyedServicesAttribute types - Add test for derived FromKeyedServicesAttribute detection Co-authored-by: captainsafia <[email protected]>
FromKeyedServicesAttribute
and FromServiceAttributes
check for derived types
|
…tors and validation - Add InheritsFrom extension method to check class inheritance in SymbolExtensions - Add TryGetAttributeInheritingFrom methods to detect attributes that inherit from base types - Update IsServiceParameter in ITypeSymbolExtensions to use inheritance checking - Update EndpointParameter.cs to use TryGetAttributeInheritingFrom for FromKeyedServicesAttribute - Add CustomFromKeyedServicesAttribute test type and SupportsDerivedFromKeyedServicesAttribute test - All builds and tests pass successfully Co-authored-by: captainsafia <[email protected]>
I've implemented all the requested changes:
I also added the necessary infrastructure methods ( All builds and tests pass successfully. The changes ensure that custom attributes inheriting from Commit: ec72a16 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes a limitation where custom attributes derived from FromKeyedServicesAttribute
and FromServicesAttribute
were not being detected by ASP.NET Core's code generation components. The framework was using strict type equality checks instead of inheritance-aware checks.
Key changes:
- Enhanced attribute detection logic to use inheritance checking instead of exact type matching
- Added robust inheritance checking methods to
SymbolExtensions.cs
for source generators - Updated attribute detection across MVC, HTTP Extensions, SignalR, and validation components
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
SymbolExtensions.cs |
Added new inheritance checking methods (InheritsFrom , TryGetAttributeInheritingFrom ) |
ITypeSymbolExtensions.cs |
Updated service parameter detection to use inheritance checking |
HubMethodDescriptor.cs |
Enhanced SignalR attribute detection with inheritance support |
BindingInfo.cs |
Fixed MVC model binding to detect derived keyed service attributes |
RequestDelegateFactory.cs |
Updated HTTP extensions to support derived attributes |
EndpointParameter.cs |
Modified source generator to use inheritance-aware attribute detection |
Test files | Added comprehensive tests validating derived attribute support |
@@ -88,7 +88,7 @@ public HubMethodDescriptor(ObjectMethodExecutor methodExecutor, IServiceProvider | |||
markedParameter = true; | |||
MarkServiceParameter(index); | |||
} | |||
else if (attribute is FromKeyedServicesAttribute keyedServicesAttribute) | |||
else if (typeof(FromKeyedServicesAttribute).IsAssignableFrom(attribute.GetType()) && attribute is FromKeyedServicesAttribute keyedServicesAttribute) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The condition typeof(FromKeyedServicesAttribute).IsAssignableFrom(attribute.GetType()) && attribute is FromKeyedServicesAttribute keyedServicesAttribute
is redundant. The pattern matching attribute is FromKeyedServicesAttribute keyedServicesAttribute
already ensures the attribute can be cast to FromKeyedServicesAttribute
, making the IsAssignableFrom
check unnecessary.
else if (typeof(FromKeyedServicesAttribute).IsAssignableFrom(attribute.GetType()) && attribute is FromKeyedServicesAttribute keyedServicesAttribute) | |
else if (attribute is FromKeyedServicesAttribute keyedServicesAttribute) |
Copilot uses AI. Check for mistakes.
This PR updates the ASP.NET Core framework to properly detect and process custom attributes that derive from
FromKeyedServicesAttribute
andFromServicesAttribute
across all code generation components.Problem
The framework was using strict type equality checks which only matched exact types, preventing derived attributes from being recognized. For example:
Solution
Enhanced inheritance checking across all code generation components:
BindingInfo.cs
) - Ensures derived attributes work in MVC controller actionsRequestDelegateFactory.cs
) - Enables derived attributes in minimal API endpointsHubMethodDescriptor.cs
) - Supports derived attributes in SignalR hub methodsEndpointParameter.cs
) - Handles derived attributes in source generatorsITypeSymbolExtensions.cs
) - Detects derived attributes during compilationInfrastructure Changes
Added robust inheritance checking methods to
SymbolExtensions.cs
:InheritsFrom()
- Checks if a type inherits from a base classTryGetAttributeInheritingFrom()
- Finds attributes that derive from a base typeChanges Made
typeof(FromKeyedServicesAttribute).IsAssignableFrom(a.GetType())
TryGetAttribute
withTryGetAttributeInheritingFrom
for inheritance supportIsServiceParameter
to useInheritsFrom
instead of exact equalitySupportsDerivedFromKeyedServicesAttribute
validates the fix end-to-endTesting
This enables developers to create extensible service injection attributes while maintaining full backward compatibility and framework support.
Fixes #63113.
💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.