Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Improve CA1305 for nullable types #7306

Merged
merged 1 commit into from
Aug 29, 2024
Merged

Improve CA1305 for nullable types #7306

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 3 additions & 20 deletions src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Runtime/SpecifyIFormatProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,17 +77,6 @@ public sealed class SpecifyIFormatProviderAnalyzer : AbstractGlobalizationDiagno

private static readonly ImmutableArray<string> s_dateInvariantFormats = ImmutableArray.Create("o", "O", "r", "R", "s", "u");

private static readonly ImmutableArray<SpecialType> s_numberTypes = ImmutableArray.Create(
SpecialType.System_Int32,
SpecialType.System_UInt32,
SpecialType.System_Int64,
SpecialType.System_UInt64,
SpecialType.System_Int16,
SpecialType.System_UInt16,
SpecialType.System_Double,
SpecialType.System_Single,
SpecialType.System_Decimal);

public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } = ImmutableArray.Create(IFormatProviderAlternateStringRule, IFormatProviderAlternateRule, IFormatProviderOptionalRule, UICultureStringRule, UICultureRule);

protected override void InitializeWorker(CompilationStartAnalysisContext context)
Expand All @@ -109,7 +98,6 @@ protected override void InitializeWorker(CompilationStartAnalysisContext context
var charType = context.Compilation.GetSpecialType(SpecialType.System_Char);
var boolType = context.Compilation.GetSpecialType(SpecialType.System_Boolean);
var guidType = typeProvider.GetOrCreateTypeByMetadataName(WellKnownTypeNames.SystemGuid);
var numberTypes = s_numberTypes.Select(context.Compilation.GetSpecialType).ToImmutableArray();

var nullableT = context.Compilation.GetSpecialType(SpecialType.System_Nullable_T);
var invariantToStringMethodsBuilder = ImmutableHashSet.CreateBuilder<IMethodSymbol>();
Expand Down Expand Up @@ -147,8 +135,6 @@ protected override void InitializeWorker(CompilationStartAnalysisContext context
GetParameterInfo(stringType),
GetParameterInfo(objectType, isArray: true, arrayRank: 1, isParams: true));

var currentCultureProperty = cultureInfoType.GetMembers("CurrentCulture").OfType<IPropertySymbol>().FirstOrDefault();
var invariantCultureProperty = cultureInfoType.GetMembers("InvariantCulture").OfType<IPropertySymbol>().FirstOrDefault();
var currentUICultureProperty = cultureInfoType.GetMembers("CurrentUICulture").OfType<IPropertySymbol>().FirstOrDefault();
var installedUICultureProperty = cultureInfoType.GetMembers("InstalledUICulture").OfType<IPropertySymbol>().FirstOrDefault();

Expand Down Expand Up @@ -213,7 +199,8 @@ protected override void InitializeWorker(CompilationStartAnalysisContext context
if (!oaContext.Options.IsConfiguredToSkipAnalysis(iformatProviderAlternateRule, targetMethod, oaContext.ContainingSymbol, oaContext.Compilation))
{
bool diagnosticReported = false;
IEnumerable<IMethodSymbol> methodsWithSameNameAsTargetMethod = targetMethod.ContainingType.GetMembers(targetMethod.Name).OfType<IMethodSymbol>().WhereMethodDoesNotContainAttribute(obsoleteAttributeType);
ITypeSymbol type = targetMethod.ContainingType.GetNullableValueTypeUnderlyingType() ?? targetMethod.ContainingType;
IEnumerable<IMethodSymbol> methodsWithSameNameAsTargetMethod = type.GetMembers(targetMethod.Name).OfType<IMethodSymbol>().WhereMethodDoesNotContainAttribute(obsoleteAttributeType);
if (methodsWithSameNameAsTargetMethod.HasMoreThan(1))
{
var correctOverloads = methodsWithSameNameAsTargetMethod.GetMethodOverloadsWithDesiredParameterAtLeadingOrTrailing(targetMethod, iformatProviderType);
Expand Down Expand Up @@ -245,11 +232,7 @@ protected override void InitializeWorker(CompilationStartAnalysisContext context
SymbolEqualityComparer.Default.Equals(x.Parameter?.Type, iformatProviderType)
&& x.ArgumentKind == ArgumentKind.DefaultValue);
var nullableType = invocationExpression.Instance?.Type.GetNullableValueTypeUnderlyingType();
var isDefaultToStringInvocation = invocationExpression.TargetMethod is { Name: nameof(object.ToString), Parameters.Length: 0 };
var isNullableNumberToStringInvocation = isDefaultToStringInvocation && numberTypes.Contains(nullableType, SymbolEqualityComparer.Default);
if (currentCallHasNullFormatProvider || isNullableNumberToStringInvocation)
if (currentCallHasNullFormatProvider)
{
oaContext.ReportDiagnostic(invocationExpression.CreateDiagnostic(IFormatProviderOptionalRule,
targetMethod.ToDisplayString(SymbolDisplayFormats.ShortSymbolDisplayFormat)));
Expand Down
85 changes: 59 additions & 26 deletions ...NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/Runtime/SpecifyIFormatProviderTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1599,16 +1599,32 @@ End Class
[InlineData("double")]
[InlineData("float")]
[InlineData("decimal")]
[InlineData("DateTime")]
[InlineData("DateOnly")]
[InlineData("TimeOnly")]
[InlineData("DateTimeOffset")]
public Task FormatProviderForNullableValueTypes(string valueType)
{
var code = $@"
public class Test {{
public void M({valueType}? x) {{
var y = {{|#0:x.ToString()|}};
}}
}}";
var code = $$"""
using System;

public class Test {
public void M({{valueType}}? x) {
var y = {|#0:x.ToString()|};
}
}
""";

DiagnosticResult result = new DiagnosticResult(SpecifyIFormatProviderAnalyzer.IFormatProviderAlternateRule)
.WithLocation(0)
.WithArguments($"{valueType}?.ToString()", $"Test.M({valueType}?)", $"{valueType}.ToString(IFormatProvider)");

return VerifyCS.VerifyAnalyzerAsync(code, new DiagnosticResult(SpecifyIFormatProviderAnalyzer.IFormatProviderOptionalRule).WithLocation(0).WithArguments($"{valueType}?.ToString()"));
return new VerifyCS.Test
{
TestCode = code,
ExpectedDiagnostics = { result },
ReferenceAssemblies = ReferenceAssemblies.Net.Net60
}.RunAsync();
}

[Theory, WorkItem(6746, "https://github.com/dotnet/roslyn-analyzers/issues/6586")]
Expand All @@ -1621,35 +1637,52 @@ public void M({valueType}? x) {{
[InlineData("double")]
[InlineData("float")]
[InlineData("decimal")]
[InlineData("DateTime")]
[InlineData("DateOnly")]
[InlineData("TimeOnly")]
[InlineData("DateTimeOffset")]
public Task FormatProviderForNullableValueTypesAlreadyProvided_NoDiagnostic(string valueType)
{
var code = $@"
using System.Globalization;
public class Test {{
public void M({valueType}? x) {{
var y = x?.ToString(CultureInfo.CurrentCulture);
}}
}}";

return VerifyCS.VerifyAnalyzerAsync(code);
var code = $$"""
using System;
using System.Globalization;

public class Test {
public void M({{valueType}}? x) {
var y = x?.ToString(CultureInfo.CurrentCulture);
}
}
""";

return new VerifyCS.Test
{
TestCode = code,
ReferenceAssemblies = ReferenceAssemblies.Net.Net60
}.RunAsync();
}

[Theory, WorkItem(6746, "https://github.com/dotnet/roslyn-analyzers/issues/6746")]
[CombinatorialData]
public Task FormatProviderForNullableValueTypes_NoDiagnostic(
[CombinatorialValues("int", "uint", "long", "ulong", "short", "ushort", "double", "float", "decimal")] string valueType,
[CombinatorialValues("int", "uint", "long", "ulong", "short", "ushort", "double", "float", "decimal", "DateTime", "DateOnly", "TimeOnly", "DateTimeOffset")] string valueType,
[CombinatorialValues("GetHashCode", "GetValueOrDefault")] string methodName
)
{
var code = $@"
public class Test {{
public void M({valueType}? x) {{
var y = x.{methodName}();
}}
}}";

return VerifyCS.VerifyAnalyzerAsync(code);
var code = $$"""
using System;

public class Test {
public void M({{valueType}}? x) {
var y = x.{{methodName}}();
}
}
""";

return new VerifyCS.Test
{
TestCode = code,
ReferenceAssemblies = ReferenceAssemblies.Net.Net60
}.RunAsync();
}

private DiagnosticResult GetIFormatProviderAlternateStringRuleCSharpResultAt(int line, int column, string arg1, string arg2, string arg3) =>
Expand Down