Skip to content

Commit

Permalink
Modify rule S2955: promote C# to SonarWay (#3698)
Browse files Browse the repository at this point in the history
  • Loading branch information
sebastien-marichal authored Feb 28, 2024
1 parent d9619fc commit 0a04e8f
Show file tree
Hide file tree
Showing 4 changed files with 76 additions and 71 deletions.
32 changes: 0 additions & 32 deletions rules/S2955/csharp/metadata.json
Original file line number Diff line number Diff line change
@@ -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"
}
64 changes: 25 additions & 39 deletions rules/S2955/csharp/rule.adoc
Original file line number Diff line number Diff line change
@@ -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>(T value)
bool IsDefault<T>(T value)
{
if (value == null) // Noncompliant
{
// ...
}
// ...
}
----

==== Compliant solution

=== Compliant solution

[source,csharp]
[source,csharp,diff-id=1,diff-type=compliant]
----
private bool IsDefault<T>(T value)
bool IsDefault<T>(T value)
{
if(object.Equals(value, default(T)))
{
// ...
}
// ...
}
----

or

[source,csharp]
----
private bool IsDefault<T>(T value) where T : class
bool IsDefault<T>(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[]
24 changes: 24 additions & 0 deletions rules/S2955/metadata.json
Original file line number Diff line number Diff line change
@@ -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"
}
27 changes: 27 additions & 0 deletions rules/S2955/rspecator.adoc
Original file line number Diff line number Diff line change
@@ -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[]

0 comments on commit 0a04e8f

Please sign in to comment.