Skip to content

Commit

Permalink
Add CA1872: Prefer 'Convert.ToHexString' over 'BitConverter.ToString' (
Browse files Browse the repository at this point in the history
…#6967)

* Add CA1872: Prefer 'Convert.ToHexString' over 'BitConverter.ToString'

This analyzer detects the usage of the pattern
`BitConverter.ToString(bytes).Replace("-", "")` to convert an array of
bytes to an uppercase hex string (without hyphens in between) and
replaces it with a call to `Convert.ToHexString(bytes)`.
The analyzer will also try to preserve chaining `ToLower*` in between
for a lowercase hex string.

* Use span overload when replacing call with two arguments

* Use Convert.ToHexStringLower if available

* Improve resource strings

* Improve description resource string

* Remove redundant helper methods

* Change invocation analyze order and remove duplicate work in fixer

* Remove temporary Net90 reference assembly
  • Loading branch information
mpidash authored Mar 19, 2024
1 parent de3a920 commit 0c86a45
Show file tree
Hide file tree
Showing 23 changed files with 2,272 additions and 1 deletion.
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(
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(
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());
}
}
}
}
Loading

0 comments on commit 0c86a45

Please sign in to comment.