Skip to content

Do not report MA0028 on StringBuilder.Insert with a Substring argument - #1429

Merged
meziantou merged 1 commit into
mainfrom
feature/ma0028-stringbuilder-insert-898ce6
Sep 7, 2026
Merged

meziantou merged 1 commit into
mainfrom
feature/ma0028-stringbuilder-insert-898ce6

Conversation

@meziantou

Copy link
Copy Markdown
Owner

What

OptimizeStringBuilderUsageAnalyzer reported MA0028 on StringBuilder.Insert when the inserted string came from Substring:

new StringBuilder().Insert(0, s.Substring(1));

Use Insert(string, int, int) or Insert(ReadOnlySpan<char>) instead of Substring

StringBuilder.Insert(string, int, int) does not exist. The closest real overload is Insert(int index, string value, int count), which repeats the value rather than slicing it — so there is no Insert counterpart to suggest here at all.

Why

The Substring branch of IsOptimizable was the only one of the four IInvocationOperation branches missing the methodName != "Insert" guard. The three siblings — ToString, string.Format and string.Join — all begin with it, and the comment above them explains exactly why Insert has to be excluded: those overloads are Append's, and Insert has no counterpart for some of them.

Change

One line, matching the neighbouring branches:

else if (methodName != "Insert" && string.Equals(targetMethod.Name, nameof(string.Substring), ...))

Plus two Insert_NoDiagnostic theory cases ("abc".Substring(2) and "abc".Substring(0, 1)) with a comment recording why Insert is excluded.

Notes for the reviewer

The code fixer was not throwing on these. Its pattern Arguments: [{ Value: IInvocationOperation ... }, ..] fails to match, because argument 0 of Insert is the int index, so it returned before calling RegisterCodeFix and no lightbulb was ever offered. The user-visible defect was the diagnostic message naming a non-existent API, so the fixer needed no change and is untouched.

docs/Rules/MA0028.md never described the Substring/Insert behaviour, so there was nothing to update there.

Verification

  • Analyzer builds clean, 0 warnings.
  • Reverted the analyzer fix while keeping the new tests: both failed with exactly the reported message, confirming the tests are a real regression guard rather than vacuous. Restored the fix and they pass.
  • MA0028 tests pass on all five Roslyn versions (4.8, 4.14, 5.0, 5.6, 5.9) — 107/107 each, up from 105 with the two added cases.
  • dotnet run --project src/DocumentationGenerator exits 0 with no markdown changes.

The Substring branch of IsOptimizable was the only one of the four
IInvocationOperation branches missing the `methodName != "Insert"` guard
that ToString, string.Format and string.Join all have.

As a result, `new StringBuilder().Insert(0, s.Substring(1))` reported
"Use Insert(string, int, int) or Insert(ReadOnlySpan<char>) instead of
Substring". StringBuilder.Insert(string, int, int) does not exist, and
the closest real overload, Insert(int index, string value, int count),
repeats the value instead of slicing it, so there is no Insert
counterpart to suggest here.

The code fixer already declined to fix these: it reads argument 0 as an
invocation, but argument 0 of Insert is the int index, so it returned
before registering the fix. The user-visible defect was the diagnostic
itself naming an API that does not exist.
@meziantou
meziantou merged commit 9a62783 into main Sep 7, 2026
13 checks passed
@meziantou
meziantou deleted the feature/ma0028-stringbuilder-insert-898ce6 branch September 7, 2026 00:07
This was referenced Sep 7, 2026
This was referenced Sep 24, 2026
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.

1 participant