fix RemoveUnusedPrivateMethods: getHostDottedClassName #727
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Closed
1 task
Pankraz76
force-pushed
the
fix-getHostDottedClassName
branch
from
e209e0f to
347ae68
Compare
|
BUILD SUCCESSFUL in 20s thanks @hazendaz for the contribution. |
3 tasks
timtebeek
reviewed
Aug 29, 2025
Comment on lines
+106
to
+587
| @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. | ||
| * | ||
| * <p> | ||
| * 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. | ||
| * </p> | ||
| * <b>Summary</b> of terminology for JEP 181 and the added attributes: | ||
| * <ol> | ||
| * <li>The "nest host" is the top level class that contains nested classes, i.e. the class that corresponds to the | ||
| * source file</li> | ||
| * <li>The "nest members" are the nested classes in the nest host.</li> | ||
| * <li>The "nest" consists of the nest host and the nest members.</li> | ||
| * <li>A "nest mate" is a class within the nest, "nest mates" can access each others private members.</li> | ||
| * <li>The nest host class has an attribute {@code NestMembers} which lists the qualified names of nested classes.</li> | ||
| * <li>A nested class has an attribute {@code NestHost} which lists the qualified name of the nest host.</li> | ||
| * </ol> | ||
| * | ||
| * @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<String> getNestMateClassNames(JavaClass javaClass, AnalysisContext analysisContext) | ||
| throws ClassNotFoundException { | ||
| List<String> 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<JavaClass> getHostClasses(JavaClass javaClass) throws ClassNotFoundException { | ||
| List<JavaClass> 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. | ||
| * | ||
| * <p> | ||
| * 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. | ||
| * </p> | ||
| * <b>Summary</b> of terminology for JEP 181 and the added attributes: | ||
| * <ol> | ||
| * <li>The "nest host" is the top level class that contains nested classes, i.e. the class that corresponds to the | ||
| * source file</li> | ||
| * <li>The "nest members" are the nested classes in the nest host.</li> | ||
| * <li>The "nest" consists of the nest host and the nest members.</li> | ||
| * <li>A "nest mate" is a class within the nest, "nest mates" can access each others private members.</li> | ||
| * <li>The nest host class has an attribute {@code NestMembers} which lists the qualified names of nested classes.</li> | ||
| * <li>A nested class has an attribute {@code NestHost} which lists the qualified name of the nest host.</li> | ||
| * </ol> | ||
| * | ||
| * @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<String> getNestMateClassNames(JavaClass javaClass, AnalysisContext analysisContext) | ||
| throws ClassNotFoundException { | ||
| List<String> 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<JavaClass> getHostClasses(JavaClass javaClass) throws ClassNotFoundException { | ||
| List<JavaClass> 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; | ||
| } | ||
| } | ||
| """ | ||
| ) | ||
| ); | ||
| } | ||
|
|
There was a problem hiding this comment.
Can we reduce this down to just the relevant bits? We surely don't need 400 lines just to replicate an issue. A lot of this will not be relevant, but still needs to be evaluated whenever we try to mentally parse this test.
There was a problem hiding this comment.
yes first idea was to keep code like it is due to licence, will try to shrink as well.
timtebeek
reviewed
Aug 29, 2025
|
|
||
| // If referenced anywhere in TypesInUse (e.g., via other classes), keep it. | ||
| for (JavaType.Method usedMethodType : cu.getTypesInUse().getUsedMethods()) { | ||
| if (methodType.equals(usedMethodType)) { |
There was a problem hiding this comment.
Why are you changing unrelated lines here and above? This removal will lead to worse performance, based on
- https://github.com/openrewrite/rewrite/blob/8ce5308a9557dd6ca91253a5c8fb7e6c34a0ef5a/rewrite-java/src/main/java/org/openrewrite/java/tree/JavaType.java#L1563-L1572
- https://github.com/openrewrite/rewrite/blob/8ce5308a9557dd6ca91253a5c8fb7e6c34a0ef5a/rewrite-java/src/main/java/org/openrewrite/java/tree/JavaType.java#L576-L583
There was a problem hiding this comment.
thanks for the hint, yes will try to undo
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What's changed?
What's your motivation?
false negative
Anything in particular you'd like reviewers to focus on?
Anyone you would like to review specifically?
Have you considered any alternatives or workarounds?
Any additional context
Checklist