6563143: javac should issue a warning for overriding equals without hashCode
Reviewed-by: jjg, mcimadamore
This commit is contained in:
parent
b2a3c762ff
commit
e6b61ae08a
@ -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
|
||||
|
@ -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;
|
||||
|
@ -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);
|
||||
|
@ -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) &&
|
||||
|
@ -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;
|
||||
|
@ -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
|
||||
|
@ -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 @@ public class Dependencies {
|
||||
return a.toString().compareTo(b.toString());
|
||||
}
|
||||
|
||||
public boolean equals(Object obj) {
|
||||
return super.equals(obj);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
|
@ -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;
|
||||
}
|
||||
}
|
||||
|
@ -0,0 +1,2 @@
|
||||
- compiler.warn.override.equals.but.not.hashcode: Other
|
||||
1 warning
|
@ -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
|
||||
|
||||
|
Loading…
Reference in New Issue
Block a user