6563143: javac should issue a warning for overriding equals without hashCode
Reviewed-by: jjg, mcimadamore
--- a/langtools/src/share/classes/com/sun/tools/javac/code/Flags.java Fri Feb 15 18:40:38 2013 -0800
+++ b/langtools/src/share/classes/com/sun/tools/javac/code/Flags.java Mon Feb 18 14:33:25 2013 +0000
@@ -258,7 +258,7 @@
public static final long CLASH = 1L<<42;
/**
- * Flag that marks either a default method or an interface containing default methods
+ * Flag that marks either a default method or an interface containing default methods.
*/
public static final long DEFAULT = 1L<<43;
@@ -268,6 +268,11 @@
*/
public static final long AUXILIARY = 1L<<44;
+ /**
+ * Flag that indicates that an override error has been detected by Check.
+ */
+ public static final long BAD_OVERRIDE = 1L<<45;
+
/** Modifier masks.
*/
public static final int
--- a/langtools/src/share/classes/com/sun/tools/javac/code/Lint.java Fri Feb 15 18:40:38 2013 -0800
+++ b/langtools/src/share/classes/com/sun/tools/javac/code/Lint.java Mon Feb 18 14:33:25 2013 +0000
@@ -26,10 +26,7 @@
package com.sun.tools.javac.code;
import java.util.EnumSet;
-import java.util.HashMap;
import java.util.Map;
-import java.util.Set;
-import javax.lang.model.element.Modifier;
import com.sun.tools.javac.code.Symbol.*;
import com.sun.tools.javac.util.Context;
import com.sun.tools.javac.util.List;
--- a/langtools/src/share/classes/com/sun/tools/javac/comp/Attr.java Fri Feb 15 18:40:38 2013 -0800
+++ b/langtools/src/share/classes/com/sun/tools/javac/comp/Attr.java Mon Feb 18 14:33:25 2013 +0000
@@ -3991,6 +3991,7 @@
attribClassBody(env, c);
chk.checkDeprecatedAnnotation(env.tree.pos(), c);
+ chk.checkClassOverrideEqualsAndHash(c);
} finally {
env.info.returnResult = prevReturnRes;
log.useSource(prev);
--- a/langtools/src/share/classes/com/sun/tools/javac/comp/Check.java Fri Feb 15 18:40:38 2013 -0800
+++ b/langtools/src/share/classes/com/sun/tools/javac/comp/Check.java Mon Feb 18 14:33:25 2013 +0000
@@ -1588,6 +1588,7 @@
(other.flags() & STATIC) == 0) {
log.error(TreeInfo.diagnosticPositionFor(m, tree), "override.static",
cannotOverride(m, other));
+ m.flags_field |= BAD_OVERRIDE;
return;
}
@@ -1599,6 +1600,7 @@
log.error(TreeInfo.diagnosticPositionFor(m, tree), "override.meth",
cannotOverride(m, other),
asFlagSet(other.flags() & (FINAL | STATIC)));
+ m.flags_field |= BAD_OVERRIDE;
return;
}
@@ -1615,6 +1617,7 @@
other.flags() == 0 ?
Flag.PACKAGE :
asFlagSet(other.flags() & AccessFlags));
+ m.flags_field |= BAD_OVERRIDE;
return;
}
@@ -1642,6 +1645,7 @@
"override.incompatible.ret",
cannotOverride(m, other),
mtres, otres);
+ m.flags_field |= BAD_OVERRIDE;
return;
}
} else if (overrideWarner.hasNonSilentLint(LintCategory.UNCHECKED)) {
@@ -1661,6 +1665,7 @@
"override.meth.doesnt.throw",
cannotOverride(m, other),
unhandledUnerased.head);
+ m.flags_field |= BAD_OVERRIDE;
return;
}
else if (unhandledUnerased.nonEmpty()) {
@@ -1956,6 +1961,33 @@
}
}
+ public void checkClassOverrideEqualsAndHash(ClassSymbol someClass) {
+ if (lint.isEnabled(LintCategory.OVERRIDES)) {
+ boolean hasEquals = false;
+ boolean hasHashCode = false;
+
+ Scope.Entry equalsAtObject = syms.objectType.tsym.members().lookup(names.equals);
+ Scope.Entry hashCodeAtObject = syms.objectType.tsym.members().lookup(names.hashCode);
+ for (Symbol s: someClass.members().getElements(new Filter<Symbol>() {
+ public boolean accepts(Symbol s) {
+ return s.kind == Kinds.MTH &&
+ (s.flags() & BAD_OVERRIDE) == 0;
+ }
+ })) {
+ MethodSymbol m = (MethodSymbol)s;
+ hasEquals |= m.name.equals(names.equals) &&
+ m.overrides(equalsAtObject.sym, someClass, types, false);
+
+ hasHashCode |= m.name.equals(names.hashCode) &&
+ m.overrides(hashCodeAtObject.sym, someClass, types, false);
+ }
+ if (hasEquals && !hasHashCode) {
+ log.warning(LintCategory.OVERRIDES, (DiagnosticPosition) null,
+ "override.equals.but.not.hashcode", someClass.fullname);
+ }
+ }
+ }
+
private boolean checkNameClash(ClassSymbol origin, Symbol s1, Symbol s2) {
ClashFilter cf = new ClashFilter(origin.type);
return (cf.accepts(s1) &&
--- a/langtools/src/share/classes/com/sun/tools/javac/comp/Resolve.java Fri Feb 15 18:40:38 2013 -0800
+++ b/langtools/src/share/classes/com/sun/tools/javac/comp/Resolve.java Mon Feb 18 14:33:25 2013 +0000
@@ -3606,6 +3606,7 @@
* while inapplicable candidates contain further details about the
* reason why the method has been considered inapplicable.
*/
+ @SuppressWarnings("overrides")
class Candidate {
final MethodResolutionPhase step;
--- a/langtools/src/share/classes/com/sun/tools/javac/resources/compiler.properties Fri Feb 15 18:40:38 2013 -0800
+++ b/langtools/src/share/classes/com/sun/tools/javac/resources/compiler.properties Mon Feb 18 14:33:25 2013 +0000
@@ -2065,6 +2065,11 @@
{0}\n\
overridden method does not throw {1}
+# 0: class name
+compiler.warn.override.equals.but.not.hashcode=\
+ Class {0}\n\
+ overrides method equals but does not overrides method hashCode from Object
+
## The following are all possible strings for the first argument ({0}) of the
## above strings.
# 0: symbol, 1: symbol, 2: symbol, 3: symbol
--- a/langtools/src/share/classes/com/sun/tools/sjavac/comp/Dependencies.java Fri Feb 15 18:40:38 2013 -0800
+++ b/langtools/src/share/classes/com/sun/tools/sjavac/comp/Dependencies.java Mon Feb 18 14:33:25 2013 +0000
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 1999, 2011, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1999, 2013, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -113,9 +113,6 @@
return a.toString().compareTo(b.toString());
}
- public boolean equals(Object obj) {
- return super.equals(obj);
- }
}
/**
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/langtools/test/tools/javac/6563143/OverridesEqualsButNotHashCodeTest.java Mon Feb 18 14:33:25 2013 +0000
@@ -0,0 +1,22 @@
+/*
+ * @test /nodynamiccopyright/
+ * @bug 6563143
+ * @summary javac should issue a warning for overriding equals without hashCode
+ * @compile/ref=OverridesEqualsButNotHashCodeTest.out -Xlint:overrides -XDrawDiagnostics OverridesEqualsButNotHashCodeTest.java
+ */
+
+@SuppressWarnings("overrides")
+public class OverridesEqualsButNotHashCodeTest {
+ @Override
+ public boolean equals(Object o) {
+ return o == this;
+ }
+}
+
+class Other {
+ @Override
+ public boolean equals(Object o) {
+ return o == this;
+ }
+}
+
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/langtools/test/tools/javac/6563143/OverridesEqualsButNotHashCodeTest.out Mon Feb 18 14:33:25 2013 +0000
@@ -0,0 +1,2 @@
+- compiler.warn.override.equals.but.not.hashcode: Other
+1 warning
--- a/langtools/test/tools/javac/diags/examples.not-yet.txt Fri Feb 15 18:40:38 2013 -0800
+++ b/langtools/test/tools/javac/diags/examples.not-yet.txt Mon Feb 18 14:33:25 2013 +0000
@@ -110,4 +110,5 @@
compiler.warn.unexpected.archive.file # Paths: zip file with unknown extn
compiler.warn.unknown.enum.constant # in bad class file
compiler.warn.unknown.enum.constant.reason # in bad class file
+compiler.warn.override.equals.but.not.hashcode # when a class overrides equals but not hashCode method from Object