Skip to content

Commit ef6de95

Browse files
Copilotstephentoub
Copilot
and
stephentoub
committed
Simplify analyzer and fixer: remove inverted pattern support, apply code review feedback
Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
1 parent 18a11bb commit ef6de95

File tree

3 files changed

+46
-245
lines changed

3 files changed

+46
-245
lines changed

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

Lines changed: 40 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -29,19 +29,12 @@ public sealed class CSharpAvoidRedundantRegexIsMatchBeforeMatchFixer : CodeFixPr
2929

3030
public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context)
3131
{
32-
var root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false);
33-
if (root is null)
32+
if (await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false) is not { } root ||
33+
await context.Document.GetSemanticModelAsync(context.CancellationToken).ConfigureAwait(false) is not { } semanticModel)
3434
{
3535
return;
3636
}
3737

38-
var semanticModel = await context.Document.GetSemanticModelAsync(context.CancellationToken).ConfigureAwait(false);
39-
if (semanticModel is null)
40-
{
41-
return;
42-
}
43-
44-
var diagnostic = context.Diagnostics[0];
4538
var node = root.FindNode(context.Span, getInnermostNodeForTie: true);
4639

4740
if (node is not InvocationExpressionSyntax isMatchInvocation)
@@ -61,7 +54,7 @@ public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context)
6154
NetCore.Analyzers.MicrosoftNetCoreAnalyzersResources.AvoidRedundantRegexIsMatchBeforeMatchFix,
6255
ct => RemoveRedundantIsMatchAsync(context.Document, root, ifStatement, isMatchInvocation, semanticModel, ct),
6356
equivalenceKey: NetCore.Analyzers.MicrosoftNetCoreAnalyzersResources.AvoidRedundantRegexIsMatchBeforeMatchFix),
64-
diagnostic);
57+
context.Diagnostics[0]);
6558
}
6659

6760
private static async Task<Document> RemoveRedundantIsMatchAsync(
@@ -74,77 +67,55 @@ private static async Task<Document> RemoveRedundantIsMatchAsync(
7467
{
7568
var editor = await DocumentEditor.CreateAsync(document, cancellationToken).ConfigureAwait(false);
7669

77-
// Determine if this is a negated pattern (if (!IsMatch) { return; })
78-
bool isNegated = ifStatement.Condition is PrefixUnaryExpressionSyntax { RawKind: (int)SyntaxKind.LogicalNotExpression };
79-
80-
if (isNegated && IsEarlyReturnPattern(ifStatement))
70+
// Find the Match call in the body and use it to replace the condition
71+
var matchCall = FindMatchCallInBlock(ifStatement.Statement, isMatchInvocation, semanticModel);
72+
73+
if (matchCall is not null)
8174
{
82-
// For inverted early return pattern: remove the entire if statement
83-
editor.RemoveNode(ifStatement);
84-
}
85-
else
86-
{
87-
// For normal pattern: transform the if statement
88-
// Find the Match call in the body and use it to replace the condition
89-
var matchCall = FindMatchCallInBlock(ifStatement.Statement, isMatchInvocation, semanticModel);
75+
// Create the new condition: Regex.Match(...) is { Success: true } variableName
76+
var matchDeclaration = matchCall.Parent?.Parent as LocalDeclarationStatementSyntax;
9077

91-
if (matchCall is not null)
78+
if (matchDeclaration is not null)
9279
{
93-
// Create the new condition: Regex.Match(...) is { Success: true } variableName
94-
var matchDeclaration = matchCall.Parent?.Parent as LocalDeclarationStatementSyntax;
95-
96-
if (matchDeclaration is not null)
80+
var variableDeclarator = matchDeclaration.Declaration.Variables.First();
81+
var variableName = variableDeclarator.Identifier.Text;
82+
83+
// Create pattern: is { Success: true }
84+
var successPattern = SyntaxFactory.RecursivePattern()
85+
.WithPropertyPatternClause(
86+
SyntaxFactory.PropertyPatternClause(
87+
SyntaxFactory.SingletonSeparatedList<SubpatternSyntax>(
88+
SyntaxFactory.Subpattern(
89+
SyntaxFactory.NameColon("Success"),
90+
SyntaxFactory.ConstantPattern(
91+
SyntaxFactory.LiteralExpression(SyntaxKind.TrueLiteralExpression))))))
92+
.WithDesignation(SyntaxFactory.SingleVariableDesignation(SyntaxFactory.Identifier(variableName)));
93+
94+
var newCondition = SyntaxFactory.IsPatternExpression(
95+
matchCall,
96+
successPattern);
97+
98+
// Remove the Match declaration from the body
99+
var newBody = ifStatement.Statement;
100+
if (ifStatement.Statement is BlockSyntax block)
97101
{
98-
var variableDeclarator = matchDeclaration.Declaration.Variables.First();
99-
var variableName = variableDeclarator.Identifier.Text;
100-
101-
// Create pattern: is { Success: true }
102-
var successPattern = SyntaxFactory.RecursivePattern()
103-
.WithPropertyPatternClause(
104-
SyntaxFactory.PropertyPatternClause(
105-
SyntaxFactory.SingletonSeparatedList<SubpatternSyntax>(
106-
SyntaxFactory.Subpattern(
107-
SyntaxFactory.NameColon("Success"),
108-
SyntaxFactory.ConstantPattern(
109-
SyntaxFactory.LiteralExpression(SyntaxKind.TrueLiteralExpression))))))
110-
.WithDesignation(SyntaxFactory.SingleVariableDesignation(SyntaxFactory.Identifier(variableName)));
111-
112-
var newCondition = SyntaxFactory.IsPatternExpression(
113-
matchCall,
114-
successPattern);
115-
116-
// Remove the Match declaration from the body
117-
var newBody = ifStatement.Statement;
118-
if (ifStatement.Statement is BlockSyntax block)
119-
{
120-
var statements = block.Statements.Where(s => s != matchDeclaration);
121-
newBody = block.WithStatements(SyntaxFactory.List(statements));
122-
}
102+
var statements = block.Statements.Where(s => s != matchDeclaration);
103+
newBody = block.WithStatements(SyntaxFactory.List(statements));
104+
}
123105

124-
// Create new if statement
125-
var newIfStatement = ifStatement
126-
.WithCondition(newCondition.WithTriviaFrom(ifStatement.Condition))
127-
.WithStatement(newBody)
128-
.WithAdditionalAnnotations(Formatter.Annotation);
106+
// Create new if statement
107+
var newIfStatement = ifStatement
108+
.WithCondition(newCondition.WithTriviaFrom(ifStatement.Condition))
109+
.WithStatement(newBody)
110+
.WithAdditionalAnnotations(Formatter.Annotation);
129111

130-
editor.ReplaceNode(ifStatement, newIfStatement);
131-
}
112+
editor.ReplaceNode(ifStatement, newIfStatement);
132113
}
133114
}
134115

135116
return editor.GetChangedDocument();
136117
}
137118

138-
private static bool IsEarlyReturnPattern(IfStatementSyntax ifStatement)
139-
{
140-
if (ifStatement.Statement is BlockSyntax block)
141-
{
142-
return block.Statements.Any(s => s is ReturnStatementSyntax or ThrowStatementSyntax or BreakStatementSyntax or ContinueStatementSyntax);
143-
}
144-
145-
return ifStatement.Statement is ReturnStatementSyntax or ThrowStatementSyntax or BreakStatementSyntax or ContinueStatementSyntax;
146-
}
147-
148119
private static InvocationExpressionSyntax? FindMatchCallInBlock(StatementSyntax statement, InvocationExpressionSyntax isMatchInvocation, SemanticModel semanticModel)
149120
{
150121
if (statement is not BlockSyntax block)

‎src/Microsoft.CodeAnalysis.NetAnalyzers/src/Microsoft.CodeAnalysis.NetAnalyzers/Microsoft.NetCore.Analyzers/Runtime/AvoidRedundantRegexIsMatchBeforeMatch.cs‎

Lines changed: 6 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -55,48 +55,26 @@ public override void Initialize(AnalysisContext context)
5555
{
5656
var conditional = (IConditionalOperation)context.Operation;
5757

58-
// Check if condition is Regex.IsMatch call (direct or negated)
59-
if (!IsRegexIsMatchCall(conditional.Condition, regexIsMatchSymbols, out var isMatchCall, out bool isNegated))
58+
// Check if condition is Regex.IsMatch call (not negated)
59+
if (!IsRegexIsMatchCall(conditional.Condition, regexIsMatchSymbols, out var isMatchCall))
6060
{
6161
return;
6262
}
6363

64-
// For normal IsMatch, look in when-true branch for corresponding Match call
65-
if (!isNegated)
64+
// Look in when-true branch for corresponding Match call
65+
if (FindMatchCallInBranch(conditional.WhenTrue, regexMatchSymbols, isMatchCall, context.Operation, out var matchCall))
6666
{
67-
if (FindMatchCallInBranch(conditional.WhenTrue, regexMatchSymbols, isMatchCall, context.Operation, out var matchCall))
68-
{
69-
context.ReportDiagnostic(isMatchCall.CreateDiagnostic(Rule));
70-
return;
71-
}
72-
}
73-
// For negated IsMatch with early return pattern, check subsequent operations
74-
else if (IsEarlyReturnPattern(conditional))
75-
{
76-
// Look for Match calls after the conditional in the parent block
77-
if (FindMatchCallAfterConditional(conditional, regexMatchSymbols, isMatchCall, context.Operation, out var subsequentMatchCall))
78-
{
79-
context.ReportDiagnostic(isMatchCall.CreateDiagnostic(Rule));
80-
return;
81-
}
67+
context.ReportDiagnostic(isMatchCall.CreateDiagnostic(Rule));
8268
}
8369
}, OperationKind.Conditional);
8470
});
8571
}
8672

87-
private static bool IsRegexIsMatchCall(IOperation condition, ImmutableArray<IMethodSymbol> regexIsMatchSymbols, out IInvocationOperation isMatchCall, out bool isNegated)
73+
private static bool IsRegexIsMatchCall(IOperation condition, ImmutableArray<IMethodSymbol> regexIsMatchSymbols, out IInvocationOperation isMatchCall)
8874
{
8975
// Handle unwrapping of conversions and parenthesized expressions
9076
var unwrapped = condition.WalkDownConversion();
9177

92-
// Check for negation
93-
isNegated = false;
94-
if (unwrapped is IUnaryOperation { OperatorKind: UnaryOperatorKind.Not } unaryOp)
95-
{
96-
isNegated = true;
97-
unwrapped = unaryOp.Operand.WalkDownConversion();
98-
}
99-
10078
if (unwrapped is IInvocationOperation invocation &&
10179
regexIsMatchSymbols.Contains(invocation.TargetMethod, SymbolEqualityComparer.Default))
10280
{
@@ -139,62 +117,6 @@ private static bool FindMatchCallRecursive(IOperation operation, ImmutableArray<
139117
return false;
140118
}
141119

142-
private static bool IsEarlyReturnPattern(IConditionalOperation conditional)
143-
{
144-
// Check if the when-true branch is an early return or throw (not break/continue/goto)
145-
if (conditional.WhenTrue is IBlockOperation block)
146-
{
147-
foreach (var statement in block.Operations)
148-
{
149-
if (statement is IReturnOperation or IThrowOperation)
150-
{
151-
return true;
152-
}
153-
}
154-
}
155-
else if (conditional.WhenTrue is IReturnOperation or IThrowOperation)
156-
{
157-
return true;
158-
}
159-
160-
return false;
161-
}
162-
163-
private static bool FindMatchCallAfterConditional(IConditionalOperation conditional, ImmutableArray<IMethodSymbol> regexMatchSymbols, IInvocationOperation isMatchCall, IOperation conditionalOperation, out IInvocationOperation matchCall)
164-
{
165-
// Navigate to the parent block to find subsequent operations
166-
var parent = conditional.Parent;
167-
while (parent is not null && parent is not IBlockOperation)
168-
{
169-
parent = parent.Parent;
170-
}
171-
172-
if (parent is IBlockOperation parentBlock)
173-
{
174-
bool foundConditional = false;
175-
foreach (var operation in parentBlock.Operations)
176-
{
177-
if (operation == conditional)
178-
{
179-
foundConditional = true;
180-
continue;
181-
}
182-
183-
if (foundConditional)
184-
{
185-
// Look for the first Match call that matches criteria
186-
if (FindMatchCallRecursive(operation, regexMatchSymbols, isMatchCall, conditionalOperation, out matchCall))
187-
{
188-
return true;
189-
}
190-
}
191-
}
192-
}
193-
194-
matchCall = null!;
195-
return false;
196-
}
197-
198120
private static bool AreInvocationsOnSameInstance(IInvocationOperation invocation1, IInvocationOperation invocation2, IOperation conditionalOperation)
199121
{
200122
var instance1 = invocation1.Instance?.WalkDownConversion();

‎src/Microsoft.CodeAnalysis.NetAnalyzers/tests/Microsoft.CodeAnalysis.NetAnalyzers.UnitTests/Microsoft.NetCore.Analyzers/Runtime/AvoidRedundantRegexIsMatchBeforeMatchTests.cs‎

Lines changed: 0 additions & 92 deletions
Original file line numberDiff line numberDiff line change
@@ -234,28 +234,6 @@ void M(string input, string pattern)
234234
""");
235235
}
236236

237-
[Fact]
238-
public async Task RedundantIsMatchGuard_InvertedWithEarlyReturn_CSharp_ReportsDiagnostic()
239-
{
240-
await VerifyCS.VerifyAnalyzerAsync("""
241-
using System.Text.RegularExpressions;
242-
243-
class C
244-
{
245-
void M(string input, string pattern)
246-
{
247-
if (!{|CA2027:Regex.IsMatch(input, pattern)|})
248-
{
249-
return;
250-
}
251-
252-
Match m = Regex.Match(input, pattern);
253-
// use m
254-
}
255-
}
256-
""");
257-
}
258-
259237
[Fact]
260238
public async Task NoRedundantIsMatchGuard_VariableReassigned_CSharp_NoDiagnostic()
261239
{
@@ -530,54 +508,6 @@ void M(string input, string pattern)
530508
""");
531509
}
532510

533-
[Fact]
534-
public async Task RedundantIsMatchGuard_InvertedWithMultipleStatements_CSharp_ReportsDiagnostic()
535-
{
536-
await VerifyCS.VerifyAnalyzerAsync("""
537-
using System.Text.RegularExpressions;
538-
539-
class C
540-
{
541-
void M(string input, string pattern)
542-
{
543-
if (!{|CA2027:Regex.IsMatch(input, pattern)|})
544-
{
545-
System.Console.WriteLine("No match");
546-
return;
547-
}
548-
549-
Match m = Regex.Match(input, pattern);
550-
System.Console.WriteLine(m.Value);
551-
}
552-
}
553-
""");
554-
}
555-
556-
[Fact]
557-
public async Task NoRedundantIsMatchGuard_InvertedWithContinue_CSharp_NoDiagnostic()
558-
{
559-
await VerifyCS.VerifyAnalyzerAsync("""
560-
using System.Text.RegularExpressions;
561-
562-
class C
563-
{
564-
void M(string[] inputs, string pattern)
565-
{
566-
foreach (var input in inputs)
567-
{
568-
if (!Regex.IsMatch(input, pattern))
569-
{
570-
continue;
571-
}
572-
573-
Match m = Regex.Match(input, pattern);
574-
System.Console.WriteLine(m.Value);
575-
}
576-
}
577-
}
578-
""");
579-
}
580-
581511
[Fact]
582512
public async Task RedundantIsMatchGuard_LocalRegexVariable_CSharp_ReportsDiagnostic()
583513
{
@@ -622,27 +552,5 @@ void M(string input, string pattern)
622552
""");
623553
}
624554

625-
[Fact]
626-
public async Task RedundantIsMatchGuard_WithThrow_CSharp_ReportsDiagnostic()
627-
{
628-
await VerifyCS.VerifyAnalyzerAsync("""
629-
using System;
630-
using System.Text.RegularExpressions;
631-
632-
class C
633-
{
634-
void M(string input, string pattern)
635-
{
636-
if (!{|CA2027:Regex.IsMatch(input, pattern)|})
637-
{
638-
throw new ArgumentException("No match");
639-
}
640-
641-
Match m = Regex.Match(input, pattern);
642-
System.Console.WriteLine(m.Value);
643-
}
644-
}
645-
""");
646-
}
647555
}
648556
}

0 commit comments

Comments
 (0)