added check for equals() vs. hashCode, RFE #554373
see also Item 8 of "Bloch, Effective Java"
This commit is contained in:
parent
dcca9953bc
commit
fc24f5f8b6
|
|
@ -0,0 +1,144 @@
|
|||
////////////////////////////////////////////////////////////////////////////////
|
||||
// checkstyle: Checks Java source code for adherence to a set of rules.
|
||||
// Copyright (C) 2001-2002 Oliver Burn
|
||||
//
|
||||
// This library is free software; you can redistribute it and/or
|
||||
// modify it under the terms of the GNU Lesser General Public
|
||||
// License as published by the Free Software Foundation; either
|
||||
// version 2.1 of the License, or (at your option) any later version.
|
||||
//
|
||||
// This library is distributed in the hope that it will be useful,
|
||||
// but WITHOUT ANY WARRANTY; without even the implied warranty of
|
||||
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
|
||||
// Lesser General Public License for more details.
|
||||
//
|
||||
// You should have received a copy of the GNU Lesser General Public
|
||||
// License along with this library; if not, write to the Free Software
|
||||
// Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
|
||||
////////////////////////////////////////////////////////////////////////////////
|
||||
|
||||
package com.puppycrawl.tools.checkstyle.checks;
|
||||
|
||||
import java.util.HashMap;
|
||||
import java.util.HashSet;
|
||||
import java.util.Iterator;
|
||||
import java.util.Map;
|
||||
import java.util.Set;
|
||||
|
||||
import antlr.collections.AST;
|
||||
import com.puppycrawl.tools.checkstyle.api.Check;
|
||||
import com.puppycrawl.tools.checkstyle.api.DetailAST;
|
||||
import com.puppycrawl.tools.checkstyle.api.TokenTypes;
|
||||
|
||||
/**
|
||||
* Checks that classes that override equals() also override hashCode().
|
||||
*
|
||||
* <p>
|
||||
* Rationale: The contract of equals() and hashCode() requires that
|
||||
* equal objects have the same hashCode. Hence, whenever you override
|
||||
* equals() you must override hashCode() to ensure that your class can
|
||||
* be used in collections that are hash based.
|
||||
* </p>
|
||||
* @author lkuehne
|
||||
*/
|
||||
public class EqualsHashCodeCheck
|
||||
extends Check
|
||||
{
|
||||
// implementation note: we have to use the following members to
|
||||
// keep track of definitions in different inner classes
|
||||
|
||||
/** maps OBJ_BLOCK to the method definition of equals() */
|
||||
private final Map objBlockEquals = new HashMap();
|
||||
|
||||
/** maps OBJ_BLOCK to the method definition of equals() */
|
||||
private final Set objBlockWithHashCode = new HashSet();
|
||||
|
||||
|
||||
public int[] getDefaultTokens()
|
||||
{
|
||||
return new int[] {TokenTypes.METHOD_DEF};
|
||||
}
|
||||
|
||||
public void beginTree()
|
||||
{
|
||||
objBlockEquals.clear();
|
||||
objBlockWithHashCode.clear();
|
||||
}
|
||||
|
||||
public void visitToken(DetailAST aAST)
|
||||
{
|
||||
// paranoia
|
||||
if (aAST.getType() != TokenTypes.METHOD_DEF) {
|
||||
return;
|
||||
}
|
||||
|
||||
AST modifiers = aAST.getFirstChild();
|
||||
final Set mods = new HashSet();
|
||||
AST modifier = modifiers.getFirstChild();
|
||||
while (modifier != null) {
|
||||
mods.add(modifier.getText());
|
||||
modifier = modifier.getNextSibling();
|
||||
}
|
||||
|
||||
AST type = modifiers.getNextSibling();
|
||||
AST methodName = type.getNextSibling();
|
||||
DetailAST parameters = (DetailAST) methodName.getNextSibling();
|
||||
|
||||
if (type.getFirstChild().getType() == TokenTypes.LITERAL_BOOLEAN
|
||||
&& "equals".equals(methodName.getText())
|
||||
&& mods.contains("public")
|
||||
&& parameters.getChildCount() == 1
|
||||
&& isObjectParam(parameters.getFirstChild())
|
||||
)
|
||||
{
|
||||
objBlockEquals.put(aAST.getParent(), aAST);
|
||||
}
|
||||
else if (type.getFirstChild().getType() == TokenTypes.LITERAL_INT
|
||||
&& "hashCode".equals(methodName.getText())
|
||||
// && modifiers.subTreeContains(TokenTypes.LITERAL_PUBLIC)
|
||||
&& parameters.getFirstChild() == null) // no params
|
||||
{
|
||||
objBlockWithHashCode.add(aAST.getParent());
|
||||
}
|
||||
}
|
||||
|
||||
private boolean isObjectParam(AST aFirstChild)
|
||||
{
|
||||
if (aFirstChild.getType() != TokenTypes.PARAMETER_DEF) {
|
||||
return false;
|
||||
}
|
||||
|
||||
final AST modifiers = aFirstChild.getFirstChild();
|
||||
AST type = modifiers.getNextSibling();
|
||||
switch (type.getFirstChild().getType())
|
||||
{
|
||||
case TokenTypes.LITERAL_BOOLEAN:
|
||||
case TokenTypes.LITERAL_BYTE:
|
||||
case TokenTypes.LITERAL_CHAR:
|
||||
case TokenTypes.LITERAL_DOUBLE:
|
||||
case TokenTypes.LITERAL_FLOAT:
|
||||
case TokenTypes.LITERAL_INT:
|
||||
case TokenTypes.LITERAL_LONG:
|
||||
case TokenTypes.LITERAL_SHORT:
|
||||
return false;
|
||||
default:
|
||||
return true;
|
||||
}
|
||||
}
|
||||
|
||||
public void finishTree()
|
||||
{
|
||||
Set equalsDefs = objBlockEquals.keySet();
|
||||
for (Iterator it = equalsDefs.iterator(); it.hasNext();) {
|
||||
Object objBlock = it.next();
|
||||
if (!objBlockWithHashCode.contains(objBlock)) {
|
||||
DetailAST equalsAST = (DetailAST) objBlockEquals.get(objBlock);
|
||||
log(equalsAST.getLineNo(), equalsAST.getColumnNo(),
|
||||
"equals.noHashCode");
|
||||
}
|
||||
}
|
||||
|
||||
objBlockEquals.clear();
|
||||
objBlockWithHashCode.clear();
|
||||
}
|
||||
}
|
||||
|
|
@ -61,4 +61,5 @@ maxParam=More than {0,number,integer} parameters.
|
|||
field.unused=''{0}'' is an unused field.
|
||||
|
||||
assignment.inner.avoid=Avoid inner assignments.
|
||||
|
||||
illegal.regexp=Line matches the illegal pattern ''{0}''.
|
||||
equals.noHashCode=Definition of ''equals()'' without corresponding defnition of ''hashCode()''.
|
||||
|
|
@ -91,4 +91,78 @@ class InputSemantic
|
|||
|
||||
/** test **/
|
||||
private static final long IGNORE = 666l + 666L;
|
||||
|
||||
void innerAssignments()
|
||||
{
|
||||
int a;
|
||||
int b;
|
||||
int c;
|
||||
|
||||
a = b = c = 1; // flag two inner assignments
|
||||
|
||||
String s = Integer.toString(b = 2); // flag inner assignment
|
||||
|
||||
Integer i = new Integer(a += 5); // flag inner assigment
|
||||
|
||||
c = b++; // common practice, don't flag
|
||||
// even though technically an assigment to b
|
||||
|
||||
for (int j = 0; j < 6; j += 2) { // common practice, don't flag
|
||||
a += j;
|
||||
}
|
||||
}
|
||||
|
||||
public class EqualsVsHashCode1
|
||||
{
|
||||
public boolean equals(int a) // wrong arg type, don't flag
|
||||
{
|
||||
return a == 1;
|
||||
}
|
||||
}
|
||||
|
||||
public class EqualsVsHashCode2
|
||||
{
|
||||
public boolean equals(String a) // flag
|
||||
{
|
||||
return true;
|
||||
}
|
||||
}
|
||||
|
||||
public class EqualsVsHashCode3
|
||||
{
|
||||
public boolean equals(Object a) // don't flag
|
||||
{
|
||||
return true;
|
||||
}
|
||||
|
||||
public int hashCode()
|
||||
{
|
||||
return 0;
|
||||
}
|
||||
}
|
||||
|
||||
public class EqualsVsHashCode4
|
||||
{
|
||||
// in anon inner class
|
||||
ByteArrayOutputStream bos1 = new ByteArrayOutputStream()
|
||||
{
|
||||
public boolean equals(Object a) // don't flag
|
||||
{
|
||||
return true;
|
||||
}
|
||||
|
||||
public int hashCode()
|
||||
{
|
||||
return 0;
|
||||
}
|
||||
};
|
||||
|
||||
ByteArrayOutputStream bos2 = new ByteArrayOutputStream()
|
||||
{
|
||||
public boolean equals(Object a) // flag
|
||||
{
|
||||
return true;
|
||||
}
|
||||
};
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -0,0 +1,27 @@
|
|||
package com.puppycrawl.tools.checkstyle;
|
||||
|
||||
import com.puppycrawl.tools.checkstyle.checks.EqualsHashCodeCheck;
|
||||
|
||||
public class EqualsHashCodeCheckTest
|
||||
extends BaseCheckTestCase
|
||||
{
|
||||
public EqualsHashCodeCheckTest(String aName)
|
||||
{
|
||||
super(aName);
|
||||
}
|
||||
|
||||
public void testIt() throws Exception
|
||||
{
|
||||
final CheckConfiguration checkConfig = new CheckConfiguration();
|
||||
checkConfig.setClassname(EqualsHashCodeCheck.class.getName());
|
||||
final Checker c = createChecker(checkConfig);
|
||||
final String fname = getPath("InputSemantic.java");
|
||||
final String[] expected = {
|
||||
"125:9: Definition of 'equals()' without corresponding defnition of 'hashCode()'.",
|
||||
"162:13: Definition of 'equals()' without corresponding defnition of 'hashCode()'.",
|
||||
};
|
||||
verify(c, fname, expected);
|
||||
|
||||
}
|
||||
|
||||
}
|
||||
Loading…
Reference in New Issue