6563143: javac should issue a warning for overriding equals without hashCode

Reviewed-by: jjg, mcimadamore

src/share/classes/com/sun/tools/javac/code/Flags.java

@@ -258,7 +258,7 @@ public class Flags {
     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 class Flags {
      */
     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

src/share/classes/com/sun/tools/javac/code/Lint.java

@@ -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;

src/share/classes/com/sun/tools/javac/comp/Attr.java

@@ -3991,6 +3991,7 @@ public class Attr extends JCTree.Visitor {
                 attribClassBody(env, c);
 
                 chk.checkDeprecatedAnnotation(env.tree.pos(), c);
+                chk.checkClassOverrideEqualsAndHash(c);
             } finally {
                 env.info.returnResult = prevReturnRes;
                 log.useSource(prev);

src/share/classes/com/sun/tools/javac/comp/Check.java

@@ -1588,6 +1588,7 @@ public class Check {
                    (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 @@ public class Check {
             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 @@ public class Check {
                       other.flags() == 0 ?
                           Flag.PACKAGE :
                           asFlagSet(other.flags() & AccessFlags));
+            m.flags_field |= BAD_OVERRIDE;
             return;
         }
 
@@ -1642,6 +1645,7 @@ public class Check {
                           "override.incompatible.ret",
                           cannotOverride(m, other),
                           mtres, otres);
+                m.flags_field |= BAD_OVERRIDE;
                 return;
             }
         } else if (overrideWarner.hasNonSilentLint(LintCategory.UNCHECKED)) {
@@ -1661,6 +1665,7 @@ public class Check {
                       "override.meth.doesnt.throw",
                       cannotOverride(m, other),
                       unhandledUnerased.head);
+            m.flags_field |= BAD_OVERRIDE;
             return;
         }
         else if (unhandledUnerased.nonEmpty()) {
@@ -1956,6 +1961,33 @@ public class Check {
         }
     }
 
+    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) &&

src/share/classes/com/sun/tools/javac/comp/Resolve.java

@@ -3606,6 +3606,7 @@ public class Resolve {
          * while inapplicable candidates contain further details about the
          * reason why the method has been considered inapplicable.
          */
+        @SuppressWarnings("overrides")
         class Candidate {
 
             final MethodResolutionPhase step;

src/share/classes/com/sun/tools/javac/resources/compiler.properties

@@ -2065,6 +2065,11 @@ compiler.warn.override.unchecked.thrown=\
     {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

src/share/classes/com/sun/tools/sjavac/comp/Dependencies.java

 /*
- * 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 @@ public class Dependencies {
              return a.toString().compareTo(b.toString());
          }
 
-         public boolean equals(Object obj) {
-             return super.equals(obj);
-         }
     }
 
     /**

test/tools/javac/6563143/OverridesEqualsButNotHashCodeTest.java

+/*
+ * @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;
+    }
+}
+

test/tools/javac/6563143/OverridesEqualsButNotHashCodeTest.out

+- compiler.warn.override.equals.but.not.hashcode: Other
+1 warning

test/tools/javac/diags/examples.not-yet.txt

@@ -110,4 +110,5 @@ compiler.warn.unchecked.cast.to.type                    # DEAD, replaced by comp
 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