Pull checkstyle#3406: Replace Guava's Joiner with Java 8 native approach

MEZk · Aug 13, 2016 · b3bc11a · b3bc11a
1 parent ed76401
commit b3bc11a
Show file tree

Hide file tree

Showing 5 changed files with 35 additions and 12 deletions.
diff --git a/config/pmd.xml b/config/pmd.xml
@@ -210,6 +210,10 @@
   </rule>
 
   <rule ref="rulesets/java/migrating.xml"/>
+  <rule ref="rulesets/java/migrating.xml/JUnit4TestShouldUseTestAnnotation">
+    <!-- False positive of PMD. Non-test methods can be named as 'test' -->
+    <property name="violationSuppressXPath" value="//ClassOrInterfaceDeclaration[@Image='PackageObjectFactory']"/>
+  </rule>
 
   <rule ref="rulesets/java/naming.xml">
     <!-- we use CheckstyleCustomShortVariable, to control lenght (will be fixed in PMD 5.4) and skip Override methods -->

diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/PackageObjectFactory.java b/src/main/java/com/puppycrawl/tools/checkstyle/PackageObjectFactory.java
@@ -22,11 +22,12 @@
 import java.lang.reflect.Constructor;
 import java.util.Iterator;
 import java.util.Set;
+import java.util.function.Predicate;
+import java.util.stream.Collectors;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 
-import com.google.common.base.Joiner;
 import com.google.common.collect.Sets;
 import com.puppycrawl.tools.checkstyle.api.CheckstyleException;
 import com.puppycrawl.tools.checkstyle.api.LocalizedMessage;
@@ -90,9 +91,9 @@ public Object createModule(String name) throws CheckstyleException {
             instance = createObjectWithIgnoringProblems(nameCheck, getAllPossibleNames(nameCheck));
             if (instance == null) {
 
-                final String attemptedNames = joinPackageNamesWithClassName(name)
+                final String attemptedNames = joinPackageNamesWithClassName(name, packages)
                         + STRING_SEPARATOR + nameCheck + STRING_SEPARATOR
-                        + joinPackageNamesWithClassName(nameCheck);
+                        + joinPackageNamesWithClassName(nameCheck, packages);
                 final LocalizedMessage exceptionMessage = new LocalizedMessage(0,
                     Definitions.CHECKSTYLE_BUNDLE, UNABLE_TO_INSTANTIATE_EXCEPTION_MESSAGE,
                     new String[] {name, attemptedNames}, null, getClass(), null);
@@ -137,11 +138,16 @@ private Set<String> getAllPossibleNames(String name) {
     /**
      * Creates a string by joining package names with a class name.
      * @param className name of the class for joining.
+     * @param packages packages names.
      * @return a string which is obtained by joining package names with a class name.
      */
-    private String joinPackageNamesWithClassName(String className) {
-        final Joiner joiner = Joiner.on(className + STRING_SEPARATOR).skipNulls();
-        return joiner.join(packages) + className;
+    private static String joinPackageNamesWithClassName(String className, Set<String> packages) {
+        return packages.stream().filter(new Predicate<String>() {
+            @Override
+            public boolean test(String name) {
+                return name != null;
+            }
+        }).collect(Collectors.joining(className + STRING_SEPARATOR, "", className));
     }
 
     /**

diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/api/FullIdent.java b/src/main/java/com/puppycrawl/tools/checkstyle/api/FullIdent.java
@@ -22,8 +22,6 @@
 import java.util.ArrayList;
 import java.util.List;
 
-import com.google.common.base.Joiner;
-
 /**
  * Represents a full identifier, including dots, with associated
  * position information.
@@ -76,7 +74,7 @@ public static FullIdent createFullIdentBelow(DetailAST ast) {
      * @return the text
      */
     public String getText() {
-        return Joiner.on("").join(elements);
+        return String.join("", elements);
     }
 
     /**

diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/checks/design/FinalClassCheck.java b/src/main/java/com/puppycrawl/tools/checkstyle/checks/design/FinalClassCheck.java
@@ -24,8 +24,6 @@
 import java.util.LinkedList;
 import java.util.List;
 
-import com.google.common.base.Joiner;
-
 import com.puppycrawl.tools.checkstyle.api.AbstractCheck;
 import com.puppycrawl.tools.checkstyle.api.DetailAST;
 import com.puppycrawl.tools.checkstyle.api.TokenTypes;
@@ -159,7 +157,7 @@ private static String extractQualifiedName(DetailAST classExtend) {
                 qualifiedNameParts.add(0, traverse.findFirstToken(TokenTypes.IDENT).getText());
                 traverse = traverse.findFirstToken(TokenTypes.DOT);
             }
-            className = Joiner.on(PACKAGE_SEPARATOR).join(qualifiedNameParts);
+            className = String.join(PACKAGE_SEPARATOR, qualifiedNameParts);
         }
         else {
             className = classExtend.findFirstToken(TokenTypes.IDENT).getText();

diff --git a/src/test/java/com/puppycrawl/tools/checkstyle/PackageObjectFactoryTest.java b/src/test/java/com/puppycrawl/tools/checkstyle/PackageObjectFactoryTest.java
@@ -19,9 +19,13 @@
 
 package com.puppycrawl.tools.checkstyle;
 
+import static junit.framework.TestCase.assertEquals;
 import static org.junit.Assert.assertNotNull;
 
+import java.lang.reflect.Method;
+import java.util.Collections;
 import java.util.HashSet;
+import java.util.Set;
 
 import org.junit.Test;
 
@@ -59,4 +63,17 @@ public void testMakeCheckFromName()
                         "com.puppycrawl.tools.checkstyle.checks.naming.ConstantName");
         assertNotNull(check);
     }
+
+    @Test
+    public void testJoinPackageNamesWhichContainNullWithClassName() throws Exception {
+        final Class<PackageObjectFactory> clazz = PackageObjectFactory.class;
+        final Method method =
+            clazz.getDeclaredMethod("joinPackageNamesWithClassName", String.class, Set.class);
+        method.setAccessible(true);
+        final Set<String> packages = Collections.singleton(null);
+        final String className = "SomeClass";
+        final String actual =
+            String.valueOf(method.invoke(PackageObjectFactory.class, className, packages));
+        assertEquals(className, actual);
+    }
 }