From f3a7f595fbb3d22fe8170419198f8ccd46136ed0 Mon Sep 17 00:00:00 2001 From: Newell Clark Date: Sat, 18 Sep 2021 10:09:34 -0400 Subject: [PATCH 1/9] Analyzer and fixer working - Supports string.Split(char, int, StringSplitOptions) - Supports string.Split(string, int, StringSplitOptions) - Supports StringBuilder(int) --- ...CSharpSuspiciousCastFromCharToInt.Fixer.cs | 33 ++ .../Core/AnalyzerReleases.Unshipped.md | 1 + .../MicrosoftNetCoreAnalyzersResources.resx | 12 + .../SuspiciousCastFromCharToInt.Fixer.cs | 63 ++++ .../Runtime/SuspiciousCastFromCharToInt.cs | 329 ++++++++++++++++++ .../MicrosoftNetCoreAnalyzersResources.cs.xlf | 20 ++ .../MicrosoftNetCoreAnalyzersResources.de.xlf | 20 ++ .../MicrosoftNetCoreAnalyzersResources.es.xlf | 20 ++ .../MicrosoftNetCoreAnalyzersResources.fr.xlf | 20 ++ .../MicrosoftNetCoreAnalyzersResources.it.xlf | 20 ++ .../MicrosoftNetCoreAnalyzersResources.ja.xlf | 20 ++ .../MicrosoftNetCoreAnalyzersResources.ko.xlf | 20 ++ .../MicrosoftNetCoreAnalyzersResources.pl.xlf | 20 ++ ...crosoftNetCoreAnalyzersResources.pt-BR.xlf | 20 ++ .../MicrosoftNetCoreAnalyzersResources.ru.xlf | 20 ++ .../MicrosoftNetCoreAnalyzersResources.tr.xlf | 20 ++ ...osoftNetCoreAnalyzersResources.zh-Hans.xlf | 20 ++ ...osoftNetCoreAnalyzersResources.zh-Hant.xlf | 20 ++ .../SuspiciousCastFromCharToIntTests.cs | 158 +++++++++ src/Utilities/Compiler/DiagnosticCategory.cs | 1 + .../DiagnosticCategoryAndIdRanges.txt | 1 + .../Extensions/IOperationExtensions.cs | 14 + src/Utilities/Compiler/WellKnownTypeNames.cs | 1 + 23 files changed, 873 insertions(+) create mode 100644 src/NetAnalyzers/CSharp/Microsoft.NetCore.Analyzers/Runtime/CSharpSuspiciousCastFromCharToInt.Fixer.cs create mode 100644 src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Runtime/SuspiciousCastFromCharToInt.Fixer.cs create mode 100644 src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Runtime/SuspiciousCastFromCharToInt.cs create mode 100644 src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/Runtime/SuspiciousCastFromCharToIntTests.cs diff --git a/src/NetAnalyzers/CSharp/Microsoft.NetCore.Analyzers/Runtime/CSharpSuspiciousCastFromCharToInt.Fixer.cs b/src/NetAnalyzers/CSharp/Microsoft.NetCore.Analyzers/Runtime/CSharpSuspiciousCastFromCharToInt.Fixer.cs new file mode 100644 index 0000000000..43fee35031 --- /dev/null +++ b/src/NetAnalyzers/CSharp/Microsoft.NetCore.Analyzers/Runtime/CSharpSuspiciousCastFromCharToInt.Fixer.cs @@ -0,0 +1,33 @@ +// 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.Generic; +using System.Collections.Immutable; +using System.Composition; +using System.Linq; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CodeActions; +using Microsoft.CodeAnalysis.CodeFixes; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Operations; +using Microsoft.CodeAnalysis.Rename; +using Microsoft.CodeAnalysis.Text; +using Microsoft.NetCore.Analyzers; +using Microsoft.NetCore.Analyzers.Runtime; + +namespace Microsoft.NetCore.CSharp.Analyzers.Runtime +{ + [ExportCodeFixProvider(LanguageNames.CSharp)] + public sealed class CSharpSuspiciousCastFromCharToIntFixer : SuspiciousCastFromCharToIntFixer + { + internal override SyntaxNode GetMemberAccessExpressionSyntax(SyntaxNode invocationExpressionSyntax) + { + return ((InvocationExpressionSyntax)invocationExpressionSyntax).Expression; + } + + internal override SyntaxNode GetDefaultValueExpression(SyntaxNode parameterSyntax) => ((ParameterSyntax)parameterSyntax).Default.Value; + } +} diff --git a/src/NetAnalyzers/Core/AnalyzerReleases.Unshipped.md b/src/NetAnalyzers/Core/AnalyzerReleases.Unshipped.md index 97d0a809bc..afec7820c5 100644 --- a/src/NetAnalyzers/Core/AnalyzerReleases.Unshipped.md +++ b/src/NetAnalyzers/Core/AnalyzerReleases.Unshipped.md @@ -4,5 +4,6 @@ Rule ID | Category | Severity | Notes --------|----------|----------|------- +CA3200 | Correctness | Warning | SuspiciousCastFromCharToIntAnalyzer, [Documentation](https://docs.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca3200) CA5404 | Security | Disabled | DoNotDisableTokenValidationChecks, [Documentation](https://docs.microsoft.com/visualstudio/code-quality/ca5404) CA5405 | Security | Disabled | DoNotAlwaysSkipTokenValidationInDelegates, [Documentation](https://docs.microsoft.com/visualstudio/code-quality/ca5405) diff --git a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/MicrosoftNetCoreAnalyzersResources.resx b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/MicrosoftNetCoreAnalyzersResources.resx index 544a1c9305..81e4604823 100644 --- a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/MicrosoftNetCoreAnalyzersResources.resx +++ b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/MicrosoftNetCoreAnalyzersResources.resx @@ -1782,4 +1782,16 @@ The {0} is set to a function that is always returning true. By setting the validation delegate, you are overriding default validation and by always returning true, this validation is completely disabled. + + Implicit cast from char to int in method call is suspicious. Consider using a different overload. + + + Suspicious implicit cast from char to int + + + Suspicious cast from char to int + + + Use correct overload + \ No newline at end of file diff --git a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Runtime/SuspiciousCastFromCharToInt.Fixer.cs b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Runtime/SuspiciousCastFromCharToInt.Fixer.cs new file mode 100644 index 0000000000..49768ee04c --- /dev/null +++ b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Runtime/SuspiciousCastFromCharToInt.Fixer.cs @@ -0,0 +1,63 @@ +// 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.Generic; +using System.Collections.Immutable; +using System.Composition; +using System.Linq; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CodeActions; +using Microsoft.CodeAnalysis.CodeFixes; +using Microsoft.CodeAnalysis.Editing; +using Microsoft.CodeAnalysis.Operations; +using Microsoft.CodeAnalysis.Rename; +using Microsoft.CodeAnalysis.Text; +using static Microsoft.NetCore.Analyzers.Runtime.SuspiciousCastFromCharToIntAnalyzer; + +namespace Microsoft.NetCore.Analyzers.Runtime +{ + public abstract class SuspiciousCastFromCharToIntFixer : CodeFixProvider + { + public sealed override ImmutableArray FixableDiagnosticIds { get; } = ImmutableArray.Create(RuleId); + + public sealed override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer; + + public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context) + { + var doc = context.Document; + var token = context.CancellationToken; + var root = await doc.GetSyntaxRootAsync(token).ConfigureAwait(false); + var model = await doc.GetSemanticModelAsync(token).ConfigureAwait(false); + + if (model.GetOperation(root.FindNode(context.Span), token) is not IArgumentOperation argumentOperation) + return; + + var targetMethod = argumentOperation.Parent switch + { + IInvocationOperation invocation => invocation.TargetMethod, + IObjectCreationOperation objectCreation => objectCreation.Constructor, + _ => default + }; + + if (targetMethod is null) + return; + + var apiHandlerCache = GetApiHandlerCache(model.Compilation); + + if (!apiHandlerCache.TryGetValue(targetMethod, out var handler)) + return; + + var codeAction = CodeAction.Create( + MicrosoftNetCoreAnalyzersResources.SuspiciousCastFromCharToIntCodeFixTitle, + token => handler.CreateChangedDocumentAsync(new FixCharCastContext(context, argumentOperation.Parent, this, token)), + MicrosoftNetCoreAnalyzersResources.SuspiciousCastFromCharToIntCodeFixTitle); + context.RegisterCodeFix(codeAction, context.Diagnostics); + } + + internal abstract SyntaxNode GetMemberAccessExpressionSyntax(SyntaxNode invocationExpressionSyntax); + + internal abstract SyntaxNode GetDefaultValueExpression(SyntaxNode parameterSyntax); + } +} diff --git a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Runtime/SuspiciousCastFromCharToInt.cs b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Runtime/SuspiciousCastFromCharToInt.cs new file mode 100644 index 0000000000..df94200518 --- /dev/null +++ b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Runtime/SuspiciousCastFromCharToInt.cs @@ -0,0 +1,329 @@ +// 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.Generic; +using System.Collections.Immutable; +using System.Diagnostics.CodeAnalysis; +using System.Linq; +using System.Threading; +using System.Threading.Tasks; +using Analyzer.Utilities; +using Analyzer.Utilities.Extensions; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CodeFixes; +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Editing; +using Microsoft.CodeAnalysis.Operations; +using Resx = Microsoft.NetCore.Analyzers.MicrosoftNetCoreAnalyzersResources; + +namespace Microsoft.NetCore.Analyzers.Runtime +{ + [DiagnosticAnalyzer(LanguageNames.CSharp, LanguageNames.VisualBasic)] + public sealed class SuspiciousCastFromCharToIntAnalyzer : DiagnosticAnalyzer + { + internal const string RuleId = "CA3200"; + + private static readonly LocalizableString s_localizableTitle = Resx.CreateLocalizableResourceString(nameof(Resx.SuspiciousCastFromCharToIntTitle)); + private static readonly LocalizableString s_localizableMessage = Resx.CreateLocalizableResourceString(nameof(Resx.SuspiciousCastFromCharToIntMessage)); + private static readonly LocalizableString s_localizableDescription = Resx.CreateLocalizableResourceString(nameof(Resx.SuspiciousCastFromCharToIntDescription)); + + internal static readonly DiagnosticDescriptor Rule = DiagnosticDescriptorHelper.Create( + RuleId, + s_localizableTitle, + s_localizableMessage, + DiagnosticCategory.Correctness, + RuleLevel.BuildWarning, + s_localizableDescription, + isPortedFxCopRule: false, + isDataflowRule: false); + + public override ImmutableArray SupportedDiagnostics { get; } = ImmutableArray.Create(Rule); + + public override void Initialize(AnalysisContext context) + { + context.EnableConcurrentExecution(); + context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); + context.RegisterCompilationStartAction(OnCompilationStart); + } + + private static void OnCompilationStart(CompilationStartAnalysisContext context) + { + var apiCache = GetApiHandlerCache(context.Compilation); + + if (apiCache.IsEmpty) + return; + + context.RegisterOperationAction(context => + { + var targetMethod = context.Operation switch + { + IInvocationOperation invocation => invocation.TargetMethod, + IObjectCreationOperation objectCreation => objectCreation.Constructor, + _ => default + }; + + RoslynDebug.Assert(targetMethod is not null, $"{nameof(targetMethod)} must not be null."); + + if (apiCache.TryGetValue(targetMethod, out var handler)) + handler.AnalyzeInvocationOperation(context); + + }, OperationKind.Invocation, OperationKind.ObjectCreation); + } + + // Property bag +#pragma warning disable CA1815 // Override equals and operator equals on value types + internal struct RequiredSymbols +#pragma warning restore CA1815 // Override equals and operator equals on value types + { + private RequiredSymbols(ITypeSymbol charType, ITypeSymbol int32Type, ITypeSymbol stringType) + { + CharType = charType; + Int32Type = int32Type; + StringType = stringType; + } + + public static bool TryGetSymbols(Compilation compilation, out RequiredSymbols result) + { + var charType = compilation.GetSpecialType(SpecialType.System_Char); + var int32Type = compilation.GetSpecialType(SpecialType.System_Int32); + var stringType = compilation.GetSpecialType(SpecialType.System_String); + + if (charType is not null && int32Type is not null && stringType is not null) + { + result = new RequiredSymbols(charType, int32Type, stringType); + return true; + } + + result = default; + return false; + } + + public ITypeSymbol CharType { get; } + public ITypeSymbol Int32Type { get; } + public ITypeSymbol StringType { get; } + } + + // Property bag +#pragma warning disable CA1815 // Override equals and operator equals on value types + internal struct FixCharCastContext +#pragma warning restore CA1815 // Override equals and operator equals on value types + { + public FixCharCastContext(CodeFixContext codeFixContext, IOperation operation, SuspiciousCastFromCharToIntFixer fixer, CancellationToken cancellationToken) + { + CodeFixContext = codeFixContext; + Operation = operation; + Fixer = fixer; + CancellationToken = cancellationToken; + } + + public CodeFixContext CodeFixContext { get; } + public IOperation Operation { get; } + public SuspiciousCastFromCharToIntFixer Fixer { get; } + public CancellationToken CancellationToken { get; } + } + + internal abstract class ApiHandler + { + protected ApiHandler(IMethodSymbol method) + { + Method = method; + } + + public IMethodSymbol Method { get; } + + public abstract void AnalyzeInvocationOperation(OperationAnalysisContext context); + + public abstract Task CreateChangedDocumentAsync(FixCharCastContext context); + } + + private sealed class StringSplitCharInt32StringSplitOptionsHandler : ApiHandler + { + private StringSplitCharInt32StringSplitOptionsHandler(IMethodSymbol splitMethod) : base(splitMethod) { } + + public static StringSplitCharInt32StringSplitOptionsHandler? Create(Compilation compilation, RequiredSymbols symbols) + { + if (!compilation.TryGetOrCreateTypeByMetadataName(WellKnownTypeNames.SystemStringSplitOptions, out var stringSplitOptions)) + return null; + + var splitMethod = symbols.StringType.GetMembers(nameof(string.Split)).OfType() + .GetFirstOrDefaultMemberWithParameterTypes(symbols.CharType, symbols.Int32Type, stringSplitOptions); + + return splitMethod is not null ? new StringSplitCharInt32StringSplitOptionsHandler(splitMethod) : null; + } + + public override void AnalyzeInvocationOperation(OperationAnalysisContext context) + { + var invocation = (IInvocationOperation)context.Operation; + var argument = invocation.Arguments.First(x => x.Parameter.Ordinal == 1); + + if (argument.Value is IConversionOperation conversion && conversion.IsImplicit && conversion.Operand.Type.SpecialType == SpecialType.System_Char) + { + context.ReportDiagnostic(argument.CreateDiagnostic(Rule)); + } + } + + public override async Task CreateChangedDocumentAsync(FixCharCastContext context) + { + var doc = context.CodeFixContext.Document; + var token = context.CancellationToken; + var invocation = (IInvocationOperation)context.Operation; + var editor = await DocumentEditor.CreateAsync(doc, token).ConfigureAwait(false); + + var charTypeExpressionSyntax = editor.Generator.TypeExpression(invocation.SemanticModel.Compilation.GetSpecialType(SpecialType.System_Char)); + var arguments = invocation.GetArgumentsInParameterOrder(); + var elementNodes = new[] + { + arguments[0].Value.Syntax, + arguments[1].Value.Syntax + }; + var arrayCreationSyntax = editor.Generator.ArrayCreationExpression(charTypeExpressionSyntax, elementNodes); + var memberAccessSyntax = context.Fixer.GetMemberAccessExpressionSyntax(invocation.Syntax); + var newInvocationSyntax = editor.Generator.InvocationExpression(memberAccessSyntax, arrayCreationSyntax, arguments[2].Syntax); + + editor.ReplaceNode(invocation.Syntax, newInvocationSyntax); + + return editor.GetChangedDocument(); + } + } + + private sealed class StringSplitStringInt32StringSplitOptionsHandler : ApiHandler + { + private StringSplitStringInt32StringSplitOptionsHandler(IMethodSymbol splitMethod) : base(splitMethod) { } + + public static StringSplitStringInt32StringSplitOptionsHandler? Create(Compilation compilation, RequiredSymbols symbols) + { + if (!compilation.TryGetOrCreateTypeByMetadataName(WellKnownTypeNames.SystemStringSplitOptions, out var stringSplitOptionsType)) + return null; + + var splitMethod = symbols.StringType.GetMembers(nameof(string.Split)).OfType() + .GetFirstOrDefaultMemberWithParameterTypes(symbols.StringType, symbols.Int32Type, stringSplitOptionsType); + + return splitMethod is not null ? new StringSplitStringInt32StringSplitOptionsHandler(splitMethod) : null; + } + + public override void AnalyzeInvocationOperation(OperationAnalysisContext context) + { + var invocation = (IInvocationOperation)context.Operation; + var argument = invocation.Arguments.First(x => x.Parameter.Ordinal == 1); + + if (argument.Value is IConversionOperation conversion && conversion.IsImplicit && conversion.Operand.Type.SpecialType == SpecialType.System_Char) + { + context.ReportDiagnostic(argument.CreateDiagnostic(Rule)); + } + } + + public override async Task CreateChangedDocumentAsync(FixCharCastContext context) + { + var editor = await DocumentEditor.CreateAsync(context.CodeFixContext.Document, context.CancellationToken).ConfigureAwait(false); + var invocation = (IInvocationOperation)context.Operation; + + var stringType = invocation.SemanticModel.Compilation.GetSpecialType(SpecialType.System_String); + var stringTypeExpressionSyntax = editor.Generator.TypeExpression(stringType); + + var arguments = invocation.GetArgumentsInParameterOrder(); + var elementNodes = new[] + { + arguments[0].Value.Syntax, + ConvertCharOperationToString(editor, ((IConversionOperation)arguments[1].Value).Operand) + }; + var arrayCreationSyntax = editor.Generator.ArrayCreationExpression(stringTypeExpressionSyntax, elementNodes); + + var optionsArgumentSyntax = arguments[2].ArgumentKind is ArgumentKind.DefaultValue ? + CreateEnumMemberAccess((INamedTypeSymbol)arguments[2].Parameter.Type, nameof(StringSplitOptions.None)) : + arguments[2].Syntax; + + var splitMemberAccessSyntax = context.Fixer.GetMemberAccessExpressionSyntax(context.Operation.Syntax); + var newInvocationSyntax = editor.Generator.InvocationExpression(splitMemberAccessSyntax, arrayCreationSyntax, optionsArgumentSyntax); + + editor.ReplaceNode(context.Operation.Syntax, newInvocationSyntax); + + return editor.GetChangedDocument(); + + // Local functions + + SyntaxNode CreateEnumMemberAccess(INamedTypeSymbol enumType, string enumMemberName) + { + RoslynDebug.Assert(editor is not null); + + var enumTypeSyntax = editor.Generator.TypeExpressionForStaticMemberAccess(enumType); + return editor.Generator.MemberAccessExpression(enumTypeSyntax, enumMemberName); + } + } + } + + private sealed class StringBuilderInt32Handler : ApiHandler + { + private StringBuilderInt32Handler(IMethodSymbol ctor) : base(ctor) { } + + public static StringBuilderInt32Handler? Create(Compilation compilation, RequiredSymbols symbols) + { + if (!compilation.TryGetOrCreateTypeByMetadataName(WellKnownTypeNames.SystemTextStringBuilder, out var stringBuilderType)) + return null; + + var ctor = stringBuilderType.GetMembers(WellKnownMemberNames.InstanceConstructorName).Cast() + .GetFirstOrDefaultMemberWithParameterTypes(symbols.Int32Type); + + return ctor is not null ? new StringBuilderInt32Handler(ctor) : null; + } + + public override void AnalyzeInvocationOperation(OperationAnalysisContext context) + { + var objectCreation = (IObjectCreationOperation)context.Operation; + var argument = objectCreation.Arguments[0]; + + if (argument.Value is IConversionOperation conversion && conversion.IsImplicit && conversion.Operand.Type.SpecialType == SpecialType.System_Char) + { + context.ReportDiagnostic(argument.CreateDiagnostic(Rule)); + } + } + + public override async Task CreateChangedDocumentAsync(FixCharCastContext context) + { + var editor = await DocumentEditor.CreateAsync(context.CodeFixContext.Document, context.CancellationToken).ConfigureAwait(false); + var instanceCreation = (IObjectCreationOperation)context.Operation; + var argument = instanceCreation.Arguments[0]; + + var newArgumentSyntax = ConvertCharOperationToString(editor, ((IConversionOperation)argument.Value).Operand); + var newObjectCreationSyntax = editor.Generator.ObjectCreationExpression(instanceCreation.Constructor.ContainingType, newArgumentSyntax); + + editor.ReplaceNode(instanceCreation.Syntax, newObjectCreationSyntax); + + return editor.GetChangedDocument(); + } + } + + internal static ImmutableDictionary GetApiHandlerCache(Compilation compilation) + { + if (!RequiredSymbols.TryGetSymbols(compilation, out var symbols)) + return ImmutableDictionary.Empty; + + var builder = ImmutableDictionary.CreateBuilder(); + + AddIfNotNull(StringSplitCharInt32StringSplitOptionsHandler.Create(compilation, symbols)); + AddIfNotNull(StringSplitStringInt32StringSplitOptionsHandler.Create(compilation, symbols)); + AddIfNotNull(StringBuilderInt32Handler.Create(compilation, symbols)); + + return builder.ToImmutable(); + + // Local functions + + void AddIfNotNull(ApiHandler? handler) + { + if (handler is not null) + builder!.Add(handler.Method, handler); + } + } + + private static SyntaxNode ConvertCharOperationToString(DocumentEditor editor, IOperation charOperation) + { + if (charOperation is ILiteralOperation charLiteral) + { + return editor.Generator.LiteralExpression(charLiteral.ConstantValue.Value.ToString()); + } + + var toStringMemberAccessSyntax = editor.Generator.MemberAccessExpression(charOperation.Syntax, nameof(object.ToString)); + + return editor.Generator.InvocationExpression(toStringMemberAccessSyntax); + } + } +} diff --git a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.cs.xlf b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.cs.xlf index 954557b2c4..3bd14a53c0 100644 --- a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.cs.xlf +++ b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.cs.xlf @@ -2317,6 +2317,26 @@ Using both 'static' and 'abstract' modifiers requires opting into preview features. See https://aka.ms/dotnet-warnings/preview-features for more information. + + Use correct overload + Use correct overload + + + + Implicit cast from char to int in method call is suspicious. Consider using a different overload. + Implicit cast from char to int in method call is suspicious. Consider using a different overload. + + + + Suspicious implicit cast from char to int + Suspicious implicit cast from char to int + + + + Suspicious cast from char to int + Suspicious cast from char to int + + Comparing strings by using the String.Length property or the String.IsNullOrEmpty method is significantly faster than using Equals. Porovnávání řetězců pomocí vlastnosti String.Length nebo metody String.IsNullOrEmpty je výrazně rychlejší než pomocí Equals. diff --git a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.de.xlf b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.de.xlf index d62bf1874b..b45b0d44b4 100644 --- a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.de.xlf +++ b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.de.xlf @@ -2317,6 +2317,26 @@ Using both 'static' and 'abstract' modifiers requires opting into preview features. See https://aka.ms/dotnet-warnings/preview-features for more information. + + Use correct overload + Use correct overload + + + + Implicit cast from char to int in method call is suspicious. Consider using a different overload. + Implicit cast from char to int in method call is suspicious. Consider using a different overload. + + + + Suspicious implicit cast from char to int + Suspicious implicit cast from char to int + + + + Suspicious cast from char to int + Suspicious cast from char to int + + Comparing strings by using the String.Length property or the String.IsNullOrEmpty method is significantly faster than using Equals. Der Zeichenfolgenvergleich mithilfe der String.Length-Eigenschaft oder der String.IsNullOrEmpty-Methode ist erheblich schneller als der Vergleich über "Equals". diff --git a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.es.xlf b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.es.xlf index c93c898eba..ec5ebae991 100644 --- a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.es.xlf +++ b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.es.xlf @@ -2317,6 +2317,26 @@ Using both 'static' and 'abstract' modifiers requires opting into preview features. See https://aka.ms/dotnet-warnings/preview-features for more information. + + Use correct overload + Use correct overload + + + + Implicit cast from char to int in method call is suspicious. Consider using a different overload. + Implicit cast from char to int in method call is suspicious. Consider using a different overload. + + + + Suspicious implicit cast from char to int + Suspicious implicit cast from char to int + + + + Suspicious cast from char to int + Suspicious cast from char to int + + Comparing strings by using the String.Length property or the String.IsNullOrEmpty method is significantly faster than using Equals. La comparación de cadenas utilizando la propiedad String.Length o el método String.IsNullOrEmpty es significativamente más rápida que si se utilizara Equals. diff --git a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.fr.xlf b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.fr.xlf index a139faa853..b4a78cdb3a 100644 --- a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.fr.xlf +++ b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.fr.xlf @@ -2317,6 +2317,26 @@ Using both 'static' and 'abstract' modifiers requires opting into preview features. See https://aka.ms/dotnet-warnings/preview-features for more information. + + Use correct overload + Use correct overload + + + + Implicit cast from char to int in method call is suspicious. Consider using a different overload. + Implicit cast from char to int in method call is suspicious. Consider using a different overload. + + + + Suspicious implicit cast from char to int + Suspicious implicit cast from char to int + + + + Suspicious cast from char to int + Suspicious cast from char to int + + Comparing strings by using the String.Length property or the String.IsNullOrEmpty method is significantly faster than using Equals. La comparaison de chaînes à l'aide de la propriété String.Length ou de la méthode String.IsNullOrEmpty est nettement plus rapide que l'utilisation de Equals. diff --git a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.it.xlf b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.it.xlf index d7062dee3a..7e2890155d 100644 --- a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.it.xlf +++ b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.it.xlf @@ -2317,6 +2317,26 @@ Using both 'static' and 'abstract' modifiers requires opting into preview features. See https://aka.ms/dotnet-warnings/preview-features for more information. + + Use correct overload + Use correct overload + + + + Implicit cast from char to int in method call is suspicious. Consider using a different overload. + Implicit cast from char to int in method call is suspicious. Consider using a different overload. + + + + Suspicious implicit cast from char to int + Suspicious implicit cast from char to int + + + + Suspicious cast from char to int + Suspicious cast from char to int + + Comparing strings by using the String.Length property or the String.IsNullOrEmpty method is significantly faster than using Equals. Il confronto di stringhe è notevolmente più rapido se si usa la proprietà String.Length o il metodo String.IsNullOrEmpty invece di Equals. diff --git a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.ja.xlf b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.ja.xlf index f78bb3c73a..b8ae1543d4 100644 --- a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.ja.xlf +++ b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.ja.xlf @@ -2317,6 +2317,26 @@ Using both 'static' and 'abstract' modifiers requires opting into preview features. See https://aka.ms/dotnet-warnings/preview-features for more information. + + Use correct overload + Use correct overload + + + + Implicit cast from char to int in method call is suspicious. Consider using a different overload. + Implicit cast from char to int in method call is suspicious. Consider using a different overload. + + + + Suspicious implicit cast from char to int + Suspicious implicit cast from char to int + + + + Suspicious cast from char to int + Suspicious cast from char to int + + Comparing strings by using the String.Length property or the String.IsNullOrEmpty method is significantly faster than using Equals. String.Length プロパティまたは String.IsNullOrEmpty メソッドを使用して文字列を比較すると、Equals を使用した場合よりも処理速度が大幅に向上します。 diff --git a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.ko.xlf b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.ko.xlf index 9a4cfb6b1a..eff3d640ca 100644 --- a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.ko.xlf +++ b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.ko.xlf @@ -2317,6 +2317,26 @@ Using both 'static' and 'abstract' modifiers requires opting into preview features. See https://aka.ms/dotnet-warnings/preview-features for more information. + + Use correct overload + Use correct overload + + + + Implicit cast from char to int in method call is suspicious. Consider using a different overload. + Implicit cast from char to int in method call is suspicious. Consider using a different overload. + + + + Suspicious implicit cast from char to int + Suspicious implicit cast from char to int + + + + Suspicious cast from char to int + Suspicious cast from char to int + + Comparing strings by using the String.Length property or the String.IsNullOrEmpty method is significantly faster than using Equals. String.Length 속성 또는 String.IsNullOrEmpty 메서드를 사용하면 Equals를 사용하는 것보다 훨씬 빠르게 문자열을 비교할 수 있습니다. diff --git a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.pl.xlf b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.pl.xlf index 3c51eeb1c6..f87dc18d1e 100644 --- a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.pl.xlf +++ b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.pl.xlf @@ -2317,6 +2317,26 @@ Using both 'static' and 'abstract' modifiers requires opting into preview features. See https://aka.ms/dotnet-warnings/preview-features for more information. + + Use correct overload + Use correct overload + + + + Implicit cast from char to int in method call is suspicious. Consider using a different overload. + Implicit cast from char to int in method call is suspicious. Consider using a different overload. + + + + Suspicious implicit cast from char to int + Suspicious implicit cast from char to int + + + + Suspicious cast from char to int + Suspicious cast from char to int + + Comparing strings by using the String.Length property or the String.IsNullOrEmpty method is significantly faster than using Equals. Porównywanie ciągów za pomocą właściwości String.Length lub metody String.IsNullOrEmpty jest znacznie szybsze niż za pomocą metody Equals. diff --git a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.pt-BR.xlf b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.pt-BR.xlf index 269870642c..d5642ecadc 100644 --- a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.pt-BR.xlf +++ b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.pt-BR.xlf @@ -2317,6 +2317,26 @@ Using both 'static' and 'abstract' modifiers requires opting into preview features. See https://aka.ms/dotnet-warnings/preview-features for more information. + + Use correct overload + Use correct overload + + + + Implicit cast from char to int in method call is suspicious. Consider using a different overload. + Implicit cast from char to int in method call is suspicious. Consider using a different overload. + + + + Suspicious implicit cast from char to int + Suspicious implicit cast from char to int + + + + Suspicious cast from char to int + Suspicious cast from char to int + + Comparing strings by using the String.Length property or the String.IsNullOrEmpty method is significantly faster than using Equals. Comparar cadeias de caracteres usando a propriedade String.Length ou o método String.IsNullOrEmpty é significativamente mais rápido do que usar Equals. diff --git a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.ru.xlf b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.ru.xlf index 96e80f7867..7d7b7226e7 100644 --- a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.ru.xlf +++ b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.ru.xlf @@ -2317,6 +2317,26 @@ Using both 'static' and 'abstract' modifiers requires opting into preview features. See https://aka.ms/dotnet-warnings/preview-features for more information. + + Use correct overload + Use correct overload + + + + Implicit cast from char to int in method call is suspicious. Consider using a different overload. + Implicit cast from char to int in method call is suspicious. Consider using a different overload. + + + + Suspicious implicit cast from char to int + Suspicious implicit cast from char to int + + + + Suspicious cast from char to int + Suspicious cast from char to int + + Comparing strings by using the String.Length property or the String.IsNullOrEmpty method is significantly faster than using Equals. Сравнение строк с помощью свойства String.Length или метода String.IsNullOrEmpty выполняется значительно быстрее, чем при использовании Equals. diff --git a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.tr.xlf b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.tr.xlf index 1a10f53684..ca962f908f 100644 --- a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.tr.xlf +++ b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.tr.xlf @@ -2317,6 +2317,26 @@ Using both 'static' and 'abstract' modifiers requires opting into preview features. See https://aka.ms/dotnet-warnings/preview-features for more information. + + Use correct overload + Use correct overload + + + + Implicit cast from char to int in method call is suspicious. Consider using a different overload. + Implicit cast from char to int in method call is suspicious. Consider using a different overload. + + + + Suspicious implicit cast from char to int + Suspicious implicit cast from char to int + + + + Suspicious cast from char to int + Suspicious cast from char to int + + Comparing strings by using the String.Length property or the String.IsNullOrEmpty method is significantly faster than using Equals. Dizeleri Equals yerine String.Length özelliğini veya String.IsNullOrEmpty yöntemini kullanarak karşılaştırmak önemli ölçüde daha hızlıdır. diff --git a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.zh-Hans.xlf b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.zh-Hans.xlf index ddf8b43692..6b7a3c0682 100644 --- a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.zh-Hans.xlf +++ b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.zh-Hans.xlf @@ -2317,6 +2317,26 @@ Using both 'static' and 'abstract' modifiers requires opting into preview features. See https://aka.ms/dotnet-warnings/preview-features for more information. + + Use correct overload + Use correct overload + + + + Implicit cast from char to int in method call is suspicious. Consider using a different overload. + Implicit cast from char to int in method call is suspicious. Consider using a different overload. + + + + Suspicious implicit cast from char to int + Suspicious implicit cast from char to int + + + + Suspicious cast from char to int + Suspicious cast from char to int + + Comparing strings by using the String.Length property or the String.IsNullOrEmpty method is significantly faster than using Equals. 相比于使用 Equals，使用 String.Length 属性或 String.IsNullOrEmpty 方法比较字符串的速度要快得多。 diff --git a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.zh-Hant.xlf b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.zh-Hant.xlf index e75593dad6..5799c6a9d6 100644 --- a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.zh-Hant.xlf +++ b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/xlf/MicrosoftNetCoreAnalyzersResources.zh-Hant.xlf @@ -2317,6 +2317,26 @@ Using both 'static' and 'abstract' modifiers requires opting into preview features. See https://aka.ms/dotnet-warnings/preview-features for more information. + + Use correct overload + Use correct overload + + + + Implicit cast from char to int in method call is suspicious. Consider using a different overload. + Implicit cast from char to int in method call is suspicious. Consider using a different overload. + + + + Suspicious implicit cast from char to int + Suspicious implicit cast from char to int + + + + Suspicious cast from char to int + Suspicious cast from char to int + + Comparing strings by using the String.Length property or the String.IsNullOrEmpty method is significantly faster than using Equals. 使用 String.Length 屬性或 String.IsNullOrEmpty 方法來比較字串的速度，大幅快過於使用 Equals。 diff --git a/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/Runtime/SuspiciousCastFromCharToIntTests.cs b/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/Runtime/SuspiciousCastFromCharToIntTests.cs new file mode 100644 index 0000000000..4ae2171598 --- /dev/null +++ b/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/Runtime/SuspiciousCastFromCharToIntTests.cs @@ -0,0 +1,158 @@ +// Copyright (c) Microsoft. All Rights Reserved. Licensed under the MIT license. See License.txt in the project root for license information. + +using System.Globalization; +using System.Threading.Tasks; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Testing; +using Xunit; + +using VerifyCS = Test.Utilities.CSharpCodeFixVerifier< + Microsoft.NetCore.Analyzers.Runtime.SuspiciousCastFromCharToIntAnalyzer, + Microsoft.NetCore.CSharp.Analyzers.Runtime.CSharpSuspiciousCastFromCharToIntFixer>; + +namespace Microsoft.NetCore.Analyzers.Runtime.UnitTests +{ + public class SuspiciousCastFromCharToIntTests + { + [Theory] + [InlineData("'a'")] + [InlineData("_char")] + [InlineData("GetChar()")] + public Task StringSplit_CharInt32StringSplitOptions_Diagnostic_CS(string charArgument) + { + string source = $@" +public class Testopolis +{{ + private char _char; + private char GetChar() => _char; + public void M() + {{ + nameof(M).Split('c', {{|#0:{charArgument}|}}, System.StringSplitOptions.None); + }} +}}"; + string fixedSource = $@" +public class Testopolis +{{ + private char _char; + private char GetChar() => _char; + public void M() + {{ + nameof(M).Split(new char[] {{ 'c', {charArgument} }}, System.StringSplitOptions.None); + }} +}}"; + var diagnostics = new[] + { + VerifyCS.Diagnostic(Rule).WithLocation(0) + }; + + return VerifyCS.VerifyCodeFixAsync(source, diagnostics, fixedSource); + } + + [Theory] + [InlineData( + "'c', {|#0:'a'|}, System.StringSplitOptions.RemoveEmptyEntries", + "new char[] { 'c', 'a' }, System.StringSplitOptions.RemoveEmptyEntries")] + [InlineData( + "{|#0:count: 'a'|}, separator: 'c', options: System.StringSplitOptions.RemoveEmptyEntries", + "new char[] { 'c', 'a' }, options: System.StringSplitOptions.RemoveEmptyEntries")] + [InlineData( + "options: System.StringSplitOptions.None, separator: 'c', {|#0:count: 'a'|}", + "new char[] { 'c', 'a' }, options: System.StringSplitOptions.None")] + public Task StringSplit_CharInt32StringSplitOptions_NamedArguments_Diagnostic_CS(string testArgumentList, string fixedArgumentList) + { + string source = $@" +public class Testopolis +{{ + private char _char; + private char GetChar() => _char; + public void M() + {{ + nameof(M).Split({testArgumentList}); + }} +}}"; + string fixedSource = $@" +public class Testopolis +{{ + private char _char; + private char GetChar() => _char; + public void M() + {{ + nameof(M).Split({fixedArgumentList}); + }} +}}"; + var diagnostic = VerifyCS.Diagnostic(Rule).WithLocation(0); + + return VerifyCS.VerifyCodeFixAsync(source, diagnostic, fixedSource); + } + + [Theory] + [InlineData("s, {|#0:c|}, System.StringSplitOptions.None", "new string[] { s, c.ToString() }, System.StringSplitOptions.None")] + [InlineData("s, {|#0:c|}", "new string[] { s, c.ToString() }, System.StringSplitOptions.None")] + [InlineData("{|#0:count: c|}, separator: s", "new string[] { s, c.ToString() }, System.StringSplitOptions.None")] + [InlineData("options: System.StringSplitOptions.None, {|#0:count: c|}, separator: s", "new string[] { s, c.ToString() }, options: System.StringSplitOptions.None")] + [InlineData("s, {|#0:'a'|}", @"new string[] { s, ""a"" }, System.StringSplitOptions.None")] + public Task StringSplit_StringInt32StringSplitOptions_NamedArguments_Diagnostic_CS(string testArgumentList, string fixedArgumentList) + { + string format = @" +public class Testopolis +{{ + public void M(string s, char c) + {{ + nameof(M).Split({0}); + }} +}}"; + var test = new VerifyCS.Test + { + TestState = + { + Sources = { string.Format(CultureInfo.InvariantCulture, format, testArgumentList) }, + ReferenceAssemblies = ReferenceAssemblies.Net.Net50, + ExpectedDiagnostics = { VerifyCS.Diagnostic(Rule).WithLocation(0) } + }, + FixedState = + { + Sources = { string.Format(CultureInfo.InvariantCulture, format, fixedArgumentList) }, + ReferenceAssemblies = ReferenceAssemblies.Net.Net50 + } + }; + + return test.RunAsync(); + } + + [Theory] + [InlineData("{|#0:c|}", "c.ToString()")] + [InlineData("{|#0:'a'|}", @"""a""")] + [InlineData("{|#0:capacity: c|}", "c.ToString()")] + [InlineData("{|#0:capacity: 'a'|}", @"""a""")] + public Task StringBuilder_Int32_Diagnostic_CS(string testArgumentList, string fixedArgumentList) + { + string format = @" +using System.Text; +public class Testopolis +{{ + public void M(string s, char c, int n) + {{ + new StringBuilder({0}); + }} +}}"; + var test = new VerifyCS.Test + { + TestState = + { + Sources = { string.Format(CultureInfo.InvariantCulture, format, testArgumentList) }, + ReferenceAssemblies = ReferenceAssemblies.Net.Net50, + ExpectedDiagnostics = { VerifyCS.Diagnostic(Rule).WithLocation(0) } + }, + FixedState = + { + Sources = { string.Format(CultureInfo.InvariantCulture, format, fixedArgumentList) }, + ReferenceAssemblies = ReferenceAssemblies.Net.Net50 + } + }; + + return test.RunAsync(); + } + + private static DiagnosticDescriptor Rule => SuspiciousCastFromCharToIntAnalyzer.Rule; + } +} diff --git a/src/Utilities/Compiler/DiagnosticCategory.cs b/src/Utilities/Compiler/DiagnosticCategory.cs index cdc09df5e4..41ffed2b05 100644 --- a/src/Utilities/Compiler/DiagnosticCategory.cs +++ b/src/Utilities/Compiler/DiagnosticCategory.cs @@ -10,6 +10,7 @@ internal static class DiagnosticCategory public const string Mobility = nameof(Mobility); public const string Performance = nameof(Performance); public const string Reliability = nameof(Reliability); + public const string Correctness = nameof(Correctness); public const string Security = nameof(Security); public const string Usage = nameof(Usage); public const string Naming = nameof(Naming); diff --git a/src/Utilities/Compiler/DiagnosticCategoryAndIdRanges.txt b/src/Utilities/Compiler/DiagnosticCategoryAndIdRanges.txt index 79ece4f197..a330cba22d 100644 --- a/src/Utilities/Compiler/DiagnosticCategoryAndIdRanges.txt +++ b/src/Utilities/Compiler/DiagnosticCategoryAndIdRanges.txt @@ -19,6 +19,7 @@ Naming: CA1700-CA1727 Interoperability: CA1400-CA1419 Maintainability: CA1500-CA1509 Reliability: CA9998-CA9999, CA2000-CA2018 +Correctness: CA3200 Documentation: CA1200-CA1200 # Microsoft CodeAnalysis API rules diff --git a/src/Utilities/Compiler/Extensions/IOperationExtensions.cs b/src/Utilities/Compiler/Extensions/IOperationExtensions.cs index 51caceed5f..47f5858579 100644 --- a/src/Utilities/Compiler/Extensions/IOperationExtensions.cs +++ b/src/Utilities/Compiler/Extensions/IOperationExtensions.cs @@ -57,6 +57,20 @@ internal static partial class IOperationExtensions return typeInfo.Type as INamedTypeSymbol; } + /// + /// Gets the arguments for the current ordered by their respective parameter's property. + /// + public static ImmutableArray GetArgumentsInParameterOrder(this IInvocationOperation invocation) + { + var result = ImmutableArray.CreateBuilder(invocation.Arguments.Length); + result.Count = invocation.Arguments.Length; + + foreach (var argument in invocation.Arguments) + result[argument.Parameter.Ordinal] = argument; + + return result.MoveToImmutable(); + } + public static bool HasNullConstantValue(this IOperation operation) { return operation.ConstantValue.HasValue && operation.ConstantValue.Value == null; diff --git a/src/Utilities/Compiler/WellKnownTypeNames.cs b/src/Utilities/Compiler/WellKnownTypeNames.cs index 19544d27bd..b7fee51b24 100644 --- a/src/Utilities/Compiler/WellKnownTypeNames.cs +++ b/src/Utilities/Compiler/WellKnownTypeNames.cs @@ -349,6 +349,7 @@ internal static class WellKnownTypeNames public const string SystemStackOverflowException = "System.StackOverflowException"; public const string SystemString = "System.String"; public const string SystemStringComparison = "System.StringComparison"; + public const string SystemStringSplitOptions = "System.StringSplitOptions"; public const string SystemSystemException = "System.SystemException"; public const string SystemTextEncoding = "System.Text.Encoding"; public const string SystemTextRegularExpressionsRegex = "System.Text.RegularExpressions.Regex"; From 935f162e7ab04bb5050109ea3dd74bdcc567364c Mon Sep 17 00:00:00 2001 From: Newell Clark Date: Sat, 18 Sep 2021 13:39:25 -0400 Subject: [PATCH 2/9] Clean up analyzer, fixer, and tests --- ...CSharpSuspiciousCastFromCharToInt.Fixer.cs | 13 - .../SuspiciousCastFromCharToInt.Fixer.cs | 214 +++++++++++++- .../Runtime/SuspiciousCastFromCharToInt.cs | 271 ++++++++---------- .../SuspiciousCastFromCharToIntTests.cs | 128 ++++++--- 4 files changed, 407 insertions(+), 219 deletions(-) diff --git a/src/NetAnalyzers/CSharp/Microsoft.NetCore.Analyzers/Runtime/CSharpSuspiciousCastFromCharToInt.Fixer.cs b/src/NetAnalyzers/CSharp/Microsoft.NetCore.Analyzers/Runtime/CSharpSuspiciousCastFromCharToInt.Fixer.cs index 43fee35031..a2e3c5f9e7 100644 --- a/src/NetAnalyzers/CSharp/Microsoft.NetCore.Analyzers/Runtime/CSharpSuspiciousCastFromCharToInt.Fixer.cs +++ b/src/NetAnalyzers/CSharp/Microsoft.NetCore.Analyzers/Runtime/CSharpSuspiciousCastFromCharToInt.Fixer.cs @@ -1,21 +1,8 @@ // 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.Generic; -using System.Collections.Immutable; -using System.Composition; -using System.Linq; -using System.Threading; -using System.Threading.Tasks; using Microsoft.CodeAnalysis; -using Microsoft.CodeAnalysis.CodeActions; using Microsoft.CodeAnalysis.CodeFixes; -using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.CSharp.Syntax; -using Microsoft.CodeAnalysis.Operations; -using Microsoft.CodeAnalysis.Rename; -using Microsoft.CodeAnalysis.Text; -using Microsoft.NetCore.Analyzers; using Microsoft.NetCore.Analyzers.Runtime; namespace Microsoft.NetCore.CSharp.Analyzers.Runtime diff --git a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Runtime/SuspiciousCastFromCharToInt.Fixer.cs b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Runtime/SuspiciousCastFromCharToInt.Fixer.cs index 49768ee04c..40ac089875 100644 --- a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Runtime/SuspiciousCastFromCharToInt.Fixer.cs +++ b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Runtime/SuspiciousCastFromCharToInt.Fixer.cs @@ -1,26 +1,23 @@ // 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.Generic; using System.Collections.Immutable; -using System.Composition; -using System.Linq; using System.Threading; using System.Threading.Tasks; +using Analyzer.Utilities; +using Analyzer.Utilities.Extensions; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CodeActions; using Microsoft.CodeAnalysis.CodeFixes; using Microsoft.CodeAnalysis.Editing; using Microsoft.CodeAnalysis.Operations; -using Microsoft.CodeAnalysis.Rename; -using Microsoft.CodeAnalysis.Text; -using static Microsoft.NetCore.Analyzers.Runtime.SuspiciousCastFromCharToIntAnalyzer; +using RequiredSymbols = Microsoft.NetCore.Analyzers.Runtime.SuspiciousCastFromCharToIntAnalyzer.RequiredSymbols; namespace Microsoft.NetCore.Analyzers.Runtime { public abstract class SuspiciousCastFromCharToIntFixer : CodeFixProvider { - public sealed override ImmutableArray FixableDiagnosticIds { get; } = ImmutableArray.Create(RuleId); + public sealed override ImmutableArray FixableDiagnosticIds { get; } = ImmutableArray.Create(SuspiciousCastFromCharToIntAnalyzer.RuleId); public sealed override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer; @@ -44,14 +41,14 @@ public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context) if (targetMethod is null) return; - var apiHandlerCache = GetApiHandlerCache(model.Compilation); + var cache = CreateFixerCache(model.Compilation); - if (!apiHandlerCache.TryGetValue(targetMethod, out var handler)) + if (!cache.TryGetValue(targetMethod, out var fixer)) return; var codeAction = CodeAction.Create( MicrosoftNetCoreAnalyzersResources.SuspiciousCastFromCharToIntCodeFixTitle, - token => handler.CreateChangedDocumentAsync(new FixCharCastContext(context, argumentOperation.Parent, this, token)), + token => fixer.GetChangedDocumentAsync(new FixCharCastContext(context, argumentOperation.Parent, this, token)), MicrosoftNetCoreAnalyzersResources.SuspiciousCastFromCharToIntCodeFixTitle); context.RegisterCodeFix(codeAction, context.Diagnostics); } @@ -59,5 +56,202 @@ public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context) internal abstract SyntaxNode GetMemberAccessExpressionSyntax(SyntaxNode invocationExpressionSyntax); internal abstract SyntaxNode GetDefaultValueExpression(SyntaxNode parameterSyntax); + + // Property bag +#pragma warning disable CA1815 // Override equals and operator equals on value types + private readonly struct FixCharCastContext +#pragma warning restore CA1815 // Override equals and operator equals on value types + { + public FixCharCastContext(CodeFixContext codeFixContext, IOperation operation, SuspiciousCastFromCharToIntFixer fixer, CancellationToken cancellationToken) + { + CodeFixContext = codeFixContext; + Operation = operation; + Fixer = fixer; + CancellationToken = cancellationToken; + } + + public CodeFixContext CodeFixContext { get; } + public IOperation Operation { get; } + public SuspiciousCastFromCharToIntFixer Fixer { get; } + public CancellationToken CancellationToken { get; } + } + + private abstract class OperationFixer + { + protected OperationFixer(IMethodSymbol invokedMethod) + { + InvokedMethod = invokedMethod; + } + + public IMethodSymbol InvokedMethod { get; } + + public abstract Task GetChangedDocumentAsync(FixCharCastContext context); + } + + /// + /// Fixes calls to string.Split(char, int, StringSplitOptions) with an implicit char-to-int conversion. + /// + private sealed class StringSplit_CharInt32StringSplitOptions : OperationFixer + { + private StringSplit_CharInt32StringSplitOptions(IMethodSymbol splitMethod) : base(splitMethod) { } + + public static StringSplit_CharInt32StringSplitOptions? Create(Compilation compilation, RequiredSymbols symbols) + { + if (!compilation.TryGetOrCreateTypeByMetadataName(WellKnownTypeNames.SystemStringSplitOptions, out var stringSplitOptionsType)) + return null; + + var splitMethod = symbols.StringType.GetMembers(nameof(string.Split)).OfType() + .GetFirstOrDefaultMemberWithParameterTypes(symbols.CharType, symbols.Int32Type, stringSplitOptionsType); + + return splitMethod is not null ? new StringSplit_CharInt32StringSplitOptions(splitMethod) : null; + } + + public override async Task GetChangedDocumentAsync(FixCharCastContext context) + { + var doc = context.CodeFixContext.Document; + var token = context.CancellationToken; + var invocation = (IInvocationOperation)context.Operation; + var editor = await DocumentEditor.CreateAsync(doc, token).ConfigureAwait(false); + + var charTypeExpressionSyntax = editor.Generator.TypeExpression(invocation.SemanticModel.Compilation.GetSpecialType(SpecialType.System_Char)); + var arguments = invocation.GetArgumentsInParameterOrder(); + var elementNodes = new[] + { + arguments[0].Value.Syntax, + arguments[1].Value.Syntax + }; + var arrayCreationSyntax = editor.Generator.ArrayCreationExpression(charTypeExpressionSyntax, elementNodes); + var memberAccessSyntax = context.Fixer.GetMemberAccessExpressionSyntax(invocation.Syntax); + var newInvocationSyntax = editor.Generator.InvocationExpression(memberAccessSyntax, arrayCreationSyntax, arguments[2].Syntax); + + editor.ReplaceNode(invocation.Syntax, newInvocationSyntax); + + return editor.GetChangedDocument(); + } + } + + /// + /// Fixes calls to String.Split(string, int, StringSplitOptions) with an implicit char-to-int conversion. + /// + private sealed class StringSplit_StringInt32StringSplitOptions : OperationFixer + { + private StringSplit_StringInt32StringSplitOptions(IMethodSymbol splitMethod) : base(splitMethod) { } + + public static StringSplit_StringInt32StringSplitOptions? Create(Compilation compilation, RequiredSymbols symbols) + { + if (!compilation.TryGetOrCreateTypeByMetadataName(WellKnownTypeNames.SystemStringSplitOptions, out var stringSplitOptionsType)) + return null; + + var splitMethod = symbols.StringType.GetMembers(nameof(string.Split)).OfType() + .GetFirstOrDefaultMemberWithParameterTypes(symbols.StringType, symbols.Int32Type, stringSplitOptionsType); + + return splitMethod is not null ? new StringSplit_StringInt32StringSplitOptions(splitMethod) : null; + } + + public override async Task GetChangedDocumentAsync(FixCharCastContext context) + { + var editor = await DocumentEditor.CreateAsync(context.CodeFixContext.Document, context.CancellationToken).ConfigureAwait(false); + var invocation = (IInvocationOperation)context.Operation; + + var stringType = invocation.SemanticModel.Compilation.GetSpecialType(SpecialType.System_String); + var stringTypeExpressionSyntax = editor.Generator.TypeExpression(stringType); + + var arguments = invocation.GetArgumentsInParameterOrder(); + var elementNodes = new[] + { + arguments[0].Value.Syntax, + ConvertCharOperationToString(editor, ((IConversionOperation)arguments[1].Value).Operand) + }; + var arrayCreationSyntax = editor.Generator.ArrayCreationExpression(stringTypeExpressionSyntax, elementNodes); + + var optionsArgumentSyntax = arguments[2].ArgumentKind is ArgumentKind.DefaultValue ? + CreateEnumMemberAccess((INamedTypeSymbol)arguments[2].Parameter.Type, nameof(StringSplitOptions.None)) : + arguments[2].Syntax; + + var splitMemberAccessSyntax = context.Fixer.GetMemberAccessExpressionSyntax(context.Operation.Syntax); + var newInvocationSyntax = editor.Generator.InvocationExpression(splitMemberAccessSyntax, arrayCreationSyntax, optionsArgumentSyntax); + + editor.ReplaceNode(context.Operation.Syntax, newInvocationSyntax); + + return editor.GetChangedDocument(); + + // Local functions + + SyntaxNode CreateEnumMemberAccess(INamedTypeSymbol enumType, string enumMemberName) + { + RoslynDebug.Assert(editor is not null); + + var enumTypeSyntax = editor.Generator.TypeExpressionForStaticMemberAccess(enumType); + return editor.Generator.MemberAccessExpression(enumTypeSyntax, enumMemberName); + } + } + } + + /// + /// Fixes calls to new StringBuilder(int) with an implicit char-to-int conversion. + /// + private sealed class StringBuilderCtor_Int32 : OperationFixer + { + private StringBuilderCtor_Int32(IMethodSymbol ctor) : base(ctor) { } + + public static StringBuilderCtor_Int32? Create(Compilation compilation, RequiredSymbols symbols) + { + if (!compilation.TryGetOrCreateTypeByMetadataName(WellKnownTypeNames.SystemTextStringBuilder, out var stringBuilderType)) + return null; + + var ctor = stringBuilderType.GetMembers(WellKnownMemberNames.InstanceConstructorName).OfType() + .GetFirstOrDefaultMemberWithParameterTypes(symbols.Int32Type); + + return ctor is not null ? new StringBuilderCtor_Int32(ctor) : null; + } + + public override async Task GetChangedDocumentAsync(FixCharCastContext context) + { + var editor = await DocumentEditor.CreateAsync(context.CodeFixContext.Document, context.CancellationToken).ConfigureAwait(false); + var instanceCreation = (IObjectCreationOperation)context.Operation; + var argument = instanceCreation.Arguments[0]; + + var newArgumentSyntax = ConvertCharOperationToString(editor, ((IConversionOperation)argument.Value).Operand); + var newObjectCreationSyntax = editor.Generator.ObjectCreationExpression(instanceCreation.Constructor.ContainingType, newArgumentSyntax); + + editor.ReplaceNode(instanceCreation.Syntax, newObjectCreationSyntax); + + return editor.GetChangedDocument(); + } + } + + private static ImmutableDictionary CreateFixerCache(Compilation compilation) + { + if (!RequiredSymbols.TryGetSymbols(compilation, out var symbols)) + return ImmutableDictionary.Empty; + + var builder = ImmutableDictionary.CreateBuilder(); + + AddIfNotNull(StringSplit_CharInt32StringSplitOptions.Create(compilation, symbols)); + AddIfNotNull(StringSplit_StringInt32StringSplitOptions.Create(compilation, symbols)); + AddIfNotNull(StringBuilderCtor_Int32.Create(compilation, symbols)); + + return builder.ToImmutable(); + + // Local functions + + void AddIfNotNull(OperationFixer? fixer) + { + if (fixer is not null) + builder!.Add(fixer.InvokedMethod, fixer); + } + } + + private static SyntaxNode ConvertCharOperationToString(DocumentEditor editor, IOperation charOperation) + { + if (charOperation is ILiteralOperation charLiteral) + { + return editor.Generator.LiteralExpression(charLiteral.ConstantValue.Value.ToString()); + } + + var toStringMemberAccessSyntax = editor.Generator.MemberAccessExpression(charOperation.Syntax, nameof(object.ToString)); + + return editor.Generator.InvocationExpression(toStringMemberAccessSyntax); + } } } diff --git a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Runtime/SuspiciousCastFromCharToInt.cs b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Runtime/SuspiciousCastFromCharToInt.cs index df94200518..5878c31dbe 100644 --- a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Runtime/SuspiciousCastFromCharToInt.cs +++ b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Runtime/SuspiciousCastFromCharToInt.cs @@ -1,18 +1,10 @@ // 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.Generic; using System.Collections.Immutable; -using System.Diagnostics.CodeAnalysis; -using System.Linq; -using System.Threading; -using System.Threading.Tasks; using Analyzer.Utilities; using Analyzer.Utilities.Extensions; using Microsoft.CodeAnalysis; -using Microsoft.CodeAnalysis.CodeFixes; using Microsoft.CodeAnalysis.Diagnostics; -using Microsoft.CodeAnalysis.Editing; using Microsoft.CodeAnalysis.Operations; using Resx = Microsoft.NetCore.Analyzers.MicrosoftNetCoreAnalyzersResources; @@ -48,31 +40,64 @@ public override void Initialize(AnalysisContext context) private static void OnCompilationStart(CompilationStartAnalysisContext context) { - var apiCache = GetApiHandlerCache(context.Compilation); + if (!RequiredSymbols.TryGetSymbols(context.Compilation, out var symbols)) + return; + + var stringSplitMethods = GetProblematicStringSplitMethods(context.Compilation, symbols); + + // Report implicit char-to-int conversions in calls to certain overloads of string.Split + context.RegisterOperationAction(context => + { + var invocation = (IInvocationOperation)context.Operation; + + if (stringSplitMethods.Contains(invocation.TargetMethod)) + { + foreach (var argument in invocation.Arguments) + { + if (IsImplicitCharToIntConversion(argument)) + { + context.ReportDiagnostic(argument.CreateDiagnostic(Rule)); + } + } + } + }, OperationKind.Invocation); - if (apiCache.IsEmpty) + if (!context.Compilation.TryGetOrCreateTypeByMetadataName(WellKnownTypeNames.SystemTextStringBuilder, out var stringBuilderType)) return; + // Report implicit char-to-int conversions in calls to all StringBuilder constructors context.RegisterOperationAction(context => { - var targetMethod = context.Operation switch + var creation = (IObjectCreationOperation)context.Operation; + + if (!creation.Constructor.ContainingType.Equals(stringBuilderType, SymbolEqualityComparer.Default)) + return; + + foreach (var argument in creation.Arguments) { - IInvocationOperation invocation => invocation.TargetMethod, - IObjectCreationOperation objectCreation => objectCreation.Constructor, - _ => default - }; + if (IsImplicitCharToIntConversion(argument)) + { + context.ReportDiagnostic(argument.CreateDiagnostic(Rule)); + } + } + }, OperationKind.ObjectCreation); - RoslynDebug.Assert(targetMethod is not null, $"{nameof(targetMethod)} must not be null."); + return; - if (apiCache.TryGetValue(targetMethod, out var handler)) - handler.AnalyzeInvocationOperation(context); + // Local functions - }, OperationKind.Invocation, OperationKind.ObjectCreation); + bool IsImplicitCharToIntConversion(IArgumentOperation argument) + { + return argument.Value is IConversionOperation conversion && + conversion.IsImplicit && + conversion.Operand.Type.Equals(symbols.CharType, SymbolEqualityComparer.Default) && + argument.Parameter.Type.Equals(symbols.Int32Type, SymbolEqualityComparer.Default); + } } // Property bag #pragma warning disable CA1815 // Override equals and operator equals on value types - internal struct RequiredSymbols + internal readonly struct RequiredSymbols #pragma warning restore CA1815 // Override equals and operator equals on value types { private RequiredSymbols(ITypeSymbol charType, ITypeSymbol int32Type, ITypeSymbol stringType) @@ -103,94 +128,82 @@ public static bool TryGetSymbols(Compilation compilation, out RequiredSymbols re public ITypeSymbol StringType { get; } } - // Property bag -#pragma warning disable CA1815 // Override equals and operator equals on value types - internal struct FixCharCastContext -#pragma warning restore CA1815 // Override equals and operator equals on value types + /// + /// Gets the overloads of string.Split that may invite unintentional casts from to . + /// + private static ImmutableHashSet GetProblematicStringSplitMethods(Compilation compilation, RequiredSymbols symbols) { - public FixCharCastContext(CodeFixContext codeFixContext, IOperation operation, SuspiciousCastFromCharToIntFixer fixer, CancellationToken cancellationToken) + if (!compilation.TryGetOrCreateTypeByMetadataName(WellKnownTypeNames.SystemStringSplitOptions, out var stringSplitOptionsType)) + return ImmutableHashSet.Empty; + + var builder = ImmutableHashSet.CreateBuilder(SymbolEqualityComparer.Default); + var splitMembers = symbols.StringType.GetMembers(nameof(string.Split)).OfType(); + + AddIfNotNull(splitMembers.GetFirstOrDefaultMemberWithParameterTypes(symbols.CharType, symbols.Int32Type, stringSplitOptionsType)); + AddIfNotNull(splitMembers.GetFirstOrDefaultMemberWithParameterTypes(symbols.StringType, symbols.Int32Type, stringSplitOptionsType)); + + return builder.ToImmutable(); + + // Local functions + + void AddIfNotNull(IMethodSymbol? method) { - CodeFixContext = codeFixContext; - Operation = operation; - Fixer = fixer; - CancellationToken = cancellationToken; + if (method is not null) + builder!.Add(method); } - - public CodeFixContext CodeFixContext { get; } - public IOperation Operation { get; } - public SuspiciousCastFromCharToIntFixer Fixer { get; } - public CancellationToken CancellationToken { get; } } - internal abstract class ApiHandler +#if false + private abstract class OperationAnalyzer { - protected ApiHandler(IMethodSymbol method) + protected OperationAnalyzer(IMethodSymbol invokedMethod) { - Method = method; + InvokedMethod = invokedMethod; } - public IMethodSymbol Method { get; } + public IMethodSymbol InvokedMethod { get; } - public abstract void AnalyzeInvocationOperation(OperationAnalysisContext context); - - public abstract Task CreateChangedDocumentAsync(FixCharCastContext context); + public abstract void AnalyzeOperation(OperationAnalysisContext context); } - private sealed class StringSplitCharInt32StringSplitOptionsHandler : ApiHandler + /// + /// Analyze string.Split(char, int, StringSplitOptions) calls. + /// + private sealed class StringSplit_CharInt32StringSplitOptions : OperationAnalyzer { - private StringSplitCharInt32StringSplitOptionsHandler(IMethodSymbol splitMethod) : base(splitMethod) { } + private StringSplit_CharInt32StringSplitOptions(IMethodSymbol stringSplit) : base(stringSplit) { } - public static StringSplitCharInt32StringSplitOptionsHandler? Create(Compilation compilation, RequiredSymbols symbols) + public static StringSplit_CharInt32StringSplitOptions? Create(Compilation compilation, RequiredSymbols symbols) { - if (!compilation.TryGetOrCreateTypeByMetadataName(WellKnownTypeNames.SystemStringSplitOptions, out var stringSplitOptions)) + if (!compilation.TryGetOrCreateTypeByMetadataName(WellKnownTypeNames.SystemStringSplitOptions, out var stringSplitOptionsType)) return null; var splitMethod = symbols.StringType.GetMembers(nameof(string.Split)).OfType() - .GetFirstOrDefaultMemberWithParameterTypes(symbols.CharType, symbols.Int32Type, stringSplitOptions); + .GetFirstOrDefaultMemberWithParameterTypes(symbols.CharType, symbols.Int32Type, stringSplitOptionsType); - return splitMethod is not null ? new StringSplitCharInt32StringSplitOptionsHandler(splitMethod) : null; + return splitMethod is not null ? new StringSplit_CharInt32StringSplitOptions(splitMethod) : null; } - public override void AnalyzeInvocationOperation(OperationAnalysisContext context) + public override void AnalyzeOperation(OperationAnalysisContext context) { var invocation = (IInvocationOperation)context.Operation; var argument = invocation.Arguments.First(x => x.Parameter.Ordinal == 1); - if (argument.Value is IConversionOperation conversion && conversion.IsImplicit && conversion.Operand.Type.SpecialType == SpecialType.System_Char) + if (argument.Value is IConversionOperation conversion && conversion.IsImplicit && conversion.Operand.Type.SpecialType is SpecialType.System_Char) { context.ReportDiagnostic(argument.CreateDiagnostic(Rule)); } } - - public override async Task CreateChangedDocumentAsync(FixCharCastContext context) - { - var doc = context.CodeFixContext.Document; - var token = context.CancellationToken; - var invocation = (IInvocationOperation)context.Operation; - var editor = await DocumentEditor.CreateAsync(doc, token).ConfigureAwait(false); - - var charTypeExpressionSyntax = editor.Generator.TypeExpression(invocation.SemanticModel.Compilation.GetSpecialType(SpecialType.System_Char)); - var arguments = invocation.GetArgumentsInParameterOrder(); - var elementNodes = new[] - { - arguments[0].Value.Syntax, - arguments[1].Value.Syntax - }; - var arrayCreationSyntax = editor.Generator.ArrayCreationExpression(charTypeExpressionSyntax, elementNodes); - var memberAccessSyntax = context.Fixer.GetMemberAccessExpressionSyntax(invocation.Syntax); - var newInvocationSyntax = editor.Generator.InvocationExpression(memberAccessSyntax, arrayCreationSyntax, arguments[2].Syntax); - - editor.ReplaceNode(invocation.Syntax, newInvocationSyntax); - - return editor.GetChangedDocument(); - } } - private sealed class StringSplitStringInt32StringSplitOptionsHandler : ApiHandler + /// + /// Analyze string.Split(string, int, StringSplitOptions) calls + /// + private sealed class StringSplit_StringInt32StringSplitOptions : OperationAnalyzer { - private StringSplitStringInt32StringSplitOptionsHandler(IMethodSymbol splitMethod) : base(splitMethod) { } + private StringSplit_StringInt32StringSplitOptions(IMethodSymbol stringSplit) : base(stringSplit) { } - public static StringSplitStringInt32StringSplitOptionsHandler? Create(Compilation compilation, RequiredSymbols symbols) + public static StringSplit_StringInt32StringSplitOptions? Create(Compilation compilation, RequiredSymbols symbols) { if (!compilation.TryGetOrCreateTypeByMetadataName(WellKnownTypeNames.SystemStringSplitOptions, out var stringSplitOptionsType)) return null; @@ -198,132 +211,72 @@ private StringSplitStringInt32StringSplitOptionsHandler(IMethodSymbol splitMetho var splitMethod = symbols.StringType.GetMembers(nameof(string.Split)).OfType() .GetFirstOrDefaultMemberWithParameterTypes(symbols.StringType, symbols.Int32Type, stringSplitOptionsType); - return splitMethod is not null ? new StringSplitStringInt32StringSplitOptionsHandler(splitMethod) : null; + return splitMethod is not null ? new StringSplit_StringInt32StringSplitOptions(splitMethod) : null; } - public override void AnalyzeInvocationOperation(OperationAnalysisContext context) + public override void AnalyzeOperation(OperationAnalysisContext context) { var invocation = (IInvocationOperation)context.Operation; var argument = invocation.Arguments.First(x => x.Parameter.Ordinal == 1); - if (argument.Value is IConversionOperation conversion && conversion.IsImplicit && conversion.Operand.Type.SpecialType == SpecialType.System_Char) + if (argument.Value is IConversionOperation conversion && conversion.IsImplicit && conversion.Operand.Type.SpecialType is SpecialType.System_Char) { context.ReportDiagnostic(argument.CreateDiagnostic(Rule)); } } - - public override async Task CreateChangedDocumentAsync(FixCharCastContext context) - { - var editor = await DocumentEditor.CreateAsync(context.CodeFixContext.Document, context.CancellationToken).ConfigureAwait(false); - var invocation = (IInvocationOperation)context.Operation; - - var stringType = invocation.SemanticModel.Compilation.GetSpecialType(SpecialType.System_String); - var stringTypeExpressionSyntax = editor.Generator.TypeExpression(stringType); - - var arguments = invocation.GetArgumentsInParameterOrder(); - var elementNodes = new[] - { - arguments[0].Value.Syntax, - ConvertCharOperationToString(editor, ((IConversionOperation)arguments[1].Value).Operand) - }; - var arrayCreationSyntax = editor.Generator.ArrayCreationExpression(stringTypeExpressionSyntax, elementNodes); - - var optionsArgumentSyntax = arguments[2].ArgumentKind is ArgumentKind.DefaultValue ? - CreateEnumMemberAccess((INamedTypeSymbol)arguments[2].Parameter.Type, nameof(StringSplitOptions.None)) : - arguments[2].Syntax; - - var splitMemberAccessSyntax = context.Fixer.GetMemberAccessExpressionSyntax(context.Operation.Syntax); - var newInvocationSyntax = editor.Generator.InvocationExpression(splitMemberAccessSyntax, arrayCreationSyntax, optionsArgumentSyntax); - - editor.ReplaceNode(context.Operation.Syntax, newInvocationSyntax); - - return editor.GetChangedDocument(); - - // Local functions - - SyntaxNode CreateEnumMemberAccess(INamedTypeSymbol enumType, string enumMemberName) - { - RoslynDebug.Assert(editor is not null); - - var enumTypeSyntax = editor.Generator.TypeExpressionForStaticMemberAccess(enumType); - return editor.Generator.MemberAccessExpression(enumTypeSyntax, enumMemberName); - } - } } - private sealed class StringBuilderInt32Handler : ApiHandler + /// + /// Analyze new StringBuilder(int) calls. + /// + private sealed class StringBuilderCtor_Int32 : OperationAnalyzer { - private StringBuilderInt32Handler(IMethodSymbol ctor) : base(ctor) { } + private StringBuilderCtor_Int32(IMethodSymbol ctor) : base(ctor) { } - public static StringBuilderInt32Handler? Create(Compilation compilation, RequiredSymbols symbols) + public static StringBuilderCtor_Int32? Create(Compilation compilation, RequiredSymbols symbols) { if (!compilation.TryGetOrCreateTypeByMetadataName(WellKnownTypeNames.SystemTextStringBuilder, out var stringBuilderType)) return null; - var ctor = stringBuilderType.GetMembers(WellKnownMemberNames.InstanceConstructorName).Cast() + var ctor = stringBuilderType.GetMembers(WellKnownMemberNames.InstanceConstructorName).OfType() .GetFirstOrDefaultMemberWithParameterTypes(symbols.Int32Type); - return ctor is not null ? new StringBuilderInt32Handler(ctor) : null; + return ctor is not null ? new StringBuilderCtor_Int32(ctor) : null; } - public override void AnalyzeInvocationOperation(OperationAnalysisContext context) + public override void AnalyzeOperation(OperationAnalysisContext context) { var objectCreation = (IObjectCreationOperation)context.Operation; var argument = objectCreation.Arguments[0]; - if (argument.Value is IConversionOperation conversion && conversion.IsImplicit && conversion.Operand.Type.SpecialType == SpecialType.System_Char) + if (argument.Value is IConversionOperation conversion && conversion.IsImplicit && conversion.Operand.Type.SpecialType is SpecialType.System_Char) { context.ReportDiagnostic(argument.CreateDiagnostic(Rule)); } } - - public override async Task CreateChangedDocumentAsync(FixCharCastContext context) - { - var editor = await DocumentEditor.CreateAsync(context.CodeFixContext.Document, context.CancellationToken).ConfigureAwait(false); - var instanceCreation = (IObjectCreationOperation)context.Operation; - var argument = instanceCreation.Arguments[0]; - - var newArgumentSyntax = ConvertCharOperationToString(editor, ((IConversionOperation)argument.Value).Operand); - var newObjectCreationSyntax = editor.Generator.ObjectCreationExpression(instanceCreation.Constructor.ContainingType, newArgumentSyntax); - - editor.ReplaceNode(instanceCreation.Syntax, newObjectCreationSyntax); - - return editor.GetChangedDocument(); - } } - internal static ImmutableDictionary GetApiHandlerCache(Compilation compilation) + private static ImmutableDictionary CreateAnalyzerCache(Compilation compilation) { if (!RequiredSymbols.TryGetSymbols(compilation, out var symbols)) - return ImmutableDictionary.Empty; + return ImmutableDictionary.Empty; - var builder = ImmutableDictionary.CreateBuilder(); + var builder = ImmutableDictionary.CreateBuilder(); - AddIfNotNull(StringSplitCharInt32StringSplitOptionsHandler.Create(compilation, symbols)); - AddIfNotNull(StringSplitStringInt32StringSplitOptionsHandler.Create(compilation, symbols)); - AddIfNotNull(StringBuilderInt32Handler.Create(compilation, symbols)); + AddIfNotNull(StringSplit_CharInt32StringSplitOptions.Create(compilation, symbols)); + AddIfNotNull(StringSplit_StringInt32StringSplitOptions.Create(compilation, symbols)); + AddIfNotNull(StringBuilderCtor_Int32.Create(compilation, symbols)); return builder.ToImmutable(); - // Local functions + // Local functions - void AddIfNotNull(ApiHandler? handler) + void AddIfNotNull(OperationAnalyzer? analyzer) { - if (handler is not null) - builder!.Add(handler.Method, handler); + if (analyzer is not null) + builder!.Add(analyzer.InvokedMethod, analyzer); } } - - private static SyntaxNode ConvertCharOperationToString(DocumentEditor editor, IOperation charOperation) - { - if (charOperation is ILiteralOperation charLiteral) - { - return editor.Generator.LiteralExpression(charLiteral.ConstantValue.Value.ToString()); - } - - var toStringMemberAccessSyntax = editor.Generator.MemberAccessExpression(charOperation.Syntax, nameof(object.ToString)); - - return editor.Generator.InvocationExpression(toStringMemberAccessSyntax); - } +#endif } } diff --git a/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/Runtime/SuspiciousCastFromCharToIntTests.cs b/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/Runtime/SuspiciousCastFromCharToIntTests.cs index 4ae2171598..6e726d6060 100644 --- a/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/Runtime/SuspiciousCastFromCharToIntTests.cs +++ b/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/Runtime/SuspiciousCastFromCharToIntTests.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.Globalization; +using System.Linq; using System.Threading.Tasks; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.Testing; @@ -14,40 +15,6 @@ namespace Microsoft.NetCore.Analyzers.Runtime.UnitTests { public class SuspiciousCastFromCharToIntTests { - [Theory] - [InlineData("'a'")] - [InlineData("_char")] - [InlineData("GetChar()")] - public Task StringSplit_CharInt32StringSplitOptions_Diagnostic_CS(string charArgument) - { - string source = $@" -public class Testopolis -{{ - private char _char; - private char GetChar() => _char; - public void M() - {{ - nameof(M).Split('c', {{|#0:{charArgument}|}}, System.StringSplitOptions.None); - }} -}}"; - string fixedSource = $@" -public class Testopolis -{{ - private char _char; - private char GetChar() => _char; - public void M() - {{ - nameof(M).Split(new char[] {{ 'c', {charArgument} }}, System.StringSplitOptions.None); - }} -}}"; - var diagnostics = new[] - { - VerifyCS.Diagnostic(Rule).WithLocation(0) - }; - - return VerifyCS.VerifyCodeFixAsync(source, diagnostics, fixedSource); - } - [Theory] [InlineData( "'c', {|#0:'a'|}, System.StringSplitOptions.RemoveEmptyEntries", @@ -58,7 +25,7 @@ public void M() [InlineData( "options: System.StringSplitOptions.None, separator: 'c', {|#0:count: 'a'|}", "new char[] { 'c', 'a' }, options: System.StringSplitOptions.None")] - public Task StringSplit_CharInt32StringSplitOptions_NamedArguments_Diagnostic_CS(string testArgumentList, string fixedArgumentList) + public Task StringSplit_CharInt32StringSplitOptions_Fixed_CS(string testArgumentList, string fixedArgumentList) { string source = $@" public class Testopolis @@ -91,7 +58,7 @@ public void M() [InlineData("{|#0:count: c|}, separator: s", "new string[] { s, c.ToString() }, System.StringSplitOptions.None")] [InlineData("options: System.StringSplitOptions.None, {|#0:count: c|}, separator: s", "new string[] { s, c.ToString() }, options: System.StringSplitOptions.None")] [InlineData("s, {|#0:'a'|}", @"new string[] { s, ""a"" }, System.StringSplitOptions.None")] - public Task StringSplit_StringInt32StringSplitOptions_NamedArguments_Diagnostic_CS(string testArgumentList, string fixedArgumentList) + public Task StringSplit_StringInt32StringSplitOptions_Fixed_CS(string testArgumentList, string fixedArgumentList) { string format = @" public class Testopolis @@ -119,12 +86,51 @@ public void M(string s, char c) return test.RunAsync(); } + [Theory] + [InlineData("ss, c, o")] + [InlineData("cc, c, o")] + [InlineData("cc, c")] + public Task StringSplit_NonProblematic_NoDiagnostic_CS(string argumentList) + { + string source = $@" +using System; +public class Testopolis +{{ + public void M(string s, char c, string[] ss, char[] cc, StringSplitOptions o) + {{ + nameof(M).Split({argumentList}); + }} +}}"; + + return VerifyCS.VerifyAnalyzerAsync(source); + } + + [Theory] + [InlineData("c, n, o")] + [InlineData("c, (int)c, o")] + [InlineData("s, n, o")] + [InlineData("s, (int)c, o")] + public Task StringSplit_NoImplicitConversion_NoDiagnostic_CS(string argumentList) + { + string source = $@" +using System; +public class Testopolis +{{ + public void M(string s, char c, int n, StringSplitOptions o) + {{ + nameof(M).Split({argumentList}); + }} +}}"; + + return VerifyCS.VerifyAnalyzerAsync(source); + } + [Theory] [InlineData("{|#0:c|}", "c.ToString()")] [InlineData("{|#0:'a'|}", @"""a""")] [InlineData("{|#0:capacity: c|}", "c.ToString()")] [InlineData("{|#0:capacity: 'a'|}", @"""a""")] - public Task StringBuilder_Int32_Diagnostic_CS(string testArgumentList, string fixedArgumentList) + public Task StringBuilderCtor_Int32_Fixed_CS(string testArgumentList, string fixedArgumentList) { string format = @" using System.Text; @@ -153,6 +159,54 @@ public void M(string s, char c, int n) return test.RunAsync(); } + [Theory] + [InlineData("{|#0:c|}, n")] + [InlineData("n, {|#0:c|}")] + [InlineData("{|#0:c|}, {|#1:c|}", 2)] + [InlineData("s, {|#0:c|}")] + [InlineData("s, n, n, {|#0:c|}")] + [InlineData("s, {|#0:c|}, n, n")] + [InlineData("s, {|#0:c|}, {|#1:c|}, {|#2:c|}", 3)] + public Task StringBuilderCtor_NonFixable_Diagnostic_CS(string argumentList, int diagnosticsCount = 1) + { + string source = $@" +using System.Text; +public class Testopolis +{{ + public void M(string s, char c, int n) + {{ + new StringBuilder({argumentList}); + }} +}}"; + var diagnostics = Enumerable.Range(0, diagnosticsCount).Select(x => VerifyCS.Diagnostic(Rule).WithLocation(x)).ToArray(); + + return VerifyCS.VerifyAnalyzerAsync(source, diagnostics); + } + + [Theory] + [InlineData("n")] + [InlineData("(int)c")] + [InlineData("n, n")] + [InlineData("(int)c, (int)c")] + [InlineData("s, n")] + [InlineData("s, (int)c")] + [InlineData("s, n, n, n")] + [InlineData("s, (int)c, n, (int)c")] + public Task StringBuilderCtor_NoImplicitConversions_NoDiagnostic_CS(string argumentList) + { + string source = $@" +using System.Text; +public class Testopolis +{{ + public void M(string s, char c, int n) + {{ + new StringBuilder({argumentList}); + }} +}}"; + + return VerifyCS.VerifyAnalyzerAsync(source); + } + private static DiagnosticDescriptor Rule => SuspiciousCastFromCharToIntAnalyzer.Rule; } } From 08940a309a1f5f30ac6c151ef02d2083e0d743f2 Mon Sep 17 00:00:00 2001 From: Newell Clark Date: Sat, 18 Sep 2021 20:19:18 -0400 Subject: [PATCH 3/9] Fix DiagnosticCategoryAndIdRanges.txt --- .../Runtime/SuspiciousCastFromCharToInt.cs | 126 ------------------ .../DiagnosticCategoryAndIdRanges.txt | 2 +- 2 files changed, 1 insertion(+), 127 deletions(-) diff --git a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Runtime/SuspiciousCastFromCharToInt.cs b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Runtime/SuspiciousCastFromCharToInt.cs index 5878c31dbe..0977a343c8 100644 --- a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Runtime/SuspiciousCastFromCharToInt.cs +++ b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Runtime/SuspiciousCastFromCharToInt.cs @@ -152,131 +152,5 @@ void AddIfNotNull(IMethodSymbol? method) builder!.Add(method); } } - -#if false - private abstract class OperationAnalyzer - { - protected OperationAnalyzer(IMethodSymbol invokedMethod) - { - InvokedMethod = invokedMethod; - } - - public IMethodSymbol InvokedMethod { get; } - - public abstract void AnalyzeOperation(OperationAnalysisContext context); - } - - /// - /// Analyze string.Split(char, int, StringSplitOptions) calls. - /// - private sealed class StringSplit_CharInt32StringSplitOptions : OperationAnalyzer - { - private StringSplit_CharInt32StringSplitOptions(IMethodSymbol stringSplit) : base(stringSplit) { } - - public static StringSplit_CharInt32StringSplitOptions? Create(Compilation compilation, RequiredSymbols symbols) - { - if (!compilation.TryGetOrCreateTypeByMetadataName(WellKnownTypeNames.SystemStringSplitOptions, out var stringSplitOptionsType)) - return null; - - var splitMethod = symbols.StringType.GetMembers(nameof(string.Split)).OfType() - .GetFirstOrDefaultMemberWithParameterTypes(symbols.CharType, symbols.Int32Type, stringSplitOptionsType); - - return splitMethod is not null ? new StringSplit_CharInt32StringSplitOptions(splitMethod) : null; - } - - public override void AnalyzeOperation(OperationAnalysisContext context) - { - var invocation = (IInvocationOperation)context.Operation; - var argument = invocation.Arguments.First(x => x.Parameter.Ordinal == 1); - - if (argument.Value is IConversionOperation conversion && conversion.IsImplicit && conversion.Operand.Type.SpecialType is SpecialType.System_Char) - { - context.ReportDiagnostic(argument.CreateDiagnostic(Rule)); - } - } - } - - /// - /// Analyze string.Split(string, int, StringSplitOptions) calls - /// - private sealed class StringSplit_StringInt32StringSplitOptions : OperationAnalyzer - { - private StringSplit_StringInt32StringSplitOptions(IMethodSymbol stringSplit) : base(stringSplit) { } - - public static StringSplit_StringInt32StringSplitOptions? Create(Compilation compilation, RequiredSymbols symbols) - { - if (!compilation.TryGetOrCreateTypeByMetadataName(WellKnownTypeNames.SystemStringSplitOptions, out var stringSplitOptionsType)) - return null; - - var splitMethod = symbols.StringType.GetMembers(nameof(string.Split)).OfType() - .GetFirstOrDefaultMemberWithParameterTypes(symbols.StringType, symbols.Int32Type, stringSplitOptionsType); - - return splitMethod is not null ? new StringSplit_StringInt32StringSplitOptions(splitMethod) : null; - } - - public override void AnalyzeOperation(OperationAnalysisContext context) - { - var invocation = (IInvocationOperation)context.Operation; - var argument = invocation.Arguments.First(x => x.Parameter.Ordinal == 1); - - if (argument.Value is IConversionOperation conversion && conversion.IsImplicit && conversion.Operand.Type.SpecialType is SpecialType.System_Char) - { - context.ReportDiagnostic(argument.CreateDiagnostic(Rule)); - } - } - } - - /// - /// Analyze new StringBuilder(int) calls. - /// - private sealed class StringBuilderCtor_Int32 : OperationAnalyzer - { - private StringBuilderCtor_Int32(IMethodSymbol ctor) : base(ctor) { } - - public static StringBuilderCtor_Int32? Create(Compilation compilation, RequiredSymbols symbols) - { - if (!compilation.TryGetOrCreateTypeByMetadataName(WellKnownTypeNames.SystemTextStringBuilder, out var stringBuilderType)) - return null; - - var ctor = stringBuilderType.GetMembers(WellKnownMemberNames.InstanceConstructorName).OfType() - .GetFirstOrDefaultMemberWithParameterTypes(symbols.Int32Type); - - return ctor is not null ? new StringBuilderCtor_Int32(ctor) : null; - } - - public override void AnalyzeOperation(OperationAnalysisContext context) - { - var objectCreation = (IObjectCreationOperation)context.Operation; - var argument = objectCreation.Arguments[0]; - - if (argument.Value is IConversionOperation conversion && conversion.IsImplicit && conversion.Operand.Type.SpecialType is SpecialType.System_Char) - { - context.ReportDiagnostic(argument.CreateDiagnostic(Rule)); - } - } - } - - private static ImmutableDictionary CreateAnalyzerCache(Compilation compilation) - { - if (!RequiredSymbols.TryGetSymbols(compilation, out var symbols)) - return ImmutableDictionary.Empty; - - var builder = ImmutableDictionary.CreateBuilder(); - - AddIfNotNull(StringSplit_CharInt32StringSplitOptions.Create(compilation, symbols)); - AddIfNotNull(StringSplit_StringInt32StringSplitOptions.Create(compilation, symbols)); - AddIfNotNull(StringBuilderCtor_Int32.Create(compilation, symbols)); - - return builder.ToImmutable(); - - // Local functions - - void AddIfNotNull(OperationAnalyzer? analyzer) - { - if (analyzer is not null) - builder!.Add(analyzer.InvokedMethod, analyzer); - } - } -#endif } } diff --git a/src/Utilities/Compiler/DiagnosticCategoryAndIdRanges.txt b/src/Utilities/Compiler/DiagnosticCategoryAndIdRanges.txt index a330cba22d..f6058b8216 100644 --- a/src/Utilities/Compiler/DiagnosticCategoryAndIdRanges.txt +++ b/src/Utilities/Compiler/DiagnosticCategoryAndIdRanges.txt @@ -19,7 +19,7 @@ Naming: CA1700-CA1727 Interoperability: CA1400-CA1419 Maintainability: CA1500-CA1509 Reliability: CA9998-CA9999, CA2000-CA2018 -Correctness: CA3200 +Correctness: CA3200, RS1000-RS1999 Documentation: CA1200-CA1200 # Microsoft CodeAnalysis API rules From 0ca57b73eeb947e34b649b14b41dd0657d5735a8 Mon Sep 17 00:00:00 2001 From: Newell Clark Date: Sat, 18 Sep 2021 20:39:15 -0400 Subject: [PATCH 4/9] Run msbuild --- .../Microsoft.CodeAnalysis.NetAnalyzers.md | 12 +++++++++++ .../Microsoft.CodeAnalysis.NetAnalyzers.sarif | 20 +++++++++++++++++++ src/NetAnalyzers/RulesMissingDocumentation.md | 1 + 3 files changed, 33 insertions(+) diff --git a/src/NetAnalyzers/Microsoft.CodeAnalysis.NetAnalyzers.md b/src/NetAnalyzers/Microsoft.CodeAnalysis.NetAnalyzers.md index e545b419f3..3283a479cd 100644 --- a/src/NetAnalyzers/Microsoft.CodeAnalysis.NetAnalyzers.md +++ b/src/NetAnalyzers/Microsoft.CodeAnalysis.NetAnalyzers.md @@ -2640,6 +2640,18 @@ Missing ValidateAntiForgeryTokenAttribute on controller action {0} |CodeFix|False| --- +## [CA3200](https://docs.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca3200): Suspicious cast from char to int + +Implicit cast from char to int in method call is suspicious. Consider using a different overload. + +|Item|Value| +|-|-| +|Category|Correctness| +|Enabled|True| +|Severity|Warning| +|CodeFix|True| +--- + ## [CA5350](https://docs.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca5350): Do Not Use Weak Cryptographic Algorithms Cryptographic algorithms degrade over time as attacks become for advances to attacker get access to more computation. Depending on the type and application of this cryptographic algorithm, further degradation of the cryptographic strength of it may allow attackers to read enciphered messages, tamper with enciphered  messages, forge digital signatures, tamper with hashed content, or otherwise compromise any cryptosystem based on this algorithm. Replace encryption uses with the AES algorithm (AES-256, AES-192 and AES-128 are acceptable) with a key length greater than or equal to 128 bits. Replace hashing uses with a hashing function in the SHA-2 family, such as SHA-2 512, SHA-2 384, or SHA-2 256. diff --git a/src/NetAnalyzers/Microsoft.CodeAnalysis.NetAnalyzers.sarif b/src/NetAnalyzers/Microsoft.CodeAnalysis.NetAnalyzers.sarif index 33ffe662a9..4cef1a3202 100644 --- a/src/NetAnalyzers/Microsoft.CodeAnalysis.NetAnalyzers.sarif +++ b/src/NetAnalyzers/Microsoft.CodeAnalysis.NetAnalyzers.sarif @@ -4500,6 +4500,26 @@ ] } }, + "CA3200": { + "id": "CA3200", + "shortDescription": "Suspicious cast from char to int", + "fullDescription": "Implicit cast from char to int in method call is suspicious. Consider using a different overload.", + "defaultLevel": "warning", + "helpUri": "https://docs.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca3200", + "properties": { + "category": "Correctness", + "isEnabledByDefault": true, + "typeName": "SuspiciousCastFromCharToIntAnalyzer", + "languages": [ + "C#", + "Visual Basic" + ], + "tags": [ + "Telemetry", + "EnabledRuleInAggressiveMode" + ] + } + }, "CA5350": { "id": "CA5350", "shortDescription": "Do Not Use Weak Cryptographic Algorithms", diff --git a/src/NetAnalyzers/RulesMissingDocumentation.md b/src/NetAnalyzers/RulesMissingDocumentation.md index 1113031ae2..526f925192 100644 --- a/src/NetAnalyzers/RulesMissingDocumentation.md +++ b/src/NetAnalyzers/RulesMissingDocumentation.md @@ -18,3 +18,4 @@ CA2255 | | All members declared in parent interfaces must have an implementation in a DynamicInterfaceCastableImplementation-attributed interface | CA2257 | | Members defined on an interface with the 'DynamicInterfaceCastableImplementationAttribute' should be 'static' | CA2258 | | Providing a 'DynamicInterfaceCastableImplementation' interface in Visual Basic is unsupported | +CA3200 | | Suspicious cast from char to int | From f1b3447f04706d5989da6ee83392e6dc6e182f92 Mon Sep 17 00:00:00 2001 From: Newell Clark Date: Sun, 19 Sep 2021 10:37:35 -0400 Subject: [PATCH 5/9] Fix possible null-deref - Fix possible null-deref in new IOperationExtensions.GetArgumentsInParameterOrder() method --- src/Utilities/Compiler/Extensions/IOperationExtensions.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Utilities/Compiler/Extensions/IOperationExtensions.cs b/src/Utilities/Compiler/Extensions/IOperationExtensions.cs index b0e4914a85..59e9bf10e8 100644 --- a/src/Utilities/Compiler/Extensions/IOperationExtensions.cs +++ b/src/Utilities/Compiler/Extensions/IOperationExtensions.cs @@ -62,11 +62,11 @@ internal static partial class IOperationExtensions /// public static ImmutableArray GetArgumentsInParameterOrder(this IInvocationOperation invocation) { - var result = ImmutableArray.CreateBuilder(invocation.Arguments.Length); - result.Count = invocation.Arguments.Length; + var result = invocation.Arguments.ToBuilder(); - foreach (var argument in invocation.Arguments) - result[argument.Parameter.Ordinal] = argument; + // IArgumentOperation.Parameter can be null if the argument is an __arglist argument. + // Sort __arglist arguments last, since __arglist parameters must always be declared last. + result.Sort((x, y) => (x.Parameter?.Ordinal ?? int.MaxValue).CompareTo(y.Parameter?.Ordinal ?? int.MaxValue)); return result.MoveToImmutable(); } From fef00999c5cd419ab5a4158464a955932bc52425 Mon Sep 17 00:00:00 2001 From: Newell Clark Date: Sun, 19 Sep 2021 11:33:07 -0400 Subject: [PATCH 6/9] Fix tests that failed in legacy netframework --- .../SuspiciousCastFromCharToIntTests.cs | 47 ++++++++++++------- 1 file changed, 30 insertions(+), 17 deletions(-) diff --git a/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/Runtime/SuspiciousCastFromCharToIntTests.cs b/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/Runtime/SuspiciousCastFromCharToIntTests.cs index 6e726d6060..7a4d2ca473 100644 --- a/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/Runtime/SuspiciousCastFromCharToIntTests.cs +++ b/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/Runtime/SuspiciousCastFromCharToIntTests.cs @@ -27,29 +27,32 @@ public class SuspiciousCastFromCharToIntTests "new char[] { 'c', 'a' }, options: System.StringSplitOptions.None")] public Task StringSplit_CharInt32StringSplitOptions_Fixed_CS(string testArgumentList, string fixedArgumentList) { - string source = $@" -public class Testopolis -{{ - private char _char; - private char GetChar() => _char; - public void M() - {{ - nameof(M).Split({testArgumentList}); - }} -}}"; - string fixedSource = $@" + string format = @" public class Testopolis {{ private char _char; private char GetChar() => _char; public void M() {{ - nameof(M).Split({fixedArgumentList}); + nameof(M).Split({0}); }} }}"; - var diagnostic = VerifyCS.Diagnostic(Rule).WithLocation(0); + var test = new VerifyCS.Test + { + TestState = + { + Sources = { string.Format(CultureInfo.InvariantCulture, format, testArgumentList) }, + ExpectedDiagnostics = { VerifyCS.Diagnostic(Rule).WithLocation(0) }, + ReferenceAssemblies = ReferenceAssemblies.Net.Net50 + }, + FixedState = + { + Sources = { string.Format(CultureInfo.InvariantCulture, format, fixedArgumentList) }, + ReferenceAssemblies = ReferenceAssemblies.Net.Net50 + } + }; - return VerifyCS.VerifyCodeFixAsync(source, diagnostic, fixedSource); + return test.RunAsync(); } [Theory] @@ -112,7 +115,13 @@ public void M(string s, char c, string[] ss, char[] cc, StringSplitOptions o) [InlineData("s, (int)c, o")] public Task StringSplit_NoImplicitConversion_NoDiagnostic_CS(string argumentList) { - string source = $@" + var test = new VerifyCS.Test + { + TestState = + { + Sources = + { + $@" using System; public class Testopolis {{ @@ -120,9 +129,13 @@ public void M(string s, char c, int n, StringSplitOptions o) {{ nameof(M).Split({argumentList}); }} -}}"; +}}" + }, + ReferenceAssemblies = ReferenceAssemblies.Net.Net50 + } + }; - return VerifyCS.VerifyAnalyzerAsync(source); + return test.RunAsync(); } [Theory] From d8fa0ae187347ca8660eae14875dca14af07ad12 Mon Sep 17 00:00:00 2001 From: Newell Clark Date: Mon, 20 Sep 2021 09:40:54 -0400 Subject: [PATCH 7/9] Apply suggested changes --- .../Core/AnalyzerReleases.Unshipped.md | 2 +- .../SuspiciousCastFromCharToInt.Fixer.cs | 17 ++++-- .../Runtime/SuspiciousCastFromCharToInt.cs | 53 ++++++------------- .../Microsoft.CodeAnalysis.NetAnalyzers.md | 24 ++++----- .../Microsoft.CodeAnalysis.NetAnalyzers.sarif | 40 +++++++------- src/NetAnalyzers/RulesMissingDocumentation.md | 2 +- .../SuspiciousCastFromCharToIntTests.cs | 27 ++++++++-- src/Utilities/Compiler/DiagnosticCategory.cs | 1 - .../DiagnosticCategoryAndIdRanges.txt | 3 +- .../Extensions/SyntaxNodeExtensions.cs | 2 +- 10 files changed, 89 insertions(+), 82 deletions(-) diff --git a/src/NetAnalyzers/Core/AnalyzerReleases.Unshipped.md b/src/NetAnalyzers/Core/AnalyzerReleases.Unshipped.md index 688b82e1a1..54bf342fd7 100644 --- a/src/NetAnalyzers/Core/AnalyzerReleases.Unshipped.md +++ b/src/NetAnalyzers/Core/AnalyzerReleases.Unshipped.md @@ -5,6 +5,6 @@ Rule ID | Category | Severity | Notes --------|----------|----------|------- CA1849 | Performance | Disabled | UseAsyncMethodInAsyncContext, [Documentation](https://docs.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1849) -CA3200 | Correctness | Warning | SuspiciousCastFromCharToIntAnalyzer, [Documentation](https://docs.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca3200) +CA2019 | Reliability | Warning | SuspiciousCastFromCharToIntAnalyzer, [Documentation](https://docs.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca2019) CA5404 | Security | Disabled | DoNotDisableTokenValidationChecks, [Documentation](https://docs.microsoft.com/visualstudio/code-quality/ca5404) CA5405 | Security | Disabled | DoNotAlwaysSkipTokenValidationInDelegates, [Documentation](https://docs.microsoft.com/visualstudio/code-quality/ca5405) diff --git a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Runtime/SuspiciousCastFromCharToInt.Fixer.cs b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Runtime/SuspiciousCastFromCharToInt.Fixer.cs index 40ac089875..d9a5b92d59 100644 --- a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Runtime/SuspiciousCastFromCharToInt.Fixer.cs +++ b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Runtime/SuspiciousCastFromCharToInt.Fixer.cs @@ -28,7 +28,16 @@ public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context) var root = await doc.GetSyntaxRootAsync(token).ConfigureAwait(false); var model = await doc.GetSemanticModelAsync(token).ConfigureAwait(false); - if (model.GetOperation(root.FindNode(context.Span), token) is not IArgumentOperation argumentOperation) + // If there were unnecessary parentheses on the argument expression, then we won't get the reported IArgumentOperation + // from the call to SemanticModel.GetOperation(). + // This is because the Syntax property of an IArgumentOperation does NOT include the unnecessary parentheses. + // In other words, if we have an IArgumentOperation 'argument', then the following two expressions are equivalent: + // argument.SemanticModel.GetOperation(argument.Syntax) + // is equivalent to + // argument.Value.WalkDownConversion() + var reportedOperation = model.GetOperation(root.FindNode(context.Span), token); + var argumentOperation = reportedOperation as IArgumentOperation ?? reportedOperation?.WalkUpConversion()?.Parent as IArgumentOperation; + if (argumentOperation is null) return; var targetMethod = argumentOperation.Parent switch @@ -49,7 +58,7 @@ public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context) var codeAction = CodeAction.Create( MicrosoftNetCoreAnalyzersResources.SuspiciousCastFromCharToIntCodeFixTitle, token => fixer.GetChangedDocumentAsync(new FixCharCastContext(context, argumentOperation.Parent, this, token)), - MicrosoftNetCoreAnalyzersResources.SuspiciousCastFromCharToIntCodeFixTitle); + nameof(MicrosoftNetCoreAnalyzersResources.SuspiciousCastFromCharToIntCodeFixTitle)); context.RegisterCodeFix(codeAction, context.Diagnostics); } @@ -222,9 +231,7 @@ public override async Task GetChangedDocumentAsync(FixCharCastContext private static ImmutableDictionary CreateFixerCache(Compilation compilation) { - if (!RequiredSymbols.TryGetSymbols(compilation, out var symbols)) - return ImmutableDictionary.Empty; - + var symbols = new RequiredSymbols(compilation); var builder = ImmutableDictionary.CreateBuilder(); AddIfNotNull(StringSplit_CharInt32StringSplitOptions.Create(compilation, symbols)); diff --git a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Runtime/SuspiciousCastFromCharToInt.cs b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Runtime/SuspiciousCastFromCharToInt.cs index 0977a343c8..cf0696ed9d 100644 --- a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Runtime/SuspiciousCastFromCharToInt.cs +++ b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Runtime/SuspiciousCastFromCharToInt.cs @@ -10,22 +10,21 @@ namespace Microsoft.NetCore.Analyzers.Runtime { - [DiagnosticAnalyzer(LanguageNames.CSharp, LanguageNames.VisualBasic)] + // Visual Basic does not allow implicit conversions from char to int +#pragma warning disable RS1004 // Recommend adding language support to diagnostic analyzer + [DiagnosticAnalyzer(LanguageNames.CSharp)] +#pragma warning restore RS1004 // Recommend adding language support to diagnostic analyzer public sealed class SuspiciousCastFromCharToIntAnalyzer : DiagnosticAnalyzer { - internal const string RuleId = "CA3200"; - - private static readonly LocalizableString s_localizableTitle = Resx.CreateLocalizableResourceString(nameof(Resx.SuspiciousCastFromCharToIntTitle)); - private static readonly LocalizableString s_localizableMessage = Resx.CreateLocalizableResourceString(nameof(Resx.SuspiciousCastFromCharToIntMessage)); - private static readonly LocalizableString s_localizableDescription = Resx.CreateLocalizableResourceString(nameof(Resx.SuspiciousCastFromCharToIntDescription)); + internal const string RuleId = "CA2019"; internal static readonly DiagnosticDescriptor Rule = DiagnosticDescriptorHelper.Create( RuleId, - s_localizableTitle, - s_localizableMessage, - DiagnosticCategory.Correctness, + Resx.CreateLocalizableResourceString(nameof(Resx.SuspiciousCastFromCharToIntCodeFixTitle)), + Resx.CreateLocalizableResourceString(nameof(Resx.SuspiciousCastFromCharToIntMessage)), + DiagnosticCategory.Reliability, RuleLevel.BuildWarning, - s_localizableDescription, + Resx.CreateLocalizableResourceString(nameof(Resx.SuspiciousCastFromCharToIntDescription)), isPortedFxCopRule: false, isDataflowRule: false); @@ -40,9 +39,7 @@ public override void Initialize(AnalysisContext context) private static void OnCompilationStart(CompilationStartAnalysisContext context) { - if (!RequiredSymbols.TryGetSymbols(context.Compilation, out var symbols)) - return; - + var symbols = new RequiredSymbols(context.Compilation); var stringSplitMethods = GetProblematicStringSplitMethods(context.Compilation, symbols); // Report implicit char-to-int conversions in calls to certain overloads of string.Split @@ -86,12 +83,12 @@ private static void OnCompilationStart(CompilationStartAnalysisContext context) // Local functions - bool IsImplicitCharToIntConversion(IArgumentOperation argument) + static bool IsImplicitCharToIntConversion(IArgumentOperation argument) { return argument.Value is IConversionOperation conversion && conversion.IsImplicit && - conversion.Operand.Type.Equals(symbols.CharType, SymbolEqualityComparer.Default) && - argument.Parameter.Type.Equals(symbols.Int32Type, SymbolEqualityComparer.Default); + conversion.Operand.Type.SpecialType is SpecialType.System_Char && + argument.Parameter.Type.SpecialType is SpecialType.System_Int32; } } @@ -100,27 +97,11 @@ bool IsImplicitCharToIntConversion(IArgumentOperation argument) internal readonly struct RequiredSymbols #pragma warning restore CA1815 // Override equals and operator equals on value types { - private RequiredSymbols(ITypeSymbol charType, ITypeSymbol int32Type, ITypeSymbol stringType) - { - CharType = charType; - Int32Type = int32Type; - StringType = stringType; - } - - public static bool TryGetSymbols(Compilation compilation, out RequiredSymbols result) + public RequiredSymbols(Compilation compilation) { - var charType = compilation.GetSpecialType(SpecialType.System_Char); - var int32Type = compilation.GetSpecialType(SpecialType.System_Int32); - var stringType = compilation.GetSpecialType(SpecialType.System_String); - - if (charType is not null && int32Type is not null && stringType is not null) - { - result = new RequiredSymbols(charType, int32Type, stringType); - return true; - } - - result = default; - return false; + CharType = compilation.GetSpecialType(SpecialType.System_Char); + Int32Type = compilation.GetSpecialType(SpecialType.System_Int32); + StringType = compilation.GetSpecialType(SpecialType.System_String); } public ITypeSymbol CharType { get; } diff --git a/src/NetAnalyzers/Microsoft.CodeAnalysis.NetAnalyzers.md b/src/NetAnalyzers/Microsoft.CodeAnalysis.NetAnalyzers.md index 3283a479cd..bdd54ae0d5 100644 --- a/src/NetAnalyzers/Microsoft.CodeAnalysis.NetAnalyzers.md +++ b/src/NetAnalyzers/Microsoft.CodeAnalysis.NetAnalyzers.md @@ -1620,6 +1620,18 @@ Number of parameters supplied in the logging message template do not match the n |CodeFix|False| --- +## [CA2019](https://docs.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca2019): Use correct overload + +Implicit cast from char to int in method call is suspicious. Consider using a different overload. + +|Item|Value| +|-|-| +|Category|Reliability| +|Enabled|True| +|Severity|Warning| +|CodeFix|True| +--- + ## [CA2100](https://docs.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca2100): Review SQL queries for security vulnerabilities SQL queries that directly use user input can be vulnerable to SQL injection attacks. Review this SQL query for potential vulnerabilities, and consider using a parameterized SQL query. @@ -2640,18 +2652,6 @@ Missing ValidateAntiForgeryTokenAttribute on controller action {0} |CodeFix|False| --- -## [CA3200](https://docs.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca3200): Suspicious cast from char to int - -Implicit cast from char to int in method call is suspicious. Consider using a different overload. - -|Item|Value| -|-|-| -|Category|Correctness| -|Enabled|True| -|Severity|Warning| -|CodeFix|True| ---- - ## [CA5350](https://docs.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca5350): Do Not Use Weak Cryptographic Algorithms Cryptographic algorithms degrade over time as attacks become for advances to attacker get access to more computation. Depending on the type and application of this cryptographic algorithm, further degradation of the cryptographic strength of it may allow attackers to read enciphered messages, tamper with enciphered  messages, forge digital signatures, tamper with hashed content, or otherwise compromise any cryptosystem based on this algorithm. Replace encryption uses with the AES algorithm (AES-256, AES-192 and AES-128 are acceptable) with a key length greater than or equal to 128 bits. Replace hashing uses with a hashing function in the SHA-2 family, such as SHA-2 512, SHA-2 384, or SHA-2 256. diff --git a/src/NetAnalyzers/Microsoft.CodeAnalysis.NetAnalyzers.sarif b/src/NetAnalyzers/Microsoft.CodeAnalysis.NetAnalyzers.sarif index 4cef1a3202..a108083a3b 100644 --- a/src/NetAnalyzers/Microsoft.CodeAnalysis.NetAnalyzers.sarif +++ b/src/NetAnalyzers/Microsoft.CodeAnalysis.NetAnalyzers.sarif @@ -2944,6 +2944,26 @@ ] } }, + "CA2019": { + "id": "CA2019", + "shortDescription": "Use correct overload", + "fullDescription": "Implicit cast from char to int in method call is suspicious. Consider using a different overload.", + "defaultLevel": "warning", + "helpUri": "https://docs.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca2019", + "properties": { + "category": "Reliability", + "isEnabledByDefault": true, + "typeName": "SuspiciousCastFromCharToIntAnalyzer", + "languages": [ + "C#", + "Visual Basic" + ], + "tags": [ + "Telemetry", + "EnabledRuleInAggressiveMode" + ] + } + }, "CA2100": { "id": "CA2100", "shortDescription": "Review SQL queries for security vulnerabilities", @@ -4500,26 +4520,6 @@ ] } }, - "CA3200": { - "id": "CA3200", - "shortDescription": "Suspicious cast from char to int", - "fullDescription": "Implicit cast from char to int in method call is suspicious. Consider using a different overload.", - "defaultLevel": "warning", - "helpUri": "https://docs.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca3200", - "properties": { - "category": "Correctness", - "isEnabledByDefault": true, - "typeName": "SuspiciousCastFromCharToIntAnalyzer", - "languages": [ - "C#", - "Visual Basic" - ], - "tags": [ - "Telemetry", - "EnabledRuleInAggressiveMode" - ] - } - }, "CA5350": { "id": "CA5350", "shortDescription": "Do Not Use Weak Cryptographic Algorithms", diff --git a/src/NetAnalyzers/RulesMissingDocumentation.md b/src/NetAnalyzers/RulesMissingDocumentation.md index 526f925192..158bf1213d 100644 --- a/src/NetAnalyzers/RulesMissingDocumentation.md +++ b/src/NetAnalyzers/RulesMissingDocumentation.md @@ -11,6 +11,7 @@ CA1843 | | Use the LoggerMessage delegates | CA1849 | | Call async methods when in an async method | CA2017 | | Parameter count mismatch | +CA2019 | | Use correct overload | CA2252 | | This API requires opting into preview features | CA2253 | | Named placeholders should not be numeric values | CA2254 | | Template should be a static expression | @@ -18,4 +19,3 @@ CA2255 | | All members declared in parent interfaces must have an implementation in a DynamicInterfaceCastableImplementation-attributed interface | CA2257 | | Members defined on an interface with the 'DynamicInterfaceCastableImplementationAttribute' should be 'static' | CA2258 | | Providing a 'DynamicInterfaceCastableImplementation' interface in Visual Basic is unsupported | -CA3200 | | Suspicious cast from char to int | diff --git a/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/Runtime/SuspiciousCastFromCharToIntTests.cs b/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/Runtime/SuspiciousCastFromCharToIntTests.cs index 7a4d2ca473..b99124ceee 100644 --- a/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/Runtime/SuspiciousCastFromCharToIntTests.cs +++ b/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/Runtime/SuspiciousCastFromCharToIntTests.cs @@ -25,6 +25,9 @@ public class SuspiciousCastFromCharToIntTests [InlineData( "options: System.StringSplitOptions.None, separator: 'c', {|#0:count: 'a'|}", "new char[] { 'c', 'a' }, options: System.StringSplitOptions.None")] + [InlineData( + "'c', ({|#0:'a'|}), System.StringSplitOptions.None", + "new char[] { 'c', 'a' }, System.StringSplitOptions.None")] public Task StringSplit_CharInt32StringSplitOptions_Fixed_CS(string testArgumentList, string fixedArgumentList) { string format = @" @@ -61,6 +64,7 @@ public void M() [InlineData("{|#0:count: c|}, separator: s", "new string[] { s, c.ToString() }, System.StringSplitOptions.None")] [InlineData("options: System.StringSplitOptions.None, {|#0:count: c|}, separator: s", "new string[] { s, c.ToString() }, options: System.StringSplitOptions.None")] [InlineData("s, {|#0:'a'|}", @"new string[] { s, ""a"" }, System.StringSplitOptions.None")] + [InlineData("s, ({|#0:c|}), System.StringSplitOptions.None", "new string[] { s, c.ToString() }, System.StringSplitOptions.None")] public Task StringSplit_StringInt32StringSplitOptions_Fixed_CS(string testArgumentList, string fixedArgumentList) { string format = @" @@ -105,7 +109,7 @@ public void M(string s, char c, string[] ss, char[] cc, StringSplitOptions o) }} }}"; - return VerifyCS.VerifyAnalyzerAsync(source); + return VerifyCS.VerifyCodeFixAsync(source, source); } [Theory] @@ -143,6 +147,7 @@ public void M(string s, char c, int n, StringSplitOptions o) [InlineData("{|#0:'a'|}", @"""a""")] [InlineData("{|#0:capacity: c|}", "c.ToString()")] [InlineData("{|#0:capacity: 'a'|}", @"""a""")] + [InlineData("({|#0:c|})", "c.ToString()")] public Task StringBuilderCtor_Int32_Fixed_CS(string testArgumentList, string fixedArgumentList) { string format = @" @@ -193,7 +198,23 @@ public void M(string s, char c, int n) }}"; var diagnostics = Enumerable.Range(0, diagnosticsCount).Select(x => VerifyCS.Diagnostic(Rule).WithLocation(x)).ToArray(); - return VerifyCS.VerifyAnalyzerAsync(source, diagnostics); + var test = new VerifyCS.Test + { + TestState = + { + Sources = { source }, + ReferenceAssemblies = ReferenceAssemblies.Net.Net50 + }, + FixedState = + { + Sources = { source }, + ReferenceAssemblies = ReferenceAssemblies.Net.Net50 + } + }; + test.TestState.ExpectedDiagnostics.AddRange(diagnostics); + test.FixedState.ExpectedDiagnostics.AddRange(diagnostics); + + return test.RunAsync(); } [Theory] @@ -217,7 +238,7 @@ public void M(string s, char c, int n) }} }}"; - return VerifyCS.VerifyAnalyzerAsync(source); + return VerifyCS.VerifyCodeFixAsync(source, source); } private static DiagnosticDescriptor Rule => SuspiciousCastFromCharToIntAnalyzer.Rule; diff --git a/src/Utilities/Compiler/DiagnosticCategory.cs b/src/Utilities/Compiler/DiagnosticCategory.cs index 41ffed2b05..cdc09df5e4 100644 --- a/src/Utilities/Compiler/DiagnosticCategory.cs +++ b/src/Utilities/Compiler/DiagnosticCategory.cs @@ -10,7 +10,6 @@ internal static class DiagnosticCategory public const string Mobility = nameof(Mobility); public const string Performance = nameof(Performance); public const string Reliability = nameof(Reliability); - public const string Correctness = nameof(Correctness); public const string Security = nameof(Security); public const string Usage = nameof(Usage); public const string Naming = nameof(Naming); diff --git a/src/Utilities/Compiler/DiagnosticCategoryAndIdRanges.txt b/src/Utilities/Compiler/DiagnosticCategoryAndIdRanges.txt index ffc077073a..fd8933f01f 100644 --- a/src/Utilities/Compiler/DiagnosticCategoryAndIdRanges.txt +++ b/src/Utilities/Compiler/DiagnosticCategoryAndIdRanges.txt @@ -18,8 +18,7 @@ Usage: CA1801, CA1806, CA1816, CA2200-CA2209, CA2211-CA2258 Naming: CA1700-CA1727 Interoperability: CA1400-CA1419 Maintainability: CA1500-CA1509 -Reliability: CA9998-CA9999, CA2000-CA2018 -Correctness: CA3200, RS1000-RS1999 +Reliability: CA9998-CA9999, CA2000-CA2019 Documentation: CA1200-CA1200 # Microsoft CodeAnalysis API rules diff --git a/src/Utilities/Compiler/Extensions/SyntaxNodeExtensions.cs b/src/Utilities/Compiler/Extensions/SyntaxNodeExtensions.cs index 8ea6ff1610..5999534288 100644 --- a/src/Utilities/Compiler/Extensions/SyntaxNodeExtensions.cs +++ b/src/Utilities/Compiler/Extensions/SyntaxNodeExtensions.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.Linq; using Microsoft.CodeAnalysis; From 5b5acd77efc863f8d753e1ef39f9ac7de48e6715 Mon Sep 17 00:00:00 2001 From: Newell Clark Date: Mon, 20 Sep 2021 09:50:23 -0400 Subject: [PATCH 8/9] Rerun msbuild --- src/NetAnalyzers/Microsoft.CodeAnalysis.NetAnalyzers.sarif | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/NetAnalyzers/Microsoft.CodeAnalysis.NetAnalyzers.sarif b/src/NetAnalyzers/Microsoft.CodeAnalysis.NetAnalyzers.sarif index a108083a3b..aa5d8b2e8f 100644 --- a/src/NetAnalyzers/Microsoft.CodeAnalysis.NetAnalyzers.sarif +++ b/src/NetAnalyzers/Microsoft.CodeAnalysis.NetAnalyzers.sarif @@ -2955,8 +2955,7 @@ "isEnabledByDefault": true, "typeName": "SuspiciousCastFromCharToIntAnalyzer", "languages": [ - "C#", - "Visual Basic" + "C#" ], "tags": [ "Telemetry", From 18388804b828651f718f68b356501c7b53b997b6 Mon Sep 17 00:00:00 2001 From: Newell Clark Date: Mon, 20 Sep 2021 12:39:32 -0400 Subject: [PATCH 9/9] Apply suggested changes --- .../CSharpSuspiciousCastFromCharToInt.Fixer.cs | 3 ++- .../Runtime/SuspiciousCastFromCharToInt.Fixer.cs | 4 ++-- .../Compiler/Extensions/IOperationExtensions.cs | 14 -------------- 3 files changed, 4 insertions(+), 17 deletions(-) diff --git a/src/NetAnalyzers/CSharp/Microsoft.NetCore.Analyzers/Runtime/CSharpSuspiciousCastFromCharToInt.Fixer.cs b/src/NetAnalyzers/CSharp/Microsoft.NetCore.Analyzers/Runtime/CSharpSuspiciousCastFromCharToInt.Fixer.cs index a2e3c5f9e7..9a4874ff1a 100644 --- a/src/NetAnalyzers/CSharp/Microsoft.NetCore.Analyzers/Runtime/CSharpSuspiciousCastFromCharToInt.Fixer.cs +++ b/src/NetAnalyzers/CSharp/Microsoft.NetCore.Analyzers/Runtime/CSharpSuspiciousCastFromCharToInt.Fixer.cs @@ -1,5 +1,6 @@ // Copyright (c) Microsoft. All Rights Reserved. Licensed under the MIT license. See License.txt in the project root for license information. +using System.Composition; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CodeFixes; using Microsoft.CodeAnalysis.CSharp.Syntax; @@ -7,7 +8,7 @@ namespace Microsoft.NetCore.CSharp.Analyzers.Runtime { - [ExportCodeFixProvider(LanguageNames.CSharp)] + [ExportCodeFixProvider(LanguageNames.CSharp), Shared] public sealed class CSharpSuspiciousCastFromCharToIntFixer : SuspiciousCastFromCharToIntFixer { internal override SyntaxNode GetMemberAccessExpressionSyntax(SyntaxNode invocationExpressionSyntax) diff --git a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Runtime/SuspiciousCastFromCharToInt.Fixer.cs b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Runtime/SuspiciousCastFromCharToInt.Fixer.cs index d9a5b92d59..d51ffbc0c2 100644 --- a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Runtime/SuspiciousCastFromCharToInt.Fixer.cs +++ b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Runtime/SuspiciousCastFromCharToInt.Fixer.cs @@ -123,7 +123,7 @@ public override async Task GetChangedDocumentAsync(FixCharCastContext var editor = await DocumentEditor.CreateAsync(doc, token).ConfigureAwait(false); var charTypeExpressionSyntax = editor.Generator.TypeExpression(invocation.SemanticModel.Compilation.GetSpecialType(SpecialType.System_Char)); - var arguments = invocation.GetArgumentsInParameterOrder(); + var arguments = invocation.Arguments.GetArgumentsInParameterOrder(); var elementNodes = new[] { arguments[0].Value.Syntax, @@ -165,7 +165,7 @@ public override async Task GetChangedDocumentAsync(FixCharCastContext var stringType = invocation.SemanticModel.Compilation.GetSpecialType(SpecialType.System_String); var stringTypeExpressionSyntax = editor.Generator.TypeExpression(stringType); - var arguments = invocation.GetArgumentsInParameterOrder(); + var arguments = invocation.Arguments.GetArgumentsInParameterOrder(); var elementNodes = new[] { arguments[0].Value.Syntax, diff --git a/src/Utilities/Compiler/Extensions/IOperationExtensions.cs b/src/Utilities/Compiler/Extensions/IOperationExtensions.cs index 59e9bf10e8..6e0011b9f6 100644 --- a/src/Utilities/Compiler/Extensions/IOperationExtensions.cs +++ b/src/Utilities/Compiler/Extensions/IOperationExtensions.cs @@ -57,20 +57,6 @@ internal static partial class IOperationExtensions return typeInfo.Type as INamedTypeSymbol; } - /// - /// Gets the arguments for the current ordered by their respective parameter's property. - /// - public static ImmutableArray GetArgumentsInParameterOrder(this IInvocationOperation invocation) - { - var result = invocation.Arguments.ToBuilder(); - - // IArgumentOperation.Parameter can be null if the argument is an __arglist argument. - // Sort __arglist arguments last, since __arglist parameters must always be declared last. - result.Sort((x, y) => (x.Parameter?.Ordinal ?? int.MaxValue).CompareTo(y.Parameter?.Ordinal ?? int.MaxValue)); - - return result.MoveToImmutable(); - } - public static bool HasNullConstantValue(this IOperation operation) { return operation.ConstantValue.HasValue && operation.ConstantValue.Value == null;