Clean up DiagnosticContext (#105848)

Removes an overload from `DiagnosticContext` that makes it possible to
pass in a `Diagnostic`. This overload is undesirable because we want
the `Location` stored in `DiagnosticContext` to always match the
location of the diagnostic.

src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/DiagnosticContext.cs

@@ -8,7 +8,7 @@
 
 namespace ILLink.Shared.TrimAnalysis
 {
-    internal readonly partial struct DiagnosticContext
+    public readonly partial struct DiagnosticContext
     {
         public readonly MessageOrigin Origin;
         private readonly bool _diagnosticsEnabled;

src/tools/illink/src/ILLink.CodeFix/RequiresAssemblyFilesCodeFixProvider.cs

@@ -15,7 +15,7 @@
 namespace ILLink.CodeFix
 {
 	[ExportCodeFixProvider (LanguageNames.CSharp, Name = nameof (RequiresAssemblyFilesCodeFixProvider)), Shared]
-	public class RequiresAssemblyFilesCodeFixProvider : BaseAttributeCodeFixProvider
+	public sealed class RequiresAssemblyFilesCodeFixProvider : BaseAttributeCodeFixProvider
 	{
 		public static ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create (
 			DiagnosticDescriptors.GetDiagnosticDescriptor (DiagnosticId.AvoidAssemblyLocationInSingleFile),

src/tools/illink/src/ILLink.CodeFix/RequiresUnreferencedCodeCodeFixProvider.cs

@@ -15,7 +15,7 @@
 namespace ILLink.CodeFix
 {
 	[ExportCodeFixProvider (LanguageNames.CSharp, Name = nameof (RequiresUnreferencedCodeCodeFixProvider)), Shared]
-	public class RequiresUnreferencedCodeCodeFixProvider : BaseAttributeCodeFixProvider
+	public sealed class RequiresUnreferencedCodeCodeFixProvider : BaseAttributeCodeFixProvider
 	{
 		public static ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create (DiagnosticDescriptors.GetDiagnosticDescriptor (DiagnosticId.RequiresUnreferencedCode));
 

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

@@ -24,7 +24,7 @@ namespace ILLink.RoslynAnalyzer.DataFlow
 	// (a set features that are checked to be enabled or disabled).
 	// The visitor takes a LocalDataFlowState as an argument, allowing for checks that
 	// depend on the current dataflow state.
-	public class FeatureChecksVisitor : OperationVisitor<StateValue, FeatureChecksValue>
+	internal sealed class FeatureChecksVisitor : OperationVisitor<StateValue, FeatureChecksValue>
 	{
 		DataFlowAnalyzerContext _dataFlowAnalyzerContext;
 
@@ -77,7 +77,7 @@ public override FeatureChecksValue VisitLiteral (ILiteralOperation operation, St
 			return FeatureChecksValue.None;
 		}
 
-		public bool? GetLiteralBool (IOperation operation)
+		public static bool? GetLiteralBool (IOperation operation)
 		{
 			if (operation is not ILiteralOperation literal)
 				return null;

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

@@ -8,7 +8,7 @@
 
 namespace ILLink.RoslynAnalyzer
 {
-	public readonly struct DataFlowAnalyzerContext
+	internal readonly struct DataFlowAnalyzerContext
 	{
 		private readonly Dictionary<RequiresAnalyzerBase, ImmutableArray<ISymbol>> _enabledAnalyzers;
 

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

@@ -15,7 +15,7 @@
 namespace ILLink.RoslynAnalyzer
 {
 	[DiagnosticAnalyzer (LanguageNames.CSharp)]
-	public class DynamicallyAccessedMembersAnalyzer : DiagnosticAnalyzer
+	public sealed class DynamicallyAccessedMembersAnalyzer : DiagnosticAnalyzer
 	{
 		internal const string DynamicallyAccessedMembers = nameof (DynamicallyAccessedMembers);
 		internal const string DynamicallyAccessedMembersAttribute = nameof (DynamicallyAccessedMembersAttribute);

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

@@ -8,6 +8,7 @@
 using ILLink.RoslynAnalyzer.DataFlow;
 using ILLink.Shared;
 using ILLink.Shared.DataFlow;
+using ILLink.Shared.TrimAnalysis;
 using Microsoft.CodeAnalysis;
 using Microsoft.CodeAnalysis.CSharp;
 using Microsoft.CodeAnalysis.CSharp.Syntax;
@@ -27,6 +28,8 @@ public abstract class RequiresAnalyzerBase : DiagnosticAnalyzer
 
 		private protected abstract DiagnosticDescriptor RequiresDiagnosticRule { get; }
 
+		private protected abstract DiagnosticId RequiresDiagnosticId { get; }
+
 		private protected abstract DiagnosticDescriptor RequiresAttributeMismatch { get; }
 		private protected abstract DiagnosticDescriptor RequiresOnStaticCtor { get; }
 
@@ -144,33 +147,30 @@ void CheckMatchingAttributesInInterfaces (
 			});
 		}
 
-		public bool CheckAndCreateRequiresDiagnostic (
-			IOperation operation,
+		internal void CheckAndCreateRequiresDiagnostic (
 			ISymbol member,
 			ISymbol containingSymbol,
 			ImmutableArray<ISymbol> incompatibleMembers,
-			[NotNullWhen (true)] out Diagnostic? diagnostic)
+			in DiagnosticContext diagnosticContext)
 		{
-			diagnostic = null;
 			// Do not emit any diagnostic if caller is annotated with the attribute too.
 			if (containingSymbol.IsInRequiresScope (RequiresAttributeName, out _))
-				return false;
+				return;
 
-			if (CreateSpecialIncompatibleMembersDiagnostic (operation, incompatibleMembers, member, out diagnostic))
-				return diagnostic != null;
+			if (CreateSpecialIncompatibleMembersDiagnostic (incompatibleMembers, member, diagnosticContext))
+				return;
 
 			// Warn on the most derived base method taking into account covariant returns
 			while (member is IMethodSymbol method && method.OverriddenMethod != null && SymbolEqualityComparer.Default.Equals (method.ReturnType, method.OverriddenMethod.ReturnType))
 				member = method.OverriddenMethod;
 
 			if (!member.DoesMemberRequire (RequiresAttributeName, out var requiresAttribute))
-				return false;
+				return;
 
 			if (!VerifyAttributeArguments (requiresAttribute))
-				return false;
+				return;
 
-			diagnostic = CreateRequiresDiagnostic (operation, member, requiresAttribute);
-			return true;
+			CreateRequiresDiagnostic (member, requiresAttribute, diagnosticContext);
 		}
 
 		[Flags]
@@ -223,16 +223,11 @@ protected static ISymbol FindContainingSymbol (OperationAnalysisContext operatio
 		/// <param name="operationContext">Analyzer operation context to be able to report the diagnostic.</param>
 		/// <param name="member">Information about the member that generated the diagnostic.</param>
 		/// <param name="requiresAttribute">Requires attribute data to print attribute arguments.</param>
-		private Diagnostic CreateRequiresDiagnostic (IOperation operation, ISymbol member, AttributeData requiresAttribute)
+		private void CreateRequiresDiagnostic (ISymbol member, AttributeData requiresAttribute, in DiagnosticContext diagnosticContext)
 		{
 			var message = GetMessageFromAttribute (requiresAttribute);
 			var url = GetUrlFromAttribute (requiresAttribute);
-			return Diagnostic.Create (
-				RequiresDiagnosticRule,
-				operation.Syntax.GetLocation (),
-				member.GetDisplayName (),
-				message,
-				url);
+			diagnosticContext.AddDiagnostic (RequiresDiagnosticId, member.GetDisplayName (), message, url);
 		}
 
 		private void ReportRequiresOnStaticCtorDiagnostic (SymbolAnalysisContext symbolAnalysisContext, IMethodSymbol ctor)
@@ -285,12 +280,10 @@ public static string GetUrlFromAttribute (AttributeData? requiresAttribute)
 		/// <param name="member">Member to compare.</param>
 		/// <returns>True if the function generated a diagnostic; otherwise, returns false</returns>
 		protected virtual bool CreateSpecialIncompatibleMembersDiagnostic (
-			IOperation operation,
 			ImmutableArray<ISymbol> specialIncompatibleMembers,
 			ISymbol member,
-			out Diagnostic? incompatibleMembersDiagnostic)
+			in DiagnosticContext diagnosticContext)
 		{
-			incompatibleMembersDiagnostic = null;
 			return false;
 		}
 
@@ -333,29 +326,26 @@ internal bool IsFeatureGuard (IPropertySymbol propertySymbol, Compilation compil
 				|| IsRequiresCheck (propertySymbol, compilation);
 		}
 
-		internal bool CheckAndCreateRequiresDiagnostic (
+		internal void CheckAndCreateRequiresDiagnostic (
 			IOperation operation,
 			ISymbol member,
 			ISymbol owningSymbol,
 			DataFlowAnalyzerContext context,
 			FeatureContext featureContext,
-			[NotNullWhen (true)] out Diagnostic? diagnostic)
+			in DiagnosticContext diagnosticContext)
 		{
 			// Warnings are not emitted if the featureContext says the feature is available.
-			if (featureContext.IsEnabled (RequiresAttributeFullyQualifiedName)) {
-				diagnostic = null;
-				return false;
-			}
+			if (featureContext.IsEnabled (RequiresAttributeFullyQualifiedName))
+				return;
 
 			ISymbol containingSymbol = operation.FindContainingSymbol (owningSymbol);
 
 			var incompatibleMembers = context.GetSpecialIncompatibleMembers (this);
-			return CheckAndCreateRequiresDiagnostic (
-				operation,
+			CheckAndCreateRequiresDiagnostic (
 				member,
 				containingSymbol,
 				incompatibleMembers,
-				out diagnostic);
+				diagnosticContext);
 		}
 
 		internal virtual bool IsIntrinsicallyHandled (

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

@@ -42,6 +42,8 @@ public sealed class RequiresAssemblyFilesAnalyzer : RequiresAnalyzerBase
 
 		private protected override DiagnosticDescriptor RequiresDiagnosticRule => s_requiresAssemblyFilesRule;
 
+		private protected override DiagnosticId RequiresDiagnosticId => DiagnosticId.RequiresAssemblyFiles;
+
 		private protected override DiagnosticDescriptor RequiresAttributeMismatch => s_requiresAssemblyFilesAttributeMismatch;
 
 		private protected override DiagnosticDescriptor RequiresOnStaticCtor => s_requiresAssemblyFilesOnStaticCtor;
@@ -98,20 +100,18 @@ internal override ImmutableArray<ISymbol> GetSpecialIncompatibleMembers (Compila
 		}
 
 		protected override bool CreateSpecialIncompatibleMembersDiagnostic (
-			IOperation operation,
 			ImmutableArray<ISymbol> dangerousPatterns,
 			ISymbol member,
-			out Diagnostic? diagnostic)
+			in DiagnosticContext diagnosticContext)
 		{
-			diagnostic = null;
 			if (member is IMethodSymbol method) {
 				if (ImmutableArrayOperations.Contains (dangerousPatterns, member, SymbolEqualityComparer.Default)) {
-					diagnostic = Diagnostic.Create (s_getFilesRule, operation.Syntax.GetLocation (), member.GetDisplayName ());
+					diagnosticContext.AddDiagnostic (DiagnosticId.AvoidAssemblyGetFilesInSingleFile, member.GetDisplayName ());
 					return true;
 				}
 				else if (method.AssociatedSymbol is ISymbol associatedSymbol &&
 					ImmutableArrayOperations.Contains (dangerousPatterns, associatedSymbol, SymbolEqualityComparer.Default)) {
-					diagnostic = Diagnostic.Create (s_locationRule, operation.Syntax.GetLocation (), member.GetDisplayName ());
+					diagnosticContext.AddDiagnostic (DiagnosticId.AvoidAssemblyLocationInSingleFile, 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.

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

@@ -34,6 +34,8 @@ public sealed class RequiresDynamicCodeAnalyzer : RequiresAnalyzerBase
 
 		private protected override DiagnosticDescriptor RequiresDiagnosticRule => s_requiresDynamicCodeRule;
 
+		private protected override DiagnosticId RequiresDiagnosticId => DiagnosticId.RequiresDynamicCode;
+
 		private protected override DiagnosticDescriptor RequiresAttributeMismatch => s_requiresDynamicCodeAttributeMismatch;
 
 		private protected override DiagnosticDescriptor RequiresOnStaticCtor => s_requiresDynamicCodeOnStaticCtor;

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

@@ -55,6 +55,8 @@ private Action<SymbolAnalysisContext> typeDerivesFromRucBase {
 
 		private protected override DiagnosticDescriptor RequiresDiagnosticRule => s_requiresUnreferencedCodeRule;
 
+		private protected override DiagnosticId RequiresDiagnosticId => DiagnosticId.RequiresUnreferencedCode;
+
 		private protected override DiagnosticDescriptor RequiresAttributeMismatch => s_requiresUnreferencedCodeAttributeMismatch;
 
 		private protected override DiagnosticDescriptor RequiresOnStaticCtor => s_requiresUnreferencedCodeOnStaticCtor;
@@ -78,12 +80,10 @@ private protected override bool IsRequiresCheck (IPropertySymbol propertySymbol,
 		}
 
 		protected override bool CreateSpecialIncompatibleMembersDiagnostic (
-			IOperation operation,
 			ImmutableArray<ISymbol> specialIncompatibleMembers,
 			ISymbol member,
-			out Diagnostic? incompatibleMembersDiagnostic)
+			in DiagnosticContext diagnosticContext)
 		{
-			incompatibleMembersDiagnostic = null;
 			// Some RUC-annotated APIs are intrinsically handled by the trimmer
 			if (member is IMethodSymbol method && Intrinsics.GetIntrinsicIdForMethod (new MethodProxy (method)) != IntrinsicId.None) {
 				return true;

src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/DiagnosticContext.cs

@@ -10,7 +10,7 @@
 
 namespace ILLink.Shared.TrimAnalysis
 {
-	internal readonly partial struct DiagnosticContext
+	public readonly partial struct DiagnosticContext
 	{
 		public List<Diagnostic> Diagnostics { get; } = new ();
 
@@ -28,14 +28,6 @@ public Diagnostic CreateDiagnostic (DiagnosticId id, params string[] args)
 			return Diagnostic.Create (DiagnosticDescriptors.GetDiagnosticDescriptor (id), Location, args);
 		}
 
-		public void AddDiagnostic (Diagnostic diagnostic)
-		{
-			if (Location == null)
-				return;
-
-			Diagnostics.Add (diagnostic);
-		}
-
 		public partial void AddDiagnostic (DiagnosticId id, params string[] args)
 		{
 			if (Location == null)

src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/FeatureCheckReturnValuePattern.cs

@@ -10,7 +10,7 @@
 
 namespace ILLink.RoslynAnalyzer.TrimAnalysis
 {
-	public readonly record struct FeatureCheckReturnValuePattern
+	internal readonly record struct FeatureCheckReturnValuePattern
 	{
 		public FeatureChecksValue ReturnValue { get; init; }
 		public ValueSet<string> FeatureCheckAnnotations { get; init; }

src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/ReflectionAccessAnalyzer.cs

@@ -85,20 +85,19 @@ internal static void GetReflectionAccessDiagnosticsForMethod (in DiagnosticConte
 			if (methodSymbol.IsInRequiresUnreferencedCodeAttributeScope (out var requiresUnreferencedCodeAttributeData)) {
 				ReportRequiresUnreferencedCodeDiagnostic (diagnosticContext, requiresUnreferencedCodeAttributeData, methodSymbol);
 			} else {
-				foreach (var diagnostic in GetDiagnosticsForReflectionAccessToDAMOnMethod (diagnosticContext, methodSymbol))
-					diagnosticContext.AddDiagnostic (diagnostic);
+				GetDiagnosticsForReflectionAccessToDAMOnMethod (diagnosticContext, methodSymbol);
 			}
 		}
 
-		internal static IEnumerable<Diagnostic> GetDiagnosticsForReflectionAccessToDAMOnMethod (DiagnosticContext diagnosticContext, IMethodSymbol methodSymbol)
+		internal static void GetDiagnosticsForReflectionAccessToDAMOnMethod (DiagnosticContext diagnosticContext, IMethodSymbol methodSymbol)
 		{
 			if (methodSymbol.IsVirtual && FlowAnnotations.GetMethodReturnValueAnnotation (methodSymbol) != DynamicallyAccessedMemberTypes.None) {
-				yield return diagnosticContext.CreateDiagnostic (DiagnosticId.DynamicallyAccessedMembersMethodAccessedViaReflection, methodSymbol.GetDisplayName ());
+				diagnosticContext.AddDiagnostic (DiagnosticId.DynamicallyAccessedMembersMethodAccessedViaReflection, methodSymbol.GetDisplayName ());
 			} else {
 				foreach (var parameter in methodSymbol.GetParameters ()) {
 					if (FlowAnnotations.GetMethodParameterAnnotation (parameter) != DynamicallyAccessedMemberTypes.None) {
-						yield return diagnosticContext.CreateDiagnostic (DiagnosticId.DynamicallyAccessedMembersMethodAccessedViaReflection, methodSymbol.GetDisplayName ());
-						yield break;
+						diagnosticContext.AddDiagnostic (DiagnosticId.DynamicallyAccessedMembersMethodAccessedViaReflection, methodSymbol.GetDisplayName ());
+						break;
 					}
 				}
 			}

src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisAssignmentPattern.cs

@@ -15,7 +15,7 @@
 
 namespace ILLink.RoslynAnalyzer.TrimAnalysis
 {
-	public readonly record struct TrimAnalysisAssignmentPattern
+	internal readonly record struct TrimAnalysisAssignmentPattern
 	{
 		public MultiValue Source { get; init; }
 		public MultiValue Target { get; init; }

src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisFieldAccessPattern.cs

@@ -11,7 +11,7 @@
 
 namespace ILLink.RoslynAnalyzer.TrimAnalysis
 {
-	public readonly record struct TrimAnalysisFieldAccessPattern
+	internal readonly record struct TrimAnalysisFieldAccessPattern
 	{
 		public IFieldSymbol Field { get; init; }
 		public IFieldReferenceOperation Operation { get; init; }
@@ -31,7 +31,6 @@ public TrimAnalysisFieldAccessPattern (
 		}
 
 		public TrimAnalysisFieldAccessPattern Merge (
-			ValueSetLattice<SingleValue> lattice,
 			FeatureContextLattice featureContextLattice,
 			TrimAnalysisFieldAccessPattern other)
 		{
@@ -49,10 +48,8 @@ public TrimAnalysisFieldAccessPattern Merge (
 		public IEnumerable<Diagnostic> CollectDiagnostics (DataFlowAnalyzerContext context)
 		{
 			DiagnosticContext diagnosticContext = new (Operation.Syntax.GetLocation ());
-			foreach (var requiresAnalyzer in context.EnabledRequiresAnalyzers) {
-				if (requiresAnalyzer.CheckAndCreateRequiresDiagnostic (Operation, Field, OwningSymbol, context, FeatureContext, out Diagnostic? diag))
-					diagnosticContext.AddDiagnostic (diag);
-			}
+			foreach (var requiresAnalyzer in context.EnabledRequiresAnalyzers)
+				requiresAnalyzer.CheckAndCreateRequiresDiagnostic (Operation, Field, OwningSymbol, context, FeatureContext, in diagnosticContext);
 
 			return diagnosticContext.Diagnostics;
 		}

src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisGenericInstantiationPattern.cs

@@ -9,7 +9,7 @@
 
 namespace ILLink.RoslynAnalyzer.TrimAnalysis
 {
-	public readonly record struct TrimAnalysisGenericInstantiationPattern
+	internal readonly record struct TrimAnalysisGenericInstantiationPattern
 	{
 		public ISymbol GenericInstantiation { get; init; }
 		public IOperation Operation { get; init; }

src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisMethodCallPattern.cs

@@ -15,7 +15,7 @@
 
 namespace ILLink.RoslynAnalyzer.TrimAnalysis
 {
-	public readonly record struct TrimAnalysisMethodCallPattern
+	internal readonly record struct TrimAnalysisMethodCallPattern
 	{
 		public IMethodSymbol CalledMethod { get; init; }
 		public MultiValue Instance { get; init; }
@@ -84,11 +84,8 @@ public IEnumerable<Diagnostic> CollectDiagnostics (DataFlowAnalyzerContext conte
 
 			foreach (var requiresAnalyzer in context.EnabledRequiresAnalyzers)
 			{
-				if (requiresAnalyzer.CheckAndCreateRequiresDiagnostic (Operation, CalledMethod, OwningSymbol, context, FeatureContext, out Diagnostic? diag)
-					&& !requiresAnalyzer.IsIntrinsicallyHandled (CalledMethod, Instance, Arguments))
-				{
-					diagnosticContext.AddDiagnostic (diag);
-				}
+				if (!requiresAnalyzer.IsIntrinsicallyHandled (CalledMethod, Instance, Arguments))
+					requiresAnalyzer.CheckAndCreateRequiresDiagnostic (Operation, CalledMethod, OwningSymbol, context, FeatureContext, in diagnosticContext);
 			}
 
 			return diagnosticContext.Diagnostics;