From 9fe4dc065ed146744f0767c0e3518c18f1646c64 Mon Sep 17 00:00:00 2001 From: Josef Pihrt Date: Sat, 23 Nov 2024 17:17:01 +0100 Subject: [PATCH] Improve fixer for RCS1228 (#1585) --- ChangeLog.md | 1 + ...ntInDocumentationCommentCodeFixProvider.cs | 158 +++++++++++++++++ .../CodeFixes/XmlNodeCodeFixProvider.cs | 161 ++---------------- ...ParagraphToDocumentationCommentAnalyzer.cs | 2 +- ...eLineDocumentationCommentTriviaAnalyzer.cs | 2 +- .../CSharp/Extensions/SyntaxExtensions.cs | 6 +- ...nusedElementInDocumentationCommentTests.cs | 63 ++++++- ...lidReferenceInDocumentationCommentTests.cs | 2 +- 8 files changed, 234 insertions(+), 161 deletions(-) create mode 100644 src/Analyzers.CodeFixes/CSharp/CodeFixes/RemoveElementInDocumentationCommentCodeFixProvider.cs diff --git a/ChangeLog.md b/ChangeLog.md index ddd32a8843..72a940d804 100644 --- a/ChangeLog.md +++ b/ChangeLog.md @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - Fix analyzer [RCS1213](https://josefpihrt.github.io/docs/roslynator/analyzers/RCS1213) ([PR](https://github.com/dotnet/roslynator/pull/1586)) +- Improve code fixer for [RCS1228](https://josefpihrt.github.io/docs/roslynator/analyzers/RCS1228) ([PR](https://github.com/dotnet/roslynator/pull/1585)) ### Changed diff --git a/src/Analyzers.CodeFixes/CSharp/CodeFixes/RemoveElementInDocumentationCommentCodeFixProvider.cs b/src/Analyzers.CodeFixes/CSharp/CodeFixes/RemoveElementInDocumentationCommentCodeFixProvider.cs new file mode 100644 index 0000000000..0ca275c2d1 --- /dev/null +++ b/src/Analyzers.CodeFixes/CSharp/CodeFixes/RemoveElementInDocumentationCommentCodeFixProvider.cs @@ -0,0 +1,158 @@ +// Copyright (c) .NET Foundation and Contributors. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Collections.Immutable; +using System.Composition; +using System.Linq; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CodeActions; +using Microsoft.CodeAnalysis.CodeFixes; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Text; +using Roslynator.CodeFixes; +using Roslynator.CSharp.Syntax; + +namespace Roslynator.CSharp.CodeFixes; + +[ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(RemoveElementInDocumentationCommentCodeFixProvider))] +[Shared] +public sealed class RemoveElementInDocumentationCommentCodeFixProvider : BaseCodeFixProvider +{ + public override ImmutableArray FixableDiagnosticIds + { + get + { + return ImmutableArray.Create( + DiagnosticIdentifiers.UnusedElementInDocumentationComment, + DiagnosticIdentifiers.InvalidReferenceInDocumentationComment); + } + } + +#if ROSLYN_4_0 + public override FixAllProvider GetFixAllProvider() + { + return FixAllProvider.Create(async (context, document, diagnostics) => await FixAllAsync(document, diagnostics, context.CancellationToken).ConfigureAwait(false)); + + static async Task FixAllAsync( + Document document, + ImmutableArray diagnostics, + CancellationToken cancellationToken) + { + foreach (Diagnostic diagnostic in diagnostics.OrderByDescending(d => d.Location.SourceSpan.Start)) + { + (Func> CreateChangedDocument, string) result + = await GetChangedDocumentAsync(document, diagnostic, cancellationToken).ConfigureAwait(false); + + document = await result.CreateChangedDocument(cancellationToken).ConfigureAwait(false); + } + + return document; + } + } +#endif + + public override async Task RegisterCodeFixesAsync(CodeFixContext context) + { + SyntaxNode root = await context.GetSyntaxRootAsync().ConfigureAwait(false); + + if (!TryFindFirstAncestorOrSelf(root, context.Span, out XmlNodeSyntax xmlNode, findInsideTrivia: true)) + return; + + Document document = context.Document; + Diagnostic diagnostic = context.Diagnostics[0]; + + (Func> createChangedDocument, string name) + = await GetChangedDocumentAsync(document, diagnostic, context.CancellationToken).ConfigureAwait(false); + + CodeAction codeAction = CodeAction.Create( + $"Remove '{name}' element", + ct => createChangedDocument(ct), + GetEquivalenceKey(diagnostic, name)); + + context.RegisterCodeFix(codeAction, diagnostic); + } + + private static async Task<(Func>, string)> GetChangedDocumentAsync( + Document document, + Diagnostic diagnostic, + CancellationToken cancellationToken) + { + SyntaxNode root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); + + if (!TryFindFirstAncestorOrSelf(root, diagnostic.Location.SourceSpan, out XmlNodeSyntax xmlNode, findInsideTrivia: true)) + throw new InvalidOperationException(); + + XmlElementInfo elementInfo = SyntaxInfo.XmlElementInfo(xmlNode); + string name = elementInfo.LocalName; + + return (ct => RemoveElementAsync(document, elementInfo, ct), name); + } + + private static Task RemoveElementAsync( + Document document, + in XmlElementInfo elementInfo, + CancellationToken cancellationToken) + { + cancellationToken.ThrowIfCancellationRequested(); + + XmlNodeSyntax element = elementInfo.Element; + + var documentationComment = (DocumentationCommentTriviaSyntax)element.Parent; + + SyntaxList content = documentationComment.Content; + + if (content.Count(f => f.IsKind(SyntaxKind.XmlElement, SyntaxKind.XmlEmptyElement)) == 1) + { + SyntaxNode declaration = documentationComment + .GetParent(ascendOutOfTrivia: true) + .FirstAncestorOrSelf(f => f is MemberDeclarationSyntax or LocalFunctionStatementSyntax); + + SyntaxNode newNode = SyntaxRefactorings.RemoveSingleLineDocumentationComment(declaration, documentationComment); + return document.ReplaceNodeAsync(declaration, newNode, cancellationToken); + } + + int start = element.FullSpan.Start; + int end = element.FullSpan.End; + + int index = content.IndexOf(element); + + if (index > 0 + && content[index - 1].IsKind(SyntaxKind.XmlText)) + { + start = content[index - 1].FullSpan.Start; + + if (index == 1) + { + SyntaxNode parent = documentationComment.GetParent(ascendOutOfTrivia: true); + SyntaxTriviaList leadingTrivia = parent.GetLeadingTrivia(); + + index = leadingTrivia.IndexOf(documentationComment.ParentTrivia); + + if (index > 0 + && leadingTrivia[index - 1].IsKind(SyntaxKind.WhitespaceTrivia)) + { + start = leadingTrivia[index - 1].FullSpan.Start; + } + + SyntaxToken token = parent.GetFirstToken().GetPreviousToken(includeDirectives: true); + parent = parent.FirstAncestorOrSelf(f => f.FullSpan.Contains(token.FullSpan)); + + if (start > 0) + { + SyntaxTrivia trivia = parent.FindTrivia(start - 1, findInsideTrivia: true); + + if (trivia.IsKind(SyntaxKind.EndOfLineTrivia) + && start == trivia.FullSpan.End) + { + start = trivia.FullSpan.Start; + } + } + } + } + + return document.WithTextChangeAsync(new TextChange(TextSpan.FromBounds(start, end), ""), cancellationToken); + } +} diff --git a/src/Analyzers.CodeFixes/CSharp/CodeFixes/XmlNodeCodeFixProvider.cs b/src/Analyzers.CodeFixes/CSharp/CodeFixes/XmlNodeCodeFixProvider.cs index 1991c8b92d..5be0d7009c 100644 --- a/src/Analyzers.CodeFixes/CSharp/CodeFixes/XmlNodeCodeFixProvider.cs +++ b/src/Analyzers.CodeFixes/CSharp/CodeFixes/XmlNodeCodeFixProvider.cs @@ -8,7 +8,6 @@ using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CodeActions; using Microsoft.CodeAnalysis.CodeFixes; -using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.CSharp.Syntax; using Roslynator.CodeFixes; using Roslynator.CSharp.Syntax; @@ -19,16 +18,7 @@ namespace Roslynator.CSharp.CodeFixes; [Shared] public sealed class XmlNodeCodeFixProvider : BaseCodeFixProvider { - public override ImmutableArray FixableDiagnosticIds - { - get - { - return ImmutableArray.Create( - DiagnosticIdentifiers.UnusedElementInDocumentationComment, - DiagnosticIdentifiers.InvalidReferenceInDocumentationComment, - DiagnosticIdentifiers.FixDocumentationCommentTag); - } - } + public override ImmutableArray FixableDiagnosticIds => ImmutableArray.Create(DiagnosticIdentifiers.FixDocumentationCommentTag); public override async Task RegisterCodeFixesAsync(CodeFixContext context) { @@ -38,148 +28,17 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context) return; Document document = context.Document; + Diagnostic diagnostic = context.Diagnostics[0]; + XmlElementInfo elementInfo = SyntaxInfo.XmlElementInfo(xmlNode); - foreach (Diagnostic diagnostic in context.Diagnostics) - { - switch (diagnostic.Id) - { - case DiagnosticIdentifiers.UnusedElementInDocumentationComment: - case DiagnosticIdentifiers.InvalidReferenceInDocumentationComment: - { - XmlElementInfo elementInfo = SyntaxInfo.XmlElementInfo(xmlNode); - - string name = elementInfo.LocalName; - - CodeAction codeAction = CodeAction.Create( - $"Remove '{name}' element", - ct => RemoveUnusedElementInDocumentationCommentAsync(document, elementInfo, ct), - GetEquivalenceKey(diagnostic, name)); - - context.RegisterCodeFix(codeAction, diagnostic); - break; - } - case DiagnosticIdentifiers.FixDocumentationCommentTag: - { - XmlElementInfo elementInfo = SyntaxInfo.XmlElementInfo(xmlNode); - - CodeAction codeAction = CodeAction.Create( - (elementInfo.GetTag() == XmlTag.C) - ? "Rename tag to 'code'" - : "Rename tag to 'c'", - ct => FixDocumentationCommentTagAsync(document, elementInfo, ct), - GetEquivalenceKey(diagnostic)); - - context.RegisterCodeFix(codeAction, diagnostic); - break; - } - } - } - } - - private static Task RemoveUnusedElementInDocumentationCommentAsync( - Document document, - in XmlElementInfo elementInfo, - CancellationToken cancellationToken) - { - cancellationToken.ThrowIfCancellationRequested(); - - XmlNodeSyntax element = elementInfo.Element; - - var documentationComment = (DocumentationCommentTriviaSyntax)element.Parent; - - SyntaxList content = documentationComment.Content; - - int count = content.Count; - int index = content.IndexOf(element); - - if (index == 0) - { - if (count == 2 - && content[1] is XmlTextSyntax xmlText - && IsNewLine(xmlText)) - { - return document.RemoveSingleLineDocumentationComment(documentationComment, cancellationToken); - } - - if (content[index + 1] is XmlTextSyntax xmlText2 - && IsXmlTextBetweenLines(xmlText2)) - { - return document.RemoveNodesAsync(new XmlNodeSyntax[] { element, xmlText2 }, SyntaxRefactorings.DefaultRemoveOptions, cancellationToken); - } - } - else if (index == 1) - { - if (count == 3 - && content[0] is XmlTextSyntax xmlText - && IsWhitespace(xmlText) - && content[2] is XmlTextSyntax xmlText2 - && IsNewLine(xmlText2)) - { - return document.RemoveSingleLineDocumentationComment(documentationComment, cancellationToken); - } - - if (content[2] is XmlTextSyntax xmlText3 - && IsXmlTextBetweenLines(xmlText3)) - { - return document.RemoveNodesAsync(new XmlNodeSyntax[] { element, xmlText3 }, SyntaxRefactorings.DefaultRemoveOptions, cancellationToken); - } - } - else if (content[index - 1] is XmlTextSyntax xmlText - && IsXmlTextBetweenLines(xmlText)) - { - return document.RemoveNodesAsync(new XmlNodeSyntax[] { xmlText, element }, SyntaxRefactorings.DefaultRemoveOptions, cancellationToken); - } - - return document.RemoveNodeAsync(element, cancellationToken); - - static bool IsXmlTextBetweenLines(XmlTextSyntax xmlText) - { - SyntaxTokenList tokens = xmlText.TextTokens; - - SyntaxTokenList.Enumerator en = tokens.GetEnumerator(); - - if (!en.MoveNext()) - return false; - - if (IsEmptyOrWhitespace(en.Current) - && !en.MoveNext()) - { - return false; - } + CodeAction codeAction = CodeAction.Create( + (elementInfo.GetTag() == XmlTag.C) + ? "Rename tag to 'code'" + : "Rename tag to 'c'", + ct => FixDocumentationCommentTagAsync(document, elementInfo, ct), + GetEquivalenceKey(diagnostic)); - if (!en.Current.IsKind(SyntaxKind.XmlTextLiteralNewLineToken)) - return false; - - if (en.MoveNext()) - { - if (!IsEmptyOrWhitespace(en.Current)) - return false; - - if (en.MoveNext()) - return false; - } - - return true; - - static bool IsEmptyOrWhitespace(SyntaxToken token) - { - return token.IsKind(SyntaxKind.XmlTextLiteralToken) - && StringUtility.IsEmptyOrWhitespace(token.ValueText); - } - } - - static bool IsWhitespace(XmlTextSyntax xmlText) - { - string text = xmlText.TextTokens.SingleOrDefault(shouldThrow: false).ValueText; - - return text.Length > 0 - && StringUtility.IsEmptyOrWhitespace(text); - } - - static bool IsNewLine(XmlTextSyntax xmlText) - { - return xmlText.TextTokens.SingleOrDefault(shouldThrow: false).IsKind(SyntaxKind.XmlTextLiteralNewLineToken); - } + context.RegisterCodeFix(codeAction, diagnostic); } private static Task FixDocumentationCommentTagAsync( diff --git a/src/Analyzers/CSharp/Analysis/AddParagraphToDocumentationCommentAnalyzer.cs b/src/Analyzers/CSharp/Analysis/AddParagraphToDocumentationCommentAnalyzer.cs index 14720c08d5..2b987e177b 100644 --- a/src/Analyzers/CSharp/Analysis/AddParagraphToDocumentationCommentAnalyzer.cs +++ b/src/Analyzers/CSharp/Analysis/AddParagraphToDocumentationCommentAnalyzer.cs @@ -41,7 +41,7 @@ private static void AnalyzeSingleLineDocumentationCommentTrivia(SyntaxNodeAnalys { var documentationComment = (DocumentationCommentTriviaSyntax)context.Node; - if (!documentationComment.IsPartOfMemberDeclaration()) + if (!documentationComment.IsPartOfDeclaration()) return; foreach (XmlNodeSyntax node in documentationComment.Content) diff --git a/src/Analyzers/CSharp/Analysis/SingleLineDocumentationCommentTriviaAnalyzer.cs b/src/Analyzers/CSharp/Analysis/SingleLineDocumentationCommentTriviaAnalyzer.cs index fe08518d82..f2272c54f5 100644 --- a/src/Analyzers/CSharp/Analysis/SingleLineDocumentationCommentTriviaAnalyzer.cs +++ b/src/Analyzers/CSharp/Analysis/SingleLineDocumentationCommentTriviaAnalyzer.cs @@ -56,7 +56,7 @@ private static void AnalyzeSingleLineDocumentationCommentTrivia(SyntaxNodeAnalys { var documentationComment = (DocumentationCommentTriviaSyntax)context.Node; - if (!documentationComment.IsPartOfMemberDeclaration()) + if (!documentationComment.IsPartOfDeclaration()) return; bool? fixDocumentationCommentTagEnabled = null; diff --git a/src/CSharp/CSharp/Extensions/SyntaxExtensions.cs b/src/CSharp/CSharp/Extensions/SyntaxExtensions.cs index ca946bbba5..f4299f2578 100644 --- a/src/CSharp/CSharp/Extensions/SyntaxExtensions.cs +++ b/src/CSharp/CSharp/Extensions/SyntaxExtensions.cs @@ -622,12 +622,12 @@ internal static IEnumerable Elements(this DocumentationComment } } - internal static bool IsPartOfMemberDeclaration(this DocumentationCommentTriviaSyntax documentationComment) + internal static bool IsPartOfDeclaration(this DocumentationCommentTriviaSyntax documentationComment) { SyntaxNode? node = documentationComment.ParentTrivia.Token.Parent; - return node is MemberDeclarationSyntax - || node?.Parent is MemberDeclarationSyntax; + return node is MemberDeclarationSyntax or LocalFunctionStatementSyntax + || node?.Parent is MemberDeclarationSyntax or LocalFunctionStatementSyntax; } #endregion DocumentationCommentTriviaSyntax diff --git a/src/Tests/Analyzers.Tests/RCS1228UnusedElementInDocumentationCommentTests.cs b/src/Tests/Analyzers.Tests/RCS1228UnusedElementInDocumentationCommentTests.cs index c6e3652338..265a93b0e5 100644 --- a/src/Tests/Analyzers.Tests/RCS1228UnusedElementInDocumentationCommentTests.cs +++ b/src/Tests/Analyzers.Tests/RCS1228UnusedElementInDocumentationCommentTests.cs @@ -8,7 +8,7 @@ namespace Roslynator.CSharp.Analysis.Tests; -public class RCS1228UnusedElementInDocumentationCommentTests : AbstractCSharpDiagnosticVerifier +public class RCS1228UnusedElementInDocumentationCommentTests : AbstractCSharpDiagnosticVerifier { public override DiagnosticDescriptor Descriptor { get; } = DiagnosticRules.UnusedElementInDocumentationComment; @@ -37,6 +37,33 @@ void M() "); } + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.UnusedElementInDocumentationComment)] + public async Task Test_FirstElement_Pragma() + { + await VerifyDiagnosticAndFixAsync(@" +class C +{ +#pragma warning disable RCS0000 + /// [||] + /// + /// + void M() + { + } +} +", @" +class C +{ +#pragma warning disable RCS0000 + /// + /// + void M() + { + } +} +"); + } + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.UnusedElementInDocumentationComment)] public async Task Test_LastElement() { @@ -108,6 +135,33 @@ void M() "); } + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.UnusedElementInDocumentationComment)] + public async Task Test_ReturnsIsOnlyElement_LocalFunction() + { + await VerifyDiagnosticAndFixAsync(@" +class C +{ + void M() + { + /// [||] + void LF() + { + } + } +} +", @" +class C +{ + void M() + { + void LF() + { + } + } +} +"); + } + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.UnusedElementInDocumentationComment)] public async Task Test_ExampleElement() { @@ -172,14 +226,15 @@ await VerifyDiagnosticAndFixAsync(""" class C { /// - /// [||] - void M(object p) => M(p); + /// [||] + /// [||] + void M(object p1, object p2) => M(p1, p2); } """, """ class C { /// - void M(object p) => M(p); + void M(object p1, object p2) => M(p1, p2); } """); } diff --git a/src/Tests/Analyzers.Tests/RCS1263InvalidReferenceInDocumentationCommentTests.cs b/src/Tests/Analyzers.Tests/RCS1263InvalidReferenceInDocumentationCommentTests.cs index 68647e3b56..8d980db15c 100644 --- a/src/Tests/Analyzers.Tests/RCS1263InvalidReferenceInDocumentationCommentTests.cs +++ b/src/Tests/Analyzers.Tests/RCS1263InvalidReferenceInDocumentationCommentTests.cs @@ -8,7 +8,7 @@ namespace Roslynator.CSharp.Analysis.Tests; -public class RCS1263InvalidReferenceInDocumentationCommentTests : AbstractCSharpDiagnosticVerifier +public class RCS1263InvalidReferenceInDocumentationCommentTests : AbstractCSharpDiagnosticVerifier { public override DiagnosticDescriptor Descriptor { get; } = DiagnosticRules.InvalidReferenceInDocumentationComment;