From 6ebcf2733b6c635d5f044aa35caf2a610c7723ff Mon Sep 17 00:00:00 2001 From: Baratali Izmailov Date: Sun, 12 Jul 2015 06:44:10 -0700 Subject: [PATCH] Issue #1293: Refactoring of EqualsAvoidNullCheck. More UTs. --- pom.xml | 1 - .../checks/coding/EqualsAvoidNullCheck.java | 57 +++++++------------ .../coding/EqualsAvoidNullCheckTest.java | 3 + .../coding/InputEqualsAvoidNull.java | 36 ++++++++++-- 4 files changed, 54 insertions(+), 43 deletions(-) diff --git a/pom.xml b/pom.xml index 1738f9de3..d3deb864a 100644 --- a/pom.xml +++ b/pom.xml @@ -1148,7 +1148,6 @@ .*.checks.coding.CovariantEqualsCheck9596 .*.checks.coding.DeclarationOrderCheck8293 .*.checks.coding.DefaultComesLastCheck87100 - .*.checks.coding.EqualsAvoidNullCheck79100 .*.checks.coding.ExplicitInitializationCheck9197 .*.checks.coding.FallThroughCheck9097 .*.checks.coding.FinalLocalVariableCheck82100 diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/checks/coding/EqualsAvoidNullCheck.java b/src/main/java/com/puppycrawl/tools/checkstyle/checks/coding/EqualsAvoidNullCheck.java index 892ec076b..2f3ec751b 100644 --- a/src/main/java/com/puppycrawl/tools/checkstyle/checks/coding/EqualsAvoidNullCheck.java +++ b/src/main/java/com/puppycrawl/tools/checkstyle/checks/coding/EqualsAvoidNullCheck.java @@ -19,7 +19,6 @@ package com.puppycrawl.tools.checkstyle.checks.coding; -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; @@ -110,7 +109,12 @@ public class EqualsAvoidNullCheck extends Check { @Override public int[] getAcceptableTokens() { - return new int[] {TokenTypes.METHOD_CALL}; + return getDefaultTokens(); + } + + @Override + public int[] getRequiredTokens() { + return getDefaultTokens(); } @Override @@ -130,19 +134,30 @@ public class EqualsAvoidNullCheck extends Check { final DetailAST expr = dot.getNextSibling().getFirstChild(); if ("equals".equals(method.getText()) - && containsOneArg(expr) && containsAllSafeTokens(expr)) { + && containsOneArgument(methodCall) && containsAllSafeTokens(expr)) { log(methodCall.getLineNo(), methodCall.getColumnNo(), MSG_EQUALS_AVOID_NULL); } if (!ignoreEqualsIgnoreCase && "equalsIgnoreCase".equals(method.getText()) - && containsOneArg(expr) && containsAllSafeTokens(expr)) { + && containsOneArgument(methodCall) && containsAllSafeTokens(expr)) { log(methodCall.getLineNo(), methodCall.getColumnNo(), MSG_EQUALS_IGNORE_CASE_AVOID_NULL); } } + /** + * Verify that method call has one argument. + * + * @param methodCall METHOD_CALL DetailAST + * @return true if method call has one argument. + */ + private static boolean containsOneArgument(DetailAST methodCall) { + final DetailAST elist = methodCall.findFirstToken(TokenTypes.ELIST); + return elist.getChildCount() == 1; + } + /** * checks for calling equals on String literal and * anon object which cannot be null @@ -158,40 +173,6 @@ public class EqualsAvoidNullCheck extends Check { || objCalledOn.getType() == TokenTypes.DOT; } - /** - * Checks if a method contains no arguments - * starting at with the argument expression. - * - * @param expr the argument expression - * @return true if the method contains no args, false if not - */ - private boolean containsNoArgs(final AST expr) { - return expr == null; - } - - /** - * Checks if a method contains multiple arguments - * starting at with the argument expression. - * - * @param expr the argument expression - * @return true if the method contains multiple args, false if not - */ - private boolean containsMultiArgs(final AST expr) { - final AST comma = expr.getNextSibling(); - return comma != null && comma.getType() == TokenTypes.COMMA; - } - - /** - * Checks if a method contains a single argument - * starting at with the argument expression. - * - * @param expr the argument expression - * @return true if the method contains a single arg, false if not - */ - private boolean containsOneArg(final AST expr) { - return !containsNoArgs(expr) && !containsMultiArgs(expr); - } - /** *

* Looks for all "safe" Token combinations in the argument diff --git a/src/test/java/com/puppycrawl/tools/checkstyle/checks/coding/EqualsAvoidNullCheckTest.java b/src/test/java/com/puppycrawl/tools/checkstyle/checks/coding/EqualsAvoidNullCheckTest.java index e63e1acbd..0f2bbe310 100644 --- a/src/test/java/com/puppycrawl/tools/checkstyle/checks/coding/EqualsAvoidNullCheckTest.java +++ b/src/test/java/com/puppycrawl/tools/checkstyle/checks/coding/EqualsAvoidNullCheckTest.java @@ -62,6 +62,8 @@ public class EqualsAvoidNullCheckTest extends BaseCheckTestSupport { "75:27: " + getCheckMessage(MSG_EQUALS_IGNORE_CASE_AVOID_NULL), "77:27: " + getCheckMessage(MSG_EQUALS_IGNORE_CASE_AVOID_NULL), "79:27: " + getCheckMessage(MSG_EQUALS_IGNORE_CASE_AVOID_NULL), + "225:24: " + getCheckMessage(MSG_EQUALS_AVOID_NULL), + "227:34: " + getCheckMessage(MSG_EQUALS_IGNORE_CASE_AVOID_NULL), }; verify(checkConfig, getPath("coding" + File.separator + "InputEqualsAvoidNull.java"), expected); } @@ -85,6 +87,7 @@ public class EqualsAvoidNullCheckTest extends BaseCheckTestSupport { "63:17: " + getCheckMessage(MSG_EQUALS_AVOID_NULL), "65:17: " + getCheckMessage(MSG_EQUALS_AVOID_NULL), "67:17: " + getCheckMessage(MSG_EQUALS_AVOID_NULL), + "225:24: " + getCheckMessage(MSG_EQUALS_AVOID_NULL), }; verify(checkConfig, getPath("coding" + File.separator + "InputEqualsAvoidNull.java"), expected); } diff --git a/src/test/resources/com/puppycrawl/tools/checkstyle/coding/InputEqualsAvoidNull.java b/src/test/resources/com/puppycrawl/tools/checkstyle/coding/InputEqualsAvoidNull.java index 3f3cec4e4..2fc959ec9 100644 --- a/src/test/resources/com/puppycrawl/tools/checkstyle/coding/InputEqualsAvoidNull.java +++ b/src/test/resources/com/puppycrawl/tools/checkstyle/coding/InputEqualsAvoidNull.java @@ -188,15 +188,43 @@ public class InputEqualsAvoidNull { new String().equalsIgnoreCase("more cheese"); - } } class InputEqualsAvoidNullOutter { public class InputEqualsAvoidNullInner { - public boolean equals(Object o) { - return true; - } + public boolean equals(Object o) { + return true; + } + } +} + +class MyString { + public boolean equals() { + return true; + } + + public boolean equals(String s1) { + return true; + } + + public boolean equalsIgnoreCase() { + return true; + } + + public boolean equalsIgnoreCase(String s1) { + return true; + } + + private String pizza; + + public void main() { + MyString myString = new MyString(); + myString.equals(); + myString.equals("what"); + myString.equalsIgnoreCase(); + myString.equalsIgnoreCase("what"); + myString.equals(this.pizza = "cold pizza"); } }