Rule S3693: Exception constructors should not throw exceptions (#629)

sonaranalyzer-dotnet/its/expected/Nancy/Nancy-{34576216-0DCA-4B0F-A0DC-9075E75A676F}-S3693.json

@@ -0,0 +1,17 @@
+{
+"issues":  [
+{
+"id":  "S3693",
+"message":  "Avoid throwing exceptions in this constructor.",
+"location":  {
+"uri":  "Nancy\src\Nancy\ModelBinding\ModelBindingException.cs",
+"region":  {
+"startLine":  36,
+"startColumn":  17,
+"endLine":  36,
+"endColumn":  62
+}
+}
+}
+]
+}

sonaranalyzer-dotnet/rspec/cs/S3693_c#.html

@@ -0,0 +1,18 @@
+<p>It may be a good idea to raise an exception in a constructor if you're unable to fully flesh the object in question, but not in an
+<code>exception</code> constructor. If you do, you'll interfere with the exception that was originally being thrown. Further, it is highly unlikely
+that an exception raised in the creation of an exception will be properly handled in the calling code, and the unexpected, unhandled exception will
+lead to program termination.</p>
+<h2>Noncompliant Code Example</h2>
+<pre>
+class MyException: Exception
+{
+    public void MyException()
+    {
+         if (bad_thing)
+         {
+             throw new Exception("A bad thing happened");  // Noncompliant
+          }
+    }
+}
+</pre>
+

sonaranalyzer-dotnet/rspec/cs/S3693_c#.json

@@ -0,0 +1,13 @@
+{
+  "title": "Exception constructors should not throw exceptions",
+  "type": "BUG",
+  "status": "ready",
+  "remediation": {
+    "func": "Constant\/Issue",
+    "constantCost": "15min"
+  },
+  "tags": [
+
+  ],
+  "defaultSeverity": "Blocker"
+}

sonaranalyzer-dotnet/rspec/cs/Sonar_way_profile.json

@@ -149,6 +149,7 @@
     "S3626",
     "S3649",
     "S3655",
+    "S3693",
     "S3776",
     "S3869",
     "S3871",

sonaranalyzer-dotnet/src/SonarAnalyzer.CSharp/RspecStrings.resx

@@ -5613,6 +5613,33 @@
   <data name="S3655_Type" xml:space="preserve">
     <value>BUG</value>
   </data>
+  <data name="S3693_Category" xml:space="preserve">
+    <value>Sonar Bug</value>
+  </data>
+  <data name="S3693_Description" xml:space="preserve">
+    <value>It may be a good idea to raise an exception in a constructor if you're unable to fully flesh the object in question, but not in an exception constructor. If you do, you'll interfere with the exception that was originally being thrown. Further, it is highly unlikely that an exception raised in the creation of an exception will be properly handled in the calling code, and the unexpected, unhandled exception will lead to program termination.</value>
+  </data>
+  <data name="S3693_IsActivatedByDefault" xml:space="preserve">
+    <value>True</value>
+  </data>
+  <data name="S3693_Remediation" xml:space="preserve">
+    <value>Constant/Issue</value>
+  </data>
+  <data name="S3693_RemediationCost" xml:space="preserve">
+    <value>15min</value>
+  </data>
+  <data name="S3693_Severity" xml:space="preserve">
+    <value>Blocker</value>
+  </data>
+  <data name="S3693_Tags" xml:space="preserve">
+    <value />
+  </data>
+  <data name="S3693_Title" xml:space="preserve">
+    <value>Exception constructors should not throw exceptions</value>
+  </data>
+  <data name="S3693_Type" xml:space="preserve">
+    <value>BUG</value>
+  </data>
   <data name="S3717_Category" xml:space="preserve">
     <value>Sonar Code Smell</value>
   </data>

sonaranalyzer-dotnet/src/SonarAnalyzer.CSharp/Rules/ExceptionConstructorShouldNotThrow.cs

@@ -0,0 +1,72 @@
+/*
+ * SonarAnalyzer for .NET
+ * Copyright (C) 2015-2017 SonarSource SA
+ * mailto: contact AT sonarsource DOT com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+
+using System.Collections.Immutable;
+using System.Linq;
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.CSharp;
+using Microsoft.CodeAnalysis.CSharp.Syntax;
+using Microsoft.CodeAnalysis.Diagnostics;
+using SonarAnalyzer.Common;
+using SonarAnalyzer.Helpers;
+
+namespace SonarAnalyzer.Rules.CSharp
+{
+    [DiagnosticAnalyzer(LanguageNames.CSharp)]
+    [Rule(DiagnosticId)]
+    public sealed class ExceptionConstructorShouldNotThrow : SonarDiagnosticAnalyzer
+    {
+        internal const string DiagnosticId = "S3693";
+        private const string MessageFormat = "Avoid throwing exceptions in this constructor.";
+
+        private static readonly DiagnosticDescriptor rule =
+            DiagnosticDescriptorBuilder.GetDescriptor(DiagnosticId, MessageFormat, RspecStrings.ResourceManager);
+        public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(rule);
+
+        protected override void Initialize(SonarAnalysisContext context)
+        {
+            context.RegisterSyntaxNodeActionInNonGenerated(
+                c =>
+                {
+                    var classDeclaration = (ClassDeclarationSyntax)c.Node;
+
+                    var classSymbol = c.SemanticModel.GetDeclaredSymbol(classDeclaration);
+                    if (classSymbol == null ||
+                        !classSymbol.DerivesFrom(KnownType.System_Exception))
+                    {
+                        return;
+                    }
+
+                    var throwStatementsPerCtor = classDeclaration.Members
+                        .OfType<ConstructorDeclarationSyntax>()
+                        .Select(ctor => ctor.DescendantNodes().OfType<ThrowStatementSyntax>().ToList())
+                        .Where(@throw => @throw.Count > 0)
+                        .ToList();
+
+                    foreach (var throwStatement in throwStatementsPerCtor)
+                    {
+                        c.ReportDiagnostic(Diagnostic.Create(rule, throwStatement.First().GetLocation(),
+                            throwStatement.Skip(1).Select(@throw => @throw.GetLocation())));
+                    }
+                },
+                SyntaxKind.ClassDeclaration);
+        }
+    }
+}

sonaranalyzer-dotnet/src/SonarAnalyzer.Utilities/Rules.Description/S3693.html

@@ -0,0 +1,18 @@
+<p>It may be a good idea to raise an exception in a constructor if you're unable to fully flesh the object in question, but not in an
+<code>exception</code> constructor. If you do, you'll interfere with the exception that was originally being thrown. Further, it is highly unlikely
+that an exception raised in the creation of an exception will be properly handled in the calling code, and the unexpected, unhandled exception will
+lead to program termination.</p>
+<h2>Noncompliant Code Example</h2>
+<pre>
+class MyException: Exception
+{
+    public void MyException()
+    {
+         if (bad_thing)
+         {
+             throw new Exception("A bad thing happened");  // Noncompliant
+          }
+    }
+}
+</pre>
+

sonaranalyzer-dotnet/src/Tests/SonarAnalyzer.UnitTest/PackagingTests/RuleTypeTests.cs

@@ -3690,7 +3690,7 @@ private static void DetectTypeChanges(ResourceManager resourceManager, IImmutabl
             //["3690"],
             //["3691"],
             //["3692"],
-            //["3693"],
+            ["3693"] = "BUG",
             //["3694"],
             //["3695"],
             //["3696"],

sonaranalyzer-dotnet/src/Tests/SonarAnalyzer.UnitTest/Rules/ExceptionConstructorShouldNotThrowTest.cs

@@ -0,0 +1,37 @@
+/*
+ * SonarAnalyzer for .NET
+ * Copyright (C) 2015-2017 SonarSource SA
+ * mailto: contact AT sonarsource DOT com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+
+using Microsoft.VisualStudio.TestTools.UnitTesting;
+using SonarAnalyzer.Rules.CSharp;
+
+namespace SonarAnalyzer.UnitTest.Rules
+{
+    [TestClass]
+    public class ExceptionConstructorShouldNotThrowTest
+    {
+        [TestMethod]
+        [TestCategory("Rule")]
+        public void ExceptionConstructorShouldNotThrow()
+        {
+            Verifier.VerifyAnalyzer(@"TestCases\ExceptionConstructorShouldNotThrow.cs",
+                new ExceptionConstructorShouldNotThrow());
+        }
+    }
+}

sonaranalyzer-dotnet/src/Tests/SonarAnalyzer.UnitTest/TestCases/ExceptionConstructorShouldNotThrow.cs

@@ -0,0 +1,77 @@
+using System;
+
+namespace Tests.Diagnostics
+{
+    class MyException : Exception
+    {
+        public MyException()
+        {
+            throw new Exception(); // Noncompliant {{Avoid throwing exceptions in this constructor.}}
+//          ^^^^^^^^^^^^^^^^^^^^^^
+        }
+    }
+
+    class MyException2 : Exception
+    {
+        public MyException2()
+        {
+
+        }
+
+        public MyException2(int i)
+        {
+            if (i == 42)
+            {
+                throw new Exception(); // Noncompliant
+            }
+        }
+    }
+
+    class MyException3 : Exception
+    {
+        public MyException3(int i)
+        {
+            if (i == 42)
+            {
+                throw new Exception(); // Noncompliant
+            }
+            else
+            {
+                throw new ArgumentException(); // Secondary
+            }
+        }
+    }
+
+    class SubException : MyException
+    {
+        public SubException()
+        {
+            throw new FieldAccessException(); // Noncompliant
+        }
+    }
+
+    class MyException4 : Exception
+    {
+        public MyException4()
+        {
+            throw; // Noncompliant
+        }
+    }
+
+    class MyException5 : Exception
+    {
+        public MyException5()
+        {
+            var ex = new Exception();
+            throw ex; // Noncompliant
+        }
+    }
+
+    class Something
+    {
+        public Something()
+        {
+            throw new Exception(); // Compliant
+        }
+    }
+}