SONARJAVA-6741 Implement new rule S9149: Static methods should not hide methods from superclasses

[romainbrenguier]

Detect static methods in subclasses that hide static methods from superclasses by having the same name and parameter types. This catches a common source of confusion where developers expect polymorphic behavior but get compile-time binding instead.

[hashicorp-vault-sonar-prod]

SONARJAVA-6741

[gitar-bot]

💡 Quality: Redundant hierarchy walk: lookupSymbols already returns inherited members

Symbol.TypeSymbol.lookupSymbols(name) returns symbols accessible from the type including inherited members (per the API javadoc, contrasted with memberSymbols() which does not). Therefore calling it on the immediate superclass already covers the whole ancestor chain, and the surrounding while (superClass != null) loop that re-invokes checkHiding on every ancestor is redundant work. Consider dropping the loop and calling checkHiding once on owner.superClass(), which also simplifies the code.

lookupSymbols already resolves inherited static methods, so a single check on the direct superclass suffices.:

Type superClass = owner.superClass();
if (superClass != null) {
  checkHiding(methodTree, methodSymbol, superClass);
}

• Apply fix

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

[github-actions]

❌ Ruling needs updating. A fix PR has been created: #5912

Please review and merge it into your branch.

[github-actions]

Ruling Diff Summary

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

S9149 (java) on commons-beanutils - 0 issues removed, 10 issues added - new ruling file

Added src/main/java/org/apache/commons/beanutils2/locale/LocaleBeanUtils.java (line 149)

       144 |      * @throws NoSuchMethodException if an accessor method for this
       145 |      *  propety cannot be found
       146 |      *
       147 |      * @see LocaleBeanUtilsBean#getIndexedProperty(Object, String)
       148 |      */
>>>    149 |     public static String getIndexedProperty(final Object bean, final String name)
       150 |             throws IllegalAccessException, InvocationTargetException,
       151 |             NoSuchMethodException {
       152 | 
       153 |         return LocaleBeanUtilsBean.getLocaleBeanUtilsInstance().getIndexedProperty(bean, name);
       154 |     }

Added src/main/java/org/apache/commons/beanutils2/locale/LocaleBeanUtils.java (line 206)

       201 |      * @throws NoSuchMethodException if an accessor method for this
       202 |      *  propety cannot be found
       203 |      *
       204 |      * @see LocaleBeanUtilsBean#getIndexedProperty(Object, String, int)
       205 |      */
>>>    206 |     public static String getIndexedProperty(final Object bean,
       207 |                                             final String name, final int index)
       208 |             throws IllegalAccessException, InvocationTargetException,
       209 |             NoSuchMethodException {
       210 |         return LocaleBeanUtilsBean.getLocaleBeanUtilsInstance().getIndexedProperty(bean, name, index);
       211 |     }

Added src/main/java/org/apache/commons/beanutils2/locale/LocaleBeanUtils.java (line 261)

       256 |      * @throws NoSuchMethodException if an accessor method for this
       257 |      *  propety cannot be found
       258 |      *
       259 |      * @see LocaleBeanUtilsBean#getSimpleProperty(Object, String)
       260 |      */
>>>    261 |     public static String getSimpleProperty(final Object bean, final String name)
       262 |             throws IllegalAccessException, InvocationTargetException,
       263 |             NoSuchMethodException {
       264 | 
       265 |         return LocaleBeanUtilsBean.getLocaleBeanUtilsInstance().getSimpleProperty(bean, name);
       266 |     }

Added 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.

Added 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 - 0 issues removed, 60 issues added - new ruling file

Added src/com/google/common/collect/ContiguousSet.java (line 193)

       188 |    *
       189 |    * @throws UnsupportedOperationException always
       190 |    * @deprecated Use {@link #create}.
       191 |    */
       192 |   @Deprecated
>>>    193 |   public static <E> ImmutableSortedSet.Builder<E> builder() {
       194 |     throw new UnsupportedOperationException();
       195 |   }
       196 | }

Added 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.

Added 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.

Added 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.

Added 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.

Added 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 | 

Added 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 |   }

Added 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.

Added 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 | 

Added 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 |   }

Added 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.

Added 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);

Added 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;

Added src/com/google/common/collect/ImmutableSortedMapFauxverideShim.java (line 37)

        32 |    * @throws UnsupportedOperationException always
        33 |    * @deprecated Use {@link ImmutableSortedMap#naturalOrder}, which offers
        34 |    *     better type-safety.
        35 |    */
        36 |   @Deprecated
>>>     37 |   public static <K, V> ImmutableSortedMap.Builder<K, V> builder() {
        38 |     throw new UnsupportedOperationException();
        39 |   }
        40 | 
        41 |   /**
        42 |    * Not supported. <b>You are attempting to create a map that may contain a

Added src/com/google/common/collect/ImmutableSortedMapFauxverideShim.java (line 51)

        46 |    * @throws UnsupportedOperationException always
        47 |    * @deprecated <b>Pass a key of type {@code Comparable} to use {@link
        48 |    *     ImmutableSortedMap#of(Comparable, Object)}.</b>
        49 |    */
        50 |   @Deprecated
>>>     51 |   public static <K, V> ImmutableSortedMap<K, V> of(K k1, V v1) {
        52 |     throw new UnsupportedOperationException();
        53 |   }
        54 | 
        55 |   /**
        56 |    * Not supported. <b>You are attempting to create a map that may contain

[gitar-bot]

CI failed: Integration test failures occurred due to mismatched ruling baselines and missing test output directories introduced by the new S9149 rule changes.

Overview

Two integration test failures were found across jobs: one due to an empty or missing actual output directory during the autoscan integration test, and another due to a diff mismatch in the ruling QA integration test caused by the new rule S9149 implementation.

Failures

Autoscan Diff Comparison Failure (confidence: high)

• Type: test
• Affected jobs: 93719493452
• Related to change: yes
• Root cause: The integration test step expected target/actual/autoscan-diffs to be generated, but it was missing or empty because the preceding test preparation phase failed or did not run.
• Suggested fix: Ensure that the build and test preparation steps successfully compile and generate the expected output directory before running the diff comparison.

Ruling QA Integration Test Mismatch (confidence: high)

• Type: test
• Affected jobs: 93443490969
• Related to change: yes
• Root cause: The new rule S9149 introduced changes to the issues reported during ruling QA, causing a discrepancy between the actual analyzer output and the expected reference results.
• Suggested fix: Review the generated diff report artifact to inspect the ruling differences, and update the expected test resources/baselines if the new rule behavior is correct.

Summary

• Change-related failures: 2 integration test failures resulting from the new rule implementation and missing/mismatched test artifacts.
• Infrastructure/flaky failures: 0 infrastructure or flaky failures.
• Recommended action: Review the integration test output diffs and update the expected baselines for rule S9149, while ensuring that the autoscan test preparation phase executes correctly.

Code Review 👍 Approved with suggestions 1 resolved / 2 findings

Implements rule S9149 to detect static methods in subclasses that hide superclass static methods. Consider removing the redundant hierarchy walk since lookupSymbols already returns inherited members.

💡 Quality: Redundant hierarchy walk: lookupSymbols already returns inherited members

📄 java-checks/src/main/java/org/sonar/java/checks/StaticMethodHidingCheck.java:44-51 📄 java-checks/src/main/java/org/sonar/java/checks/StaticMethodHidingCheck.java:55

Symbol.TypeSymbol.lookupSymbols(name) returns symbols accessible from the type including inherited members (per the API javadoc, contrasted with memberSymbols() which does not). Therefore calling it on the immediate superclass already covers the whole ancestor chain, and the surrounding while (superClass != null) loop that re-invokes checkHiding on every ancestor is redundant work. Consider dropping the loop and calling checkHiding once on owner.superClass(), which also simplifies the code.

lookupSymbols already resolves inherited static methods, so a single check on the direct superclass suffices.

Type superClass = owner.superClass();
if (superClass != null) {
  checkHiding(methodTree, methodSymbol, superClass);
}

✅ 1 resolved

✅ Quality: S9149 not added to Sonar_way profile; rule inactive by default

📄 sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9149.json:10
The rule metadata declares "status": "ready", but S9149 is not present in the Sonar way profile (profiles/Sonar_way). No test enforces this, so it passes CI, but the rule will not be enabled in the default quality profile and thus won't run for users. If the rule is intended to ship active, add S9149 to the Sonar way profile; otherwise this is expected for a draft.

🤖 Prompt for agents

Code Review: Implements rule S9149 to detect static methods in subclasses that hide superclass static methods. Consider removing the redundant hierarchy walk since lookupSymbols already returns inherited members.

1. 💡 Quality: Redundant hierarchy walk: lookupSymbols already returns inherited members
   Files: java-checks/src/main/java/org/sonar/java/checks/StaticMethodHidingCheck.java:44-51, java-checks/src/main/java/org/sonar/java/checks/StaticMethodHidingCheck.java:55

   `Symbol.TypeSymbol.lookupSymbols(name)` returns symbols accessible from the type *including inherited members* (per the API javadoc, contrasted with `memberSymbols()` which does not). Therefore calling it on the immediate superclass already covers the whole ancestor chain, and the surrounding `while (superClass != null)` loop that re-invokes `checkHiding` on every ancestor is redundant work. Consider dropping the loop and calling `checkHiding` once on `owner.superClass()`, which also simplifies the code.

   Fix (lookupSymbols already resolves inherited static methods, so a single check on the direct superclass suffices.):
   Type superClass = owner.superClass();
   if (superClass != null) {
     checkHiding(methodTree, methodSymbol, superClass);
   }

Implementation Status ✅ 1 / 1 issues implemented

✅ SONARJAVA-6741 — 1 / 1 objectives

The PR successfully implements the new rule S9149 ("Static methods should not hide methods from superclasses") along with its test cases, metadata, documentation, and ruling configuration.

✅ 1 complete

• ✅ Implement new rule S9149: Static methods should not hide methods from superclasses

Tip

Comment Gitar fix CI or enable auto-apply: gitar auto-apply:on

Options

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

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}

[sonarqube-next]

Quality Gate failed

Failed conditions
1 New issue
89.7% Coverage on New Code (required ≥ 90%)

See analysis details on SonarQube

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE