Skip to content

Commit 9534121

Browse files
paulirwinclaude
andcommitted
Address Copilot review: handle non-reduced extension calls
The previous filter checked Parameters[0], which is correct for instance methods and reduced extension calls (sb.IndexOf("x")) but not for static-form extension calls (StringBuilderExtensions.IndexOf(sb, "x")) where Parameters[0] is the receiver. The two call shapes resolve to the same underlying API but my filter would silently skip the static form. Use the same value-parameter-index pattern as LuceneDev6005: index 1 when IsExtensionMethod && ReducedFrom == null, else 0. Adds tests for both reduced-form (already worked) and static-form (was the regression Copilot flagged) calls into a J2N-style StringBuilderExtensions stub. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent ef9493e commit 9534121

2 files changed

Lines changed: 89 additions & 5 deletions

File tree

src/Lucene.Net.CodeAnalysis.Dev/LuceneDev6xxx/LuceneDev6001_6002_StringComparisonAnalyzer.cs

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -118,10 +118,17 @@ static bool HasStringComparisonParameter(IMethodSymbol? m, INamedTypeSymbol scTy
118118
var (hasStringComparisonArg, isValidValue, invalidArgLocation, comparisonValueName) =
119119
CheckStringComparisonArgument(invocation, semantic, stringComparisonType);
120120

121-
// The rule only applies to overloads whose first parameter is a string —
121+
// The rule only applies to overloads whose value parameter is a string —
122122
// overloads like IndexOf(char) / StartsWith(char) have no StringComparison sibling.
123-
static bool FirstParameterIsString(IMethodSymbol? m)
124-
=> m != null && m.Parameters.Length > 0 && m.Parameters[0].Type.SpecialType == SpecialType.System_String;
123+
// For non-reduced extension method calls (e.g. StringBuilderExtensions.IndexOf(sb, "x")),
124+
// Parameters[0] is the receiver, so skip past it.
125+
static bool ValueParameterIsString(IMethodSymbol? m)
126+
{
127+
if (m == null || m.Parameters.Length == 0) return false;
128+
int valueIndex = m.IsExtensionMethod && m.ReducedFrom == null ? 1 : 0;
129+
return m.Parameters.Length > valueIndex
130+
&& m.Parameters[valueIndex].Type.SpecialType == SpecialType.System_String;
131+
}
125132

126133
// If resolved symbol available
127134
if (methodSymbol != null)
@@ -130,7 +137,7 @@ static bool FirstParameterIsString(IMethodSymbol? m)
130137
if (!ContainingTypeIsStringOrJ2N(methodSymbol.ContainingType))
131138
return;
132139

133-
if (!FirstParameterIsString(methodSymbol))
140+
if (!ValueParameterIsString(methodSymbol))
134141
return;
135142

136143
// If the method has StringComparison parameter in signature
@@ -170,7 +177,7 @@ static bool FirstParameterIsString(IMethodSymbol? m)
170177
{
171178
// Check if any candidate is from String or J2N types and takes a string first parameter
172179
var relevantCandidates = candidateSymbols
173-
.Where(c => ContainingTypeIsStringOrJ2N(c.ContainingType) && FirstParameterIsString(c))
180+
.Where(c => ContainingTypeIsStringOrJ2N(c.ContainingType) && ValueParameterIsString(c))
174181
.ToImmutableArray();
175182

176183
if (relevantCandidates.Length == 0)

tests/Lucene.Net.CodeAnalysis.Dev.Tests/LuceneDev6xxx/TestLuceneDev6001_6002_StringComparisonAnalyzer.cs

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -581,5 +581,82 @@ public void M()
581581

582582
await test.RunAsync();
583583
}
584+
585+
[Test]
586+
public async Task Detects_J2NStringBuilderExtensions_ReducedForm_MissingStringComparison()
587+
{
588+
var testCode = @"
589+
using System.Text;
590+
using J2N.Text;
591+
592+
namespace J2N.Text
593+
{
594+
public static class StringBuilderExtensions
595+
{
596+
public static int IndexOf(this StringBuilder sb, string value) => 0;
597+
}
598+
}
599+
600+
public class Sample
601+
{
602+
public void M()
603+
{
604+
var sb = new StringBuilder(""hello"");
605+
int index = sb.IndexOf(""he"");
606+
}
607+
}";
608+
609+
var expected = new DiagnosticResult(Descriptors.LuceneDev6001_MissingStringComparison.Id, DiagnosticSeverity.Error)
610+
.WithSeverity(DiagnosticSeverity.Error)
611+
.WithMessageFormat(Descriptors.LuceneDev6001_MissingStringComparison.MessageFormat)
612+
.WithArguments("IndexOf")
613+
.WithLocation("/0/Test0.cs", line: 18, column: 24);
614+
615+
var test = new InjectableCSharpAnalyzerTest(() => new LuceneDev6001_6002_StringComparisonAnalyzer())
616+
{
617+
TestCode = testCode,
618+
ExpectedDiagnostics = { expected }
619+
};
620+
621+
await test.RunAsync();
622+
}
623+
624+
[Test]
625+
public async Task Detects_J2NStringBuilderExtensions_StaticForm_MissingStringComparison()
626+
{
627+
var testCode = @"
628+
using System.Text;
629+
630+
namespace J2N.Text
631+
{
632+
public static class StringBuilderExtensions
633+
{
634+
public static int IndexOf(this StringBuilder sb, string value) => 0;
635+
}
636+
}
637+
638+
public class Sample
639+
{
640+
public void M()
641+
{
642+
var sb = new StringBuilder(""hello"");
643+
int index = J2N.Text.StringBuilderExtensions.IndexOf(sb, ""he"");
644+
}
645+
}";
646+
647+
var expected = new DiagnosticResult(Descriptors.LuceneDev6001_MissingStringComparison.Id, DiagnosticSeverity.Error)
648+
.WithSeverity(DiagnosticSeverity.Error)
649+
.WithMessageFormat(Descriptors.LuceneDev6001_MissingStringComparison.MessageFormat)
650+
.WithArguments("IndexOf")
651+
.WithLocation("/0/Test0.cs", line: 17, column: 54);
652+
653+
var test = new InjectableCSharpAnalyzerTest(() => new LuceneDev6001_6002_StringComparisonAnalyzer())
654+
{
655+
TestCode = testCode,
656+
ExpectedDiagnostics = { expected }
657+
};
658+
659+
await test.RunAsync();
660+
}
584661
}
585662
}

0 commit comments

Comments
 (0)