Skip to content

Commit 1c82f19

Browse files
paulirwinpaulirwin
committed
Address LuceneDev600x review feedback and fix tests
- LuceneDev6003 code fix: use getInnermostNodeForTie + descendant fallback so FindNode resolves to the LiteralExpressionSyntax. - LuceneDev6003 analyzer: detect char overload correctly for spans. ReadOnlySpan<T>.IndexOf(T) / LastIndexOf(T) are MemoryExtensions extensions, so the previous search on the containing type missed them. StartsWith/EndsWith intentionally remain excluded (no char overload exists for spans). - LuceneDev6001 analyzer: pass the invalid StringComparison value name to Diagnostic.Create so the {1} placeholder in the message format is substituted (previously rendered as literal "{1}"). - LuceneDev6001 code fix: use continue instead of return inside the diagnostic loop so multiple diagnostics in one file all get fixes offered. - Sample project: uncomment violating examples so the diagnostics fire live in the IDE (matches the existing LuceneDevXxxxSample convention). Add .editorconfig to downgrade error-severity rules to warning in the sample project so it still compiles. - LuceneDev6003 sample: rewrite to use the analyzer's target methods (IndexOf/LastIndexOf/StartsWith/EndsWith) rather than string.Equals, including a ReadOnlySpan<char> example. - Clean up excessive blank lines in 6002 code fix and a stray trailing space in a variable name. - Update 6001 tests to include the comparison-value argument. - Fix off-by-one spans in 6003 code fix tests and add a span test.
1 parent 03733bd commit 1c82f19

13 files changed

Lines changed: 147 additions & 263 deletions

‎src/Lucene.Net.CodeAnalysis.Dev.CodeFixes/LuceneDev6xxx/LuceneDev6001_StringComparisonCodeFixProvider.cs‎

Lines changed: 10 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -54,28 +54,23 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context)
5454
var semanticModel = await context.Document.GetSemanticModelAsync(context.CancellationToken).ConfigureAwait(false);
5555
if (semanticModel == null) continue;
5656

57-
//Double check to Skip char literals and single-character string literals when safe ---
57+
// Skip char literals and single-character string literals when safe (covered by 6003 instead).
5858
var firstArgExpr = invocation.ArgumentList.Arguments.FirstOrDefault()?.Expression;
5959
if (firstArgExpr is LiteralExpressionSyntax lit)
6060
{
6161
if (lit.IsKind(SyntaxKind.CharacterLiteralExpression))
62-
return; // already char overload; no diagnostic
62+
continue;
6363

6464
if (lit.IsKind(SyntaxKind.StringLiteralExpression) && lit.Token.ValueText.Length == 1)
6565
{
66-
// Check if a StringComparison argument is present
67-
bool hasStringComparisonArgForLiteral = invocation.ArgumentList.Arguments.Any(arg =>
68-
semanticModel.GetTypeInfo(arg.Expression).Type is INamedTypeSymbol t &&
69-
t.ToDisplayString() == "System.StringComparison"
70-
|| (semanticModel.GetSymbolInfo(arg.Expression).Symbol is IFieldSymbol f &&
71-
f.ContainingType?.ToDisplayString() == "System.StringComparison"));
72-
73-
if (!hasStringComparisonArgForLiteral)
74-
{
75-
// safe to convert to char (6003), so skip 6001 reporting
76-
return;
77-
}
78-
// else: has StringComparison -> do not skip; let codefix handle it
66+
bool hasStringComparisonArg = invocation.ArgumentList.Arguments.Any(arg =>
67+
(semanticModel.GetTypeInfo(arg.Expression).Type is INamedTypeSymbol t &&
68+
t.ToDisplayString() == "System.StringComparison")
69+
|| (semanticModel.GetSymbolInfo(arg.Expression).Symbol is IFieldSymbol f &&
70+
f.ContainingType?.ToDisplayString() == "System.StringComparison"));
71+
72+
if (!hasStringComparisonArg)
73+
continue;
7974
}
8075
}
8176

‎src/Lucene.Net.CodeAnalysis.Dev.CodeFixes/LuceneDev6xxx/LuceneDev6002_SpanComparisonCodeFixProvider.cs‎

Lines changed: 6 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -64,59 +64,28 @@ public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context)
6464
if (invocation == null)
6565
return;
6666

67-
//Double check to Skip char literals and single-character string literals when safe ---
67+
// Skip char literals and single-character string literals when safe (covered by 6003 instead).
6868
var firstArgExpr = invocation.ArgumentList.Arguments.FirstOrDefault()?.Expression;
69-
7069
if (firstArgExpr is LiteralExpressionSyntax lit)
71-
7270
{
73-
7471
if (lit.IsKind(SyntaxKind.CharacterLiteralExpression))
75-
76-
return; // already char overload; skip 6002 fix
77-
78-
72+
return;
7973

8074
if (lit.IsKind(SyntaxKind.StringLiteralExpression) && lit.Token.ValueText.Length == 1)
81-
8275
{
83-
84-
// Check if a StringComparison argument is present
85-
8676
var semanticModel = await context.Document.GetSemanticModelAsync(context.CancellationToken).ConfigureAwait(false);
87-
8877
if (semanticModel == null)
89-
9078
return;
9179

92-
93-
94-
bool hasStringComparisonArgForLiteral = invocation.ArgumentList.Arguments.Any(arg =>
95-
96-
semanticModel.GetTypeInfo(arg.Expression).Type is INamedTypeSymbol t &&
97-
98-
t.ToDisplayString() == "System.StringComparison"
99-
80+
bool hasStringComparisonArg = invocation.ArgumentList.Arguments.Any(arg =>
81+
(semanticModel.GetTypeInfo(arg.Expression).Type is INamedTypeSymbol t &&
82+
t.ToDisplayString() == "System.StringComparison")
10083
|| (semanticModel.GetSymbolInfo(arg.Expression).Symbol is IFieldSymbol f &&
101-
10284
f.ContainingType?.ToDisplayString() == "System.StringComparison"));
10385

104-
105-
106-
if (!hasStringComparisonArgForLiteral)
107-
108-
{
109-
110-
// safe to convert to char (6003), skip 6002 fix
111-
86+
if (!hasStringComparisonArg)
11287
return;
113-
114-
}
115-
116-
// else: has StringComparison -> let the codefix continue
117-
11888
}
119-
12089
}
12190
switch (diagnostic.Id)
12291
{

‎src/Lucene.Net.CodeAnalysis.Dev.CodeFixes/LuceneDev6xxx/LuceneDev6003_SingleCharStringCodeFixProvider.cs‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,10 @@
1616
* limitations under the License.
1717
*/
1818

19-
using System;
2019
using System.Collections.Immutable;
2120
using System.Composition;
2221
using System.Globalization;
22+
using System.Linq;
2323
using System.Threading;
2424
using System.Threading.Tasks;
2525
using Lucene.Net.CodeAnalysis.Dev.Utility;
@@ -49,9 +49,12 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context)
4949
return;
5050

5151
var diagnosticSpan = diagnostic.Location.SourceSpan;
52-
var node = root.FindNode(diagnosticSpan);
52+
var node = root.FindNode(diagnosticSpan, getInnermostNodeForTie: true);
5353

54-
if (node is LiteralExpressionSyntax literal && literal.IsKind(SyntaxKind.StringLiteralExpression))
54+
var literal = node as LiteralExpressionSyntax
55+
?? node.DescendantNodesAndSelf().OfType<LiteralExpressionSyntax>().FirstOrDefault(l => l.Span == diagnosticSpan);
56+
57+
if (literal != null && literal.IsKind(SyntaxKind.StringLiteralExpression))
5558
{
5659
context.RegisterCodeFix(
5760
CodeAction.Create(

‎src/Lucene.Net.CodeAnalysis.Dev.Sample/.editorconfig‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
# The Sample project intentionally contains code that fires Lucene.NET analyzers
2+
# so contributors can see the diagnostics live in their IDE. Downgrade Error-severity
3+
# rules to Warning here so the project still compiles.
4+
[*.cs]
5+
dotnet_diagnostic.LuceneDev6001_1.severity = warning
6+
dotnet_diagnostic.LuceneDev6001_2.severity = warning
7+
dotnet_diagnostic.LuceneDev6002_2.severity = warning

‎src/Lucene.Net.CodeAnalysis.Dev.Sample/LuceneDev6xxx/LuceneDev6001_StringComparisonSample.cs‎

Lines changed: 14 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
/*
1+
/*
22
* Licensed to the Apache Software Foundation (ASF) under one
33
* or more contributor license agreements. See the NOTICE file
44
* distributed with this work for additional information
@@ -17,37 +17,26 @@
1717
* under the License.
1818
*/
1919

20+
using System;
21+
2022
namespace Lucene.Net.CodeAnalysis.Dev.Sample;
2123

2224
public class LuceneDev6001_StringComparisonSample
2325
{
24-
// public void BadExample_MissingStringComparison()
25-
// {
26-
// string text = "Hello World";
27-
28-
// //Missing StringComparison parameter
29-
// int index = text.IndexOf("Hello");
30-
// bool starts = text.StartsWith("Hello");
31-
// bool ends = text.EndsWith("World");
32-
// }
33-
34-
public void GoodExample_Ordinal()
26+
public void MyMethod()
3527
{
3628
string text = "Hello World";
3729

38-
//Correct usage with StringComparison.Ordinal
39-
int index = text.IndexOf("Hello", System.StringComparison.Ordinal);
40-
bool starts = text.StartsWith("Hello", System.StringComparison.Ordinal);
41-
bool ends = text.EndsWith("World", System.StringComparison.Ordinal);
42-
}
43-
44-
public void GoodExample_OrdinalIgnoreCase()
45-
{
46-
string text = "Hello World";
30+
// Missing StringComparison argument: triggers LuceneDev6001_1 (Error).
31+
int index1 = text.IndexOf("Hello");
32+
bool starts1 = text.StartsWith("Hello");
33+
bool ends1 = text.EndsWith("World");
34+
int lastIndex1 = text.LastIndexOf("World");
4735

48-
// Correct usage with StringComparison.OrdinalIgnoreCase
49-
int index = text.IndexOf("hello", System.StringComparison.OrdinalIgnoreCase);
50-
bool starts = text.StartsWith("HELLO", System.StringComparison.OrdinalIgnoreCase);
51-
bool ends = text.EndsWith("world", System.StringComparison.OrdinalIgnoreCase);
36+
// Invalid StringComparison value: triggers LuceneDev6001_2 (Error).
37+
int index2 = text.IndexOf("Hello", StringComparison.CurrentCulture);
38+
bool starts2 = text.StartsWith("hello", StringComparison.CurrentCultureIgnoreCase);
39+
bool ends2 = text.EndsWith("World", StringComparison.InvariantCulture);
40+
int lastIndex2 = text.LastIndexOf("world", StringComparison.InvariantCultureIgnoreCase);
5241
}
5342
}

‎src/Lucene.Net.CodeAnalysis.Dev.Sample/LuceneDev6xxx/LuceneDev6002_SpanComparisonSample.cs‎

Lines changed: 18 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
/*
1+
/*
22
* Licensed to the Apache Software Foundation (ASF) under one
33
* or more contributor license agreements. See the NOTICE file
44
* distributed with this work for additional information
@@ -19,90 +19,24 @@
1919

2020
using System;
2121

22-
namespace Lucene.Net.CodeAnalysis.Dev.Sample.LuceneDev6xxx
22+
namespace Lucene.Net.CodeAnalysis.Dev.Sample.LuceneDev6xxx;
23+
24+
public class LuceneDev6002_SpanComparisonSample
2325
{
24-
/// <summary>
25-
/// Sample code demonstrating LuceneDev6002 analyzer rules for Span types.
26-
/// Rule: Span types should not use StringComparison.Ordinal (redundant)
27-
/// and must only use Ordinal or OrdinalIgnoreCase.
28-
/// </summary>
29-
public class LuceneDev6002_SpanComparisonSample
26+
public void MyMethod()
3027
{
31-
// public void BadExamples_RedundantOrdinal()
32-
// {
33-
// ReadOnlySpan<char> span = "Hello World".AsSpan();
34-
35-
// // Redundant StringComparison.Ordinal
36-
// int index1 = span.IndexOf("Hello".AsSpan(), StringComparison.Ordinal);
37-
// int index2 = span.LastIndexOf("World".AsSpan(), StringComparison.Ordinal);
38-
// bool starts = span.StartsWith("Hello".AsSpan(), StringComparison.Ordinal);
39-
// bool ends = span.EndsWith("World".AsSpan(), StringComparison.Ordinal);
40-
// }
41-
42-
// public void BadExamples_InvalidComparison()
43-
// {
44-
// ReadOnlySpan<char> span = "Hello World".AsSpan();
45-
46-
// // Culture-sensitive comparisons are not allowed on Span types
47-
// int index1 = span.IndexOf("Hello", StringComparison.CurrentCulture);
48-
// int index2 = span.LastIndexOf("World", StringComparison.CurrentCultureIgnoreCase);
49-
// bool starts = span.StartsWith("Hello", StringComparison.InvariantCulture);
50-
// bool ends = span.EndsWith("World", StringComparison.InvariantCultureIgnoreCase);
51-
// }
52-
53-
public void GoodExamples_NoStringComparison()
54-
{
55-
ReadOnlySpan<char> span = "Hello World".AsSpan();
56-
57-
// Correct: defaults to Ordinal
58-
int index1 = span.IndexOf("Hello".AsSpan());
59-
int index2 = span.LastIndexOf("World".AsSpan());
60-
bool starts = span.StartsWith("Hello".AsSpan());
61-
bool ends = span.EndsWith("World".AsSpan());
62-
63-
// Single char operations
64-
int charIndex = span.IndexOf('H');
65-
bool startsWithChar = span[0] == 'H';
66-
}
67-
68-
public void GoodExamples_WithOrdinalIgnoreCase()
69-
{
70-
ReadOnlySpan<char> span = "Hello World".AsSpan();
71-
72-
// Correct: case-insensitive search
73-
int index = span.IndexOf("hello", StringComparison.OrdinalIgnoreCase);
74-
int lastIndex = span.LastIndexOf("WORLD", StringComparison.OrdinalIgnoreCase);
75-
bool starts = span.StartsWith("HELLO", StringComparison.OrdinalIgnoreCase);
76-
bool ends = span.EndsWith("world", StringComparison.OrdinalIgnoreCase);
77-
}
78-
79-
public void RealWorldExamples()
80-
{
81-
string path = @"C:\Users\Documents\file.txt";
82-
ReadOnlySpan<char> pathSpan = path.AsSpan();
83-
84-
// Correct: OrdinalIgnoreCase allowed
85-
bool isTxtFile = pathSpan.EndsWith(".txt", StringComparison.OrdinalIgnoreCase);
86-
87-
// Correct: No StringComparison needed
88-
ReadOnlySpan<char> url = "https://example.com".AsSpan();
89-
bool isHttps = url.StartsWith("https://");
90-
91-
ReadOnlySpan<char> token = "Bearer:abc123".AsSpan();
92-
int separatorIndex = token.IndexOf(':');
93-
}
94-
95-
public void StringTypeComparison()
96-
{
97-
// Analyzer applies only to Span types
98-
string text = "Hello World";
99-
100-
// String types require StringComparison
101-
int index = text.IndexOf("Hello", StringComparison.Ordinal);
102-
103-
// Span types should not specify Ordinal
104-
ReadOnlySpan<char> span = text.AsSpan();
105-
int index2 = span.IndexOf("Hello");
106-
}
28+
ReadOnlySpan<char> span = "Hello World".AsSpan();
29+
30+
// Redundant StringComparison.Ordinal on span: triggers LuceneDev6002_1 (Warning).
31+
int index1 = span.IndexOf("Hello".AsSpan(), StringComparison.Ordinal);
32+
int lastIndex1 = span.LastIndexOf("World".AsSpan(), StringComparison.Ordinal);
33+
bool starts1 = span.StartsWith("Hello".AsSpan(), StringComparison.Ordinal);
34+
bool ends1 = span.EndsWith("World".AsSpan(), StringComparison.Ordinal);
35+
36+
// Invalid comparison on span: triggers LuceneDev6002_2 (Error).
37+
int index2 = span.IndexOf("Hello".AsSpan(), StringComparison.CurrentCulture);
38+
int lastIndex2 = span.LastIndexOf("World".AsSpan(), StringComparison.CurrentCultureIgnoreCase);
39+
bool starts2 = span.StartsWith("Hello".AsSpan(), StringComparison.InvariantCulture);
40+
bool ends2 = span.EndsWith("World".AsSpan(), StringComparison.InvariantCultureIgnoreCase);
10741
}
10842
}

‎src/Lucene.Net.CodeAnalysis.Dev.Sample/LuceneDev6xxx/LuceneDev6003_SingleCharStringSample.cs‎

Lines changed: 17 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
/*
1+
/*
22
* Licensed to the Apache Software Foundation (ASF) under one
33
* or more contributor license agreements. See the NOTICE file
44
* distributed with this work for additional information
@@ -19,40 +19,26 @@
1919

2020
using System;
2121

22-
namespace Lucene.Net.CodeAnalysis.Dev.Sample.LuceneDev6xxx
22+
namespace Lucene.Net.CodeAnalysis.Dev.Sample.LuceneDev6xxx;
23+
24+
public class LuceneDev6003_SingleCharStringSample
2325
{
24-
/// <summary>
25-
/// Sample code for LuceneDev6003: Suggest using char overloads instead of single-character string literals.
26-
/// </summary>
27-
public class LuceneDev6003_SingleCharStringSample
26+
public void MyMethod()
2827
{
29-
public void Example()
30-
{
31-
string input = "Hello";
32-
33-
// BAD: Using string.Equals with single-character string literal
34-
// if (string.Equals(input[0].ToString(), "H"))
35-
// {
36-
// Console.WriteLine("Starts with H");
37-
// }
28+
string text = "Hello World";
3829

39-
// BAD: Using Equals instance method
40-
// if (input[0].ToString().Equals("H"))
41-
// {
42-
// Console.WriteLine("Starts with H");
43-
// }
30+
// Single-character string literal: triggers LuceneDev6003 (Info).
31+
int index1 = text.IndexOf("H", StringComparison.Ordinal);
32+
int lastIndex1 = text.LastIndexOf("d", StringComparison.Ordinal);
33+
bool starts1 = text.StartsWith("H", StringComparison.Ordinal);
34+
bool ends1 = text.EndsWith("d", StringComparison.Ordinal);
4435

45-
// GOOD: Using char comparison instead of string
46-
if (input[0] == 'H')
47-
{
48-
Console.WriteLine("Starts with H");
49-
}
36+
// Escaped single-character string literal: also triggers LuceneDev6003.
37+
int newlineIndex = text.IndexOf("\n", StringComparison.Ordinal);
5038

51-
//GOOD: Using Char.Equals
52-
if (char.Equals(input[0], 'H'))
53-
{
54-
Console.WriteLine("Starts with H");
55-
}
56-
}
39+
// IndexOf/LastIndexOf have a char overload on ReadOnlySpan<char>: triggers LuceneDev6003.
40+
ReadOnlySpan<char> span = text.AsSpan();
41+
int index2 = span.IndexOf("H");
42+
int lastIndex2 = span.LastIndexOf("d");
5743
}
5844
}

0 commit comments

Comments
 (0)