SonarSource · valhristov · Oct 19, 2017 · Oct 17, 2017 · Oct 17, 2017 · Oct 17, 2017
diff --git a/sonaranalyzer-dotnet/rspec/cs/S1226_c#.html b/sonaranalyzer-dotnet/rspec/cs/S1226_c#.html
@@ -3,10 +3,12 @@
 before reassignment.</p>
 <h2>Noncompliant Code Example</h2>
 <pre>
-public void DoTheThing(string str, int i, List&lt;string&gt; strings) {
-  str = i.toString(i);  // Noncompliant
+public void DoTheThing(string str, int i, List&lt;string&gt; strings)
+{
+  str = i.ToString(i);  // Noncompliant
 
-  foreach (String s in strings) {
+  foreach (var s in strings)
+  {
     s = "hello world";  // Noncompliant
   }
 }

diff --git a/sonaranalyzer-dotnet/rspec/cs/S1226_c#.json b/sonaranalyzer-dotnet/rspec/cs/S1226_c#.json
@@ -1,6 +1,6 @@
 {
-  "type": "CODE_SMELL",
   "title": "Method parameters, caught exceptions and foreach variables' initial values should not be ignored",
+  "type": "BUG",
   "status": "ready",
   "remediation": {
     "func": "Constant\/Issue",

diff --git a/sonaranalyzer-dotnet/rspec/cs/S1751_c#.json b/sonaranalyzer-dotnet/rspec/cs/S1751_c#.json
@@ -1,6 +1,6 @@
 {
   "title": "Jump statements should not be used unconditionally",
-  "type": "BUG",
+  "type": "CODE_SMELL",
   "status": "ready",
   "remediation": {
     "func": "Constant\/Issue",

diff --git a/sonaranalyzer-dotnet/rspec/cs/S2234_c#.json b/sonaranalyzer-dotnet/rspec/cs/S2234_c#.json
@@ -1,6 +1,6 @@
 {
   "title": "Parameters should be passed in the correct order",
-  "type": "BUG",
+  "type": "CODE_SMELL",
   "status": "ready",
   "remediation": {
     "func": "Constant\/Issue",

diff --git a/sonaranalyzer-dotnet/rspec/cs/S2681_c#.json b/sonaranalyzer-dotnet/rspec/cs/S2681_c#.json
@@ -1,6 +1,6 @@
 {
   "title": "Multiline blocks should be enclosed in curly braces",
-  "type": "BUG",
+  "type": "CODE_SMELL",
   "status": "ready",
   "remediation": {
     "func": "Constant\/Issue",

diff --git a/sonaranalyzer-dotnet/rspec/cs/S3010_c#.json b/sonaranalyzer-dotnet/rspec/cs/S3010_c#.json
@@ -1,6 +1,6 @@
 {
   "title": "Static fields should not be updated in constructors",
-  "type": "BUG",
+  "type": "CODE_SMELL",
   "status": "ready",
   "remediation": {
     "func": "Constant\/Issue",

diff --git a/sonaranalyzer-dotnet/rspec/cs/S4158_c#.json b/sonaranalyzer-dotnet/rspec/cs/S4158_c#.json
@@ -1,6 +1,6 @@
 {
   "title": "Empty collections should not be accessed or iterated",
-  "type": "CODE_SMELL",
+  "type": "BUG",
   "status": "ready",
   "remediation": {
     "func": "Constant\/Issue",

diff --git a/sonaranalyzer-dotnet/rspec/vbnet/S1226_vb.net.json b/sonaranalyzer-dotnet/rspec/vbnet/S1226_vb.net.json
@@ -1,6 +1,6 @@
 {
   "title": "Method parameters and caught exceptions should not be reassigned",
-  "type": "CODE_SMELL",
+  "type": "BUG",
   "status": "ready",
   "remediation": {
     "func": "Constant\/Issue",

diff --git a/sonaranalyzer-dotnet/src/SonarAnalyzer.CSharp/RspecStrings.resx b/sonaranalyzer-dotnet/src/SonarAnalyzer.CSharp/RspecStrings.resx
@@ -1105,7 +1105,7 @@
     <value>CODE_SMELL</value>
   </data>
   <data name="S1226_Category" xml:space="preserve">
-    <value>Sonar Code Smell</value>
+    <value>Sonar Bug</value>
   </data>
   <data name="S1226_Description" xml:space="preserve">
     <value>While it is technically correct to assign to parameters from within method bodies, doing so before the parameter value is read is likely a bug. Instead, initial values of parameters, caught exceptions, and foreach parameters should be, if not treated as final, then at least read before reassignment.</value>
@@ -1129,7 +1129,7 @@
     <value>Method parameters, caught exceptions and foreach variables' initial values should not be ignored</value>
   </data>
   <data name="S1226_Type" xml:space="preserve">
-    <value>CODE_SMELL</value>
+    <value>BUG</value>
   </data>
   <data name="S1227_Category" xml:space="preserve">
     <value>Sonar Code Smell</value>
@@ -1807,7 +1807,7 @@
     <value>CODE_SMELL</value>
   </data>
   <data name="S1751_Category" xml:space="preserve">
-    <value>Sonar Bug</value>
+    <value>Sonar Code Smell</value>
   </data>
   <data name="S1751_Description" xml:space="preserve">
     <value>Having an unconditional break, return, (@)throw or goto in a loop renders it useless; the loop will only execute once and the loop structure itself is simply wasted keystrokes.</value>
@@ -1831,7 +1831,7 @@
     <value>Jump statements should not be used unconditionally</value>
   </data>
   <data name="S1751_Type" xml:space="preserve">
-    <value>BUG</value>
+    <value>CODE_SMELL</value>
   </data>
   <data name="S1764_Category" xml:space="preserve">
     <value>Sonar Bug</value>
@@ -2590,7 +2590,7 @@
     <value>VULNERABILITY</value>
   </data>
   <data name="S2234_Category" xml:space="preserve">
-    <value>Sonar Bug</value>
+    <value>Sonar Code Smell</value>
   </data>
   <data name="S2234_Description" xml:space="preserve">
     <value>When the names of parameters in a method call match the names of the method arguments, it contributes to clearer, more readable code. However, when the names match, but are passed in a different order than the method arguments, it indicates a mistake in the parameter order which will likely lead to unexpected results.</value>
@@ -2614,7 +2614,7 @@
     <value>Parameters should be passed in the correct order</value>
   </data>
   <data name="S2234_Type" xml:space="preserve">
-    <value>BUG</value>
+    <value>CODE_SMELL</value>
   </data>
   <data name="S2259_Category" xml:space="preserve">
     <value>Sonar Bug</value>
@@ -3481,7 +3481,7 @@
     <value>BUG</value>
   </data>
   <data name="S2681_Category" xml:space="preserve">
-    <value>Sonar Bug</value>
+    <value>Sonar Code Smell</value>
   </data>
   <data name="S2681_Description" xml:space="preserve">
     <value>Curly braces can be omitted from a one-line block, such as with an if statement or for loop, but doing so can be misleading and induce bugs. </value>
@@ -3505,7 +3505,7 @@
     <value>Multiline blocks should be enclosed in curly braces</value>
   </data>
   <data name="S2681_Type" xml:space="preserve">
-    <value>BUG</value>
+    <value>CODE_SMELL</value>
   </data>
   <data name="S2688_Category" xml:space="preserve">
     <value>Sonar Bug</value>
@@ -4129,7 +4129,7 @@
     <value>BUG</value>
   </data>
   <data name="S3010_Category" xml:space="preserve">
-    <value>Sonar Bug</value>
+    <value>Sonar Code Smell</value>
   </data>
   <data name="S3010_Description" xml:space="preserve">
     <value>Assigning a value to a static field in a constructor could cause unreliable behavior at runtime since it will change the value for all instances of the class.</value>
@@ -4153,7 +4153,7 @@
     <value>Static fields should not be updated in constructors</value>
   </data>
   <data name="S3010_Type" xml:space="preserve">
-    <value>BUG</value>
+    <value>CODE_SMELL</value>
   </data>
   <data name="S3052_Category" xml:space="preserve">
     <value>Sonar Code Smell</value>
@@ -7978,7 +7978,7 @@
     <value>CODE_SMELL</value>
   </data>
   <data name="S4158_Category" xml:space="preserve">
-    <value>Sonar Code Smell</value>
+    <value>Sonar Bug</value>
   </data>
   <data name="S4158_Description" xml:space="preserve">
     <value>When a collection is empty it makes no sense to access or iterate it. Doing so anyway is surely an error; either population was accidentally omitted or the developer doesn't understand the situation.</value>
@@ -8002,7 +8002,7 @@
     <value>Empty collections should not be accessed or iterated</value>
   </data>
   <data name="S4158_Type" xml:space="preserve">
-    <value>CODE_SMELL</value>
+    <value>BUG</value>
   </data>
   <data name="S4214_Category" xml:space="preserve">
     <value>Sonar Code Smell</value>

diff --git a/sonaranalyzer-dotnet/src/SonarAnalyzer.Utilities/Rules.Description/S1226_cs.html b/sonaranalyzer-dotnet/src/SonarAnalyzer.Utilities/Rules.Description/S1226_cs.html
@@ -1,63 +1,15 @@
-<p>While it is technically correct to assign to parameters from within method bodies, it is better to use temporary variables to store intermediate
-results.</p>
-<p>This rule will typically detect cases where a constructor parameter is assigned to itself instead of a field of the same name, i.e. when
-<code>this</code> was forgotten.</p>
-<p>Allowing parameters to be assigned to also reduces the code readability as developers will not be able to know whether the original parameter or
-some temporary variable is being accessed without going through the whole method.</p>
-<p>Moreover, some developers might also expect assignments of method parameters to be visible from callers, which is not the case and can confuse
-them.</p>
-<p>All parameters should be treated as <code>readonly</code>.</p>
+<p>While it is technically correct to assign to parameters from within method bodies, doing so before the parameter value is read is likely a bug.
+Instead, initial values of parameters, caught exceptions, and foreach parameters should be, if not treated as <code>final</code>, then at least read
+before reassignment.</p>
 <h2>Noncompliant Code Example</h2>
 <pre>
-class MyClass
+public void DoTheThing(string str, int i, List&lt;string&gt; strings)
 {
-  public string name;
+  str = i.ToString(i);  // Noncompliant
 
-  public MyClass(string name)
+  foreach (var s in strings)
   {
-    name = name;                    // Noncompliant - useless identity assignment
-  }
-
-  public int Add(int a, int b)
-  {
-    a = a + b;                      // Noncompliant
-
-    /* additional logic */
-
-    return a;                       // Seems like the parameter is returned as is, what is the point?
-  }
-
-  public static void Main()
-  {
-    MyClass foo = new MyClass();
-    int a = 40;
-    int b = 2;
-    foo.Add(a, b);                  // Variable "a" will still hold 40 after this call
-  }
-}
-</pre>
-<h2>Compliant Solution</h2>
-<pre>
-class MyClass
-{
-  public string name;
-
-  public MyClass(string name)
-  {
-    this.name = name;               // Compliant
-  }
-
-  public int Add(int a, int b)
-  {
-    return a + b;                   // Compliant
-  }
-
-  public static void Main()
-  {
-    MyClass foo = new MyClass();
-    int a = 40;
-    int b = 2;
-    foo.Add(a, b);
+    s = "hello world";  // Noncompliant
   }
 }
 </pre>

diff --git a/sonaranalyzer-dotnet/src/SonarAnalyzer.VisualBasic/RspecStrings.resx b/sonaranalyzer-dotnet/src/SonarAnalyzer.VisualBasic/RspecStrings.resx
@@ -442,7 +442,7 @@
     <value>CODE_SMELL</value>
   </data>
   <data name="S1226_Category" xml:space="preserve">
-    <value>Sonar Code Smell</value>
+    <value>Sonar Bug</value>
   </data>
   <data name="S1226_Description" xml:space="preserve">
     <value>While it is technically correct to assign to parameters from within method bodies, doing so before the parameter value is read is likely a bug. Instead, initial values of parameters should be, if not treated as readonly then at least read before reassignment.</value>
@@ -466,7 +466,7 @@
     <value>Method parameters and caught exceptions should not be reassigned</value>
   </data>
   <data name="S1226_Type" xml:space="preserve">
-    <value>CODE_SMELL</value>
+    <value>BUG</value>
   </data>
   <data name="S131_Category" xml:space="preserve">
     <value>Sonar Code Smell</value>

diff --git a/sonaranalyzer-dotnet/src/Tests/SonarAnalyzer.UnitTest/PackagingTests/CsRuleTypeMapping.cs b/sonaranalyzer-dotnet/src/Tests/SonarAnalyzer.UnitTest/PackagingTests/CsRuleTypeMapping.cs
@@ -1223,7 +1223,7 @@ static internal class CsRuleTypeMapping
             //["1223"],
             //["1224"],
             //["1225"],
-            ["1226"] = "CODE_SMELL",
+            ["1226"] = "BUG",
             ["1227"] = "CODE_SMELL",
             //["1228"],
             //["1229"],
@@ -1748,7 +1748,7 @@ static internal class CsRuleTypeMapping
             //["1748"],
             //["1749"],
             //["1750"],
-            ["1751"] = "BUG",
+            ["1751"] = "CODE_SMELL",
             //["1752"],
             //["1753"],
             //["1754"],
@@ -2231,7 +2231,7 @@ static internal class CsRuleTypeMapping
             //["2231"],
             //["2232"],
             //["2233"],
-            ["2234"] = "BUG",
+            ["2234"] = "CODE_SMELL",
             //["2235"],
             //["2236"],
             //["2237"],
@@ -2678,7 +2678,7 @@ static internal class CsRuleTypeMapping
             //["2678"],
             //["2679"],
             //["2680"],
-            ["2681"] = "BUG",
+            ["2681"] = "CODE_SMELL",
             //["2682"],
             //["2683"],
             //["2684"],
@@ -3007,7 +3007,7 @@ static internal class CsRuleTypeMapping
             //["3007"],
             //["3008"],
             //["3009"],
-            ["3010"] = "BUG",
+            ["3010"] = "CODE_SMELL",
             //["3011"],
             //["3012"],
             //["3013"],
@@ -4155,7 +4155,7 @@ static internal class CsRuleTypeMapping
             //["4155"],
             //["4156"],
             //["4157"],
-            ["4158"] = "CODE_SMELL",
+            ["4158"] = "BUG",
             //["4159"],
             //["4160"],
             //["4161"],

diff --git a/sonaranalyzer-dotnet/src/Tests/SonarAnalyzer.UnitTest/PackagingTests/VbRuleTypeMapping.cs b/sonaranalyzer-dotnet/src/Tests/SonarAnalyzer.UnitTest/PackagingTests/VbRuleTypeMapping.cs
@@ -1223,7 +1223,7 @@ static internal class VbRuleTypeMapping
             //["1223"],
             //["1224"],
             //["1225"],
-            ["1226"] = "CODE_SMELL",
+            ["1226"] = "BUG",
             //["1227"],
             //["1228"],
             //["1229"],