Skip to content

Commit 2a4326a

Browse files
paulirwinclaude
andcommitted
Address Copilot review: TryFormat, null-provider, comment
Three fixes from PR #28 review: 1. LuceneDev2001 missed `int.TryFormat(buffer, out _)` because the resolved overload declares an *optional* IFormatProvider parameter, and the analyzer bailed whenever the signature had one. Switch to checking whether the invocation actually supplies a provider arg. 2. NumericTypeHelper.GetFormatProviderArgument returned null for `null` literal arguments because GetTypeInfo(...).Type is null for them, so calls like `i.ToString((string)null, null)` slipped past the 2007/2008 explicit-culture analyzer. Fall back to ConvertedType so overload-resolution-chosen parameter types are picked up. 3. Stale comment in LuceneDev2003 claimed using-static forms aren't handled, but the implementation already supports IdentifierNameSyntax and verifies the containing type is System.String. Comment updated. Adds three regression tests covering each fix. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent f42dfc9 commit 2a4326a

5 files changed

Lines changed: 81 additions & 4 deletions

File tree

src/Lucene.Net.CodeAnalysis.Dev/LuceneDev2xxx/LuceneDev2001_BclNumericToStringAnalyzer.cs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,9 @@ private static void AnalyzeInvocation(SyntaxNodeAnalysisContext ctx)
6464
if (!NumericTypeHelper.IsBclNumericSpecialType(containing))
6565
return;
6666

67-
if (NumericTypeHelper.HasFormatProviderParameter(method, semantic.Compilation))
67+
// Bail only when the call site actually supplies a provider argument; methods like
68+
// TryFormat declare an *optional* IFormatProvider parameter that callers often omit.
69+
if (NumericTypeHelper.GetFormatProviderArgument(invocation, semantic) is not null)
6870
return;
6971

7072
// Exempt parameterless ToString() inside a class's ToString() override —

src/Lucene.Net.CodeAnalysis.Dev/LuceneDev2xxx/LuceneDev2003_StringFormatNumericAnalyzer.cs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,9 @@ private static void AnalyzeInvocation(SyntaxNodeAnalysisContext ctx)
4343
{
4444
var invocation = (InvocationExpressionSyntax)ctx.Node;
4545

46-
// Match Format(...) on System.String. Don't bother with using-static or alias forms;
47-
// the canonical Lucene.NET style is string.Format(...).
46+
// Match Format(...) called as either `string.Format(...)` (member access) or `Format(...)`
47+
// (identifier — e.g. via `using static System.String;`). Containing-type check below
48+
// confirms the resolved method really is on System.String.
4849
string? methodName = invocation.Expression switch
4950
{
5051
MemberAccessExpressionSyntax m => m.Name.Identifier.ValueText,

src/Lucene.Net.CodeAnalysis.Dev/Utility/NumericTypeHelper.cs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,10 @@ public static bool HasFormatProviderParameter(IMethodSymbol? method, Compilation
134134

135135
foreach (var arg in invocation.ArgumentList.Arguments)
136136
{
137-
var argType = semanticModel.GetTypeInfo(arg.Expression).Type;
137+
// For literals like `null` or `default`, GetTypeInfo(...).Type is null but
138+
// ConvertedType reflects the parameter type chosen by overload resolution.
139+
var typeInfo = semanticModel.GetTypeInfo(arg.Expression);
140+
var argType = typeInfo.Type ?? typeInfo.ConvertedType;
138141
if (argType is null) continue;
139142
if (SymbolEqualityComparer.Default.Equals(argType, fpType))
140143
return arg.Expression;

tests/Lucene.Net.CodeAnalysis.Dev.Tests/LuceneDev2xxx/TestLuceneDev2001_BclNumericToStringAnalyzer.cs

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,51 @@ public class Sample
7474
await test.RunAsync();
7575
}
7676

77+
[Test]
78+
public async Task IntTryFormat_WithoutProvider_ReportsDiagnostic()
79+
{
80+
var testCode = @"
81+
using System;
82+
83+
public class Sample
84+
{
85+
public bool M(int i, Span<char> buffer) => i.TryFormat(buffer, out _);
86+
}";
87+
88+
var expected = new DiagnosticResult(Descriptors.LuceneDev2001_BclNumericToStringMissingFormatProvider)
89+
.WithSeverity(DiagnosticSeverity.Warning)
90+
.WithMessageFormat(Descriptors.LuceneDev2001_BclNumericToStringMissingFormatProvider.MessageFormat)
91+
.WithArguments("TryFormat", "Int32")
92+
.WithLocation("/0/Test0.cs", line: 6, column: 50);
93+
94+
var test = new InjectableCSharpAnalyzerTest(() => new LuceneDev2001_BclNumericToStringAnalyzer())
95+
{
96+
TestCode = testCode,
97+
ReferenceAssemblies = ReferenceAssemblies.Net.Net80,
98+
ExpectedDiagnostics = { expected }
99+
};
100+
await test.RunAsync();
101+
}
102+
103+
[Test]
104+
public async Task IntTryFormat_WithProvider_NoDiagnostic()
105+
{
106+
var testCode = @"
107+
using System;
108+
using System.Globalization;
109+
110+
public class Sample
111+
{
112+
public bool M(int i, Span<char> buffer) => i.TryFormat(buffer, out _, provider: CultureInfo.InvariantCulture);
113+
}";
114+
var test = new InjectableCSharpAnalyzerTest(() => new LuceneDev2001_BclNumericToStringAnalyzer())
115+
{
116+
TestCode = testCode,
117+
ReferenceAssemblies = ReferenceAssemblies.Net.Net80
118+
};
119+
await test.RunAsync();
120+
}
121+
77122
[Test]
78123
public async Task IntToString_WithProvider_NoDiagnostic()
79124
{

tests/Lucene.Net.CodeAnalysis.Dev.Tests/LuceneDev2xxx/TestLuceneDev2007_2008_NumericExplicitCultureAnalyzer.cs

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,32 @@ public class Sample
8181
await test.RunAsync();
8282
}
8383

84+
[Test]
85+
public async Task NullProviderLiteral_Reports2007()
86+
{
87+
// Regression: passing `null` for IFormatProvider should be treated as a non-invariant
88+
// explicit provider (it's effectively current-culture). The argument's TypeInfo.Type
89+
// is null for the `null` literal, so detection must fall back to ConvertedType.
90+
var testCode = @"
91+
public class Sample
92+
{
93+
public string M(int i) => i.ToString((string)null, null);
94+
}";
95+
96+
var expected = new DiagnosticResult(Descriptors.LuceneDev2007_NumericNonInvariantFormatProvider)
97+
.WithSeverity(DiagnosticSeverity.Warning)
98+
.WithMessageFormat(Descriptors.LuceneDev2007_NumericNonInvariantFormatProvider.MessageFormat)
99+
.WithArguments("ToString")
100+
.WithLocation("/0/Test0.cs", line: 4, column: 33);
101+
102+
var test = new InjectableCSharpAnalyzerTest(() => new LuceneDev2007_2008_NumericExplicitCultureAnalyzer())
103+
{
104+
TestCode = testCode,
105+
ExpectedDiagnostics = { expected }
106+
};
107+
await test.RunAsync();
108+
}
109+
84110
[Test]
85111
public async Task IsEnabledByDefault_2007True_2008False()
86112
{

0 commit comments

Comments
 (0)