Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Instantiate ArgumentExceptions Correctly analyzer updates and fixer implementation #3500

Merged
merged 10 commits into from
Apr 26, 2020
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view

This file was deleted.

3 changes: 2 additions & 1 deletion src/NetAnalyzers/Core/AnalyzerReleases.Unshipped.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,5 @@ CA2011 | Reliability | Info | AvoidInfiniteRecursion, [Documentation](https://do
CA2012 | Reliability | Hidden | UseValueTasksCorrectlyAnalyzer, [Documentation](https://docs.microsoft.com/visualstudio/code-quality/ca2012)
CA2013 | Reliability | Warning | DoNotUseReferenceEqualsWithValueTypesAnalyzer, [Documentation](https://docs.microsoft.com/visualstudio/code-quality/ca2013)
CA2014 | Reliability | Warning | DoNotUseStackallocInLoopsAnalyzer, [Documentation](https://docs.microsoft.com/visualstudio/code-quality/ca2014)
CA2015 | Reliability | Warning | DoNotDefineFinalizersForTypesDerivedFromMemoryManager, [Documentation](https://docs.microsoft.com/visualstudio/code-quality/ca2015)
CA2208 | Usage | Info | InstantiateArgumentExceptionsCorrectlyAnalyzer, [Documentation](https://docs.microsoft.com/visualstudio/code-quality/ca2208)
buyaa-n marked this conversation as resolved.
Show resolved Hide resolved
CA2015 | Reliability | Warning | DoNotDefineFinalizersForTypesDerivedFromMemoryManager, [Documentation](https://docs.microsoft.com/visualstudio/code-quality/ca2015)
Original file line number Diff line number Diff line change
Expand Up @@ -1275,6 +1275,12 @@
<data name="DoNotUseStackallocInLoopsMessage" xml:space="preserve">
<value>Potential stack overflow. Move the stackalloc out of the loop.</value>
</data>
<data name="InstantiateArgumentExceptionsCorrectlyChangeToTwoArgumentCodeFixTitle" xml:space="preserve">
<value>Change to call the two argument constructor, pass null for the message.</value>
</data>
<data name="InstantiateArgumentExceptionsCorrectlyFlipArgumentOrderCodeFixTitle" xml:space="preserve">
<value>Swap the arguments order</value>
buyaa-n marked this conversation as resolved.
Show resolved Hide resolved
</data>
<data name="PreferTypedStringBuilderAppendOverloadsTitle" xml:space="preserve">
<value>Prefer strongly-typed Append and Insert method overloads on StringBuilder.</value>
</data>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,29 +1,116 @@
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System.Collections.Immutable;
using System.Composition;
using System.Globalization;
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;

buyaa-n marked this conversation as resolved.
Show resolved Hide resolved
namespace Microsoft.NetCore.Analyzers.Runtime
{
/// <summary>
/// CA2208: Instantiate argument exceptions correctly
/// </summary>
public abstract class InstantiateArgumentExceptionsCorrectlyFixer : CodeFixProvider
[ExportCodeFixProvider(LanguageNames.CSharp, LanguageNames.VisualBasic), Shared]
public sealed class InstantiateArgumentExceptionsCorrectlyFixer : CodeFixProvider
{
public sealed override ImmutableArray<string> FixableDiagnosticIds => ImmutableArray<string>.Empty;
public sealed override ImmutableArray<string> FixableDiagnosticIds => ImmutableArray.Create(InstantiateArgumentExceptionsCorrectlyAnalyzer.RuleId);

public sealed override FixAllProvider GetFixAllProvider()
public sealed override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer;

public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context)
{
// See https://github.com/dotnet/roslyn/blob/master/docs/analyzers/FixAllProvider.md for more information on Fix All Providers
return WellKnownFixAllProviders.BatchFixer;
var diagnostic = context.Diagnostics.First();
string paramPositionString = diagnostic.Properties.GetValueOrDefault(InstantiateArgumentExceptionsCorrectlyAnalyzer.MessagePosition);
if (paramPositionString != null)
{
SyntaxNode root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false);
SyntaxNode node = root.FindNode(context.Span, getInnermostNodeForTie: true);
buyaa-n marked this conversation as resolved.
Show resolved Hide resolved
await PopulateCodeFixAsync(context, diagnostic, paramPositionString, node).ConfigureAwait(false);
}
}

public sealed override Task RegisterCodeFixesAsync(CodeFixContext context)
private static async Task PopulateCodeFixAsync(CodeFixContext context, Diagnostic diagnostic, string paramPositionString, SyntaxNode node)
{
// Fixer not yet implemented.
return Task.CompletedTask;
SemanticModel model = await context.Document.GetSemanticModelAsync(context.CancellationToken).ConfigureAwait(false);
var operation = model.GetOperation(node, context.CancellationToken);
if (operation is IObjectCreationOperation creation)
{
int paramPosition = int.Parse(paramPositionString, CultureInfo.InvariantCulture);
buyaa-n marked this conversation as resolved.
Show resolved Hide resolved
CodeAction? codeAction = null;
if (creation.Arguments.Length == 1)
{
// Add null message
codeAction = CodeAction.Create(
title: MicrosoftNetCoreAnalyzersResources.InstantiateArgumentExceptionsCorrectlyChangeToTwoArgumentCodeFixTitle,
createChangedDocument: c => AddNullMessageToArgumentList(context.Document, creation, c),
equivalenceKey: MicrosoftNetCoreAnalyzersResources.InstantiateArgumentExceptionsCorrectlyChangeToTwoArgumentCodeFixTitle);
}
else
{
// Swap message and paramete name
codeAction = CodeAction.Create(
title: MicrosoftNetCoreAnalyzersResources.InstantiateArgumentExceptionsCorrectlyFlipArgumentOrderCodeFixTitle,
createChangedDocument: c => SwapArgumentsOrder(context.Document, creation, paramPosition, creation.Arguments.Length, c),
equivalenceKey: MicrosoftNetCoreAnalyzersResources.InstantiateArgumentExceptionsCorrectlyFlipArgumentOrderCodeFixTitle);
}
context.RegisterCodeFix(codeAction, diagnostic);
}
}

private static async Task<Document> SwapArgumentsOrder(Document document, IObjectCreationOperation creation, int paramPosition, int argumentCount, CancellationToken token)
{
DocumentEditor editor = await DocumentEditor.CreateAsync(document, token).ConfigureAwait(false);
SyntaxNode parameter = AddNameOfIfLiteral(creation.Arguments[paramPosition].Value, editor.Generator);
SyntaxNode newCreation;
if (argumentCount == 2)
{
if (paramPosition == 0)
{
newCreation = editor.Generator.ObjectCreationExpression(creation.Type, creation.Arguments[1].Syntax, parameter);
}
else
{
newCreation = editor.Generator.ObjectCreationExpression(creation.Type, parameter, creation.Arguments[0].Syntax);
}
}
else // 3 arguments
buyaa-n marked this conversation as resolved.
Show resolved Hide resolved
{
if (paramPosition == 0)
{
newCreation = editor.Generator.ObjectCreationExpression(creation.Type, creation.Arguments[1].Syntax, parameter, creation.Arguments[2].Syntax);
}
else
{
newCreation = editor.Generator.ObjectCreationExpression(creation.Type, parameter, creation.Arguments[1].Syntax, creation.Arguments[0].Syntax);
}
}
editor.ReplaceNode(creation.Syntax, newCreation);
return editor.GetChangedDocument();
}

private static async Task<Document> AddNullMessageToArgumentList(Document document, IObjectCreationOperation creation, CancellationToken token)
buyaa-n marked this conversation as resolved.
Show resolved Hide resolved
{
DocumentEditor editor = await DocumentEditor.CreateAsync(document, token).ConfigureAwait(false);
SyntaxNode argument = AddNameOfIfLiteral(creation.Arguments[0].Value, editor.Generator);
SyntaxNode newCreation = editor.Generator.ObjectCreationExpression(creation.Type, editor.Generator.Argument(editor.Generator.NullLiteralExpression()), argument);
editor.ReplaceNode(creation.Syntax, newCreation);
return editor.GetChangedDocument();
}

private static SyntaxNode AddNameOfIfLiteral(IOperation expression, SyntaxGenerator generator)
{
if (expression is ILiteralOperation literal)
{
return generator.NameOfExpression(generator.IdentifierName(literal.ConstantValue.Value.ToString()));
}
return expression.Syntax;
}
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System.Collections.Immutable;
using System.Globalization;
using Analyzer.Utilities;
using Analyzer.Utilities.Extensions;
using Microsoft.CodeAnalysis;
Expand All @@ -16,6 +17,7 @@ namespace Microsoft.NetCore.Analyzers.Runtime
public sealed class InstantiateArgumentExceptionsCorrectlyAnalyzer : DiagnosticAnalyzer
{
internal const string RuleId = "CA2208";
internal const string MessagePosition = nameof(MessagePosition);

private static readonly LocalizableString s_localizableTitle = new LocalizableResourceString(nameof(MicrosoftNetCoreAnalyzersResources.InstantiateArgumentExceptionsCorrectlyTitle), MicrosoftNetCoreAnalyzersResources.ResourceManager, typeof(MicrosoftNetCoreAnalyzersResources));

Expand All @@ -28,7 +30,7 @@ public sealed class InstantiateArgumentExceptionsCorrectlyAnalyzer : DiagnosticA
s_localizableTitle,
s_localizableMessageNoArguments,
DiagnosticCategory.Usage,
RuleLevel.IdeHidden_BulkConfigurable,
RuleLevel.IdeSuggestion,
description: s_localizableDescription,
isPortedFxCopRule: true,
isDataflowRule: false);
Expand All @@ -37,7 +39,7 @@ public sealed class InstantiateArgumentExceptionsCorrectlyAnalyzer : DiagnosticA
s_localizableTitle,
s_localizableMessageIncorrectMessage,
DiagnosticCategory.Usage,
RuleLevel.IdeHidden_BulkConfigurable,
RuleLevel.IdeSuggestion,
description: s_localizableDescription,
isPortedFxCopRule: true,
isDataflowRule: false);
Expand All @@ -46,7 +48,7 @@ public sealed class InstantiateArgumentExceptionsCorrectlyAnalyzer : DiagnosticA
s_localizableTitle,
s_localizableMessageIncorrectParameterName,
DiagnosticCategory.Usage,
RuleLevel.IdeHidden_BulkConfigurable,
RuleLevel.IdeSuggestion,
description: s_localizableDescription,
isPortedFxCopRule: true,
isDataflowRule: false);
Expand Down Expand Up @@ -91,14 +93,15 @@ private static void AnalyzeObjectCreation(

if (creation.Arguments.Length == 0)
{
if (HasMessageOrParameterNameConstructor(creation.Type))
if (HasParameters(owningSymbol) && HasMessageOrParameterNameConstructor(creation.Type))
{
// Call the {0} constructor that contains a message and/ or paramName parameter
context.ReportDiagnostic(context.Operation.Syntax.CreateDiagnostic(RuleNoArguments, creation.Type.Name));
}
}
else
{
Diagnostic? diagnostic = null;
foreach (IArgumentOperation argument in creation.Arguments)
{
if (argument.Parameter.Type.SpecialType != SpecialType.System_String)
Expand All @@ -112,11 +115,25 @@ private static void AnalyzeObjectCreation(
continue;
}

CheckArgument(owningSymbol, creation, argument.Parameter, value, context);
diagnostic = CheckArgument(owningSymbol, creation, argument.Parameter, value, context);
buyaa-n marked this conversation as resolved.
Show resolved Hide resolved
if (diagnostic != null && !diagnostic.Properties.IsEmpty)
buyaa-n marked this conversation as resolved.
Show resolved Hide resolved
{
break;
}
}
if (diagnostic != null)
{
context.ReportDiagnostic(diagnostic);
}
}
}
private static void CheckArgument(

private static bool HasParameters(ISymbol owningSymbol)
{
return owningSymbol.GetParameters().Length > 0;
}

private static Diagnostic? CheckArgument(
ISymbol targetSymbol,
IObjectCreationOperation creation,
IParameterSymbol parameter,
Expand All @@ -125,26 +142,28 @@ private static void CheckArgument(
{
bool matchesParameter = MatchesParameter(targetSymbol, creation, stringArgument);
DiagnosticDescriptor? rule = null;
var dictBuilder = ImmutableDictionary.CreateBuilder<string, string?>();

if (IsMessage(parameter) && matchesParameter)
{
rule = RuleIncorrectMessage;
dictBuilder.Add(MessagePosition, parameter.Ordinal.ToString(CultureInfo.InvariantCulture));
}
else if (IsParameterName(parameter) && !matchesParameter)
else if (HasParameters(targetSymbol) && IsParameterName(parameter) && !matchesParameter)
{
// Allow argument exceptions in accessors to use the associated property symbol name.
if (MatchesAssociatedSymbol(targetSymbol, stringArgument))
if (!MatchesAssociatedSymbol(targetSymbol, stringArgument))
{
return;
rule = RuleIncorrectParameterName;
}

rule = RuleIncorrectParameterName;
}

if (rule != null)
{
context.ReportDiagnostic(context.Operation.Syntax.CreateDiagnostic(rule, targetSymbol.Name, stringArgument, parameter.Name, creation.Type.Name));
return context.Operation.CreateDiagnostic(rule, dictBuilder.ToImmutable(), targetSymbol.Name, stringArgument, parameter.Name, creation.Type.Name);
}

return null;
}

private static bool IsMessage(IParameterSymbol parameter)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1027,11 +1027,21 @@
<target state="translated">Inicializujte statická pole typu hodnot jako vložená</target>
<note />
</trans-unit>
<trans-unit id="InstantiateArgumentExceptionsCorrectlyChangeToTwoArgumentCodeFixTitle">
<source>Change to call the two argument constructor, pass null for the message.</source>
<target state="new">Change to call the two argument constructor, pass null for the message.</target>
<note />
</trans-unit>
<trans-unit id="InstantiateArgumentExceptionsCorrectlyDescription">
<source>A call is made to the default (parameterless) constructor of an exception type that is or derives from ArgumentException, or an incorrect string argument is passed to a parameterized constructor of an exception type that is or derives from ArgumentException.</source>
<target state="translated">Zavolal se výchozí konstruktor (bez parametrů) typu výjimky, který je třídou ArgumentException nebo je z ní odvozený, nebo se do jeho konstruktoru s parametry předal nesprávný argument řetězce.</target>
<note />
</trans-unit>
<trans-unit id="InstantiateArgumentExceptionsCorrectlyFlipArgumentOrderCodeFixTitle">
<source>Swap the arguments order</source>
<target state="new">Swap the arguments order</target>
<note />
</trans-unit>
<trans-unit id="InstantiateArgumentExceptionsCorrectlyMessageIncorrectMessage">
<source>Method {0} passes parameter name '{1}' as the {2} argument to a {3} constructor. Replace this argument with a descriptive message and pass the parameter name in the correct position.</source>
<target state="translated">Metoda {0} předává název parametru {1} jako argument {2} konstruktoru {3}. Nahraďte tento argument popisnou zprávou a předejte název parametru na správné pozici.</target>
Expand All @@ -1044,7 +1054,7 @@
</trans-unit>
<trans-unit id="InstantiateArgumentExceptionsCorrectlyMessageNoArguments">
<source>Call the {0} constructor that contains a message and/or paramName parameter.</source>
<target state="translated">Zavolejte konstruktor {0}, který obsahuje zprávu a/nebo parametr paramName.</target>
<target state="needs-review-translation">Zavolejte konstruktor {0}, který obsahuje zprávu a/nebo parametr paramName.</target>
<note />
</trans-unit>
<trans-unit id="InstantiateArgumentExceptionsCorrectlyTitle">
Expand Down
Loading