Misleading rule description (squid:S1452)

[k.p.sza...@gmail.com]

Hello,

I found one of the standard Sonar Java rules to have a misleading description (or downright incorrect). I'm talking about "Generic wildcard types should not be used in return parameters", which says:

Using a wildcard as a return type implicitly means that the return value should be considered read-only, but without any way to enforce this contract. 

Let's take the example of method returning a "List<? extends Animal>". Is it possible on this list to add a Dog, a Cat, ... we simply don't know. The consumer of a method should not have to deal with such disruptive questions.

 This is simply not true. The following code will not compile:

import java.util.ArrayList;
import java.util.List;

public class S1452 {

    class Animal {}
    class Dog extends Animal {}
    class Cat extends Animal {}

    List<? extends Animal> getAnimals() {
        return new ArrayList<>();
    }

    void check() {
        getAnimals().add(new Dog()); // does not compile
        getAnimals().add(new Cat()); // does not compile
        getAnimals().add(new Animal()); // does not compile
        getAnimals().add(null); // OK
    }
}

On the other hand the "compliant" solution without a wildcard accepts any Animal class:

List<Animal> getAnimals() {
    return new ArrayList<>();
}

void check() {
    getAnimals().add(new Dog()); // OK
    getAnimals().add(new Cat()); // OK
    getAnimals().add(new Animal()); // OK
    getAnimals().add(null); // OK
}

Wildcards in Java work similarly to Scala existential types. If I return a List<? extends Animal> it explicitly means that this is a list of some Animal class, but the caller does not know if it's a list of List<Dog>, List<Cat>, etc., so the compiler does not allow adding any elements except for null, which is always valid.

Note that I'm not necessarily advocating to remove this rule altogether. I agree it has its merits as mentioned in Effective Java:

• Do not use wildcard types as return types
  • Rather than providing additional flexibility for your users, it would force them to use wildcard types in client code
  • If the user of a class has to think about wildcard types, there is probably something wrong with the class’s API

I just think this rule description stems from a misunderstanding of how Java wildcards work, and I don't want the misunderstanding to spread onto other users of Sonar. The description should be changed.

Best regards,

Krzysztof Szafrański

[jeanchrist...@sonarsource.com]

Hello Krzysztof,

Thank you for your feedback. You are absolutely right and I've updated RSPEC-1452 accordingly.

Thanks again.

Regards,