Skip to content

Commit

Permalink
Instantiate ArgumentExceptions Correctly analyzer updates and fixer i…
Browse files Browse the repository at this point in the history
…mplementation (#3500)

* ArgumentException analyzer updates and fixer implementation
  • Loading branch information
buyaa-n committed Apr 26, 2020
1 parent 10a2a14 commit d7d170a
Show file tree
Hide file tree
Showing 20 changed files with 752 additions and 136 deletions.

This file was deleted.

7 changes: 6 additions & 1 deletion src/NetAnalyzers/Core/AnalyzerReleases.Unshipped.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,9 @@ CA2012 | Reliability | Hidden | UseValueTasksCorrectlyAnalyzer, [Documentation](
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)
CA2247 | Usage | Warning | DoNotCreateTaskCompletionSourceWithWrongArguments, [Documentation](https://docs.microsoft.com/visualstudio/code-quality/ca2247)
CA2247 | Usage | Warning | DoNotCreateTaskCompletionSourceWithWrongArguments, [Documentation](https://docs.microsoft.com/visualstudio/code-quality/ca2247)

### Changed Rules
Rule ID | New Category | New Severity | Old Category | Old Severity | Notes
--------|--------------|--------------|--------------|--------------|-------
CA2208 | Usage | Info | Usage | Hidden | InstantiateArgumentExceptionsCorrectlyAnalyzer, [Documentation](https://docs.microsoft.com/visualstudio/code-quality/ca2208)
Original file line number Diff line number Diff line change
Expand Up @@ -1287,6 +1287,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>
</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,122 @@
// 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.Linq;
using System.Threading;
using System.Threading.Tasks;
using System.Diagnostics;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CodeActions;
using Microsoft.CodeAnalysis.CodeFixes;
using Microsoft.CodeAnalysis.Editing;
using Microsoft.CodeAnalysis.Operations;

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);
if (node != null)
{
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)
{
if (int.TryParse(paramPositionString, out int paramPosition))
{
CodeAction? codeAction = null;
if (creation.Arguments.Length == 1)
{
// Add null message
codeAction = CodeAction.Create(
title: MicrosoftNetCoreAnalyzersResources.InstantiateArgumentExceptionsCorrectlyChangeToTwoArgumentCodeFixTitle,
createChangedDocument: c => AddNullMessageToArgumentListAsync(context.Document, creation, c),
equivalenceKey: MicrosoftNetCoreAnalyzersResources.InstantiateArgumentExceptionsCorrectlyChangeToTwoArgumentCodeFixTitle);
}
else
{
// Swap message and paramete name
codeAction = CodeAction.Create(
title: MicrosoftNetCoreAnalyzersResources.InstantiateArgumentExceptionsCorrectlyFlipArgumentOrderCodeFixTitle,
createChangedDocument: c => SwapArgumentsOrderAsync(context.Document, creation, paramPosition, creation.Arguments.Length, c),
equivalenceKey: MicrosoftNetCoreAnalyzersResources.InstantiateArgumentExceptionsCorrectlyFlipArgumentOrderCodeFixTitle);
}
context.RegisterCodeFix(codeAction, diagnostic);
}
}
}

private static async Task<Document> SwapArgumentsOrderAsync(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
{
Debug.Assert(argumentCount == 3);
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> AddNullMessageToArgumentListAsync(Document document, IObjectCreationOperation creation, CancellationToken token)
{
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 @@ -84,21 +86,22 @@ private static void AnalyzeObjectCreation(
ITypeSymbol argumentExceptionType)
{
var creation = (IObjectCreationOperation)context.Operation;
if (!creation.Type.Inherits(argumentExceptionType))
if (!creation.Type.Inherits(argumentExceptionType) || !MatchesConfiguredVisibility(owningSymbol, context))
{
return;
}

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,39 +115,53 @@ private static void AnalyzeObjectCreation(
continue;
}

CheckArgument(owningSymbol, creation, argument.Parameter, value, context);
diagnostic = CheckArgument(owningSymbol, creation, argument.Parameter, value, context);

// RuleIncorrectMessage is the highest priority rule, no need to check other rules
if (diagnostic != null && diagnostic.Descriptor.Equals(RuleIncorrectMessage))
{
break;
}
}

if (diagnostic != null)
{
context.ReportDiagnostic(diagnostic);
}
}
}
private static void CheckArgument(

private static bool MatchesConfiguredVisibility(ISymbol owningSymbol, OperationAnalysisContext context) =>
owningSymbol.MatchesConfiguredVisibility(context.Options, RuleIncorrectParameterName, context.Compilation,
context.CancellationToken, defaultRequiredVisibility: SymbolVisibilityGroup.All);

private static bool HasParameters(ISymbol owningSymbol) => owningSymbol.GetParameters().Length > 0;

private static Diagnostic? CheckArgument(
ISymbol targetSymbol,
IObjectCreationOperation creation,
IParameterSymbol parameter,
string stringArgument,
OperationAnalysisContext context)
{
bool matchesParameter = MatchesParameter(targetSymbol, creation, stringArgument);
DiagnosticDescriptor? rule = null;

if (IsMessage(parameter) && matchesParameter)
{
rule = RuleIncorrectMessage;
var dictBuilder = ImmutableDictionary.CreateBuilder<string, string?>();
dictBuilder.Add(MessagePosition, parameter.Ordinal.ToString(CultureInfo.InvariantCulture));
return context.Operation.CreateDiagnostic(RuleIncorrectMessage, dictBuilder.ToImmutable(), targetSymbol.Name, stringArgument, parameter.Name, creation.Type.Name);
}
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;
return context.Operation.CreateDiagnostic(RuleIncorrectParameterName, targetSymbol.Name, stringArgument, parameter.Name, creation.Type.Name);
}

rule = RuleIncorrectParameterName;
}

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

private static bool IsMessage(IParameterSymbol parameter)
Expand Down Expand Up @@ -222,6 +239,20 @@ private static bool MatchesParameterCore(ISymbol? symbol, string stringArgumentV
}
}

if (symbol is IMethodSymbol method)
{
if (method.IsGenericMethod)
{
foreach (ITypeParameterSymbol parameter in method.TypeParameters)
{
if (parameter.Name == stringArgumentValue)
{
return true;
}
}
}

}
return false;
}

Expand Down
Loading

0 comments on commit d7d170a

Please sign in to comment.