Skip to content

Commit

Permalink
Added support for StringSyntaxAttribute.CompositeFormat in CA2241 (#6… (
Browse files Browse the repository at this point in the history 
#6272)

* Added support for StringSyntaxAttribute.CompositeFormat in CA2241 (#6021)

* Use a ConcurrentDictionary for methods investigated.

This way we only need to check every method definition once.
  • Loading branch information
@manfred-brands
manfred-brands authored Jun 27, 2023
1 parent 8e86168 commit 03fd0f4
Show file tree
Hide file tree
Showing 3 changed files with 129 additions and 24 deletions.
87 changes: 64 additions & 23 deletions ...rs/Core/Microsoft.NetCore.Analyzers/Runtime/ProvideCorrectArgumentsToFormattingMethods.cs
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the MIT license. See License.txt in the project root for license information.

using System;
using System.Collections.Concurrent;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Diagnostics.CodeAnalysis;
Expand Down Expand Up @@ -55,14 +56,13 @@ public override void Initialize(AnalysisContext context)
}
IArgumentOperation formatStringArgument = invocation.Arguments[info.FormatStringIndex];
if (!Equals(formatStringArgument?.Value?.Type, formatInfo.String) ||
!(formatStringArgument?.Value?.ConstantValue.Value is string))
if (!Equals(formatStringArgument.Value.Type, formatInfo.String) ||
!(formatStringArgument.Value.ConstantValue.Value is string stringFormat))
{
// wrong argument
// wrong argument or not a constant
return;
}
var stringFormat = (string)formatStringArgument.Value.ConstantValue.Value;
int expectedStringFormatArgumentCount = GetFormattingArguments(stringFormat);
// explicit parameter case
Expand Down Expand Up @@ -315,28 +315,29 @@ private static int GetFormattingArguments(string format)
private class StringFormatInfo
{
private const string Format = "format";
private const string CompositeFormat = "CompositeFormat";

private readonly ImmutableDictionary<IMethodSymbol, Info> _map;
private readonly ConcurrentDictionary<IMethodSymbol, Info?> _map;

public StringFormatInfo(Compilation compilation)
{
ImmutableDictionary<IMethodSymbol, Info>.Builder builder = ImmutableDictionary.CreateBuilder<IMethodSymbol, Info>();
_map = new ConcurrentDictionary<IMethodSymbol, Info?>();

INamedTypeSymbol? console = compilation.GetOrCreateTypeByMetadataName(WellKnownTypeNames.SystemConsole);
AddStringFormatMap(builder, console, "Write");
AddStringFormatMap(builder, console, "WriteLine");
AddStringFormatMap(console, "Write");
AddStringFormatMap(console, "WriteLine");

INamedTypeSymbol @string = compilation.GetSpecialType(SpecialType.System_String);
AddStringFormatMap(builder, @string, "Format");

_map = builder.ToImmutable();
AddStringFormatMap(@string, "Format");

String = @string;
Object = compilation.GetSpecialType(SpecialType.System_Object);
StringSyntaxAttribute = compilation.GetOrCreateTypeByMetadataName(WellKnownTypeNames.SystemDiagnosticsCodeAnalysisStringSyntaxAttributeName);
}

public INamedTypeSymbol String { get; }
public INamedTypeSymbol Object { get; }
public INamedTypeSymbol? StringSyntaxAttribute { get; }

public Info? TryGet(IMethodSymbol method, OperationAnalysisContext context)
{
Expand All @@ -345,10 +346,16 @@ public StringFormatInfo(Compilation compilation)
return info;
}

if (StringSyntaxAttribute is not null &&
TryGetFormatInfoByCompositeFormatStringSyntaxAttribute(method, out info))
{
return info;
}

// Check if this the underlying method is user configured string formatting method.
var additionalStringFormatMethodsOption = context.Options.GetAdditionalStringFormattingMethodsOption(Rule, context.Operation.Syntax.SyntaxTree, context.Compilation);
if (additionalStringFormatMethodsOption.Contains(method.OriginalDefinition) &&
TryGetFormatInfo(method, out info))
TryGetFormatInfoByParameterName(method, out info))
{
return info;
}
Expand All @@ -358,16 +365,17 @@ public StringFormatInfo(Compilation compilation)
var determineAdditionalStringFormattingMethodsAutomatically = context.Options.GetBoolOptionValue(EditorConfigOptionNames.TryDetermineAdditionalStringFormattingMethodsAutomatically,
Rule, context.Operation.Syntax.SyntaxTree, context.Compilation, defaultValue: false);
if (determineAdditionalStringFormattingMethodsAutomatically &&
TryGetFormatInfo(method, out info) &&
TryGetFormatInfoByParameterName(method, out info) &&
info.ExpectedStringFormatArgumentCount == -1)
{
return info;
}

_map.TryAdd(method, null);
return null;
}

private static void AddStringFormatMap(ImmutableDictionary<IMethodSymbol, Info>.Builder builder, INamedTypeSymbol? type, string methodName)
private void AddStringFormatMap(INamedTypeSymbol? type, string methodName)
{
if (type == null)
{
Expand All @@ -376,19 +384,27 @@ private static void AddStringFormatMap(ImmutableDictionary<IMethodSymbol, Info>.

foreach (IMethodSymbol method in type.GetMembers(methodName).OfType<IMethodSymbol>())
{
if (TryGetFormatInfo(method, out var formatInfo))
{
builder.Add(method, formatInfo);
}
TryGetFormatInfoByParameterName(method, out var _);
}
}

private static bool TryGetFormatInfo(IMethodSymbol method, [NotNullWhen(returnValue: true)] out Info? formatInfo)
private bool TryGetFormatInfoByCompositeFormatStringSyntaxAttribute(IMethodSymbol method, [NotNullWhen(returnValue: true)] out Info? formatInfo)
{
int formatIndex = FindParameterIndexOfCompositeFormatStringSyntaxAttribute(method.Parameters);
return TryGetFormatInfo(method, formatIndex, out formatInfo);
}

private bool TryGetFormatInfoByParameterName(IMethodSymbol method, [NotNullWhen(returnValue: true)] out Info? formatInfo)
{
int formatIndex = FindParameterIndexByParameterName(method.Parameters, Format);
return TryGetFormatInfo(method, formatIndex, out formatInfo);
}

private bool TryGetFormatInfo(IMethodSymbol method, int formatIndex, [NotNullWhen(returnValue: true)] out Info? formatInfo)
{
formatInfo = default;

int formatIndex = FindParameterIndexOfName(method.Parameters, Format);
if (formatIndex < 0 || formatIndex == method.Parameters.Length - 1)
if (formatIndex < 0)
{
// no valid format string
return false;
Expand All @@ -402,6 +418,8 @@ private static bool TryGetFormatInfo(IMethodSymbol method, [NotNullWhen(returnVa

int expectedArguments = GetExpectedNumberOfArguments(method.Parameters, formatIndex);
formatInfo = new Info(formatIndex, expectedArguments);
_map.TryAdd(method, formatInfo);

return true;
}

Expand All @@ -417,7 +435,7 @@ private static int GetExpectedNumberOfArguments(ImmutableArray<IParameterSymbol>
return parameters.Length - formatIndex - 1;
}

private static int FindParameterIndexOfName(ImmutableArray<IParameterSymbol> parameters, string name)
private static int FindParameterIndexByParameterName(ImmutableArray<IParameterSymbol> parameters, string name)
{
for (var i = 0; i < parameters.Length; i++)
{
Expand All @@ -430,6 +448,29 @@ private static int FindParameterIndexOfName(ImmutableArray<IParameterSymbol> par
return -1;
}

private int FindParameterIndexOfCompositeFormatStringSyntaxAttribute(ImmutableArray<IParameterSymbol> parameters)
{
for (var i = 0; i < parameters.Length; i++)
{
if (String.Equals(parameters[i].Type, SymbolEqualityComparer.Default))
{
foreach (AttributeData attribute in parameters[i].GetAttributes())
{
if (StringSyntaxAttribute!.Equals(attribute.AttributeClass, SymbolEqualityComparer.Default))
{
ImmutableArray<TypedConstant> arguments = attribute.ConstructorArguments;
if (arguments.Length == 1 && CompositeFormat.Equals(arguments[0].Value))
{
return i;
}
}
}
}
}

return -1;
}

public class Info
{
public Info(int formatIndex, int expectedArguments)
Expand All @@ -443,4 +484,4 @@ public Info(int formatIndex, int expectedArguments)
}
}
}
}
}
65 changes: 64 additions & 1 deletion ...ts/Microsoft.NetCore.Analyzers/Runtime/ProvideCorrectArgumentsToFormattingMethodsTests.cs
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the MIT license. See License.txt in the project root for license information.
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the MIT license. See License.txt in the project root for license information.

using System.Threading.Tasks;
using Microsoft.CodeAnalysis.Testing;
Expand Down Expand Up @@ -510,6 +510,69 @@ End Class"
await basicTest.RunAsync();
}

[Fact]
[WorkItem(6012, "https://github.com/dotnet/roslyn-analyzers/issues/6012")]
public async Task EditorConfigConfiguration_StringSyntaxAnnotatedMethodsAsync()
{
var csharpTest = new VerifyCS.Test
{
TestState =
{
Sources =
{
@"
using System.Diagnostics.CodeAnalysis;
class Test
{
public static string MyFormat([StringSyntax(StringSyntaxAttribute.CompositeFormat)] string specification, params object[] args) => specification;
void M1(string param)
{
var a = MyFormat("""", 1);
}
}"
},
ReferenceAssemblies = ReferenceAssemblies.Net.Net70,
}
};

csharpTest.ExpectedDiagnostics.Add(
// Test0.cs(10,17): warning CA2241: Provide correct arguments to formatting methods
GetCSharpResultAt(10, 17));

await csharpTest.RunAsync();

var basicTest = new VerifyVB.Test
{
TestState =
{
Sources =
{
@"
Imports System.Diagnostics.CodeAnalysis
Class Test
Public Shared Function MyFormat(<StringSyntax(StringSyntaxAttribute.CompositeFormat)> specification As String, ParamArray args As Object()) As String
Return specification
End Function
Private Sub M1(ByVal param As String)
Dim a = MyFormat("""", 1)
End Sub
End Class"
},
ReferenceAssemblies = ReferenceAssemblies.Net.Net70,
}
};

basicTest.ExpectedDiagnostics.Add(
// Test0.vb(10,17): warning CA2241: Provide correct arguments to formatting methods
GetBasicResultAt(10, 17));

await basicTest.RunAsync();
}

#endregion

private static DiagnosticResult GetCSharpResultAt(int line, int column)
Expand Down
1 change: 1 addition & 0 deletions src/Utilities/Compiler/WellKnownTypeNames.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,7 @@ internal static class WellKnownTypeNames
public const string SystemDiagnosticsCodeAnalysisConstantExpectedAttribute = "System.Diagnostics.CodeAnalysis.ConstantExpectedAttribute";
public const string SystemDiagnosticsCodeAnalysisNotNullAttribute = "System.Diagnostics.CodeAnalysis.NotNullAttribute";
public const string SystemDiagnosticsCodeAnalysisNotNullIfNotNullAttribute = "System.Diagnostics.CodeAnalysis.NotNullIfNotNullAttribute";
public const string SystemDiagnosticsCodeAnalysisStringSyntaxAttributeName = "System.Diagnostics.CodeAnalysis.StringSyntaxAttribute";
public const string SystemDiagnosticsConditionalAttribute = "System.Diagnostics.ConditionalAttribute";
public const string SystemDiagnosticsContractsPureAttribute = "System.Diagnostics.Contracts.PureAttribute";
public const string SystemDiagnosticsDebug = "System.Diagnostics.Debug";
Expand Down

0 comments on commit 03fd0f4

Please sign in to comment.