diff --git a/analyzers/tests/SonarAnalyzer.TestFramework.Test/Verification/CodeFixProviderSnippetTest.cs b/analyzers/tests/SonarAnalyzer.TestFramework.Test/Verification/CodeFixProviderSnippetTest.cs new file mode 100644 index 00000000000..4530b82f7dc --- /dev/null +++ b/analyzers/tests/SonarAnalyzer.TestFramework.Test/Verification/CodeFixProviderSnippetTest.cs @@ -0,0 +1,172 @@ +/* + * SonarAnalyzer for .NET + * Copyright (C) 2015-2024 SonarSource SA + * mailto: contact AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ + +using Microsoft.CodeAnalysis.CodeFixes; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using SonarAnalyzer.AnalysisContext; +using SonarAnalyzer.Analyzers; +using SonarAnalyzer.TestFramework.Verification; + +namespace SonarAnalyzer.Test.TestFramework.Tests.Verification; + +[TestClass] +public class CodeFixProviderSnippetTest +{ + [TestMethod] + public void VerifyCodeFix_Snippet() + { + var verifier = new VerifierBuilder + { + Analyzers = [() => new TestRule([SyntaxKind.ClassDeclaration])], + CodeFix = () => new TestRuleCodeFix((root, node) => + root.ReplaceNode(node, node.WithIdentifier(SyntaxFactory.Identifier("Changed").WithTriviaFrom(node.Identifier)))), + } + .AddSnippet(""" + class C { } + """) + .WithCodeFixedSnippet(""" + class Changed { } + """); + Action a = () => verifier.VerifyCodeFix(); + a.Should().NotThrow(); + } + + [TestMethod] + public void VerifyCodeFix_Snippet_Fail() + { + var verifier = new VerifierBuilder + { + Analyzers = [() => new TestRule([SyntaxKind.ClassDeclaration])], + CodeFix = () => new TestRuleCodeFix((root, node) => + root.ReplaceNode(node, node.WithIdentifier(SyntaxFactory.Identifier("ActualChange").WithTriviaFrom(node.Identifier)))), + } + .AddSnippet(""" + class C { } + """) + .WithCodeFixedSnippet(""" + class ExpectedChange { } + """); + Action a = () => verifier.VerifyCodeFix(); + a.Should().Throw().WithMessage( + """Expected ActualCodeWithReplacedComments().ToUnixLineEndings() to be "class ExpectedChange { }" with a length of 24 """ + + """because VerifyWhileDocumentChanges updates the document until all issues are fixed, even if the fix itself creates a new issue again. Language: CSharp7, """ + + """but "class ActualChange { }" has a length of 22, differs near "Act" (index 6)."""); + } + + [TestMethod] + public void VerifyCodeFix_Snippet_Fail_WithNoncompliantComment() + { + var verifier = new VerifierBuilder + { + Analyzers = [() => new TestRule([SyntaxKind.ClassDeclaration])], + CodeFix = () => new TestRuleCodeFix((root, node) => + root.ReplaceNode(node, node.WithIdentifier(SyntaxFactory.Identifier("ActualChange").WithTriviaFrom(node.Identifier)))), + } + .AddSnippet(""" + class C { } // Noncompliant + """) + .WithCodeFixedSnippet(""" + class ExpectedChange { } // Fixed + """); + Action a = () => verifier.VerifyCodeFix(); + a.Should().Throw().WithMessage( + """Expected ActualCodeWithReplacedComments().ToUnixLineEndings() to be "class ExpectedChange { } // Fixed" with a length of 33 """ + + """because VerifyWhileDocumentChanges updates the document until all issues are fixed, even if the fix itself creates a new issue again. Language: CSharp7, """ + + """but "class ActualChange { } // Fixed" has a length of 31, differs near "Act" (index 6)."""); + } + + [DataTestMethod] + [DataRow(true)] + [DataRow(false)] + public void VerifyCodeFix_Snippet_MultipleIssues(bool withFixAllProvider) + { + var verifier = new VerifierBuilder + { + Analyzers = [() => new TestRule([SyntaxKind.ClassDeclaration], x => x is ClassDeclarationSyntax c && c.Identifier.ValueText.Length == 1)], + CodeFix = () => new TestRuleCodeFix(withFixAllProvider, (root, node) => + root.ReplaceNode(node, node.WithIdentifier(SyntaxFactory.Identifier($"{node.Identifier.ValueText}Changed").WithTriviaFrom(node.Identifier)))), + } + .AddSnippet(""" + class A { } // Noncompliant + class B { } // Noncompliant + class C { } // Noncompliant + """) + .WithCodeFixedSnippet(""" + class AChanged { } // Fixed + class BChanged { } // Fixed + class CChanged { } // Fixed + """); + Action a = () => verifier.VerifyCodeFix(); + a.Should().NotThrow(); + } + + [DiagnosticAnalyzer(LanguageNames.CSharp)] + public class TestRule : SonarDiagnosticAnalyzer + { + public const string DiagnosticId = "TEST001"; + + private readonly DiagnosticDescriptor rule = AnalysisScaffolding.CreateDescriptorMain(DiagnosticId); + + public override ImmutableArray SupportedDiagnostics => [rule]; + + public SyntaxKind[] RegisterSyntaxKinds { get; } + public Func Filter { get; } + + public TestRule(SyntaxKind[] registerSyntaxKinds, Func filter = null) + { + RegisterSyntaxKinds = registerSyntaxKinds; + Filter = filter ?? (_ => true); + } + + protected override void Initialize(SonarAnalysisContext context) => + context.RegisterNodeAction(TestGeneratedCodeRecognizer.Instance, c => + { + if (Filter(c.Node)) + { + c.ReportIssue(rule, c.Context.Node); + } + }, RegisterSyntaxKinds); + } + + [ExportCodeFixProvider(LanguageNames.CSharp)] + public class TestRuleCodeFix(bool withFixAllProvider, Func createChangedRoot) : SonarCodeFix where TSyntax : SyntaxNode + { + public override ImmutableArray FixableDiagnosticIds => [TestRule.DiagnosticId]; + + public TestRuleCodeFix(Func createChangedRoot) : this(true, createChangedRoot) { } + + public override FixAllProvider GetFixAllProvider() => withFixAllProvider + ? base.GetFixAllProvider() + : null; + + protected override Task RegisterCodeFixesAsync(SyntaxNode root, SonarCodeFixContext context) + { + context.RegisterCodeFix("TestTitle", async x => + { + var root = await context.Document.GetSyntaxRootAsync(x); + var node = (TSyntax)root.FindNode(context.Span); + var newRoot = createChangedRoot(root, node); + return context.Document.WithSyntaxRoot(newRoot); + }, context.Diagnostics); + return Task.CompletedTask; + } + } +} diff --git a/analyzers/tests/SonarAnalyzer.TestFramework.Test/Verification/VerifierTest.cs b/analyzers/tests/SonarAnalyzer.TestFramework.Test/Verification/VerifierTest.cs index f0db6af2451..3edef97264d 100644 --- a/analyzers/tests/SonarAnalyzer.TestFramework.Test/Verification/VerifierTest.cs +++ b/analyzers/tests/SonarAnalyzer.TestFramework.Test/Verification/VerifierTest.cs @@ -19,7 +19,6 @@ */ using System.IO; -using System.Reflection; using Microsoft.CodeAnalysis.CSharp; using SonarAnalyzer.Protobuf; using SonarAnalyzer.Rules.CSharp; @@ -38,6 +37,10 @@ public class VerifierTest .AddPaths("Path.cs") .WithCodeFix() .WithCodeFixedPaths("Expected.cs"); + private static readonly VerifierBuilder DummySnippetCodeFixCS = new VerifierBuilder() + .AddSnippet("class C { }") + .WithCodeFix() + .WithCodeFixedSnippet("class C { }"); private static readonly VerifierBuilder DummyWithLocation = new VerifierBuilder(); @@ -73,9 +76,24 @@ public void Constructor_MixedLanguagePaths_Throws() => .Invoking(x => x.Build()).Should().Throw().WithMessage("Path 'File.txt' doesn't match C# file extension '.cs'."); [TestMethod] - public void Constructor_CodeFix_MissingCodeFixedPath_Throws() => + public void Constructor_CodeFix_MissingCodeFixed_Throws() => DummyCodeFixCS.WithCodeFixedPaths(null) - .Invoking(x => x.Build()).Should().Throw().WithMessage("CodeFixedPath was not set."); + .Invoking(x => x.Build()).Should().Throw().WithMessage("Either CodeFixedPath or CodeFixed must be specified."); + + [TestMethod] + public void Constructor_CodeFix_FixedSnippetAndFixedPath_Throws() => + DummyCodeFixCS.WithCodeFixedSnippet("Wrong") + .Invoking(x => x.Build()).Should().Throw().WithMessage("Either CodeFixedPath or CodeFixed must be specified, but not both."); + + [TestMethod] + public void Constructor_CodeFix_ActualPath_FixedSnippet_DoesNotThrow() => + DummyCodeFixCS.WithCodeFixedPaths(null).WithCodeFixedSnippet("OK") + .Invoking(x => x.Build()).Should().NotThrow(); + + [TestMethod] + public void Constructor_CodeFix_ActualSnippet_FixedPath_DoesNotThrow() => + DummySnippetCodeFixCS.WithCodeFixedSnippet(null).WithCodeFixedPaths("File.cs") + .Invoking(x => x.Build()).Should().NotThrow(); [TestMethod] public void Constructor_CodeFix_WrongCodeFixedPath_Throws() => @@ -98,9 +116,27 @@ public void Constructor_CodeFix_MultiplePaths_Throws() => .Invoking(x => x.Build()).Should().Throw().WithMessage("Paths must contain only 1 file, but 3 were found."); [TestMethod] - public void Constructor_CodeFix_WithSnippets_Throws() => + public void Constructor_CodeFix_SnippetAndPath_Throws() => DummyCodeFixCS.AddSnippet("Wrong") - .Invoking(x => x.Build()).Should().Throw().WithMessage("Snippets must be empty when CodeFix is set."); + .Invoking(x => x.Build()).Should().Throw().WithMessage("Either Paths or Snippets must be specified, but not both."); + + [TestMethod] + public void Constructor_CodeFix_NoSnippetAndPath_Throws() => + new VerifierBuilder().WithCodeFix().WithCodeFixedSnippet("OK") + .Invoking(x => x.Build()).Should().Throw().WithMessage("Paths cannot be empty. Add at least one file using builder.AddPaths() or AddSnippet()."); + + [TestMethod] + public void Constructor_CodeFix_WithSinglePath_DoesNotThrow() => + DummyCodeFixCS.Invoking(x => x.Build()).Should().NotThrow(); + + [TestMethod] + public void Constructor_CodeFix_WithSingleSnippets_DoesNotThrow() => + DummySnippetCodeFixCS.Invoking(x => x.Build()).Should().NotThrow(); + + [TestMethod] + public void Constructor_CodeFix_WithMoreSnippets_Throws() => + DummySnippetCodeFixCS.AddSnippet("Second").AddSnippet("Third") + .Invoking(x => x.Build()).Should().Throw().WithMessage("Snippets must contain only 1 snippet, but 3 were found."); [TestMethod] public void Constructor_CodeFix_WrongLanguage_Throws() => diff --git a/analyzers/tests/SonarAnalyzer.TestFramework/Verification/CodeFixVerifier.cs b/analyzers/tests/SonarAnalyzer.TestFramework/Verification/CodeFixVerifier.cs index 6c69f456090..b4ebda4ff35 100644 --- a/analyzers/tests/SonarAnalyzer.TestFramework/Verification/CodeFixVerifier.cs +++ b/analyzers/tests/SonarAnalyzer.TestFramework/Verification/CodeFixVerifier.cs @@ -42,7 +42,10 @@ public CodeFixVerifier(DiagnosticAnalyzer analyzer, CodeFixProvider codeFix, Doc this.codeFixTitle = codeFixTitle; } - public void VerifyWhileDocumentChanges(ParseOptions parseOptions, string pathToExpected) + public void VerifyWhileDocumentChanges(ParseOptions parseOptions, FileInfo pathToExpected) => + VerifyWhileDocumentChanges(parseOptions, File.ReadAllText(pathToExpected.FullName)); + + public void VerifyWhileDocumentChanges(ParseOptions parseOptions, string expectedCode) { var state = new State(analyzer, originalDocument, parseOptions); var codeFixExecuted = false; @@ -63,10 +66,13 @@ public void VerifyWhileDocumentChanges(ParseOptions parseOptions, string pathToE while (codeBeforeFix != state.ActualCode); codeFixExecuted.Should().BeTrue(); - state.AssertExpected(pathToExpected, nameof(VerifyWhileDocumentChanges) + " updates the document until all issues are fixed, even if the fix itself creates a new issue again"); + state.AssertExpected(expectedCode, nameof(VerifyWhileDocumentChanges) + " updates the document until all issues are fixed, even if the fix itself creates a new issue again"); } - public void VerifyFixAllProvider(FixAllProvider fixAllProvider, ParseOptions parseOptions, string pathToExpected) + public void VerifyFixAllProvider(FixAllProvider fixAllProvider, ParseOptions parseOptions, FileInfo pathToExpected) => + VerifyFixAllProvider(fixAllProvider, parseOptions, File.ReadAllText(pathToExpected.FullName)); + + public void VerifyFixAllProvider(FixAllProvider fixAllProvider, ParseOptions parseOptions, string expectedCode) { var state = new State(analyzer, originalDocument, parseOptions); state.Diagnostics.Should().NotBeEmpty(); @@ -78,7 +84,7 @@ public void VerifyFixAllProvider(FixAllProvider fixAllProvider, ParseOptions par codeActionToExecute.Should().NotBeNull(); new State(analyzer, ApplyCodeFix(state.Document, codeActionToExecute), parseOptions) - .AssertExpected(pathToExpected, $"{nameof(VerifyFixAllProvider)} runs {fixAllProvider.GetType().Name} once"); + .AssertExpected(expectedCode, $"{nameof(VerifyFixAllProvider)} runs {fixAllProvider.GetType().Name} once"); } private string CodeFixTitle(CodeFixProvider codeFix, State state) => @@ -116,11 +122,8 @@ public State(DiagnosticAnalyzer analyzer, Document document, ParseOptions parseO ActualCode = document.GetSyntaxRootAsync().Result.GetText().ToString(); } - public void AssertExpected(string pathToExpected, string becauseMessage) - { - var expected = File.ReadAllText(pathToExpected).ToUnixLineEndings(); - ActualCodeWithReplacedComments().ToUnixLineEndings().Should().Be(expected, $"{becauseMessage}. Language: {compilation.LanguageVersionString()}"); - } + public void AssertExpected(string expectedCode, string becauseMessage) => + ActualCodeWithReplacedComments().ToUnixLineEndings().Should().Be(expectedCode, $"{becauseMessage}. Language: {compilation.LanguageVersionString()}"); private string ActualCodeWithReplacedComments() => ActualCode.ToUnixLineEndings() diff --git a/analyzers/tests/SonarAnalyzer.TestFramework/Verification/Verifier.cs b/analyzers/tests/SonarAnalyzer.TestFramework/Verification/Verifier.cs index f14bfbdb730..8926207156a 100644 --- a/analyzers/tests/SonarAnalyzer.TestFramework/Verification/Verifier.cs +++ b/analyzers/tests/SonarAnalyzer.TestFramework/Verification/Verifier.cs @@ -137,15 +137,32 @@ public void VerifyNoIssuesIgnoreErrors() // This should never have any argume public void VerifyCodeFix() // This should never have any arguments { _ = codeFix ?? throw new InvalidOperationException($"{nameof(builder.CodeFix)} was not set."); - var document = CreateProject(false).FindDocument(Path.Combine(builder.BasePath ?? string.Empty, Path.GetFileName(builder.Paths.Single()))); + var project = CreateProject(false); + var document = builder.Paths.Any() + ? project.FindDocument(Path.Combine(builder.BasePath ?? string.Empty, Path.GetFileName(builder.Paths.Single()))) + : project.Project.Documents.Single(); var codeFixVerifier = new CodeFixVerifier(analyzers.Single(), codeFix, document, builder.CodeFixTitle); var fixAllProvider = codeFix.GetFixAllProvider(); foreach (var parseOptions in builder.ParseOptions.OrDefault(language.LanguageName)) { - codeFixVerifier.VerifyWhileDocumentChanges(parseOptions, TestCasePath(builder.CodeFixedPath)); - if (fixAllProvider is not null) + switch (builder) { - codeFixVerifier.VerifyFixAllProvider(fixAllProvider, parseOptions, TestCasePath(builder.CodeFixedPathBatch ?? builder.CodeFixedPath)); + case { CodeFixedPath: { } path }: + codeFixVerifier.VerifyWhileDocumentChanges(parseOptions, new FileInfo(TestCasePath(path))); + if (fixAllProvider is not null) + { + codeFixVerifier.VerifyFixAllProvider(fixAllProvider, parseOptions, new FileInfo(TestCasePath(builder.CodeFixedPathBatch ?? builder.CodeFixedPath))); + } + break; + case { CodeFixed: { } fixedCode }: + codeFixVerifier.VerifyWhileDocumentChanges(parseOptions, fixedCode); + if (fixAllProvider is not null) + { + codeFixVerifier.VerifyFixAllProvider(fixAllProvider, parseOptions, fixedCode); + } + break; + default: + throw new InvalidOperationException($"No fixed code found. Specify {nameof(builder.CodeFixedPath)} or {nameof(builder.CodeFixed)}."); } } } @@ -262,17 +279,25 @@ private void ValidateExtension(string path) private void ValidateCodeFix() { - _ = builder.CodeFixedPath ?? throw new ArgumentException($"{nameof(builder.CodeFixedPath)} was not set."); ValidateSingleAnalyzer(nameof(builder.CodeFix)); - if (builder.Paths.Length != 1) + switch (builder) { - throw new ArgumentException($"{nameof(builder.Paths)} must contain only 1 file, but {builder.Paths.Length} were found."); + case { Paths.Length: > 1 }: + throw new ArgumentException($"{nameof(builder.Paths)} must contain only 1 file, but {builder.Paths.Length} were found."); + case { Snippets.Length: > 1 }: + throw new ArgumentException($"{nameof(builder.Snippets)} must contain only 1 snippet, but {builder.Snippets.Length} were found."); + case { Paths.Length: 1, Snippets.Length: 1 }: + throw new ArgumentException($"Either {nameof(builder.Paths)} or {nameof(builder.Snippets)} must be specified, but not both."); + case { Paths.Length: 0, Snippets.Length: 0 }: + throw new ArgumentException($"Either {nameof(builder.Paths)} or {nameof(builder.Snippets)} must contain a single item, but both were empty."); + case { CodeFixedPath: null, CodeFixed: null }: + throw new ArgumentException($"Either {nameof(builder.CodeFixedPath)} or {nameof(builder.CodeFixed)} must be specified."); + case { CodeFixedPath: not null, CodeFixed: not null }: + throw new ArgumentException($"Either {nameof(builder.CodeFixedPath)} or {nameof(builder.CodeFixed)} must be specified, but not both."); + case { CodeFixedPath: { } codeFixPath}: + ValidateExtension(codeFixPath); + break; } - if (builder.Snippets.Any()) - { - throw new ArgumentException($"{nameof(builder.Snippets)} must be empty when {nameof(builder.CodeFix)} is set."); - } - ValidateExtension(builder.CodeFixedPath); if (builder.CodeFixedPathBatch is not null) { ValidateExtension(builder.CodeFixedPathBatch); diff --git a/analyzers/tests/SonarAnalyzer.TestFramework/Verification/VerifierBuilder.cs b/analyzers/tests/SonarAnalyzer.TestFramework/Verification/VerifierBuilder.cs index 4a019fe2da5..c189c324bde 100644 --- a/analyzers/tests/SonarAnalyzer.TestFramework/Verification/VerifierBuilder.cs +++ b/analyzers/tests/SonarAnalyzer.TestFramework/Verification/VerifierBuilder.cs @@ -45,6 +45,7 @@ public record VerifierBuilder public string BasePath { get; init; } public Func CodeFix { get; init; } public string CodeFixedPath { get; init; } + public string CodeFixed { get; init; } public string CodeFixedPathBatch { get; init; } public string CodeFixTitle { get; init; } public bool ConcurrentAnalysis { get; init; } = true; @@ -96,6 +97,9 @@ public VerifierBuilder WithBasePath(string basePath) => public VerifierBuilder WithCodeFixedPaths(string codeFixedPath, string codeFixedPathBatch = null) => this with { CodeFixedPath = codeFixedPath, CodeFixedPathBatch = codeFixedPathBatch }; + public VerifierBuilder WithCodeFixedSnippet(string codeFixed) => + this with { CodeFixed = codeFixed }; + public VerifierBuilder WithCodeFixTitle(string codeFixTitle) => this with { CodeFixTitle = codeFixTitle };