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.

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

Added support for StringSyntaxAttribute.CompositeFormat in CA2241 (#6… #6272

Merged
merged 5 commits into from
Jun 27, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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);
buyaa-n marked this conversation as resolved.
Show resolved Hide resolved
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)
}
}
}
}
}
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 =
{
@"
stephentoub marked this conversation as resolved.
Show resolved Hide resolved
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
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,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