Mercurial > hg > openjdk > jigsaw > langtools
changeset 1727:2620c953e9fe
6563143: javac should issue a warning for overriding equals without hashCode
Reviewed-by: jjg, mcimadamore
author | vromero |
---|---|
date | Mon, 18 Feb 2013 14:33:25 +0000 |
parents | f1f605f85850 |
children | 87884cd0fea3 |
files | src/share/classes/com/sun/tools/javac/code/Flags.java src/share/classes/com/sun/tools/javac/code/Lint.java src/share/classes/com/sun/tools/javac/comp/Attr.java src/share/classes/com/sun/tools/javac/comp/Check.java src/share/classes/com/sun/tools/javac/comp/Resolve.java src/share/classes/com/sun/tools/javac/resources/compiler.properties src/share/classes/com/sun/tools/sjavac/comp/Dependencies.java test/tools/javac/6563143/OverridesEqualsButNotHashCodeTest.java test/tools/javac/6563143/OverridesEqualsButNotHashCodeTest.out test/tools/javac/diags/examples.not-yet.txt |
diffstat | 10 files changed, 71 insertions(+), 8 deletions(-) [+] |
line wrap: on
line diff
--- a/src/share/classes/com/sun/tools/javac/code/Flags.java Fri Feb 15 18:40:38 2013 -0800 +++ b/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/src/share/classes/com/sun/tools/javac/code/Lint.java Fri Feb 15 18:40:38 2013 -0800 +++ b/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/src/share/classes/com/sun/tools/javac/comp/Attr.java Fri Feb 15 18:40:38 2013 -0800 +++ b/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/src/share/classes/com/sun/tools/javac/comp/Check.java Fri Feb 15 18:40:38 2013 -0800 +++ b/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/src/share/classes/com/sun/tools/javac/comp/Resolve.java Fri Feb 15 18:40:38 2013 -0800 +++ b/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/src/share/classes/com/sun/tools/javac/resources/compiler.properties Fri Feb 15 18:40:38 2013 -0800 +++ b/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/src/share/classes/com/sun/tools/sjavac/comp/Dependencies.java Fri Feb 15 18:40:38 2013 -0800 +++ b/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/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/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/test/tools/javac/diags/examples.not-yet.txt Fri Feb 15 18:40:38 2013 -0800 +++ b/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