diff --git a/pmd/etc/Java1.4-c.jjt b/pmd/etc/Java1.4-c.jjt index 52f3ebf78b..f93c1d273a 100644 --- a/pmd/etc/Java1.4-c.jjt +++ b/pmd/etc/Java1.4-c.jjt @@ -576,7 +576,9 @@ void FormalParameter() : void ConstructorDeclaration() : {} { - [ "public" | "protected" | "private" ] + [ "public" { ((AccessNode) jjtThis).setPublic( true ); } + | "protected" { ((AccessNode) jjtThis).setProtected( true ); } + | "private" ] { ((AccessNode) jjtThis).setPrivate( true ); } FormalParameters() [ "throws" NameList() ] "{" [ LOOKAHEAD(ExplicitConstructorInvocation()) ExplicitConstructorInvocation() ] diff --git a/pmd/etc/build.xml b/pmd/etc/build.xml index f3b994c231..76d16405ca 100644 --- a/pmd/etc/build.xml +++ b/pmd/etc/build.xml @@ -60,7 +60,7 @@ - + @@ -83,6 +83,34 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + @@ -107,12 +135,6 @@ - - - - - - @@ -125,25 +147,6 @@ - - - - - - - - - - - - - - - - - - - diff --git a/pmd/etc/changelog.txt b/pmd/etc/changelog.txt index 8d00f64612..914653bfd4 100644 --- a/pmd/etc/changelog.txt +++ b/pmd/etc/changelog.txt @@ -1,5 +1,5 @@ ???? 2002 - ????: -Added new rules: StringToStringRule, AvoidReassigningParametersRule +Added new rules: StringToStringRule, AvoidReassigningParametersRule, UnnecessaryConstructorRule Fixed bug 631010: AvoidDeeplyNestedIfStmtsRule works correctly with if..else stmts now Fixed bug 631605: OnlyOneReturn handles line spillover now. diff --git a/pmd/regress/test/net/sourceforge/pmd/rules/UnnecessaryConstructorRuleTest.java b/pmd/regress/test/net/sourceforge/pmd/rules/UnnecessaryConstructorRuleTest.java new file mode 100644 index 0000000000..3694f58943 --- /dev/null +++ b/pmd/regress/test/net/sourceforge/pmd/rules/UnnecessaryConstructorRuleTest.java @@ -0,0 +1,27 @@ +/* + * User: tom + * Date: Nov 1, 2002 + * Time: 9:12:42 AM + */ +package test.net.sourceforge.pmd.rules; + +import net.sourceforge.pmd.rules.UnnecessaryConstructorRule; + +public class UnnecessaryConstructorRuleTest extends RuleTst { + + public void test1() throws Throwable { + super.runTest("UnnecessaryConstructor1.java", 1, new UnnecessaryConstructorRule()); + } + + public void testPrivate() throws Throwable { + super.runTest("UnnecessaryConstructor2.java", 0, new UnnecessaryConstructorRule()); + } + + public void testHasArgs() throws Throwable { + super.runTest("UnnecessaryConstructor3.java", 0, new UnnecessaryConstructorRule()); + } + + public void testHasBody() throws Throwable { + super.runTest("UnnecessaryConstructor4.java", 0, new UnnecessaryConstructorRule()); + } +} diff --git a/pmd/rulesets/newrules.xml b/pmd/rulesets/newrules.xml index c61179c021..223fa6d295 100644 --- a/pmd/rulesets/newrules.xml +++ b/pmd/rulesets/newrules.xml @@ -42,6 +42,22 @@ public class Foo { + + +Unnecessary constructor detects when a constructor is not necessary; i.e., when there's only one constructor, +it's public, has an empty body, and takes no arguments. + + + + + + diff --git a/pmd/src/net/sourceforge/pmd/PMD.java b/pmd/src/net/sourceforge/pmd/PMD.java index da43aff8a6..547b15b479 100644 --- a/pmd/src/net/sourceforge/pmd/PMD.java +++ b/pmd/src/net/sourceforge/pmd/PMD.java @@ -30,6 +30,7 @@ public class PMD { try { JavaParser parser = new JavaParser(reader); ASTCompilationUnit c = parser.CompilationUnit(); + Thread.yield(); //c.dump(""); SymbolFacade stb = new SymbolFacade(); stb.initializeWith(c); diff --git a/pmd/src/net/sourceforge/pmd/ast/ASTConstructorDeclaration.java b/pmd/src/net/sourceforge/pmd/ast/ASTConstructorDeclaration.java index a045b1a25a..0ca8787b41 100644 --- a/pmd/src/net/sourceforge/pmd/ast/ASTConstructorDeclaration.java +++ b/pmd/src/net/sourceforge/pmd/ast/ASTConstructorDeclaration.java @@ -2,7 +2,7 @@ package net.sourceforge.pmd.ast; -public class ASTConstructorDeclaration extends SimpleNode { +public class ASTConstructorDeclaration extends AccessNode { public ASTConstructorDeclaration(int id) { super(id); } diff --git a/pmd/src/net/sourceforge/pmd/ast/JavaParser.java b/pmd/src/net/sourceforge/pmd/ast/JavaParser.java index 866841cd6a..a09099bcb6 100644 --- a/pmd/src/net/sourceforge/pmd/ast/JavaParser.java +++ b/pmd/src/net/sourceforge/pmd/ast/JavaParser.java @@ -1444,9 +1444,11 @@ public class JavaParser/*@bgen(jjtree)*/implements JavaParserTreeConstants, Java switch ((jj_ntk==-1)?jj_ntk():jj_ntk) { case PUBLIC: jj_consume_token(PUBLIC); + ((AccessNode) jjtn000).setPublic( true ); break; case PROTECTED: jj_consume_token(PROTECTED); + ((AccessNode) jjtn000).setProtected( true ); break; case PRIVATE: jj_consume_token(PRIVATE); @@ -1461,6 +1463,7 @@ public class JavaParser/*@bgen(jjtree)*/implements JavaParserTreeConstants, Java jj_la1[39] = jj_gen; ; } + ((AccessNode) jjtn000).setPrivate( true ); jj_consume_token(IDENTIFIER); FormalParameters(); switch ((jj_ntk==-1)?jj_ntk():jj_ntk) { @@ -4603,24 +4606,6 @@ jjtree.openNodeScope(jjtn000);boolean hasElse = false; return retval; } - final private boolean jj_3R_210() { - if (jj_scan_token(ANDASSIGN)) return true; - if (jj_la == 0 && jj_scanpos == jj_lastpos) return false; - return false; - } - - final private boolean jj_3R_252() { - if (jj_3R_257()) return true; - if (jj_la == 0 && jj_scanpos == jj_lastpos) return false; - Token xsp; - while (true) { - xsp = jj_scanpos; - if (jj_3R_295()) { jj_scanpos = xsp; break; } - if (jj_la == 0 && jj_scanpos == jj_lastpos) return false; - } - return false; - } - final private boolean jj_3R_247() { if (jj_3R_252()) return true; if (jj_la == 0 && jj_scanpos == jj_lastpos) return false; @@ -5092,12 +5077,6 @@ jjtree.openNodeScope(jjtn000);boolean hasElse = false; return false; } - final private boolean jj_3R_322() { - if (jj_scan_token(PRIVATE)) return true; - if (jj_la == 0 && jj_scanpos == jj_lastpos) return false; - return false; - } - final private boolean jj_3R_69() { if (jj_scan_token(STATIC)) return true; if (jj_la == 0 && jj_scanpos == jj_lastpos) return false; @@ -5138,14 +5117,6 @@ jjtree.openNodeScope(jjtn000);boolean hasElse = false; return false; } - final private boolean jj_3R_352() { - if (jj_scan_token(COMMA)) return true; - if (jj_la == 0 && jj_scanpos == jj_lastpos) return false; - if (jj_3R_351()) return true; - if (jj_la == 0 && jj_scanpos == jj_lastpos) return false; - return false; - } - final private boolean jj_3R_106() { Token xsp; xsp = jj_scanpos; @@ -5160,8 +5131,10 @@ jjtree.openNodeScope(jjtn000);boolean hasElse = false; return false; } - final private boolean jj_3R_321() { - if (jj_scan_token(PROTECTED)) return true; + final private boolean jj_3R_352() { + if (jj_scan_token(COMMA)) return true; + if (jj_la == 0 && jj_scanpos == jj_lastpos) return false; + if (jj_3R_351()) return true; if (jj_la == 0 && jj_scanpos == jj_lastpos) return false; return false; } @@ -5207,6 +5180,18 @@ jjtree.openNodeScope(jjtn000);boolean hasElse = false; return false; } + final private boolean jj_3R_322() { + if (jj_scan_token(PRIVATE)) return true; + if (jj_la == 0 && jj_scanpos == jj_lastpos) return false; + return false; + } + + final private boolean jj_3R_321() { + if (jj_scan_token(PROTECTED)) return true; + if (jj_la == 0 && jj_scanpos == jj_lastpos) return false; + return false; + } + final private boolean jj_3R_334() { if (jj_scan_token(LBRACKET)) return true; if (jj_la == 0 && jj_scanpos == jj_lastpos) return false; @@ -6059,6 +6044,14 @@ jjtree.openNodeScope(jjtn000);boolean hasElse = false; return false; } + final private boolean jj_3R_381() { + if (jj_scan_token(COLON)) return true; + if (jj_la == 0 && jj_scanpos == jj_lastpos) return false; + if (jj_3R_60()) return true; + if (jj_la == 0 && jj_scanpos == jj_lastpos) return false; + return false; + } + final private boolean jj_3R_267() { Token xsp; while (true) { @@ -6077,14 +6070,6 @@ jjtree.openNodeScope(jjtn000);boolean hasElse = false; return false; } - final private boolean jj_3R_381() { - if (jj_scan_token(COLON)) return true; - if (jj_la == 0 && jj_scanpos == jj_lastpos) return false; - if (jj_3R_60()) return true; - if (jj_la == 0 && jj_scanpos == jj_lastpos) return false; - return false; - } - final private boolean jj_3R_86() { if (jj_scan_token(PROTECTED)) return true; if (jj_la == 0 && jj_scanpos == jj_lastpos) return false; @@ -6295,12 +6280,6 @@ jjtree.openNodeScope(jjtn000);boolean hasElse = false; return false; } - final private boolean jj_3R_261() { - if (jj_3R_269()) return true; - if (jj_la == 0 && jj_scanpos == jj_lastpos) return false; - return false; - } - final private boolean jj_3R_227() { if (jj_scan_token(ASSERT)) return true; if (jj_la == 0 && jj_scanpos == jj_lastpos) return false; @@ -6315,6 +6294,12 @@ jjtree.openNodeScope(jjtn000);boolean hasElse = false; return false; } + final private boolean jj_3R_261() { + if (jj_3R_269()) return true; + if (jj_la == 0 && jj_scanpos == jj_lastpos) return false; + return false; + } + final private boolean jj_3R_260() { if (jj_3R_268()) return true; if (jj_la == 0 && jj_scanpos == jj_lastpos) return false; @@ -6333,6 +6318,28 @@ jjtree.openNodeScope(jjtn000);boolean hasElse = false; return false; } + final private boolean jj_3R_380() { + if (jj_scan_token(FINALLY)) return true; + if (jj_la == 0 && jj_scanpos == jj_lastpos) return false; + if (jj_3R_70()) return true; + if (jj_la == 0 && jj_scanpos == jj_lastpos) return false; + return false; + } + + final private boolean jj_3R_379() { + if (jj_scan_token(CATCH)) return true; + if (jj_la == 0 && jj_scanpos == jj_lastpos) return false; + if (jj_scan_token(LPAREN)) return true; + if (jj_la == 0 && jj_scanpos == jj_lastpos) return false; + if (jj_3R_351()) return true; + if (jj_la == 0 && jj_scanpos == jj_lastpos) return false; + if (jj_scan_token(RPAREN)) return true; + if (jj_la == 0 && jj_scanpos == jj_lastpos) return false; + if (jj_3R_70()) return true; + if (jj_la == 0 && jj_scanpos == jj_lastpos) return false; + return false; + } + final private boolean jj_3_2() { if (jj_3R_43()) return true; if (jj_la == 0 && jj_scanpos == jj_lastpos) return false; @@ -6365,36 +6372,6 @@ jjtree.openNodeScope(jjtn000);boolean hasElse = false; return false; } - final private boolean jj_3R_380() { - if (jj_scan_token(FINALLY)) return true; - if (jj_la == 0 && jj_scanpos == jj_lastpos) return false; - if (jj_3R_70()) return true; - if (jj_la == 0 && jj_scanpos == jj_lastpos) return false; - return false; - } - - final private boolean jj_3R_379() { - if (jj_scan_token(CATCH)) return true; - if (jj_la == 0 && jj_scanpos == jj_lastpos) return false; - if (jj_scan_token(LPAREN)) return true; - if (jj_la == 0 && jj_scanpos == jj_lastpos) return false; - if (jj_3R_351()) return true; - if (jj_la == 0 && jj_scanpos == jj_lastpos) return false; - if (jj_scan_token(RPAREN)) return true; - if (jj_la == 0 && jj_scanpos == jj_lastpos) return false; - if (jj_3R_70()) return true; - if (jj_la == 0 && jj_scanpos == jj_lastpos) return false; - return false; - } - - final private boolean jj_3R_310() { - if (jj_scan_token(IMPLEMENTS)) return true; - if (jj_la == 0 && jj_scanpos == jj_lastpos) return false; - if (jj_3R_324()) return true; - if (jj_la == 0 && jj_scanpos == jj_lastpos) return false; - return false; - } - final private boolean jj_3R_226() { if (jj_scan_token(TRY)) return true; if (jj_la == 0 && jj_scanpos == jj_lastpos) return false; @@ -6412,6 +6389,14 @@ jjtree.openNodeScope(jjtn000);boolean hasElse = false; return false; } + final private boolean jj_3R_310() { + if (jj_scan_token(IMPLEMENTS)) return true; + if (jj_la == 0 && jj_scanpos == jj_lastpos) return false; + if (jj_3R_324()) return true; + if (jj_la == 0 && jj_scanpos == jj_lastpos) return false; + return false; + } + final private boolean jj_3R_308() { if (jj_scan_token(STRICTFP)) return true; if (jj_la == 0 && jj_scanpos == jj_lastpos) return false; @@ -6544,14 +6529,14 @@ jjtree.openNodeScope(jjtn000);boolean hasElse = false; return false; } - final private boolean jj_3R_67() { - if (jj_scan_token(PUBLIC)) return true; + final private boolean jj_3R_377() { + if (jj_scan_token(IDENTIFIER)) return true; if (jj_la == 0 && jj_scanpos == jj_lastpos) return false; return false; } - final private boolean jj_3R_377() { - if (jj_scan_token(IDENTIFIER)) return true; + final private boolean jj_3R_67() { + if (jj_scan_token(PUBLIC)) return true; if (jj_la == 0 && jj_scanpos == jj_lastpos) return false; return false; } @@ -6594,6 +6579,12 @@ jjtree.openNodeScope(jjtn000);boolean hasElse = false; return false; } + final private boolean jj_3R_376() { + if (jj_scan_token(IDENTIFIER)) return true; + if (jj_la == 0 && jj_scanpos == jj_lastpos) return false; + return false; + } + final private boolean jj_3R_175() { if (jj_scan_token(CLASS)) return true; if (jj_la == 0 && jj_scanpos == jj_lastpos) return false; @@ -6611,12 +6602,6 @@ jjtree.openNodeScope(jjtn000);boolean hasElse = false; return false; } - final private boolean jj_3R_376() { - if (jj_scan_token(IDENTIFIER)) return true; - if (jj_la == 0 && jj_scanpos == jj_lastpos) return false; - return false; - } - final private boolean jj_3R_66() { if (jj_scan_token(FINAL)) return true; if (jj_la == 0 && jj_scanpos == jj_lastpos) return false; @@ -6676,6 +6661,12 @@ jjtree.openNodeScope(jjtn000);boolean hasElse = false; return false; } + final private boolean jj_3R_387() { + if (jj_3R_397()) return true; + if (jj_la == 0 && jj_scanpos == jj_lastpos) return false; + return false; + } + final private boolean jj_3_1() { Token xsp; while (true) { @@ -6688,12 +6679,6 @@ jjtree.openNodeScope(jjtn000);boolean hasElse = false; return false; } - final private boolean jj_3R_387() { - if (jj_3R_397()) return true; - if (jj_la == 0 && jj_scanpos == jj_lastpos) return false; - return false; - } - final private boolean jj_3R_64() { if (jj_scan_token(FINAL)) return true; if (jj_la == 0 && jj_scanpos == jj_lastpos) return false; @@ -8215,6 +8200,24 @@ jjtree.openNodeScope(jjtn000);boolean hasElse = false; return false; } + final private boolean jj_3R_210() { + if (jj_scan_token(ANDASSIGN)) return true; + if (jj_la == 0 && jj_scanpos == jj_lastpos) return false; + return false; + } + + final private boolean jj_3R_252() { + if (jj_3R_257()) return true; + if (jj_la == 0 && jj_scanpos == jj_lastpos) return false; + Token xsp; + while (true) { + xsp = jj_scanpos; + if (jj_3R_295()) { jj_scanpos = xsp; break; } + if (jj_la == 0 && jj_scanpos == jj_lastpos) return false; + } + return false; + } + public JavaParserTokenManager token_source; JavaCharStream jj_input_stream; public Token token, jj_nt; diff --git a/pmd/src/net/sourceforge/pmd/rules/UnnecessaryConstructorRule.java b/pmd/src/net/sourceforge/pmd/rules/UnnecessaryConstructorRule.java new file mode 100644 index 0000000000..b30abc3bfe --- /dev/null +++ b/pmd/src/net/sourceforge/pmd/rules/UnnecessaryConstructorRule.java @@ -0,0 +1,55 @@ +/* + * User: tom + * Date: Nov 1, 2002 + * Time: 8:54:28 AM + */ +package net.sourceforge.pmd.rules; + +import net.sourceforge.pmd.AbstractRule; +import net.sourceforge.pmd.RuleContext; +import net.sourceforge.pmd.ast.*; + +import java.util.ArrayList; +import java.util.List; + +public class UnnecessaryConstructorRule extends AbstractRule { + + private List constructors; + + public Object visit(ASTCompilationUnit node, Object data) { + constructors = new ArrayList(); + node.findChildrenOfType(ASTConstructorDeclaration.class, constructors); + + // there must be be the only constructor + // TODO this gets thrown off by inner classes + if (constructors.size() > 1) { + return data; + } + + return super.visit(node, data); + } + + public Object visit(ASTConstructorDeclaration node, Object data) { + // must be public + AccessNode accessNode = (AccessNode)node; + if (!accessNode.isPublic()) { + return data; + } + + // must have no parameters + ASTFormalParameters params = (ASTFormalParameters)accessNode.jjtGetChild(0); + if (params.jjtGetNumChildren() > 0) { + return data; + } + + // the constructor must be empty + if (node.jjtGetNumChildren() > 1) { + return data; + } + + RuleContext ctx = (RuleContext)data; + ctx.getReport().addRuleViolation(createRuleViolation(ctx, node.getBeginLine())); + + return data; + } +} diff --git a/pmd/test-data/UnnecessaryConstructor1.java b/pmd/test-data/UnnecessaryConstructor1.java new file mode 100644 index 0000000000..5c24457f57 --- /dev/null +++ b/pmd/test-data/UnnecessaryConstructor1.java @@ -0,0 +1,3 @@ +public class UnnecessaryConstructor1 { + public UnnecessaryConstructor1() {} +} \ No newline at end of file diff --git a/pmd/test-data/UnnecessaryConstructor2.java b/pmd/test-data/UnnecessaryConstructor2.java new file mode 100644 index 0000000000..391074162c --- /dev/null +++ b/pmd/test-data/UnnecessaryConstructor2.java @@ -0,0 +1,3 @@ +public class UnnecessaryConstructor2 { + private UnnecessaryConstructor2() {} +} \ No newline at end of file diff --git a/pmd/test-data/UnnecessaryConstructor3.java b/pmd/test-data/UnnecessaryConstructor3.java new file mode 100644 index 0000000000..b1355fca54 --- /dev/null +++ b/pmd/test-data/UnnecessaryConstructor3.java @@ -0,0 +1,3 @@ +public class UnnecessaryConstructor3 { + public UnnecessaryConstructor3(int x) {} +} \ No newline at end of file diff --git a/pmd/test-data/UnnecessaryConstructor4.java b/pmd/test-data/UnnecessaryConstructor4.java new file mode 100644 index 0000000000..4a8485a1e7 --- /dev/null +++ b/pmd/test-data/UnnecessaryConstructor4.java @@ -0,0 +1,5 @@ +public class UnnecessaryConstructor4 { + public UnnecessaryConstructor4() { + int x = 2; + } +} \ No newline at end of file