Issue #12923: static imports with method reference reported as unused

checkstyle · Feb 25, 2024 · 42435df · 42435df
1 parent d14a403
commit 42435df
Show file tree

Hide file tree

Showing 8 changed files with 350 additions and 7 deletions.
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
@@ -65,6 +65,19 @@
  * comment {@code {@link List}}. The alternative to avoid introducing a compile-time
  * dependency would be to write the Javadoc comment as {@code {&#64;link java.util.List}}.
  * </li>
+ * <li>
+ * A static method is imported when used as method reference. In that case,
+ * only the type needs to be imported and that's enough to resolve the method.
+ * <pre>
+ * import static java.lang.String.format; // This is not needed
+ * class FooBar {
+ *   public static void main(String[] args) {
+ *     Optional&lt;String&gt; test = Optional.empty();
+ *     test.map(String::format);
+ *   }
+ * }
+ * </pre>
+ * </li>
  * </ul>
  * <p>
  * The main limitation of this check is handling the cases where:
@@ -256,21 +269,35 @@ private void processIdent(DetailAST ast) {
         final DetailAST parent = ast.getParent();
         final int parentType = parent.getType();
 
-        final boolean isPossibleDotClassOrInMethod = parentType == TokenTypes.DOT
-                || parentType == TokenTypes.METHOD_DEF;
-
-        final boolean isQualifiedIdent = parentType == TokenTypes.DOT
-                && !TokenUtil.isOfType(ast.getPreviousSibling(), TokenTypes.DOT)
-                && ast.getNextSibling() != null;
+        final boolean isClassOrMethod = parentType == TokenTypes.DOT
+                || parentType == TokenTypes.METHOD_DEF || parentType == TokenTypes.METHOD_REF;
 
         if (TokenUtil.isTypeDeclaration(parentType)) {
             currentFrame.addDeclaredType(ast.getText());
         }
-        else if (!isPossibleDotClassOrInMethod || isQualifiedIdent) {
+        else if (!isClassOrMethod || isQualifiedIdentifier(ast)) {
             currentFrame.addReferencedType(ast.getText());
         }
     }
 
+    /**
+     * Checks whether ast is a fully qualified identifier.
+     *
+     * @param ast to check
+     * @return true if given ast is a fully qualified identifier
+     */
+    private static boolean isQualifiedIdentifier(DetailAST ast) {
+        final DetailAST parent = ast.getParent();
+        final int parentType = parent.getType();
+
+        final boolean isQualifiedIdent = parentType == TokenTypes.DOT
+                && !TokenUtil.isOfType(ast.getPreviousSibling(), TokenTypes.DOT)
+                && ast.getNextSibling() != null;
+        final boolean isQualifiedIdentFromMethodRef = parentType == TokenTypes.METHOD_REF
+                && ast.getNextSibling() != null;
+        return isQualifiedIdent || isQualifiedIdentFromMethodRef;
+    }
+
     /**
      * Collects the details of imports.
      *

diff --git a/...main/resources/com/puppycrawl/tools/checkstyle/meta/checks/imports/UnusedImportsCheck.xml b/...main/resources/com/puppycrawl/tools/checkstyle/meta/checks/imports/UnusedImportsCheck.xml
@@ -29,6 +29,19 @@
  comment {@code {@link List}}. The alternative to avoid introducing a compile-time
  dependency would be to write the Javadoc comment as {@code {&amp;#64;link java.util.List}}.
  &lt;/li&gt;
+ &lt;li&gt;
+ A static method is imported when used as method reference. In that case,
+ only the type needs to be imported and that's enough to resolve the method.
+ &lt;pre&gt;
+ import static java.lang.String.format; // This is not needed
+ class FooBar {
+   public static void main(String[] args) {
+     Optional&amp;lt;String&amp;gt; test = Optional.empty();
+     test.map(String::format);
+   }
+ }
+ &lt;/pre&gt;
+ &lt;/li&gt;
  &lt;/ul&gt;
  &lt;p&gt;
  The main limitation of this check is handling the cases where:

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
@@ -330,4 +330,51 @@ public void testStateIsClearedOnBeginTreeCollect() throws Exception {
         );
         verifyWithInlineConfigParser(file1, file2, expectedFirstInput, expectedSecondInput);
     }
+
+    @Test
+    public void testStaticMethodRefImports() throws Exception {
+        final String[] expected = {
+            "26:15: " + getCheckMessage(MSG_KEY, "java.lang.String.format"),
+            "27:15: " + getCheckMessage(MSG_KEY, "java.util.Arrays.sort"),
+            "28:15: " + getCheckMessage(MSG_KEY, "java.util.List.of"),
+            "29:15: " + getCheckMessage(MSG_KEY, "java.util.Collections.emptyMap"),
+        };
+        verifyWithInlineConfigParser(
+                getPath("InputUnusedImportsFromStaticMethodRef.java"), expected);
+    }
+
+    @Test
+    public void testStaticMethodRefImportsExtended() throws Exception {
+        final String[] expected = {
+            "15:8: " + getCheckMessage(MSG_KEY, "java.util.Objects"),
+            "16:15: " + getCheckMessage(MSG_KEY, "java.util.Arrays.asList"),
+        };
+        verifyWithInlineConfigParser(
+                getPath("InputUnusedImportsFromStaticMethodRefExtended.java"), expected);
+    }
+
+    @Test
+    public void testStaticMethodRefImportsWithJavadocDisabled() throws Exception {
+        final String[] expected = {
+            "24:8: " + getCheckMessage(MSG_KEY, "java.util.Arrays"),
+            "25:15: " + getCheckMessage(MSG_KEY, "java.lang.Integer.parseInt"),
+            "26:15: " + getCheckMessage(MSG_KEY, "java.lang.String.format"),
+            "27:15: " + getCheckMessage(MSG_KEY, "java.util.List.of"),
+            "28:15: " + getCheckMessage(MSG_KEY, "java.util.Collections.emptyMap"),
+        };
+        verifyWithInlineConfigParser(
+                getPath("InputUnusedImportsFromStaticMethodRefJavadocDisabled.java"), expected);
+    }
+
+    @Test
+    public void testStaticMethodRefImportsInDocsOnly() throws Exception {
+        final String[] expected = {
+            "11:15: " + getCheckMessage(MSG_KEY, "java.lang.Integer.parseInt"),
+            "12:15: " + getCheckMessage(MSG_KEY, "java.util.Collections.emptyEnumeration"),
+            "13:15: " + getCheckMessage(MSG_KEY, "java.util.Arrays.sort"),
+        };
+        verifyWithInlineConfigParser(
+                getPath("InputUnusedImportsFromStaticMethodRefInDocsOnly.java"), expected);
+    }
+
 }
diff --git a/.../tools/checkstyle/checks/imports/unusedimports/InputUnusedImportsFromStaticMethodRef.java b/.../tools/checkstyle/checks/imports/unusedimports/InputUnusedImportsFromStaticMethodRef.java
@@ -0,0 +1,90 @@
+/*
+UnusedImports
+processJavadoc = (default)true
+
+
+*/
+
+package com.puppycrawl.tools.checkstyle.checks.imports.unusedimports;
+
+import static java.lang.annotation.ElementType.TYPE_USE;
+import static java.lang.annotation.RetentionPolicy.RUNTIME;
+
+import java.lang.annotation.Retention;
+import java.lang.annotation.Target;
+import java.lang.ref.WeakReference;
+import java.util.ArrayList;
+import java.util.Optional;
+import java.util.Objects;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Map;
+import java.util.Collections;
+import java.util.function.Consumer;
+import java.util.function.Function;
+import java.util.function.Supplier;
+import static java.lang.String.format; // violation 'Unused import - java.lang.String.format.'
+import static java.util.Arrays.sort; // violation 'Unused import - java.util.Arrays.sort.'
+import static java.util.List.of; // violation 'Unused import - java.util.List.of.'
+import static java.util.Collections.emptyMap; // violation 'Unused import - java.util.Collections.emptyMap.'
+
+/**
+ * Use {@link Collections::emptyMap} in this javadoc.
+ */
+public class InputUnusedImportsFromStaticMethodRef {
+    public static <T, R> Function<T, R> func(Function<T, R> f) { return f; }
+
+    InputUnusedImportsFromStaticMethodRef() {
+    }
+
+    void testMethodRef() {
+        Optional<String> test = Optional.empty();
+        test.map(String::format);
+    }
+
+    void testMethodRefWithClass() {
+        Optional<String> test = Optional.empty();
+        test.map(Objects::nonNull);
+    }
+
+    void testMethodRefAssignment() {
+        int[] array = { 10, 2, 19, 5, 17 };
+        Consumer<int[]> consumer = Arrays::sort;
+        consumer.accept(array);
+    }
+
+    void testConstructorMethodRefArgument() {
+        func(func(@CJ WeakReference::new));
+    }
+
+    void testConstructorMethodRefAssignment() {
+        ListGetter<String> listGetter = ArrayList::new;
+        listGetter.get();
+    }
+    void testMethodRefOnGenericTypeAssignment() {
+        Function<String, List<String>> fn = List::of;
+        List<String> list = fn.apply("test");
+    }
+
+    void testMethodRefOnGenericTypeArgument() {
+        Function<String, List<String>> fn = func(List::of);
+        List<String> list = fn.apply("test");
+    }
+
+    void testMethodRefImportedInJavadoc() {
+        Supplier<Map<String, String>> supplier = Collections::emptyMap;
+        Map<String, String> list = supplier.get();
+    }
+
+    void testMethodRefNotImported() {
+        Function<String, Integer> parseIntFunc = Integer::parseInt;
+        int number = parseIntFunc.apply("42");
+    }
+
+}
+
+@Retention(RUNTIME) @Target({TYPE_USE}) @interface CJ { }
+interface ListGetter<T> {
+    List<T> get();
+}
+
diff --git a/...heckstyle/checks/imports/unusedimports/InputUnusedImportsFromStaticMethodRefExtended.java b/...heckstyle/checks/imports/unusedimports/InputUnusedImportsFromStaticMethodRefExtended.java
@@ -0,0 +1,32 @@
+/*
+UnusedImports
+processJavadoc = (default)true
+
+
+*/
+
+package com.puppycrawl.tools.checkstyle.checks.imports.unusedimports;
+
+import java.util.Optional;
+
+import java.util.function.Function;
+import java.util.List;
+import java.util.Arrays;
+import java.util.Objects; // violation 'Unused import - java.util.Objects.'
+import static java.util.Arrays.asList; // violation 'Unused import - java.util.Arrays.asList.'
+
+public class InputUnusedImportsFromStaticMethodRefExtended {
+    InputUnusedImportsFromStaticMethodRefExtended() {
+    }
+
+    void testMethodRefWithQualifiedName()
+    {
+        Optional<String> test = Optional.empty();
+        test.map(java.util.Objects::nonNull);
+    }
+
+    void testMethodRefWithGenericType()
+    {
+        Function<String[],List<String>> listGetter = Arrays::<String>asList;
+    }
+}
diff --git a/...ckstyle/checks/imports/unusedimports/InputUnusedImportsFromStaticMethodRefInDocsOnly.java b/...ckstyle/checks/imports/unusedimports/InputUnusedImportsFromStaticMethodRefInDocsOnly.java
@@ -0,0 +1,36 @@
+/*
+UnusedImports
+processJavadoc = (default)true
+
+
+*/
+package com.puppycrawl.tools.checkstyle.checks.imports.unusedimports;
+
+import java.util.Collections;
+import java.util.Arrays;
+import static java.lang.Integer.parseInt; // violation 'Unused import - java.lang.Integer.parseInt.'
+import static java.util.Collections.emptyEnumeration; // violation 'Unused import - java.util.Collections.emptyEnumeration.'
+import static java.util.Arrays.sort; // violation 'Unused import - java.util.Arrays.sort.'
+
+public class InputUnusedImportsFromStaticMethodRefInDocsOnly {
+
+    /**
+     * Use {@link Collections::emptyEnumeration} this line is skipped as it is not valid link.
+     * @see Collections#emptyEnumeration
+     * @throws IllegalAccessError::new
+     */
+    public static void m() {}
+
+    /**
+     * @see Arrays#sort
+     * @throws IllegalAccessError::new
+     */
+    public static void n() {}
+
+    /**
+     * Use {@link Integer::parseInt} this line is skipped as it is not valid link.
+     * @throws IllegalAccessError::new
+     */
+    public static void l() {}
+
+}
diff --git a/...le/checks/imports/unusedimports/InputUnusedImportsFromStaticMethodRefJavadocDisabled.java b/...le/checks/imports/unusedimports/InputUnusedImportsFromStaticMethodRefJavadocDisabled.java
@@ -0,0 +1,85 @@
+/*
+UnusedImports
+processJavadoc = false
+
+
+*/
+
+package com.puppycrawl.tools.checkstyle.checks.imports.unusedimports;
+
+import static java.lang.annotation.ElementType.TYPE_USE;
+import static java.lang.annotation.RetentionPolicy.RUNTIME;
+
+import java.lang.annotation.Retention;
+import java.lang.annotation.Target;
+import java.lang.ref.WeakReference;
+import java.util.ArrayList;
+import java.util.Optional;
+import java.util.Objects;
+import java.util.List;
+import java.util.Map;
+import java.util.Collections;
+import java.util.function.Function;
+import java.util.function.Supplier;
+import java.util.Arrays; // violation 'Unused import - java.util.Arrays.'
+import static java.lang.Integer.parseInt; // violation 'Unused import - java.lang.Integer.'
+import static java.lang.String.format; // violation 'Unused import - java.lang.String.format.'
+import static java.util.List.of; // violation 'Unused import - java.util.List.of.'
+import static java.util.Collections.emptyMap; // violation 'Unused import - java.util.Collections.emptyMap.'
+
+/**
+ * Use {@link Arrays::sort} in this javadoc.
+ * Use {@link Integer::parseInt} in this javadoc.
+ */
+public class InputUnusedImportsFromStaticMethodRefJavadocDisabled {
+
+    public static <T, R> Function<T, R> func(Function<T, R> f) { return f; }
+
+    InputUnusedImportsFromStaticMethodRefJavadocDisabled() {
+    }
+
+    void testMethodRef()
+    {
+        Optional<String> test = Optional.empty();
+        test.map(String::format);
+    }
+
+    void testMethodRefWithClass()
+    {
+        Optional<String> test = Optional.empty();
+        test.map(Objects::nonNull);
+    }
+    void testConstructorMethodRefArgument()
+    {
+        func(func(@CJ2 WeakReference::new));
+    }
+
+    void testConstructorMethodRefAssignment()
+    {
+        ListGetter2<String> listGetter = ArrayList::new;
+        listGetter.get();
+    }
+    void testMethodRefOnGenericTypeAssignment()
+    {
+        Function<String, List<String>> fn = List::of;
+        List<String> list = fn.apply("test");
+    }
+
+    void testMethodRefOnGenericTypeArgument()
+    {
+        Function<String, List<String>> fn = func(List::of);
+        List<String> list = fn.apply("test");
+    }
+    void testMethodRefImportedInJavadoc()
+    {
+        Supplier<Map<String, String>> supplier = Collections::emptyMap;
+        Map<String, String> list = supplier.get();
+    }
+
+}
+
+@Retention(RUNTIME) @Target({TYPE_USE}) @interface CJ2 { }
+
+interface ListGetter2<T> {
+    List<T> get();
+}