rspec/rules/S2330/csharp/rule.adoc

== Why is this an issue?

Array covariance is the principle that if an implicit or explicit reference conversion exits from type ``++A++`` to ``++B++``, then the same conversion exists from the array type ``++A[]++`` to ``++B[]++``.


While this array conversion can be useful in readonly situations to pass instances of ``++A[]++`` where ``++B[]++`` is expected, it must be used with care, since assigning an instance of ``++B++`` into an array of ``++A++`` will cause an ``++ArrayTypeMismatchException++`` to be thrown at runtime.


=== Noncompliant code example

[source,csharp]
----
abstract class Fruit { }
class Apple : Fruit { }
class Orange : Fruit { }

class Program
{
  static void Main(string[] args)
  {
    Fruit[] fruits = new Apple[1]; // Noncompliant - array covariance is used
    FillWithOranges(fruits);
  }

  // Just looking at the code doesn't reveal anything suspicious
  static void FillWithOranges(Fruit[] fruits)
  {
    for (int i = 0; i < fruits.Length; i++)
    {
      fruits[i] = new Orange(); // Will throw an ArrayTypeMismatchException
    }
  }
}
----


=== Compliant solution

[source,csharp]
----
abstract class Fruit { }
class Apple : Fruit { }
class Orange : Fruit { }

class Program
{
  static void Main(string[] args)
  {
    Orange[] fruits = new Orange[1]; // Compliant
    FillWithOranges(fruits);
  }

  static void FillWithOranges(Orange[] fruits)
  {
    for (int i = 0; i < fruits.Length; i++)
    {
      fruits[i] = new Orange();
    }
  }
}
----


ifdef::env-github,rspecator-view[]

'''
== Implementation Specification
(visible only on this page)

=== Message

Refactor the code to not rely on potentially unsafe array conversions.


'''
== Comments And Links
(visible only on this page)

=== relates to: S3062

=== on 11 May 2015, 14:33:24 Dinesh Bolkensteyn wrote:
I don't get this explanation - I think we need a real example where something fails

=== on 12 May 2015, 13:09:33 Ann Campbell wrote:
how 'bout now [~dinesh.bolkensteyn]?

=== on 12 May 2015, 16:20:25 Dinesh Bolkensteyn wrote:
Note that we probably also want an issue here:


----
FillWithOranges(new Apple[1]); // Noncompliant
void FillWithOranges(Fruit[] fruits) { fruits[0] = new Orange(); }
----

and that, in an ideal world, we would actually be able to flag the ``++FillWithOranges++`` method which should be refactored to take ``++Orange[]++`` as argument


I'm thinking that we can change this rule to something like: Don't assign into arrays of A when there exists subtypes of A.


So, on the trivial example:

----
object[] a = new string[42]; // This is actually OK - as long as we only plan to read from "a" then nothing bad will happen
a[0] = 42; // Noncompliant - here the rule determines that "a" is an array of "object", for which it is able to find many derived types ({{string}}, {{Int32}}, etc.)
----

WDYT [~ann.campbell.2]?

=== on 14 May 2015, 12:51:42 Ann Campbell wrote:
thanks [~dinesh.bolkensteyn]

=== on 15 Jun 2015, 12:48:51 Tamas Vajk wrote:
\[~ann.campbell.2] this rule is missing the severity and scale mappings

=== on 15 Jun 2015, 13:13:21 Tamas Vajk wrote:
\[~ann.campbell.2] can you create a separate rule, which is an actual bug detection rule based on this one?


This rule says that we should report on ``++Fruit[] fruits = new Apple[1];++``, however, that's perfectly fine, that line won't throw an exception. The problem comes when we try to put a non-``++Apple++`` derived fruit in the array. So we should mark this line: ``++fruits[i] = new Orange();++`` that there is a potential bug here. 


However, currently we have no way of finding these real bugs. So that's why I would create a separate rule.

=== on 15 Jun 2015, 14:24:22 Ann Campbell wrote:
\[~tamas.vajk] SQALE was already there. I've added severity

=== on 15 Jun 2015, 14:47:57 Tamas Vajk wrote:
\[~ann.campbell.2] True, and it should be disabled by default, if I'm right?

=== on 16 Jun 2015, 11:31:44 Ann Campbell wrote:
Correct [~tamas.vajk]

endif::env-github,rspecator-view[]