diff --git a/src/main/java/org/openrewrite/staticanalysis/RemoveUnusedPrivateMethods.java b/src/main/java/org/openrewrite/staticanalysis/RemoveUnusedPrivateMethods.java index 3b699e9931..346fcbd154 100644 --- a/src/main/java/org/openrewrite/staticanalysis/RemoveUnusedPrivateMethods.java +++ b/src/main/java/org/openrewrite/staticanalysis/RemoveUnusedPrivateMethods.java @@ -21,11 +21,11 @@ import org.openrewrite.java.NoMissingTypes; import org.openrewrite.java.RemoveUnusedImports; import org.openrewrite.java.search.FindAnnotations; -import org.openrewrite.java.service.AnnotationService; import org.openrewrite.java.tree.*; import java.util.List; import java.util.Set; +import java.util.concurrent.atomic.AtomicBoolean; import static java.util.Collections.singleton; @@ -75,53 +75,100 @@ private boolean unusedWarningsSuppressed(J classDeclaration) { @Override public J.@Nullable MethodDeclaration visitMethodDeclaration(J.MethodDeclaration method, ExecutionContext ctx) { J.MethodDeclaration m = super.visitMethodDeclaration(method, ctx); + JavaType.Method methodType = method.getMethodType(); - if (methodType != null && methodType.hasFlags(Flag.Private) && - !method.isConstructor() && - service(AnnotationService.class).getAllAnnotations(getCursor()).isEmpty()) { + if (methodType == null) { + return m; + } + + // Only consider private, non-constructors. + if (!methodType.hasFlags(Flag.Private) || method.isConstructor()) { + return m; + } + + // Serialization hooks & similar: do not remove even if private. + switch (m.getSimpleName()) { + case "readObject": + case "readObjectNoData": + case "readResolve": + case "writeObject": + case "writeReplace": + return m; + } - J.ClassDeclaration classDeclaration = getCursor().firstEnclosing(J.ClassDeclaration.class); - if (classDeclaration == null) { + // If we're missing the enclosing class or CU, bail out. + J.ClassDeclaration classDeclaration = getCursor().firstEnclosing(J.ClassDeclaration.class); + if (classDeclaration == null) { + return m; + } + JavaSourceFile cu = getCursor().firstEnclosingOrThrow(JavaSourceFile.class); + + // If referenced anywhere in TypesInUse (e.g., via other classes), keep it. + for (JavaType.Method usedMethodType : cu.getTypesInUse().getUsedMethods()) { + if (methodType.equals(usedMethodType)) { return m; } - switch (m.getSimpleName()) { - case "readObject": - case "readObjectNoData": - case "readResolve": - case "writeObject": - case "writeReplace": - return m; + } + + // Do not touch if the compilation unit references JUnit's MethodSource (reflective use). + for (JavaType javaType : cu.getTypesInUse().getTypesInUse()) { + if (TypeUtils.isOfClassType(javaType, "org.junit.jupiter.params.provider.MethodSource")) { + return m; } + } - JavaSourceFile cu = getCursor().firstEnclosingOrThrow(JavaSourceFile.class); - for (JavaType.Method usedMethodType : cu.getTypesInUse().getUsedMethods()) { - if (methodType.getName().equals(usedMethodType.getName()) && methodType.equals(usedMethodType)) { - return m; - } + // Temporary stop-gap until we have DFA: + // If the declared method shows generic type artifacts, be conservative and keep it. + for (JavaType.Method declared : cu.getTypesInUse().getDeclaredMethods()) { + if (methodType.equals(declared) && m.toString().contains("Generic{")) { + return m; } + } - for (JavaType javaType : cu.getTypesInUse().getTypesInUse()) { - if (TypeUtils.isOfClassType(javaType, "org.junit.jupiter.params.provider.MethodSource")) { - return m; + // Scan the enclosing class for in-class usages (method calls or method references). + // IMPORTANT: self-recursion alone does NOT count as usage. + AtomicBoolean usedInClass = new AtomicBoolean(false); + + new JavaIsoVisitor() { + @Override + public J.MethodInvocation visitMethodInvocation(J.MethodInvocation call, ExecutionContext ctx2) { + J.MethodInvocation c = super.visitMethodInvocation(call, ctx2); + JavaType.Method calledType = c.getMethodType(); + if (calledType != null && calledType.equals(methodType)) { + // Ignore self-recursion: only count if the enclosing method is NOT the same declaration. + J.MethodDeclaration enclosing = getCursor().firstEnclosing(J.MethodDeclaration.class); + if (enclosing == null || enclosing != method) { + usedInClass.set(true); + } } + return c; } - // Temporary stop-gap until we have data flow analysis. - // Do not remove method declarations with generic types since the method invocation in `cu.getTypesInUse` will be bounded with a type. - for (JavaType.Method usedMethodType : cu.getTypesInUse().getDeclaredMethods()) { - if (methodType.getName().equals(usedMethodType.getName()) && methodType.equals(usedMethodType) && m.toString().contains("Generic{")) { - return m; + @Override + public J.MemberReference visitMemberReference(J.MemberReference ref, ExecutionContext ctx2) { + J.MemberReference r = super.visitMemberReference(ref, ctx2); + JavaType.Method refType = r.getMethodType(); + if (refType != null && refType.equals(methodType)) { + J.MethodDeclaration enclosing = getCursor().firstEnclosing(J.MethodDeclaration.class); + if (enclosing == null || enclosing != method) { + usedInClass.set(true); + } } + return r; } + }.visit(classDeclaration, ctx); - doAfterVisit(new RemoveUnusedImports().getVisitor()); - //noinspection ConstantConditions - return null; + if (usedInClass.get()) { + return m; } - return m; + // No external nor internal usages -> remove it. + doAfterVisit(new RemoveUnusedImports().getVisitor()); + //noinspection ConstantConditions + return null; } }; + return Preconditions.check(new NoMissingTypes(), Repeat.repeatUntilStable(visitor)); } } diff --git a/src/test/java/org/openrewrite/staticanalysis/RemoveUnusedPrivateMethodsTest.java b/src/test/java/org/openrewrite/staticanalysis/RemoveUnusedPrivateMethodsTest.java index ce59a53659..2326cd75c4 100644 --- a/src/test/java/org/openrewrite/staticanalysis/RemoveUnusedPrivateMethodsTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/RemoveUnusedPrivateMethodsTest.java @@ -103,6 +103,488 @@ private void dontRemove2() { ); } + @Test + void getHostDottedClassName() { + rewriteRun( + // Add type stubs for all the unresolved classes + java( + """ + package org.apache.bcel.classfile; + public class Attribute {} + """, + spec -> spec.path("org/apache/bcel/classfile/Attribute.java") + ), + java( + """ + package org.apache.bcel.classfile; + public class JavaClass { + public Attribute[] getAttributes() { return null; } + public int getMajor() { return 0; } + public String getClassName() { return ""; } + } + """, + spec -> spec.path("org/apache/bcel/classfile/JavaClass.java") + ), + java( + """ + package org.apache.bcel.classfile; + public class NestHost extends Attribute { + public int getHostClassIndex() { return 0; } + public ConstantPool getConstantPool() { return null; } + } + """, + spec -> spec.path("org/apache/bcel/classfile/NestHost.java") + ), + java( + """ + package org.apache.bcel.classfile; + public class NestMembers extends Attribute { + public String[] getClassNames() { return null; } + } + """, + spec -> spec.path("org/apache/bcel/classfile/NestMembers.java") + ), + java( + """ + package org.apache.bcel.classfile; + public class ConstantPool { + public String getConstantString(int index, byte type) { return null; } + } + """, + spec -> spec.path("org/apache/bcel/classfile/ConstantPool.java") + ), + java( + """ + package org.apache.bcel; + public class Const { + public static final byte CONSTANT_Class = 7; + public static final int MAJOR_11 = 55; + } + """, + spec -> spec.path("org/apache/bcel/Const.java") + ), + java( + """ + package org.apache.bcel; + import org.apache.bcel.classfile.JavaClass; + public class Repository { + public static JavaClass lookupClass(String name) { return null; } + } + """, + spec -> spec.path("org/apache/bcel/Repository.java") + ), + java( + """ + package edu.umd.cs.findbugs.ba; + import org.apache.bcel.classfile.JavaClass; + public class AnalysisContext { + public JavaClass lookupClass(String name) { return null; } + } + """, + spec -> spec.path("edu/umd/cs/findbugs/ba/AnalysisContext.java") + ), + java( + """ + package javax.annotation; + public @interface CheckForNull {} + """, + spec -> spec.path("javax/annotation/CheckForNull.java") + ), + java( + """ + package edu.umd.cs.findbugs.util; + public class ClassName { + public static String toDottedClassName(String name) { return name; } + } + """, + spec -> spec.path("edu/umd/cs/findbugs/util/ClassName.java") + ), + + // Your actual test code + java( + """ + /* + * Contributions to SpotBugs + * Copyright (C) 2020, Simeon Andreev + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + */ + package edu.umd.cs.findbugs.util; + + import java.util.ArrayList; + import java.util.Arrays; + import java.util.List; + + import javax.annotation.CheckForNull; + + import org.apache.bcel.Const; + import org.apache.bcel.Repository; + import org.apache.bcel.classfile.Attribute; + import org.apache.bcel.classfile.ConstantPool; + import org.apache.bcel.classfile.JavaClass; + import org.apache.bcel.classfile.NestHost; + import org.apache.bcel.classfile.NestMembers; + + import edu.umd.cs.findbugs.ba.AnalysisContext; + + /** + * Provides checks to support JEP 181, improved nested member access. + * + *

+ * In short, JEP 181 defines "nest mates", "nest host" and "nest members" attributes in compiled files. Access rules are + * relaxed to allow private member access between nest mates. This removes the need for the compiler to generate + * synthetic accessors. Extra attributes are added to the separate .class files to let the compiler know which classes + * are nested in which class. + *

+ * Summary of terminology for JEP 181 and the added attributes: + *
    + *
  1. The "nest host" is the top level class that contains nested classes, i.e. the class that corresponds to the + * source file
    2. + *
  2. The "nest members" are the nested classes in the nest host.
    3. + *
  3. The "nest" consists of the nest host and the nest members.
    4. + *
  4. A "nest mate" is a class within the nest, "nest mates" can access each others private members.
    5. + *
  5. The nest host class has an attribute {@code NestMembers} which lists the qualified names of nested classes.
    6. + *
  6. A nested class has an attribute {@code NestHost} which lists the qualified name of the nest host.
    7. + *
+ * + * @see "https://openjdk.java.net/jeps/181" + */ + public class NestedAccessUtil { + + private static final int JAVA_11_CLASS_VERSION = Const.MAJOR_11; + + /** + * Checks if the specified class is a nested class or defines nested classes. + * + * @param javaClass + * The class for which to check. + * @return {@code true} if the specified class is a nested class or defines nested class, {@code false} otherwise. + */ + public static boolean hasNest(JavaClass javaClass) { + if (supportsNestedAccess(javaClass)) { + Attribute[] attributes = javaClass.getAttributes(); + for (Attribute attribute : attributes) { + if (attribute instanceof NestHost || attribute instanceof NestMembers) { + return true; + } + } + } + return false; + } + + /** + * Checks whether the specified class supports nested access as per JEP 181. + * + * @param javaClass + * The class for which to check. + * @return {@code true} if the specified class supports nested access as per JEP 181. + * + * @see NestedAccessUtil + */ + public static boolean supportsNestedAccess(JavaClass javaClass) { + return hasJava11OrAboveClassVersion(javaClass); + } + + /** + * Retrieves the qualified class names of all nest mates of the specified class. + * + * @param javaClass + * The class for which qualified class names of nest mates are retrieved. + * @param analysisContext + * The analysis context, used to look-up a nest host class if required. + * @return The qualified class name of all nest mates. If the specified class is not a nested class or does not have + * nested classes, an empty list is returned. + * @throws ClassNotFoundException + * If a nest host class was looked-up but could not be found. + * + * @see NestedAccessUtil + */ + public static List getNestMateClassNames(JavaClass javaClass, AnalysisContext analysisContext) + throws ClassNotFoundException { + List nestMateClassNames = new ArrayList<>(); + + // check if the specified class is a nest member, if so retrieve all nest members from the nest host + String nestHostClassName = getHostClassName(javaClass); + if (nestHostClassName != null) { + JavaClass nestedHostClass = analysisContext.lookupClass(nestHostClassName); + String[] nestMemberClassNames = getNestMemberClassNames(nestedHostClass); + if (nestMemberClassNames != null) { + nestMateClassNames.addAll(Arrays.asList(nestMemberClassNames)); + nestMateClassNames.add(nestHostClassName); + } + } else { + // check if the specified class is a nest host, if so retrieve the nest members from the specified class. + String[] nestMemberClassNames = getNestMemberClassNames(javaClass); + if (nestMemberClassNames != null) { + nestMateClassNames.addAll(Arrays.asList(nestMemberClassNames)); + // the nest host class is always in the nest + String className = javaClass.getClassName(); + nestMateClassNames.add(className); + } + } + return nestMateClassNames; + } + + private static boolean hasJava11OrAboveClassVersion(JavaClass... javaClasses) { + for (JavaClass javaClass : javaClasses) { + int javaClassVersion = javaClass.getMajor(); + if (javaClassVersion < JAVA_11_CLASS_VERSION) { + return false; + } + } + return true; + } + + @CheckForNull + private static String[] getNestMemberClassNames(JavaClass javaClass) { + Attribute[] sourceAttributes = javaClass.getAttributes(); + for (Attribute sourceAttribute : sourceAttributes) { + if (sourceAttribute instanceof NestMembers) { + NestMembers nestMembersAttribute = (NestMembers) sourceAttribute; + return nestMembersAttribute.getClassNames(); + } + } + return null; + } + + @CheckForNull + private static String getHostDottedClassName(JavaClass javaClass) { + String hostClassName = getHostClassName(javaClass); + if (hostClassName != null) { + return ClassName.toDottedClassName(hostClassName); + } + return null; + } + + @CheckForNull + private static String getHostClassName(JavaClass javaClass) { + Attribute[] attributes = javaClass.getAttributes(); + for (Attribute attribute : attributes) { + if (attribute instanceof NestHost) { + NestHost nestHostAttribute = (NestHost) attribute; + int targetHostClassIndex = nestHostAttribute.getHostClassIndex(); + ConstantPool constantPool = nestHostAttribute.getConstantPool(); + return constantPool.getConstantString(targetHostClassIndex, Const.CONSTANT_Class); + } + } + return null; + } + + public static List getHostClasses(JavaClass javaClass) throws ClassNotFoundException { + List list = new ArrayList<>(); + if (javaClass != null) { + String hostClassName = getHostClassName(javaClass); + if (hostClassName != null) { + JavaClass hostClass = Repository.lookupClass(hostClassName); + list.add(hostClass); + list.addAll(getHostClasses(hostClass)); + } + } + return list; + } + } + """, + """ + /* + * Contributions to SpotBugs + * Copyright (C) 2020, Simeon Andreev + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + */ + package edu.umd.cs.findbugs.util; + + import java.util.ArrayList; + import java.util.Arrays; + import java.util.List; + + import javax.annotation.CheckForNull; + + import org.apache.bcel.Const; + import org.apache.bcel.Repository; + import org.apache.bcel.classfile.Attribute; + import org.apache.bcel.classfile.ConstantPool; + import org.apache.bcel.classfile.JavaClass; + import org.apache.bcel.classfile.NestHost; + import org.apache.bcel.classfile.NestMembers; + + import edu.umd.cs.findbugs.ba.AnalysisContext; + + /** + * Provides checks to support JEP 181, improved nested member access. + * + *

+ * In short, JEP 181 defines "nest mates", "nest host" and "nest members" attributes in compiled files. Access rules are + * relaxed to allow private member access between nest mates. This removes the need for the compiler to generate + * synthetic accessors. Extra attributes are added to the separate .class files to let the compiler know which classes + * are nested in which class. + *

+ * Summary of terminology for JEP 181 and the added attributes: + *
    + *
  1. The "nest host" is the top level class that contains nested classes, i.e. the class that corresponds to the + * source file
    2. + *
  2. The "nest members" are the nested classes in the nest host.
    3. + *
  3. The "nest" consists of the nest host and the nest members.
    4. + *
  4. A "nest mate" is a class within the nest, "nest mates" can access each others private members.
    5. + *
  5. The nest host class has an attribute {@code NestMembers} which lists the qualified names of nested classes.
    6. + *
  6. A nested class has an attribute {@code NestHost} which lists the qualified name of the nest host.
    7. + *
+ * + * @see "https://openjdk.java.net/jeps/181" + */ + public class NestedAccessUtil { + + private static final int JAVA_11_CLASS_VERSION = Const.MAJOR_11; + + /** + * Checks if the specified class is a nested class or defines nested classes. + * + * @param javaClass + * The class for which to check. + * @return {@code true} if the specified class is a nested class or defines nested class, {@code false} otherwise. + */ + public static boolean hasNest(JavaClass javaClass) { + if (supportsNestedAccess(javaClass)) { + Attribute[] attributes = javaClass.getAttributes(); + for (Attribute attribute : attributes) { + if (attribute instanceof NestHost || attribute instanceof NestMembers) { + return true; + } + } + } + return false; + } + + /** + * Checks whether the specified class supports nested access as per JEP 181. + * + * @param javaClass + * The class for which to check. + * @return {@code true} if the specified class supports nested access as per JEP 181. + * + * @see NestedAccessUtil + */ + public static boolean supportsNestedAccess(JavaClass javaClass) { + return hasJava11OrAboveClassVersion(javaClass); + } + + /** + * Retrieves the qualified class names of all nest mates of the specified class. + * + * @param javaClass + * The class for which qualified class names of nest mates are retrieved. + * @param analysisContext + * The analysis context, used to look-up a nest host class if required. + * @return The qualified class name of all nest mates. If the specified class is not a nested class or does not have + * nested classes, an empty list is returned. + * @throws ClassNotFoundException + * If a nest host class was looked-up but could not be found. + * + * @see NestedAccessUtil + */ + public static List getNestMateClassNames(JavaClass javaClass, AnalysisContext analysisContext) + throws ClassNotFoundException { + List nestMateClassNames = new ArrayList<>(); + + // check if the specified class is a nest member, if so retrieve all nest members from the nest host + String nestHostClassName = getHostClassName(javaClass); + if (nestHostClassName != null) { + JavaClass nestedHostClass = analysisContext.lookupClass(nestHostClassName); + String[] nestMemberClassNames = getNestMemberClassNames(nestedHostClass); + if (nestMemberClassNames != null) { + nestMateClassNames.addAll(Arrays.asList(nestMemberClassNames)); + nestMateClassNames.add(nestHostClassName); + } + } else { + // check if the specified class is a nest host, if so retrieve the nest members from the specified class. + String[] nestMemberClassNames = getNestMemberClassNames(javaClass); + if (nestMemberClassNames != null) { + nestMateClassNames.addAll(Arrays.asList(nestMemberClassNames)); + // the nest host class is always in the nest + String className = javaClass.getClassName(); + nestMateClassNames.add(className); + } + } + return nestMateClassNames; + } + + private static boolean hasJava11OrAboveClassVersion(JavaClass... javaClasses) { + for (JavaClass javaClass : javaClasses) { + int javaClassVersion = javaClass.getMajor(); + if (javaClassVersion < JAVA_11_CLASS_VERSION) { + return false; + } + } + return true; + } + + @CheckForNull + private static String[] getNestMemberClassNames(JavaClass javaClass) { + Attribute[] sourceAttributes = javaClass.getAttributes(); + for (Attribute sourceAttribute : sourceAttributes) { + if (sourceAttribute instanceof NestMembers) { + NestMembers nestMembersAttribute = (NestMembers) sourceAttribute; + return nestMembersAttribute.getClassNames(); + } + } + return null; + } + + @CheckForNull + private static String getHostClassName(JavaClass javaClass) { + Attribute[] attributes = javaClass.getAttributes(); + for (Attribute attribute : attributes) { + if (attribute instanceof NestHost) { + NestHost nestHostAttribute = (NestHost) attribute; + int targetHostClassIndex = nestHostAttribute.getHostClassIndex(); + ConstantPool constantPool = nestHostAttribute.getConstantPool(); + return constantPool.getConstantString(targetHostClassIndex, Const.CONSTANT_Class); + } + } + return null; + } + + public static List getHostClasses(JavaClass javaClass) throws ClassNotFoundException { + List list = new ArrayList<>(); + if (javaClass != null) { + String hostClassName = getHostClassName(javaClass); + if (hostClassName != null) { + JavaClass hostClass = Repository.lookupClass(hostClassName); + list.add(hostClass); + list.addAll(getHostClasses(hostClass)); + } + } + return list; + } + } + """ + ) + ); + } + @SuppressWarnings("MissingSerialAnnotation") @Test void doNotRemoveCustomizedSerialization() {