Skip to content

SONARJAVA-6757: Fix FP in S9149 for intentional hiding with deprecation annotations #5924

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
romainbrenguier wants to merge 2 commits into
base: master
Choose a base branch
from
+71 −32
Open

SONARJAVA-6757: Fix FP in S9149 for intentional hiding with deprecation annotations #5924

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2b85125
SONARJAVA-6757 Fix FP in S9149 for intentional hiding with deprecatio…
romainbrenguier Aug 14, 2026
a76732f
Update ruling results for PR #5924 (#5926)
github-actions[bot] Aug 14, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 0 additions & 31 deletions its/ruling/src/test/resources/guava/java-S9149.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,4 @@
{
"com.google.guava:guava:src/com/google/common/collect/ContiguousSet.java": [
193
],
"com.google.guava:guava:src/com/google/common/collect/ImmutableBiMap.java": [
41,
48,
Expand Down Expand Up @@ -40,43 +37,15 @@
180,
218
],
"com.google.guava:guava:src/com/google/common/collect/ImmutableSortedMapFauxverideShim.java": [
37,
51,
65,
80,
95,
110
],
"com.google.guava:guava:src/com/google/common/collect/ImmutableSortedMultiset.java": [
63,
171,
189
],
"com.google.guava:guava:src/com/google/common/collect/ImmutableSortedMultisetFauxverideShim.java": [
44,
58,
72,
86,
100,
115,
130,
145
],
"com.google.guava:guava:src/com/google/common/collect/ImmutableSortedSet.java": [
78,
200,
237,
256
],
"com.google.guava:guava:src/com/google/common/collect/ImmutableSortedSetFauxverideShim.java": [
46,
60,
74,
88,
103,
118,
133,
147
]
}
65 changes: 65 additions & 0 deletions java-checks-test-sources/default/src/main/java/checks/StaticMethodHidingCheckSample.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package checks;

import com.google.errorprone.annotations.DoNotCall;
import java.util.List;

class StaticMethodHidingCheckSample {
Expand Down Expand Up @@ -223,4 +224,68 @@ static class MultiChild extends MultiParent {
static void second() { // Noncompliant {{Rename this method; it hides "second" in "MultiParent".}}
}
}

// --- Compliant: intentional hiding with @Deprecated annotation ---

static class DeprecatingParent {
static void oldMethod() {
}

static String convert(String input) {
return input;
}
}

static class DeprecatingChild extends DeprecatingParent {
@Deprecated
static void oldMethod() { // Compliant - intentional hiding with @Deprecated
throw new UnsupportedOperationException();
}

@Deprecated
static String convert(String input) { // Compliant - intentional hiding with @Deprecated
throw new UnsupportedOperationException();
}
}

// --- Compliant: intentional hiding with @DoNotCall annotation ---

static class DoNotCallParent {
static void unsafeMethod() {
}
}

static class DoNotCallChild extends DoNotCallParent {
@DoNotCall("Use alternative method")
static void unsafeMethod() { // Compliant - intentional hiding with @DoNotCall
throw new UnsupportedOperationException();
}
}

// --- Compliant: intentional hiding with both @Deprecated and @DoNotCall ---

static class CombinedParent {
static void legacyApi() {
}
}

static class CombinedChild extends CombinedParent {
@Deprecated
@DoNotCall("Use newApi instead")
static void legacyApi() { // Compliant - intentional hiding with @Deprecated and @DoNotCall
throw new UnsupportedOperationException();
}
}

// --- Noncompliant: hiding without any deprecation annotation ---

static class PlainParent {
static void compute() {
}
}

static class PlainChild extends PlainParent {
static void compute() { // Noncompliant {{Rename this method; it hides "compute" in "PlainParent".}}
}
}
}
7 changes: 6 additions & 1 deletion java-checks/src/main/java/org/sonar/java/checks/StaticMethodHidingCheck.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public void visitNode(Tree tree) {
}
MethodTree methodTree = (MethodTree) tree;
Symbol.MethodSymbol methodSymbol = methodTree.symbol();
if (!methodSymbol.isStatic()) {
if (!methodSymbol.isStatic() || isIntentionalHiding(methodSymbol)) {
return;
}
Symbol.TypeSymbol owner = (Symbol.TypeSymbol) methodSymbol.owner();
Expand Down Expand Up @@ -79,6 +79,11 @@ private void reportHidingIssue(MethodTree methodTree, Symbol.MethodSymbol method
}
}

private static boolean isIntentionalHiding(Symbol.MethodSymbol methodSymbol) {
return methodSymbol.metadata().isAnnotatedWith("java.lang.Deprecated")

@nathsou nathsou Aug 14, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

It could be worth considering additional common decorators with similar meaning:

  • com.google.errorprone.annotations.InlineMe
  • org.jetbrains.annotations.ApiStatus.Obsolete
  • org.jetbrains.annotations.ApiStatus.ScheduledForRemoval
  • kotlin.Deprecated

Also, have you considered updating the RSPEC to document these exceptions?

|| methodSymbol.metadata().isAnnotatedWith("com.google.errorprone.annotations.DoNotCall");
}
Comment on lines +82 to +85

@gitar-bot gitar-bot Bot Aug 14, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Edge Case: @deprecated blanket-suppresses S9149, risking false negatives

isIntentionalHiding treats any @deprecated hiding method as intentional. @deprecated is applied for many reasons unrelated to method hiding (e.g. a method scheduled for removal that accidentally hides a parent static method), so this may suppress genuinely accidental hiding that developers still want flagged. This is a reasonable tradeoff to remove the Guava FPs, but consider whether @DonotCall alone (a much more specific signal) would suffice, or document this broadening in the rule metadata so users understand deprecated methods are now exempt.

Was this helpful? React with 👍 / 👎


private static boolean hasSameParameterTypes(Symbol.MethodSymbol method, Symbol.MethodSymbol candidate) {
List<Type> methodParams = method.parameterTypes();
List<Type> candidateParams = candidate.parameterTypes();
Expand Down
Loading