Please remove rule java:S1452 from existence

SonarQube Cloud
,
1

The reasoning behind this rule makes some wild claims,
does not explain conclusions, and is mostly moot point, the way I see it.
Allow me to clarify, taking what you state in the rule:

A return type containing wildcards cannot be narrowed down in any context.
This indicates that the developer’s intention was likely something else.

No explanation is given what narrowing down means here and why this is a problem.
The only reference that I can find in stated sources [1]
is where they talk about contravariance in method parameters:
“What must be obeyed is that methods taking a Car in original class must be able to take a Car in the subclass instead of narrowing it down as we saw in the SportsCar example.”
as far as I can tell speaking to how an override cannot require subtypes of parameter types specified in the super method.
A totally different story from whatever purportedly applies to return types,
it’s just completely unclear what the first sentence goes on about, and why
this causes misalignment with developer intentions.

The core problem lies in type variance.

Yes, wildcards express variant types.
This is a core feature of java though, and as you’ll see:
there’s absolutely no problem with java having this feature.

Expressions at an input position, […] covariance.
Expressions at an output position, […] contravariance.

True, but very peculiar to talk about expressions outside the method,
used to make a call and receiving its outcome when the issue flagging is
the method itself and its return type.

Peculiar indeed, because when it comes to the rules of overriding a method
in java regarding the parts, then return types (outputs) are covariant
and parameters (inputs) are contravariant.

I guess this peculiarity is mostly a matter of perspective.
But it’s totally unfair to proclaim there is such a singular thing as an “input position” or “output position” and go on to make broad claims about them.

Then some needless stuffing it seems:

In Java, type parameters of a generic type are invariant by default […]

Facts on facts. Don’t see how this relates to any issue though?

<? extends Foo> for covariance (input positions)

Clarifying my alternative perspective:
a method with return type Animal can also return a Dog,
covariant in output position, right?

However, covariance is ineffective for the return type of a method since it is not an input position.

What do you mean, ineffective? It can be used, it has purpose and meaning.
Also: I argue return types are in fact covariant in java. I will get to examples later.

Making it contravariant also has no effect since it is the receiver of the return value which must be contravariant (use-site variance in Java).

Again: contravariant return types can be used, have an effect and mean something different from the invariant or covariant alternate types. Furthermore, what a method decides to return is completely separate from what any receiver does or does not do. What if we called the method from a piece of handwritten JVM bytecode, what then can anyone declare about any receiver?

On to the examples:

List<? extends Animal> getAnimals() { ... } // Noncompliant, wildcard with no use
List<? super Plant> getLifeforms() { ... }  // Noncompliant, wildcard with no use

List<Animal> getAnimals() { ... }           // Compliant, using invariant type instead
List<Plant> getLifeforms() { ... }          // Compliant, using invariant type instead

List<Dog> getAnimals() { ... }              // Compliant, using subtype instead
List<Lifeform> getLifeforms() { ... }       // Compliant, using supertype instead

Allow me to make a counter example from yours:

private List<Dog> dogs = ...;
private List<Cat> cats = ...;

List<? extends Animal> getAnimals() {
	if (LocalDate.now().getDayOfWeek() == DayOfWeek.FRIDAY) {
		return cats;
	} else {
		return dogs;
	}
}

Allowing wildcards here is like deferring exact type choice to runtime.
Effecting read-onlyness of the return value is a bonus in many cases.
There’s just no other valid return type for this method.

And then my own example (abbreviated), where we encountered sonar claiming this issue:

/* Yes, Functions may very well be covariant on their return types */
Function<JsonNode, ? extends Value> createParser(Type type) {
	return switch (type) {
		case Type.INT -> this::parseInt;
		// ...
		case EnumType enumType -> creatEnumParser(enumType);
	}
}
IntValue parseInt(JsonNode input) {...}
Function<JsonNode, EnumValue> enumExtractor(EnumType enumType) {
	return jsonNode -> enumType.getConstant(jsonNode.asText());
}

Curiously though, were one to inline the enumExtractor in this last example,
the necessity of the wildcard also disappears: type inference does automatic conversion
in this case. Regardless, the types here are expressing what they need to express, and it all works perfectly!

In final, getting back to your conclusion:

Consequently, a return type containing wildcards is generally a mistake.

Well no, this is not a consequence of the horseradish that came before it.
I mean, if you were to write “Type variance is so complex, even static analysis tool authors get it wrong, so please don’t use it” then I maybe could get behind it. But as it stand, I believe the best course of action would be for you to remove the rule, having it no longer be part of any profile, least of all the “Sonar way”.
I know we can make a custom profile ourselves, but how many more unsuspecting java developers must suffer?

Sources (taken from sonar’s own resources list on this issue)

[1] (Sinisa Louc - A Complete Guide to Variance in Java and Scala)[https://medium.com/javarevisited/variance-in-java-and-scala-63af925d21dc\\]

2 Likes