From 72d8f0cc81bb2cf065a50950c3261dfba9005db6 Mon Sep 17 00:00:00 2001 From: Josef Pihrt Date: Sun, 21 Jan 2024 19:48:18 +0100 Subject: [PATCH] Remove interpolation and concatenation from RCS1197 (#1370) --- ChangeLog.md | 5 + ...eStringBuilderAppendCallCodeFixProvider.cs | 170 +-------- ...OptimizeStringBuilderAppendCallAnalysis.cs | 27 -- ...197OptimizeStringBuilderAppendCallTests.cs | 325 +++--------------- 4 files changed, 62 insertions(+), 465 deletions(-) diff --git a/ChangeLog.md b/ChangeLog.md index 8ec689d57a..577feddf89 100644 --- a/ChangeLog.md +++ b/ChangeLog.md @@ -15,6 +15,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - [CLI] Spellcheck file names ([PR](https://github.com/dotnet/roslynator/pull/1368)) - `roslynator spellcheck --scope file-name` +### Changed + +- Update analyzer [RCS1197](https://josefpihrt.github.io/docs/roslynator/analyzers/RCS1197) ([PR](https://github.com/dotnet/roslynator/pull/1370)) + - Do not report interpolated string and string concatenation + ### Fixed - Fix analyzer [RCS1055](https://josefpihrt.github.io/docs/roslynator/analyzers/RCS1055) ([PR](https://github.com/dotnet/roslynator/pull/1361)) diff --git a/src/Analyzers.CodeFixes/CSharp/CodeFixes/OptimizeStringBuilderAppendCallCodeFixProvider.cs b/src/Analyzers.CodeFixes/CSharp/CodeFixes/OptimizeStringBuilderAppendCallCodeFixProvider.cs index bd1d9dc4e1..a7397f6851 100644 --- a/src/Analyzers.CodeFixes/CSharp/CodeFixes/OptimizeStringBuilderAppendCallCodeFixProvider.cs +++ b/src/Analyzers.CodeFixes/CSharp/CodeFixes/OptimizeStringBuilderAppendCallCodeFixProvider.cs @@ -11,7 +11,6 @@ using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.CSharp.Syntax; using Roslynator.CodeFixes; -using Roslynator.CSharp.Refactorings; using Roslynator.CSharp.Syntax; using static Microsoft.CodeAnalysis.CSharp.SyntaxFactory; using static Roslynator.CSharp.CSharpFactory; @@ -94,8 +93,6 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context) SimpleMemberInvocationExpressionInfo invocationInfo, CancellationToken cancellationToken) { - SemanticModel semanticModel = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false); - InvocationExpressionSyntax invocation = invocationInfo.InvocationExpression; InvocationExpressionSyntax newInvocation; @@ -103,74 +100,12 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context) ExpressionSyntax expression = argument.Expression; - switch (expression.Kind()) - { - case SyntaxKind.InterpolatedStringExpression: - { - newInvocation = ConvertInterpolatedStringExpressionToInvocationExpression((InterpolatedStringExpressionSyntax)expression, invocationInfo, semanticModel); - break; - } - case SyntaxKind.AddExpression: - { - ImmutableArray expressions = SyntaxInfo.BinaryExpressionInfo((BinaryExpressionSyntax)expression) - .AsChain() - .ToImmutableArray(); - - newInvocation = invocation - .ReplaceNode(invocationInfo.Name, IdentifierName("Append").WithTriviaFrom(invocationInfo.Name)) - .WithArgumentList(invocation.ArgumentList.WithArguments(SingletonSeparatedList(Argument(ReplaceStringLiteralWithCharacterLiteral(expressions[0])))).WithoutTrailingTrivia()); - - for (int i = 1; i < expressions.Length; i++) - { - ExpressionSyntax argumentExpression = expressions[i]; - - string methodName; - if (i == expressions.Length - 1 - && isAppendLine - && semanticModel - .GetTypeInfo(argumentExpression, cancellationToken) - .ConvertedType? - .SpecialType == SpecialType.System_String) - { - methodName = "AppendLine"; - } - else - { - methodName = "Append"; - - argumentExpression = ReplaceStringLiteralWithCharacterLiteral(argumentExpression); - } - - newInvocation = SimpleMemberInvocationExpression( - newInvocation, - IdentifierName(methodName), - ArgumentList(Argument(argumentExpression))); - - if (i == expressions.Length - 1 - && isAppendLine - && !string.Equals(methodName, "AppendLine", StringComparison.Ordinal)) - { - newInvocation = SimpleMemberInvocationExpression( - newInvocation, - IdentifierName("AppendLine"), - ArgumentList()); - } - } - - break; - } - default: - { - newInvocation = CreateInvocationExpression( - (InvocationExpressionSyntax)expression, - invocation); + newInvocation = CreateInvocationExpression( + (InvocationExpressionSyntax)expression, + invocation); - if (isAppendLine) - newInvocation = SimpleMemberInvocationExpression(newInvocation, IdentifierName("AppendLine"), ArgumentList()); - - break; - } - } + if (isAppendLine) + newInvocation = SimpleMemberInvocationExpression(newInvocation, IdentifierName("AppendLine"), ArgumentList()); newInvocation = newInvocation .WithTriviaFrom(invocation) @@ -179,68 +114,6 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context) return await document.ReplaceNodeAsync(invocation, newInvocation, cancellationToken).ConfigureAwait(false); } - private static InvocationExpressionSyntax ConvertInterpolatedStringExpressionToInvocationExpression( - InterpolatedStringExpressionSyntax interpolatedString, - in SimpleMemberInvocationExpressionInfo invocationInfo, - SemanticModel semanticModel) - { - bool isVerbatim = interpolatedString.IsVerbatim(); - - bool isAppendLine = string.Equals(invocationInfo.NameText, "AppendLine", StringComparison.Ordinal); - - InvocationExpressionSyntax invocation = invocationInfo.InvocationExpression; - - InvocationExpressionSyntax newExpression = null; - - SyntaxList contents = interpolatedString.Contents; - - for (int i = 0; i < contents.Count; i++) - { - (SyntaxKind contentKind, string methodName, ImmutableArray arguments) = ConvertInterpolatedStringToStringBuilderMethodRefactoring.Refactor(contents[i], isVerbatim); - - if (i == contents.Count - 1 - && isAppendLine - && string.Equals(methodName, "Append", StringComparison.Ordinal) - && (contentKind == SyntaxKind.InterpolatedStringText - || semanticModel.IsImplicitConversion(((InterpolationSyntax)contents[i]).Expression, semanticModel.Compilation.GetSpecialType(SpecialType.System_String)))) - { - methodName = "AppendLine"; - } - else if (methodName == "Append") - { - arguments = ReplaceStringLiteralWithCharacterLiteral(arguments); - } - - if (newExpression is null) - { - arguments = arguments.Replace(arguments[0], arguments[0].WithLeadingTrivia(interpolatedString.GetLeadingTrivia())); - - newExpression = invocation - .ReplaceNode(invocationInfo.Name, IdentifierName(methodName).WithTriviaFrom(invocationInfo.Name)) - .WithArgumentList(invocation.ArgumentList.WithArguments(arguments.ToSeparatedSyntaxList()).WithoutTrailingTrivia()); - } - else - { - newExpression = SimpleMemberInvocationExpression( - newExpression, - IdentifierName(methodName), - ArgumentList(arguments.ToSeparatedSyntaxList())); - } - - if (i == contents.Count - 1 - && isAppendLine - && !string.Equals(methodName, "AppendLine", StringComparison.Ordinal)) - { - newExpression = SimpleMemberInvocationExpression( - newExpression, - IdentifierName("AppendLine"), - ArgumentList()); - } - } - - return newExpression; - } - private static InvocationExpressionSyntax CreateInvocationExpression( InvocationExpressionSyntax innerInvocationExpression, InvocationExpressionSyntax outerInvocationExpression) @@ -317,37 +190,4 @@ private static InvocationExpressionSyntax CreateNewInvocationExpression(Invocati .WithExpression(memberAccess.WithName(IdentifierName(methodName).WithTriviaFrom(memberAccess.Name))) .WithArgumentList(argumentList); } - - private static ExpressionSyntax ReplaceStringLiteralWithCharacterLiteral(ExpressionSyntax expression) - { - if (expression.IsKind(SyntaxKind.StringLiteralExpression)) - { - var literalExpression = (LiteralExpressionSyntax)expression; - - if (literalExpression.Token.ValueText.Length == 1) - return SyntaxRefactorings.ReplaceStringLiteralWithCharacterLiteral(literalExpression); - } - - return expression; - } - - private static ImmutableArray ReplaceStringLiteralWithCharacterLiteral(ImmutableArray arguments) - { - ArgumentSyntax argument = arguments.SingleOrDefault(shouldThrow: false); - - if (argument is not null) - { - ExpressionSyntax expression = argument.Expression; - - if (expression is not null) - { - ExpressionSyntax newExpression = ReplaceStringLiteralWithCharacterLiteral(expression); - - if (newExpression != expression) - arguments = arguments.Replace(argument, argument.WithExpression(newExpression)); - } - } - - return arguments; - } } diff --git a/src/Analyzers/CSharp/Analysis/OptimizeStringBuilderAppendCallAnalysis.cs b/src/Analyzers/CSharp/Analysis/OptimizeStringBuilderAppendCallAnalysis.cs index 1a4e5bba5a..809c8cefae 100644 --- a/src/Analyzers/CSharp/Analysis/OptimizeStringBuilderAppendCallAnalysis.cs +++ b/src/Analyzers/CSharp/Analysis/OptimizeStringBuilderAppendCallAnalysis.cs @@ -69,33 +69,6 @@ public static void Analyze(SyntaxNodeAnalysisContext context, in SimpleMemberInv SyntaxKind expressionKind = expression.Kind(); - switch (expressionKind) - { - case SyntaxKind.InterpolatedStringExpression: - { - if (((CSharpCompilation)context.Compilation).LanguageVersion <= LanguageVersion.CSharp9 - || !context.SemanticModel.HasConstantValue(expression, context.CancellationToken)) - { - ReportDiagnostic(argument); - } - - return; - } - case SyntaxKind.AddExpression: - { - BinaryExpressionInfo binaryExpressionInfo = SyntaxInfo.BinaryExpressionInfo((BinaryExpressionSyntax)expression); - - if (binaryExpressionInfo.Success - && binaryExpressionInfo.AsChain().Reverse().IsStringConcatenation(context.SemanticModel, context.CancellationToken) - && !context.SemanticModel.GetConstantValue(expression, context.CancellationToken).HasValue) - { - ReportDiagnostic(argument); - } - - return; - } - } - if (expressionKind != SyntaxKind.InvocationExpression) return; diff --git a/src/Tests/Analyzers.Tests/RCS1197OptimizeStringBuilderAppendCallTests.cs b/src/Tests/Analyzers.Tests/RCS1197OptimizeStringBuilderAppendCallTests.cs index fdf2df679e..b7e27d0d00 100644 --- a/src/Tests/Analyzers.Tests/RCS1197OptimizeStringBuilderAppendCallTests.cs +++ b/src/Tests/Analyzers.Tests/RCS1197OptimizeStringBuilderAppendCallTests.cs @@ -305,22 +305,9 @@ void M() } [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.OptimizeStringBuilderAppendCall)] - public async Task Test_InterpolatedString() + public async Task TestNoDiagnostic_InterpolatedString() { - await VerifyDiagnosticAndFixAsync(@" -using System.Text; - -class C -{ - void M() - { - string s = null; - var sb = new StringBuilder(); - - sb.Append([|$""{s}s""|]); - } -} -", @" + await VerifyNoDiagnosticAsync(@" using System.Text; class C @@ -330,30 +317,16 @@ void M() string s = null; var sb = new StringBuilder(); - sb.Append(s).Append('s'); + sb.Append($""{s}s""); } } "); } [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.OptimizeStringBuilderAppendCall)] - public async Task Test_InterpolatedString_Braces() - { - await VerifyDiagnosticAndFixAsync(@" -using System.Text; - -class C -{ - void M() + public async Task TestNoDiagnostic_InterpolatedString_Braces() { - string s = null; - var sb = new StringBuilder(); - - sb.Append( - [|$""a{{b}}c{s}a{{b}}c""|]); - } -} -", @" + await VerifyNoDiagnosticAsync(@" using System.Text; class C @@ -364,27 +337,16 @@ void M() var sb = new StringBuilder(); sb.Append( - ""a{b}c"").Append(s).Append(""a{b}c""); + $""a{{b}}c{s}a{{b}}c""); } } "); } [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.OptimizeStringBuilderAppendCall)] - public async Task Test_InterpolatedRawString_ContainingQuotes() + public async Task TestNoDiagnostic_InterpolatedRawString_ContainingQuotes() { - await VerifyDiagnosticAndFixAsync(@" -using System.Text; -class C -{ - void M() - { - string s = null; - var sb = new StringBuilder(); - sb.Append([|$""""""""{s}""""""""|]); - } -} -", @" + await VerifyNoDiagnosticAsync(@" using System.Text; class C { @@ -392,16 +354,16 @@ void M() { string s = null; var sb = new StringBuilder(); - sb.Append("""""""""""""""").Append(s).Append(""""""""""""""""); + sb.Append($""""""""{s}""""""""); } } ", options: WellKnownCSharpTestOptions.Default_CSharp11); } [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.OptimizeStringBuilderAppendCall)] - public async Task Test_InterpolatedMultilineRawString_ContainingQuotes() + public async Task TestNoDiagnostic_InterpolatedMultilineRawString_ContainingQuotes() { - await VerifyDiagnosticAndFixAsync(@" + await VerifyNoDiagnosticAsync(@" using System.Text; class C { @@ -409,47 +371,19 @@ void M() { string s = null; var sb = new StringBuilder(); - sb.Append([|$"""""" + sb.Append($"""""" {s} -""""""|] +"""""" ); } } -", @" -using System.Text; -class C -{ - void M() - { - string s = null; - var sb = new StringBuilder(); - sb.Append("""""" - -"""""").Append(s).Append("""""" - -""""""); - } -} ", options: WellKnownCSharpTestOptions.Default_CSharp11); } [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.OptimizeStringBuilderAppendCall)] - public async Task Test_InterpolatedString_Char() + public async Task TestNoDiagnostic_InterpolatedString_Char() { - await VerifyDiagnosticAndFixAsync(@" -using System.Text; - -class C -{ - void M() - { - string s = null; - var sb = new StringBuilder(); - - sb.Append([|$""\""{s}'""|]); - } -} -", @" + await VerifyNoDiagnosticAsync(@" using System.Text; class C @@ -459,29 +393,16 @@ void M() string s = null; var sb = new StringBuilder(); - sb.Append('\""').Append(s).Append('\''); + sb.Append($""\""{s}'""); } } "); } [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.OptimizeStringBuilderAppendCall)] - public async Task Test_InterpolatedString_AppendLine() - { - await VerifyDiagnosticAndFixAsync(@" -using System.Text; - -class C -{ - void M() + public async Task TestNoDiagnostic_InterpolatedString_AppendLine() { - string s = null; - var sb = new StringBuilder(); - - sb.AppendLine([|$""{s}s""|]); - } -} -", @" + await VerifyNoDiagnosticAsync(@" using System.Text; class C @@ -491,28 +412,16 @@ void M() string s = null; var sb = new StringBuilder(); - sb.Append(s).AppendLine(""s""); + sb.AppendLine($""{s}s""); } } "); } [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.OptimizeStringBuilderAppendCall)] - public async Task Test_InterpolatedString_AppendLine2() - { - await VerifyDiagnosticAndFixAsync(@" -using System.Text; - -class C -{ - void M() + public async Task TestNoDiagnostic_InterpolatedString_AppendLine2() { - var sb = new StringBuilder(); - - sb.AppendLine([|$""ab{'s'}""|]); - } -} -", @" + await VerifyNoDiagnosticAsync(@" using System.Text; class C @@ -521,28 +430,16 @@ void M() { var sb = new StringBuilder(); - sb.Append(""ab"").Append('s').AppendLine(); + sb.AppendLine($""ab{'s'}""); } } "); } [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.OptimizeStringBuilderAppendCall)] - public async Task Test_InterpolatedString_AppendLine3() + public async Task TestNoDiagnostic_InterpolatedString_AppendLine3() { - await VerifyDiagnosticAndFixAsync(@" -using System.Text; - -class C -{ - void M() - { - var sb = new StringBuilder(); - - sb.AppendLine([|$""ab{'s'}s""|]); - } -} -", @" + await VerifyNoDiagnosticAsync(@" using System.Text; class C @@ -551,29 +448,16 @@ void M() { var sb = new StringBuilder(); - sb.Append(""ab"").Append('s').AppendLine(""s""); + sb.AppendLine($""ab{'s'}s""); } } "); } [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.OptimizeStringBuilderAppendCall)] - public async Task Test_InterpolatedString_WithFormat_AppendLine() - { - await VerifyDiagnosticAndFixAsync(@" -using System.Text; - -class C -{ - void M() + public async Task TestNoDiagnostic_InterpolatedString_WithFormat_AppendLine() { - string s = null; - var sb = new StringBuilder(); - - sb.AppendLine([|$""{s,1:f}""|]); - } -} -", @" + await VerifyNoDiagnosticAsync(@" using System.Text; class C @@ -583,29 +467,16 @@ void M() string s = null; var sb = new StringBuilder(); - sb.AppendFormat(""{0,1:f}"", s).AppendLine(); + sb.AppendLine($""{s,1:f}""); } } "); } [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.OptimizeStringBuilderAppendCall)] - public async Task Test_InterpolatedString_WithFormat_AppendLine2() + public async Task TestNoDiagnostic_InterpolatedString_WithFormat_AppendLine2() { - await VerifyDiagnosticAndFixAsync(@" -using System; -using System.Text; - -class C -{ - void M(DateTime x) - { - string s = null; - var sb = new StringBuilder(); - sb.Append([|$@""{x:hh\:mm\:ss\.fff}""|]).ToString(); - } -} -", @" + await VerifyNoDiagnosticAsync(@" using System; using System.Text; @@ -615,29 +486,16 @@ void M(DateTime x) { string s = null; var sb = new StringBuilder(); - sb.AppendFormat(@""{0:hh\:mm\:ss\.fff}"", x).ToString(); + sb.Append($@""{x:hh\:mm\:ss\.fff}"").ToString(); } } "); } [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.OptimizeStringBuilderAppendCall)] - public async Task Test_Concatenation() + public async Task TestNoDiagnostic_Concatenation() { - await VerifyDiagnosticAndFixAsync(@" -using System.Text; - -class C -{ - void M() - { - string s = null; - var sb = new StringBuilder(); - - sb.Append([|""ab"" + s + ""cd""|]).Append([|""ef"" + s + ""gh""|]); - } -} -", @" + await VerifyNoDiagnosticAsync(@" using System.Text; class C @@ -647,30 +505,16 @@ void M() string s = null; var sb = new StringBuilder(); - sb.Append(""ab"").Append(s).Append(""cd"").Append(""ef"").Append(s).Append(""gh""); + sb.Append(""ab"" + s + ""cd"").Append(""ef"" + s + ""gh""); } } "); } [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.OptimizeStringBuilderAppendCall)] - public async Task Test_Concatenation_Char() + public async Task TestNoDiagnostic_Concatenation_Char() { - await VerifyDiagnosticAndFixAsync(@" -using System.Text; - -class C -{ - void M() - { - char a = 'a'; - - var sb = new StringBuilder(); - - sb.Append([|a + ""b"" + ""c""|]); - } -} -", @" + await VerifyNoDiagnosticAsync(@" using System.Text; class C @@ -681,29 +525,16 @@ void M() var sb = new StringBuilder(); - sb.Append(a).Append('b').Append('c'); + sb.Append(a + ""b"" + ""c""); } } "); } [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.OptimizeStringBuilderAppendCall)] - public async Task Test_Concatenation_AppendLine() - { - await VerifyDiagnosticAndFixAsync(@" -using System.Text; - -class C -{ - void M() + public async Task TestNoDiagnostic_Concatenation_AppendLine() { - string s = null; - var sb = new StringBuilder(); - - sb.Append([|s + ""ab""|]); - } -} -", @" + await VerifyNoDiagnosticAsync(@" using System.Text; class C @@ -713,29 +544,16 @@ void M() string s = null; var sb = new StringBuilder(); - sb.Append(s).Append(""ab""); + sb.Append(s + ""ab""); } } "); } [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.OptimizeStringBuilderAppendCall)] - public async Task Test_Concatenation_AppendLine2() + public async Task TestNoDiagnostic_Concatenation_AppendLine2() { - await VerifyDiagnosticAndFixAsync(@" -using System.Text; - -class C -{ - void M() - { - string s = null; - var sb = new StringBuilder(); - - sb.AppendLine([|""ab"" + s|]); - } -} -", @" + await VerifyNoDiagnosticAsync(@" using System.Text; class C @@ -745,29 +563,16 @@ void M() string s = null; var sb = new StringBuilder(); - sb.Append(""ab"").AppendLine(s); + sb.AppendLine(""ab"" + s); } } "); } [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.OptimizeStringBuilderAppendCall)] - public async Task Test_Concatenation_AppendLine3() - { - await VerifyDiagnosticAndFixAsync(@" -using System.Text; - -class C -{ - void M() + public async Task TestNoDiagnostic_Concatenation_AppendLine3() { - int i = 0; - var sb = new StringBuilder(); - - sb.AppendLine([|""ab"" + i|]); - } -} -", @" + await VerifyNoDiagnosticAsync(@" using System.Text; class C @@ -777,29 +582,16 @@ void M() int i = 0; var sb = new StringBuilder(); - sb.Append(""ab"").Append(i).AppendLine(); + sb.AppendLine(""ab"" + i); } } "); } [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.OptimizeStringBuilderAppendCall)] - public async Task Test_Concatenation_AppendLine4() - { - await VerifyDiagnosticAndFixAsync(@" -using System.Text; - -class C -{ - void M() + public async Task TestNoDiagnostic_Concatenation_AppendLine4() { - object o = null; - var sb = new StringBuilder(); - - sb.AppendLine([|""ab"" + o|]); - } -} -", @" + await VerifyNoDiagnosticAsync(@" using System.Text; class C @@ -809,29 +601,16 @@ void M() object o = null; var sb = new StringBuilder(); - sb.Append(""ab"").Append(o).AppendLine(); + sb.AppendLine(""ab"" + o); } } "); } [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.OptimizeStringBuilderAppendCall)] - public async Task Test_Concatenation_AppendLine5() + public async Task TestNoDiagnostic_Concatenation_AppendLine5() { - await VerifyDiagnosticAndFixAsync(@" -using System.Text; - -class C -{ - void M() - { - string s = null; - var sb = new StringBuilder(); - - sb.AppendLine([|""ab"" + s + ""b""|]).AppendLine([|""ef"" + s + ""d""|]); - } -} -", @" + await VerifyNoDiagnosticAsync(@" using System.Text; class C @@ -841,7 +620,7 @@ void M() string s = null; var sb = new StringBuilder(); - sb.Append(""ab"").Append(s).AppendLine(""b"").Append(""ef"").Append(s).AppendLine(""d""); + sb.AppendLine(""ab"" + s + ""b"").AppendLine(""ef"" + s + ""d""); } } ");