Skip to content

CA2028: Avoid redundant Regex.IsMatch before Regex.Match#54071

Merged
danmoseley merged 23 commits into
dotnet:mainfrom
danmoseley:sdk-ca2027
May 18, 2026
Merged

CA2028: Avoid redundant Regex.IsMatch before Regex.Match#54071
danmoseley merged 23 commits into
dotnet:mainfrom
danmoseley:sdk-ca2027

Conversation

@danmoseley

@danmoseley danmoseley commented Apr 25, 2026

Copy link
Copy Markdown
Member

Implements CA2028: Avoid redundant Regex.IsMatch before Regex.Match.

Closes dotnet/runtime#111239
Supersedes #51214 (abandoned — Copilot was not converging at the time)

Pattern detected

// Before (local declaration)
if (Regex.IsMatch(input, pattern))
{
    Match m = Regex.Match(input, pattern);
    // use m
}

// Before (pre-declared variable)
Match m = null;
if (Regex.IsMatch(input, pattern))
{
    m = Regex.Match(input, pattern);
    // use m
}

// After (both cases)
if (Regex.Match(input, pattern) is { Success: true } m)
{
    // use m
}

What's included

  • Analyzer (IOperation-based, language-agnostic): registers on IConditionalOperation, validates operand equivalence (restricted to stable sources: locals, parameters, constants, readonly/const fields), tracks intervening writes including ref/out argument mutations, and reports diagnostic with additional location on the Match call.
  • C# fixer with two paths:
    • Local declaration: Match m = Regex.Match(...) inside the if body — replaces with is pattern.
    • Pre-declared variable assignment: Match m = null; if (...) { m = Regex.Match(...); ... } — removes the declaration and assignment, replaces with is pattern. Includes safety checks: variable must not be referenced after the if statement, initializer must be absent/null/default.
  • 82 tests covering: all static/instance overloads (including timeout), diagnostic+fix, diagnostic-only (no fix offered), no-diagnostic, VB smoke tests, real-world patterns found via GitHub code search (else-if chains, foreach loops, const fields), parenthesized arguments, pre-declared variable assignment patterns, edge cases (intervening writes, mutable receivers, property/method arguments, ref/out mutations, readonly-field receiver reassignment, span overloads with Net70 reference assemblies).

Addresses all feedback from #51214

All 15 review comments from @stephentoub on the abandoned PR are addressed:

  • Resource strings don't mention "pattern matching"
  • Inverted case (!IsMatch guard) intentionally not supported per reviewer guidance — adds too much complexity
  • Full intervening-write analysis prevents false positives from variable reassignment
  • Instance receiver equivalence validated with same rules as arguments
  • No VB fixer (VB analyzer works, fixer is EmptyCodeFixProvider)
  • No unnecessary wrapper methods; context.Diagnostics[0] not inlined because it's used 4x
  • Extensive test coverage (82 tests vs ~10 in original)

Design decisions

  • Rule ID CA2028 — CA2027 is already taken by DoNotUseNonCancelableTaskDelayWithWhenAny
  • Category: Reliability, severity: IdeSuggestion
  • Operand equivalence restricted to stable sources — properties, method calls, mutable fields are rejected
  • Does not recurse into lambdas, local functions, or nested blocks for Match call search
  • AddMutableSymbol recurses into IFieldReferenceOperation.Instance to handle obj.ReadonlyField receiver patterns
  • Span overloads (ReadOnlySpan) intentionally not flagged — IsMatch returns bool but there's no corresponding Match overload returning Match
  • Named arguments matched by Parameter.Ordinal (not array index) for correctness
  • WalkDownParentheses() applied consistently before WalkDownConversion() in all operand/symbol tracking paths

danmoseley and others added 6 commits April 25, 2026 02:22
Implements analyzer CA2028 that detects the pattern where Regex.IsMatch()
is used as a condition followed by Regex.Match() with the same arguments
in the if body, causing the regex engine to execute twice.

The C#-specific fixer transforms the pattern to use property pattern
matching: if (Regex.Match(...) is { Success: true } m) { ... }

Key features:
- IOperation-based analyzer (works for C# and VB)
- Semantic argument equivalence (locals, parameters, constants, readonly fields)
- Intervening write detection (bails if tracked symbols are modified)
- Instance method receiver verification
- Fixer gates on C# >= 8.0 (property patterns), first-statement guard,
  and name collision detection for else branch
- 46 comprehensive tests including real-world patterns from GitHub

Addresses dotnet/runtime#111239. Successor to abandoned PR dotnet#51214.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
ConstantPatternString_Flags, InstanceWithStartAtParameter_Flags, and
MultipleMatchCallsInBody_FlagsFirst all have fixable patterns (Match is
first statement, local declaration) but were only testing the analyzer.
Convert them to use VerifyCodeFixCSharp9Async with proper fixedSource.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add 3 real-world-inspired tests: else-if chain, loop, const field
- Change all 'diagnostic but no fix' C# tests to verify fixer
  produces no code changes (source -> source)
- All 49 tests pass

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Analyzer:
- Detect ref/out argument mutations of tracked symbols

Fixer:
- Reject fix when declared type is not var or exact Match type
- Broaden name-collision check to patterns, foreach, catch, out-var
- Check subsequent sibling statements for name conflicts

Tests:
- 8 new tests for ref/out, name conflicts, type declarations
- All 57 tests passing

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…source attributions

- Fix timeout overload test to use local variable (method calls aren't stable operands)
- Remove span overload test (framework lacks ReadOnlySpan<char> Regex APIs)
- Fix AddMutableSymbol to recurse into IFieldReferenceOperation.Instance (Opus B1)
- Add readonly-field receiver/argument reassignment tests
- Remove source attributions from test comments

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
ReadOnlySpan<char> IsMatch has no corresponding Match overload, so no
diagnostic should fire. Uses ReferenceAssemblies.Net.Net70 to make the
span APIs available in the test framework.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings April 25, 2026 09:56
@danmoseley danmoseley requested a review from a team as a code owner April 25, 2026 09:56

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds the CA2028 analyzer and C# code fix to detect and fix redundant Regex.IsMatch(...) guards immediately followed by Regex.Match(...) with equivalent operands, reducing duplicated regex evaluation work across C# and VB (analyzer) with a C#-only fixer.

Changes:

  • Introduces CA2028 analyzer (IOperation-based) to detect redundant IsMatchMatch patterns with operand equivalence + intervening-write checks.
  • Adds a C# code fix to rewrite the pattern into Regex.Match(...) is { Success: true } m when safe.
  • Adds extensive unit tests plus rule metadata/resource updates (resx/xlf), documentation, and SARIF entries.
Show a summary per file
File Description
src/Microsoft.CodeAnalysis.NetAnalyzers/tests/Microsoft.CodeAnalysis.NetAnalyzers.UnitTests/Microsoft.NetCore.Analyzers/Runtime/AvoidRedundantRegexIsMatchBeforeMatchTests.cs Adds CA2028 analyzer/fixer coverage across many positive/negative/edge scenarios (incl. VB analyzer smoke tests).
src/Microsoft.CodeAnalysis.NetAnalyzers/src/Microsoft.CodeAnalysis.NetAnalyzers/Microsoft.NetCore.Analyzers/Runtime/AvoidRedundantRegexIsMatchBeforeMatch.cs New CA2028 analyzer implementation for redundant Regex.IsMatch before Regex.Match.
src/Microsoft.CodeAnalysis.NetAnalyzers/src/Microsoft.CodeAnalysis.CSharp.NetAnalyzers/Microsoft.NetCore.Analyzers/Runtime/CSharpAvoidRedundantRegexIsMatchBeforeMatch.Fixer.cs New C# code fix provider for CA2028 using property-pattern matching.
src/Microsoft.CodeAnalysis.NetAnalyzers/src/Microsoft.CodeAnalysis.NetAnalyzers/Microsoft.NetCore.Analyzers/MicrosoftNetCoreAnalyzersResources.resx Adds CA2028 title/message/description/fix strings.
src/Microsoft.CodeAnalysis.NetAnalyzers/src/Microsoft.CodeAnalysis.NetAnalyzers/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.zh-Hant.xlf Adds new CA2028 localized resource entries (state=new).
src/Microsoft.CodeAnalysis.NetAnalyzers/src/Microsoft.CodeAnalysis.NetAnalyzers/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.zh-Hans.xlf Adds new CA2028 localized resource entries (state=new).
src/Microsoft.CodeAnalysis.NetAnalyzers/src/Microsoft.CodeAnalysis.NetAnalyzers/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.tr.xlf Adds new CA2028 localized resource entries (state=new).
src/Microsoft.CodeAnalysis.NetAnalyzers/src/Microsoft.CodeAnalysis.NetAnalyzers/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.ru.xlf Adds new CA2028 localized resource entries (state=new).
src/Microsoft.CodeAnalysis.NetAnalyzers/src/Microsoft.CodeAnalysis.NetAnalyzers/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.pt-BR.xlf Adds new CA2028 localized resource entries (state=new).
src/Microsoft.CodeAnalysis.NetAnalyzers/src/Microsoft.CodeAnalysis.NetAnalyzers/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.pl.xlf Adds new CA2028 localized resource entries (state=new).
src/Microsoft.CodeAnalysis.NetAnalyzers/src/Microsoft.CodeAnalysis.NetAnalyzers/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.ko.xlf Adds new CA2028 localized resource entries (state=new).
src/Microsoft.CodeAnalysis.NetAnalyzers/src/Microsoft.CodeAnalysis.NetAnalyzers/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.ja.xlf Adds new CA2028 localized resource entries (state=new).
src/Microsoft.CodeAnalysis.NetAnalyzers/src/Microsoft.CodeAnalysis.NetAnalyzers/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.it.xlf Adds new CA2028 localized resource entries (state=new).
src/Microsoft.CodeAnalysis.NetAnalyzers/src/Microsoft.CodeAnalysis.NetAnalyzers/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.fr.xlf Adds new CA2028 localized resource entries (state=new).
src/Microsoft.CodeAnalysis.NetAnalyzers/src/Microsoft.CodeAnalysis.NetAnalyzers/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.es.xlf Adds new CA2028 localized resource entries (state=new).
src/Microsoft.CodeAnalysis.NetAnalyzers/src/Microsoft.CodeAnalysis.NetAnalyzers/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.de.xlf Adds new CA2028 localized resource entries (state=new).
src/Microsoft.CodeAnalysis.NetAnalyzers/src/Microsoft.CodeAnalysis.NetAnalyzers/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.cs.xlf Adds new CA2028 localized resource entries (state=new).
src/Microsoft.CodeAnalysis.NetAnalyzers/src/Microsoft.CodeAnalysis.NetAnalyzers/AnalyzerReleases.Unshipped.md Registers CA2028 in the unshipped analyzer release list.
src/Microsoft.CodeAnalysis.NetAnalyzers/src/Microsoft.CodeAnalysis.NetAnalyzers.sarif Adds CA2028 rule metadata for SARIF.
src/Microsoft.CodeAnalysis.NetAnalyzers/src/Microsoft.CodeAnalysis.NetAnalyzers.md Documents CA2028 in the analyzer rules list.

Copilot's findings

  • Files reviewed: 20/20 changed files
  • Comments generated: 4

@danmoseley danmoseley requested a review from stephentoub April 25, 2026 10:02
… params, fix diagnostic ID range

- Remove unused root variable and matchCallNode parameter in ApplyFixAsync
- Add WalkDownParentheses() before WalkDownConversion() in GetUnwrappedInvocation
- Fix trailing trivia on parenthesized conditions causing extra whitespace
- Rewrite HasConflictingNameInSubsequentSiblings to walk up else-if chains
- Add ForEachVariableStatementSyntax handling for deconstruction foreach
- Update DiagnosticCategoryAndIdRanges.txt to include CA2028
- Add 3 new tests: parenthesized condition, deconstruction foreach, non-block parent

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot's findings

Comments suppressed due to low confidence (4)

src/Microsoft.CodeAnalysis.NetAnalyzers/src/Microsoft.CodeAnalysis.CSharp.NetAnalyzers/Microsoft.NetCore.Analyzers/Runtime/CSharpAvoidRedundantRegexIsMatchBeforeMatch.Fixer.cs:18

  • using Microsoft.CodeAnalysis.Operations; is not used in this file. Please remove it to avoid unused using warnings.
using Microsoft.CodeAnalysis.Operations;
using Microsoft.NetCore.Analyzers;

src/Microsoft.CodeAnalysis.NetAnalyzers/src/Microsoft.CodeAnalysis.CSharp.NetAnalyzers/Microsoft.NetCore.Analyzers/Runtime/CSharpAvoidRedundantRegexIsMatchBeforeMatch.Fixer.cs:199

  • The fix currently strips trailing trivia from the original if condition (WithTrailingTrivia(TriviaList())). This can drop end-of-condition comments/formatting. Preserve the original condition trivia (or use WithTriviaFrom) so the code fix doesn't delete user comments.
            var newCondition = SyntaxFactory.IsPatternExpression(
                matchCallExpression.WithoutTrivia(),
                successPattern)
                .WithLeadingTrivia(ifStatement.Condition.GetLeadingTrivia())
                .WithTrailingTrivia(SyntaxFactory.TriviaList());

src/Microsoft.CodeAnalysis.NetAnalyzers/src/Microsoft.CodeAnalysis.CSharp.NetAnalyzers/Microsoft.NetCore.Analyzers/Runtime/CSharpAvoidRedundantRegexIsMatchBeforeMatch.Fixer.cs:196

  • matchCallExpression.WithoutTrivia() will drop any leading/trailing trivia (including comments) attached to the Regex.Match(...) initializer when it’s moved into the if condition. Please preserve trivia from the original initializer where possible.
            var newCondition = SyntaxFactory.IsPatternExpression(
                matchCallExpression.WithoutTrivia(),
                successPattern)

src/Microsoft.CodeAnalysis.NetAnalyzers/src/Microsoft.CodeAnalysis.CSharp.NetAnalyzers/Microsoft.NetCore.Analyzers/Runtime/CSharpAvoidRedundantRegexIsMatchBeforeMatch.Fixer.cs:205

  • editor.RemoveNode(matchDeclarationStatement) uses the default remove options, which can drop leading/trailing trivia (e.g., comments) attached to the declaration statement. Consider using remove options that preserve trivia so the fixer doesn’t delete comments inside the if body.
            // Remove the Match declaration statement from the if body
            editor.RemoveNode(matchDeclarationStatement);

  • Files reviewed: 21/21 changed files
  • Comments generated: 1

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot's findings

  • Files reviewed: 21/21 changed files
  • Comments generated: 3

…gion rename

- Fixer: verify initializer expression matches the Match invocation span
  before offering code fix (unwrapping parens/casts)
- Analyzer: handle ICoalesceAssignmentOperation (??=) in GetWrittenSymbol
- Analyzer: handle IDeconstructionAssignmentOperation in ContainsWriteToSymbols
  with recursive ContainsTrackedSymbolReference helper
- Tests: add InterveningCoalesceAssignment and InterveningDeconstructionAssignment
- Tests: rename model-specific region to content-based name

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot's findings

  • Files reviewed: 21/21 changed files
  • Comments generated: 1

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot's findings

  • Files reviewed: 21/21 changed files
  • Comments generated: 3

…based type check, preserve leading trivia

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

@krwq krwq left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Couple of comments about potential false positives from AI to verify

- Skip ternary `?:` (and VB `If(...)`) — only report on if-statements
  by bailing when conditional.Type is not null.
- Extend FindMatchInExpression to recognize Match calls inside
  object/array creation, tuples, interpolated strings, coalesce,
  binary, unary, and await expressions.
- Restrict IInstanceReferenceOperation equivalence to reference types,
  since `this` can be reassigned inside struct instance methods.
- Add WalkDownParentheses() to ref/out arg unwrap in
  ContainsWriteToSymbols for consistency with other unwrap sites.
- In the C# fixer, extract a ContainsIdentifierReference helper that
  excludes member-access right-hand sides, qualified-name suffixes,
  and named-argument labels, so unrelated `something.m = 1` or
  `Helper(m: 1)` after the if no longer suppress the fix.
- Use Formatter.Annotation on the synthesized IsPatternExpression in
  BuildIsPatternCondition so the host's formatter handles spacing.
- Add 13 regression tests covering each change.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot's findings

  • Files reviewed: 21/21 changed files
  • Comments generated: 2

- FindMatchInExpression now walks IObjectOrCollectionInitializerOperation
  (member assignments inside `new Foo { Bar = Regex.Match(...) }`) and
  ISimpleAssignmentOperation (the member-assignment shape).
- ContainsWriteToSymbols now short-circuits at IAnonymousFunctionOperation
  and ILocalFunctionOperation, so writes inside a local function or lambda
  body are no longer treated as intervening writes when the function itself
  is the statement being scanned.
- Add regression tests for both.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot's findings

  • Files reviewed: 21/21 changed files
  • Comments generated: 2

- C# fixer now accepts DefaultExpressionSyntax (e.g. `default(Match)`)
  as a removable initializer in addition to the `null` and `default`
  literal forms.
- Remove the unreachable second ISimpleAssignmentOperation branch in
  FindMatchInExpression (the earlier branch handles every case).
- Add a regression test for the default(T) initializer case.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@danmoseley danmoseley requested a review from Copilot May 12, 2026 18:05

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot's findings

  • Files reviewed: 21/21 changed files
  • Comments generated: 1

…t scan

Pattern variables introduced by 'is { Success: true } m' scope to the entire
enclosing block. The fixer's HasConflictingName check now also rejects names
that are already bound by a subsequent LINQ query clause (from/let/join/
join-into/into) anywhere in the parent block, preventing the fixer from
introducing CS0136-style name collisions.

Added a regression test that confirms no fix is offered when the parent
block has a later 'from m in items' query and the fixer would otherwise
name its pattern variable 'm'.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot's findings

  • Files reviewed: 21/21 changed files
  • Comments generated: 0 new

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@danmoseley danmoseley enabled auto-merge (squash) May 12, 2026 22:27
@danmoseley

Copy link
Copy Markdown
Member Author

The failures are unrelated.


struct S
{
public Regex r;

@krwq krwq May 15, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Per AI:

The diagnostic doesn't fire — but not because of the new IInstanceReferenceOperation restriction. It doesn't fire because the existing readonly-field
equivalence check already rejects non-readonly fields. To actually exercise the new behavior, the field must be public readonly Regex r;. As written, this test would pass even if the new restriction were reverted.

@krwq krwq left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM but one test case is probably always passing and need extra readonly on the field. Note that this is only partially reviewed manually, most of the feedback is from AI.

@danmoseley

Copy link
Copy Markdown
Member Author

/ba-g unrelated

@danmoseley danmoseley merged commit 17682c1 into dotnet:main May 18, 2026
21 of 24 checks passed
@danmoseley danmoseley deleted the sdk-ca2027 branch May 18, 2026 17:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Analyzer]: Regex.IsMatch guarding Regex.Match

5 participants