From 378958ef42dd49495c19ce2cfe46217df41d24ed Mon Sep 17 00:00:00 2001 From: Sven Boemer Date: Thu, 21 Sep 2023 23:31:52 +0000 Subject: [PATCH 1/4] Simplify handling of properties in illink analyzer Previously this was effectively visiting referenced properties twice for each reference - it would visit both the referenced get/set method, and the property itself. This change only visits the get/set method, and handles the property warnings there by treating the getter/setter as annotated when the property is annotated. This also allows some simplification of the override checking logic. --- .../RequiresAnalyzerBase.cs | 7 +-- .../RequiresAssemblyFilesAnalyzer.cs | 4 +- .../RequiresISymbolExtensions.cs | 36 +++++++-------- .../RequiresAssemblyFilesAnalyzerTests.cs | 18 ++++---- ...nconditionalSuppressMessageCodeFixTests.cs | 2 +- .../RequiresAttributeMismatch.cs | 45 +++++++++---------- 6 files changed, 50 insertions(+), 62 deletions(-) diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresAnalyzerBase.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresAnalyzerBase.cs index cd3dca23044064..81f60e7aeb2960 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresAnalyzerBase.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresAnalyzerBase.cs @@ -96,9 +96,6 @@ public override void Initialize (AnalysisContext context) if (usageInfo.HasFlag (ValueUsageInfo.Write) && prop.SetMethod != null) CheckCalledMember (operationContext, prop.SetMethod, incompatibleMembers); - - if (AnalyzerDiagnosticTargets.HasFlag (DiagnosticTargets.Property)) - CheckCalledMember (operationContext, prop, incompatibleMembers); }, OperationKind.PropertyReference); context.RegisterOperationAction (operationContext => { @@ -330,8 +327,8 @@ private bool HasMismatchingAttributes (ISymbol member1, ISymbol member2) { bool member1CreatesRequirement = member1.DoesMemberRequire (RequiresAttributeName, out _); bool member2CreatesRequirement = member2.DoesMemberRequire (RequiresAttributeName, out _); - bool member1FulfillsRequirement = member1.IsOverrideInRequiresScope (RequiresAttributeName); - bool member2FulfillsRequirement = member2.IsOverrideInRequiresScope (RequiresAttributeName); + bool member1FulfillsRequirement = member1.IsInRequiresScope (RequiresAttributeName); + bool member2FulfillsRequirement = member2.IsInRequiresScope (RequiresAttributeName); return (member1CreatesRequirement && !member2FulfillsRequirement) || (member2CreatesRequirement && !member1FulfillsRequirement); } diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresAssemblyFilesAnalyzer.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresAssemblyFilesAnalyzer.cs index 863bd0f745a555..507284afdad8cb 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresAssemblyFilesAnalyzer.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresAssemblyFilesAnalyzer.cs @@ -87,15 +87,13 @@ protected override bool ReportSpecialIncompatibleMembersDiagnostic (OperationAna } else if (method.AssociatedSymbol is ISymbol associatedSymbol && ImmutableArrayOperations.Contains (dangerousPatterns, associatedSymbol, SymbolEqualityComparer.Default)) { + operationContext.ReportDiagnostic (Diagnostic.Create (s_locationRule, operationContext.Operation.Syntax.GetLocation (), member.GetDisplayName ())); // The getters for CodeBase and EscapedCodeBase have RAF attribute on them // so our caller will produce the RAF warning (IL3002) by default. Since we handle these properties specifically // here and produce different warning (IL3000) we don't want the caller to produce IL3002. // So we need to return true from here for the getters, to suppress the RAF warning. return true; } - } else if (member is IPropertySymbol && ImmutableArrayOperations.Contains (dangerousPatterns, member, SymbolEqualityComparer.Default)) { - operationContext.ReportDiagnostic (Diagnostic.Create (s_locationRule, operationContext.Operation.Syntax.GetLocation (), member.GetDisplayName ())); - return true; } return false; diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresISymbolExtensions.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresISymbolExtensions.cs index d57d1d62d68a82..f7688e1fd4cf41 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresISymbolExtensions.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresISymbolExtensions.cs @@ -18,6 +18,17 @@ public static bool DoesMemberRequire (this ISymbol member, string requiresAttrib if (!member.IsStaticConstructor () && member.TryGetAttribute (requiresAttribute, out requiresAttributeData)) return true; + if (member is IMethodSymbol method) { + switch (method.MethodKind) { + case MethodKind.PropertyGet: + case MethodKind.PropertySet: + var propertySymbol = (IPropertySymbol) method.AssociatedSymbol!; + if (propertySymbol.TryGetAttribute (requiresAttribute, out requiresAttributeData)) + return true; + break; + } + } + // Also check the containing type if (member.IsStatic || member.IsConstructor ()) return member.ContainingType.TryGetAttribute (requiresAttribute, out requiresAttributeData); @@ -25,28 +36,16 @@ public static bool DoesMemberRequire (this ISymbol member, string requiresAttrib return false; } - // TODO: Consider sharing with ILLink IsInRequiresScope method - /// - /// True if the source of a call is considered to be annotated with the Requires... attribute - /// - public static bool IsInRequiresScope (this ISymbol member, string attributeName, [NotNullWhen (true)] out AttributeData? requiresAttribute) + public static bool IsInRequiresScope (this ISymbol member, string attributeName) { - return member.IsInRequiresScope (attributeName, true, out requiresAttribute); + return member.IsInRequiresScope (attributeName, out _); } + // TODO: Consider sharing with ILLink IsInRequiresScope method /// - /// True if member of a call is considered to be annotated with the Requires... attribute. - /// Doesn't check the associated symbol for overrides and virtual methods because the analyzer should warn on mismatched between the property AND the accessors + /// True if the source of a call is considered to be annotated with the Requires... attribute /// - /// - /// Symbol that is either an overriding member or an overriden/virtual member - /// - public static bool IsOverrideInRequiresScope (this ISymbol member, string requiresAttribute) - { - return member.IsInRequiresScope (requiresAttribute, false, out _); - } - - private static bool IsInRequiresScope (this ISymbol member, string attributeName, bool checkAssociatedSymbol, [NotNullWhen (true)] out AttributeData? requiresAttribute) + public static bool IsInRequiresScope (this ISymbol member, string attributeName, [NotNullWhen (true)] out AttributeData? requiresAttribute) { // Requires attribute on a type does not silence warnings that originate // from the type directly. We also only check the containing type for members @@ -67,8 +66,7 @@ private static bool IsInRequiresScope (this ISymbol member, string attributeName if (member.ContainingType is ITypeSymbol containingType && containingType.TryGetAttribute (attributeName, out requiresAttribute)) return true; - // Only check associated symbol if not override or virtual method - if (checkAssociatedSymbol && member is IMethodSymbol { AssociatedSymbol: { } associated } && associated.TryGetAttribute (attributeName, out requiresAttribute)) + if (member is IMethodSymbol { AssociatedSymbol: { } associated } && associated.TryGetAttribute (attributeName, out requiresAttribute)) return true; // When using instance fields suppress the warning if the constructor has already the Requires annotation diff --git a/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/RequiresAssemblyFilesAnalyzerTests.cs b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/RequiresAssemblyFilesAnalyzerTests.cs index 7b5e1e923bb477..94dcec893b2975 100644 --- a/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/RequiresAssemblyFilesAnalyzerTests.cs +++ b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/RequiresAssemblyFilesAnalyzerTests.cs @@ -105,9 +105,9 @@ void M() """; return VerifyRequiresAssemblyFilesAnalyzer (TestRequiresAssemblyFilesOnProperty, // (11,3): warning IL3002: Using member 'C.P' which has 'RequiresAssemblyFilesAttribute' can break functionality when embedded in a single-file app. - VerifyCS.Diagnostic (DiagnosticId.RequiresAssemblyFiles).WithSpan (11, 3, 11, 4).WithArguments ("C.P", "", ""), + VerifyCS.Diagnostic (DiagnosticId.RequiresAssemblyFiles).WithSpan (11, 3, 11, 4).WithArguments ("C.P.set", "", ""), // (12,35): warning IL3002: Using member 'C.P' which has 'RequiresAssemblyFilesAttribute' can break functionality when embedded in a single-file app. - VerifyCS.Diagnostic (DiagnosticId.RequiresAssemblyFiles).WithSpan (12, 35, 12, 36).WithArguments ("C.P", "", "")); + VerifyCS.Diagnostic (DiagnosticId.RequiresAssemblyFiles).WithSpan (12, 35, 12, 36).WithArguments ("C.P.get", "", "")); } [Fact] @@ -142,7 +142,7 @@ void M () """; return VerifyRequiresAssemblyFilesAnalyzer (TestRequiresAssemblyFilesOnMethodInsideProperty, // (23,3): warning IL3002: Using member 'C.P' which has 'RequiresAssemblyFilesAttribute' can break functionality when embedded in a single-file app. - VerifyCS.Diagnostic (DiagnosticId.RequiresAssemblyFiles).WithSpan (23, 3, 23, 4).WithArguments ("C.P", "", "")); + VerifyCS.Diagnostic (DiagnosticId.RequiresAssemblyFiles).WithSpan (23, 3, 23, 4).WithArguments ("C.P.set", "", "")); } [Fact] @@ -227,7 +227,7 @@ class C return VerifyRequiresAssemblyFilesAnalyzer (src, // (5,26): warning IL3000: 'System.Reflection.Assembly.Location' always returns an empty string for assemblies embedded in a single-file app. If the path to the app directory is needed, consider calling 'System.AppContext.BaseDirectory'. - VerifyCS.Diagnostic (DiagnosticId.AvoidAssemblyLocationInSingleFile).WithSpan (4, 23, 4, 63).WithArguments ("System.Reflection.Assembly.Location")); + VerifyCS.Diagnostic (DiagnosticId.AvoidAssemblyLocationInSingleFile).WithSpan (4, 23, 4, 63).WithArguments ("System.Reflection.Assembly.Location.get")); } [Fact] @@ -249,7 +249,7 @@ public void M() """; return VerifyRequiresAssemblyFilesAnalyzer (src, // (7,7): warning IL3000: 'System.Reflection.Assembly.Location' always returns an empty string for assemblies embedded in a single-file app. If the path to the app directory is needed, consider calling 'System.AppContext.BaseDirectory'. - VerifyCS.Diagnostic (DiagnosticId.AvoidAssemblyLocationInSingleFile).WithSpan (7, 7, 7, 17).WithArguments ("System.Reflection.Assembly.Location") + VerifyCS.Diagnostic (DiagnosticId.AvoidAssemblyLocationInSingleFile).WithSpan (7, 7, 7, 17).WithArguments ("System.Reflection.Assembly.Location.get") ); } @@ -297,9 +297,9 @@ public void M() // (8,7): warning SYSLIB0044: 'AssemblyName.EscapedCodeBase' is obsolete: 'AssemblyName.CodeBase and AssemblyName.EscapedCodeBase are obsolete. Using them for loading an assembly is not supported.' DiagnosticResult.CompilerWarning ("SYSLIB0044").WithSpan (8, 7, 8, 24).WithArguments ("System.Reflection.AssemblyName.EscapedCodeBase", "AssemblyName.CodeBase and AssemblyName.EscapedCodeBase are obsolete. Using them for loading an assembly is not supported."), // (7,7): warning IL3000: 'System.Reflection.AssemblyName.CodeBase' always returns an empty string for assemblies embedded in a single-file app. If the path to the app directory is needed, consider calling 'System.AppContext.BaseDirectory'. - VerifyCS.Diagnostic (DiagnosticId.AvoidAssemblyLocationInSingleFile).WithSpan (7, 7, 7, 17).WithArguments ("System.Reflection.AssemblyName.CodeBase"), + VerifyCS.Diagnostic (DiagnosticId.AvoidAssemblyLocationInSingleFile).WithSpan (7, 7, 7, 17).WithArguments ("System.Reflection.AssemblyName.CodeBase.get"), // (8,7): warning IL3000: 'System.Reflection.AssemblyName.EscapedCodeBase' always returns an empty string for assemblies embedded in a single-file app. If the path to the app directory is needed, consider calling 'System.AppContext.BaseDirectory'. - VerifyCS.Diagnostic (DiagnosticId.AvoidAssemblyLocationInSingleFile).WithSpan (8, 7, 8, 24).WithArguments ("System.Reflection.AssemblyName.EscapedCodeBase") + VerifyCS.Diagnostic (DiagnosticId.AvoidAssemblyLocationInSingleFile).WithSpan (8, 7, 8, 24).WithArguments ("System.Reflection.AssemblyName.EscapedCodeBase.get") ); } @@ -322,7 +322,7 @@ public void M() """; return VerifyRequiresAssemblyFilesAnalyzer (src, // (7,7): warning IL3000: 'System.Reflection.Assembly.Location' always returns an empty string for assemblies embedded in a single-file app. If the path to the app directory is needed, consider calling 'System.AppContext.BaseDirectory'. - VerifyCS.Diagnostic (DiagnosticId.AvoidAssemblyLocationInSingleFile).WithSpan (7, 7, 7, 17).WithArguments ("System.Reflection.Assembly.Location"), + VerifyCS.Diagnostic (DiagnosticId.AvoidAssemblyLocationInSingleFile).WithSpan (7, 7, 7, 17).WithArguments ("System.Reflection.Assembly.Location.get"), // (8,7): warning IL3001: Assemblies embedded in a single-file app cannot have additional files in the manifest. VerifyCS.Diagnostic (DiagnosticId.AvoidAssemblyGetFilesInSingleFile).WithSpan (8, 7, 8, 19).WithArguments ("System.Reflection.Assembly.GetFiles()") ); @@ -476,7 +476,7 @@ public void M2() { fixedSource: fixtest, baselineExpected: new[] { // /0/Test0.cs(6,24): warning IL3000: 'System.Reflection.Assembly.Location' always returns an empty string for assemblies embedded in a single-file app. If the path to the app directory is needed, consider calling 'System.AppContext.BaseDirectory'. - VerifyCS.Diagnostic (DiagnosticId.AvoidAssemblyLocationInSingleFile).WithSpan (6, 24, 6, 41).WithArguments ("System.Reflection.Assembly.Location", "", ""), + VerifyCS.Diagnostic (DiagnosticId.AvoidAssemblyLocationInSingleFile).WithSpan (6, 24, 6, 41).WithArguments ("System.Reflection.Assembly.Location.get", "", ""), // /0/Test0.cs(8,7): warning IL3001: 'System.Reflection.Assembly.GetFiles()' will throw for assemblies embedded in a single-file app VerifyCS.Diagnostic (DiagnosticId.AvoidAssemblyGetFilesInSingleFile).WithSpan (8, 7, 8, 26).WithArguments("System.Reflection.Assembly.GetFiles()", "", ""), }, diff --git a/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/UnconditionalSuppressMessageCodeFixTests.cs b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/UnconditionalSuppressMessageCodeFixTests.cs index c59a2763fe4bdb..ad9c1bebb6a18c 100644 --- a/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/UnconditionalSuppressMessageCodeFixTests.cs +++ b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/UnconditionalSuppressMessageCodeFixTests.cs @@ -226,7 +226,7 @@ public void M2() { fixtest, baselineExpected: new[] { // /0/Test0.cs(7,27): warning IL3000: 'System.Reflection.Assembly.Location' always returns an empty string for assemblies embedded in a single-file app. If the path to the app directory is needed, consider calling 'System.AppContext.BaseDirectory'. - VerifyCSUSMwithRAF.Diagnostic(DiagnosticId.AvoidAssemblyLocationInSingleFile).WithSpan (7, 27, 7, 44).WithArguments ("System.Reflection.Assembly.Location", "", ""), + VerifyCSUSMwithRAF.Diagnostic(DiagnosticId.AvoidAssemblyLocationInSingleFile).WithSpan (7, 27, 7, 44).WithArguments ("System.Reflection.Assembly.Location.get", "", ""), // /0/Test0.cs(9,13): warning IL3001: 'System.Reflection.Assembly.GetFiles()' will throw for assemblies embedded in a single-file app VerifyCSUSMwithRAF.Diagnostic(DiagnosticId.AvoidAssemblyGetFilesInSingleFile).WithSpan (9, 13, 9, 32).WithArguments("System.Reflection.Assembly.GetFiles()", "", ""), }, diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresAttributeMismatch.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresAttributeMismatch.cs index ee4338b4501b37..d942eecee0abc4 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresAttributeMismatch.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresAttributeMismatch.cs @@ -196,9 +196,9 @@ public override string VirtualPropertyAnnotationInAccesor { [RequiresAssemblyFiles ("Message")] [ExpectedWarning ("IL3003", "DerivedClassWithRequires.VirtualPropertyAnnotationInProperty", "BaseClassWithoutRequires.VirtualPropertyAnnotationInProperty", ProducedBy = Tool.Analyzer)] public override string VirtualPropertyAnnotationInProperty { - [ExpectedWarning ("IL3003", "DerivedClassWithRequires.VirtualPropertyAnnotationInProperty", "BaseClassWithoutRequires.VirtualPropertyAnnotationInProperty", ProducedBy = Tool.NativeAot)] + [ExpectedWarning ("IL3003", "DerivedClassWithRequires.VirtualPropertyAnnotationInProperty", "BaseClassWithoutRequires.VirtualPropertyAnnotationInProperty", ProducedBy = Tool.Analyzer | Tool.NativeAot)] get; - [ExpectedWarning ("IL3003", "DerivedClassWithRequires.VirtualPropertyAnnotationInProperty", "BaseClassWithoutRequires.VirtualPropertyAnnotationInProperty", ProducedBy = Tool.NativeAot)] + [ExpectedWarning ("IL3003", "DerivedClassWithRequires.VirtualPropertyAnnotationInProperty", "BaseClassWithoutRequires.VirtualPropertyAnnotationInProperty", ProducedBy = Tool.Analyzer | Tool.NativeAot)] set; } } @@ -256,9 +256,9 @@ public override string VirtualPropertyAnnotationInAccesor { [ExpectedWarning ("IL3003", "DerivedClassWithoutRequires.VirtualPropertyAnnotationInProperty", "BaseClassWithRequires.VirtualPropertyAnnotationInProperty", ProducedBy = Tool.Analyzer)] public override string VirtualPropertyAnnotationInProperty { - [ExpectedWarning ("IL3003", "DerivedClassWithoutRequires.VirtualPropertyAnnotationInProperty", "BaseClassWithRequires.VirtualPropertyAnnotationInProperty", ProducedBy = Tool.NativeAot)] + [ExpectedWarning ("IL3003", "DerivedClassWithoutRequires.VirtualPropertyAnnotationInProperty", "BaseClassWithRequires.VirtualPropertyAnnotationInProperty", ProducedBy = Tool.Analyzer | Tool.NativeAot)] get; - [ExpectedWarning ("IL3003", "DerivedClassWithoutRequires.VirtualPropertyAnnotationInProperty", "BaseClassWithRequires.VirtualPropertyAnnotationInProperty", ProducedBy = Tool.NativeAot)] + [ExpectedWarning ("IL3003", "DerivedClassWithoutRequires.VirtualPropertyAnnotationInProperty", "BaseClassWithRequires.VirtualPropertyAnnotationInProperty", ProducedBy = Tool.Analyzer | Tool.NativeAot)] set; } @@ -267,7 +267,7 @@ public override string VirtualPropertyAnnotationInPropertyAndAccessor { [ExpectedWarning ("IL2046", "VirtualPropertyAnnotationInPropertyAndAccessor.get", "BaseClassWithRequires.VirtualPropertyAnnotationInPropertyAndAccessor.get")] [ExpectedWarning ("IL3003", "DerivedClassWithoutRequires.VirtualPropertyAnnotationInPropertyAndAccessor.get", "BaseClassWithRequires.VirtualPropertyAnnotationInPropertyAndAccessor.get", ProducedBy = Tool.Analyzer | Tool.NativeAot)] get; - [ExpectedWarning ("IL3003", "DerivedClassWithoutRequires.VirtualPropertyAnnotationInPropertyAndAccessor", "BaseClassWithRequires.VirtualPropertyAnnotationInPropertyAndAccessor", ProducedBy = Tool.NativeAot)] + [ExpectedWarning ("IL3003", "DerivedClassWithoutRequires.VirtualPropertyAnnotationInPropertyAndAccessor", "BaseClassWithRequires.VirtualPropertyAnnotationInPropertyAndAccessor", ProducedBy = Tool.Analyzer | Tool.NativeAot)] set; } } @@ -287,7 +287,6 @@ public override void VirtualMethod () [ExpectedWarning ("IL3003", "DerivedClassWithAllWarnings.VirtualPropertyAnnotationInAccesor", "BaseClassWithRequires.VirtualPropertyAnnotationInAccesor", ProducedBy = Tool.Analyzer)] public override string VirtualPropertyAnnotationInAccesor { [ExpectedWarning ("IL2046", "DerivedClassWithAllWarnings.VirtualPropertyAnnotationInAccesor.get", "BaseClassWithRequires.VirtualPropertyAnnotationInAccesor.get")] - [ExpectedWarning ("IL3003", "DerivedClassWithAllWarnings.VirtualPropertyAnnotationInAccesor.get", "BaseClassWithRequires.VirtualPropertyAnnotationInAccesor.get", ProducedBy = Tool.Analyzer)] [ExpectedWarning ("IL3051", "DerivedClassWithAllWarnings.VirtualPropertyAnnotationInAccesor.get", "BaseClassWithRequires.VirtualPropertyAnnotationInAccesor.get", ProducedBy = Tool.Analyzer | Tool.NativeAot)] get { return name; } [RequiresAssemblyFiles ("Message")] @@ -302,12 +301,10 @@ public override string VirtualPropertyAnnotationInProperty { [RequiresAssemblyFiles ("Message")] [RequiresUnreferencedCode ("Message")] [ExpectedWarning ("IL2046", "VirtualPropertyAnnotationInProperty.get", "BaseClassWithRequires.VirtualPropertyAnnotationInProperty.get")] - [ExpectedWarning ("IL3003", "DerivedClassWithAllWarnings.VirtualPropertyAnnotationInProperty.get", "BaseClassWithRequires.VirtualPropertyAnnotationInProperty.get", ProducedBy = Tool.Analyzer)] get; [RequiresAssemblyFiles ("Message")] [RequiresUnreferencedCode ("Message")] [ExpectedWarning ("IL2046", "VirtualPropertyAnnotationInProperty.set", "BaseClassWithRequires.VirtualPropertyAnnotationInProperty.set")] - [ExpectedWarning ("IL3003", "DerivedClassWithAllWarnings.VirtualPropertyAnnotationInProperty.set", "BaseClassWithRequires.VirtualPropertyAnnotationInProperty.set", ProducedBy = Tool.Analyzer)] set; } @@ -319,7 +316,6 @@ public override string VirtualPropertyAnnotationInPropertyAndAccessor { [RequiresAssemblyFiles ("Message")] [RequiresUnreferencedCode ("Message")] [ExpectedWarning ("IL2046", "VirtualPropertyAnnotationInPropertyAndAccessor.set", "BaseClassWithRequires.VirtualPropertyAnnotationInPropertyAndAccessor.set")] - [ExpectedWarning ("IL3003", "DerivedClassWithAllWarnings.VirtualPropertyAnnotationInPropertyAndAccessor.set", "BaseClassWithRequires.VirtualPropertyAnnotationInPropertyAndAccessor.set", ProducedBy = Tool.Analyzer)] set; } } @@ -389,9 +385,9 @@ public string PropertyAnnotationInAccesor { [RequiresAssemblyFiles ("Message")] [ExpectedWarning ("IL3003", "ImplementationClassWithRequires.PropertyAnnotationInProperty", "IBaseWithoutRequires.PropertyAnnotationInProperty", ProducedBy = Tool.Analyzer)] public string PropertyAnnotationInProperty { - [ExpectedWarning ("IL3003", "ImplementationClassWithRequires.PropertyAnnotationInProperty", "IBaseWithoutRequires.PropertyAnnotationInProperty", ProducedBy = Tool.NativeAot)] + [ExpectedWarning ("IL3003", "ImplementationClassWithRequires.PropertyAnnotationInProperty", "IBaseWithoutRequires.PropertyAnnotationInProperty", ProducedBy = Tool.NativeAot | Tool.Analyzer)] get; - [ExpectedWarning ("IL3003", "ImplementationClassWithRequires.PropertyAnnotationInProperty", "IBaseWithoutRequires.PropertyAnnotationInProperty", ProducedBy = Tool.NativeAot)] + [ExpectedWarning ("IL3003", "ImplementationClassWithRequires.PropertyAnnotationInProperty", "IBaseWithoutRequires.PropertyAnnotationInProperty", ProducedBy = Tool.NativeAot | Tool.Analyzer)] set; } @@ -403,7 +399,7 @@ public string PropertyAnnotationInPropertyAndAccessor { [ExpectedWarning ("IL2046", "ImplementationClassWithRequires.PropertyAnnotationInPropertyAndAccessor.get", "IBaseWithoutRequires.PropertyAnnotationInPropertyAndAccessor.get")] [ExpectedWarning ("IL3003", "ImplementationClassWithRequires.PropertyAnnotationInPropertyAndAccessor.get", "IBaseWithoutRequires.PropertyAnnotationInPropertyAndAccessor.get", ProducedBy = Tool.Analyzer | Tool.NativeAot)] get; - [ExpectedWarning ("IL3003", "ImplementationClassWithRequires.PropertyAnnotationInPropertyAndAccessor", "IBaseWithoutRequires.PropertyAnnotationInPropertyAndAccessor", ProducedBy = Tool.NativeAot)] + [ExpectedWarning ("IL3003", "ImplementationClassWithRequires.PropertyAnnotationInPropertyAndAccessor", "IBaseWithoutRequires.PropertyAnnotationInPropertyAndAccessor", ProducedBy = Tool.Analyzer | Tool.NativeAot)] set; } } @@ -439,9 +435,9 @@ string IBaseWithoutRequires.PropertyAnnotationInAccesor { [RequiresAssemblyFiles ("Message")] [ExpectedWarning ("IL3003", "ExplicitImplementationClassWithRequires.Mono.Linker.Tests.Cases.RequiresCapability.RequiresAttributeMismatch.IBaseWithoutRequires.PropertyAnnotationInProperty", "IBaseWithoutRequires.PropertyAnnotationInProperty", ProducedBy = Tool.Analyzer)] string IBaseWithoutRequires.PropertyAnnotationInProperty { - [ExpectedWarning ("IL3003", "ExplicitImplementationClassWithRequires.Mono.Linker.Tests.Cases.RequiresCapability.RequiresAttributeMismatch.IBaseWithoutRequires.PropertyAnnotationInProperty", "IBaseWithoutRequires.PropertyAnnotationInProperty", ProducedBy = Tool.NativeAot)] + [ExpectedWarning ("IL3003", "Mono.Linker.Tests.Cases.RequiresCapability.RequiresAttributeMismatch.IBaseWithoutRequires.PropertyAnnotationInProperty", "IBaseWithoutRequires.PropertyAnnotationInProperty", ProducedBy = Tool.Analyzer | Tool.NativeAot)] get; - [ExpectedWarning ("IL3003", "ExplicitImplementationClassWithRequires.Mono.Linker.Tests.Cases.RequiresCapability.RequiresAttributeMismatch.IBaseWithoutRequires.PropertyAnnotationInProperty", "IBaseWithoutRequires.PropertyAnnotationInProperty", ProducedBy = Tool.NativeAot)] + [ExpectedWarning ("IL3003", "Mono.Linker.Tests.Cases.RequiresCapability.RequiresAttributeMismatch.IBaseWithoutRequires.PropertyAnnotationInProperty", "IBaseWithoutRequires.PropertyAnnotationInProperty", ProducedBy = Tool.Analyzer | Tool.NativeAot)] set; } @@ -453,7 +449,7 @@ string IBaseWithoutRequires.PropertyAnnotationInPropertyAndAccessor { [ExpectedWarning ("IL2046", "PropertyAnnotationInPropertyAndAccessor.get", "IBaseWithoutRequires.PropertyAnnotationInPropertyAndAccessor.get")] [ExpectedWarning ("IL3003", "PropertyAnnotationInPropertyAndAccessor.get", "IBaseWithoutRequires.PropertyAnnotationInPropertyAndAccessor.get", ProducedBy = Tool.Analyzer | Tool.NativeAot)] get; - [ExpectedWarning ("IL3003", "PropertyAnnotationInPropertyAndAccessor", "IBaseWithoutRequires.PropertyAnnotationInPropertyAndAccessor", ProducedBy = Tool.NativeAot)] + [ExpectedWarning ("IL3003", "PropertyAnnotationInPropertyAndAccessor", "IBaseWithoutRequires.PropertyAnnotationInPropertyAndAccessor", ProducedBy = Tool.Analyzer | Tool.NativeAot)] set; } } @@ -478,9 +474,9 @@ public string PropertyAnnotationInAccesor { [ExpectedWarning ("IL3003", "ImplementationClassWithoutRequires.PropertyAnnotationInProperty", "IBaseWithRequires.PropertyAnnotationInProperty", ProducedBy = Tool.Analyzer)] public string PropertyAnnotationInProperty { - [ExpectedWarning ("IL3003", "ImplementationClassWithoutRequires.PropertyAnnotationInProperty", "IBaseWithRequires.PropertyAnnotationInProperty", ProducedBy = Tool.NativeAot)] + [ExpectedWarning ("IL3003", "ImplementationClassWithoutRequires.PropertyAnnotationInProperty", "IBaseWithRequires.PropertyAnnotationInProperty", ProducedBy = Tool.Analyzer | Tool.NativeAot)] get; - [ExpectedWarning ("IL3003", "ImplementationClassWithoutRequires.PropertyAnnotationInProperty", "IBaseWithRequires.PropertyAnnotationInProperty", ProducedBy = Tool.NativeAot)] + [ExpectedWarning ("IL3003", "ImplementationClassWithoutRequires.PropertyAnnotationInProperty", "IBaseWithRequires.PropertyAnnotationInProperty", ProducedBy = Tool.Analyzer | Tool.NativeAot)] set; } @@ -489,7 +485,6 @@ public string PropertyAnnotationInPropertyAndAccessor { [RequiresAssemblyFiles ("Message")] [RequiresUnreferencedCode ("Message")] [ExpectedWarning ("IL2046", "ImplementationClassWithoutRequires.PropertyAnnotationInPropertyAndAccessor.get", "IBaseWithRequires.PropertyAnnotationInPropertyAndAccessor.get")] - [ExpectedWarning ("IL3003", "ImplementationClassWithoutRequires.PropertyAnnotationInPropertyAndAccessor.get", "IBaseWithRequires.PropertyAnnotationInPropertyAndAccessor.get", ProducedBy = Tool.Analyzer)] get; [ExpectedWarning ("IL2046", "ImplementationClassWithoutRequires.PropertyAnnotationInPropertyAndAccessor.set", "IBaseWithRequires.PropertyAnnotationInPropertyAndAccessor.set")] [ExpectedWarning ("IL3003", "ImplementationClassWithoutRequires.PropertyAnnotationInPropertyAndAccessor.set", "IBaseWithRequires.PropertyAnnotationInPropertyAndAccessor.set", ProducedBy = Tool.Analyzer | Tool.NativeAot)] @@ -521,15 +516,15 @@ string IBaseWithRequires.PropertyAnnotationInAccesor { [ExpectedWarning ("IL3003", "ExplicitImplementationClassWithoutRequires.Mono.Linker.Tests.Cases.RequiresCapability.RequiresAttributeMismatch.IBaseWithRequires.PropertyAnnotationInProperty", "IBaseWithRequires.PropertyAnnotationInProperty", ProducedBy = Tool.Analyzer)] string IBaseWithRequires.PropertyAnnotationInProperty { - [ExpectedWarning ("IL3003", "ExplicitImplementationClassWithoutRequires.Mono.Linker.Tests.Cases.RequiresCapability.RequiresAttributeMismatch.IBaseWithRequires.PropertyAnnotationInProperty", "IBaseWithRequires.PropertyAnnotationInProperty", ProducedBy = Tool.NativeAot)] + [ExpectedWarning ("IL3003", "Mono.Linker.Tests.Cases.RequiresCapability.RequiresAttributeMismatch.IBaseWithRequires.PropertyAnnotationInProperty", "IBaseWithRequires.PropertyAnnotationInProperty", ProducedBy = Tool.Analyzer | Tool.NativeAot)] get; - [ExpectedWarning ("IL3003", "ExplicitImplementationClassWithoutRequires.Mono.Linker.Tests.Cases.RequiresCapability.RequiresAttributeMismatch.IBaseWithRequires.PropertyAnnotationInProperty", "IBaseWithRequires.PropertyAnnotationInProperty", ProducedBy = Tool.NativeAot)] + [ExpectedWarning ("IL3003", "Mono.Linker.Tests.Cases.RequiresCapability.RequiresAttributeMismatch.IBaseWithRequires.PropertyAnnotationInProperty", "IBaseWithRequires.PropertyAnnotationInProperty", ProducedBy = Tool.Analyzer | Tool.NativeAot)] set; } [ExpectedWarning ("IL3003", "ExplicitImplementationClassWithoutRequires.Mono.Linker.Tests.Cases.RequiresCapability.RequiresAttributeMismatch.IBaseWithRequires.PropertyAnnotationInPropertyAndAccessor", "IBaseWithRequires.PropertyAnnotationInPropertyAndAccessor", ProducedBy = Tool.Analyzer)] string IBaseWithRequires.PropertyAnnotationInPropertyAndAccessor { - [ExpectedWarning ("IL3003", "ExplicitImplementationClassWithoutRequires.Mono.Linker.Tests.Cases.RequiresCapability.RequiresAttributeMismatch.IBaseWithRequires.PropertyAnnotationInPropertyAndAccessor", "IBaseWithRequires.PropertyAnnotationInPropertyAndAccessor", ProducedBy = Tool.NativeAot)] + [ExpectedWarning ("IL3003", "Mono.Linker.Tests.Cases.RequiresCapability.RequiresAttributeMismatch.IBaseWithRequires.PropertyAnnotationInPropertyAndAccessor", "IBaseWithRequires.PropertyAnnotationInPropertyAndAccessor", ProducedBy = Tool.Analyzer | Tool.NativeAot)] get; [ExpectedWarning ("IL2046", "PropertyAnnotationInPropertyAndAccessor.set", "IBaseWithRequires.PropertyAnnotationInPropertyAndAccessor.set")] [ExpectedWarning ("IL3003", "PropertyAnnotationInPropertyAndAccessor.set", "IBaseWithRequires.PropertyAnnotationInPropertyAndAccessor.set", ProducedBy = Tool.Analyzer | Tool.NativeAot)] @@ -557,9 +552,9 @@ public string PropertyAnnotationInAccesor { [ExpectedWarning ("IL3003", "ImplementationClassWithoutRequiresInSource.PropertyAnnotationInProperty", "IBaseWithRequiresInReference.PropertyAnnotationInProperty", ProducedBy = Tool.Analyzer)] public string PropertyAnnotationInProperty { - [ExpectedWarning ("IL3003", "ImplementationClassWithoutRequiresInSource.PropertyAnnotationInProperty.get", "IBaseWithRequiresInReference.PropertyAnnotationInProperty.get", ProducedBy = Tool.NativeAot)] + [ExpectedWarning ("IL3003", "ImplementationClassWithoutRequiresInSource.PropertyAnnotationInProperty.get", "IBaseWithRequiresInReference.PropertyAnnotationInProperty.get", ProducedBy = Tool.Analyzer | Tool.NativeAot)] get; - [ExpectedWarning ("IL3003", "ImplementationClassWithoutRequiresInSource.PropertyAnnotationInProperty.set", "IBaseWithRequiresInReference.PropertyAnnotationInProperty.set", ProducedBy = Tool.NativeAot)] + [ExpectedWarning ("IL3003", "ImplementationClassWithoutRequiresInSource.PropertyAnnotationInProperty.set", "IBaseWithRequiresInReference.PropertyAnnotationInProperty.set", ProducedBy = Tool.Analyzer | Tool.NativeAot)] set; } } @@ -591,9 +586,9 @@ public string PropertyAnnotationInAccesor { [ExpectedWarning ("IL3003", "ImplementationClassWithRequiresInSource.PropertyAnnotationInProperty", "IBaseWithoutRequiresInReference.PropertyAnnotationInProperty", ProducedBy = Tool.Analyzer)] [RequiresAssemblyFiles ("Message")] public string PropertyAnnotationInProperty { - [ExpectedWarning ("IL3003", "ImplementationClassWithRequiresInSource.PropertyAnnotationInProperty", "IBaseWithoutRequiresInReference.PropertyAnnotationInProperty", ProducedBy = Tool.NativeAot)] + [ExpectedWarning ("IL3003", "ImplementationClassWithRequiresInSource.PropertyAnnotationInProperty", "IBaseWithoutRequiresInReference.PropertyAnnotationInProperty", ProducedBy = Tool.Analyzer | Tool.NativeAot)] get; - [ExpectedWarning ("IL3003", "ImplementationClassWithRequiresInSource.PropertyAnnotationInProperty", "IBaseWithoutRequiresInReference.PropertyAnnotationInProperty", ProducedBy = Tool.NativeAot)] + [ExpectedWarning ("IL3003", "ImplementationClassWithRequiresInSource.PropertyAnnotationInProperty", "IBaseWithoutRequiresInReference.PropertyAnnotationInProperty", ProducedBy = Tool.Analyzer | Tool.NativeAot)] set; } } From 7efb349e1f4c3ad29914d4dc3109def125c33199 Mon Sep 17 00:00:00 2001 From: Sven Boemer Date: Thu, 21 Sep 2023 23:35:48 +0000 Subject: [PATCH 2/4] Stop warning on properties Equivalent warnings are now reported on the get/set methods. --- .../src/ILLink.RoslynAnalyzer/RequiresAnalyzerBase.cs | 8 -------- .../RequiresCapability/RequiresAttributeMismatch.cs | 6 ------ 2 files changed, 14 deletions(-) diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresAnalyzerBase.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresAnalyzerBase.cs index 81f60e7aeb2960..2c99f83b14aeea 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresAnalyzerBase.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresAnalyzerBase.cs @@ -54,14 +54,6 @@ public override void Initialize (AnalysisContext context) CheckMatchingAttributesInInterfaces (symbolAnalysisContext, typeSymbol); }, SymbolKind.NamedType); - - context.RegisterSymbolAction (symbolAnalysisContext => { - var propertySymbol = (IPropertySymbol) symbolAnalysisContext.Symbol; - if (AnalyzerDiagnosticTargets.HasFlag (DiagnosticTargets.Property)) { - CheckMatchingAttributesInOverrides (symbolAnalysisContext, propertySymbol); - } - }, SymbolKind.Property); - context.RegisterSymbolAction (symbolAnalysisContext => { var eventSymbol = (IEventSymbol) symbolAnalysisContext.Symbol; if (AnalyzerDiagnosticTargets.HasFlag (DiagnosticTargets.Event)) { diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresAttributeMismatch.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresAttributeMismatch.cs index d942eecee0abc4..9f018488acb28f 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresAttributeMismatch.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresAttributeMismatch.cs @@ -194,7 +194,6 @@ public override string VirtualPropertyAnnotationInAccesor { } [RequiresAssemblyFiles ("Message")] - [ExpectedWarning ("IL3003", "DerivedClassWithRequires.VirtualPropertyAnnotationInProperty", "BaseClassWithoutRequires.VirtualPropertyAnnotationInProperty", ProducedBy = Tool.Analyzer)] public override string VirtualPropertyAnnotationInProperty { [ExpectedWarning ("IL3003", "DerivedClassWithRequires.VirtualPropertyAnnotationInProperty", "BaseClassWithoutRequires.VirtualPropertyAnnotationInProperty", ProducedBy = Tool.Analyzer | Tool.NativeAot)] get; @@ -254,7 +253,6 @@ public override string VirtualPropertyAnnotationInAccesor { set { name = value; } } - [ExpectedWarning ("IL3003", "DerivedClassWithoutRequires.VirtualPropertyAnnotationInProperty", "BaseClassWithRequires.VirtualPropertyAnnotationInProperty", ProducedBy = Tool.Analyzer)] public override string VirtualPropertyAnnotationInProperty { [ExpectedWarning ("IL3003", "DerivedClassWithoutRequires.VirtualPropertyAnnotationInProperty", "BaseClassWithRequires.VirtualPropertyAnnotationInProperty", ProducedBy = Tool.Analyzer | Tool.NativeAot)] get; @@ -262,7 +260,6 @@ public override string VirtualPropertyAnnotationInProperty { set; } - [ExpectedWarning ("IL3003", "DerivedClassWithoutRequires.VirtualPropertyAnnotationInPropertyAndAccessor", "BaseClassWithRequires.VirtualPropertyAnnotationInPropertyAndAccessor", ProducedBy = Tool.Analyzer)] public override string VirtualPropertyAnnotationInPropertyAndAccessor { [ExpectedWarning ("IL2046", "VirtualPropertyAnnotationInPropertyAndAccessor.get", "BaseClassWithRequires.VirtualPropertyAnnotationInPropertyAndAccessor.get")] [ExpectedWarning ("IL3003", "DerivedClassWithoutRequires.VirtualPropertyAnnotationInPropertyAndAccessor.get", "BaseClassWithRequires.VirtualPropertyAnnotationInPropertyAndAccessor.get", ProducedBy = Tool.Analyzer | Tool.NativeAot)] @@ -284,7 +281,6 @@ public override void VirtualMethod () private string name; [RequiresAssemblyFiles ("Message")] - [ExpectedWarning ("IL3003", "DerivedClassWithAllWarnings.VirtualPropertyAnnotationInAccesor", "BaseClassWithRequires.VirtualPropertyAnnotationInAccesor", ProducedBy = Tool.Analyzer)] public override string VirtualPropertyAnnotationInAccesor { [ExpectedWarning ("IL2046", "DerivedClassWithAllWarnings.VirtualPropertyAnnotationInAccesor.get", "BaseClassWithRequires.VirtualPropertyAnnotationInAccesor.get")] [ExpectedWarning ("IL3051", "DerivedClassWithAllWarnings.VirtualPropertyAnnotationInAccesor.get", "BaseClassWithRequires.VirtualPropertyAnnotationInAccesor.get", ProducedBy = Tool.Analyzer | Tool.NativeAot)] @@ -296,7 +292,6 @@ public override string VirtualPropertyAnnotationInAccesor { set { name = value; } } - [ExpectedWarning ("IL3003", "DerivedClassWithAllWarnings.VirtualPropertyAnnotationInProperty", "BaseClassWithRequires.VirtualPropertyAnnotationInProperty", ProducedBy = Tool.Analyzer)] public override string VirtualPropertyAnnotationInProperty { [RequiresAssemblyFiles ("Message")] [RequiresUnreferencedCode ("Message")] @@ -308,7 +303,6 @@ public override string VirtualPropertyAnnotationInProperty { set; } - [ExpectedWarning ("IL3003", "DerivedClassWithAllWarnings.VirtualPropertyAnnotationInPropertyAndAccessor", "BaseClassWithRequires.VirtualPropertyAnnotationInPropertyAndAccessor", ProducedBy = Tool.Analyzer)] public override string VirtualPropertyAnnotationInPropertyAndAccessor { [ExpectedWarning ("IL2046", "VirtualPropertyAnnotationInPropertyAndAccessor.get", "BaseClassWithRequires.VirtualPropertyAnnotationInPropertyAndAccessor.get")] [ExpectedWarning ("IL3003", "DerivedClassWithAllWarnings.VirtualPropertyAnnotationInPropertyAndAccessor.get", "BaseClassWithRequires.VirtualPropertyAnnotationInPropertyAndAccessor.get", ProducedBy = Tool.Analyzer | Tool.NativeAot)] From b6129a0e405e3052869871bc22178277c88a8cbd Mon Sep 17 00:00:00 2001 From: Sven Boemer Date: Thu, 21 Sep 2023 23:59:16 +0000 Subject: [PATCH 3/4] Same treatment for events --- .../ILLink.RoslynAnalyzer/RequiresAnalyzerBase.cs | 10 ---------- .../RequiresISymbolExtensions.cs | 12 ++---------- .../RequiresAssemblyFilesAnalyzerTests.cs | 5 +++-- .../RequiresCapability/BasicRequires.cs | 4 ++++ 4 files changed, 9 insertions(+), 22 deletions(-) diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresAnalyzerBase.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresAnalyzerBase.cs index 2c99f83b14aeea..8245f4c6a8ddd8 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresAnalyzerBase.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresAnalyzerBase.cs @@ -54,13 +54,6 @@ public override void Initialize (AnalysisContext context) CheckMatchingAttributesInInterfaces (symbolAnalysisContext, typeSymbol); }, SymbolKind.NamedType); - context.RegisterSymbolAction (symbolAnalysisContext => { - var eventSymbol = (IEventSymbol) symbolAnalysisContext.Symbol; - if (AnalyzerDiagnosticTargets.HasFlag (DiagnosticTargets.Event)) { - CheckMatchingAttributesInOverrides (symbolAnalysisContext, eventSymbol); - } - }, SymbolKind.Event); - context.RegisterOperationAction (operationContext => { var methodInvocation = (IInvocationOperation) operationContext.Operation; CheckCalledMember (operationContext, methodInvocation.TargetMethod, incompatibleMembers); @@ -103,9 +96,6 @@ public override void Initialize (AnalysisContext context) if (eventSymbol.RaiseMethod is IMethodSymbol eventRaiseMethod) CheckCalledMember (operationContext, eventRaiseMethod, incompatibleMembers); - - if (AnalyzerDiagnosticTargets.HasFlag (DiagnosticTargets.Event)) - CheckCalledMember (operationContext, eventSymbol, incompatibleMembers); }, OperationKind.EventReference); context.RegisterOperationAction (operationContext => { diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresISymbolExtensions.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresISymbolExtensions.cs index f7688e1fd4cf41..a5e4622a9c92cb 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresISymbolExtensions.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresISymbolExtensions.cs @@ -18,16 +18,8 @@ public static bool DoesMemberRequire (this ISymbol member, string requiresAttrib if (!member.IsStaticConstructor () && member.TryGetAttribute (requiresAttribute, out requiresAttributeData)) return true; - if (member is IMethodSymbol method) { - switch (method.MethodKind) { - case MethodKind.PropertyGet: - case MethodKind.PropertySet: - var propertySymbol = (IPropertySymbol) method.AssociatedSymbol!; - if (propertySymbol.TryGetAttribute (requiresAttribute, out requiresAttributeData)) - return true; - break; - } - } + if (member is IMethodSymbol { AssociatedSymbol: { } associated } && associated.TryGetAttribute (requiresAttribute, out requiresAttributeData)) + return true; // Also check the containing type if (member.IsStatic || member.IsConstructor ()) diff --git a/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/RequiresAssemblyFilesAnalyzerTests.cs b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/RequiresAssemblyFilesAnalyzerTests.cs index 94dcec893b2975..0b2d44da17b55e 100644 --- a/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/RequiresAssemblyFilesAnalyzerTests.cs +++ b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/RequiresAssemblyFilesAnalyzerTests.cs @@ -75,13 +75,14 @@ class C void M() { - var handler = E; + E += (sender, e) => { }; + var evt = E; } } """; return VerifyRequiresAssemblyFilesAnalyzer (TestRequiresAssemblyFieldsOnEvent, // (11,17): warning IL3002: Using member 'C.E' which has 'RequiresAssemblyFilesAttribute' can break functionality when embedded in a single-file app. - VerifyCS.Diagnostic (DiagnosticId.RequiresAssemblyFiles).WithSpan (11, 17, 11, 18).WithArguments ("C.E", "", "")); + VerifyCS.Diagnostic (DiagnosticId.RequiresAssemblyFiles).WithSpan (11, 3, 11, 4).WithArguments ("C.E.add", "", "")); } [Fact] diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/BasicRequires.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/BasicRequires.cs index 0d6b360ac71478..40c4f33ed27b82 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/BasicRequires.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/BasicRequires.cs @@ -155,6 +155,9 @@ static event EventHandler EventToTestAdd { remove { } } + [RequiresAssemblyFiles ("Message for --AnnotatedEvent--")] + static event EventHandler AnnotatedEvent; + [ExpectedWarning ("IL2026", "--EventToTestRemove.remove--")] [ExpectedWarning ("IL3002", "--EventToTestRemove.remove--", ProducedBy = Tool.Analyzer | Tool.NativeAot)] [ExpectedWarning ("IL3050", "--EventToTestRemove.remove--", ProducedBy = Tool.Analyzer | Tool.NativeAot)] @@ -165,6 +168,7 @@ public static void Test () { EventToTestRemove -= (sender, e) => { }; EventToTestAdd += (sender, e) => { }; + var evt = AnnotatedEvent; } } From d806d44f6402b1cc1f7d0692172d752ef53f24bc Mon Sep 17 00:00:00 2001 From: Sven Boemer Date: Fri, 22 Sep 2023 00:02:05 +0000 Subject: [PATCH 4/4] Update comment --- .../RequiresCapability/RequiresAttributeMismatch.cs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresAttributeMismatch.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresAttributeMismatch.cs index 9f018488acb28f..4653f445cabb39 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresAttributeMismatch.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresAttributeMismatch.cs @@ -17,15 +17,14 @@ namespace Mono.Linker.Tests.Cases.RequiresCapability class RequiresAttributeMismatch { // General comment about IL3003 - // Analyzer looks at properties, events and methods separately. So mismatch on property level will be reported - // only on the property and not on the accessors. And vice versa. // NativeAOT doesn't really see properties and events, only methods. So it is much easier to handle everything // on the method level. While it's a discrepancy in behavior, it's small and should have no adverse effects // as suppressing the warning on the property level will suppress it for the accessors as well. // So NativeAOT will report all mismatches on the accessors. If the mismatch is on the property // then both accessors will report IL3003 (the attribute on the property is treated as if it was specified // on all accessors, always). - // This discrepancy is tracked by https://github.com/dotnet/runtime/issues/83235. + // The analyzer matches this behavior, treating the get/set methods as annotated if the property is annotated, + // and warning only on the get/set methods. [ExpectedWarning ("IL2026", "BaseClassWithRequires.VirtualPropertyAnnotationInAccesor.get")] [ExpectedWarning ("IL3002", "BaseClassWithRequires.VirtualPropertyAnnotationInAccesor.get", ProducedBy = Tool.NativeAot)] [ExpectedWarning ("IL3050", "BaseClassWithRequires.VirtualPropertyAnnotationInAccesor.get", ProducedBy = Tool.NativeAot)]