Report Requires diagnostics from dataflow analyzer

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

@@ -11,16 +11,8 @@ internal static class AnalyzerOptionsExtensions
 	{
 		public static string? GetMSBuildPropertyValue (
 			this AnalyzerOptions options,
-			string optionName,
-			Compilation compilation)
+			string optionName)
 		{
-			// MSBuild property values should be set at compilation level, and cannot have different values per-tree.
-			// So, we default to first syntax tree.
-			var tree = compilation.SyntaxTrees.FirstOrDefault ();
-			if (tree is null) {
-				return null;
-			}
-
 			return options.AnalyzerConfigOptionsProvider.GlobalOptions.TryGetValue (
 					$"build_property.{optionName}", out var value)
 				? value
@@ -29,10 +21,9 @@ internal static class AnalyzerOptionsExtensions
 
 		public static bool IsMSBuildPropertyValueTrue (
 			this AnalyzerOptions options,
-			string propertyName,
-			Compilation compilation)
+			string propertyName)
 		{
-			var propertyValue = GetMSBuildPropertyValue (options, propertyName, compilation);
+			var propertyValue = GetMSBuildPropertyValue (options, propertyName);
 			if (!string.Equals (propertyValue?.Trim (), "true", System.StringComparison.OrdinalIgnoreCase))
 				return false;
 

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

@@ -31,7 +31,7 @@ public override void Initialize (AnalysisContext context)
 			context.ConfigureGeneratedCodeAnalysis (GeneratedCodeAnalysisFlags.ReportDiagnostics);
 			context.RegisterCompilationStartAction (context => {
 				var compilation = context.Compilation;
-				if (!context.Options.IsMSBuildPropertyValueTrue (MSBuildPropertyOptionNames.EnableTrimAnalyzer, compilation))
+				if (!context.Options.IsMSBuildPropertyValueTrue (MSBuildPropertyOptionNames.EnableTrimAnalyzer))
 					return;
 
 				context.RegisterOperationAction (operationContext => {

src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/CapturedReferenceValue.cs

@@ -16,6 +16,7 @@ public CapturedReferenceValue (IOperation operation)
 		{
 			switch (operation.Kind) {
 			case OperationKind.PropertyReference:
+			case OperationKind.EventReference:
 			case OperationKind.LocalReference:
 			case OperationKind.FieldReference:
 			case OperationKind.ParameterReference:
@@ -26,7 +27,6 @@ public CapturedReferenceValue (IOperation operation)
 			case OperationKind.None:
 			case OperationKind.InstanceReference:
 			case OperationKind.Invocation:
-			case OperationKind.EventReference:
 			case OperationKind.Invalid:
 				// These will just be ignored when referenced later.
 				break;

src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/LocalDataFlowAnalysis.cs

@@ -58,7 +58,6 @@ public void InterproceduralAnalyze ()
 				return;
 			}
 
-
 			Debug.Assert (Context.OwningSymbol is not IMethodSymbol methodSymbol ||
 				methodSymbol.MethodKind is not (MethodKind.LambdaMethod or MethodKind.LocalFunction));
 			var startMethod = new MethodBodyValue (Context.OwningSymbol, Context.GetControlFlowGraph (OperationBlock));
@@ -68,9 +67,6 @@ public void InterproceduralAnalyze ()
 				oldInterproceduralState = interproceduralState.Clone ();
 
 				foreach (var method in oldInterproceduralState.Methods) {
-					if (method.OwningSymbol.IsInRequiresUnreferencedCodeAttributeScope (out _))
-						continue;
-
 					AnalyzeMethod (method, ref interproceduralState);
 				}
 			}

src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/LocalDataFlowVisitor.cs

@@ -97,7 +97,7 @@ public void Transfer (BlockProxy block, LocalDataFlowState<TValue, TValueLattice
 			HandleReturnValue (branchValue, branchValueOperation);
 		}
 
-		public abstract TValue GetFieldTargetValue (IFieldSymbol field);
+		public abstract TValue GetFieldTargetValue (IFieldSymbol field, IFieldReferenceOperation fieldReferenceOperation);
 
 		public abstract TValue GetParameterTargetValue (IParameterSymbol parameter);
 
@@ -170,7 +170,7 @@ TValue ProcessSingleTargetAssignment (IOperation targetOperation, IAssignmentOpe
 			case IFieldReferenceOperation:
 			case IParameterReferenceOperation: {
 					TValue targetValue = targetOperation switch {
-						IFieldReferenceOperation fieldRef => GetFieldTargetValue (fieldRef.Field),
+						IFieldReferenceOperation fieldRef => GetFieldTargetValue (fieldRef.Field, fieldRef),
 						IParameterReferenceOperation parameterRef => GetParameterTargetValue (parameterRef.Parameter),
 						_ => throw new InvalidOperationException ()
 					};
@@ -212,6 +212,13 @@ TValue ProcessSingleTargetAssignment (IOperation targetOperation, IAssignmentOpe
 					// even though a property setter has no return value.
 					return value;
 				}
+			case IEventReferenceOperation eventRef: {
+					// Handles assignment to an event like 'Event = Handler;', which is a write to the underlying field,
+					// not a call to an event accessor method. There is no Roslyn API to access the field,
+					// so just visit the instance and the value. https://github.com/dotnet/roslyn/issues/40103
+					Visit (eventRef.Instance, state);
+					return Visit (operation.Value, state);
+				}
 			case IImplicitIndexerReferenceOperation indexerRef: {
 					// An implicit reference to an indexer where the argument is a System.Index
 					TValue instanceValue = Visit (indexerRef.Instance, state);
@@ -276,10 +283,6 @@ TValue ProcessSingleTargetAssignment (IOperation targetOperation, IAssignmentOpe
 			// also produces warnings for ref params/locals/returns.
 			// https://github.com/dotnet/linker/issues/2632
 			// https://github.com/dotnet/linker/issues/2158
-			case IEventReferenceOperation:
-				// An event assignment is an assignment to the generated backing field for
-				// auto-implemented events. There is no Roslyn API to access the field, so
-				// skip this. https://github.com/dotnet/roslyn/issues/40103
 				Visit (targetOperation, state);
 				break;
 
@@ -359,6 +362,26 @@ TValue ProcessAssignment (IAssignmentOperation operation, LocalDataFlowState<TVa
 			return value;
 		}
 
+		public override TValue VisitEventAssignment (IEventAssignmentOperation operation, LocalDataFlowState<TValue, TValueLattice> state)
+		{
+			var eventReference = (IEventReferenceOperation) operation.EventReference;
+			TValue instanceValue = Visit (eventReference.Instance, state);
+			TValue value = Visit (operation.HandlerValue, state);
+			if (operation.Adds) {
+				IMethodSymbol? addMethod = eventReference.Event.AddMethod;
+				Debug.Assert (addMethod != null);
+				if (addMethod != null)
+					HandleMethodCall (addMethod, instanceValue, ImmutableArray.Create (value), operation);
+				return value;
+			} else {
+				IMethodSymbol? removeMethod = eventReference.Event.RemoveMethod;
+				Debug.Assert (removeMethod != null);
+				if (removeMethod != null)
+					HandleMethodCall (removeMethod, instanceValue, ImmutableArray.Create (value), operation);
+				return value;
+			}
+		}
+
 		TValue GetFlowCaptureValue (IFlowCaptureReferenceOperation operation, LocalDataFlowState<TValue, TValueLattice> state)
 		{
 			Debug.Assert (!IsLValueFlowCapture (operation.Id),
@@ -465,43 +488,58 @@ public override TValue VisitInvocation (IInvocationOperation operation, LocalDat
 
 		public override TValue VisitDelegateCreation (IDelegateCreationOperation operation, LocalDataFlowState<TValue, TValueLattice> state)
 		{
-			if (operation.Target is IFlowAnonymousFunctionOperation lambda) {
-				VisitFlowAnonymousFunction (lambda, state);
-
-				// Instance of a lambda or local function should be the instance of the containing method.
-				// Don't need to track a dataflow value, since the delegate creation will warn if the
-				// lambda or local function has an annotated this parameter.
-				var instance = TopValue;
-				return HandleDelegateCreation (lambda.Symbol, instance, operation);
-			}
+			Visit (operation.Target, state);
 
-			Debug.Assert (operation.Target is IMemberReferenceOperation,
-				$"{operation.Target.GetType ()}: {operation.Syntax.GetLocation ().GetLineSpan ()}");
-			if (operation.Target is not IMemberReferenceOperation memberReference)
-				return TopValue;
+			IMethodSymbol? targetMethodSymbol = null;
+			switch (operation.Target) {
+				case IFlowAnonymousFunctionOperation lambda:
+					// Tracking lambdas is handled by normal visiting logic for IFlowAnonymousFunctionOperation.
 
-			TValue instanceValue = Visit (memberReference.Instance, state);
+					// Instance of a lambda or local function should be the instance of the containing method.
+					// Don't need to track a dataflow value, since the delegate creation will warn if the
+					// lambda or local function has an annotated this parameter.
+					targetMethodSymbol = lambda.Symbol;
+					break;
+				case IMethodReferenceOperation methodReference:
+					IMethodSymbol method = methodReference.Method.OriginalDefinition;
+					if (method.ContainingSymbol is IMethodSymbol) {
+						// Track references to local functions
+						var localFunction = method;
+						Debug.Assert (localFunction.MethodKind == MethodKind.LocalFunction);;
+						var localFunctionCFG = ControlFlowGraph.GetLocalFunctionControlFlowGraphInScope (localFunction);
+						InterproceduralState.TrackMethod (new MethodBodyValue (localFunction, localFunctionCFG));
+					}
+					targetMethodSymbol = method;
+					break;
+				case IMemberReferenceOperation:
+				case IInvocationOperation:
+					// No method symbol.
+					break;
+				default:
+					// Unimplemented case that might need special handling.
+					// Fail in debug mode only.
+					Debug.Fail ($"{operation.Target.GetType ()}: {operation.Target.Syntax.GetLocation ().GetLineSpan ()}");
+					break;
+			}
 
-			if (memberReference.Member is not IMethodSymbol method)
+			if (targetMethodSymbol == null)
 				return TopValue;
 
-			// Track references to local functions
-			if (method.OriginalDefinition.ContainingSymbol is IMethodSymbol) {
-				var localFunction = method.OriginalDefinition;
-				Debug.Assert (localFunction.MethodKind == MethodKind.LocalFunction);;
-				var localFunctionCFG = ControlFlowGraph.GetLocalFunctionControlFlowGraphInScope (localFunction);
-				InterproceduralState.TrackMethod (new MethodBodyValue (localFunction, localFunctionCFG));
-			}
-
-			return HandleDelegateCreation (method, instanceValue, operation);
+			return HandleDelegateCreation (targetMethodSymbol, operation);
 		}
 
-		public abstract TValue HandleDelegateCreation (IMethodSymbol methodReference, TValue instance, IOperation operation);
+		public abstract TValue HandleDelegateCreation (IMethodSymbol methodReference, IOperation operation);
 
 		public override TValue VisitPropertyReference (IPropertyReferenceOperation operation, LocalDataFlowState<TValue, TValueLattice> state)
 		{
-			if (!operation.GetValueUsageInfo (OwningSymbol).HasFlag (ValueUsageInfo.Read))
+			if (operation.GetValueUsageInfo (OwningSymbol).HasFlag (ValueUsageInfo.Write)) {
+				// Property references may be passed as ref/out parameters.
+				// Enable this assert once we have support for deconstruction assignments.
+				// https://github.com/dotnet/linker/issues/3158
+				// Debug.Assert (operation.GetValueUsageInfo (OwningSymbol).HasFlag (ValueUsageInfo.Reference),
+				// $"{operation.Syntax.GetLocation ().GetLineSpan ()}");
 				return TopValue;
+			}
 
 			// Accessing property for reading is really a call to the getter
 			// The setter case is handled in assignment operation since here we don't have access to the value to pass to the setter
@@ -516,11 +554,27 @@ public override TValue VisitPropertyReference (IPropertyReferenceOperation opera
 			return HandleMethodCall (getMethod!, instanceValue, arguments.ToImmutableArray (), operation);
 		}
 
-		public override TValue VisitImplicitIndexerReference (IImplicitIndexerReferenceOperation operation, LocalDataFlowState<TValue, TValueLattice> state)
+		public override TValue VisitEventReference (IEventReferenceOperation operation, LocalDataFlowState<TValue, TValueLattice> state)
 		{
+			// Writing to an event should not go through this path.
+			Debug.Assert (operation.GetValueUsageInfo (OwningSymbol).HasFlag (ValueUsageInfo.Read));
 			if (!operation.GetValueUsageInfo (OwningSymbol).HasFlag (ValueUsageInfo.Read))
 				return TopValue;
 
+			Visit (operation.Instance, state);
+			// Accessing event for reading retrieves the event delegate from the event's backing field,
+			// so there is no method call to handle.
+			return TopValue;
+		}
+
+		public override TValue VisitImplicitIndexerReference (IImplicitIndexerReferenceOperation operation, LocalDataFlowState<TValue, TValueLattice> state)
+		{
+			if (operation.GetValueUsageInfo (OwningSymbol).HasFlag (ValueUsageInfo.Write)) {
+				// Implicit indexer references may be passed as ref/out parameters.
+				Debug.Assert (operation.GetValueUsageInfo (OwningSymbol).HasFlag (ValueUsageInfo.Reference));
+				return TopValue;
+			}
+
 			TValue instanceValue = Visit (operation.Instance, state);
 			TValue indexArgumentValue = Visit (operation.Argument, state);
 

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

@@ -0,0 +1,48 @@
+// Copyright (c) .NET Foundation and contributors. All rights reserved.
+// Licensed under the MIT license. See LICENSE file in the project root for full license information.
+
+using System.Collections.Generic;
+using System.Collections.Immutable;
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.Diagnostics;
+
+namespace ILLink.RoslynAnalyzer
+{
+	public readonly struct DataFlowAnalyzerContext
+	{
+		private readonly Dictionary<RequiresAnalyzerBase, ImmutableArray<ISymbol>> _enabledAnalyzers;
+
+		public IEnumerable<RequiresAnalyzerBase> EnabledRequiresAnalyzers => _enabledAnalyzers.Keys;
+
+		public ImmutableArray<ISymbol> GetSpecialIncompatibleMembers (RequiresAnalyzerBase analyzer)
+		{
+			if (!_enabledAnalyzers.TryGetValue (analyzer, out var members))
+				throw new System.ArgumentException ($"Analyzer {analyzer.GetType ().Name} is not in the cache");
+			return members;
+		}
+
+		public readonly bool EnableTrimAnalyzer { get; }
+
+		public readonly bool AnyAnalyzersEnabled => EnableTrimAnalyzer || _enabledAnalyzers.Count > 0;
+
+		DataFlowAnalyzerContext (Dictionary<RequiresAnalyzerBase, ImmutableArray<ISymbol>> enabledAnalyzers, bool enableTrimAnalyzer)
+		{
+			_enabledAnalyzers = enabledAnalyzers;
+			EnableTrimAnalyzer = enableTrimAnalyzer;
+		}
+
+		public static DataFlowAnalyzerContext Create (AnalyzerOptions options, Compilation compilation, ImmutableArray<RequiresAnalyzerBase> requiresAnalyzers)
+		{
+			var enabledAnalyzers = new Dictionary<RequiresAnalyzerBase, ImmutableArray<ISymbol>> ();
+			foreach (var analyzer in requiresAnalyzers) {
+				if (analyzer.IsAnalyzerEnabled (options)) {
+					var incompatibleMembers = analyzer.GetSpecialIncompatibleMembers (compilation);
+					enabledAnalyzers.Add (analyzer, incompatibleMembers);
+				}
+			}
+			return new DataFlowAnalyzerContext (
+				enabledAnalyzers,
+				options.IsMSBuildPropertyValueTrue (MSBuildPropertyOptionNames.EnableTrimAnalyzer));
+		}
+	}
+}

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

@@ -25,6 +25,12 @@ public class DynamicallyAccessedMembersAnalyzer : DiagnosticAnalyzer
 		internal const string DynamicallyAccessedMembersAttribute = nameof (DynamicallyAccessedMembersAttribute);
 		public const string attributeArgument = "attributeArgument";
 		public const string FullyQualifiedDynamicallyAccessedMembersAttribute = "System.Diagnostics.CodeAnalysis." + DynamicallyAccessedMembersAttribute;
+		public static Lazy<ImmutableArray<RequiresAnalyzerBase>> RequiresAnalyzers { get; } = new Lazy<ImmutableArray<RequiresAnalyzerBase>> (GetRequiresAnalyzers);
+		static ImmutableArray<RequiresAnalyzerBase> GetRequiresAnalyzers () =>
+			ImmutableArray.Create<RequiresAnalyzerBase> (
+				new RequiresAssemblyFilesAnalyzer (),
+				new RequiresUnreferencedCodeAnalyzer (),
+				new RequiresDynamicCodeAnalyzer ());
 
 		public static ImmutableArray<DiagnosticDescriptor> GetSupportedDiagnostics ()
 		{
@@ -49,6 +55,12 @@ public static ImmutableArray<DiagnosticDescriptor> GetSupportedDiagnostics ()
 			diagDescriptorsArrayBuilder.Add (DiagnosticDescriptors.GetDiagnosticDescriptor (DiagnosticId.UnrecognizedTypeNameInTypeGetType));
 			diagDescriptorsArrayBuilder.Add (DiagnosticDescriptors.GetDiagnosticDescriptor (DiagnosticId.UnrecognizedParameterInMethodCreateInstance));
 			diagDescriptorsArrayBuilder.Add (DiagnosticDescriptors.GetDiagnosticDescriptor (DiagnosticId.ParametersOfAssemblyCreateInstanceCannotBeAnalyzed));
+
+			foreach (var requiresAnalyzer in RequiresAnalyzers.Value) {
+				foreach (var diagnosticDescriptor in requiresAnalyzer.SupportedDiagnostics)
+					diagDescriptorsArrayBuilder.Add (diagnosticDescriptor);
+			}
+
 			return diagDescriptorsArrayBuilder.ToImmutable ();
 
 			void AddRange (DiagnosticId first, DiagnosticId last)
@@ -72,20 +84,23 @@ public override void Initialize (AnalysisContext context)
 				context.EnableConcurrentExecution ();
 			context.ConfigureGeneratedCodeAnalysis (GeneratedCodeAnalysisFlags.ReportDiagnostics);
 			context.RegisterCompilationStartAction (context => {
-				if (!context.Options.IsMSBuildPropertyValueTrue (MSBuildPropertyOptionNames.EnableTrimAnalyzer, context.Compilation))
+				var dataFlowAnalyzerContext = DataFlowAnalyzerContext.Create (context.Options, context.Compilation, RequiresAnalyzers.Value);
+				if (!dataFlowAnalyzerContext.AnyAnalyzersEnabled)
 					return;
 
 				context.RegisterOperationBlockAction (context => {
-					if (context.OwningSymbol.IsInRequiresUnreferencedCodeAttributeScope (out _))
-						return;
-
 					foreach (var operationBlock in context.OperationBlocks) {
 						TrimDataFlowAnalysis trimDataFlowAnalysis = new (context, operationBlock);
 						trimDataFlowAnalysis.InterproceduralAnalyze ();
-						foreach (var diagnostic in trimDataFlowAnalysis.TrimAnalysisPatterns.CollectDiagnostics ())
+						foreach (var diagnostic in trimDataFlowAnalysis.CollectDiagnostics (dataFlowAnalyzerContext))
 							context.ReportDiagnostic (diagnostic);
 					}
 				});
+
+				// Remaining actions are only for DynamicallyAccessedMembers analysis.
+				if (!dataFlowAnalyzerContext.EnableTrimAnalyzer)
+					return;
+
 				// Examine generic instantiations in base types and interface list
 				context.RegisterSymbolAction (context => {
 					var type = (INamedTypeSymbol) context.Symbol;