Skip to content

Commit b499986

Browse files
mrahhalmrahhal
authored
Add UseStringMethodCharOverloadWithSingleCharacters analyzer and fixer (#6799)
* Implement UseStringMethodCharOverloadWithSingleCharacters analyzer with fixer * Use StringComparison's InvariantCulture * Use NotNullWhen instead * Verify diagnostic message * Pack * Rename * Add more tests * Rename * Refactor a bit * Use ImmutableArray instead of set * Remove unneeded check * Add link comment for IsASCII * Move resolving StringComparison related symbols to compilation start * Use IsPrintableAscii * Detect CultureInfo argument too and set the relevant comparison * Pass symbols used in AnalyzeOperation instead of storing in fields * Rename * Preserve certain args (IndexOf/LastIndexOf) * Remove unneeded * Update description * Pack * Use else if * Use WellKnownTypeNames * Update message to use just one arg for the method
1 parent f23133f commit b499986

File tree

28 files changed

+1406
-1
lines changed

28 files changed

+1406
-1
lines changed

‎src/NetAnalyzers/CSharp/Microsoft.NetCore.Analyzers/Performance/CSharpUseStringMethodCharOverloadWithSingleCharacters.Fixer.cs

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the MIT license. See License.txt in the project root for license information.
2+
3+
using System.Composition;
4+
using System.Linq;
5+
using Analyzer.Utilities.Extensions;
6+
using Microsoft.CodeAnalysis;
7+
using Microsoft.CodeAnalysis.CodeActions;
8+
using Microsoft.CodeAnalysis.CodeFixes;
9+
using Microsoft.CodeAnalysis.CSharp;
10+
using Microsoft.CodeAnalysis.CSharp.Syntax;
11+
using Microsoft.CodeAnalysis.Editing;
12+
using Microsoft.CodeAnalysis.Operations;
13+
using Microsoft.NetCore.Analyzers.Performance;
14+
15+
namespace Microsoft.NetCore.CSharp.Analyzers.Performance
16+
{
17+
[ExportCodeFixProvider(LanguageNames.CSharp), Shared]
18+
public sealed class CSharpUseStringMethodCharOverloadWithSingleCharactersFixer : UseStringMethodCharOverloadWithSingleCharactersFixer
19+
{
20+
protected override bool TryGetChar(SemanticModel model, SyntaxNode argumentListNode, out char c)
21+
{
22+
c = default;
23+
24+
if (argumentListNode is not ArgumentListSyntax argumentList)
25+
{
26+
return false;
27+
}
28+
29+
ArgumentSyntax? stringArgumentNode = null;
30+
foreach (var argument in argumentList.Arguments)
31+
{
32+
var argumentOperation = model.GetOperation(argument) as IArgumentOperation;
33+
if (argumentOperation?.Parameter != null && argumentOperation.Parameter.Ordinal == 0)
34+
{
35+
stringArgumentNode = argument;
36+
break;
37+
}
38+
}
39+
40+
if (stringArgumentNode != null &&
41+
stringArgumentNode.Expression is LiteralExpressionSyntax containedLiteralExpressionSyntax)
42+
{
43+
return TryGetCharFromLiteralExpressionSyntax(containedLiteralExpressionSyntax, out c);
44+
}
45+
46+
return false;
47+
48+
static bool TryGetCharFromLiteralExpressionSyntax(LiteralExpressionSyntax sourceLiteralExpressionSyntax, out char parsedCharLiteral)
49+
{
50+
parsedCharLiteral = default;
51+
if (sourceLiteralExpressionSyntax.Token.Value is string sourceLiteralValue && char.TryParse(sourceLiteralValue, out parsedCharLiteral))
52+
{
53+
return true;
54+
}
55+
56+
return false;
57+
}
58+
}
59+
60+
protected override CodeAction CreateCodeAction(Document document, SyntaxNode argumentListNode, char sourceCharLiteral)
61+
{
62+
return new CSharpReplaceStringLiteralWithCharLiteralCodeAction(document, argumentListNode, sourceCharLiteral);
63+
}
64+
65+
private sealed class CSharpReplaceStringLiteralWithCharLiteralCodeAction : ReplaceStringLiteralWithCharLiteralCodeAction
66+
{
67+
public CSharpReplaceStringLiteralWithCharLiteralCodeAction(Document document, SyntaxNode argumentListNode, char sourceCharLiteral) : base(document, argumentListNode, sourceCharLiteral)
68+
{
69+
}
70+
71+
protected override void ApplyFix(
72+
DocumentEditor editor,
73+
SemanticModel model,
74+
SyntaxNode oldArgumentListNode,
75+
char c)
76+
{
77+
var argumentNode = editor.Generator.Argument(editor.Generator.LiteralExpression(c));
78+
var arguments = new[] { argumentNode }.Concat(((ArgumentListSyntax)oldArgumentListNode).Arguments
79+
.Select(arg => model.GetOperation(arg) as IArgumentOperation)
80+
.Where(PreserveArgument)
81+
.Select(arg => arg!.Syntax));
82+
var argumentListNode = SyntaxFactory.ArgumentList(SyntaxFactory.SeparatedList(arguments));
83+
84+
editor.ReplaceNode(oldArgumentListNode, argumentListNode);
85+
}
86+
}
87+
}
88+
}

‎src/NetAnalyzers/CSharp/Microsoft.NetCore.Analyzers/Performance/CSharpUseStringMethodCharOverloadWithSingleCharacters.cs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the MIT license. See License.txt in the project root for license information.
2+
3+
using Microsoft.CodeAnalysis.CSharp.Syntax;
4+
using Microsoft.CodeAnalysis.Diagnostics;
5+
using Microsoft.NetCore.Analyzers.Performance;
6+
7+
namespace Microsoft.CodeAnalysis.CSharp.NetAnalyzers.Microsoft.NetCore.Analyzers.Performance
8+
{
9+
/// <inheritdoc/>
10+
[DiagnosticAnalyzer(LanguageNames.CSharp)]
11+
public sealed class CSharpUseStringMethodCharOverloadWithSingleCharacters : UseStringMethodCharOverloadWithSingleCharacters
12+
{
13+
protected override SyntaxNode? GetArgumentList(SyntaxNode argumentNode)
14+
{
15+
return argumentNode.FirstAncestorOrSelf<ArgumentListSyntax>();
16+
}
17+
}
18+
}

‎src/NetAnalyzers/Core/AnalyzerReleases.Unshipped.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,9 @@
44

55
Rule ID | Category | Severity | Notes
66
--------|----------|----------|-------
7+
CA1865 | Performance | Info | UseStringMethodCharOverloadWithSingleCharacters, [Documentation](https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1865)
8+
CA1866 | Performance | Info | UseStringMethodCharOverloadWithSingleCharacters, [Documentation](https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1866)
9+
CA1867 | Performance | Disabled | UseStringMethodCharOverloadWithSingleCharacters, [Documentation](https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1867)
710
CA2261 | Usage | Warning | DoNotUseConfigureAwaitWithSuppressThrowing, [Documentation](https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca2250)
811
CA1510 | Maintainability | Info | UseExceptionThrowHelpers, [Documentation](https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1510)
912
CA1511 | Maintainability | Info | UseExceptionThrowHelpers, [Documentation](https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1511)

‎src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/MicrosoftNetCoreAnalyzersResources.resx

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2111,4 +2111,13 @@ Widening and user defined conversions are not supported with generic types.</val
21112111
<data name="UseCompositeFormatDescription" xml:space="preserve">
21122112
<value>Cache and use a 'CompositeFormat' instance as the argument to this formatting operation, rather than passing in the original format string. This reduces the cost of the formatting operation.</value>
21132113
</data>
2114+
<data name="UseStringMethodCharOverloadWithSingleCharactersDescription" xml:space="preserve">
2115+
<value>The char overload is a better performing overload than a string with a single char.</value>
2116+
</data>
2117+
<data name="UseStringMethodCharOverloadWithSingleCharactersMessage" xml:space="preserve">
2118+
<value>Use 'string.{0}(char)' instead of 'string.{0}(string)' when you have a string with a single char</value>
2119+
</data>
2120+
<data name="UseStringMethodCharOverloadWithSingleCharactersTitle" xml:space="preserve">
2121+
<value>Use char overload</value>
2122+
</data>
21142123
</root>

‎src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Performance/UseStringMethodCharOverloadWithSingleCharacters.Fixer.cs

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the MIT license. See License.txt in the project root for license information.
2+
3+
using System.Collections.Immutable;
4+
using System.Threading;
5+
using System.Threading.Tasks;
6+
using Analyzer.Utilities;
7+
using Microsoft.CodeAnalysis;
8+
using Microsoft.CodeAnalysis.CodeActions;
9+
using Microsoft.CodeAnalysis.CodeFixes;
10+
using Microsoft.CodeAnalysis.Editing;
11+
using Microsoft.CodeAnalysis.Operations;
12+
13+
namespace Microsoft.NetCore.Analyzers.Performance
14+
{
15+
public abstract class UseStringMethodCharOverloadWithSingleCharactersFixer : CodeFixProvider
16+
{
17+
public override ImmutableArray<string> FixableDiagnosticIds { get; } = ImmutableArray.Create(
18+
UseStringMethodCharOverloadWithSingleCharacters.SafeTransformationRule.Id);
19+
20+
public override async Task RegisterCodeFixesAsync(CodeFixContext context)
21+
{
22+
var root = await context.Document.GetRequiredSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false);
23+
var argumentListNode = root.FindNode(context.Span, getInnermostNodeForTie: true);
24+
25+
var model = await context.Document.GetRequiredSemanticModelAsync(context.CancellationToken).ConfigureAwait(false);
26+
if (TryGetChar(model, argumentListNode, out var c))
27+
{
28+
context.RegisterCodeFix(CreateCodeAction(context.Document, argumentListNode, c), context.Diagnostics);
29+
}
30+
}
31+
32+
public override FixAllProvider GetFixAllProvider()
33+
{
34+
return WellKnownFixAllProviders.BatchFixer;
35+
}
36+
37+
protected abstract bool TryGetChar(SemanticModel model, SyntaxNode argumentListNode, out char c);
38+
39+
protected abstract CodeAction CreateCodeAction(Document document, SyntaxNode argumentListNode, char sourceCharLiteral);
40+
41+
protected abstract class ReplaceStringLiteralWithCharLiteralCodeAction : CodeAction
42+
{
43+
private readonly Document _document;
44+
private readonly SyntaxNode _argumentListNode;
45+
private readonly char _sourceCharLiteral;
46+
47+
protected ReplaceStringLiteralWithCharLiteralCodeAction(Document document, SyntaxNode argumentListNode, char sourceCharLiteral)
48+
{
49+
_document = document;
50+
_argumentListNode = argumentListNode;
51+
_sourceCharLiteral = sourceCharLiteral;
52+
}
53+
54+
public override string Title => MicrosoftNetCoreAnalyzersResources.ReplaceStringLiteralWithCharLiteralCodeActionTitle;
55+
56+
public override string EquivalenceKey => nameof(ReplaceStringLiteralWithCharLiteralCodeAction);
57+
58+
protected abstract void ApplyFix(DocumentEditor editor, SemanticModel model, SyntaxNode oldArgumentListNode, char c);
59+
60+
protected static bool PreserveArgument(IArgumentOperation? argument)
61+
{
62+
// In our target methods, IndexOf/LastIndexOf have additional int arguments for the `startIndex` and `count`
63+
// that we want to preserve when fixing.
64+
// A better method might be to detect StringComparison and CultureInfo in particular and return false on these instead,
65+
// but that will require a lot of additional effort to resolve these types from here.
66+
return argument?.Value.Type != null && argument.Value.Type.SpecialType == SpecialType.System_Int32;
67+
}
68+
69+
protected override async Task<Document> GetChangedDocumentAsync(CancellationToken cancellationToken)
70+
{
71+
var editor = await DocumentEditor.CreateAsync(_document, cancellationToken).ConfigureAwait(false);
72+
var model = await _document.GetRequiredSemanticModelAsync(cancellationToken).ConfigureAwait(false);
73+
74+
ApplyFix(editor, model, _argumentListNode, _sourceCharLiteral);
75+
76+
return editor.GetChangedDocument();
77+
}
78+
}
79+
}
80+
}

0 commit comments

Comments
 (0)