Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add CA1872: Prefer 'Convert.ToHexString' over 'BitConverter.ToString' #6967

Merged
merged 13 commits into from
Mar 19, 2024
Merged
1 change: 1 addition & 0 deletions src/NetAnalyzers/Core/AnalyzerReleases.Unshipped.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ Rule ID | Category | Severity | Notes
CA1514 | Maintainability | Info | AvoidLengthCheckWhenSlicingToEndAnalyzer, [Documentation](https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1514)
CA1515 | Maintainability | Disabled | MakeTypesInternal, [Documentation](https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1515)
CA1871 | Performance | Info | DoNotPassNonNullableValueToArgumentNullExceptionThrowIfNull, [Documentation](https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1871)
CA1872 | Performance | Info | PreferConvertToHexStringOverBitConverterAnalyzer, [Documentation](https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1872)
CA2262 | Usage | Info | ProvideHttpClientHandlerMaxResponseHeaderLengthValueCorrectly, [Documentation](https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca2262)
CA2263 | Usage | Info | PreferGenericOverloadsAnalyzer, [Documentation](https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca2263)
CA2264 | Usage | Warning | DoNotPassNonNullableValueToArgumentNullExceptionThrowIfNull, [Documentation](https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca2264)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,18 @@
<data name="DoNotUseCountWhenAnyCanBeUsedTitle" xml:space="preserve">
<value>Do not use Count() or LongCount() when Any() can be used</value>
</data>
<data name="PreferConvertToHexStringOverBitConverterTitle" xml:space="preserve">
<value>Prefer 'Convert.ToHexString' and 'Convert.ToHexStringLower' over call chains based on 'BitConverter.ToString'</value>
</data>
<data name="PreferConvertToHexStringOverBitConverterDescription" xml:space="preserve">
<value>Use 'Convert.ToHexString' or 'Convert.ToHexStringLower' when encoding bytes to a hexadecimal string representation. These methods are more efficient and allocation-friendly than using 'BitConverter.ToString' in combination with 'String.Replace' to replace dashes and 'String.ToLower'.</value>
</data>
<data name="PreferConvertToHexStringOverBitConverterMessage" xml:space="preserve">
<value>Prefer '{0}' over call chains based on '{1}'</value>
</data>
<data name="PreferConvertToHexStringOverBitConverterCodeFixTitle" xml:space="preserve">
<value>Replace with 'Convert.{0}'</value>
</data>
<data name="DoNotUseTimersThatPreventPowerStateChangesTitle" xml:space="preserve">
<value>Do not use timers that prevent power state changes</value>
</data>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the MIT license. See License.txt in the project root for license information.

using System;
using System.Collections.Immutable;
using System.Composition;
using System.Globalization;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
using Analyzer.Utilities;
using Analyzer.Utilities.Extensions;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CodeActions;
using Microsoft.CodeAnalysis.CodeFixes;
using Microsoft.CodeAnalysis.Editing;
using Microsoft.CodeAnalysis.Operations;
using Microsoft.CodeAnalysis.Text;

namespace Microsoft.NetCore.Analyzers.Performance
{
using static MicrosoftNetCoreAnalyzersResources;

/// <summary>
/// CA1872: <inheritdoc cref="PreferConvertToHexStringOverBitConverterTitle"/>
/// </summary>
[ExportCodeFixProvider(LanguageNames.CSharp, LanguageNames.VisualBasic), Shared]
public sealed class PreferConvertToHexStringOverBitConverterFixer : CodeFixProvider
{
private static readonly SyntaxAnnotation s_asSpanSymbolAnnotation = new("SymbolId", WellKnownTypeNames.SystemMemoryExtensions);

public sealed override ImmutableArray<string> FixableDiagnosticIds { get; } =
ImmutableArray.Create(PreferConvertToHexStringOverBitConverterAnalyzer.RuleId);

public sealed override FixAllProvider GetFixAllProvider()
{
return WellKnownFixAllProviders.BatchFixer;
}

public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context)
{
var diagnostic = context.Diagnostics.FirstOrDefault();

if (diagnostic is not { AdditionalLocations.Count: > 0, Properties.Count: 1 } ||
!diagnostic.Properties.TryGetValue(PreferConvertToHexStringOverBitConverterAnalyzer.ReplacementPropertiesKey, out var convertToHexStringName) ||
convertToHexStringName is null)
{
return;
}

var root = await context.Document.GetRequiredSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false);
var semanticModel = await context.Document.GetRequiredSemanticModelAsync(context.CancellationToken).ConfigureAwait(false);

var bitConverterInvocation = GetInvocationFromTextSpan(diagnostic.AdditionalLocations[0].SourceSpan);
var outerInvocation = GetInvocationFromTextSpan(context.Span);

if (bitConverterInvocation is null || outerInvocation is null)
{
return;
}

var toLowerInvocation = diagnostic.AdditionalLocations.Count == 2
? GetInvocationFromTextSpan(diagnostic.AdditionalLocations[1].SourceSpan)
: null;

var codeAction = CodeAction.Create(
string.Format(CultureInfo.CurrentCulture, PreferConvertToHexStringOverBitConverterCodeFixTitle, convertToHexStringName),
ReplaceWithConvertToHexStringCall,
nameof(PreferConvertToHexStringOverBitConverterCodeFixTitle));

context.RegisterCodeFix(codeAction, context.Diagnostics);

IInvocationOperation? GetInvocationFromTextSpan(TextSpan span)
{
var node = root.FindNode(span, getInnermostNodeForTie: true);

if (node is null)
{
return null;
}

return semanticModel.GetOperation(node, context.CancellationToken) as IInvocationOperation;
}

async Task<Document> ReplaceWithConvertToHexStringCall(CancellationToken cancellationToken)
{
var editor = await DocumentEditor.CreateAsync(context.Document, cancellationToken).ConfigureAwait(false);
var generator = editor.Generator;
var bitConverterArgumentsInParameterOrder = bitConverterInvocation.Arguments.GetArgumentsInParameterOrder();

var typeExpression = generator.DottedName(WellKnownTypeNames.SystemConvert);
var methodExpression = generator.MemberAccessExpression(typeExpression, convertToHexStringName);
var methodInvocation = bitConverterArgumentsInParameterOrder.Length switch
{
// BitConverter.ToString(data).Replace("-", "") => Convert.ToHexString(data)
1 => generator.InvocationExpression(methodExpression, bitConverterArgumentsInParameterOrder[0].Value.Syntax),
// BitConverter.ToString(data, start).Replace("-", "") => Convert.ToHexString(data.AsSpan().Slice(start))
2 => generator.InvocationExpression(
buyaa-n marked this conversation as resolved.
Show resolved Hide resolved
methodExpression,
generator.InvocationExpression(generator.MemberAccessExpression(
generator.InvocationExpression(generator.MemberAccessExpression(
bitConverterArgumentsInParameterOrder[0].Value.Syntax,
nameof(MemoryExtensions.AsSpan))),
WellKnownMemberNames.SliceMethodName),
bitConverterArgumentsInParameterOrder[1].Value.Syntax))
.WithAddImportsAnnotation()
.WithAdditionalAnnotations(s_asSpanSymbolAnnotation),
// BitConverter.ToString(data, start, length).Replace("-", "") => Convert.ToHexString(data, start, length)
3 => generator.InvocationExpression(methodExpression, bitConverterArgumentsInParameterOrder.Select(a => a.Value.Syntax).ToArray()),
_ => throw new NotImplementedException()
};

// This branch is hit when string.ToLower* is used and Convert.ToHexStringLower is not available.
if (toLowerInvocation is not null)
{
methodInvocation = generator.InvocationExpression(
buyaa-n marked this conversation as resolved.
Show resolved Hide resolved
generator.MemberAccessExpression(methodInvocation, toLowerInvocation.TargetMethod.Name),
toLowerInvocation.Arguments.Select(a => a.Value.Syntax).ToArray());
}

editor.ReplaceNode(outerInvocation.Syntax, methodInvocation.WithTriviaFrom(outerInvocation.Syntax));

return context.Document.WithSyntaxRoot(editor.GetChangedRoot());
}
}
}
}