From e196edcf8b06f6c287c06f01748dafa7fd9892e4 Mon Sep 17 00:00:00 2001 From: James Gorman Date: Mon, 31 Mar 2014 13:48:48 +1100 Subject: [PATCH] Update UnusedImportsCheck to correctly detect classes in parameters and inline tags nested within block tags. fixing checkstyle issues Fixing even more checkstyle issues --- .../tools/checkstyle/api/Utils.java | 1 + .../checks/imports/UnusedImportsCheck.java | 113 ++++++++++++++---- .../imports/UnusedImportsCheckTest.java | 10 ++ .../tools/checkstyle/imports/InputImport.java | 20 ++++ 4 files changed, 124 insertions(+), 20 deletions(-) diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/api/Utils.java b/src/main/java/com/puppycrawl/tools/checkstyle/api/Utils.java index 227ab10ee..feeeb9567 100644 --- a/src/main/java/com/puppycrawl/tools/checkstyle/api/Utils.java +++ b/src/main/java/com/puppycrawl/tools/checkstyle/api/Utils.java @@ -202,6 +202,7 @@ public final class Utils lnr = new LineNumberReader(new InputStreamReader(fr, aCharsetName)); } catch (final UnsupportedEncodingException ex) { + fr.close(); final String message = "unsupported charset: " + ex.getMessage(); throw new UnsupportedEncodingException(message); } diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/checks/imports/UnusedImportsCheck.java b/src/main/java/com/puppycrawl/tools/checkstyle/checks/imports/UnusedImportsCheck.java index 133bdb259..482e0ebc0 100644 --- a/src/main/java/com/puppycrawl/tools/checkstyle/checks/imports/UnusedImportsCheck.java +++ b/src/main/java/com/puppycrawl/tools/checkstyle/checks/imports/UnusedImportsCheck.java @@ -18,8 +18,6 @@ //////////////////////////////////////////////////////////////////////////////// package com.puppycrawl.tools.checkstyle.checks.imports; -import com.puppycrawl.tools.checkstyle.checks.javadoc.JavadocTags; - import com.google.common.collect.Sets; import com.puppycrawl.tools.checkstyle.api.Check; import com.puppycrawl.tools.checkstyle.api.DetailAST; @@ -30,6 +28,9 @@ import com.puppycrawl.tools.checkstyle.api.TokenTypes; import com.puppycrawl.tools.checkstyle.api.Utils; import com.puppycrawl.tools.checkstyle.checks.javadoc.JavadocTag; import com.puppycrawl.tools.checkstyle.checks.javadoc.JavadocUtils; + +import java.util.HashSet; +import java.util.List; import java.util.Set; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -51,15 +52,25 @@ import java.util.regex.Pattern; */ public class UnusedImportsCheck extends Check { - /** flag to indicate when time to start collecting references */ + /** regex to match class names. */ + private static final Pattern CLASS_NAME = Pattern.compile( + "((:?[\\p{L}_$][\\p{L}\\p{N}_$]*\\.)*[\\p{L}_$][\\p{L}\\p{N}_$]*)"); + /** regex to match the first class name. */ + private static final Pattern FIRST_CLASS_NAME = Pattern.compile( + "^" + CLASS_NAME); + /** regex to match argument names. */ + private static final Pattern ARGUMENT_NAME = Pattern.compile( + "[(,]\\s*" + CLASS_NAME.pattern()); + + /** flag to indicate when time to start collecting references. */ private boolean mCollect; /** flag whether to process Javadoc comments. */ private boolean mProcessJavadoc; - /** set of the imports */ + /** set of the imports. */ private final Set mImports = Sets.newHashSet(); - /** set of references - possibly to imports or other things */ + /** set of references - possibly to imports or other things. */ private final Set mReferenced = Sets.newHashSet(); /** Default constructor. */ @@ -195,21 +206,83 @@ public class UnusedImportsCheck extends Check final int lineNo = aAST.getLineNo(); final TextBlock cmt = contents.getJavadocBefore(lineNo); if (cmt != null) { - final JavadocTags tags = JavadocUtils.getJavadocTags(cmt, - JavadocUtils.JavadocTagType.ALL); - for (final JavadocTag tag : tags.getValidTags()) { - if (tag.canReferenceImports()) { - String identifier = tag.getArg1(); - // Trim off method or link text - final Pattern pattern = - Utils.getPattern("(.+?)(?:\\s+|#|\\$).*"); - final Matcher matcher = pattern.matcher(identifier); - if (matcher.find()) { - identifier = matcher.group(1); - } - mReferenced.add(identifier); - } - } + mReferenced.addAll(processJavadoc(cmt)); } } + + /** + * Process a javadoc {@link TextBlock} and return the set of classes + * referenced within. + * @param aCmt The javadoc block to parse + * @return a set of classes referenced in the javadoc block + */ + private Set processJavadoc(TextBlock aCmt) + { + final Set references = new HashSet(); + // process all the @link type tags + // INLINEs inside BLOCKs get hidden when using ALL + for (final JavadocTag tag + : getValidTags(aCmt, JavadocUtils.JavadocTagType.INLINE)) + { + if (tag.canReferenceImports()) { + references.addAll(processJavadocTag(tag)); + } + } + // process all the @throws type tags + for (final JavadocTag tag + : getValidTags(aCmt, JavadocUtils.JavadocTagType.BLOCK)) + { + if (tag.canReferenceImports()) { + references.addAll( + matchPattern(tag.getArg1(), FIRST_CLASS_NAME)); + } + } + return references; + } + + /** + * Returns the list of valid tags found in a javadoc {@link TextBlock} + * @param aCmt The javadoc block to parse + * @param aTagType The type of tags we're interested in + * @return the list of tags + */ + private List getValidTags(TextBlock aCmt, + JavadocUtils.JavadocTagType aTagType) + { + return JavadocUtils.getJavadocTags(aCmt, aTagType).getValidTags(); + } + + /** + * Returns a list of references found in a javadoc {@link JavadocTag} + * @param aTag The javadoc tag to parse + * @return A list of references found in this tag + */ + private Set processJavadocTag(JavadocTag aTag) + { + final Set references = new HashSet(); + final String identifier = aTag.getArg1().trim(); + for (Pattern pattern : new Pattern[] + {FIRST_CLASS_NAME, ARGUMENT_NAME}) + { + references.addAll(matchPattern(identifier, pattern)); + } + return references; + } + + /** + * Extracts a list of texts matching a {@link Pattern} from a + * {@link String}. + * @param aIdentifier The String to match the pattern against + * @param aPattern The Pattern used to extract the texts + * @return A list of texts which matched the pattern + */ + private Set matchPattern(String aIdentifier, Pattern aPattern) + { + final Set references = new HashSet(); + final Matcher matcher = aPattern.matcher(aIdentifier); + while (matcher.find()) { + references.add(matcher.group(1)); + } + return references; + } } diff --git a/src/test/java/com/puppycrawl/tools/checkstyle/checks/imports/UnusedImportsCheckTest.java b/src/test/java/com/puppycrawl/tools/checkstyle/checks/imports/UnusedImportsCheckTest.java index 6cd125c83..25bbc2b9d 100644 --- a/src/test/java/com/puppycrawl/tools/checkstyle/checks/imports/UnusedImportsCheckTest.java +++ b/src/test/java/com/puppycrawl/tools/checkstyle/checks/imports/UnusedImportsCheckTest.java @@ -45,6 +45,15 @@ public class UnusedImportsCheckTest extends BaseCheckTestSupport "33:8: Unused import - java.util.Date.", "34:8: Unused import - java.util.Calendar.", "35:8: Unused import - java.util.BitSet.", + "37:8: Unused import - com.test.TestClass1.", + "38:8: Unused import - com.test.TestClass2.", + "39:8: Unused import - com.test.TestClass3.", + "40:8: Unused import - com.test.TestClass4.", + "41:8: Unused import - com.test.TestClass5.", + "42:8: Unused import - com.test.TestClass6.", + "43:8: Unused import - com.test.TestClass7.", + "44:8: Unused import - com.test.TestClass8.", + "45:8: Unused import - com.test.TestClass9.", }; verify(checkConfig, getPath("imports" + File.separator + "InputImport.java"), expected); @@ -66,6 +75,7 @@ public class UnusedImportsCheckTest extends BaseCheckTestSupport "27:15: Unused import - java.io.File.createTempFile.", //"29:8: Unused import - java.awt.Component.", // Should be detected "32:8: Unused import - java.awt.Label.", + "45:8: Unused import - com.test.TestClass9.", }; verify(checkConfig, getPath("imports" + File.separator + "InputImport.java"), expected); diff --git a/src/test/resources/com/puppycrawl/tools/checkstyle/imports/InputImport.java b/src/test/resources/com/puppycrawl/tools/checkstyle/imports/InputImport.java index a87a8a370..09884318b 100644 --- a/src/test/resources/com/puppycrawl/tools/checkstyle/imports/InputImport.java +++ b/src/test/resources/com/puppycrawl/tools/checkstyle/imports/InputImport.java @@ -34,6 +34,16 @@ import java.util.Date; import java.util.Calendar; import java.util.BitSet; +import com.test.TestClass1; +import com.test.TestClass2; +import com.test.TestClass3; +import com.test.TestClass4; +import com.test.TestClass5; +import com.test.TestClass6; +import com.test.TestClass7; +import com.test.TestClass8; +import com.test.TestClass9; + /** * Test case for imports * Here's an import used only by javadoc: {@link Date}. @@ -87,4 +97,14 @@ class InputImport * @exception HeadlessException if no graphis environment can be found. */ public void render() {} + + /** + * First is a class with a method with arguments {@link TestClass1#method1(TestClass2)}. + * Next is a class with typed method {@link TestClass3#method2(TestClass4, TestClass5)}. + * + * @param param1 with a link {@link TestClass6} + * @throws TestClass7 when broken + * @deprecated in 1 for removal in 2. Use {@link TestClass8} + */ + public void aMethodWithManyLinks() {} }