From 0a04e8fc5e9f857c6c5335e77a6bf62050875807 Mon Sep 17 00:00:00 2001 From: Sebastien Marichal Date: Wed, 28 Feb 2024 16:48:56 +0100 Subject: [PATCH] Modify rule S2955: promote C# to SonarWay (#3698) --- rules/S2955/csharp/metadata.json | 32 ---------------- rules/S2955/csharp/rule.adoc | 64 +++++++++++++------------------- rules/S2955/metadata.json | 24 ++++++++++++ rules/S2955/rspecator.adoc | 27 ++++++++++++++ 4 files changed, 76 insertions(+), 71 deletions(-) create mode 100644 rules/S2955/rspecator.adoc diff --git a/rules/S2955/csharp/metadata.json b/rules/S2955/csharp/metadata.json index 968536c9bb9..2c63c085104 100644 --- a/rules/S2955/csharp/metadata.json +++ b/rules/S2955/csharp/metadata.json @@ -1,34 +1,2 @@ { - "title": "Generic parameters not constrained to reference types should not be compared to \"null\"", - "type": "BUG", - "code": { - "impacts": { - "RELIABILITY": "LOW" - }, - "attribute": "COMPLETE" - }, - "status": "ready", - "remediation": { - "func": "Constant\/Issue", - "constantCost": "10min" - }, - "tags": [ - - ], - "extra": { - "replacementRules": [ - - ], - "legacyKeys": [ - - ] - }, - "defaultSeverity": "Minor", - "ruleSpecification": "RSPEC-2955", - "sqKey": "S2955", - "scope": "All", - "defaultQualityProfiles": [ - - ], - "quickfix": "covered" } diff --git a/rules/S2955/csharp/rule.adoc b/rules/S2955/csharp/rule.adoc index 90af30afd0c..1e1e20927a7 100644 --- a/rules/S2955/csharp/rule.adoc +++ b/rules/S2955/csharp/rule.adoc @@ -1,76 +1,62 @@ == Why is this an issue? -When constraints have not been applied to restrict a generic type parameter to be a reference type, then a value type, such as a ``++struct++``, could also be passed. In such cases, comparing the type parameter to ``++null++`` would always be false, because a ``++struct++`` can be empty, but never ``++null++``. If a value type is truly what's expected, then the comparison should use ``++default()++``. If it's not, then constraints should be added so that no value type can be passed. +In C#, without constraints on a generic type parameter, both https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/keywords/reference-types[reference] and https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/builtin-types/value-types[value] types can be passed. However, comparing this type parameter to `null` can be misleading as value types, like `struct`, can never be null. +== How to fix it -=== Noncompliant code example +To avoid unexpected comparisons: -[source,csharp] +* if you expect a value type, use https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/operators/default#default-operator[default()] for comparison +* if you expect a reference type, add a https://learn.microsoft.com/en-us/dotnet/csharp/programming-guide/generics/constraints-on-type-parameters[constraint] to prevent value types from being passed + +=== Code examples + +==== Noncompliant code example + +[source,csharp,diff-id=1,diff-type=noncompliant] ---- -private bool IsDefault(T value) +bool IsDefault(T value) { if (value == null) // Noncompliant { // ... } - // ... } ---- +==== Compliant solution -=== Compliant solution - -[source,csharp] +[source,csharp,diff-id=1,diff-type=compliant] ---- -private bool IsDefault(T value) +bool IsDefault(T value) { if(object.Equals(value, default(T))) { // ... } - // ... } ---- + or [source,csharp] ---- -private bool IsDefault(T value) where T : class +bool IsDefault(T value) where T : class { - if (value == null) + if (value == null) { // ... } - // ... } ---- +== Resources -ifdef::env-github,rspecator-view[] - -''' -== Implementation Specification -(visible only on this page) - -=== Message - -Use a comparison to "default(xxx)" instead or add a constraint to "xxx" so that it can't be a value type. - - -''' -== Comments And Links -(visible only on this page) - -=== on 22 May 2015, 10:04:25 Tamas Vajk wrote: -Fixed some minor wording issues, and the sample - -=== on 22 May 2015, 12:07:32 Ann Campbell wrote: -Thanks [~tamas.vajk]. Looks good. - -=== on 29 May 2015, 12:50:40 Tamas Vajk wrote: -\[~ann.campbell.2] Could you run through the description? I've change the wording "false negative" because it sounded strange. +=== Documentation -=== on 29 May 2015, 14:50:44 Ann Campbell wrote: -looks good [~tamas.vajk] +* Microsoft Learn - https://learn.microsoft.com/en-us/dotnet/csharp/programming-guide/generics/constraints-on-type-parameters[Constraints on type parameters] +* Microsoft Learn - https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/keywords/reference-types[Reference types] +* Microsoft Learn - https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/builtin-types/value-types[Value types] +* Microsoft Learn - https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/operators/default#default-operator[`default` operator] -endif::env-github,rspecator-view[] +include::../rspecator.adoc[] diff --git a/rules/S2955/metadata.json b/rules/S2955/metadata.json index 2c63c085104..a41fa4c2302 100644 --- a/rules/S2955/metadata.json +++ b/rules/S2955/metadata.json @@ -1,2 +1,26 @@ { + "title": "Generic parameters not constrained to reference types should not be compared to \"null\"", + "type": "BUG", + "code": { + "impacts": { + "RELIABILITY": "LOW" + }, + "attribute": "COMPLETE" + }, + "status": "ready", + "remediation": { + "func": "Constant\/Issue", + "constantCost": "10min" + }, + "tags": [ + + ], + "defaultSeverity": "Minor", + "ruleSpecification": "RSPEC-2955", + "sqKey": "S2955", + "scope": "All", + "defaultQualityProfiles": [ + "Sonar way" + ], + "quickfix": "covered" } diff --git a/rules/S2955/rspecator.adoc b/rules/S2955/rspecator.adoc new file mode 100644 index 00000000000..a4ef703ba59 --- /dev/null +++ b/rules/S2955/rspecator.adoc @@ -0,0 +1,27 @@ +ifdef::env-github,rspecator-view[] + +''' +== Implementation Specification +(visible only on this page) + +=== Message + +Use a comparison to "default(xxx)" instead or add a constraint to "xxx" so that it can't be a value type. + +''' +== Comments And Links +(visible only on this page) + +=== on 22 May 2015, 10:04:25 Tamas Vajk wrote: +Fixed some minor wording issues, and the sample + +=== on 22 May 2015, 12:07:32 Ann Campbell wrote: +Thanks [~tamas.vajk]. Looks good. + +=== on 29 May 2015, 12:50:40 Tamas Vajk wrote: +\[~ann.campbell.2] Could you run through the description? I've change the wording "false negative" because it sounded strange. + +=== on 29 May 2015, 14:50:44 Ann Campbell wrote: +looks good [~tamas.vajk] + +endif::env-github,rspecator-view[]