From fc24f5f8b689ce613cdd0bb2c00e8e50cddc7da5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lars=20K=C3=BChne?= Date: Mon, 21 Oct 2002 18:04:54 +0000 Subject: [PATCH] added check for equals() vs. hashCode, RFE #554373 see also Item 8 of "Bloch, Effective Java" --- .../checks/EqualsHashCodeCheck.java | 144 ++++++++++++++++++ .../checkstyle/checks/messages.properties | 3 +- .../tools/checkstyle/InputSemantic.java | 74 +++++++++ .../checkstyle/EqualsHashCodeCheckTest.java | 27 ++++ 4 files changed, 247 insertions(+), 1 deletion(-) create mode 100644 src/checkstyle/com/puppycrawl/tools/checkstyle/checks/EqualsHashCodeCheck.java create mode 100644 src/tests/com/puppycrawl/tools/checkstyle/EqualsHashCodeCheckTest.java diff --git a/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/EqualsHashCodeCheck.java b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/EqualsHashCodeCheck.java new file mode 100644 index 000000000..dc7bbbe05 --- /dev/null +++ b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/EqualsHashCodeCheck.java @@ -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(). + * + *

+ * 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. + *

+ * @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(); + } +} diff --git a/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/messages.properties b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/messages.properties index 89a5cc75e..7f4cccef6 100644 --- a/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/messages.properties +++ b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/messages.properties @@ -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()''. \ No newline at end of file diff --git a/src/testinputs/com/puppycrawl/tools/checkstyle/InputSemantic.java b/src/testinputs/com/puppycrawl/tools/checkstyle/InputSemantic.java index e1e2472f0..46d7687f8 100644 --- a/src/testinputs/com/puppycrawl/tools/checkstyle/InputSemantic.java +++ b/src/testinputs/com/puppycrawl/tools/checkstyle/InputSemantic.java @@ -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; + } + }; + } } diff --git a/src/tests/com/puppycrawl/tools/checkstyle/EqualsHashCodeCheckTest.java b/src/tests/com/puppycrawl/tools/checkstyle/EqualsHashCodeCheckTest.java new file mode 100644 index 000000000..944137e9f --- /dev/null +++ b/src/tests/com/puppycrawl/tools/checkstyle/EqualsHashCodeCheckTest.java @@ -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); + + } + +}