rspec/rules/S3610/csharp/rule.adoc

78 lines
2.7 KiB
Plaintext
Raw Normal View History

== Why is this an issue?
2021-04-28 16:49:39 +02:00
Calling ``++GetType()++`` on a nullable object returns the underlying value type. Thus, comparing the returned ``++Type++`` object to ``++typeof(Nullable<SomeType>)++`` doesn't make sense. The comparison either throws an exception or the result can be known at compile time.
=== Noncompliant code example
2021-04-28 16:49:39 +02:00
2022-02-04 17:28:24 +01:00
[source,csharp]
2021-04-28 16:49:39 +02:00
----
int? nullable = 42;
bool comparison = nullable.GetType() == typeof(Nullable<int>); // Noncompliant, always false
comparison = nullable.GetType() != typeof(Nullable<int>); // Noncompliant, always true
nullable = null;
comparison = nullable.GetType() != typeof(Nullable<int>); // Noncompliant, calling GetType on a null always throws an exception
----
ifdef::env-github,rspecator-view[]
'''
== Implementation Specification
(visible only on this page)
=== Message
Remove this redundant type comparison.
=== Highlighting
full expression
'''
== Comments And Links
(visible only on this page)
=== relates to: S2219
=== on 25 Apr 2016, 10:24:59 Tamas Vajk wrote:
\[~ann.campbell.2], could you review this RSPEC? Thanks.
=== on 26 Apr 2016, 17:40:16 Ann Campbell wrote:
\[~tamas.vajk] this rule is a subset of what would be covered by an implementation of RSPEC-2583. When reading it my first thought was that you wrote it stand-alone to cover a R# rule, but there are no references...?
Also, if we retain this RSpec, IMO you should add a compliant solution. From the description and code sample, I'm _guessing_ compliance is a straightforward code change...?
=== on 27 Apr 2016, 08:41:57 Tamas Vajk wrote:
\[~ann.campbell.2] yes, it's a special case of RSPEC-2583.
No, it's not a R# rule. It's the outcome of a bug in one of our rules (RSPEC-2219, added an exception there).
The compliant solution is not straightforward. We can't simply replace the comparison with ``++true++`` or ``++false++`` because that was definitely not what was meant. We could change the ``++typeof(Nullable<int>)++`` to ``++typeof(int)++``, but I don't think that covers the user intent. Most probably the user wanted to check if ``++nullable++`` is a ``++Nullable<T>++`` or not, but that's not possible with ``++GetType++``. And there's no straightforward way to do it, unless you have the type of ``++nullable++`` at compile time.
=== on 27 Apr 2016, 17:44:22 Ann Campbell wrote:
\[~tamas.vajk] I had assumed you'd want to use ``++is++`` (or something similar) instead. Glad I didn't fill in a compliant solution! :-)
For me, the description moves very quickly from
____
Calling GetType() on a nullable object returns the underlying value type.
____
to
____
Thus, comparing the returned Type object to typeof(Nullable<SomeType>) doesn't make sense.
____
But if C#ers will understand, I'm good with it.
endif::env-github,rspecator-view[]