Skip to content

Commit

Permalink
MA0011, MA0075, MA0076 better detects culture-sensitive operations (#496
Browse files Browse the repository at this point in the history
)
  • Loading branch information
meziantou authored Apr 13, 2023
1 parent 9376400 commit 7521e02
Showing 7 changed files with 182 additions and 155 deletions.
193 changes: 99 additions & 94 deletions src/Meziantou.Analyzer/Internals/CultureSensitiveFormattingContext.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
using System;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Operations;

namespace Meziantou.Analyzer.Internals;
@@ -45,60 +44,99 @@ public CultureSensitiveFormattingContext(Compilation compilation)
public INamedTypeSymbol? SystemWindowsFontStretchSymbol { get; }
public INamedTypeSymbol? SystemWindowsMediaBrushSymbol { get; }

public bool IsCultureSensitiveOperation(IOperation operation)
public bool IsCultureSensitiveOperation(IOperation operation, CultureSensitiveOptions options)
{
// Unwrap implicit conversion to Nullable<T>
if (options.HasFlag(CultureSensitiveOptions.UnwrapNullableOfT) && operation is IConversionOperation { Conversion.IsNullable: true, Operand: var conversionOperand })
{
operation = conversionOperand;
}

if (operation is IInvocationOperation invocation)
{
var methodName = invocation.TargetMethod.Name;
if (methodName is "ToString")
{
// Boolean.ToString(IFormatProvider) should not be used
if (invocation.TargetMethod.ContainingType.IsBoolean())
return false;

// Char.ToString(IFormatProvider) should not be used
if (invocation.TargetMethod.ContainingType.IsChar())
return false;

// Guid.ToString(IFormatProvider) should not be used
if (invocation.TargetMethod.ContainingType.IsEqualTo(GuidSymbol))
return false;

// Enum.ToString(IFormatProvider) should not be used
if (invocation.TargetMethod.ContainingType.IsEqualTo(EnumSymbol))
return false;

// DateTime.ToString() or DateTimeOffset.ToString() with invariant formats (o, O, r, R, s, u)
if (invocation.Arguments.Length == 1 && (invocation.TargetMethod.ContainingType.IsDateTime() || invocation.TargetMethod.ContainingType.IsEqualTo(DateTimeOffsetSymbol)))
// Try get the format. Most of ToString have only 1 string parameter to define the format
IOperation? format = null;
if (invocation.Arguments.Length > 0)
{
if (IsInvariantDateTimeFormat(invocation.Arguments[0].Value))
return false;
foreach (var arg in invocation.Arguments)
{
if (arg.Value is { ConstantValue: { HasValue: true, Value: string } })
{
if (format != null)
{
format = null;
break;
}

format = arg.Value;
}
}
}

return IsCultureSensitiveType(invocation.TargetMethod.ContainingType, format, instance: invocation.Instance, options);
}
else if (methodName is "Parse" or "TryParse")

if (methodName is "Parse" or "TryParse")
{
var type = invocation.TargetMethod.ContainingType;

// Guid.Parse / Guid.TryParse are culture insensitive
if (invocation.TargetMethod.ContainingType.IsEqualTo(GuidSymbol))
if (type.IsEqualTo(GuidSymbol))
return false;

// Char.Parse / Char.TryParse are culture insensitive
if (invocation.TargetMethod.ContainingType.IsChar())
if (type.IsChar())
return false;

return IsCultureSensitiveType(type, format: null, instance: null, options);
}
else if (methodName is "Append" or "AppendLine" && invocation.TargetMethod.ContainingType.IsEqualTo(StringBuilderSymbol))
{
// stringBuilder.AppendLine($"foo{bar}") when bar is a string
if (invocation.Arguments.Length == 1 && invocation.Arguments[0].Value.Type.IsEqualTo(StringBuilder_AppendInterpolatedStringHandlerSymbol) && !IsCultureSensitiveOperation(invocation.Arguments[0].Value))
// StringBuilder.AppendLine($"foo{bar}") when bar is a string
if (invocation.Arguments.Length == 1 && invocation.Arguments[0].Value.Type.IsEqualTo(StringBuilder_AppendInterpolatedStringHandlerSymbol) && !IsCultureSensitiveOperation(invocation.Arguments[0].Value, options))
return false;
}

return true;
}

#if CSHARP10_OR_GREATER
if (operation is IInterpolatedStringHandlerCreationOperation handler)
return IsCultureSensitiveOperation(handler.Content);
return IsCultureSensitiveOperation(handler.Content, options);

if (operation is IInterpolatedStringAdditionOperation interpolatedStringAddition)
return IsCultureSensitiveOperation(interpolatedStringAddition.Left) || IsCultureSensitiveOperation(interpolatedStringAddition.Right);
return IsCultureSensitiveOperation(interpolatedStringAddition.Left, options) || IsCultureSensitiveOperation(interpolatedStringAddition.Right, options);
#endif

if (operation is IInterpolationOperation content)
return IsCultureSensitiveType(content.Expression.Type, content.FormatString, content.Expression, options);

if (operation is IInterpolatedStringTextOperation)
return false;

#if CSHARP10_OR_GREATER
if (operation is IInterpolatedStringAppendOperation append)
{
if (append.AppendCall is IInvocationOperation appendInvocation)
{
if (appendInvocation.Arguments.Length == 1)
return IsCultureSensitiveType(appendInvocation.Arguments[0].Value.Type, format: null, instance: null, options);

if (appendInvocation.Arguments.Length == 2)
return IsCultureSensitiveType(appendInvocation.Arguments[0].Value.Type, format: appendInvocation.Arguments[1].Value, instance: null, options);

// Unknown case
return true;
}
else
{
// Unknown case
return true;
}
}
#endif

if (operation is IInterpolatedStringOperation interpolatedString)
@@ -108,85 +146,45 @@ public bool IsCultureSensitiveOperation(IOperation operation)

foreach (var part in interpolatedString.Parts)
{
if (part is IInterpolatedStringTextOperation)
continue;

if (part is IInterpolationOperation content)
{
if (content.Expression.Type.IsDateTime() || content.Expression.Type.IsEqualTo(DateTimeOffsetSymbol))
{
if (!IsInvariantDateTimeFormat(content.FormatString))
return true;
}
else if (IsCultureSensitiveType(content.Expression.GetActualType()))
{
return true;
}
}
#if CSHARP10_OR_GREATER
else if (part is IInterpolatedStringAppendOperation append)
{
if (append.AppendCall is IInvocationOperation appendInvocation)
{
if (appendInvocation.Arguments.Length == 1)
{
if (IsCultureSensitiveType(appendInvocation.Arguments[0].Value.Type))
return true;
}
else if (appendInvocation.Arguments.Length == 2)
{
var expression = appendInvocation.Arguments[0].Value;
if (expression.Type.IsDateTime() || expression.Type.IsEqualTo(DateTimeOffsetSymbol))
{
if (!IsInvariantDateTimeFormat(appendInvocation.Arguments[1].Value))
return true;
}
else
{
return true;
}
}
else
{
return true;
}
}
else
{
return true;
}
}
#endif
else
{
if (IsCultureSensitiveOperation(part, options))
return true;
}
}

return false;
}

if (operation is ILocalReferenceOperation localReference)
return IsCultureSensitiveType(localReference.Type);
return IsCultureSensitiveType(localReference.Type, options);

if (operation is IParameterReferenceOperation parameterReference)
return IsCultureSensitiveType(parameterReference.Type);
return IsCultureSensitiveType(parameterReference.Type, options);

if (operation is IMemberReferenceOperation memberReference)
return IsCultureSensitiveType(memberReference.Type);
return IsCultureSensitiveType(memberReference.Type, options);

if (operation is ILiteralOperation literal)
return IsCultureSensitiveType(literal.Type);
return IsCultureSensitiveType(literal.Type, format: null, literal, options);

if (operation is IConversionOperation conversion)
return IsCultureSensitiveType(conversion.Type, format: null, instance: operation, options);

if (operation is IObjectCreationOperation objectCreation)
return IsCultureSensitiveType(objectCreation.Type, format: null, instance: null, options);

// Unknown operation
return true;
}

public bool IsCultureSensitiveType(ITypeSymbol? typeSymbol)
private bool IsCultureSensitiveType(ITypeSymbol? typeSymbol, CultureSensitiveOptions options)
{
if (typeSymbol == null)
return true;

if (options.HasFlag(CultureSensitiveOptions.UnwrapNullableOfT))
{
typeSymbol = typeSymbol.GetUnderlyingNullableType();
}

if (typeSymbol.IsEnumeration())
return false;

@@ -217,9 +215,6 @@ public bool IsCultureSensitiveType(ITypeSymbol? typeSymbol)
if (typeSymbol.IsEqualTo(UInt128Symbol))
return false;

if (typeSymbol.IsEqualTo(TimeSpanSymbol))
return false;

if (typeSymbol.IsEqualTo(GuidSymbol))
return false;

@@ -235,9 +230,9 @@ public bool IsCultureSensitiveType(ITypeSymbol? typeSymbol)
return typeSymbol.Implements(SystemIFormattableSymbol);
}

public bool IsCultureSensitiveType(ITypeSymbol? symbol, IOperation? format, IOperation? instance = null)
private bool IsCultureSensitiveType(ITypeSymbol? symbol, IOperation? format, IOperation? instance, CultureSensitiveOptions options)
{
if (!IsCultureSensitiveType(symbol))
if (!IsCultureSensitiveType(symbol, options))
return false;

if (instance != null)
@@ -246,12 +241,16 @@ public bool IsCultureSensitiveType(ITypeSymbol? symbol, IOperation? format, IOpe
return false;
}

var isDateTime = symbol.IsDateTime() || symbol.IsEqualToAny(DateTimeOffsetSymbol, DateOnlySymbol, TimeOnlySymbol);
if (isDateTime)
if (symbol.IsDateTime() || symbol.IsEqualToAny(DateTimeOffsetSymbol, DateOnlySymbol, TimeOnlySymbol))
{
if (IsInvariantDateTimeFormat(format))
return false;
}
else if (symbol.IsEqualTo(TimeSpanSymbol))
{
if (IsInvariantTimeSpanFormat(format))
return false;
}

return true;
}
@@ -261,12 +260,18 @@ private static bool IsInvariantDateTimeFormat(IOperation? valueOperation)
return valueOperation is { ConstantValue: { HasValue: true, Value: "o" or "O" or "r" or "R" or "s" or "u" } };
}

private static bool IsInvariantTimeSpanFormat(IOperation? valueOperation)
{
return valueOperation is { ConstantValue: { HasValue: true, Value: "c" or "C" } };
}

// Only negative numbers are culture-sensitive (negative sign)
// For instance, https://source.dot.net/#System.Private.CoreLib/Int32.cs,8d6f2d8bc0589463
private static bool IsConstantPositiveNumber(IOperation operation)
{
if (operation.Type != null && operation.ConstantValue.HasValue)
{
// Only consider types where ToString() is culture-insensitive for positive values
var constantValue = operation.ConstantValue.Value;
bool? result = operation.Type.SpecialType switch
{
10 changes: 10 additions & 0 deletions src/Meziantou.Analyzer/Internals/CultureSensitiveOptions.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
using System;

namespace Meziantou.Analyzer.Internals;

[Flags]
internal enum CultureSensitiveOptions
{
None,
UnwrapNullableOfT,
}
10 changes: 10 additions & 0 deletions src/Meziantou.Analyzer/Internals/OperationExtensions.cs
Original file line number Diff line number Diff line change
@@ -145,6 +145,16 @@ public static IOperation UnwrapImplicitConversionOperations(this IOperation oper

return operation;
}

public static IOperation UnwrapConversionOperations(this IOperation operation)
{
if (operation is IConversionOperation conversionOperation)
{
return UnwrapImplicitConversionOperations(conversionOperation.Operand);
}

return operation;
}

public static bool HasArgumentOfType(this IInvocationOperation operation, ITypeSymbol argumentTypeSymbol)
{
Original file line number Diff line number Diff line change
@@ -120,6 +120,8 @@ public void AnalyzeInterpolatedString(OperationAnalysisContext context)
if (IsExcludedMethod(context, s_stringInterpolationRule, operation))
return;

var unwrapNullable = MustUnwrapNullableTypes(context, s_stringInterpolationRule, operation) ? CultureSensitiveOptions.UnwrapNullableOfT : CultureSensitiveOptions.None;

var parent = operation.Parent;
if (parent is IConversionOperation conversionOperation)
{
@@ -135,12 +137,7 @@ public void AnalyzeInterpolatedString(OperationAnalysisContext context)
if (expression == null || type == null)
continue;

if (MustUnwrapNullableTypes(context, s_stringInterpolationRule, operation))
{
type = type.GetUnderlyingNullableType();
}

if (_cultureSensitiveContext.IsCultureSensitiveType(type, format: part.FormatString, instance: expression))
if (_cultureSensitiveContext.IsCultureSensitiveOperation(part, unwrapNullable))
{
context.ReportDiagnostic(s_stringInterpolationRule, part);
}
@@ -167,17 +164,8 @@ private bool IsNonCultureSensitiveOperand(OperationAnalysisContext context, Diag
if (operand is IConversionOperation conversion && conversion.IsImplicit && conversion.Type.IsObject() && conversion.Operand.Type != null)
{
var value = conversion.Operand;
var type = value.Type;
if (MustUnwrapNullableTypes(context, rule, operand))
{
type = type.GetUnderlyingNullableType();
if (conversion.Operand is IConversionOperation { Conversion.IsNullable: true } operandConversion)
{
value = operandConversion.Operand;
}
}

if (_cultureSensitiveContext.IsCultureSensitiveType(type, format: null, instance: value))
var options = MustUnwrapNullableTypes(context, rule, operand) ? CultureSensitiveOptions.UnwrapNullableOfT : CultureSensitiveOptions.None;
if (_cultureSensitiveContext.IsCultureSensitiveOperation(value, options))
return false;
}

Loading
Oops, something went wrong.

0 comments on commit 7521e02

Please sign in to comment.