Skip to content

Skip char overloads in LuceneDev6001/6002#32

Open
paulirwin wants to merge 2 commits intoapache:mainfrom
paulirwin:issue/1211-fix
Open

Skip char overloads in LuceneDev6001/6002#32
paulirwin wants to merge 2 commits intoapache:mainfrom
paulirwin:issue/1211-fix

Conversation

@paulirwin
Copy link
Copy Markdown
Contributor

The analyzer's literal-only guard only suppressed 6001/6002 when the first argument was a 'x' char literal or "x" single-character string literal. Calls like text.IndexOf(c) where c is a char variable bound to string.IndexOf(char) — which has no StringComparison sibling — falsely tripped 6001 (#1211).

Filter both the resolved-symbol and ambiguous-candidate paths to overloads whose first parameter is System.String, so char overloads are excluded from the rule entirely instead of relying on syntactic literal recognition.

The analyzer's literal-only guard only suppressed 6001/6002 when the
first argument was a `'x'` char literal or `"x"` single-character
string literal. Calls like `text.IndexOf(c)` where `c` is a char
variable bound to `string.IndexOf(char)` — which has no
StringComparison sibling — falsely tripped 6001 (#1211).

Filter both the resolved-symbol and ambiguous-candidate paths to
overloads whose first parameter is `System.String`, so char overloads
are excluded from the rule entirely instead of relying on syntactic
literal recognition.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@paulirwin paulirwin requested a review from Copilot May 2, 2026 20:55
@paulirwin paulirwin added the notes:bug-fix Contains a fix for a bug label May 2, 2026
@paulirwin paulirwin marked this pull request as ready for review May 2, 2026 20:55
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the LuceneDev6001/6002 analyzer to avoid false positives on string APIs that don’t have StringComparison overloads (notably IndexOf(char) / StartsWith(char)), and adds regression tests to cover those cases.

Changes:

  • Filter analyzer (resolved symbol + ambiguous-candidate paths) to only consider overloads whose first parameter is System.String.
  • Add new analyzer tests ensuring IndexOf(char) and StartsWith(char) calls do not produce diagnostics.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/Lucene.Net.CodeAnalysis.Dev/LuceneDev6xxx/LuceneDev6001_6002_StringComparisonAnalyzer.cs Adds a first-parameter-type filter to exclude char overloads from 6001/6002 analysis.
tests/Lucene.Net.CodeAnalysis.Dev.Tests/LuceneDev6xxx/TestLuceneDev6001_6002_StringComparisonAnalyzer.cs Adds regression tests for IndexOf(char) / StartsWith(char) literal + variable cases.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

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>
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

}";

var test = new InjectableCSharpAnalyzerTest(() => new LuceneDev6001_6002_StringComparisonAnalyzer())
{
if (candidateSymbols.Length > 0)
{
// Check if any candidate is from String or J2N types
// Check if any candidate is from String or J2N types and takes a string first parameter
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

notes:bug-fix Contains a fix for a bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants