Warn on RUC annotated attribute ctors (analyzer) #2201
Conversation
|
Might be worth fixing the RUC-annotated-property case as well while you're at it: class AAttribute : Attribute
{
public Type SomeProperty
{
[RUC]
set { ... }
}
}
[A(SomeProperty = typeof(Foo))]
class Foo { } |
| }, SymbolKind.Method); | ||
|
|
||
| context.RegisterSymbolAction (symbolAnalysisContext => { | ||
| var typeSymbol = (INamedTypeSymbol) symbolAnalysisContext.Symbol; | ||
| CheckMatchingAttributesInInterfaces (symbolAnalysisContext, typeSymbol); | ||
| CheckAttributeCtor (symbolAnalysisContext, typeSymbol); |
There was a problem hiding this comment.
If I read this correctly it will only work if the attribute is on a method or type(which doesn't seem to be covered by the test BTW). What if I put such attribute on anything else (field, property, method parameter, ...) - ideally we should generate the warning in all cases where the attribute is instantiated (liker will).
mateoatr
force-pushed
the
rucOnAttributeCtor
branch
from
dd42258 to
8ff9cc8
Compare
|
This takes some of the changes from the test restructure PR (#2113). In particular, it grabs the |
Add ProducedBy from Test Restructure Add support for SetupCompileBefore in the analyzer Run analyzer tests on all members (not only methods) Move tests to RequiresCapability
mateoatr
force-pushed
the
rucOnAttributeCtor
branch
from
d64a2c3 to
3a5ace0
Compare
| } | ||
| } | ||
|
|
||
| CheckAttributeCtor (symbolAnalysisContext, propertySymbol); |
There was a problem hiding this comment.
Don't we need to do this for getter/setter as well (similar to what we do for events with add/remove).
There was a problem hiding this comment.
In fact, RUC is not allowed on the property itself, only on the methods...
There was a problem hiding this comment.
This is needed for RequiresAssemblyFilesAttribute, which can be put on properties.
| @@ -125,6 +149,21 @@ public override void Initialize (AnalysisContext context) | |||
| foreach (var extraSyntaxNodeAction in ExtraSyntaxNodeActions) | |||
| context.RegisterSyntaxNodeAction (extraSyntaxNodeAction.Action, extraSyntaxNodeAction.SyntaxKind); | |||
|
|
|||
| void CheckAttributeCtor ( | |||
There was a problem hiding this comment.
In a separate change:
This should be called CheckAttributeInstantiation and it should also handle the case where the instantiation sets a property on the attribute - as in that case the property setter is called and it can have RUC on it.
test/Mono.Linker.Tests.Cases.Expectations/Assertions/ExpectedWarningAttribute.cs
Outdated
Show resolved
Hide resolved
test/ILLink.RoslynAnalyzer.Tests/Verifiers/CSharpAnalyzerVerifier`1.cs
Outdated
Show resolved
Hide resolved
test/ILLink.RoslynAnalyzer.Tests/Verifiers/CSharpAnalyzerVerifier`1.cs
Outdated
Show resolved
Hide resolved
Check for instantiations that set annotated properties
Co-authored-by: Andy Gocke <angocke@microsoft.com>
Rename Linker => Trimmer in ProducedBy
|
/azp run |
|
No pipelines are associated with this pull request. |
|
/azp run |
|
Pull request contains merge conflicts. |
* Warn on usage of attributes with annotated ctors * PR feedback Add ProducedBy from Test Restructure Add support for SetupCompileBefore in the analyzer Run analyzer tests on all members (not only methods) Move tests to RequiresCapability * Enable analyzer tests for attributes which use RUC annotated properties * Lint * Rename CheckAttributeCtor Check for instantiations that set annotated properties * Apply suggestions from code review Co-authored-by: Andy Gocke <angocke@microsoft.com> * Update comment Rename Linker => Trimmer in ProducedBy * Fix applied suggestions * Lint Co-authored-by: Andy Gocke <angocke@microsoft.com> Commit migrated from dotnet/linker@6f237bb
Fix #2094.