[romainbrenguier]

Summary

S9149 (static methods should not hide methods from superclasses) raised on hiding that is required by the design of the parent class. The rule no longer raises in two such cases:

• Covariant static factories: the hiding method returns a strict subtype of the hidden method's return type (e.g. Guava's ImmutableBiMap.of() / copyOf() / builder() hiding ImmutableMap's).
• JUnit 3 public static Test suite(): the JUnit 3 runner looks this method up on each test class, so every subclass has to redeclare it.

Hiding with the same or an unrelated return type, including unknown types, is still reported.

Ruling / autoscan

• guava: all 37 issues removed (all covariant factories).
• commons-beanutils: the 2 suite() issues in test classes removed; the LocaleBeanUtils issues remain.
• autoscan: expecting 1 more false positive, because junit.framework.Test cannot be resolved without bytecode. This is a prediction and not verified locally.

Not addressed

The ticket also suggests not raising when the hidden method comes from an external library. This was not done: the semantic model has no reliable way to tell library classes apart from project classes compiled in other files. Those issues are still reported (e.g. hiding Thread.sleep).

🤖 Generated with Claude Code

Agent workflow

Tool link: https://github.com/SonarSource/languages-experimental-tooling/tree/romain/my-tickets/personal/romain-brenguier
PR created using uv run create_with_claude.py SONARJAVA-7242.txt

[hashicorp-vault-sonar-prod]

SONARJAVA-7242

[github-actions]

Ruling Diff Summary

Detected changes in 2 rule files: 39 issues removed, 0 issues added.

S9149 (java) on commons-beanutils - 2 issues removed, 0 issues added

Removed src/test/java/org/apache/commons/beanutils2/BeanUtils2TestCase.java (line 57)

        52 | 
        53 | 
        54 |     /**
        55 |      * Return the tests included in this test suite.
        56 |      */
>>>     57 |     public static Test suite() {
        58 |         return new TestSuite(BeanUtils2TestCase.class);
        59 |     }
        60 | 
        61 |     /**
        62 |      * Tear down instance variables required by this test case.

Removed src/test/java/org/apache/commons/beanutils2/WrapDynaBeanTestCase.java (line 75)

        70 | 
        71 | 
        72 |     /**
        73 |      * Return the tests included in this test suite.
        74 |      */
>>>     75 |     public static Test suite() {
        76 | 
        77 |         return new TestSuite(WrapDynaBeanTestCase.class);
        78 | 
        79 |     }
        80 | 

S9149 (java) on guava - 37 issues removed, 0 issues added - deleted ruling file

Removed src/com/google/common/collect/ImmutableBiMap.java (line 41)

        36 |   /**
        37 |    * Returns the empty bimap.
        38 |    */
        39 |   // Casting to any type is safe because the set will never hold any elements.
        40 |   @SuppressWarnings("unchecked")
>>>     41 |   public static <K, V> ImmutableBiMap<K, V> of() {
        42 |     return (ImmutableBiMap<K, V>) RegularImmutableBiMap.EMPTY;
        43 |   }
        44 | 
        45 |   /**
        46 |    * Returns an immutable bimap containing a single entry.

Removed src/com/google/common/collect/ImmutableBiMap.java (line 48)

        43 |   }
        44 | 
        45 |   /**
        46 |    * Returns an immutable bimap containing a single entry.
        47 |    */
>>>     48 |   public static <K, V> ImmutableBiMap<K, V> of(K k1, V v1) {
        49 |     return new SingletonImmutableBiMap<K, V>(k1, v1);
        50 |   }
        51 | 
        52 |   /**
        53 |    * Returns an immutable map containing the given entries, in order.

Removed src/com/google/common/collect/ImmutableBiMap.java (line 57)

        52 |   /**
        53 |    * Returns an immutable map containing the given entries, in order.
        54 |    *
        55 |    * @throws IllegalArgumentException if duplicate keys or values are added
        56 |    */
>>>     57 |   public static <K, V> ImmutableBiMap<K, V> of(K k1, V v1, K k2, V v2) {
        58 |     return RegularImmutableBiMap.fromEntries(entryOf(k1, v1), entryOf(k2, v2));
        59 |   }
        60 | 
        61 |   /**
        62 |    * Returns an immutable map containing the given entries, in order.

Removed src/com/google/common/collect/ImmutableListMultimap.java (line 51)

        46 |     implements ListMultimap<K, V> {
        47 | 
        48 |   /** Returns the empty multimap. */
        49 |   // Casting is safe because the multimap will never hold any elements.
        50 |   @SuppressWarnings("unchecked")
>>>     51 |   public static <K, V> ImmutableListMultimap<K, V> of() {
        52 |     return (ImmutableListMultimap<K, V>) EmptyImmutableListMultimap.INSTANCE;
        53 |   }
        54 | 
        55 |   /**
        56 |    * Returns an immutable multimap containing a single entry.

Removed src/com/google/common/collect/ImmutableListMultimap.java (line 58)

        53 |   }
        54 | 
        55 |   /**
        56 |    * Returns an immutable multimap containing a single entry.
        57 |    */
>>>     58 |   public static <K, V> ImmutableListMultimap<K, V> of(K k1, V v1) {
        59 |     ImmutableListMultimap.Builder<K, V> builder = ImmutableListMultimap.builder();
        60 |     builder.put(k1, v1);
        61 |     return builder.build();
        62 |   }
        63 | 

Removed src/com/google/common/collect/ImmutableListMultimap.java (line 67)

        62 |   }
        63 | 
        64 |   /**
        65 |    * Returns an immutable multimap containing the given entries, in order.
        66 |    */
>>>     67 |   public static <K, V> ImmutableListMultimap<K, V> of(K k1, V v1, K k2, V v2) {
        68 |     ImmutableListMultimap.Builder<K, V> builder = ImmutableListMultimap.builder();
        69 |     builder.put(k1, v1);
        70 |     builder.put(k2, v2);
        71 |     return builder.build();
        72 |   }

Removed src/com/google/common/collect/ImmutableSetMultimap.java (line 59)

        54 |     implements SetMultimap<K, V> {
        55 | 
        56 |   /** Returns the empty multimap. */
        57 |   // Casting is safe because the multimap will never hold any elements.
        58 |   @SuppressWarnings("unchecked")
>>>     59 |   public static <K, V> ImmutableSetMultimap<K, V> of() {
        60 |     return (ImmutableSetMultimap<K, V>) EmptyImmutableSetMultimap.INSTANCE;
        61 |   }
        62 | 
        63 |   /**
        64 |    * Returns an immutable multimap containing a single entry.

Removed src/com/google/common/collect/ImmutableSetMultimap.java (line 66)

        61 |   }
        62 | 
        63 |   /**
        64 |    * Returns an immutable multimap containing a single entry.
        65 |    */
>>>     66 |   public static <K, V> ImmutableSetMultimap<K, V> of(K k1, V v1) {
        67 |     ImmutableSetMultimap.Builder<K, V> builder = ImmutableSetMultimap.builder();
        68 |     builder.put(k1, v1);
        69 |     return builder.build();
        70 |   }
        71 | 

Removed src/com/google/common/collect/ImmutableSetMultimap.java (line 77)

        72 |   /**
        73 |    * Returns an immutable multimap containing the given entries, in order.
        74 |    * Repeated occurrences of an entry (according to {@link Object#equals}) after
        75 |    * the first are ignored.
        76 |    */
>>>     77 |   public static <K, V> ImmutableSetMultimap<K, V> of(K k1, V v1, K k2, V v2) {
        78 |     ImmutableSetMultimap.Builder<K, V> builder = ImmutableSetMultimap.builder();
        79 |     builder.put(k1, v1);
        80 |     builder.put(k2, v2);
        81 |     return builder.build();
        82 |   }

Removed src/com/google/common/collect/ImmutableSortedMap.java (line 85)

        80 |    * Returns the empty sorted map.
        81 |    */
        82 |   @SuppressWarnings("unchecked")
        83 |   // unsafe, comparator() returns a comparator on the specified type
        84 |   // TODO(kevinb): evaluate whether or not of().comparator() should return null
>>>     85 |   public static <K, V> ImmutableSortedMap<K, V> of() {
        86 |     return (ImmutableSortedMap<K, V>) NATURAL_EMPTY_MAP;
        87 |   }
        88 | 
        89 |   /**
        90 |    * Returns an immutable map containing a single entry.

Removed src/com/google/common/collect/ImmutableSortedMap.java (line 180)

       175 |    *         comparable
       176 |    * @throws NullPointerException if any key or value in {@code map} is null
       177 |    * @throws IllegalArgumentException if any two keys are equal according to
       178 |    *         their natural ordering
       179 |    */
>>>    180 |   public static <K, V> ImmutableSortedMap<K, V> copyOf(Map<? extends K, ? extends V> map) {
       181 |     // Hack around K not being a subtype of Comparable.
       182 |     // Unsafe, see ImmutableSortedSetFauxverideShim.
       183 |     @SuppressWarnings("unchecked")
       184 |     Ordering<K> naturalOrder = (Ordering<K>) NATURAL_ORDER;
       185 |     return copyOfInternal(map, naturalOrder);

Removed src/com/google/common/collect/ImmutableSortedMap.java (line 218)

       213 |    * @throws IllegalArgumentException if any two keys are equal according to the
       214 |    *         comparator
       215 |    * @since 19.0
       216 |    */
       217 |   @Beta
>>>    218 |   public static <K, V> ImmutableSortedMap<K, V> copyOf(
       219 |       Iterable<? extends Entry<? extends K, ? extends V>> entries) {
       220 |     // Hack around K not being a subtype of Comparable.
       221 |     // Unsafe, see ImmutableSortedSetFauxverideShim.
       222 |     @SuppressWarnings("unchecked")
       223 |     Ordering<K> naturalOrder = (Ordering<K>) NATURAL_ORDER;

Removed src/com/google/common/collect/ImmutableSortedMultiset.java (line 63)

        58 | 
        59 |   /**
        60 |    * Returns the empty immutable sorted multiset.
        61 |    */
        62 |   @SuppressWarnings("unchecked")
>>>     63 |   public static <E> ImmutableSortedMultiset<E> of() {
        64 |     return (ImmutableSortedMultiset) NATURAL_EMPTY_MULTISET;
        65 |   }
        66 | 
        67 |   /**
        68 |    * Returns an immutable sorted multiset containing a single element.

Removed src/com/google/common/collect/ImmutableSortedMultiset.java (line 171)

       166 |    * comparable.
       167 |    *
       168 |    * @throws ClassCastException if the elements are not mutually comparable
       169 |    * @throws NullPointerException if any of {@code elements} is null
       170 |    */
>>>    171 |   public static <E> ImmutableSortedMultiset<E> copyOf(Iterable<? extends E> elements) {
       172 |     // Hack around E not being a subtype of Comparable.
       173 |     // Unsafe, see ImmutableSortedMultisetFauxverideShim.
       174 |     @SuppressWarnings("unchecked")
       175 |     Ordering<E> naturalOrder = (Ordering<E>) Ordering.<Comparable>natural();
       176 |     return copyOf(naturalOrder, elements);

Removed src/com/google/common/collect/ImmutableSortedMultiset.java (line 189)

       184 |    * comparable.
       185 |    *
       186 |    * @throws ClassCastException if the elements are not mutually comparable
       187 |    * @throws NullPointerException if any of {@code elements} is null
       188 |    */
>>>    189 |   public static <E> ImmutableSortedMultiset<E> copyOf(Iterator<? extends E> elements) {
       190 |     // Hack around E not being a subtype of Comparable.
       191 |     // Unsafe, see ImmutableSortedMultisetFauxverideShim.
       192 |     @SuppressWarnings("unchecked")
       193 |     Ordering<E> naturalOrder = (Ordering<E>) Ordering.<Comparable>natural();
       194 |     return copyOf(naturalOrder, elements);

[gitar-bot]

Code Review 👍 Approved with suggestions 0 closed / 1 findings

🟡 Medium risk · The S9149 rule now suppresses covariant-return and JUnit 3 suite hiding cases, which could conceal unintended static-method hiding.

Refines S9149 so it no longer raises when a static method hides a superclass method with a covariant return type (such as Guava's ImmutableBiMap.of()) or when a JUnit 3 public static Test suite() must be redeclared; hiding with the same or unrelated return type is still reported. Approved with suggestions: the fixture UnrelatedReturnParent.value() / UnrelatedReturnChild.value() in java-checks-test-sources/default/src/main/java/checks/StaticMethodHidingCheckSample.java (lines 370-380) both return Integer, so it does not test an unrelated return type. Either rename it to reflect the same-return-type case or add a true unrelated-return case (e.g. Integer vs String) under files/non-compiling/checks/StaticMethodHidingCheckSample.java.

💡 Quality: "Unrelated return type" test actually uses identical return types

📄 java-checks-test-sources/default/src/main/java/checks/StaticMethodHidingCheckSample.java:370-380

UnrelatedReturnParent.value() and UnrelatedReturnChild.value() both return Integer, so this fixture repeats the same-return-type case that ImmutableSetLike.create() already covers (Object vs Object). The PR description says hiding with an "unrelated return type" is still reported, but no test checks that. Java (JLS 8.4.8.3) also rejects hiding with a return type that is not return-type-substitutable, so that case can only happen in non-compiling code. Either rename the fixture to say it covers the same return type, or move a real unrelated-return case (e.g. Integer vs String) into files/non-compiling/checks/StaticMethodHidingCheckSample.java so the stated behavior is actually tested.

Rename the fixture so it describes what it tests

static class SameReturnParent {
  static Integer value() {
    return 1;
  }
}

static class SameReturnChild extends SameReturnParent {
  static Integer value() { // Noncompliant {{Rename this method; it hides "value" in "SameReturnParent".}}
    return 2;
  }
}

🤖 Prompt for agents

Code Review: Refines S9149 so it no longer raises when a static method hides a superclass method with a covariant return type (such as Guava's `ImmutableBiMap.of()`) or when a JUnit 3 `public static Test suite()` must be redeclared; hiding with the same or unrelated return type is still reported. Approved with suggestions: the fixture `UnrelatedReturnParent.value()` / `UnrelatedReturnChild.value()` in `java-checks-test-sources/default/src/main/java/checks/StaticMethodHidingCheckSample.java` (lines 370-380) both return `Integer`, so it does not test an unrelated return type. Either rename it to reflect the same-return-type case or add a true unrelated-return case (e.g. `Integer` vs `String`) under `files/non-compiling/checks/StaticMethodHidingCheckSample.java`.

1. 💡 Quality: "Unrelated return type" test actually uses identical return types
   Files: java-checks-test-sources/default/src/main/java/checks/StaticMethodHidingCheckSample.java:370-380

   `UnrelatedReturnParent.value()` and `UnrelatedReturnChild.value()` both return `Integer`, so this fixture repeats the same-return-type case that `ImmutableSetLike.create()` already covers (`Object` vs `Object`). The PR description says hiding with an "unrelated return type" is still reported, but no test checks that. Java (JLS 8.4.8.3) also rejects hiding with a return type that is not return-type-substitutable, so that case can only happen in non-compiling code. Either rename the fixture to say it covers the same return type, or move a real unrelated-return case (e.g. `Integer` vs `String`) into `files/non-compiling/checks/StaticMethodHidingCheckSample.java` so the stated behavior is actually tested.

   Fix (Rename the fixture so it describes what it tests):
   static class SameReturnParent {
     static Integer value() {
       return 1;
     }
   }
   
   static class SameReturnChild extends SameReturnParent {
     static Integer value() { // Noncompliant {{Rename this method; it hides "value" in "SameReturnParent".}}
       return 2;
     }
   }

Verification Rules 2/2 met Knowledge ✅ 1 Repos 4 inspected

Functional validation No results

Rules 2 passed 1 not applicable

✅ Internal artifact registries
✅ Repository license compliance

Knowledge ✅ 1 expectation met

Ruling baseline project-key format expectation met

Broader impact Repos 4 inspected

Cross-repo 4 repositories inspected 0 open impacts

SonarSource/sonarlint-vscode Inspected
SonarSource/sonarlint-intellij Inspected
SonarSource/sonarlint-language-server Inspected
SonarSource/sonar-java-symbolic-execution Inspected

Auto-approval Not enabled

Set up

_{For more verification details, comment gitar display:verbose.}

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing compact verification results.

Comment with these commands to change the behavior for this request:

Auto-apply	Compact
gitar auto-apply:on	gitar display:verbose

_{Was this helpful? React with 👍 / 👎 | Gitar}

[gitar-bot]

💡 Quality: "Unrelated return type" test actually uses identical return types

UnrelatedReturnParent.value() and UnrelatedReturnChild.value() both return Integer, so this fixture repeats the same-return-type case that ImmutableSetLike.create() already covers (Object vs Object). The PR description says hiding with an "unrelated return type" is still reported, but no test checks that. Java (JLS 8.4.8.3) also rejects hiding with a return type that is not return-type-substitutable, so that case can only happen in non-compiling code. Either rename the fixture to say it covers the same return type, or move a real unrelated-return case (e.g. Integer vs String) into files/non-compiling/checks/StaticMethodHidingCheckSample.java so the stated behavior is actually tested.

Rename the fixture so it describes what it tests:

static class SameReturnParent {
  static Integer value() {
    return 1;
  }
}

static class SameReturnChild extends SameReturnParent {
  static Integer value() { // Noncompliant {{Rename this method; it hides "value" in "SameReturnParent".}}
    return 2;
  }
}

• Apply fix

_{Check the box to apply the fix or reply for a change | Was this helpful? React with 👍 / 👎}

[sonarqube-next]

Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
0 Dependency risks
91.7% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube

[romainbrenguier]

/pvf S9149

[hashicorp-vault-sonar-prod]

📊 Performance Validation complete
🆕 0 new · 🗑️ 177 lost issues
View dashboard