Skip to content

Conversation

@rombirli
Copy link
Contributor

@rombirli rombirli commented Jan 21, 2026

⚠️ Warning: Impact on Rule S1452 Sensitivity

This PR aims to remove False Positives (FPs) in code examples involving nested generics, such as:

public class Baz {
  void bar() {
    foo(listOfLists());
  }

  boolean foo(List<List<? extends A>> listList) {
    return true;
  }

  // FP here: The rule suggests removing the wildcard, 
  // but returning List<List<A>> instead would break the code.
  List<List<? extends A>> listOfLists() { 
    return new ArrayList<>();
  }
}

The Proposed Solution:
The fix stops checking wildcards after depth 1 nested types (i.e., removing the recursive check).

Reviewer Note:
Please pay attention to this change in behavior; while it resolves the FPs, it means the rule will no longer flag potential issues in deeper nested structures. I would like your feedback on whether this trade-off makes sense or if we should consider a more surgical approach to the recursion.

@hashicorp-vault-sonar-prod
Copy link

hashicorp-vault-sonar-prod bot commented Jan 21, 2026

SONARJAVA-5936

@rombirli rombirli changed the title SONARJAVA-5936 Remove FPs for wildcards in nested types SONARJAVA-5936 : S1452 Remove FPs for wildcards in nested types Jan 21, 2026
@rombirli rombirli requested a review from BartLucien January 22, 2026 06:55
@rombirli rombirli marked this pull request as ready for review January 22, 2026 06:55
Copy link
Contributor

@BartLucien BartLucien left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just have a small comment concerning the tests.
While it may create some false negatives, as having wildcard in nested types won't be detected. I think it is better than having a issue raised that breaks the compilation if it is fixed. What do you think?

I do not approve as I think there should be a more authoritative answer to your concern.

private static class B extends checks.A {
}
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would add another example with another data structure to have a more robust test. Something similar to the FP that disappeared from the ITS tests seem like good candidates to me.

@rombirli rombirli force-pushed the rombirli/SONARJAVA-5936-S1452-fp-wildcard-nested-type branch from 08adf1a to c96bddd Compare January 23, 2026 12:05
@sonarqube-next
Copy link

private static class A {
}

private static class B extends checks.A {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a particulat reason to use checks.A rather than just A?

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

public class WildCardReturnParameterNestedTypeCheck {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's easier to find samples if the filename ends in ...Sample.java. Maybe we can rename all sample for this check?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants