Skip to content

Commit cc35ba1

Browse files
klueverError Prone Team
kluever
authored and
Error Prone Team
committed
Discourage overriding Throwable.equals() and hashCode().
PiperOrigin-RevId: 900291065
1 parent ba4ccd1 commit cc35ba1

4 files changed

Lines changed: 232 additions & 0 deletions

File tree

‎core/src/main/java/com/google/errorprone/bugpatterns/ThrowableEqualsHashCode.java‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
/*
2+
* Copyright 2026 The Error Prone Authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package com.google.errorprone.bugpatterns;
18+
19+
import static com.google.errorprone.matchers.Matchers.anyOf;
20+
import static com.google.errorprone.matchers.Matchers.equalsMethodDeclaration;
21+
import static com.google.errorprone.matchers.Matchers.hashCodeMethodDeclaration;
22+
import static com.google.errorprone.util.ASTHelpers.getSymbol;
23+
import static com.google.errorprone.util.ASTHelpers.isSubtype;
24+
25+
import com.google.errorprone.BugPattern;
26+
import com.google.errorprone.BugPattern.SeverityLevel;
27+
import com.google.errorprone.VisitorState;
28+
import com.google.errorprone.bugpatterns.BugChecker.MethodTreeMatcher;
29+
import com.google.errorprone.fixes.SuggestedFix;
30+
import com.google.errorprone.matchers.Description;
31+
import com.google.errorprone.matchers.Matcher;
32+
import com.sun.source.tree.MethodTree;
33+
import com.sun.tools.javac.code.Symbol.ClassSymbol;
34+
35+
/** A {@link BugChecker}; see the associated {@link BugPattern} annotation for details. */
36+
@BugPattern(
37+
summary = "Overriding Throwable.equals() or hashCode() is discouraged.",
38+
severity = SeverityLevel.WARNING)
39+
public final class ThrowableEqualsHashCode extends BugChecker implements MethodTreeMatcher {
40+
41+
private static final Matcher<MethodTree> MATCHER =
42+
anyOf(equalsMethodDeclaration(), hashCodeMethodDeclaration());
43+
44+
@Override
45+
public Description matchMethod(MethodTree tree, VisitorState state) {
46+
if (MATCHER.matches(tree, state)) {
47+
ClassSymbol classSymbol = getSymbol(tree).enclClass();
48+
if (classSymbol != null
49+
&& isSubtype(classSymbol.type, state.getSymtab().throwableType, state)) {
50+
return describeMatch(tree, SuggestedFix.delete(tree));
51+
}
52+
}
53+
return Description.NO_MATCH;
54+
}
55+
}

‎core/src/main/java/com/google/errorprone/scanner/BuiltInCheckerSuppliers.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -408,6 +408,7 @@
408408
import com.google.errorprone.bugpatterns.ThrowIfUncheckedKnownUnchecked;
409409
import com.google.errorprone.bugpatterns.ThrowNull;
410410
import com.google.errorprone.bugpatterns.ThrowSpecificExceptions;
411+
import com.google.errorprone.bugpatterns.ThrowableEqualsHashCode;
411412
import com.google.errorprone.bugpatterns.ThrowsUncheckedException;
412413
import com.google.errorprone.bugpatterns.ToStringReturnsNull;
413414
import com.google.errorprone.bugpatterns.TooManyParameters;
@@ -1164,6 +1165,7 @@ public static ScannerSupplier warningChecks() {
11641165
ThreadPriorityCheck.class,
11651166
ThreeLetterTimeZoneID.class,
11661167
ThrowIfUncheckedKnownUnchecked.class,
1168+
ThrowableEqualsHashCode.class,
11671169
TimeInStaticInitializer.class,
11681170
TimeUnitConversionChecker.class,
11691171
ToStringReturnsNull.class,

‎core/src/test/java/com/google/errorprone/bugpatterns/ThrowableEqualsHashCodeTest.java‎

Lines changed: 142 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,142 @@
1+
/*
2+
* Copyright 2026 The Error Prone Authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package com.google.errorprone.bugpatterns;
18+
19+
import com.google.errorprone.BugCheckerRefactoringTestHelper;
20+
import org.junit.Test;
21+
import org.junit.runner.RunWith;
22+
import org.junit.runners.JUnit4;
23+
24+
/** Tests for {@link ThrowableEqualsHashCode}. */
25+
@RunWith(JUnit4.class)
26+
public final class ThrowableEqualsHashCodeTest {
27+
28+
private final BugCheckerRefactoringTestHelper refactoringHelper =
29+
BugCheckerRefactoringTestHelper.newInstance(ThrowableEqualsHashCode.class, getClass());
30+
31+
@Test
32+
public void positive() {
33+
refactoringHelper
34+
.addInputLines(
35+
"TestException.java",
36+
"""
37+
public class TestException extends Exception {
38+
@Override
39+
public boolean equals(Object obj) {
40+
return false;
41+
}
42+
43+
@Override
44+
public int hashCode() {
45+
return 42;
46+
}
47+
}
48+
""")
49+
.addOutputLines(
50+
"TestException.java",
51+
"""
52+
public class TestException extends Exception {
53+
}
54+
""")
55+
.doTest();
56+
}
57+
58+
@Test
59+
public void positiveEquals() {
60+
refactoringHelper
61+
.addInputLines(
62+
"TestException.java",
63+
"""
64+
public class TestException extends Exception {
65+
@Override
66+
public boolean equals(Object obj) {
67+
return false;
68+
}
69+
}
70+
""")
71+
.addOutputLines(
72+
"TestException.java",
73+
"""
74+
public class TestException extends Exception {
75+
}
76+
""")
77+
.doTest();
78+
}
79+
80+
@Test
81+
public void positiveHashCode() {
82+
refactoringHelper
83+
.addInputLines(
84+
"TestException.java",
85+
"""
86+
public class TestException extends Exception {
87+
@Override
88+
public int hashCode() {
89+
return 42;
90+
}
91+
}
92+
""")
93+
.addOutputLines(
94+
"TestException.java",
95+
"""
96+
public class TestException extends Exception {
97+
}
98+
""")
99+
.doTest();
100+
}
101+
102+
@Test
103+
public void negativeIntegerWrapper() {
104+
refactoringHelper
105+
.addInputLines(
106+
"IntegerWrapper.java",
107+
"""
108+
public final class IntegerWrapper {
109+
private final int value;
110+
111+
public IntegerWrapper(int value) {
112+
this.value = value;
113+
}
114+
115+
@Override
116+
public boolean equals(Object object) {
117+
return object instanceof IntegerWrapper that && this.value == that.value;
118+
}
119+
120+
@Override
121+
public int hashCode() {
122+
return value;
123+
}
124+
}
125+
""")
126+
.expectUnchanged()
127+
.doTest();
128+
}
129+
130+
@Test
131+
public void negativeThrowableNoOverrides() {
132+
refactoringHelper
133+
.addInputLines(
134+
"TestException.java",
135+
"""
136+
public class TestException extends Exception {
137+
}
138+
""")
139+
.expectUnchanged()
140+
.doTest();
141+
}
142+
}

‎docs/bugpattern/ThrowableEqualsHashCode.md‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
Throwables should not override `equals()` or `hashCode()`.
2+
3+
1. **Exceptions are Events, Not Value Objects** Philosophically, an exception
4+
represents a unique historical event: something went wrong at a specific
5+
time, in a specific thread, at a specific line of code. It is not a data
6+
container or a value type (like a `String` or a `Money` object). Even if two
7+
`IllegalArgumentException`s are thrown with the exact same message (`"ID
8+
cannot be null"`), and perhaps even identical stack traces, they represent
9+
two distinct failures that happened independently. Treating them as "equal"
10+
conceptually conflates two different events.
11+
12+
2. **The Stack Trace Problem** When an exception is instantiated (or thrown),
13+
Java populates its stack trace via `fillInStackTrace()`.
14+
15+
* Complexity: Are you going to include the stack trace in your `equals()`
16+
comparison? If you do, comparing arrays of `StackTraceElement` is
17+
computationally expensive. Exceptions can also have causes and
18+
suppressed exceptions. This adds expense, too. Also, will all the
19+
transitive causes and suppressed exceptions themselves implement
20+
`equals()` the way you want? Plus, causes and suppressed exceptions make
21+
exceptions mutable, and mutable objects generally shouldn't implement
22+
`equals()`.
23+
* Brittleness: If you don't include the stack trace, you are saying that
24+
an exception thrown in `ServiceA` is equal to an exception thrown in
25+
`ServiceB` just because they share a message or an error code. This
26+
masks critical debugging context.
27+
28+
3. **It Hides Bad Architecture** The primary reason you would need to override
29+
these methods is if you are placing Exceptions into a `HashSet`, or using
30+
them as keys in a `HashMap`. If you are doing this, you are likely using
31+
Exceptions for normal business logic or control flow, which is a known
32+
anti-pattern. Exceptions are for exceptional circumstances; they are heavy
33+
(because of the stack trace) and slow to generate.

0 commit comments

Comments
 (0)