Don't warn for reflection access to compiler-generated code (#104757)

* Don't warn for reflection access to compiler-generated code

* Remove unnecessary suppressions

Author: sbomer

eng/testing/tests.wasm.targets

@@ -38,8 +38,6 @@
   <PropertyGroup Condition="'$(EnableAggressiveTrimming)' == 'true'">
     <!-- suppress warnings as these are tests, and not expected to be trim-safe -->
     <SuppressTrimAnalysisWarnings>true</SuppressTrimAnalysisWarnings>
-    <!-- This warning code isn't yet included in SuppressTrimAnalysisWarnings -->
-    <NoWarn>$(NoWarn);IL2118</NoWarn>
     <!-- IL2121: Unnecessary UnconditionalSuppressMessage attribute -->
     <NoWarn>$(NoWarn);IL2121</NoWarn>
   </PropertyGroup>

src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/DiagnosticSourceEventSource.cs

@@ -648,9 +648,6 @@ public FilterAndTransform(string filterAndPayloadSpec, int startIdx, int endIdx,
 
                         [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2026:RequiresUnreferencedCode",
                             Justification = "DiagnosticSource.Write is marked with RequiresUnreferencedCode.")]
-                        [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2119",
-                            Justification = "DAM on EventSource references this compiler-generated local function which calls a " +
-                                            "method that requires unreferenced code. EventSource will not access this local function.")]
                         void OnEventWritten(KeyValuePair<string, object?> evnt)
                         {
                             // The filter given to the DiagnosticSource may not work if users don't is 'IsEnabled' as expected.

src/libraries/System.Diagnostics.Tracing/tests/TrimmingTests/EventSourceManifestTest.cs

@@ -35,9 +35,6 @@ void EventSourceTest_Method_4(){}
         int EventSourceTest_Method_7() => 5; 
     }
 
-    [UnconditionalSuppressMessage ("ReflectionAnalysis", "IL2118",
-        Justification = "DAM on EventSource.GenerateManifest references compiler-generated local function GetTrimSafeTraceLoggingEventTypes " +
-                        "which calls a constructor that requires unreferenced code. EventSource will not access this local function.")]
     public static int Main()
     {
         string manifest = EventSource.GenerateManifest(typeof(EventSourceTest), null);

src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventSource.cs

@@ -2160,9 +2160,6 @@ private unsafe void WriteEventString(string msgString)
 
                 [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2026:RequiresUnreferencedCode",
                     Justification = "The call to TraceLoggingEventTypes with the below parameter values are trim safe")]
-                [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2119",
-                    Justification = "DAM on EventSource references this compiler-generated local function which calls a " +
-                                    "constructor that requires unreferenced code. EventSource will not access this local function.")]
                 static TraceLoggingEventTypes GetTrimSafeTraceLoggingEventTypes() =>
                     new TraceLoggingEventTypes(EventName, EventTags.None, new Type[] { typeof(string) });
 

src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Type/TypeTests.cs

@@ -149,9 +149,6 @@ public static IEnumerable<object[]> FindMembers_TestData()
 
         [Theory]
         [MemberData(nameof(FindMembers_TestData))]
-        [UnconditionalSuppressMessage ("ReflectionAnalysis", "IL2118",
-            Justification = "DAM on FindMembers references compiler-generated members which use reflection. " +
-                            "These members are not accessed by the test.")]
         public void FindMembers_Invoke_ReturnsExpected(MemberTypes memberType, BindingFlags bindingAttr, MemberFilter filter, object filterCriteria, int expectedLength)
         {
             Assert.Equal(expectedLength, typeof(TypeTests).FindMembers(memberType, bindingAttr, filter, filterCriteria).Length);

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

@@ -182,9 +182,9 @@ public enum DiagnosticId
 		DynamicallyAccessedMembersOnTypeReferencesMemberOnBaseWithDynamicallyAccessedMembers = 2115,
 		RequiresUnreferencedCodeOnStaticConstructor = 2116,
 		MethodsAreAssociatedWithUserMethod = 2117,
-		CompilerGeneratedMemberAccessedViaReflection = 2118,
-		DynamicallyAccessedMembersOnTypeReferencesCompilerGeneratedMember = 2119,
-		DynamicallyAccessedMembersOnTypeReferencesCompilerGeneratedMemberOnBase = 2120,
+		_unused_CompilerGeneratedMemberAccessedViaReflection = 2118,
+		_unused_DynamicallyAccessedMembersOnTypeReferencesCompilerGeneratedMember = 2119,
+		_unused_DynamicallyAccessedMembersOnTypeReferencesCompilerGeneratedMemberOnBase = 2120,
 		RedundantSuppression = 2121,
 		TypeNameIsNotAssemblyQualified = 2122,
 

src/tools/illink/src/linker/Linker.Steps/MarkStep.cs

@@ -1616,20 +1616,6 @@ void ReportWarningsForReflectionAccess (in MessageOrigin origin, MethodDefinitio
 					break;
 				}
 			}
-
-			// Warn on reflection access to compiler-generated methods, if the method isn't already unsafe to access via reflection
-			// due to annotations. For the annotation-based warnings, we skip virtual overrides since those will produce warnings on
-			// the base, but for unannotated compiler-generated methods this is not the case, so we must produce these warnings even
-			// for virtual overrides. This ensures that we include the unannotated MoveNext state machine method. Lambdas and local
-			// functions should never be virtual overrides in the first place.
-			bool isCoveredByAnnotations = isReflectionAccessCoveredByRUC || isReflectionAccessCoveredByDAM;
-			switch (dependencyKind) {
-			case DependencyKind.AccessedViaReflection:
-			case DependencyKind.DynamicallyAccessedMember:
-				if (ShouldWarnForReflectionAccessToCompilerGeneratedCode (method, isCoveredByAnnotations))
-					Context.LogWarning (origin, DiagnosticId.CompilerGeneratedMemberAccessedViaReflection, method.GetDisplayName ());
-				break;
-			}
 		}
 
 		void ReportWarningsForTypeHierarchyReflectionAccess (IMemberDefinition member, MessageOrigin origin)
@@ -1672,23 +1658,6 @@ static bool IsDeclaredWithinType (IMemberDefinition member, TypeDefinition type)
 				var id = reportOnMember ? DiagnosticId.DynamicallyAccessedMembersOnTypeReferencesMemberWithDynamicallyAccessedMembers : DiagnosticId.DynamicallyAccessedMembersOnTypeReferencesMemberOnBaseWithDynamicallyAccessedMembers;
 				Context.LogWarning (origin, id, type.GetDisplayName (), ((MemberReference) member).GetDisplayName ());
 			}
-
-			// Warn on reflection access to compiler-generated methods, if the method isn't already unsafe to access via reflection
-			// due to annotations. For the annotation-based warnings, we skip virtual overrides since those will produce warnings on
-			// the base, but for unannotated compiler-generated methods this is not the case, so we must produce these warnings even
-			// for virtual overrides. This ensures that we include the unannotated MoveNext state machine method. Lambdas and local
-			// functions should never be virtual overrides in the first place.
-			bool isCoveredByAnnotations = isReflectionAccessCoveredByRUC || isReflectionAccessCoveredByDAM;
-			if (member is MethodDefinition method && ShouldWarnForReflectionAccessToCompilerGeneratedCode (method, isCoveredByAnnotations)) {
-				var id = reportOnMember ? DiagnosticId.DynamicallyAccessedMembersOnTypeReferencesCompilerGeneratedMember : DiagnosticId.DynamicallyAccessedMembersOnTypeReferencesCompilerGeneratedMemberOnBase;
-				Context.LogWarning (origin, id, type.GetDisplayName (), method.GetDisplayName ());
-			}
-
-			// Warn on reflection access to compiler-generated fields.
-			if (member is FieldDefinition field && ShouldWarnForReflectionAccessToCompilerGeneratedCode (field, isCoveredByAnnotations)) {
-				var id = reportOnMember ? DiagnosticId.DynamicallyAccessedMembersOnTypeReferencesCompilerGeneratedMember : DiagnosticId.DynamicallyAccessedMembersOnTypeReferencesCompilerGeneratedMemberOnBase;
-				Context.LogWarning (origin, id, type.GetDisplayName (), field.GetDisplayName ());
-			}
 		}
 
 		void MarkField (FieldDefinition field, in DependencyInfo reason, in MessageOrigin origin)
@@ -1752,24 +1721,6 @@ void MarkField (FieldDefinition field, in DependencyInfo reason, in MessageOrigi
 			}
 		}
 
-		bool ShouldWarnForReflectionAccessToCompilerGeneratedCode (FieldDefinition field, bool isCoveredByAnnotations)
-		{
-			// No need to warn if it's already covered by the Requires attribute or explicit annotations on the field.
-			if (isCoveredByAnnotations)
-				return false;
-
-			if (!CompilerGeneratedState.IsNestedFunctionOrStateMachineMember (field))
-				return false;
-
-			// Only warn for types which are interesting for dataflow. Note that this does
-			// not include integer types, even though we track integers in the dataflow analysis.
-			// Technically we should also warn for integer types, but this leads to more warnings
-			// for example about the compiler-generated "state" field for state machine methods.
-			// This should be ok because in most cases the state machine types will also have other
-			// hoisted locals that produce warnings anyway when accessed via reflection.
-			return Annotations.FlowAnnotations.IsTypeInterestingForDataflow (field.FieldType);
-		}
-
 		void ProcessAnalysisAnnotationsForField (FieldDefinition field, DependencyKind dependencyKind, in MessageOrigin origin)
 		{
 			switch (dependencyKind) {
@@ -1790,32 +1741,21 @@ void ProcessAnalysisAnnotationsForField (FieldDefinition field, DependencyKind d
 			if (Annotations.ShouldSuppressAnalysisWarningsForRequiresUnreferencedCode (origin.Provider, out _))
 				return;
 
-			bool isReflectionAccessCoveredByRUC;
-			if (isReflectionAccessCoveredByRUC = Annotations.ShouldSuppressAnalysisWarningsForRequiresUnreferencedCode (field, out RequiresUnreferencedCodeAttribute? requiresUnreferencedCodeAttribute))
+			if (Annotations.ShouldSuppressAnalysisWarningsForRequiresUnreferencedCode (field, out RequiresUnreferencedCodeAttribute? requiresUnreferencedCodeAttribute))
 				ReportRequiresUnreferencedCode (field.GetDisplayName (), requiresUnreferencedCodeAttribute!, new DiagnosticContext (origin, diagnosticsEnabled: true, Context));
 
-			bool isReflectionAccessCoveredByDAM = false;
 			switch (dependencyKind) {
 			case DependencyKind.AccessedViaReflection:
 			case DependencyKind.DynamicDependency:
 			case DependencyKind.DynamicallyAccessedMember:
 			case DependencyKind.InteropMethodDependency:
 			case DependencyKind.Ldtoken:
 			case DependencyKind.UnsafeAccessorTarget:
-				if (isReflectionAccessCoveredByDAM = Annotations.FlowAnnotations.ShouldWarnWhenAccessedForReflection (field))
+				if (Annotations.FlowAnnotations.ShouldWarnWhenAccessedForReflection (field))
 					Context.LogWarning (origin, DiagnosticId.DynamicallyAccessedMembersFieldAccessedViaReflection, field.GetDisplayName ());
 
 				break;
 			}
-
-			switch (dependencyKind) {
-			case DependencyKind.AccessedViaReflection:
-			case DependencyKind.DynamicallyAccessedMember:
-				bool isCoveredByAnnotations = isReflectionAccessCoveredByRUC || isReflectionAccessCoveredByDAM;
-				if (ShouldWarnForReflectionAccessToCompilerGeneratedCode (field, isCoveredByAnnotations))
-					Context.LogWarning (origin, DiagnosticId.CompilerGeneratedMemberAccessedViaReflection, field.GetDisplayName ());
-				break;
-			}
 		}
 
 		/// <summary>

src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/AnnotatedMembersAccessedViaReflection.cs

@@ -333,8 +333,6 @@ static void Ldvirtftn ()
 			[ExpectedWarning ("IL2111", nameof (MethodWithSingleAnnotatedParameter))]
 			[ExpectedWarning ("IL2111", nameof (IWithAnnotatedMethod.AnnotatedMethod))]
 			[ExpectedWarning ("IL2111", nameof (IWithAnnotatedMethod.AnnotatedMethod))]
-			[UnexpectedWarning ("IL2118", nameof (LdftnOnLambdaTriggersLamdaAnalysis), Tool.Trimmer, "https://github.com/dotnet/runtime/issues/85042")]
-			[UnexpectedWarning ("IL2118", nameof (LdftnOnLocalMethodTriggersLocalMethodAnalysis), Tool.Trimmer, "https://github.com/dotnet/runtime/issues/85042")]
 			static void DynamicallyAccessedMembersAll1 ()
 			{
 				typeof (AnnotatedMethodParameters).RequiresAll ();
@@ -347,8 +345,6 @@ static void DynamicallyAccessedMembersAll1 ()
 			[ExpectedWarning ("IL2111", nameof (MethodWithSingleAnnotatedParameter))]
 			[ExpectedWarning ("IL2111", nameof (IWithAnnotatedMethod.AnnotatedMethod))]
 			[ExpectedWarning ("IL2111", nameof (IWithAnnotatedMethod.AnnotatedMethod))]
-			[UnexpectedWarning ("IL2118", nameof (LdftnOnLambdaTriggersLamdaAnalysis), Tool.Trimmer, "https://github.com/dotnet/runtime/issues/85042")]
-			[UnexpectedWarning ("IL2118", nameof (LdftnOnLocalMethodTriggersLocalMethodAnalysis), Tool.Trimmer, "https://github.com/dotnet/runtime/issues/85042")]
 			static void DynamicallyAccessedMembersAll2 ()
 			{
 				typeof (AnnotatedMethodParameters).RequiresAll ();