Skip to content

Add CA1881 analyzer: prefer MemoryExtensions.Split over counted string.Split - #1

Open
Mrnikbobjeff wants to merge 1 commit into
mainfrom
feature/loving-dirac-ljyoyk
Open

Mrnikbobjeff wants to merge 1 commit into
mainfrom
feature/loving-dirac-ljyoyk

Conversation

@Mrnikbobjeff

Copy link
Copy Markdown
Owner

Implements the analyzer from dotnet/runtime#85487, following what the 2023-06-01 API review approved.

What it does

CA1881 reports string.Split calls that pass a count and suggests the .NET 8 span-based MemoryExtensions.Split/SplitAny overloads with a Span<Range> destination. The destination's length stands in for the count, so the result is the same (the last range holds the remainder). Category Performance, severity Info, no code fix, as the review decided.

Reported Suggested
Split(char, int[, options]) Split(Span<Range>, char, options)
Split(string, int[, options]) Split(Span<Range>, ReadOnlySpan<char>, options)
Split(char[], int[, options]) SplitAny(Span<Range>, ReadOnlySpan<char>, options)
Split(string[], int, options) SplitAny(Span<Range>, ReadOnlySpan<string>, options)

The rule:

  • is C# only;
  • does nothing before .NET 8, when the span overloads are missing;
  • skips expression trees, async methods, lambdas and local functions, and iterators, where span locals can't be used.

Where the code lives

CA rules now live in dotnet/sdk (src/Microsoft.CodeAnalysis.NetAnalyzers), so this is written against that tree:

  • src/Microsoft.CodeAnalysis.NetAnalyzers/... contains the new analyzer and test files at their dotnet/sdk paths. They aren't part of this repo's build.
  • upstream/0001-Add-CA1881.patch is the complete dotnet/sdk change: resx, 13 xlf files, AnalyzerReleases.Unshipped.md, ID range, WellKnownTypeNames, and the generated md/sarif. It applies with git am on dotnet/sdk 3b1d59f.
  • docs/analyzers/CA1881/ has the design notes, review notes, verification details, open items, and a placeholder for the review video transcript. YouTube wasn't reachable from the build environment.

Testing

The real dotnet/sdk build couldn't run because the Arcade/dnceng feeds were blocked. I used a throwaway harness instead, built from upstream's own Analyzer.Utilities sources and CSharpCodeFixVerifier, against Roslyn 4.14.0 with warnings as errors and release-tracking analyzers on.

  • All 26 tests pass.
  • Breaking each guard on purpose makes the matching tests fail:
    • expression-tree/async/iterator guard removed: 4 tests fail;
    • iterator check alone removed: 1 fails;
    • API-availability check removed: 1 fails;
    • reporting disabled: 12 fail.
  • The README example, built as a .NET 8 app with the analyzer attached, reports CA1881 and produces the same parts as string.Split.
  • git am of the patch on a clean dotnet/sdk 3b1d59f gives a tree identical to the one tested.

Before upstreaming

  • The xlf, md and sarif entries were written by hand in the generator's format. Build once in dotnet/sdk and commit anything that changes.
  • Check that CA1881 is still free with NextDiagnosticId.cs Performance.
  • A ca1881.md page in dotnet/docs is required within a week of merge.
  • Run the rule over a large codebase and review the hits.
  • Fill in the video transcript (17:22 to 35:03).

🤖 Generated with Claude Code

https://claude.ai/code/session_01AdmaanBw8ruzpHQFgrisSa


Generated by Claude Code

…g.Split

Implements dotnet#85487 as approved in the 2023-06-01 API
review: report string.Split calls that pass a count, and suggest the
span-based MemoryExtensions.Split/SplitAny overloads (.NET 8) with a
Span<Range> destination. Category Performance, severity Info, no fixer.

CA rules now live in dotnet/sdk (src/Microsoft.CodeAnalysis.NetAnalyzers),
so the analyzer and its tests are added at their upstream paths, and
upstream/0001-Add-CA1881.patch carries the full change (resx, xlf,
release tracking, ID range, generated md/sarif) for `git am` on
dotnet/sdk 3b1d59f. docs/analyzers/CA1881 has the review notes, design,
verification details and open items; the video transcript is still
pending because YouTube was not reachable from the build environment.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AdmaanBw8ruzpHQFgrisSa
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants