From 03914ce3a0cb0a8bafdb7c8f47f72dd064a0c5a2 Mon Sep 17 00:00:00 2001 From: ychulovskyy Date: Wed, 25 Feb 2015 21:37:24 +0100 Subject: [PATCH] Issue #572 BooleanExpressionComplexity misidentifies integer expression as boolean expression --- .../BooleanExpressionComplexityCheck.java | 24 ++++++++++-- .../BooleanExpressionComplexityCheckTest.java | 2 + ...eanExpressionComplexityCheckTestInput.java | 37 +++++++++++++++++++ src/xdocs/config_metrics.xml | 8 +++- 4 files changed, 67 insertions(+), 4 deletions(-) diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/checks/metrics/BooleanExpressionComplexityCheck.java b/src/main/java/com/puppycrawl/tools/checkstyle/checks/metrics/BooleanExpressionComplexityCheck.java index 16b41658d..d97891498 100644 --- a/src/main/java/com/puppycrawl/tools/checkstyle/checks/metrics/BooleanExpressionComplexityCheck.java +++ b/src/main/java/com/puppycrawl/tools/checkstyle/checks/metrics/BooleanExpressionComplexityCheck.java @@ -27,6 +27,9 @@ import com.puppycrawl.tools.checkstyle.checks.CheckUtils; /** * Restricts nested boolean operators (&&, ||, &, | and ^) to * a specified depth (default = 3). + * Note: &, | and ^ are not checked if they are part of constructor or + * method call because they can be applied to non boolean variables and + * Checkstyle does not know types of methods from different classes. * * @author Simon Harris * @author o_sukhodolsky @@ -126,14 +129,18 @@ public final class BooleanExpressionComplexityCheck extends Check visitExpr(); break; case TokenTypes.BOR: - if (!isPipeOperator(ast)) { + if (!isPipeOperator(ast) && !isPassedInParameter(ast)) { + context.visitBooleanOperator(); + } + break; + case TokenTypes.BAND: + case TokenTypes.BXOR: + if (!isPassedInParameter(ast)) { context.visitBooleanOperator(); } break; case TokenTypes.LAND: - case TokenTypes.BAND: case TokenTypes.LOR: - case TokenTypes.BXOR: context.visitBooleanOperator(); break; default: @@ -141,6 +148,17 @@ public final class BooleanExpressionComplexityCheck extends Check } } + /** + * Checks if logical operator is part of constructor or method call. + * @param logicalOperator logical operator + * @return true if logical operator is part of constructor or method call + */ + private boolean isPassedInParameter(DetailAST logicalOperator) + { + return logicalOperator.getParent().getType() == TokenTypes.EXPR + && logicalOperator.getParent().getParent().getType() == TokenTypes.ELIST; + } + /** * Checks if {@link TokenTypes#BOR binary OR} is applied to exceptions * in diff --git a/src/test/java/com/puppycrawl/tools/checkstyle/checks/metrics/BooleanExpressionComplexityCheckTest.java b/src/test/java/com/puppycrawl/tools/checkstyle/checks/metrics/BooleanExpressionComplexityCheckTest.java index 0749b910d..338fa08f8 100644 --- a/src/test/java/com/puppycrawl/tools/checkstyle/checks/metrics/BooleanExpressionComplexityCheckTest.java +++ b/src/test/java/com/puppycrawl/tools/checkstyle/checks/metrics/BooleanExpressionComplexityCheckTest.java @@ -37,6 +37,8 @@ public class BooleanExpressionComplexityCheckTest extends BaseCheckTestSupport String[] expected = { "13:9: " + getCheckMessage(MSG_KEY, 4, 3), "32:9: " + getCheckMessage(MSG_KEY, 6, 3), + "38:34: " + getCheckMessage(MSG_KEY, 4, 3), + "40:34: " + getCheckMessage(MSG_KEY, 4, 3), }; verify(checkConfig, getPath("metrics" + File.separator + "BooleanExpressionComplexityCheckTestInput.java"), expected); diff --git a/src/test/resources/com/puppycrawl/tools/checkstyle/metrics/BooleanExpressionComplexityCheckTestInput.java b/src/test/resources/com/puppycrawl/tools/checkstyle/metrics/BooleanExpressionComplexityCheckTestInput.java index faa981e88..4aaab6dc9 100644 --- a/src/test/resources/com/puppycrawl/tools/checkstyle/metrics/BooleanExpressionComplexityCheckTestInput.java +++ b/src/test/resources/com/puppycrawl/tools/checkstyle/metrics/BooleanExpressionComplexityCheckTestInput.java @@ -31,4 +31,41 @@ public class BooleanExpressionComplexityCheckTestInput { { return (((_a & (_b & _c)) | (_c ^ _d) | (_a & _d))); } + + public void notIgnoredMethodParameters() + { + new Settings(Settings.FALSE && Settings.FALSE && Settings.FALSE + && Settings.TRUE && Settings.TRUE); + new Settings(Settings.FALSE || Settings.FALSE || Settings.FALSE + || Settings.TRUE || Settings.TRUE); + } + + public void ignoredMethodParameters() + { + new Settings(Settings.RESIZABLE | Settings.SCROLLBARS | Settings.LOCATION_BAR + | Settings.MENU_BAR | Settings.TOOL_BAR); + new Settings(Settings.RESIZABLE & Settings.SCROLLBARS & Settings.LOCATION_BAR + & Settings.MENU_BAR & Settings.TOOL_BAR); + new Settings(Settings.RESIZABLE ^ Settings.SCROLLBARS ^ Settings.LOCATION_BAR + ^ Settings.MENU_BAR ^ Settings.TOOL_BAR); + } + + private class Settings { + public final static int RESIZABLE = 1; + public final static int SCROLLBARS = 2; + public final static int LOCATION_BAR = 3; + public final static int MENU_BAR = 4; + public final static int TOOL_BAR = 5; + + public final static boolean TRUE = true; + public final static boolean FALSE = false; + + public Settings(int flag) + { + } + + public Settings(boolean flag) + { + } + } } diff --git a/src/xdocs/config_metrics.xml b/src/xdocs/config_metrics.xml index 0da22fe85..079ef39d9 100644 --- a/src/xdocs/config_metrics.xml +++ b/src/xdocs/config_metrics.xml @@ -28,9 +28,15 @@ Note that the operators & and | are not only integer bitwise operators, they are also the - non-shortcut versions of the boolean operators + non-shortcut versions of the boolean operators. && and ||.

+

+ Note that &, | and ^ are not checked + if they are part of constructor or method call + because they can be applied to non boolean variables and + Checkstyle does not know types of methods from different classes. +