mirror of
https://github.com/openjdk/jdk.git
synced 2025-09-20 19:14:38 +02:00
8009138: javac, equals-hashCode warning tuning
Reviewed-by: mcimadamore
This commit is contained in:
parent
1823b93919
commit
7fe4d602a6
9 changed files with 99 additions and 50 deletions
|
@ -237,7 +237,7 @@ public abstract class Symbol implements Element {
|
||||||
}
|
}
|
||||||
|
|
||||||
/** Has this symbol an empty name? This includes anonymous
|
/** Has this symbol an empty name? This includes anonymous
|
||||||
* inner classses.
|
* inner classes.
|
||||||
*/
|
*/
|
||||||
public boolean isAnonymous() {
|
public boolean isAnonymous() {
|
||||||
return name.isEmpty();
|
return name.isEmpty();
|
||||||
|
|
|
@ -148,6 +148,7 @@ public class Symtab {
|
||||||
public final Type listType;
|
public final Type listType;
|
||||||
public final Type collectionsType;
|
public final Type collectionsType;
|
||||||
public final Type comparableType;
|
public final Type comparableType;
|
||||||
|
public final Type comparatorType;
|
||||||
public final Type arraysType;
|
public final Type arraysType;
|
||||||
public final Type iterableType;
|
public final Type iterableType;
|
||||||
public final Type iteratorType;
|
public final Type iteratorType;
|
||||||
|
@ -502,6 +503,7 @@ public class Symtab {
|
||||||
listType = enterClass("java.util.List");
|
listType = enterClass("java.util.List");
|
||||||
collectionsType = enterClass("java.util.Collections");
|
collectionsType = enterClass("java.util.Collections");
|
||||||
comparableType = enterClass("java.lang.Comparable");
|
comparableType = enterClass("java.lang.Comparable");
|
||||||
|
comparatorType = enterClass("java.util.Comparator");
|
||||||
arraysType = enterClass("java.util.Arrays");
|
arraysType = enterClass("java.util.Arrays");
|
||||||
iterableType = target.hasIterable()
|
iterableType = target.hasIterable()
|
||||||
? enterClass("java.lang.Iterable")
|
? enterClass("java.lang.Iterable")
|
||||||
|
|
|
@ -4016,7 +4016,7 @@ public class Attr extends JCTree.Visitor {
|
||||||
attribClassBody(env, c);
|
attribClassBody(env, c);
|
||||||
|
|
||||||
chk.checkDeprecatedAnnotation(env.tree.pos(), c);
|
chk.checkDeprecatedAnnotation(env.tree.pos(), c);
|
||||||
chk.checkClassOverrideEqualsAndHash(env.tree.pos(), c);
|
chk.checkClassOverrideEqualsAndHashIfNeeded(env.tree.pos(), c);
|
||||||
} finally {
|
} finally {
|
||||||
env.info.returnResult = prevReturnRes;
|
env.info.returnResult = prevReturnRes;
|
||||||
log.useSource(prev);
|
log.useSource(prev);
|
||||||
|
|
|
@ -1972,14 +1972,32 @@ public class Check {
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
|
|
||||||
public void checkClassOverrideEqualsAndHash(DiagnosticPosition pos,
|
public void checkClassOverrideEqualsAndHashIfNeeded(DiagnosticPosition pos,
|
||||||
|
ClassSymbol someClass) {
|
||||||
|
/* At present, annotations cannot possibly have a method that is override
|
||||||
|
* equivalent with Object.equals(Object) but in any case the condition is
|
||||||
|
* fine for completeness.
|
||||||
|
*/
|
||||||
|
if (someClass == (ClassSymbol)syms.objectType.tsym ||
|
||||||
|
someClass.isInterface() || someClass.isEnum() ||
|
||||||
|
(someClass.flags() & ANNOTATION) != 0 ||
|
||||||
|
(someClass.flags() & ABSTRACT) != 0) return;
|
||||||
|
//anonymous inner classes implementing interfaces need especial treatment
|
||||||
|
if (someClass.isAnonymous()) {
|
||||||
|
List<Type> interfaces = types.interfaces(someClass.type);
|
||||||
|
if (interfaces != null && !interfaces.isEmpty() &&
|
||||||
|
interfaces.head.tsym == syms.comparatorType.tsym) return;
|
||||||
|
}
|
||||||
|
checkClassOverrideEqualsAndHash(pos, someClass);
|
||||||
|
}
|
||||||
|
|
||||||
|
private void checkClassOverrideEqualsAndHash(DiagnosticPosition pos,
|
||||||
ClassSymbol someClass) {
|
ClassSymbol someClass) {
|
||||||
if (lint.isEnabled(LintCategory.OVERRIDES)) {
|
if (lint.isEnabled(LintCategory.OVERRIDES)) {
|
||||||
MethodSymbol equalsAtObject = (MethodSymbol)syms.objectType
|
MethodSymbol equalsAtObject = (MethodSymbol)syms.objectType
|
||||||
.tsym.members().lookup(names.equals).sym;
|
.tsym.members().lookup(names.equals).sym;
|
||||||
MethodSymbol hashCodeAtObject = (MethodSymbol)syms.objectType
|
MethodSymbol hashCodeAtObject = (MethodSymbol)syms.objectType
|
||||||
.tsym.members().lookup(names.hashCode).sym;
|
.tsym.members().lookup(names.hashCode).sym;
|
||||||
|
|
||||||
boolean overridesEquals = types.implementation(equalsAtObject,
|
boolean overridesEquals = types.implementation(equalsAtObject,
|
||||||
someClass, false, equalsHasCodeFilter).owner == someClass;
|
someClass, false, equalsHasCodeFilter).owner == someClass;
|
||||||
boolean overridesHashCode = types.implementation(hashCodeAtObject,
|
boolean overridesHashCode = types.implementation(hashCodeAtObject,
|
||||||
|
@ -1987,7 +2005,7 @@ public class Check {
|
||||||
|
|
||||||
if (overridesEquals && !overridesHashCode) {
|
if (overridesEquals && !overridesHashCode) {
|
||||||
log.warning(LintCategory.OVERRIDES, pos,
|
log.warning(LintCategory.OVERRIDES, pos,
|
||||||
"override.equals.but.not.hashcode", someClass.fullname);
|
"override.equals.but.not.hashcode", someClass);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
@ -2077,7 +2077,7 @@ compiler.warn.override.unchecked.thrown=\
|
||||||
{0}\n\
|
{0}\n\
|
||||||
overridden method does not throw {1}
|
overridden method does not throw {1}
|
||||||
|
|
||||||
# 0: class name
|
# 0: symbol
|
||||||
compiler.warn.override.equals.but.not.hashcode=\
|
compiler.warn.override.equals.but.not.hashcode=\
|
||||||
Class {0} overrides equals, but neither it nor any superclass overrides hashCode method
|
Class {0} overrides equals, but neither it nor any superclass overrides hashCode method
|
||||||
|
|
||||||
|
|
|
@ -0,0 +1,71 @@
|
||||||
|
/*
|
||||||
|
* @test /nodynamiccopyright/
|
||||||
|
* @bug 6563143 8008436 8009138
|
||||||
|
* @summary javac should issue a warning for overriding equals without hashCode
|
||||||
|
* @summary javac should not issue a warning for overriding equals without hasCode
|
||||||
|
* @summary javac, equals-hashCode warning tuning
|
||||||
|
* if hashCode has been overriden by a superclass
|
||||||
|
* @compile/ref=EqualsHashCodeWarningTest.out -Xlint:overrides -XDrawDiagnostics EqualsHashCodeWarningTest.java
|
||||||
|
*/
|
||||||
|
|
||||||
|
import java.util.Comparator;
|
||||||
|
|
||||||
|
public class EqualsHashCodeWarningTest {
|
||||||
|
@Override
|
||||||
|
public boolean equals(Object o) {
|
||||||
|
return o == this;
|
||||||
|
}
|
||||||
|
|
||||||
|
@Override
|
||||||
|
public int hashCode() {
|
||||||
|
return 0;
|
||||||
|
}
|
||||||
|
|
||||||
|
public Comparator m() {
|
||||||
|
return new Comparator() {
|
||||||
|
@Override
|
||||||
|
public boolean equals(Object o) {return true;}
|
||||||
|
|
||||||
|
@Override
|
||||||
|
public int compare(Object o1, Object o2) {
|
||||||
|
return 0;
|
||||||
|
}
|
||||||
|
};
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
class SubClass extends EqualsHashCodeWarningTest {
|
||||||
|
@Override
|
||||||
|
public boolean equals(Object o) {
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
@SuppressWarnings("overrides")
|
||||||
|
class DontWarnMe {
|
||||||
|
@Override
|
||||||
|
public boolean equals(Object o) {
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
class DoWarnMe {
|
||||||
|
@Override
|
||||||
|
public boolean equals(Object o) {
|
||||||
|
return o == this;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
abstract class IamAbstractGetMeOutOfHere {
|
||||||
|
public boolean equals(Object o){return true;}
|
||||||
|
}
|
||||||
|
|
||||||
|
interface I {
|
||||||
|
public boolean equals(Object o);
|
||||||
|
}
|
||||||
|
|
||||||
|
enum E {
|
||||||
|
A, B
|
||||||
|
}
|
||||||
|
|
||||||
|
@interface anno {}
|
|
@ -0,0 +1,2 @@
|
||||||
|
EqualsHashCodeWarningTest.java:52:1: compiler.warn.override.equals.but.not.hashcode: DoWarnMe
|
||||||
|
1 warning
|
|
@ -1,42 +0,0 @@
|
||||||
/*
|
|
||||||
* @test /nodynamiccopyright/
|
|
||||||
* @bug 6563143 8008436
|
|
||||||
* @summary javac should issue a warning for overriding equals without hashCode
|
|
||||||
* @summary javac should not issue a warning for overriding equals without hasCode
|
|
||||||
* if hashCode has been overriden by a superclass
|
|
||||||
* @compile/ref=OverridesEqualsButNotHashCodeTest.out -Xlint:overrides -XDrawDiagnostics OverridesEqualsButNotHashCodeTest.java
|
|
||||||
*/
|
|
||||||
|
|
||||||
public class OverridesEqualsButNotHashCodeTest {
|
|
||||||
@Override
|
|
||||||
public boolean equals(Object o) {
|
|
||||||
return o == this;
|
|
||||||
}
|
|
||||||
|
|
||||||
@Override
|
|
||||||
public int hashCode() {
|
|
||||||
return 0;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
class SubClass extends OverridesEqualsButNotHashCodeTest {
|
|
||||||
@Override
|
|
||||||
public boolean equals(Object o) {
|
|
||||||
return o == this;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
@SuppressWarnings("overrides")
|
|
||||||
class NoWarning {
|
|
||||||
@Override
|
|
||||||
public boolean equals(Object o) {
|
|
||||||
return o == this;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
class DoWarnMe {
|
|
||||||
@Override
|
|
||||||
public boolean equals(Object o) {
|
|
||||||
return o == this;
|
|
||||||
}
|
|
||||||
}
|
|
|
@ -1,2 +0,0 @@
|
||||||
OverridesEqualsButNotHashCodeTest.java:37:1: compiler.warn.override.equals.but.not.hashcode: DoWarnMe
|
|
||||||
1 warning
|
|
Loading…
Add table
Add a link
Reference in a new issue