From 03fd0f4c310c8104f131488a622d74d2d6c9ae3c Mon Sep 17 00:00:00 2001 From: Manfred Brands Date: Tue, 27 Jun 2023 21:11:31 +0200 Subject: [PATCH] =?UTF-8?q?Added=20support=20for=20StringSyntaxAttribute.C?= =?UTF-8?q?ompositeFormat=20in=20CA2241=20(#6=E2=80=A6=20(#6272)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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. --- ...videCorrectArgumentsToFormattingMethods.cs | 87 ++++++++++++++----- ...orrectArgumentsToFormattingMethodsTests.cs | 65 +++++++++++++- src/Utilities/Compiler/WellKnownTypeNames.cs | 1 + 3 files changed, 129 insertions(+), 24 deletions(-) diff --git a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Runtime/ProvideCorrectArgumentsToFormattingMethods.cs b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Runtime/ProvideCorrectArgumentsToFormattingMethods.cs index a2760e2c9e..2d5d0cb7c7 100644 --- a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Runtime/ProvideCorrectArgumentsToFormattingMethods.cs +++ b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Runtime/ProvideCorrectArgumentsToFormattingMethods.cs @@ -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; @@ -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 @@ -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 _map; + private readonly ConcurrentDictionary _map; public StringFormatInfo(Compilation compilation) { - ImmutableDictionary.Builder builder = ImmutableDictionary.CreateBuilder(); + _map = new ConcurrentDictionary(); 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) { @@ -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; } @@ -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.Builder builder, INamedTypeSymbol? type, string methodName) + private void AddStringFormatMap(INamedTypeSymbol? type, string methodName) { if (type == null) { @@ -376,19 +384,27 @@ private static void AddStringFormatMap(ImmutableDictionary. foreach (IMethodSymbol method in type.GetMembers(methodName).OfType()) { - 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; @@ -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; } @@ -417,7 +435,7 @@ private static int GetExpectedNumberOfArguments(ImmutableArray return parameters.Length - formatIndex - 1; } - private static int FindParameterIndexOfName(ImmutableArray parameters, string name) + private static int FindParameterIndexByParameterName(ImmutableArray parameters, string name) { for (var i = 0; i < parameters.Length; i++) { @@ -430,6 +448,29 @@ private static int FindParameterIndexOfName(ImmutableArray par return -1; } + private int FindParameterIndexOfCompositeFormatStringSyntaxAttribute(ImmutableArray 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 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) @@ -443,4 +484,4 @@ public Info(int formatIndex, int expectedArguments) } } } -} \ No newline at end of file +} diff --git a/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/Runtime/ProvideCorrectArgumentsToFormattingMethodsTests.cs b/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/Runtime/ProvideCorrectArgumentsToFormattingMethodsTests.cs index 993e70d7f1..19d10ea590 100644 --- a/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/Runtime/ProvideCorrectArgumentsToFormattingMethodsTests.cs +++ b/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/Runtime/ProvideCorrectArgumentsToFormattingMethodsTests.cs @@ -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; @@ -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( 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) diff --git a/src/Utilities/Compiler/WellKnownTypeNames.cs b/src/Utilities/Compiler/WellKnownTypeNames.cs index 3039e160c0..f5c28b9742 100644 --- a/src/Utilities/Compiler/WellKnownTypeNames.cs +++ b/src/Utilities/Compiler/WellKnownTypeNames.cs @@ -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";