From cdb5525716b80f3e3f0b3e997c501477b21c720a Mon Sep 17 00:00:00 2001 From: Xavier Le Vourch Date: Wed, 16 Apr 2008 21:18:35 +0000 Subject: [PATCH] Fixed false positive in UnusedImports: javadoc comments are parsed to check @see and other tags git-svn-id: https://pmd.svn.sourceforge.net/svnroot/pmd/branches/pmd/4.2.x@6018 51baf565-9d33-0410-a72c-fc3788e3496d --- pmd/etc/changelog.txt | 4 ++ pmd/etc/grammar/Java.jjt | 5 +- .../pmd/rules/imports/xml/UnusedImports.xml | 21 +++++++++ .../typeresolution/xml/UnusedImports.xml | 21 +++++++++ .../pmd/ast/ASTCompilationUnit.java | 10 ++++ .../net/sourceforge/pmd/ast/JavaParser.java | 19 ++++---- .../pmd/ast/JavaParserTokenManager.java | 6 +++ .../pmd/rules/imports/UnusedImportsRule.java | 47 +++++++++++++++++++ 8 files changed, 123 insertions(+), 10 deletions(-) diff --git a/pmd/etc/changelog.txt b/pmd/etc/changelog.txt index 3e4d889c47..0c40804082 100644 --- a/pmd/etc/changelog.txt +++ b/pmd/etc/changelog.txt @@ -1,3 +1,7 @@ +???? - 4.2.2: + +Fixed false positive in UnusedImports: javadoc comments are parsed to check @see and other tags + April 11, 2008 - 4.2.1: '41' and '42' shortcuts for rulesets added diff --git a/pmd/etc/grammar/Java.jjt b/pmd/etc/grammar/Java.jjt index 34ae6f1b93..cd2168c94f 100644 --- a/pmd/etc/grammar/Java.jjt +++ b/pmd/etc/grammar/Java.jjt @@ -163,6 +163,8 @@ public class JavaParser { } PARSER_END(JavaParser) TOKEN_MGR_DECLS : { + protected List formalComments = new ArrayList(); + private Map excludeMap = new HashMap(); private String excludeMarker = PMD.EXCLUDE_MARKER; @@ -204,7 +206,7 @@ MORE : SPECIAL_TOKEN : { - : DEFAULT + { formalComments.add(matchedToken); } : DEFAULT } @@ -1077,6 +1079,7 @@ ASTCompilationUnit CompilationUnit() : ( < "~[]" > )? { + jjtThis.setFormalComments(token_source.formalComments); return jjtThis; } } diff --git a/pmd/regress/test/net/sourceforge/pmd/rules/imports/xml/UnusedImports.xml b/pmd/regress/test/net/sourceforge/pmd/rules/imports/xml/UnusedImports.xml index 611818cf96..e1efeda4ce 100644 --- a/pmd/regress/test/net/sourceforge/pmd/rules/imports/xml/UnusedImports.xml +++ b/pmd/regress/test/net/sourceforge/pmd/rules/imports/xml/UnusedImports.xml @@ -178,6 +178,27 @@ On demand import import java.util.*; public class Foo { List list = new ArrayList(); +} + ]]> + + + + 0 + diff --git a/pmd/regress/test/net/sourceforge/pmd/rules/typeresolution/xml/UnusedImports.xml b/pmd/regress/test/net/sourceforge/pmd/rules/typeresolution/xml/UnusedImports.xml index bf1330169e..aeb92a76b6 100644 --- a/pmd/regress/test/net/sourceforge/pmd/rules/typeresolution/xml/UnusedImports.xml +++ b/pmd/regress/test/net/sourceforge/pmd/rules/typeresolution/xml/UnusedImports.xml @@ -154,4 +154,25 @@ public class Foo { } ]]> + + + 0 + + diff --git a/pmd/src/net/sourceforge/pmd/ast/ASTCompilationUnit.java b/pmd/src/net/sourceforge/pmd/ast/ASTCompilationUnit.java index d4092b873a..aa1e0c9d6a 100644 --- a/pmd/src/net/sourceforge/pmd/ast/ASTCompilationUnit.java +++ b/pmd/src/net/sourceforge/pmd/ast/ASTCompilationUnit.java @@ -2,6 +2,7 @@ package net.sourceforge.pmd.ast; +import java.util.List; import net.sourceforge.pmd.typeresolution.ClassTypeResolver; // FUTURE Change this class to extend from SimpleJavaNode, as TypeNode is not appropriate (unless I'm wrong) @@ -17,6 +18,15 @@ public class ASTCompilationUnit extends SimpleJavaTypeNode implements Compilatio super(p, id); } + private List formalComments; + + public List getFormalComments() { + return formalComments; + } + + public void setFormalComments(List formalComments) { + this.formalComments = formalComments; + } /** * Accept the visitor. * diff --git a/pmd/src/net/sourceforge/pmd/ast/JavaParser.java b/pmd/src/net/sourceforge/pmd/ast/JavaParser.java index 0a1cb6513d..add76d8ac2 100644 --- a/pmd/src/net/sourceforge/pmd/ast/JavaParser.java +++ b/pmd/src/net/sourceforge/pmd/ast/JavaParser.java @@ -169,6 +169,7 @@ public class JavaParser/*@bgen(jjtree)*/implements JavaParserTreeConstants, Java jj_consume_token(0); jjtree.closeNodeScope(jjtn000, true); jjtc000 = false; + jjtn000.setFormalComments(token_source.formalComments); {if (true) return jjtn000;} } catch (Throwable jjte000) { if (jjtc000) { @@ -8001,6 +8002,15 @@ jjtn000.setModifiers(modifiers); return false; } + private boolean jj_3R_188() { + if (jj_scan_token(WHILE)) return true; + if (jj_scan_token(LPAREN)) return true; + if (jj_3R_88()) return true; + if (jj_scan_token(RPAREN)) return true; + if (jj_3R_91()) return true; + return false; + } + private boolean jj_3_1() { Token xsp; while (true) { @@ -8011,15 +8021,6 @@ jjtn000.setModifiers(modifiers); return false; } - private boolean jj_3R_188() { - if (jj_scan_token(WHILE)) return true; - if (jj_scan_token(LPAREN)) return true; - if (jj_3R_88()) return true; - if (jj_scan_token(RPAREN)) return true; - if (jj_3R_91()) return true; - return false; - } - private boolean jj_3_44() { if (jj_3R_74()) return true; return false; diff --git a/pmd/src/net/sourceforge/pmd/ast/JavaParserTokenManager.java b/pmd/src/net/sourceforge/pmd/ast/JavaParserTokenManager.java index 3301c836a8..ff6ae6e19e 100644 --- a/pmd/src/net/sourceforge/pmd/ast/JavaParserTokenManager.java +++ b/pmd/src/net/sourceforge/pmd/ast/JavaParserTokenManager.java @@ -6,6 +6,8 @@ import net.sourceforge.pmd.PMD; /** Token Manager. */ public class JavaParserTokenManager implements JavaParserConstants { + protected List formalComments = new ArrayList(); + private Map excludeMap = new HashMap(); private String excludeMarker = PMD.EXCLUDE_MARKER; @@ -2051,6 +2053,10 @@ void SkipLexicalActions(Token matchedToken) excludeMap.put(matchedToken.beginLine, matchedToken.image.substring(startOfNOPMD + excludeMarker.length())); } break; + case 9 : + image.append(input_stream.GetSuffix(jjimageLen + (lengthOfMatch = jjmatchedPos + 1))); + formalComments.add(matchedToken); + break; default : break; } diff --git a/pmd/src/net/sourceforge/pmd/rules/imports/UnusedImportsRule.java b/pmd/src/net/sourceforge/pmd/rules/imports/UnusedImportsRule.java index 60d8d7919e..9e120368a4 100644 --- a/pmd/src/net/sourceforge/pmd/rules/imports/UnusedImportsRule.java +++ b/pmd/src/net/sourceforge/pmd/rules/imports/UnusedImportsRule.java @@ -10,10 +10,14 @@ import net.sourceforge.pmd.ast.ASTImportDeclaration; import net.sourceforge.pmd.ast.ASTName; import net.sourceforge.pmd.ast.SimpleJavaNode; import net.sourceforge.pmd.ast.SimpleNode; +import net.sourceforge.pmd.ast.Token; import net.sourceforge.pmd.rules.ImportWrapper; import java.util.HashSet; +import java.util.List; import java.util.Set; +import java.util.regex.Matcher; +import java.util.regex.Pattern; public class UnusedImportsRule extends AbstractRule { @@ -22,12 +26,55 @@ public class UnusedImportsRule extends AbstractRule { public Object visit(ASTCompilationUnit node, Object data) { imports.clear(); super.visit(node, data); + visitFormalComments(node); for (ImportWrapper wrapper : imports) { addViolation(data, wrapper.getNode(), wrapper.getFullName()); } return data; } + /* + * Patterns to match the following constructs: + * + * @see package.class#member label + * {@linkplain package.class#member label} + * {@link package.class#member label} + * {@value package.class#field} + */ + private static final Pattern SEE_PATTERN = Pattern.compile( + "@see\\s+(\\p{Alpha}\\p{Alnum}*)[\\s#]"); + + private static final Pattern LINK_PATTERNS = Pattern.compile( + "\\{@link(?:plain)?\\s+(\\p{Alpha}\\p{Alnum}*)[\\s#]"); + + private static final Pattern VALUE_PATTERN = Pattern.compile( + "\\{@value\\s+(\\p{Alpha}\\p{Alnum}*)[\\s#]"); + + private static final Pattern[] PATTERNS = { SEE_PATTERN, LINK_PATTERNS, VALUE_PATTERN }; + + private void visitFormalComments(ASTCompilationUnit node) { + if (imports.isEmpty()) { + return; + } + List formals = node.getFormalComments(); + for (Token formal: formals) { + for (Pattern p: PATTERNS) { + Matcher m = p.matcher(formal.image); + while (m.find()) { + String s = m.group(1); + ImportWrapper candidate = new ImportWrapper(s, s, new SimpleJavaNode(-1)); + + if (imports.contains(candidate)) { + imports.remove(candidate); + if (imports.isEmpty()) { + return; + } + } + } + } + } + } + public Object visit(ASTImportDeclaration node, Object data) { if (!node.isImportOnDemand()) { ASTName importedType = (ASTName) node.jjtGetChild(0);