Update edited document before continuing code fixing (#2250)

* PR feedback

* Remove unnecessary method

6 files changed, 78 insertions, 87 deletions

diff --git a/src/ILLink.CodeFix/BaseAttributeCodeFixProvider.cs b/src/ILLink.CodeFix/BaseAttributeCodeFixProvider.cs
index a5431142b..33c88c28f 100644
--- a/src/ILLink.CodeFix/BaseAttributeCodeFixProvider.cs
+++ b/src/ILLink.CodeFix/BaseAttributeCodeFixProvider.cs
@@ -32,55 +32,43 @@ namespace ILLink.CodeFix
 
 		protected async Task BaseRegisterCodeFixesAsync (CodeFixContext context)
 		{
-			var root = await context.Document.GetSyntaxRootAsync (context.CancellationToken).ConfigureAwait (false);
-
+			var document = context.Document;
+			var root = await document.GetSyntaxRootAsync (context.CancellationToken).ConfigureAwait (false);
 			var diagnostic = context.Diagnostics.First ();
-			var diagnosticSpan = diagnostic.Location.SourceSpan;
-
-			SyntaxNode targetNode = root!.FindNode (diagnosticSpan);
-			CSharpSyntaxNode? declarationSyntax = FindAttributableParent (targetNode, AttributableParentTargets);
-			if (declarationSyntax is not null) {
-				var semanticModel = await context.Document.GetSemanticModelAsync (context.CancellationToken).ConfigureAwait (false);
-				var symbol = semanticModel!.Compilation.GetTypeByMetadataName (FullyQualifiedAttributeName);
-				var document = context.Document;
-				var editor = new SyntaxEditor (root, document.Project.Solution.Workspace);
-				var generator = editor.Generator;
-				var attrArgs = GetAttributeArguments (semanticModel, targetNode, declarationSyntax, generator, diagnostic);
-				var codeFixTitle = CodeFixTitle.ToString ();
-
-				// Register a code action that will invoke the fix.
-				context.RegisterCodeFix (
-					CodeAction.Create (
-						title: codeFixTitle,
-						createChangedDocument: c => AddAttribute (
-							document, editor, generator, declarationSyntax, attrArgs, symbol!, c),
-						equivalenceKey: codeFixTitle),
-					diagnostic);
-			}
+			SyntaxNode targetNode = root!.FindNode (diagnostic.Location.SourceSpan);
+			if (FindAttributableParent (targetNode, AttributableParentTargets) is not SyntaxNode attributableNode)
+				return;
+
+			var model = await document.GetSemanticModelAsync (context.CancellationToken).ConfigureAwait (false);
+			var targetSymbol = model!.GetSymbolInfo (targetNode).Symbol!;
+
+			var attributableSymbol = model!.GetDeclaredSymbol (attributableNode)!;
+			var attributeSymbol = model!.Compilation.GetTypeByMetadataName (FullyQualifiedAttributeName)!;
+			var attributeArguments = GetAttributeArguments (attributableSymbol, targetSymbol, SyntaxGenerator.GetGenerator (document), diagnostic);
+			var codeFixTitle = CodeFixTitle.ToString ();
+
+			context.RegisterCodeFix (CodeAction.Create (
+				title: codeFixTitle,
+				createChangedDocument: ct => AddAttributeAsync (
+					document, attributableNode, attributeArguments, attributeSymbol, ct),
+				equivalenceKey: codeFixTitle), diagnostic);
 		}
 
-		private static async Task<Document> AddAttribute (
+		private static async Task<Document> AddAttributeAsync (
 			Document document,
-			SyntaxEditor editor,
-			SyntaxGenerator generator,
-			CSharpSyntaxNode containingDecl,
-			SyntaxNode[] attrArgs,
-			ITypeSymbol AttributeSymbol,
+			SyntaxNode targetNode,
+			SyntaxNode[] attributeArguments,
+			ITypeSymbol attributeSymbol,
 			CancellationToken cancellationToken)
 		{
-			var semanticModel = await document.GetSemanticModelAsync (cancellationToken).ConfigureAwait (false);
-			if (semanticModel is null)
-				return document;
-
-			var newAttribute = generator
-				.Attribute (generator.TypeExpression (AttributeSymbol), attrArgs)
-				.WithAdditionalAnnotations (
-					Simplifier.Annotation,
-					Simplifier.AddImportsAnnotation);
-
-			editor.AddAttribute (containingDecl, newAttribute);
-
-			return document.WithSyntaxRoot (editor.GetChangedRoot ());
+			var editor = await DocumentEditor.CreateAsync (document, cancellationToken).ConfigureAwait (false);
+			var generator = editor.Generator;
+			var attribute = generator.Attribute (
+				generator.TypeExpression (attributeSymbol), attributeArguments)
+				.WithAdditionalAnnotations (Simplifier.Annotation, Simplifier.AddImportsAnnotation);
+
+			editor.AddAttribute (targetNode, attribute);
+			return editor.GetChangedDocument ();
 		}
 
 		[Flags]
@@ -100,20 +88,27 @@ namespace ILLink.CodeFix
 				switch (parentNode) {
 				case LambdaExpressionSyntax:
 					return null;
+
 				case LocalFunctionStatementSyntax or BaseMethodDeclarationSyntax when targets.HasFlag (AttributeableParentTargets.MethodOrConstructor):
 				case PropertyDeclarationSyntax when targets.HasFlag (AttributeableParentTargets.Property):
 				case FieldDeclarationSyntax when targets.HasFlag (AttributeableParentTargets.Field):
 				case EventDeclarationSyntax when targets.HasFlag (AttributeableParentTargets.Event):
 					return (CSharpSyntaxNode) parentNode;
+
 				default:
 					parentNode = parentNode.Parent;
 					break;
 				}
 			}
+
 			return null;
 		}
 
-		protected abstract SyntaxNode[] GetAttributeArguments (SemanticModel semanticModel, SyntaxNode targetNode, CSharpSyntaxNode declarationSyntax, SyntaxGenerator generator, Diagnostic diagnostic);
+		protected abstract SyntaxNode[] GetAttributeArguments (
+			ISymbol attributableSymbol,
+			ISymbol targetSymbol,
+			SyntaxGenerator syntaxGenerator,
+			Diagnostic diagnostic);
 
 		protected static bool HasPublicAccessibility (ISymbol? m)
 		{
diff --git a/src/ILLink.CodeFix/RequiresAssemblyFilesCodeFixProvider.cs b/src/ILLink.CodeFix/RequiresAssemblyFilesCodeFixProvider.cs
index a4a65999d..00c430697 100644
--- a/src/ILLink.CodeFix/RequiresAssemblyFilesCodeFixProvider.cs
+++ b/src/ILLink.CodeFix/RequiresAssemblyFilesCodeFixProvider.cs
@@ -12,7 +12,6 @@ using ILLink.RoslynAnalyzer;
 using ILLink.Shared;
 using Microsoft.CodeAnalysis;
 using Microsoft.CodeAnalysis.CodeFixes;
-using Microsoft.CodeAnalysis.CSharp;
 using Microsoft.CodeAnalysis.Editing;
 
 namespace ILLink.CodeFix
@@ -35,15 +34,13 @@ namespace ILLink.CodeFix
 
 		public sealed override Task RegisterCodeFixesAsync (CodeFixContext context) => BaseRegisterCodeFixesAsync (context);
 
-		protected override SyntaxNode[] GetAttributeArguments (SemanticModel semanticModel, SyntaxNode targetNode, CSharpSyntaxNode containingDecl, SyntaxGenerator generator, Diagnostic diagnostic)
+		protected override SyntaxNode[] GetAttributeArguments (ISymbol attributableSymbol, ISymbol targetSymbol, SyntaxGenerator syntaxGenerator, Diagnostic diagnostic)
 		{
-			var containingSymbol = semanticModel.GetDeclaredSymbol (containingDecl);
-			var name = semanticModel.GetSymbolInfo (targetNode).Symbol?.Name;
-			if (string.IsNullOrEmpty (name) || HasPublicAccessibility (containingSymbol!)) {
+			var symbolDisplayName = targetSymbol.GetDisplayName ();
+			if (string.IsNullOrEmpty (symbolDisplayName) || HasPublicAccessibility (attributableSymbol))
 				return Array.Empty<SyntaxNode> ();
-			} else {
-				return new[] { generator.AttributeArgument (generator.LiteralExpression ($"Calls {name}")) };
-			}
+
+			return new[] { syntaxGenerator.AttributeArgument (syntaxGenerator.LiteralExpression ($"Calls {symbolDisplayName}")) };
 		}
 	}
 }
diff --git a/src/ILLink.CodeFix/RequiresUnreferencedCodeCodeFixProvider.cs b/src/ILLink.CodeFix/RequiresUnreferencedCodeCodeFixProvider.cs
index ba24b1ca2..903b108b0 100644
--- a/src/ILLink.CodeFix/RequiresUnreferencedCodeCodeFixProvider.cs
+++ b/src/ILLink.CodeFix/RequiresUnreferencedCodeCodeFixProvider.cs
@@ -12,7 +12,6 @@ using ILLink.RoslynAnalyzer;
 using ILLink.Shared;
 using Microsoft.CodeAnalysis;
 using Microsoft.CodeAnalysis.CodeFixes;
-using Microsoft.CodeAnalysis.CSharp;
 using Microsoft.CodeAnalysis.Editing;
 
 namespace ILLink.CodeFix
@@ -32,15 +31,13 @@ namespace ILLink.CodeFix
 
 		public sealed override Task RegisterCodeFixesAsync (CodeFixContext context) => BaseRegisterCodeFixesAsync (context);
 
-		protected override SyntaxNode[] GetAttributeArguments (SemanticModel semanticModel, SyntaxNode targetNode, CSharpSyntaxNode containingDecl, SyntaxGenerator generator, Diagnostic diagnostic)
+		protected override SyntaxNode[] GetAttributeArguments (ISymbol attributableSymbol, ISymbol targetSymbol, SyntaxGenerator syntaxGenerator, Diagnostic diagnostic)
 		{
-			var containingSymbol = semanticModel.GetDeclaredSymbol (containingDecl);
-			var name = semanticModel.GetSymbolInfo (targetNode).Symbol?.Name;
-			if (string.IsNullOrEmpty (name) || HasPublicAccessibility (containingSymbol!)) {
+			var symbolDisplayName = targetSymbol.GetDisplayName ();
+			if (string.IsNullOrEmpty (symbolDisplayName) || HasPublicAccessibility (attributableSymbol))
 				return Array.Empty<SyntaxNode> ();
-			} else {
-				return new[] { generator.AttributeArgument (generator.LiteralExpression ($"Calls {name}")) };
-			}
+
+			return new[] { syntaxGenerator.AttributeArgument (syntaxGenerator.LiteralExpression ($"Calls {symbolDisplayName}")) };
 		}
 	}
 }
diff --git a/src/ILLink.CodeFix/UnconditionalSuppressMessageCodeFixProvider.cs b/src/ILLink.CodeFix/UnconditionalSuppressMessageCodeFixProvider.cs
index 2cdd032e4..d8a347c18 100644
--- a/src/ILLink.CodeFix/UnconditionalSuppressMessageCodeFixProvider.cs
+++ b/src/ILLink.CodeFix/UnconditionalSuppressMessageCodeFixProvider.cs
@@ -11,7 +11,6 @@ using ILLink.CodeFixProvider;
 using ILLink.Shared;
 using Microsoft.CodeAnalysis;
 using Microsoft.CodeAnalysis.CodeFixes;
-using Microsoft.CodeAnalysis.CSharp;
 using Microsoft.CodeAnalysis.Editing;
 
 namespace ILLink.CodeFix
@@ -19,6 +18,7 @@ namespace ILLink.CodeFix
 	[ExportCodeFixProvider (LanguageNames.CSharp, Name = nameof (UnconditionalSuppressMessageCodeFixProvider)), Shared]
 	public class UnconditionalSuppressMessageCodeFixProvider : BaseAttributeCodeFixProvider
 	{
+		const string Justification = nameof (Justification);
 		const string UnconditionalSuppressMessageAttribute = nameof (UnconditionalSuppressMessageAttribute);
 		public const string FullyQualifiedUnconditionalSuppressMessageAttribute = "System.Diagnostics.CodeAnalysis." + UnconditionalSuppressMessageAttribute;
 
@@ -37,22 +37,24 @@ namespace ILLink.CodeFix
 
 		public sealed override Task RegisterCodeFixesAsync (CodeFixContext context) => BaseRegisterCodeFixesAsync (context);
 
-		protected override SyntaxNode[] GetAttributeArguments (SemanticModel semanticModel, SyntaxNode targetNode, CSharpSyntaxNode containingDecl, SyntaxGenerator generator, Diagnostic diagnostic)
+		protected override SyntaxNode[] GetAttributeArguments (ISymbol attributableSymbol, ISymbol targetSymbol, SyntaxGenerator syntaxGenerator, Diagnostic diagnostic)
 		{
-			// UnconditionalSuppressMessage("Rule Category", "Rule Id", Justification = "<Pending>")
-			var category = generator.LiteralExpression (diagnostic.Descriptor.Category);
-			var categoryArgument = generator.AttributeArgument (category);
-
-			var title = diagnostic.Descriptor.Title.ToString (CultureInfo.CurrentUICulture);
-			var ruleIdText = string.IsNullOrWhiteSpace (title) ? diagnostic.Id : string.Format ("{0}:{1}", diagnostic.Id, title);
-			var ruleId = generator.LiteralExpression (ruleIdText);
-			var ruleIdArgument = generator.AttributeArgument (ruleId);
-
-			var justificationExpr = generator.LiteralExpression ("<Pending>");
-			var justificationArgument = generator.AttributeArgument ("Justification", justificationExpr);
-
-			return new[] { categoryArgument, ruleIdArgument, justificationArgument };
+			// Category of the attribute
+			var ruleCategory = syntaxGenerator.AttributeArgument (
+				syntaxGenerator.LiteralExpression (diagnostic.Descriptor.Category));
+
+			// Identifier of the analysis rule the attribute applies to
+			var ruleTitle = diagnostic.Descriptor.Title.ToString (CultureInfo.CurrentUICulture);
+			var ruleId = syntaxGenerator.AttributeArgument (
+				syntaxGenerator.LiteralExpression (
+					string.IsNullOrWhiteSpace (ruleTitle) ? diagnostic.Id : $"{diagnostic.Id}:{ruleTitle}"));
+
+			// The user should provide a justification for the suppression
+			var suppressionJustification = syntaxGenerator.AttributeArgument (Justification,
+				syntaxGenerator.LiteralExpression ("<Pending>"));
+
+			// [UnconditionalSuppressWarning (category, id, Justification = "<Pending>")]
+			return new[] { ruleCategory, ruleId, suppressionJustification };
 		}
-
 	}
 }
diff --git a/test/ILLink.RoslynAnalyzer.Tests/RequiresAssemblyFilesAnalyzerTests.cs b/test/ILLink.RoslynAnalyzer.Tests/RequiresAssemblyFilesAnalyzerTests.cs
index 5d9801dd2..54e13b846 100644
--- a/test/ILLink.RoslynAnalyzer.Tests/RequiresAssemblyFilesAnalyzerTests.cs
+++ b/test/ILLink.RoslynAnalyzer.Tests/RequiresAssemblyFilesAnalyzerTests.cs
@@ -474,16 +474,16 @@ public class C
 {
     [RequiresAssemblyFiles(""message"")]
     public int M1() => 0;
-    [RequiresAssemblyFiles(""Calls M1"")]
+    [RequiresAssemblyFiles(""Calls C.M1()"")]
     int M2() => M1();
 }
 class D
 {
-    [RequiresAssemblyFiles(""Calls M1"")]
+    [RequiresAssemblyFiles(""Calls C.M1()"")]
     public int M3(C c) => c.M1();
     public class E
     {
-        [RequiresAssemblyFiles(""Calls M1"")]
+        [RequiresAssemblyFiles(""Calls C.M1()"")]
         public int M4(C c) => c.M1();
     }
 }
@@ -578,7 +578,7 @@ public class C
     [RequiresAssemblyFiles(""message"")]
     public int M1() => 0;
 
-    [RequiresAssemblyFiles(""Calls M1"")]
+    [RequiresAssemblyFiles(""Calls C.M1()"")]
     int M2 => M1();
 }";
 			return VerifyRequiresAssemblyFilesCodeFix (
@@ -663,10 +663,10 @@ public class C
     [RequiresAssemblyFiles(""message"")]
     public int M1() => 0;
 
-    [RequiresAssemblyFiles(""Calls Wrapper"")]
+    [RequiresAssemblyFiles(""Calls Wrapper()"")]
     Action M2()
     {
-        [global::System.Diagnostics.CodeAnalysis.RequiresAssemblyFilesAttribute(""Calls M1"")] void Wrapper () => M1();
+        [global::System.Diagnostics.CodeAnalysis.RequiresAssemblyFilesAttribute(""Calls C.M1()"")] void Wrapper () => M1();
         return Wrapper;
     }
 }";
diff --git a/test/ILLink.RoslynAnalyzer.Tests/RequiresUnreferencedCodeAnalyzerTests.cs b/test/ILLink.RoslynAnalyzer.Tests/RequiresUnreferencedCodeAnalyzerTests.cs
index af9cf087a..9b448970e 100644
--- a/test/ILLink.RoslynAnalyzer.Tests/RequiresUnreferencedCodeAnalyzerTests.cs
+++ b/test/ILLink.RoslynAnalyzer.Tests/RequiresUnreferencedCodeAnalyzerTests.cs
@@ -111,17 +111,17 @@ public class C
     [RequiresUnreferencedCodeAttribute(""message"")]
     public int M1() => 0;
 
-    [RequiresUnreferencedCode(""Calls M1"")]
+    [RequiresUnreferencedCode(""Calls C.M1()"")]
     int M2() => M1();
 }
 class D
 {
-    [RequiresUnreferencedCode(""Calls M1"")]
+    [RequiresUnreferencedCode(""Calls C.M1()"")]
     public int M3(C c) => c.M1();
 
     public class E
     {
-        [RequiresUnreferencedCode(""Calls M1"")]
+        [RequiresUnreferencedCode(""Calls C.M1()"")]
         public int M4(C c) => c.M1();
     }
 }
@@ -221,10 +221,10 @@ public class C
     [RequiresUnreferencedCodeAttribute(""message"")]
     public int M1() => 0;
 
-    [RequiresUnreferencedCode(""Calls Wrapper"")]
+    [RequiresUnreferencedCode(""Calls Wrapper()"")]
     Action M2()
     {
-        [global::System.Diagnostics.CodeAnalysis.RequiresUnreferencedCodeAttribute(""Calls M1"")] void Wrapper () => M1();
+        [global::System.Diagnostics.CodeAnalysis.RequiresUnreferencedCodeAttribute(""Calls C.M1()"")] void Wrapper () => M1();
         return Wrapper;
     }
 }";