From 9f2721bfb12620fded8c1fd4c15bd503ddb193cd Mon Sep 17 00:00:00 2001 From: Vicente Romero Date: Tue, 26 Feb 2013 09:04:19 +0000 Subject: [PATCH] 8008436: javac should not issue a warning for overriding equals without hasCode if hashCode has been overriden by a superclass Reviewed-by: jjg, mcimadamore --- .../com/sun/tools/javac/code/Symbol.java | 2 +- .../com/sun/tools/javac/comp/Attr.java | 2 +- .../com/sun/tools/javac/comp/Check.java | 39 ++++++++++--------- .../tools/javac/resources/compiler.properties | 3 +- .../OverridesEqualsButNotHashCodeTest.java | 26 +++++++++++-- .../OverridesEqualsButNotHashCodeTest.out | 2 +- 6 files changed, 47 insertions(+), 27 deletions(-) diff --git a/langtools/src/share/classes/com/sun/tools/javac/code/Symbol.java b/langtools/src/share/classes/com/sun/tools/javac/code/Symbol.java index db688a52d2c..f9ef7271e28 100644 --- a/langtools/src/share/classes/com/sun/tools/javac/code/Symbol.java +++ b/langtools/src/share/classes/com/sun/tools/javac/code/Symbol.java @@ -1305,7 +1305,7 @@ public abstract class Symbol implements Element { return implementation(origin, types, checkResult, implementation_filter); } // where - private static final Filter implementation_filter = new Filter() { + public static final Filter implementation_filter = new Filter() { public boolean accepts(Symbol s) { return s.kind == Kinds.MTH && (s.flags() & SYNTHETIC) == 0; diff --git a/langtools/src/share/classes/com/sun/tools/javac/comp/Attr.java b/langtools/src/share/classes/com/sun/tools/javac/comp/Attr.java index a50621a0fae..567b31e2a53 100644 --- a/langtools/src/share/classes/com/sun/tools/javac/comp/Attr.java +++ b/langtools/src/share/classes/com/sun/tools/javac/comp/Attr.java @@ -3992,7 +3992,7 @@ public class Attr extends JCTree.Visitor { attribClassBody(env, c); chk.checkDeprecatedAnnotation(env.tree.pos(), c); - chk.checkClassOverrideEqualsAndHash(c); + chk.checkClassOverrideEqualsAndHash(env.tree.pos(), c); } finally { env.info.returnResult = prevReturnRes; log.useSource(prev); diff --git a/langtools/src/share/classes/com/sun/tools/javac/comp/Check.java b/langtools/src/share/classes/com/sun/tools/javac/comp/Check.java index 43d1e6a70ec..602fed28679 100644 --- a/langtools/src/share/classes/com/sun/tools/javac/comp/Check.java +++ b/langtools/src/share/classes/com/sun/tools/javac/comp/Check.java @@ -1964,28 +1964,29 @@ public class Check { } } - public void checkClassOverrideEqualsAndHash(ClassSymbol someClass) { + private Filter equalsHasCodeFilter = new Filter() { + public boolean accepts(Symbol s) { + return MethodSymbol.implementation_filter.accepts(s) && + (s.flags() & BAD_OVERRIDE) == 0; + + } + }; + + public void checkClassOverrideEqualsAndHash(DiagnosticPosition pos, + ClassSymbol someClass) { if (lint.isEnabled(LintCategory.OVERRIDES)) { - boolean hasEquals = false; - boolean hasHashCode = false; + MethodSymbol equalsAtObject = (MethodSymbol)syms.objectType + .tsym.members().lookup(names.equals).sym; + MethodSymbol hashCodeAtObject = (MethodSymbol)syms.objectType + .tsym.members().lookup(names.hashCode).sym; - 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() { - 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); + boolean overridesEquals = types.implementation(equalsAtObject, + someClass, false, equalsHasCodeFilter).owner == someClass; + boolean overridesHashCode = types.implementation(hashCodeAtObject, + someClass, false, equalsHasCodeFilter) != hashCodeAtObject; - hasHashCode |= m.name.equals(names.hashCode) && - m.overrides(hashCodeAtObject.sym, someClass, types, false); - } - if (hasEquals && !hasHashCode) { - log.warning(LintCategory.OVERRIDES, (DiagnosticPosition) null, + if (overridesEquals && !overridesHashCode) { + log.warning(LintCategory.OVERRIDES, pos, "override.equals.but.not.hashcode", someClass.fullname); } } diff --git a/langtools/src/share/classes/com/sun/tools/javac/resources/compiler.properties b/langtools/src/share/classes/com/sun/tools/javac/resources/compiler.properties index 741019ae7a6..af04e264369 100644 --- a/langtools/src/share/classes/com/sun/tools/javac/resources/compiler.properties +++ b/langtools/src/share/classes/com/sun/tools/javac/resources/compiler.properties @@ -2071,8 +2071,7 @@ compiler.warn.override.unchecked.thrown=\ # 0: class name compiler.warn.override.equals.but.not.hashcode=\ - Class {0}\n\ - overrides method equals but does not overrides method hashCode from Object + Class {0} overrides equals, but neither it nor any superclass overrides hashCode method ## The following are all possible strings for the first argument ({0}) of the ## above strings. diff --git a/langtools/test/tools/javac/6563143/OverridesEqualsButNotHashCodeTest.java b/langtools/test/tools/javac/6563143/OverridesEqualsButNotHashCodeTest.java index c586ec4857b..07d92f4ee3a 100644 --- a/langtools/test/tools/javac/6563143/OverridesEqualsButNotHashCodeTest.java +++ b/langtools/test/tools/javac/6563143/OverridesEqualsButNotHashCodeTest.java @@ -1,22 +1,42 @@ /* * @test /nodynamiccopyright/ - * @bug 6563143 + * @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 */ -@SuppressWarnings("overrides") public class OverridesEqualsButNotHashCodeTest { @Override public boolean equals(Object o) { return o == this; } + + @Override + public int hashCode() { + return 0; + } } -class Other { +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; + } +} diff --git a/langtools/test/tools/javac/6563143/OverridesEqualsButNotHashCodeTest.out b/langtools/test/tools/javac/6563143/OverridesEqualsButNotHashCodeTest.out index ad47ad61c97..9d3825a0584 100644 --- a/langtools/test/tools/javac/6563143/OverridesEqualsButNotHashCodeTest.out +++ b/langtools/test/tools/javac/6563143/OverridesEqualsButNotHashCodeTest.out @@ -1,2 +1,2 @@ -- compiler.warn.override.equals.but.not.hashcode: Other +OverridesEqualsButNotHashCodeTest.java:37:1: compiler.warn.override.equals.but.not.hashcode: DoWarnMe 1 warning