roslyn-sdk: Special test scenario with new unit test framework fails

I’ve got a scenario where my test case has an error and a warning - i provide a codefix or the warning. After the warning is fixed, the error is fixed automatically too.

If i expect both diagnostics get this: grafik

If i expect just one diagnostic i get this: grafik

The old Inherited CodeFixTest version makes the test pass just fine.

this is my test method:


		[TestMethod]
		public async Task MixedMethodsWithPartialFix()
		{
			var test = @"namespace ConsoleApplication1
{
	using System;
	using System.Collections.Generic;
	using System.Linq;
	using System.Text;
	using System.Threading.Tasks;
	using System.Diagnostics;
	using System.IO;

	public class TestClass
	{
		private string _workspaceFile;

		public async Task<Configuration[]> LoadStorageContentAsync()
		{
			var fileInfo = new FileInfo(_workspaceFile);
			if (!fileInfo.Exists)
			{
				Log.Error($""No configuration storage located at { fileInfo.FullName}."");
				return Array.Empty<Configuration>();
			}

			using (var stream = new StreamReader(new FileStream(fileInfo.FullName, FileMode.Open)))
			{
				try
				{
					var storage = new Storage();
					return Task.FromResult(storage.Configurations.ToArray());
				}
				catch (Exception e)
				{
					Log.Error(e);
					return Array.Empty<Configuration>();
				}
			}
		}

		public class Log
		{
			public static void Error(Exception p0)
			{
				throw new NotImplementedException();
			}

			public static void Error(string p0)
			{
				throw new NotImplementedException();
			}
		}

		public class Storage
		{
			public Configuration[] Configurations { get; set; }
		}
		public class Configuration
		{
			public Guid Id { get; set; }
			public string ConfigurationName { get; set; }
		}
	}
}";


			var fixtest = @"namespace ConsoleApplication1
{
	using System;
	using System.Collections.Generic;
	using System.Linq;
	using System.Text;
	using System.Threading.Tasks;
	using System.Diagnostics;
	using System.IO;

	public class TestClass
	{
		private string _workspaceFile;

		public Task<Configuration[]> LoadStorageContentAsync()
		{
			var fileInfo = new FileInfo(_workspaceFile);
			if (!fileInfo.Exists)
			{
				Log.Error($""No configuration storage located at { fileInfo.FullName}."");
				return Task.FromResult(Array.Empty<Configuration>());
			}

			using (var stream = new StreamReader(new FileStream(fileInfo.FullName, FileMode.Open)))
			{
				try
				{
					var storage = new Storage();
					return Task.FromResult(storage.Configurations.ToArray());
				}
				catch (Exception e)
				{
					Log.Error(e);
					return Task.FromResult(Array.Empty<Configuration>());
				}
			}
		}

		public class Log
		{
			public static void Error(Exception p0)
			{
				throw new NotImplementedException();
			}

			public static void Error(string p0)
			{
				throw new NotImplementedException();
			}
		}

		public class Storage
		{
			public Configuration[] Configurations { get; set; }
		}
		public class Configuration
		{
			public Guid Id { get; set; }
			public string ConfigurationName { get; set; }
		}
	}
}";

			// await new CodeFixTest<EmptyDiagnosticAnalyzer,
			// 	Amusoft.CodeAnalysis.Analyzers.CS1998.FixByWrappingInTaskResult>()
			// {
			// 	CompilerDiagnostics = CompilerDiagnostics.Errors | CompilerDiagnostics.Warnings,
			// 	TestState =
			// 	{
			// 		Sources = { test },
			// 		ExpectedDiagnostics =
			// 		{
			// 			CompilerWarning("CS1591").WithSpan(11, 15, 11, 24),
			// 			CompilerWarning("CS0649").WithSpan(13, 18, 13, 32),
			// 			CompilerWarning("CS1591").WithSpan(15, 38, 15, 61),
			// 			CompilerWarning("CS1998").WithLocation(15,38),
			// 			CompilerError("CS4016").WithSpan(29, 13, 29, 62).WithArguments("ConsoleApplication1.TestClass.Configuration[]"),
			//
			// 			CompilerWarning("CS1591").WithSpan(39, 16, 39, 19),
			// 			CompilerWarning("CS1591").WithSpan(41, 23, 41, 28),
			// 			CompilerWarning("CS1591").WithSpan(46, 23, 46, 28),
			// 			CompilerWarning("CS1591").WithSpan(52, 16, 52, 23),
			// 			CompilerWarning("CS1591").WithSpan(54, 27, 54, 41),
			// 			CompilerWarning("CS1591").WithSpan(56, 16, 56, 29),
			// 			CompilerWarning("CS1591").WithSpan(58, 16, 58, 18),
			// 			CompilerWarning("CS1591").WithSpan(59, 18, 59, 35),
			// 		},
			// 	},
			// 	FixedState =
			// 	{
			// 		ExpectedDiagnostics =
			// 		{
			// 			CompilerWarning("CS1591").WithSpan(11, 15, 11, 24),
			// 			CompilerWarning("CS0649").WithSpan(13, 18, 13, 32),
			// 			CompilerWarning("CS1591").WithSpan(15, 32, 15, 55),
			// 			CompilerWarning("CS1591").WithSpan(39, 16, 39, 19),
			// 			CompilerWarning("CS1591").WithSpan(41, 23, 41, 28),
			// 			CompilerWarning("CS1591").WithSpan(46, 23, 46, 28),
			// 			CompilerWarning("CS1591").WithSpan(52, 16, 52, 23),
			// 			CompilerWarning("CS1591").WithSpan(54, 27, 54, 41),
			// 			CompilerWarning("CS1591").WithSpan(56, 16, 56, 29),
			// 			CompilerWarning("CS1591").WithSpan(58, 16, 58, 18),
			// 			CompilerWarning("CS1591").WithSpan(59, 18, 59, 35),
			// 		},
			// 		Sources = { fixtest }
			// 	},
			// }.RunAsync();
			var expected = new[]
			{
				CompilerWarning("CS1998").WithLocation(15, 38),
				// CompilerError("CS4016").WithSpan(29, 13, 29, 62)
				// 	.WithArguments("ConsoleApplication1.TestClass.Configuration[]"),
			};

			await Verifier.VerifyCodeFixAsync(test, expected, fixtest);
		}

This is my CodeFix:


	[ExportCodeFixProvider(LanguageNames.CSharp, Name = "CS1998-FixByWrappingInTaskResult"), Shared]
	public class FixByWrappingInTaskResult : CodeFixProviderBase
	{
		/// <inheritdoc />
		protected override string DiagnosticId { get; } = "CS1998";

		/// <inheritdoc />
		protected override string GetEquivalenceKey(SyntaxNode rootNode)
		{
			return "CS1998-FixByWrappingInTaskResult";
		}

		/// <inheritdoc />
		protected override string GetTitle(SyntaxNode rootNode)
		{
			return Resources.MessageFormat_CS1998_FixByWrappingInTaskResult;
		}

		/// <inheritdoc />
		protected override async Task<SyntaxNode> FixedDiagnosticAsync(SyntaxNode rootNode, SyntaxNode diagnosticNode,
			CodeFixContext context,
			CancellationToken cancellationToken)
		{
			var semanticModel = await context.Document.GetSemanticModelAsync(cancellationToken)
				.ConfigureAwait(false);
			if (diagnosticNode is MethodDeclarationSyntax methodDeclarationSyntax)
			{
				var controlFlowAnalysis = semanticModel.AnalyzeControlFlow(methodDeclarationSyntax.Body);
				if (controlFlowAnalysis.ReturnStatements.Length > 0)
				{
					var documentEditor = await DocumentEditor.CreateAsync(context.Document, cancellationToken)
						.ConfigureAwait(false);

					RemoveAsyncFromMethod(documentEditor, methodDeclarationSyntax, semanticModel);

					foreach (var returnStatement in controlFlowAnalysis.ReturnStatements)
					{
						if (returnStatement is ReturnStatementSyntax returnStatementSyntax)
						{
							if (!ShouldRewrite(returnStatementSyntax))
								continue;

							documentEditor.ReplaceNode(returnStatementSyntax, RewriteExit(returnStatementSyntax));
						}
					}

					return await documentEditor.GetChangedDocument().GetSyntaxRootAsync(cancellationToken);
				}
			}

			return rootNode;
		}

		private static void RemoveAsyncFromMethod(DocumentEditor documentEditor, MethodDeclarationSyntax methodDeclarationSyntax, SemanticModel semanticModel)
		{
			documentEditor.SetModifiers(methodDeclarationSyntax, DeclarationModifiers.From(semanticModel.GetDeclaredSymbol(methodDeclarationSyntax)).WithAsync(false));
		}

		private bool ShouldRewrite(ReturnStatementSyntax returnStatementSyntax)
		{
			if (returnStatementSyntax.Expression is InvocationExpressionSyntax invocationExpressionSyntax)
			{
				if (invocationExpressionSyntax.Expression is MemberAccessExpressionSyntax memberAccessExpressionSyntax)
				{
					if (memberAccessExpressionSyntax.Name.Identifier.Text.Equals("FromResult")
					&& memberAccessExpressionSyntax.Expression is IdentifierNameSyntax expressionIdentifierName
					&& expressionIdentifierName.Identifier.Text.Equals("Task"))
					{
						return false;
					}
				}
			}

			return true;
		}

		private SyntaxNode RewriteExit(ReturnStatementSyntax returnStatement)
		{
			return returnStatement.WithExpression(
				InvocationExpression(
					MemberAccessExpression(
						SyntaxKind.SimpleMemberAccessExpression,
						IdentifierName("Task"),
						IdentifierName("FromResult")))
				.WithArgumentList(
					ArgumentList(
						SingletonSeparatedList<ArgumentSyntax>(
							Argument(
								returnStatement.Expression))))
				);
		}
	}

	/// <summary>
	/// <inheritdoc />
	/// </summary>
	public abstract class CodeFixProviderBase : CodeFixProvider
	{
		public sealed override FixAllProvider GetFixAllProvider()
		{
			// See https://github.com/dotnet/roslyn/blob/master/docs/analyzers/FixAllProvider.md for more information on Fix All Providers
			return WellKnownFixAllProviders.BatchFixer;
		}

		/// <inheritdoc />
		public override ImmutableArray<string> FixableDiagnosticIds => new[] { DiagnosticId }.ToImmutableArray();

		protected abstract string DiagnosticId { get; }

		protected const string TitleAnnotation = "META:CodeFixTitleAnnotation";
		protected const string MemberAnnotation = "META:MemberAnnotation";
		protected const string TypeAnnotation = "META:TypeAnnotation";

		protected abstract string GetEquivalenceKey(SyntaxNode rootNode);
		protected abstract string GetTitle(SyntaxNode rootNode);

		protected virtual async Task<Document> GetFixedDocumentAsync(CodeFixContext context,
			CancellationToken cancellationToken, Diagnostic diagnostic)
		{
			var rootNode = await context.Document.GetSyntaxRootAsync(cancellationToken)
				.ConfigureAwait(false);
			var fixedRoot = await FixedDiagnosticAsync(rootNode, rootNode.FindNode(diagnostic.Location.SourceSpan), context, cancellationToken)
				.ConfigureAwait(false);

			return context.Document
				.WithSyntaxRoot(fixedRoot);
		}

		protected abstract Task<SyntaxNode> FixedDiagnosticAsync(SyntaxNode rootNode, SyntaxNode diagnosticNode,
			CodeFixContext context, CancellationToken cancellationToken);

		/// <inheritdoc />
		public override async Task RegisterCodeFixesAsync(CodeFixContext context)
		{
			var originalRoot = await context.Document.GetSyntaxRootAsync(context.CancellationToken)
				.ConfigureAwait(false);

			foreach (var diagnostic in context.Diagnostics)
			{
				if (!diagnostic.Id.Equals(DiagnosticId, StringComparison.OrdinalIgnoreCase))
					continue;

				var fixedDocument = await GetFixedDocumentAsync(context, context.CancellationToken, diagnostic)
					.ConfigureAwait(false);
				var fixedRoot = await fixedDocument.GetSyntaxRootAsync(context.CancellationToken)
					.ConfigureAwait(false);

				if(fixedRoot.IsEquivalentTo(originalRoot))
					continue;

				context.RegisterCodeFix(CodeAction.Create(GetTitle(fixedRoot), c => Task.FromResult(fixedDocument), GetEquivalenceKey(fixedRoot)), diagnostic);
			}
		}

		protected string GetAnnotationValue(SyntaxNode rootNode, string annotation, string defaultValue = "unknown annotation")
		{
			return rootNode.GetAnnotations(annotation).FirstOrDefault(d => d.Kind == annotation)?.Data ?? defaultValue;
		}
	}

@sharwell Willing to contribute again if this is an option. Unless you want to take care of it yourself of course 😃

About this issue

Original URL
State: closed
Created 4 years ago
Comments: 16 (16 by maintainers)

Most upvoted comments

what are your thoughts on introducing a settings property to alter the values of the test prior to running it

I’m not a big fan of this right now, for two reasons:

Currently the “full” test form allows for basically anything you need to do, which makes it easy to just have one consistent way to get things done.
We could add a refactoring which converts a test from short form to full form, so if/when you need to do something special in a test, it’s not tedious.

That way inheriting from the test would not be necessary anymore

I don’t see this as a particular advantage. I find the inherited types quite convenient for tailoring the test framework to the majority characteristics of an analyzer library.

sharwell on Jan 20, 2020