diff --git a/pmd/etc/changelog.txt b/pmd/etc/changelog.txt index 1c658d1543..c9807e192e 100644 --- a/pmd/etc/changelog.txt +++ b/pmd/etc/changelog.txt @@ -469,21 +469,23 @@ AvoidUsingHardCodedIP modified to not use InetAddress.getByName(String), instead New Java rules: - Basic ruleset: EmptyInitializer,EmptyStatementBlock,ExtendsObject,UselessParentheses,CheckSkipResult - Controversial ruleset: AvoidLiteralsInIfCondition, AvoidPrefixingMethodParameters, OneDeclarationByLine + Basic ruleset: ExtendsObject,CheckSkipResult,AvoidBranchingStatementAsLastInLoop + Controversial ruleset: AvoidLiteralsInIfCondition, AvoidPrefixingMethodParameters, OneDeclarationPerLine Coupling ruleset: LoosePackageCoupling Design ruleset: LogicInversion,UseVarargs,FieldDeclarationsShouldBeAtStartOfClass + Empty ruleset: EmptyInitializer,EmptyStatementBlock Import ruleset: UnnecessaryFullyQualifiedName Optimization ruleset: RedundantFieldInitializer Naming ruleset: ShortClassName StrictException ruleset: AvoidThrowingNewInstanceOfSameException, AvoidCatchingGenericException + Unnecessary ruleset: UselessParentheses JUnit ruleset: JUnitTestContainsTooManyAsserts, UseAssertTrueInsteadOfAssertEquals New Java ruleset: android.xml: new rules specific to the Android platform New ECMAScript rules: - Basic ruleset: AssignmentInOperand,InnaccurateNumericLiteral,UnreachableCode + Basic ruleset: AssignmentInOperand,ConsistentReturn,InnaccurateNumericLiteral,UnreachableCode Braces ruleset: ForLoopsMustUseBraces,IfStmtsMustUseBraces,IfElseStmtsMustUseBraces,WhileLoopsMustUseBraces Unnecessary ruleset: UnnecessaryParentheses,UnnecessaryBlock diff --git a/pmd/regress/test/net/sourceforge/pmd/lang/java/rule/basic/BasicRulesTest.java b/pmd/regress/test/net/sourceforge/pmd/lang/java/rule/basic/BasicRulesTest.java index e7ce360115..f9d7e3df55 100644 --- a/pmd/regress/test/net/sourceforge/pmd/lang/java/rule/basic/BasicRulesTest.java +++ b/pmd/regress/test/net/sourceforge/pmd/lang/java/rule/basic/BasicRulesTest.java @@ -13,6 +13,7 @@ public class BasicRulesTest extends SimpleAggregatorTst { @Before public void setUp() { + addRule(RULESET, "AvoidBranchingStatementAsLastInLoop"); addRule(RULESET, "AvoidDecimalLiteralsInBigDecimalConstructor"); addRule(RULESET, "AvoidMultipleUnaryOperators"); addRule(RULESET, "AvoidThreadGroup"); diff --git a/pmd/regress/test/net/sourceforge/pmd/lang/java/rule/basic/xml/AvoidBranchingStatementAsLastInLoop.xml b/pmd/regress/test/net/sourceforge/pmd/lang/java/rule/basic/xml/AvoidBranchingStatementAsLastInLoop.xml new file mode 100644 index 0000000000..f3143d44a0 --- /dev/null +++ b/pmd/regress/test/net/sourceforge/pmd/lang/java/rule/basic/xml/AvoidBranchingStatementAsLastInLoop.xml @@ -0,0 +1,195 @@ + + + + + + ok: no violations + 0 + + + + violations: break:for/do/while, continue:for/do/while and return:for/do/while + 9 + + + + violations: break:for/do/while + for|do|while + + + 3 + + + + violations: continue:for/do/while + + for|do|while + + 3 + + + + violations: return:for/do/while + + + for|do|while + 3 + + + + violations: break:for + for + + + 1 + + + + violations: break:do + do + + + 1 + + + + violations: break:while + while + + + 1 + + + + violations: continue:for + + for + + 1 + + + + violations: continue:do + + do + + 1 + + + + violations: continue:while + + while + + 1 + + + + violations: return:for + + + for + 1 + + + + violations: return:do + + + do + 1 + + + + violations: return:while + + + while + 1 + + + diff --git a/pmd/rulesets/internal/dogfood.xml b/pmd/rulesets/internal/dogfood.xml index 72df930afb..5392921134 100644 --- a/pmd/rulesets/internal/dogfood.xml +++ b/pmd/rulesets/internal/dogfood.xml @@ -31,6 +31,8 @@ + + diff --git a/pmd/rulesets/java/basic.xml b/pmd/rulesets/java/basic.xml index 0e0c102fab..bd60b95e64 100644 --- a/pmd/rulesets/java/basic.xml +++ b/pmd/rulesets/java/basic.xml @@ -743,6 +743,42 @@ public class Foo { + + + + + 2 + + 25) { + break; + } + } + } +} + ]]> + + + diff --git a/pmd/rulesets/releases/50.xml b/pmd/rulesets/releases/50.xml index 432f3ec2a4..1f9619650b 100644 --- a/pmd/rulesets/releases/50.xml +++ b/pmd/rulesets/releases/50.xml @@ -6,10 +6,25 @@ This ruleset contains links to rules that are new in PMD v5.0 - + + + + + + + + + + + + + + + + @@ -17,11 +32,15 @@ This ruleset contains links to rules that are new in PMD v5.0 - + + + + + diff --git a/pmd/src/net/sourceforge/pmd/lang/java/rule/basic/AvoidBranchingStatementAsLastInLoopRule.java b/pmd/src/net/sourceforge/pmd/lang/java/rule/basic/AvoidBranchingStatementAsLastInLoopRule.java new file mode 100644 index 0000000000..1c099864dd --- /dev/null +++ b/pmd/src/net/sourceforge/pmd/lang/java/rule/basic/AvoidBranchingStatementAsLastInLoopRule.java @@ -0,0 +1,85 @@ +package net.sourceforge.pmd.lang.java.rule.basic; + +import net.sourceforge.pmd.lang.ast.Node; +import net.sourceforge.pmd.lang.java.ast.ASTBreakStatement; +import net.sourceforge.pmd.lang.java.ast.ASTContinueStatement; +import net.sourceforge.pmd.lang.java.ast.ASTDoStatement; +import net.sourceforge.pmd.lang.java.ast.ASTForStatement; +import net.sourceforge.pmd.lang.java.ast.ASTReturnStatement; +import net.sourceforge.pmd.lang.java.ast.ASTWhileStatement; +import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule; +import net.sourceforge.pmd.lang.rule.properties.EnumeratedMultiProperty; + +public class AvoidBranchingStatementAsLastInLoopRule extends AbstractJavaRule { + + public static final String CHECK_FOR = "for"; + public static final String CHECK_DO = "do"; + public static final String CHECK_WHILE = "while"; + + private static final String[] ALL_LOOP_TYPES_LABELS = new String[] { CHECK_FOR, CHECK_DO, CHECK_WHILE }; + private static final String[] ALL_LOOP_TYPES_VALUES = ALL_LOOP_TYPES_LABELS; + private static final int[] ALL_LOOP_TYPES_DEFAULTS = new int[] { 0, 1, 2 }; + + public static final EnumeratedMultiProperty CHECK_BREAK_LOOP_TYPES = new EnumeratedMultiProperty( + "checkBreakLoopTypes", "Check for break statements in loop types", ALL_LOOP_TYPES_LABELS, + ALL_LOOP_TYPES_VALUES, ALL_LOOP_TYPES_DEFAULTS, 1); + public static final EnumeratedMultiProperty CHECK_CONTINUE_LOOP_TYPES = new EnumeratedMultiProperty( + "checkContinueLoopTypes", "Check for continue statements in loop types", ALL_LOOP_TYPES_LABELS, + ALL_LOOP_TYPES_VALUES, ALL_LOOP_TYPES_DEFAULTS, 2); + public static final EnumeratedMultiProperty CHECK_RETURN_LOOP_TYPES = new EnumeratedMultiProperty( + "checkReturnLoopTypes", "Check for return statements in loop types", ALL_LOOP_TYPES_LABELS, + ALL_LOOP_TYPES_VALUES, ALL_LOOP_TYPES_DEFAULTS, 3); + + public AvoidBranchingStatementAsLastInLoopRule() { + definePropertyDescriptor(CHECK_BREAK_LOOP_TYPES); + definePropertyDescriptor(CHECK_CONTINUE_LOOP_TYPES); + definePropertyDescriptor(CHECK_RETURN_LOOP_TYPES); + + addRuleChainVisit(ASTBreakStatement.class); + addRuleChainVisit(ASTContinueStatement.class); + addRuleChainVisit(ASTReturnStatement.class); + } + + @Override + public Object visit(ASTBreakStatement node, Object data) { + return check(CHECK_BREAK_LOOP_TYPES, node, data); + } + + @Override + public Object visit(ASTContinueStatement node, Object data) { + return check(CHECK_CONTINUE_LOOP_TYPES, node, data); + } + + @Override + public Object visit(ASTReturnStatement node, Object data) { + return check(CHECK_RETURN_LOOP_TYPES, node, data); + } + + protected Object check(EnumeratedMultiProperty property, Node node, Object data) { + Node parent = node.getNthParent(5); + if (parent instanceof ASTForStatement) { + if (hasPropertyValue(property, CHECK_FOR)) { + super.addViolation(data, node); + } + } else if (parent instanceof ASTWhileStatement) { + if (hasPropertyValue(property, CHECK_WHILE)) { + super.addViolation(data, node); + } + } else if (parent instanceof ASTDoStatement) { + if (hasPropertyValue(property, CHECK_DO)) { + super.addViolation(data, node); + } + } + return data; + } + + protected boolean hasPropertyValue(EnumeratedMultiProperty property, String value) { + final Object[] values = getProperty(property); + for (int i = 0; i < values.length; i++) { + if (value.equals(values[i])) { + return true; + } + } + return false; + } +}