Fix #17: Warn when provided argument name in ArgumentException is incorrect

src/ErrorProne.NET/ErrorProne.NET.Test/ErrorProne.NET.Test.csproj

@@ -120,6 +120,7 @@
     <Reference Include="System.Xml" />
   </ItemGroup>
   <ItemGroup>
+    <Compile Include="ExceptionHandlingRules\ArgumentExceptionUsesStringAnalyzerTests.cs" />
     <Compile Include="DebuggerDisplayAnalysis\DebuggerDisplayAttributeAnalyzerTests.cs" />
     <Compile Include="ExceptionHandlingRules\ExtractPreconditionInIteratorBlockTests.cs" />
     <Compile Include="ExceptionHandlingRules\ExtractPreconditionInAsyncMethodTests.cs" />

src/ErrorProne.NET/ErrorProne.NET.Test/ExceptionHandlingRules/ArgumentExceptionUsesStringAnalyzerTests.cs

@@ -0,0 +1,154 @@
+using ErrorProne.NET.Common;
+using ErrorProne.NET.Rules.ExceptionHandling;
+using NUnit.Framework;
+using RoslynNunitTestRunner;
+
+namespace ErrorProne.NET.Test.ExceptionHandling
+{
+    [TestFixture]
+    public class ArgumentExceptionUsesStringAnalyzerTests : CSharpAnalyzerTestFixture<ArgumentExceptionUsesStringAnalyzer> {
+        [Test]
+        public void ShouldWarnOnArgumentExceptionHasNoParametersInCtor() {
+            var code = @"
+using System;
+class Test
+{
+    public Test(string arg) 
+    {
+        throw [|new ArgumentException()|];
+    }
+}";
+            HasDiagnostic(code, RuleIds.ArgumentExceptionParamNameRequired);
+        }
+
+        [Test]
+        public void ShouldWarnOnArgumentExceptionHasNoParametersInMethod() {
+            var code = @"
+using System;
+class Test
+{
+    public Foo(string arg) 
+    {
+        throw [|new ArgumentException()|];
+    }
+}";
+            HasDiagnostic(code, RuleIds.ArgumentExceptionParamNameRequired);
+        }
+
+        [Test]
+        public void ShouldWarnOnArgumentExceptionHasOnlyMessageParameterInCtor() {
+            var code = @"
+using System;
+class Test
+{
+    public Test(string arg) 
+    {
+        throw [|new ArgumentException(""It's an argument exception"")|];
+    }
+}";
+            HasDiagnostic(code, RuleIds.ArgumentExceptionParamNameRequired);
+        }
+
+        [Test]
+        public void ShouldWarnOnArgumentExceptionHasOnlyMessageParameterInMethod() {
+            var code = @"
+using System;
+class Test
+{
+    public Foo(string arg) 
+    {
+        throw [|new ArgumentException(""It's an argument exception"")|];
+    }
+}";
+            HasDiagnostic(code, RuleIds.ArgumentExceptionParamNameRequired);
+        }
+
+        [Test]
+        public void ShouldWarnOnArgumentExceptionHasWrongParameterNameInCtor() {
+            var code = @"
+using System;
+class Test
+{
+    public Test(string arg) 
+    {
+        throw new ArgumentException(""It's an argument exception"", [|""arg1""|]);
+    }
+}";
+            HasDiagnostic(code, RuleIds.ArgumentExceptionMethodHasNoSuchParamName);
+        }
+
+        [Test]
+        public void ShouldWarnOnArgumentExceptionHasWrongParameterNameInMethod() {
+            var code = @"
+using System;
+class Test
+{
+    public Foo(string arg) 
+    {
+        throw new ArgumentException(""It's an argument exception"", [|""arg1""|]);
+    }
+}";
+            HasDiagnostic(code, RuleIds.ArgumentExceptionMethodHasNoSuchParamName);
+        }
+
+        [Test]
+        public void ShouldWarnOnArgumentExceptionHasStringParameterNameInCtor() {
+            var code = @"
+using System;
+class Test
+{
+    public Test(string arg) 
+    {
+        throw new ArgumentException(""It's an argument exception"", [|""arg""|]);
+    }
+}";
+            HasDiagnostic(code, RuleIds.ArgumentExceptionParamNameShouldNotBeString);
+        }
+
+        [Test]
+        public void ShouldWarnOnArgumentExceptionHasStringParameterNameInMethod() {
+            var code = @"
+using System;
+class Test
+{
+    public Foo(string arg) 
+    {
+        throw new ArgumentException(""It's an argument exception"", [|""arg""|]);
+    }
+}";
+            HasDiagnostic(code, RuleIds.ArgumentExceptionParamNameShouldNotBeString);
+        }
+
+        [Test]
+        public void ShouldNotWarnOnArgumentExceptionHasNameofInCtor() {
+            var code = @"
+using System;
+class Test
+{
+    public Test(string arg) 
+    {
+        throw new ArgumentException(""It's an argument exception"", nameof(arg));
+    }
+}";
+            NoDiagnostic(code, RuleIds.ArgumentExceptionParamNameRequired);
+            NoDiagnostic(code, RuleIds.ArgumentExceptionMethodHasNoSuchParamName);
+            NoDiagnostic(code, RuleIds.ArgumentExceptionParamNameShouldNotBeString);
+        }
+
+        [Test]
+        public void ShouldNotWarnOnArgumentExceptionHasNameofInMethod() {
+            var code = @"
+using System;
+class Test
+{
+    public Foo(string arg) 
+    {
+        throw new ArgumentException(""It's an argument exception"", nameof(arg));
+    }
+}";
+            NoDiagnostic(code, RuleIds.ArgumentExceptionParamNameRequired);
+            NoDiagnostic(code, RuleIds.ArgumentExceptionMethodHasNoSuchParamName);
+            NoDiagnostic(code, RuleIds.ArgumentExceptionParamNameShouldNotBeString);
+        }
+    }
+}

src/ErrorProne.NET/ErrorProne.NET/Common/RuleIds.cs

@@ -5,7 +5,6 @@ namespace ErrorProne.NET.Common
 {
     public static class RuleIds
     {
-        // 
         public const string UnobservedPureMethodInvocationId = "ERP001";
         public const string SideEffectFreeExceptionContructionId = "ERP002";
         public const string AssignmentFreeImmutableObjectContructionId = "ERP003";
@@ -41,5 +40,10 @@ public static class RuleIds
 
         // Errors in DebuggerDisplayAttribute
         public const string DebuggerDisplayAttributeInvalidFormat = "ERP103";
+
+        // Argument exception
+        public const string ArgumentExceptionParamNameRequired = "ERP061";
+        public const string ArgumentExceptionMethodHasNoSuchParamName = "ERP062";
+        public const string ArgumentExceptionParamNameShouldNotBeString = "ERP063";
     }
 }

src/ErrorProne.NET/ErrorProne.NET/ErrorProne.NET.csproj

@@ -79,6 +79,7 @@
     <CodeAnalysisRuleSet>ErrorProne.NET.ruleset</CodeAnalysisRuleSet>
   </PropertyGroup>
   <ItemGroup>
+    <Compile Include="Rules\ExceptionHandling\ArgumentExceptionUsesStringAnalyzer.cs" />
     <Compile Include="Annotations\ImmutableAttribute.cs" />
     <Compile Include="Annotations\ReadOnlyAttribute.cs" />
     <Compile Include="Annotations\StringFormatMethodAttribute.cs" />
@@ -110,6 +111,7 @@
     <Compile Include="Extensions\PropertyDeclarationExtensions.cs" />
     <Compile Include="Extensions\SymbolExtensions.cs" />
     <Compile Include="Extensions\SyntaxNodeExtensions.cs" />
+    <Compile Include="Extensions\SyntaxNodeAnalysisContextExtensions.cs" />
     <Compile Include="Extensions\TypeSymbolExtensions.cs" />
     <Compile Include="Rules\ExceptionHandling\AsyncMethodPreconditionCodeFixProvider.cs" />
     <Compile Include="Rules\Formatting\RegexPatternAnalyzer.cs" />

src/ErrorProne.NET/ErrorProne.NET/Extensions/SyntaxNodeAnalysisContextExtensions.cs

@@ -0,0 +1,22 @@
+using System.Diagnostics.Contracts;
+using System.Text.RegularExpressions;
+using ErrorProne.NET.Common;
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.CSharp.Syntax;
+using Microsoft.CodeAnalysis.Diagnostics;
+
+namespace ErrorProne.NET.Extensions {
+    public static class SyntaxNodeAnalysisContextExtensions {
+        public static INamedTypeSymbol GetClrType<T>(this SyntaxNodeAnalysisContext context) => context.SemanticModel.GetClrType(typeof(T));
+
+        public static ITypeSymbol GetTypeSymbol(this SyntaxNodeAnalysisContext context, TypeSyntax typeSyntax) {
+            Contract.Requires(typeSyntax != null);
+            return context.SemanticModel.GetSymbolInfo(typeSyntax).Symbol as ITypeSymbol;
+        }
+
+        public static IMethodSymbol GetCtorSymbol(this SyntaxNodeAnalysisContext context, ObjectCreationExpressionSyntax objectCreationExpressionSyntax) {
+            Contract.Requires(objectCreationExpressionSyntax != null);
+            return context.SemanticModel.GetSymbolInfo(objectCreationExpressionSyntax).Symbol as IMethodSymbol;
+        }
+    }
+}

src/ErrorProne.NET/ErrorProne.NET/Rules/ExceptionHandling/ArgumentExceptionUsesStringAnalyzer.cs

@@ -0,0 +1,88 @@
+using System;
+using System.Collections.Immutable;
+using System.Linq;
+using ErrorProne.NET.Common;
+using ErrorProne.NET.Extensions;
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.CSharp;
+using Microsoft.CodeAnalysis.CSharp.Syntax;
+using Microsoft.CodeAnalysis.Diagnostics;
+
+namespace ErrorProne.NET.Rules.ExceptionHandling {
+    /// <summary>
+    /// Detects `ArgumentException` ctor's that use strings instead of nameof operator for argument names
+    /// </summary>
+    [DiagnosticAnalyzer(LanguageNames.CSharp)]
+    public sealed class ArgumentExceptionUsesStringAnalyzer : DiagnosticAnalyzer {
+        private const string ParamNameRequiredTitle = "ArgumentException should provide argument name.";
+        private const string ParamNameRequiredMessage = "ArgumentException instance misses invalid argument name.";
+        private const string ParamNameRequiredDescription = "ArgumentException indicates that a method was called with an invalid argument, so it is better to provide argument name for further investigation of exception cause.";
+
+        private const string MethodHasNoSuchParamNameRuleTitle = "Declaring method has no argument with specified name.";
+        private const string MethodHasNoSuchParamNameRuleMessageFormat = "{0} '{1}{2}' has no argument with name '{3}'.";
+        private const string MethodHasNoSuchParamNameRuleDescription = "ArgumentException indicates that a method on the top of the call stack was called with an invalid argument, so it is confusing if ArgumentException.ParamName provides missing paramater name. Consider using nameof operator to avoid typos and post-refactoring errors.";
+
+        private const string ParamNameShouldNotBeStringRuleTitle = "Use nameof instead of string.";
+        private const string ParamNameShouldNotBeStringRuleMessageFormat = "ArgumentException constructor obtains parameter name from string \"{0}\" instead of nameof({0}).";
+        private const string ParamNameShouldNotBeStringRuleDescription = "It is better to obtain string name of the paramater by using nameof operator to avoid typos and post-refactoring errors.";
+
+        private const string Category = "CodeSmell";
+
+        private static readonly DiagnosticDescriptor ParamNameRequiredRule =
+            new DiagnosticDescriptor(RuleIds.ArgumentExceptionParamNameRequired, ParamNameRequiredTitle, ParamNameRequiredMessage, Category, DiagnosticSeverity.Warning, description: ParamNameRequiredDescription, isEnabledByDefault: true);
+        private static readonly DiagnosticDescriptor MethodHasNoSuchParamNameRule =
+            new DiagnosticDescriptor(RuleIds.ArgumentExceptionMethodHasNoSuchParamName, MethodHasNoSuchParamNameRuleTitle, MethodHasNoSuchParamNameRuleMessageFormat, Category, DiagnosticSeverity.Warning, description: MethodHasNoSuchParamNameRuleDescription, isEnabledByDefault: true);
+        private static readonly DiagnosticDescriptor ParamNameShouldNotBeStringRule =
+            new DiagnosticDescriptor(RuleIds.ArgumentExceptionParamNameShouldNotBeString, ParamNameShouldNotBeStringRuleTitle, ParamNameShouldNotBeStringRuleMessageFormat, Category, DiagnosticSeverity.Warning, description: ParamNameShouldNotBeStringRuleDescription, isEnabledByDefault: true);
+
+        public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } = ImmutableArray.Create(ParamNameRequiredRule, MethodHasNoSuchParamNameRule, ParamNameShouldNotBeStringRule);
+
+        public override void Initialize(AnalysisContext context) {
+            context.RegisterSyntaxNodeAction(AnalyzeObjectCreation, SyntaxKind.ObjectCreationExpression);
+        }
+
+        private void AnalyzeObjectCreation(SyntaxNodeAnalysisContext context) {
+            var objectCreation = (ObjectCreationExpressionSyntax)context.Node;
+            var method = context.GetCtorSymbol(objectCreation);
+            if (!Equals(method.ContainingType, context.GetClrType<ArgumentException>())) {
+                return;
+            }
+
+            var stringType = context.GetClrType<string>();
+            var paramNameArgument = method.Parameters.FirstOrDefault(p => p.Name == "paramName");
+            if (paramNameArgument == null || !Equals(paramNameArgument.Type, stringType)) {
+                context.ReportDiagnostic(Diagnostic.Create(ParamNameRequiredRule, context.Node.GetLocation()));
+                return;
+            }
+
+            var argumentIndex = method.Parameters.IndexOf(paramNameArgument);
+            var argument = objectCreation.ArgumentList.Arguments[argumentIndex];
+            if (argument.Expression is LiteralExpressionSyntax) {
+                var literal = argument.Expression.ToString();
+                literal = literal.Substring(1, literal.Length - 2);
+
+                var methodBaseDeclaration = context.Node.EnumerateParents()
+                    .OfType<BaseMethodDeclarationSyntax>()
+                    .FirstOrDefault(d => d is MethodDeclarationSyntax || d is ConstructorDeclarationSyntax);
+
+                if (methodBaseDeclaration == null) {
+                    return;
+                }
+
+                var ctorDeclaration = methodBaseDeclaration as ConstructorDeclarationSyntax;
+
+                var parameters = methodBaseDeclaration.ParameterList.Parameters;
+                if (parameters.Any(p => p.Identifier.ValueText.Equals(literal))) {
+                    context.ReportDiagnostic(Diagnostic.Create(ParamNameShouldNotBeStringRule, argument.GetLocation(), literal));
+                } else if (ctorDeclaration != null) {
+                    context.ReportDiagnostic(Diagnostic.Create(MethodHasNoSuchParamNameRule, argument.GetLocation(), 
+                        "Constructor", ctorDeclaration.Identifier.Text, ctorDeclaration.ParameterList.ToString(), literal));
+                } else {
+                    var methodDeclaration = (MethodDeclarationSyntax)methodBaseDeclaration;
+                    context.ReportDiagnostic(Diagnostic.Create(MethodHasNoSuchParamNameRule, argument.GetLocation(),
+                        "Method", methodDeclaration.Identifier.Text, methodDeclaration.ParameterList.ToString(), literal));
+                }
+            }
+        }
+    }
+}

src/ErrorProne.NET/ErrorProne.NET/Rules/Formatting/RegexPatternAnalyzer.cs

@@ -36,8 +36,7 @@ public override void Initialize(AnalysisContext context)
         private void AnalyzeRegexCreation(SyntaxNodeAnalysisContext context)
         {
             var objectCreation = (ObjectCreationExpressionSyntax) context.Node;
-
-            var type = context.SemanticModel.GetSymbolInfo(objectCreation.Type).Symbol as ITypeSymbol;
+            var type = context.GetTypeSymbol(objectCreation.Type);
             if (type == null || !type.Equals(context.SemanticModel.GetClrType(typeof(Regex))))
             {
                 return;