16f6c0aecf
Inline adoc files when they are included exactly once. Also fix language tags because this inlining gives us better information on what language the code is written in.
123 lines
3.9 KiB
Plaintext
123 lines
3.9 KiB
Plaintext
== Why is this an issue?
|
||
|
||
To check the type of an object there are several options:
|
||
|
||
* ``++expr is SomeType++`` or ``++expr.GetType() == typeof(SomeType)++`` if the type is known at compile time,
|
||
* ``++typeInstance.IsInstanceOfType(expr)++`` if the type is calculated during runtime.
|
||
|
||
If runtime calculated ``++Type++``s need to be compared:
|
||
|
||
* ``++typeInstance1.IsAssignableFrom(typeInstance2)++``.
|
||
|
||
Depending on whether the type is returned by a ``++GetType()++`` or ``++typeof()++`` call, the ``++IsAssignableFrom()++`` and ``++IsInstanceOfType()++`` might be simplified. Similarly, if the type is ``++sealed++``, the type comparison with ``++==++`` can be converted to an ``++is++`` call. Simplifying the calls also make ``++null++`` checking unnecessary because both ``++is++`` and ``++IsInstanceOfType++`` performs it already.
|
||
|
||
|
||
Finally, utilizing the most concise language constructs for type checking makes the code more readable, so
|
||
|
||
* ``++expr as T != null++`` checks should be simplified to ``++expr is T++``, and
|
||
* ``++expr is T++`` should be converted to ``++expr != null++``, when ``++expr++`` is of type ``++T++``.
|
||
|
||
=== Noncompliant code example
|
||
|
||
[source,csharp]
|
||
----
|
||
class Fruit { }
|
||
sealed class Apple : Fruit { }
|
||
|
||
class Program
|
||
{
|
||
static void Main()
|
||
{
|
||
var apple = new Apple();
|
||
var b = apple != null && apple.GetType() == typeof (Apple); // Noncompliant
|
||
b = typeof(Apple).IsInstanceOfType(apple); // Noncompliant
|
||
if (apple != null)
|
||
{
|
||
b = typeof(Apple).IsAssignableFrom(apple.GetType()); // Noncompliant
|
||
}
|
||
var appleType = typeof (Apple);
|
||
if (apple != null)
|
||
{
|
||
b = appleType.IsAssignableFrom(apple.GetType()); // Noncompliant
|
||
}
|
||
|
||
Fruit f = apple;
|
||
if (f as Apple != null) // Noncompliant
|
||
{
|
||
}
|
||
if (apple is Apple) // Noncompliant
|
||
{
|
||
}
|
||
}
|
||
}
|
||
----
|
||
|
||
=== Compliant solution
|
||
|
||
[source,csharp]
|
||
----
|
||
class Fruit { }
|
||
sealed class Apple : Fruit { }
|
||
|
||
class Program
|
||
{
|
||
static void Main()
|
||
{
|
||
var apple = new Apple();
|
||
var b = apple is Apple;
|
||
b = apple is Apple;
|
||
b = apple is Apple;
|
||
var appleType = typeof(Apple);
|
||
b = appleType.IsInstanceOfType(apple);
|
||
|
||
Fruit f = apple;
|
||
if (f is Apple)
|
||
{
|
||
}
|
||
if (apple != null)
|
||
{
|
||
}
|
||
}
|
||
}
|
||
----
|
||
|
||
=== Exceptions
|
||
|
||
Calling ``++GetType++`` on an object of ``++Nullable<T>++`` type returns the underlying generic type parameter ``++T++``, thus a comparison with ``++typeof(Nullable<T>)++`` can't be simplified to use the ``++is++`` operator, which doesn't make difference between ``++T++`` and ``++T?++``.
|
||
|
||
[source,csharp]
|
||
----
|
||
int? i = 42;
|
||
bool condition = i.GetType() == typeof(int?); // false;
|
||
condition = i is int?; // true
|
||
----
|
||
No issue is reported on the following expressions:
|
||
|
||
* ``++expr is T++`` when either operand of the ``++is++`` operator is a value type. In that case CS0183 or CS0184 reports
|
||
* ``++expr is object++``, as this is a common and efficient pattern to do null checks
|
||
|
||
ifdef::env-github,rspecator-view[]
|
||
|
||
'''
|
||
== Implementation Specification
|
||
(visible only on this page)
|
||
|
||
=== Message
|
||
|
||
Use [the "is" operator|the "IsInstanceOfType()" method|a "null" check] instead.
|
||
|
||
|
||
'''
|
||
== Comments And Links
|
||
(visible only on this page)
|
||
|
||
=== on 24 May 2019, 16:47:10 Andrei Epure wrote:
|
||
\[~nicolas.harraudeau] there's an interesting remark from a user (issue https://github.com/SonarSource/sonar-dotnet/issues/2424[#2424]) who says that it's actually better to perform ``++expr is object++`` rather than ``++expr != null++`` because of performance implications
|
||
|
||
=== on 24 May 2019, 17:15:38 Nicolas Harraudeau wrote:
|
||
\[~andrei.epure] It looks to me like this is a tradeof between readability and performance. Using ``++expr is object++`` to do a null check is more performant but less intuitive than ``++expr != null++``. We could simply accept both ``++expr is object++`` and ``++expr != null++``, explaining in the description the difference. What do you think?
|
||
|
||
include::../comments-and-links.adoc[]
|
||
|
||
endif::env-github,rspecator-view[]
|