Analyzer warns when Requires... Attribute is on a static constructor (dotnet/linker#2455)

Adds warnings in the analyzer for IL2116 (RUC not allowed on static ctor) and adds IL2117 (RequiresDynamicCode not allowed on static ctor) and IL2118 (RequiresAssemblyFiles not allowed on static ctor).

Fixes ExpectedWarning not having effects on constructors by adding a Visit override in the compilation tree walker in the test infra.

Adds case in GetDisplayName to differentiate .cctor's and .ctor's.

Linker doesn't seem to be checking ctors for calls to methods with RUC.
This commit removes the check for the expected warnings in the linker
until the bug is fixed.

Commit migrated from dotnet/linker@2999a6a

Author: jtschuster

src/tools/illink/src/ILLink.RoslynAnalyzer/ISymbolExtensions.cs

@@ -52,9 +52,12 @@ public static string GetDisplayName (this ISymbol symbol)
 			case IParameterSymbol parameterSymbol:
 				sb.Append (parameterSymbol.Name);
 				break;
-
 			default:
-				sb.Append (symbol.ToDisplayString ());
+				if (symbol.IsStaticConstructor ()) {
+					sb.Append (symbol.ContainingType.ToDisplayString ());
+					sb.Append ("..cctor()");
+				} else
+					sb.Append (symbol.ToDisplayString ());
 				break;
 			}
 

src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresAnalyzerBase.cs

@@ -25,6 +25,7 @@ public abstract class RequiresAnalyzerBase : DiagnosticAnalyzer
 		private protected abstract DiagnosticDescriptor RequiresDiagnosticRule { get; }
 
 		private protected abstract DiagnosticDescriptor RequiresAttributeMismatch { get; }
+		private protected abstract DiagnosticDescriptor RequiresOnStaticCtor { get; }
 
 		private protected virtual ImmutableArray<(Action<OperationAnalysisContext> Action, OperationKind[] OperationKind)> ExtraOperationActions { get; } = ImmutableArray<(Action<OperationAnalysisContext> Action, OperationKind[] OperationKind)>.Empty;
 
@@ -43,6 +44,8 @@ public override void Initialize (AnalysisContext context)
 				var incompatibleMembers = GetSpecialIncompatibleMembers (compilation);
 				context.RegisterSymbolAction (symbolAnalysisContext => {
 					var methodSymbol = (IMethodSymbol) symbolAnalysisContext.Symbol;
+					if (methodSymbol.IsStaticConstructor () && methodSymbol.HasAttribute (RequiresAttributeName))
+						ReportRequiresOnStaticCtorDiagnostic (symbolAnalysisContext, methodSymbol);
 					CheckMatchingAttributesInOverrides (symbolAnalysisContext, methodSymbol);
 					CheckAttributeInstantiation (symbolAnalysisContext, methodSymbol);
 					foreach (var typeParameter in methodSymbol.TypeParameters)
@@ -332,6 +335,14 @@ private void ReportRequiresDiagnostic (OperationAnalysisContext operationContext
 				url));
 		}
 
+		private void ReportRequiresOnStaticCtorDiagnostic (SymbolAnalysisContext symbolAnalysisContext, IMethodSymbol ctor)
+		{
+			symbolAnalysisContext.ReportDiagnostic (Diagnostic.Create (
+				RequiresOnStaticCtor,
+				ctor.Locations[0],
+				ctor.GetDisplayName ()));
+		}
+
 		private void ReportMismatchInAttributesDiagnostic (SymbolAnalysisContext symbolAnalysisContext, ISymbol member, ISymbol baseMember, bool isInterface = false)
 		{
 			string message = MessageFormat.FormatRequiresAttributeMismatch (member.HasAttribute (RequiresAttributeName), isInterface, RequiresAttributeName, member.GetDisplayName (), baseMember.GetDisplayName ());

src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresAssemblyFilesAnalyzer.cs

@@ -26,7 +26,9 @@ public sealed class RequiresAssemblyFilesAnalyzer : RequiresAnalyzerBase
 
 		static readonly DiagnosticDescriptor s_requiresAssemblyFilesAttributeMismatch = DiagnosticDescriptors.GetDiagnosticDescriptor (DiagnosticId.RequiresAssemblyFilesAttributeMismatch);
 
-		public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create (s_locationRule, s_getFilesRule, s_requiresAssemblyFilesRule, s_requiresAssemblyFilesAttributeMismatch);
+		static readonly DiagnosticDescriptor s_requiresAssemblyFilesOnStaticCtor = DiagnosticDescriptors.GetDiagnosticDescriptor (DiagnosticId.RequiresAssemblyFilesOnStaticConstructor);
+
+		public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create (s_locationRule, s_getFilesRule, s_requiresAssemblyFilesRule, s_requiresAssemblyFilesAttributeMismatch, s_requiresAssemblyFilesOnStaticCtor);
 
 		private protected override string RequiresAttributeName => RequiresAssemblyFilesAttribute;
 
@@ -38,6 +40,8 @@ public sealed class RequiresAssemblyFilesAnalyzer : RequiresAnalyzerBase
 
 		private protected override DiagnosticDescriptor RequiresAttributeMismatch => s_requiresAssemblyFilesAttributeMismatch;
 
+		private protected override DiagnosticDescriptor RequiresOnStaticCtor => s_requiresAssemblyFilesOnStaticCtor;
+
 		protected override bool IsAnalyzerEnabled (AnalyzerOptions options, Compilation compilation)
 		{
 			var isSingleFileAnalyzerEnabled = options.GetMSBuildPropertyValue (MSBuildPropertyOptionNames.EnableSingleFileAnalyzer, compilation);

src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresDynamicCodeAnalyzer.cs

@@ -14,11 +14,12 @@ public sealed class RequiresDynamicCodeAnalyzer : RequiresAnalyzerBase
 		const string RequiresDynamicCodeAttribute = nameof (RequiresDynamicCodeAttribute);
 		public const string FullyQualifiedRequiresDynamicCodeAttribute = "System.Diagnostics.CodeAnalysis." + RequiresDynamicCodeAttribute;
 
+		static readonly DiagnosticDescriptor s_requiresDynaicCodeOnStaticCtor = DiagnosticDescriptors.GetDiagnosticDescriptor (DiagnosticId.RequiresDynamicCodeOnStaticConstructor);
 		static readonly DiagnosticDescriptor s_requiresDynamicCodeRule = DiagnosticDescriptors.GetDiagnosticDescriptor (DiagnosticId.RequiresDynamicCode);
 		static readonly DiagnosticDescriptor s_requiresDynamicCodeAttributeMismatch = DiagnosticDescriptors.GetDiagnosticDescriptor (DiagnosticId.RequiresDynamicCodeAttributeMismatch);
 
 		public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics =>
-			ImmutableArray.Create (s_requiresDynamicCodeRule, s_requiresDynamicCodeAttributeMismatch);
+			ImmutableArray.Create (s_requiresDynamicCodeRule, s_requiresDynamicCodeAttributeMismatch, s_requiresDynaicCodeOnStaticCtor);
 
 		private protected override string RequiresAttributeName => RequiresDynamicCodeAttribute;
 
@@ -30,6 +31,8 @@ public sealed class RequiresDynamicCodeAnalyzer : RequiresAnalyzerBase
 
 		private protected override DiagnosticDescriptor RequiresAttributeMismatch => s_requiresDynamicCodeAttributeMismatch;
 
+		private protected override DiagnosticDescriptor RequiresOnStaticCtor => s_requiresDynaicCodeOnStaticCtor;
+
 		protected override bool IsAnalyzerEnabled (AnalyzerOptions options, Compilation compilation) =>
 			options.IsMSBuildPropertyValueTrue (MSBuildPropertyOptionNames.EnableAOTAnalyzer, compilation);
 

src/tools/illink/src/ILLink.RoslynAnalyzer/RequiresUnreferencedCodeAnalyzer.cs

@@ -22,6 +22,7 @@ public sealed class RequiresUnreferencedCodeAnalyzer : RequiresAnalyzerBase
 			new LocalizableResourceString (nameof (SharedStrings.DynamicTypeInvocationMessage), SharedStrings.ResourceManager, typeof (SharedStrings)));
 		static readonly DiagnosticDescriptor s_makeGenericTypeRule = DiagnosticDescriptors.GetDiagnosticDescriptor (DiagnosticId.MakeGenericType);
 		static readonly DiagnosticDescriptor s_makeGenericMethodRule = DiagnosticDescriptors.GetDiagnosticDescriptor (DiagnosticId.MakeGenericMethod);
+		static readonly DiagnosticDescriptor s_requiresUnreferencedCodeOnStaticCtor = DiagnosticDescriptors.GetDiagnosticDescriptor (DiagnosticId.RequiresUnreferencedCodeOnStaticConstructor);
 
 		static readonly DiagnosticDescriptor s_typeDerivesFromRucClassRule = DiagnosticDescriptors.GetDiagnosticDescriptor (DiagnosticId.RequiresUnreferencedCodeOnBaseClass);
 
@@ -53,7 +54,7 @@ private Action<SymbolAnalysisContext> typeDerivesFromRucBase {
 		}
 
 		public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics =>
-			ImmutableArray.Create (s_dynamicTypeInvocationRule, s_makeGenericMethodRule, s_makeGenericTypeRule, s_requiresUnreferencedCodeRule, s_requiresUnreferencedCodeAttributeMismatch, s_typeDerivesFromRucClassRule);
+			ImmutableArray.Create (s_dynamicTypeInvocationRule, s_makeGenericMethodRule, s_makeGenericTypeRule, s_requiresUnreferencedCodeRule, s_requiresUnreferencedCodeAttributeMismatch, s_typeDerivesFromRucClassRule, s_requiresUnreferencedCodeOnStaticCtor);
 
 		private protected override string RequiresAttributeName => RequiresUnreferencedCodeAttribute;
 
@@ -65,6 +66,8 @@ private Action<SymbolAnalysisContext> typeDerivesFromRucBase {
 
 		private protected override DiagnosticDescriptor RequiresAttributeMismatch => s_requiresUnreferencedCodeAttributeMismatch;
 
+		private protected override DiagnosticDescriptor RequiresOnStaticCtor => s_requiresUnreferencedCodeOnStaticCtor;
+
 		protected override bool IsAnalyzerEnabled (AnalyzerOptions options, Compilation compilation) =>
 			options.IsMSBuildPropertyValueTrue (MSBuildPropertyOptionNames.EnableTrimAnalyzer, compilation);
 

src/tools/illink/src/ILLink.Shared/DiagnosticId.cs

@@ -179,10 +179,12 @@ public enum DiagnosticId
 		AvoidAssemblyGetFilesInSingleFile = 3001,
 		RequiresAssemblyFiles = 3002,
 		RequiresAssemblyFilesAttributeMismatch = 3003,
+		RequiresAssemblyFilesOnStaticConstructor = 3004,
 
 		// Dynamic code diagnostic ids.
 		RequiresDynamicCode = 3050,
-		RequiresDynamicCodeAttributeMismatch = 3051
+		RequiresDynamicCodeAttributeMismatch = 3051,
+		RequiresDynamicCodeOnStaticConstructor = 3052
 	}
 
 	public static class DiagnosticIdExtensions

src/tools/illink/src/ILLink.Shared/SharedStrings.resx

@@ -1113,4 +1113,22 @@
   <data name="InterfaceRequiresMismatchMessage" xml:space="preserve">
     <value>Interface member '{2}' with '{0}' has an implementation member '{1}' without '{0}'</value>
   </data>
-</root>
+  <data name="RequiresOnBaseClassMessage" xml:space="preserve">
+    <value>Type '{0}' derives from '{1}' which has 'RequiresUnreferencedCodeAttribute'. {2}{3}</value>
+  </data>
+  <data name="RequiresOnBaseClassTitle" xml:space="preserve">
+    <value>Types that derive from a base class with 'RequiresUnreferencedCodeAttribute' need to explicitly use the 'RequiresUnreferencedCodeAttribute' or suppress this warning</value>
+  </data>
+  <data name="RequiresDynamicCodeOnStaticConstructorMessage" xml:space="preserve">
+    <value>'RequiresDynamicCodeAttribute' cannot be placed directly on static constructor '{0}'.</value>
+  </data>
+  <data name="RequiresDynamicCodeOnStaticConstructorTitle" xml:space="preserve">
+    <value>The use of 'RequiresDynamicCodeAttribute' on static constructors is disallowed since is a method not callable by the user, is only called by the runtime. Placing the attribute directly on the static constructor will have no effect, instead use 'RequiresUnreferencedCodeAttribute' on the type which will handle warning and silencing from the static constructor.</value>
+  </data>
+  <data name="RequiresAssemblyFilesOnStaticConstructorMessage" xml:space="preserve">
+    <value>'RequiresAssemblyFilesAttribute' cannot be placed directly on static constructor '{0}'.</value>
+  </data>
+  <data name="RequiresAssemblyFilesOnStaticConstructorTitle" xml:space="preserve">
+    <value>The use of 'RequiresAssemblyFilesAttribute' on static constructors is disallowed since is a method not callable by the user, is only called by the runtime. Placing the attribute directly on the static constructor will have no effect, instead use 'RequiresUnreferencedCodeAttribute' on the type which will handle warning and silencing from the static constructor.</value>
+  </data>
+</root>

src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/TestChecker.cs

@@ -71,6 +71,12 @@ public override void VisitClassDeclaration (ClassDeclarationSyntax node)
 			CheckMember (node);
 		}
 
+		public override void VisitConstructorDeclaration (ConstructorDeclarationSyntax node)
+		{
+			base.VisitConstructorDeclaration (node);
+			CheckMember (node);
+		}
+
 		public override void VisitInterfaceDeclaration (InterfaceDeclarationSyntax node)
 		{
 			base.VisitInterfaceDeclaration (node);

src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresOnStaticConstructor.cs

@@ -28,7 +28,7 @@ public static void Main ()
 
 		class StaticCtor
 		{
-			[ExpectedWarning ("IL2116", "StaticCtor..cctor()", ProducedBy = ProducedBy.Trimmer)]
+			[ExpectedWarning ("IL2116", "StaticCtor..cctor()")]
 			[RequiresUnreferencedCode ("Message for --TestStaticCtor--")]
 			static StaticCtor ()
 			{
@@ -42,7 +42,7 @@ static void TestStaticCctorRequires ()
 
 		class StaticCtorTriggeredByFieldAccess
 		{
-			[ExpectedWarning ("IL2116", "StaticCtorTriggeredByFieldAccess..cctor()", ProducedBy = ProducedBy.Trimmer)]
+			[ExpectedWarning ("IL2116", "StaticCtorTriggeredByFieldAccess..cctor()")]
 			[RequiresUnreferencedCode ("Message for --StaticCtorTriggeredByFieldAccess.Cctor--")]
 			static StaticCtorTriggeredByFieldAccess ()
 			{
@@ -59,9 +59,7 @@ static void TestStaticCtorMarkingIsTriggeredByFieldAccess ()
 
 		struct StaticCCtorForFieldAccess
 		{
-			// TODO: Analyzer still allows RUC/RAF on static constructor with no warning
-			// https://github.com/dotnet/linker/issues/2347
-			[ExpectedWarning ("IL2116", "StaticCCtorForFieldAccess..cctor()", ProducedBy = ProducedBy.Trimmer)]
+			[ExpectedWarning ("IL2116", "StaticCCtorForFieldAccess..cctor()")]
 			[RequiresUnreferencedCode ("Message for --StaticCCtorForFieldAccess.cctor--")]
 			static StaticCCtorForFieldAccess () { }
 
@@ -75,9 +73,9 @@ static void TestStaticCtorMarkingIsTriggeredByFieldAccessOnExplicitLayout ()
 
 		class StaticCtorTriggeredByMethodCall
 		{
-			// TODO: Analyzer still allows RUC/RAF on static constructor with no warning
-			// https://github.com/dotnet/linker/issues/2347
-			[ExpectedWarning ("IL2116", "StaticCtorTriggeredByMethodCall..cctor()", ProducedBy = ProducedBy.Trimmer)]
+			[ExpectedWarning ("IL2116", "StaticCtorTriggeredByMethodCall..cctor()")]
+			[ExpectedWarning ("IL3004", "StaticCtorTriggeredByMethodCall..cctor()", ProducedBy = ProducedBy.Analyzer)]
+			[ExpectedWarning ("IL3052", "StaticCtorTriggeredByMethodCall..cctor()", ProducedBy = ProducedBy.Analyzer)]
 			[RequiresUnreferencedCode ("Message for --StaticCtorTriggeredByMethodCall.Cctor--")]
 			[RequiresAssemblyFiles ("Message for --StaticCtorTriggeredByMethodCall.Cctor--")]
 			[RequiresDynamicCode ("Message for --StaticCtorTriggeredByMethodCall.Cctor--")]