Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

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

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
buyaa-n merged 13 commits intodotnet:mainfrommpidash:issue-81796
Mar 19, 2024

Conversation

@mpidash
Copy link

Fixesdotnet/runtime#81796.

This analyzer detects the usage of the patternBitConverter.ToString(bytes).Replace("-", "") to convert an array of bytes to an uppercase hex string (without hyphens in between) and replaces it with a call toConvert.ToHexString(bytes).
The analyzer will also try to preserve chainingToLower* in between (before and afterstring.Replace) for a lowercase hex string.

I have found 2 warnings indotnet/runtime, 1 warning indotnet/roslyn and no warnings indotnet/aspnetcore anddotnet/roslyn-analyzers:

  1. runtime\src\coreclr\tools\aot\ILCompiler.Compiler\Compiler\XmlObjectDumper.cs(73,20): info CA1872: Prefer 'System.Convert.ToHexString(byte[])' over 'System.BitConverter.ToString(byte[])'
  2. runtime\src\coreclr\tools\Common\Compiler\NativeAotNameMangler.cs(125,38): warning CA1872: Prefer 'System.Convert.ToHexString(byte[])' over 'System.BitConverter.ToString(byte[])'
  3. roslyn\src\Tools\BuildValidator\CompilationDiff.cs(404,46): info CA1872: Prefer 'System.Convert.ToHexString(byte[])' over 'System.BitConverter.ToString(byte[])

This analyzer detects the usage of the pattern`BitConverter.ToString(bytes).Replace("-", "")` to convert an array ofbytes to an uppercase hex string (without hyphens in between) andreplaces it with a call to `Convert.ToHexString(bytes)`.The analyzer will also try to preserve chaining `ToLower*` in betweenfor a lowercase hex string.
@mpidashmpidash requested a review froma team as acode ownerSeptember 30, 2023 13:56
@codecov
Copy link

codecovbot commentedSep 30, 2023•
edited
Loading

Codecov Report

Attention: Patch coverage is98.25386% with26 lines in your changes are missing coverage. Please review.

Project coverage is 96.47%. Comparing base(de3a920) to head(f3fee4a).

Additional details and impacted files
@@            Coverage Diff             @@##             main    #6967      +/-   ##==========================================- Coverage   96.47%   96.47%   -0.01%==========================================  Files        1436     1439       +3       Lines      342881   344370    +1489       Branches    11292    11324      +32     ==========================================+ Hits       330797   332230    +1433- Misses       9230     9272      +42- Partials     2854     2868      +14

@mpidash
Copy link
Author

@buyaa-n I've addressed your feedback and also tried to improve the messages, PTAL 😸.

Here are the findings with the new message template:

W:\roslyn\src\Tools\BuildValidator\CompilationDiff.cs(404,46): info CA1872: Prefer 'System.Convert.ToHexString(byte[])' over call chains based on 'System.BitConverter.ToString(byte[])'W:\runtime\src\coreclr\tools\Common\Compiler\NativeAotNameMangler.cs(111,38): warning CA1872: Prefer 'System.Convert.ToHexString(byte[])' over call chains based on 'System.BitConverter.ToString(byte[])' (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1872) [W:\runtime\src\coreclr\tools\aot\ILCompiler.Compiler\ILCompiler.Compiler.csproj]W:\runtime\src\coreclr\tools\aot\ILCompiler.Compiler\Compiler\XmlObjectDumper.cs(73,20): warning CA1872: Prefer 'System.Convert.ToHexStringLower(byte[])' over call chains based on 'System.BitConverter.ToString(byte[])' (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1872) [W:\runtime\src\coreclr\tools\aot\ILCompiler.Compiler\ILCompiler.Compiler.csproj]W:\runtime\src\libraries\System.Private.Xml\tests\XmlNodeReader\System.Xml.XmlNodeReader.Tests\XmlNodeReaderReadTests.cs(183,41): warning CA1872: Prefer 'System.Convert.ToHexString(byte[])' over call chains based on 'System.BitConverter.ToString(byte[])' (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1872) [W:\runtime\src\libraries\System.Private.Xml\tests\System.Private.Xml.Tests.csproj]W:\runtime\src\libraries\System.Private.Xml\tests\XmlNodeReader\System.Xml.XmlNodeReader.Tests\XmlNodeReaderReadTests.cs(233,46): warning CA1872: Prefer 'System.Convert.ToHexString(byte[])' over call chains based on 'System.BitConverter.ToString(byte[])' (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1872) [W:\runtime\src\libraries\System.Private.Xml\tests\System.Private.Xml.Tests.csproj]

And an example of an applied code fix:

image
image
image

@gewarren: It would be great if you could also take a look at the resource strings, thanks!

buyaa-n reacted with thumbs up emoji

@mpidashmpidash requested a review frombuyaa-nMarch 8, 2024 22:57
Copy link

@gewarrengewarren left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

I left some feedback on the resource strings. Thanks for tagging me!

@buyaa-n
Copy link

@buyaa-n I've addressed your feedback and also tried to improve the messages, PTAL 😸.

Thank you@mpidash, the messages and test coverage look great to me, functional testing worked well too, this time I looked into the code more and left few more feedback.

@mpidash
Copy link
Author

Thanks for the review@buyaa-n! I've addressed your feedback, PTAL.

Copy link

@buyaa-nbuyaa-n left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Thank you@mpidash, LGTM.

It's good to go after the#6967 (comment) done

mpidash reacted with hooray emoji
@buyaa-nbuyaa-n merged commit0c86a45 intodotnet:mainMar 19, 2024
@mpidashmpidash deleted the issue-81796 branchMarch 19, 2024 20:47
arunchndr pushed a commit that referenced this pull requestMar 22, 2024
…#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 ofbytes to an uppercase hex string (without hyphens in between) andreplaces it with a call to `Convert.ToHexString(bytes)`.The analyzer will also try to preserve chaining `ToLower*` in betweenfor 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
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@carlossanlopcarlossanlopAwaiting requested review from carlossanlop

2 more reviewers

@gewarrengewarrengewarren left review comments

@buyaa-nbuyaa-nbuyaa-n approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Analyzer] BitConverter.ToString(bytes).Replace("-", "") to Convert.ToHexString

3 participants

@mpidash@buyaa-n@gewarren
[8]ページ先頭
©2009-2025 Movatter.jp