Skip to content

Commit 729e5b0

Browse files
meziantoumeziantou
committed
Improve support for culture-sensitive ToString and support null format
1 parent ce060a1 commit 729e5b0

File tree

4 files changed

+195
-586
lines changed

4 files changed

+195
-586
lines changed

‎src/Meziantou.Analyzer/Internals/CultureSensitiveFormattingContext.cs‎

Lines changed: 43 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ namespace Meziantou.Analyzer.Internals;
55

66
internal sealed class CultureSensitiveFormattingContext(Compilation compilation)
77
{
8+
private readonly HashSet<ISymbol> _excludedMethods = CreateExcludedMethods(compilation);
9+
810
public INamedTypeSymbol? CultureInsensitiveTypeAttributeSymbol { get; } = compilation.GetBestTypeByMetadataName("Meziantou.Analyzer.Annotations.CultureInsensitiveTypeAttribute");
911
public INamedTypeSymbol? FormatProviderSymbol { get; } = compilation.GetBestTypeByMetadataName("System.IFormatProvider");
1012
public INamedTypeSymbol? CultureInfoSymbol { get; } = compilation.GetBestTypeByMetadataName("System.Globalization.CultureInfo");
@@ -24,6 +26,26 @@ internal sealed class CultureSensitiveFormattingContext(Compilation compilation)
2426
public INamedTypeSymbol? SystemIFormattableSymbol { get; } = compilation.GetBestTypeByMetadataName("System.IFormattable");
2527
public INamedTypeSymbol? SystemWindowsFontStretchSymbol { get; } = compilation.GetBestTypeByMetadataName("System.Windows.FontStretch");
2628
public INamedTypeSymbol? SystemWindowsMediaBrushSymbol { get; } = compilation.GetBestTypeByMetadataName("System.Windows.Media.Brush");
29+
public INamedTypeSymbol? NuGetVersioningSemanticVersionSymbol { get; } = compilation.GetBestTypeByMetadataName("NuGet.Versioning.SemanticVersion");
30+
31+
private static HashSet<ISymbol> CreateExcludedMethods(Compilation compilation)
32+
{
33+
var result = new HashSet<ISymbol>(SymbolEqualityComparer.Default);
34+
AddDocumentationId(result, compilation, "M:System.Convert.ToChar(System.String)");
35+
AddDocumentationId(result, compilation, "M:System.Convert.ToChar(System.Object)");
36+
AddDocumentationId(result, compilation, "M:System.Convert.ToBoolean(System.String)");
37+
AddDocumentationId(result, compilation, "M:System.Convert.ToBoolean(System.Object)");
38+
return result;
39+
40+
static void AddDocumentationId(HashSet<ISymbol> result, Compilation compilation, string id)
41+
{
42+
foreach (var item in DocumentationCommentId.GetSymbolsForDeclarationId(id, compilation))
43+
{
44+
result.Add(item);
45+
}
46+
}
47+
}
48+
2749

2850
private static bool MustUnwrapNullableOfT(CultureSensitiveOptions options)
2951
{
@@ -40,6 +62,9 @@ public bool IsCultureSensitiveOperation(IOperation operation, CultureSensitiveOp
4062

4163
if (operation is IInvocationOperation invocation)
4264
{
65+
if (_excludedMethods.Contains(invocation.TargetMethod))
66+
return false;
67+
4368
var methodName = invocation.TargetMethod.Name;
4469
if (methodName is "ToString")
4570
{
@@ -49,7 +74,7 @@ public bool IsCultureSensitiveOperation(IOperation operation, CultureSensitiveOp
4974
{
5075
foreach (var arg in invocation.Arguments)
5176
{
52-
if (arg.Value is { ConstantValue: { HasValue: true, Value: string } })
77+
if (arg.Value is { ConstantValue: { HasValue: true, Value: string } } or IConversionOperation { Type.SpecialType: SpecialType.System_String, ConstantValue: { HasValue: true, Value: null } })
5378
{
5479
if (format is not null)
5580
{
@@ -252,25 +277,31 @@ private bool IsCultureSensitiveType(ITypeSymbol? typeSymbol, CultureSensitiveOpt
252277
if (typeSymbol.IsOrInheritFrom(SystemWindowsMediaBrushSymbol))
253278
return false;
254279

280+
if (typeSymbol.IsOrInheritFrom(NuGetVersioningSemanticVersionSymbol))
281+
return false;
282+
255283
if (!typeSymbol.Implements(SystemIFormattableSymbol))
256284
return false;
257285

258-
if (!IsCultureSensitiveTypeUsingAttribute(typeSymbol, format: null))
286+
if (!IsCultureSensitiveTypeUsingAttribute(typeSymbol, hasFormat: false, format: null))
259287
return false;
260288

261289
return true;
262290
}
263291

264-
private bool IsCultureSensitiveTypeUsingAttribute(ITypeSymbol typeSymbol, string? format)
292+
private bool IsCultureSensitiveTypeUsingAttribute(ITypeSymbol typeSymbol, bool hasFormat, string? format)
265293
{
266294
var attributes = typeSymbol.GetAttributes(CultureInsensitiveTypeAttributeSymbol);
267295
foreach (var attr in attributes)
268296
{
269297
if (attr.ConstructorArguments.IsEmpty)
270298
return false; // no format is set, so the type is culture insensitive
271299

272-
var attrFormat = attr.ConstructorArguments[0].Value;
273-
if (attrFormat is null || (attrFormat is string attrFormatValue && (string.IsNullOrEmpty(attrFormatValue) || attrFormatValue == format)))
300+
if (!hasFormat)
301+
continue;
302+
303+
var attrFormat = attr.ConstructorArguments[0].Value as string;
304+
if (attrFormat == format)
274305
return false; // no format is set, so the type is culture insensitive
275306
}
276307

@@ -284,8 +315,11 @@ private bool IsCultureSensitiveTypeUsingAttribute(ITypeSymbol typeSymbol, string
284315
if (attribute.ConstructorArguments.Length == 1)
285316
return false;
286317

287-
var attrFormat = attribute.ConstructorArguments[1].Value;
288-
if (attrFormat is null || (attrFormat is string attrFormatValue && (string.IsNullOrEmpty(attrFormatValue) || attrFormatValue == format)))
318+
if (!hasFormat)
319+
continue;
320+
321+
var attrFormat = attribute.ConstructorArguments[1].Value as string;
322+
if (attrFormat == format)
289323
return false; // no format is set, so the type is culture insensitive
290324
}
291325
}
@@ -298,6 +332,7 @@ private bool IsCultureSensitiveType(ITypeSymbol? symbol, IOperation? format, IOp
298332
if (!IsCultureSensitiveType(symbol, options))
299333
return false;
300334

335+
var hasFormatString = format is { ConstantValue.HasValue: true };
301336
var formatString = format?.ConstantValue.Value as string;
302337

303338
if (instance is not null)
@@ -320,7 +355,7 @@ private bool IsCultureSensitiveType(ITypeSymbol? symbol, IOperation? format, IOp
320355
return false;
321356
}
322357

323-
if (symbol is not null && !IsCultureSensitiveTypeUsingAttribute(symbol, formatString))
358+
if (symbol is not null && !IsCultureSensitiveTypeUsingAttribute(symbol, hasFormatString, formatString))
324359
return false;
325360

326361
return true;

‎src/Meziantou.Analyzer/Rules/UseIFormatProviderAnalyzer.cs‎

Lines changed: 12 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -37,25 +37,6 @@ private sealed class AnalyzerContext(Compilation compilation)
3737
{
3838
private readonly CultureSensitiveFormattingContext _cultureSensitiveContext = new(compilation);
3939
private readonly OverloadFinder _overloadFinder = new(compilation);
40-
private readonly HashSet<ISymbol> _excludedMethods = CreateExcludedMethods(compilation);
41-
42-
private static HashSet<ISymbol> CreateExcludedMethods(Compilation compilation)
43-
{
44-
var result = new HashSet<ISymbol>(SymbolEqualityComparer.Default);
45-
AddDocumentationId(result, compilation, "M:System.Convert.ToChar(System.String)");
46-
AddDocumentationId(result, compilation, "M:System.Convert.ToChar(System.Object)");
47-
AddDocumentationId(result, compilation, "M:System.Convert.ToBoolean(System.String)");
48-
AddDocumentationId(result, compilation, "M:System.Convert.ToBoolean(System.Object)");
49-
return result;
50-
51-
static void AddDocumentationId(HashSet<ISymbol> result, Compilation compilation, string id)
52-
{
53-
foreach (var item in DocumentationCommentId.GetSymbolsForDeclarationId(id, compilation))
54-
{
55-
result.Add(item);
56-
}
57-
}
58-
}
5940

6041
public void AnalyzeInvocation(OperationAnalysisContext context)
6142
{
@@ -65,7 +46,7 @@ public void AnalyzeInvocation(OperationAnalysisContext context)
6546

6647
if (IsExcludedMethod(context, operation))
6748
return;
68-
49+
6950
var options = MustUnwrapNullableTypes(context, operation) ? CultureSensitiveOptions.UnwrapNullableOfT : CultureSensitiveOptions.None;
7051
if (!_cultureSensitiveContext.IsCultureSensitiveOperation(operation, options))
7152
return;
@@ -81,7 +62,11 @@ public void AnalyzeInvocation(OperationAnalysisContext context)
8162
var overload = _overloadFinder.FindOverloadWithAdditionalParameterOfType(operation.TargetMethod, operation, includeObsoleteMethods: false, allowOptionalParameters: false, _cultureSensitiveContext.FormatProviderSymbol);
8263
if (overload is not null)
8364
{
84-
context.ReportDiagnostic(Rule, operation, operation.TargetMethod.Name, _cultureSensitiveContext.FormatProviderSymbol.ToDisplayString());
65+
if (_cultureSensitiveContext.IsCultureSensitiveOperation(operation, CultureSensitiveOptions.None))
66+
{
67+
context.ReportDiagnostic(Rule, operation, operation.TargetMethod.Name, _cultureSensitiveContext.FormatProviderSymbol.ToDisplayString());
68+
}
69+
8570
return;
8671
}
8772

@@ -108,17 +93,18 @@ public void AnalyzeInvocation(OperationAnalysisContext context)
10893
var overload = _overloadFinder.FindOverloadWithAdditionalParameterOfType(operation.TargetMethod, includeObsoleteMethods: false, allowOptionalParameters: false, _cultureSensitiveContext.CultureInfoSymbol);
10994
if (overload is not null)
11095
{
111-
context.ReportDiagnostic(Rule, operation, operation.TargetMethod.Name, _cultureSensitiveContext.CultureInfoSymbol.ToDisplayString());
96+
if (_cultureSensitiveContext.IsCultureSensitiveOperation(operation, CultureSensitiveOptions.None))
97+
{
98+
context.ReportDiagnostic(Rule, operation, operation.TargetMethod.Name, _cultureSensitiveContext.CultureInfoSymbol.ToDisplayString());
99+
}
100+
112101
return;
113102
}
114103
}
115104
}
116105

117-
private bool IsExcludedMethod(OperationAnalysisContext context, IInvocationOperation operation)
106+
private static bool IsExcludedMethod(OperationAnalysisContext context, IInvocationOperation operation)
118107
{
119-
if (_excludedMethods.Contains(operation.TargetMethod))
120-
return true;
121-
122108
// ToString show culture-sensitive data by default
123109
if (operation?.GetContainingMethod(context.CancellationToken)?.Name == "ToString")
124110
{

‎tests/Meziantou.Analyzer.Test/Rules/DoNotUseImplicitCultureSensitiveToStringAnalyzerTests.cs‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -357,6 +357,34 @@ await CreateProjectBuilder()
357357
.ValidateAsync();
358358
}
359359

360+
[Fact]
361+
public async Task ToString_IgnoreTypeUsingAssemblyAttribute()
362+
{
363+
var sourceCode = """
364+
[assembly: Meziantou.Analyzer.Annotations.CultureInsensitiveTypeAttribute(typeof(System.DateTime))]
365+
_ = "" + new System.DateTime().ToString();
366+
""";
367+
await CreateProjectBuilder()
368+
.WithSourceCode(sourceCode)
369+
.WithOutputKind(Microsoft.CodeAnalysis.OutputKind.ConsoleApplication)
370+
.ValidateAsync();
371+
}
372+
373+
[Fact]
374+
public async Task ToString_IgnoreTypeUsingAssemblyAttribute_SpecificFormat()
375+
{
376+
var sourceCode = """
377+
[assembly: Meziantou.Analyzer.Annotations.CultureInsensitiveTypeAttribute(typeof(System.DateTime), "test")]
378+
_ = "" + new System.DateTime().ToString("test");
379+
_ = "" + [|new System.DateTime().ToString((string?)null)|];
380+
_ = "" + [|new System.DateTime().ToString("")|];
381+
""";
382+
await CreateProjectBuilder()
383+
.WithSourceCode(sourceCode)
384+
.WithOutputKind(Microsoft.CodeAnalysis.OutputKind.ConsoleApplication)
385+
.ValidateAsync();
386+
}
387+
360388
[Fact]
361389
public async Task IgnoreTypeUsingAssemblyAttribute()
362390
{

0 commit comments

Comments
 (0)