diff --git a/sonaranalyzer-dotnet/rspec/cs/S1226_c#.html b/sonaranalyzer-dotnet/rspec/cs/S1226_c#.html index 55e06043ff7..23e947f98cc 100644 --- a/sonaranalyzer-dotnet/rspec/cs/S1226_c#.html +++ b/sonaranalyzer-dotnet/rspec/cs/S1226_c#.html @@ -3,10 +3,12 @@ before reassignment.
-public void DoTheThing(string str, int i, List<string> strings) { - str = i.toString(i); // Noncompliant +public void DoTheThing(string str, int i, List<string> 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 index 3b785c9faed..1c29482fe39 100644 --- 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 index 051ef7268fe..b4c0fa7950a 100644 --- 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 index 88dd9645246..1b962c3f912 100644 --- 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 index bafc2ecb768..387862ea0df 100644 --- 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 index d660add8f7e..0a4705965d8 100644 --- 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 index 95b0238f75d..7dad3f9e871 100644 --- 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 index 4fea214378f..ef0277014d6 100644 --- 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 index f76922e4f7b..7249dafa6c3 100644 --- a/sonaranalyzer-dotnet/src/SonarAnalyzer.CSharp/RspecStrings.resx +++ b/sonaranalyzer-dotnet/src/SonarAnalyzer.CSharp/RspecStrings.resx @@ -1105,7 +1105,7 @@CODE_SMELL -Sonar Code Smell +Sonar Bug 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. @@ -1129,7 +1129,7 @@Method parameters, caught exceptions and foreach variables' initial values should not be ignored -CODE_SMELL +BUG Sonar Code Smell @@ -1807,7 +1807,7 @@CODE_SMELL -Sonar Bug +Sonar Code Smell 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. @@ -1831,7 +1831,7 @@Jump statements should not be used unconditionally -BUG +CODE_SMELL Sonar Bug @@ -2590,7 +2590,7 @@VULNERABILITY -Sonar Bug +Sonar Code Smell 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. @@ -2614,7 +2614,7 @@Parameters should be passed in the correct order -BUG +CODE_SMELL Sonar Bug @@ -3481,7 +3481,7 @@BUG -Sonar Bug +Sonar Code Smell 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. @@ -3505,7 +3505,7 @@Multiline blocks should be enclosed in curly braces -BUG +CODE_SMELL Sonar Bug @@ -4129,7 +4129,7 @@BUG -Sonar Bug +Sonar Code Smell 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. @@ -4153,7 +4153,7 @@Static fields should not be updated in constructors -BUG +CODE_SMELL Sonar Code Smell @@ -7978,7 +7978,7 @@CODE_SMELL -Sonar Code Smell +Sonar Bug 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. @@ -8002,7 +8002,7 @@Empty collections should not be accessed or iterated -CODE_SMELL +BUG Sonar Code Smell diff --git a/sonaranalyzer-dotnet/src/SonarAnalyzer.Utilities/Rules.Description/S1226_cs.html b/sonaranalyzer-dotnet/src/SonarAnalyzer.Utilities/Rules.Description/S1226_cs.html index 4d63cef2adb..23e947f98cc 100644 --- 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 @@ -While it is technically correct to assign to parameters from within method bodies, it is better to use temporary variables to store intermediate -results.
-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 -
-this
was forgotten.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.
-Moreover, some developers might also expect assignments of method parameters to be visible from callers, which is not the case and can confuse -them.
-All parameters should be treated as
+readonly
.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.Noncompliant Code Example
-class MyClass +public void DoTheThing(string str, int i, List<string> 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 - } -} --Compliant Solution
--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 } }diff --git a/sonaranalyzer-dotnet/src/SonarAnalyzer.VisualBasic/RspecStrings.resx b/sonaranalyzer-dotnet/src/SonarAnalyzer.VisualBasic/RspecStrings.resx index e30b18d929d..8303db6cfd8 100644 --- a/sonaranalyzer-dotnet/src/SonarAnalyzer.VisualBasic/RspecStrings.resx +++ b/sonaranalyzer-dotnet/src/SonarAnalyzer.VisualBasic/RspecStrings.resx @@ -442,7 +442,7 @@CODE_SMELL -Sonar Code Smell +Sonar Bug 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. @@ -466,7 +466,7 @@Method parameters and caught exceptions should not be reassigned -CODE_SMELL +BUG Sonar Code Smell diff --git a/sonaranalyzer-dotnet/src/Tests/SonarAnalyzer.UnitTest/PackagingTests/CsRuleTypeMapping.cs b/sonaranalyzer-dotnet/src/Tests/SonarAnalyzer.UnitTest/PackagingTests/CsRuleTypeMapping.cs index 81de44aa60a..605a69dfa6a 100644 --- 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 index 613461fa37e..c9846e02f81 100644 --- 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"],