-
Notifications
You must be signed in to change notification settings - Fork 5.1k
Fix finding attribute data for syntax for assembly/module symbols #72535
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
Fix finding attribute data for syntax for assembly/module symbols #72535
Conversation
Correctly handle attributes with assembly or module targets, and use 'null' fallback for unknown scenarios.
Tagging subscribers to this area: @dotnet/interop-contrib Issue DetailsCorrectly handle attributes with assembly or module targets, and use 'null' fallback for unknown scenarios. Unblocks dotnet/sdk and dotnet/aspnetcore intake of dotnet/runtime.
|
case SyntaxKind.ReturnKeyword: | ||
return ((IMethodSymbol)targetSymbol).GetReturnTypeAttributes().First(attributeSyntaxLocationMatches); | ||
case SyntaxKind.AssemblyKeyword: | ||
return targetSymbol.ContainingAssembly.GetAttributes().First(attributeSyntaxLocationMatches); | ||
case SyntaxKind.ModuleKeyword: | ||
return targetSymbol.ContainingModule.GetAttributes().First(attributeSyntaxLocationMatches); | ||
default: | ||
return null; |
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.
I didn't look at the context of the callers or what's possible and what's not.
But there are other attribute targets to consider (only if they're relevant):
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 rest of them aren't relevant enough to cover here. Returning null
is fine. Technically, we don't need to handle assembly
and module
as well as this PR does, but I'd rather cleanly handle them than returning null
if possible.
If your PR to add IAttributeOperation
ever gets merged, we're going to switch to use that API from Roslyn and work with the IOperation
tree instead of the syntax tree (and effectively obsolete all usages of this class).
Timeouts in the System.Net.Http.Functional test suites are unrelated. Installer test is a known issue (linked automatically by Build Analysis) |
Correctly handle attributes with assembly or module targets, and use 'null' fallback for unknown scenarios.
Unblocks dotnet/sdk and dotnet/aspnetcore intake of dotnet/runtime.