Instantiate ArgumentExceptions Correctly analyzer updates and fixer implementation

src/NetAnalyzers/Core/AnalyzerReleases.Unshipped.md

@@ -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)
+CA2015 | Reliability | Warning | DoNotDefineFinalizersForTypesDerivedFromMemoryManager, [Documentation](https://docs.microsoft.com/visualstudio/code-quality/ca2015)

src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/MicrosoftNetCoreAnalyzersResources.resx

@@ -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>
+  </data>
   <data name="PreferTypedStringBuilderAppendOverloadsTitle" xml:space="preserve">
     <value>Prefer strongly-typed Append and Insert method overloads on StringBuilder.</value>
   </data>

src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Runtime/InstantiateArgumentExceptionsCorrectly.Fixer.cs

@@ -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;
 
 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);
+                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);
+                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
+            {
+                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)
+        {
+            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;
         }
     }
 }

src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Runtime/InstantiateArgumentExceptionsCorrectly.cs

@@ -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;
@@ -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));
 
@@ -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);
@@ -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);
@@ -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);
@@ -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)
@@ -112,11 +115,31 @@ private static void AnalyzeObjectCreation(
                         continue;
                     }
 
-                    CheckArgument(owningSymbol, creation, argument.Parameter, value, context);
+                    diagnostic = CheckArgument(owningSymbol, creation, argument.Parameter, value, context);
+                    if (diagnostic != null && !diagnostic.Properties.IsEmpty)
+                    {
+                        break;
+                    }
+                }
+
+                if (diagnostic != null)
+                {
+                    context.ReportDiagnostic(diagnostic);
                 }
             }
         }
-        private static void CheckArgument(
+
+        private static bool MatchesConfiguredVisibility(ISymbol owningSymbol, OperationAnalysisContext context)
+        { // Should add for all rules?
+            return owningSymbol.MatchesConfiguredVisibility(context.Options, RuleIncorrectParameterName, context.Compilation, context.CancellationToken, defaultRequiredVisibility: SymbolVisibilityGroup.All);
+        }
+
+        private static bool HasParameters(ISymbol owningSymbol)
+        {
+            return owningSymbol.GetParameters().Length > 0;
+        }
+
+        private static Diagnostic? CheckArgument(
             ISymbol targetSymbol,
             IObjectCreationOperation creation,
             IParameterSymbol parameter,
@@ -125,26 +148,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)
@@ -222,6 +247,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;
         }