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.
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.
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.