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 analyzer/fixer to suggest using cached SearchValues instances#6898

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 27 commits intodotnet:mainfromMihaZupan:searchvalues
Sep 8, 2023

Conversation

@MihaZupan
Copy link
Member

@MihaZupanMihaZupan commentedAug 29, 2023
edited
Loading

Fixesdotnet/runtime#78587

Flags 3 cases in dotnet/runtime, all of them in projects where we've avoided making the switch because they multi-target to older TFMs (SearchValues is a .NET 8+ API).

The analyzer flags cases like

conststringMyValues="abcdef";readonlychar[]MyValues=new[]{'a','b','c','d','e','f'};span.IndexOfAny("abcdef");span.IndexOfAny("abcdef"u8);span.IndexOfAny(MyValues);span.IndexOfAny(new[]{'a','b','c','d','e','f'});string.IndexOfAny("abc".ToCharArray());

for all{Last}IndexOfAny{Except} andContainsAny{Except} overloads forbyte/char and suggests that they be rewritten as

privatestaticreadonlySearchValues<char>s_myChars=SearchValues.Create("abcdef");span.IndexOfAny(s_myChars);

We'll also rewrite thestring.IndexOfAny(char[]) overload to usestring.AsSpan().IndexOfAny(SearchValues) instead.
We won't do the same forIndexOfAny(char[], int) orIndexOfAny(char[], int, int) as they would require us to inject more logic to account for the differences in behavior (returning offset from start vs. from 0)

If the values are specified by a constant in an appropriate scope, we'll reuse said constant to feed theSearchValues.Create.

If we're not using the original member and there are now no other uses, we'll remove it.

For a full list of patterns that we do/don't recognize, seethis test source.


Potential improvements:

  • IDE0300 will suggest replacingnew[] { 'a', 'b', 'c' } with['a', 'b', 'c'], whereas this analyzer does not recognize collection literals.

@MihaZupanMihaZupan requested a review froma team as acode ownerAugust 29, 2023 23:16
@codecov
Copy link

codecovbot commentedAug 29, 2023
edited
Loading

Codecov Report

Merging#6898 (1fe8dd8) intomain (2d42aa2) willdecrease coverage by0.01%.
Report is 5 commits behind head on main.
The diff coverage is95.53%.

@@            Coverage Diff             @@##             main    #6898      +/-   ##==========================================- Coverage   96.40%   96.40%   -0.01%==========================================  Files        1403     1402       -1       Lines      331075   331964     +889       Branches    10893    11036     +143     ==========================================+ Hits       319166   320015     +849- Misses       9178     9186       +8- Partials     2731     2763      +32

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 for the great test coverage, in general LGTM, I would let@mavasani take a look to the new utilities and some of approaches used in the analyzer/fixer.

MihaZupan reacted with hooray emoji
@buyaa-n
Copy link

FYI@mavasani we want to add this new analyzer into .NET 8 asThis is about a new .NET 8 feature, and it'd be very nice to have a better end-to-end story for the release by having the analyzer/fixer shipping together with the library support it targets. It would be great if you could take a look

@mavasani
Copy link

Overall, this looks good to me. I primarily looked at the analyzer and test code, just skimmed over the code fixer code.

buyaa-n reacted with thumbs up emojiMihaZupan reacted with hooray emoji

@MihaZupan
Copy link
MemberAuthor

/backport to release/8.0.1xx

@github-actions
Copy link

Started backporting to release/8.0.1xx:https://github.com/dotnet/roslyn-analyzers/actions/runs/6124792900

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@stephentoubstephentoubAwaiting requested review from stephentoub

3 more reviewers

@mavasanimavasanimavasani left review comments

@Youssef1313Youssef1313Youssef1313 left review comments

@buyaa-nbuyaa-nbuyaa-n approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Analyzer] Use IndexOfAnyValues instead of inlined or cached array

5 participants

@MihaZupan@buyaa-n@mavasani@stephentoub@Youssef1313

[8]ページ先頭

©2009-2025 Movatter.jp