Skip to content

Commit 0ae7d8d

Browse files
paulirwinclaude
andcommitted
Address Copilot review feedback on Lucene1001/Lucene1002 analyzers
- Lucene1001 CS code fix: emit ThrowStatement (not ExpressionStatement) when converting an expression-bodied `=> throw ...` member, so the produced code is valid C#. - Lucene1001 CS + VB analyzers: exclude lambdas, anonymous methods, and (CS) local functions from the base-call scan. A base call inside an uninvoked delegate body should not satisfy the contract. - Lucene1002 CS + VB code fixes: use ElasticCarriageReturnLineFeed instead of hardcoded line endings so the formatter normalizes to the document's convention. - DiagnosticVerifier: harden System.Runtime.dll lookup with a TRUSTED_PLATFORM_ASSEMBLIES fallback and a clear error if neither resolves. - Tests: add VerifyCSharpFix/VerifyBasicFix coverage for Lucene1002, a VerifyCSharpFix for the Lucene1001 expression-bodied case plus a new throw-expression regression, lambda/local-function exclusion tests for Lucene1001 (CS + VB), and the missing VB indirect-Tokenizer-inheritance test for Lucene1002. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 5807c22 commit 0ae7d8d

10 files changed

Lines changed: 359 additions & 8 deletions

src/dotnet/Lucene.Net.CodeAnalysis.CSharp/Lucene1001_AddBaseMethodCallCSCodeFixProvider.cs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,8 +88,10 @@ private static async Task<Document> AddBaseCallAsync(Document document, MethodDe
8888
{
8989
// Convert expression body => to a block containing base.X(); followed by the original expression as a statement.
9090
var existingExpression = methodDeclaration.ExpressionBody.Expression;
91-
var existingStatement = SyntaxFactory.ExpressionStatement(existingExpression)
92-
.WithAdditionalAnnotations(Formatter.Annotation);
91+
StatementSyntax existingStatement = existingExpression is ThrowExpressionSyntax throwExpression
92+
? SyntaxFactory.ThrowStatement(throwExpression.Expression)
93+
: SyntaxFactory.ExpressionStatement(existingExpression);
94+
existingStatement = existingStatement.WithAdditionalAnnotations(Formatter.Annotation);
9395
var newBody = SyntaxFactory.Block(baseStatement, existingStatement)
9496
.WithAdditionalAnnotations(Formatter.Annotation);
9597
newMethodDeclaration = methodDeclaration

src/dotnet/Lucene.Net.CodeAnalysis.CSharp/Lucene1001_TokenStreamOverrideMustCallBaseMethodCSAnalyzer.cs

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,13 +103,16 @@ private static void AnalyzeNode(SyntaxNodeAnalysisContext context)
103103
private static bool ContainsBaseCall(MethodDeclarationSyntax methodDeclaration, string methodName)
104104
{
105105
// Inspect either a block body or an expression-bodied member.
106+
SyntaxNode body;
106107
IEnumerable<SyntaxNode> nodes;
107108
if (methodDeclaration.Body is not null)
108109
{
110+
body = methodDeclaration.Body;
109111
nodes = methodDeclaration.Body.DescendantNodes();
110112
}
111113
else if (methodDeclaration.ExpressionBody is not null)
112114
{
115+
body = methodDeclaration.ExpressionBody;
113116
nodes = methodDeclaration.ExpressionBody.DescendantNodesAndSelf();
114117
}
115118
else
@@ -123,7 +126,25 @@ private static bool ContainsBaseCall(MethodDeclarationSyntax methodDeclaration,
123126
if (node is InvocationExpressionSyntax invocation &&
124127
invocation.Expression is MemberAccessExpressionSyntax memberAccess &&
125128
memberAccess.Expression.IsKind(SyntaxKind.BaseExpression) &&
126-
memberAccess.Name.Identifier.ValueText == methodName)
129+
memberAccess.Name.Identifier.ValueText == methodName &&
130+
!IsInsideNestedFunction(invocation, body))
131+
{
132+
return true;
133+
}
134+
}
135+
return false;
136+
}
137+
138+
// A base call inside a lambda, anonymous method, or local function does not necessarily
139+
// execute when the enclosing method runs (the delegate may never be invoked). Treat such
140+
// occurrences as not satisfying the contract.
141+
private static bool IsInsideNestedFunction(SyntaxNode node, SyntaxNode body)
142+
{
143+
for (var ancestor = node.Parent; ancestor is not null && ancestor != body; ancestor = ancestor.Parent)
144+
{
145+
if (ancestor is LambdaExpressionSyntax ||
146+
ancestor is AnonymousMethodExpressionSyntax ||
147+
ancestor is LocalFunctionStatementSyntax)
127148
{
128149
return true;
129150
}

src/dotnet/Lucene.Net.CodeAnalysis.CSharp/Lucene1002_AddEndOverrideCSCodeFixProvider.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ private static async Task<Document> AddEndOverrideAsync(Document document, Class
7878

7979
var todoComment = SyntaxFactory.Comment("// TODO: set the final offset and finish up other end-of-stream attributes");
8080
var closeBrace = SyntaxFactory.Token(SyntaxKind.CloseBraceToken)
81-
.WithLeadingTrivia(SyntaxFactory.TriviaList(todoComment, SyntaxFactory.EndOfLine("\n")));
81+
.WithLeadingTrivia(SyntaxFactory.TriviaList(todoComment, SyntaxFactory.ElasticCarriageReturnLineFeed));
8282

8383
var body = SyntaxFactory.Block(
8484
SyntaxFactory.Token(SyntaxKind.OpenBraceToken),

src/dotnet/Lucene.Net.CodeAnalysis.VisualBasic/Lucene1001_TokenStreamOverrideMustCallBaseMethodVBAnalyzer.cs

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,22 @@ private static bool ContainsBaseCall(MethodBlockSyntax methodBlock, string metho
102102
if (node is InvocationExpressionSyntax invocation &&
103103
invocation.Expression is MemberAccessExpressionSyntax memberAccess &&
104104
memberAccess.Expression.IsKind(SyntaxKind.MyBaseExpression) &&
105-
memberAccess.Name.Identifier.ValueText == methodName)
105+
memberAccess.Name.Identifier.ValueText == methodName &&
106+
!IsInsideLambda(invocation, methodBlock))
107+
{
108+
return true;
109+
}
110+
}
111+
return false;
112+
}
113+
114+
// A base call inside a lambda does not necessarily execute when the enclosing method runs
115+
// (the delegate may never be invoked). Treat such occurrences as not satisfying the contract.
116+
private static bool IsInsideLambda(SyntaxNode node, SyntaxNode body)
117+
{
118+
for (var ancestor = node.Parent; ancestor is not null && ancestor != body; ancestor = ancestor.Parent)
119+
{
120+
if (ancestor is LambdaExpressionSyntax)
106121
{
107122
return true;
108123
}

src/dotnet/Lucene.Net.CodeAnalysis.VisualBasic/Lucene1002_AddEndOverrideVBCodeFixProvider.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ private static async Task<Document> AddEndOverrideAsync(Document document, Class
9090
SyntaxFactory.ArgumentList()));
9191

9292
var endSubStatement = SyntaxFactory.EndSubStatement()
93-
.WithLeadingTrivia(SyntaxFactory.CommentTrivia("' TODO: set the final offset and finish up other end-of-stream attributes"), SyntaxFactory.EndOfLine("\r\n"));
93+
.WithLeadingTrivia(SyntaxFactory.CommentTrivia("' TODO: set the final offset and finish up other end-of-stream attributes"), SyntaxFactory.ElasticCarriageReturnLineFeed);
9494

9595
var methodBlock = SyntaxFactory.SubBlock(
9696
subStatement,

src/dotnet/Lucene.Net.Tests.CodeAnalysis/Helpers/DiagnosticVerifier.Helper.cs

Lines changed: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
using System;
66
using System.Collections.Generic;
77
using System.Collections.Immutable;
8+
using System.IO;
89
using System.Linq;
910

1011
namespace TestHelper
@@ -35,8 +36,38 @@ public abstract partial class DiagnosticVerifier
3536
private static readonly MetadataReference CorlibReference = MetadataReference.CreateFromFile(typeof(object).Assembly.Location);
3637
// LUCENENET: typeof(object) lives in System.Private.CoreLib and BCL types are forwarded through
3738
// System.Runtime; analyzer tests that reference base methods returning void (e.g. base.Reset())
38-
// need the runtime reference to resolve System.Void.
39-
private static readonly MetadataReference SystemRuntimeReference = MetadataReference.CreateFromFile(System.IO.Path.Combine(System.IO.Path.GetDirectoryName(typeof(object).Assembly.Location), "System.Runtime.dll"));
39+
// need the runtime reference to resolve System.Void. Try the directory next to corlib first;
40+
// fall back to scanning TRUSTED_PLATFORM_ASSEMBLIES for hosts where the file isn't co-located.
41+
private static readonly MetadataReference SystemRuntimeReference = ResolveSystemRuntimeReference();
42+
43+
private static MetadataReference ResolveSystemRuntimeReference()
44+
{
45+
var corlibDir = Path.GetDirectoryName(typeof(object).Assembly.Location);
46+
if (!string.IsNullOrEmpty(corlibDir))
47+
{
48+
var candidate = Path.Combine(corlibDir, "System.Runtime.dll");
49+
if (File.Exists(candidate))
50+
{
51+
return MetadataReference.CreateFromFile(candidate);
52+
}
53+
}
54+
55+
if (AppContext.GetData("TRUSTED_PLATFORM_ASSEMBLIES") is string tpa)
56+
{
57+
foreach (var path in tpa.Split(Path.PathSeparator))
58+
{
59+
if (Path.GetFileName(path).Equals("System.Runtime.dll", StringComparison.OrdinalIgnoreCase) &&
60+
File.Exists(path))
61+
{
62+
return MetadataReference.CreateFromFile(path);
63+
}
64+
}
65+
}
66+
67+
throw new FileNotFoundException(
68+
"Unable to locate System.Runtime.dll. Looked next to corlib and in TRUSTED_PLATFORM_ASSEMBLIES.");
69+
}
70+
4071
private static readonly MetadataReference SystemCoreReference = MetadataReference.CreateFromFile(typeof(Enumerable).Assembly.Location);
4172
private static readonly MetadataReference CSharpSymbolsReference = MetadataReference.CreateFromFile(typeof(CSharpCompilation).Assembly.Location);
4273
private static readonly MetadataReference CodeAnalysisReference = MetadataReference.CreateFromFile(typeof(Compilation).Assembly.Location);

src/dotnet/Lucene.Net.Tests.CodeAnalysis/TestLucene1001_AddBaseMethodCallCSCodeFixProvider.cs

Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -214,6 +214,73 @@ sealed class TypeName : TokenStream
214214
};
215215

216216
VerifyCSharpDiagnostic(test, expected);
217+
218+
var fixtest = @"
219+
using Lucene.Net.Analysis;
220+
221+
namespace MyNamespace
222+
{
223+
sealed class TypeName : TokenStream
224+
{
225+
public override bool IncrementToken() => false;
226+
227+
public override void Reset()
228+
{
229+
base.Reset();
230+
System.Diagnostics.Debug.WriteLine(""hi"");
231+
}
232+
}
233+
}";
234+
VerifyCSharpFix(test, fixtest);
235+
}
236+
237+
[Test]
238+
public void TestExpressionBodiedThrow_Diagnostic_CodeFix()
239+
{
240+
// Regression: an expression-bodied void method whose body is `=> throw ...;`
241+
// must be converted to a block whose trailing statement is a ThrowStatement,
242+
// not an ExpressionStatement (which would be invalid C#).
243+
var test = @"
244+
using Lucene.Net.Analysis;
245+
using System;
246+
247+
namespace MyNamespace
248+
{
249+
sealed class TypeName : TokenStream
250+
{
251+
public override bool IncrementToken() => false;
252+
253+
public override void Reset() => throw new NotSupportedException();
254+
}
255+
}";
256+
var expected = new DiagnosticResult
257+
{
258+
Id = Lucene1001_TokenStreamOverrideMustCallBaseMethodCSAnalyzer.DiagnosticId,
259+
Message = "Override of 'Reset()' on type 'TypeName' must call base.Reset().",
260+
Severity = DiagnosticSeverity.Warning,
261+
Locations = new[] { new DiagnosticResultLocation("Test0.cs", 11, 30) }
262+
};
263+
264+
VerifyCSharpDiagnostic(test, expected);
265+
266+
var fixtest = @"
267+
using Lucene.Net.Analysis;
268+
using System;
269+
270+
namespace MyNamespace
271+
{
272+
sealed class TypeName : TokenStream
273+
{
274+
public override bool IncrementToken() => false;
275+
276+
public override void Reset()
277+
{
278+
base.Reset();
279+
throw new NotSupportedException();
280+
}
281+
}
282+
}";
283+
VerifyCSharpFix(test, fixtest);
217284
}
218285

219286
[Test]
@@ -286,5 +353,66 @@ public override void Reset()
286353

287354
VerifyCSharpDiagnostic(test, expected);
288355
}
356+
357+
[Test]
358+
public void TestBaseCallInsideLambda_Diagnostic()
359+
{
360+
// A base call inside an uninvoked lambda does not satisfy the contract.
361+
var test = @"
362+
using Lucene.Net.Analysis;
363+
using System;
364+
365+
namespace MyNamespace
366+
{
367+
sealed class TypeName : TokenStream
368+
{
369+
public override bool IncrementToken() => false;
370+
371+
public override void Reset()
372+
{
373+
Action a = () => base.Reset();
374+
}
375+
}
376+
}";
377+
var expected = new DiagnosticResult
378+
{
379+
Id = Lucene1001_TokenStreamOverrideMustCallBaseMethodCSAnalyzer.DiagnosticId,
380+
Message = "Override of 'Reset()' on type 'TypeName' must call base.Reset().",
381+
Severity = DiagnosticSeverity.Warning,
382+
Locations = new[] { new DiagnosticResultLocation("Test0.cs", 11, 30) }
383+
};
384+
385+
VerifyCSharpDiagnostic(test, expected);
386+
}
387+
388+
[Test]
389+
public void TestBaseCallInsideLocalFunction_Diagnostic()
390+
{
391+
// A base call inside an uninvoked local function does not satisfy the contract.
392+
var test = @"
393+
using Lucene.Net.Analysis;
394+
395+
namespace MyNamespace
396+
{
397+
sealed class TypeName : TokenStream
398+
{
399+
public override bool IncrementToken() => false;
400+
401+
public override void Reset()
402+
{
403+
void Local() => base.Reset();
404+
}
405+
}
406+
}";
407+
var expected = new DiagnosticResult
408+
{
409+
Id = Lucene1001_TokenStreamOverrideMustCallBaseMethodCSAnalyzer.DiagnosticId,
410+
Message = "Override of 'Reset()' on type 'TypeName' must call base.Reset().",
411+
Severity = DiagnosticSeverity.Warning,
412+
Locations = new[] { new DiagnosticResultLocation("Test0.cs", 10, 30) }
413+
};
414+
415+
VerifyCSharpDiagnostic(test, expected);
416+
}
289417
}
290418
}

src/dotnet/Lucene.Net.Tests.CodeAnalysis/TestLucene1001_AddBaseMethodCallVBCodeFixProvider.cs

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,5 +141,37 @@ End Class
141141

142142
VerifyBasicDiagnostic(test, expected);
143143
}
144+
145+
[Test]
146+
public void TestBaseCallInsideLambda_Diagnostic()
147+
{
148+
// A base call inside an uninvoked lambda does not satisfy the contract.
149+
var test = @"
150+
Imports Lucene.Net.Analysis
151+
Imports System
152+
153+
Namespace MyNamespace
154+
NotInheritable Class TypeName
155+
Inherits TokenStream
156+
157+
Public Overrides Function IncrementToken() As Boolean
158+
Return False
159+
End Function
160+
161+
Public Overrides Sub Reset()
162+
Dim a As Action = Sub() MyBase.Reset()
163+
End Sub
164+
End Class
165+
End Namespace";
166+
var expected = new DiagnosticResult
167+
{
168+
Id = Lucene1001_TokenStreamOverrideMustCallBaseMethodVBAnalyzer.DiagnosticId,
169+
Message = "Override of 'Reset()' on type 'TypeName' must call MyBase.Reset().",
170+
Severity = DiagnosticSeverity.Warning,
171+
Locations = new[] { new DiagnosticResultLocation("Test0.vb", 13, 30) }
172+
};
173+
174+
VerifyBasicDiagnostic(test, expected);
175+
}
144176
}
145177
}

src/dotnet/Lucene.Net.Tests.CodeAnalysis/TestLucene1002_AddEndOverrideCSCodeFixProvider.cs

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,44 @@ sealed class TypeName : TokenStream
145145
VerifyCSharpDiagnostic(test);
146146
}
147147

148+
[Test]
149+
public void TestTokenizerMissingEndOverride_CodeFix()
150+
{
151+
var test = @"
152+
using Lucene.Net.Analysis;
153+
using System.IO;
154+
155+
namespace MyNamespace
156+
{
157+
sealed class TypeName : Tokenizer
158+
{
159+
public TypeName(TextReader input) : base(input) { }
160+
161+
public override bool IncrementToken() => false;
162+
}
163+
}";
164+
var fixtest = @"
165+
using Lucene.Net.Analysis;
166+
using System.IO;
167+
168+
namespace MyNamespace
169+
{
170+
sealed class TypeName : Tokenizer
171+
{
172+
public TypeName(TextReader input) : base(input) { }
173+
174+
public override bool IncrementToken() => false;
175+
176+
public override void End()
177+
{
178+
base.End();
179+
// TODO: set the final offset and finish up other end-of-stream attributes
180+
}
181+
}
182+
}";
183+
VerifyCSharpFix(test, fixtest);
184+
}
185+
148186
[Test]
149187
public void TestIndirectTokenizerInheritance_Diagnostic()
150188
{

0 commit comments

Comments
 (0)