Skip to content

Commit 347ae68

Browse files
author
Vincent Potucek
committed
fix RemoveUnusedPrivateMethods: getHostDottedClassName
1 parent 232339c commit 347ae68

File tree

2 files changed

+559
-30
lines changed

2 files changed

+559
-30
lines changed

src/main/java/org/openrewrite/staticanalysis/RemoveUnusedPrivateMethods.java

Lines changed: 77 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,11 @@
2121
import org.openrewrite.java.NoMissingTypes;
2222
import org.openrewrite.java.RemoveUnusedImports;
2323
import org.openrewrite.java.search.FindAnnotations;
24-
import org.openrewrite.java.service.AnnotationService;
2524
import org.openrewrite.java.tree.*;
2625

2726
import java.util.List;
2827
import java.util.Set;
28+
import java.util.concurrent.atomic.AtomicBoolean;
2929

3030
import static java.util.Collections.singleton;
3131

@@ -75,53 +75,100 @@ private boolean unusedWarningsSuppressed(J classDeclaration) {
7575
@Override
7676
public J.@Nullable MethodDeclaration visitMethodDeclaration(J.MethodDeclaration method, ExecutionContext ctx) {
7777
J.MethodDeclaration m = super.visitMethodDeclaration(method, ctx);
78+
7879
JavaType.Method methodType = method.getMethodType();
79-
if (methodType != null && methodType.hasFlags(Flag.Private) &&
80-
!method.isConstructor() &&
81-
service(AnnotationService.class).getAllAnnotations(getCursor()).isEmpty()) {
80+
if (methodType == null) {
81+
return m;
82+
}
83+
84+
// Only consider private, non-constructors.
85+
if (!methodType.hasFlags(Flag.Private) || method.isConstructor()) {
86+
return m;
87+
}
88+
89+
// Serialization hooks & similar: do not remove even if private.
90+
switch (m.getSimpleName()) {
91+
case "readObject":
92+
case "readObjectNoData":
93+
case "readResolve":
94+
case "writeObject":
95+
case "writeReplace":
96+
return m;
97+
}
8298

83-
J.ClassDeclaration classDeclaration = getCursor().firstEnclosing(J.ClassDeclaration.class);
84-
if (classDeclaration == null) {
99+
// If we're missing the enclosing class or CU, bail out.
100+
J.ClassDeclaration classDeclaration = getCursor().firstEnclosing(J.ClassDeclaration.class);
101+
if (classDeclaration == null) {
102+
return m;
103+
}
104+
JavaSourceFile cu = getCursor().firstEnclosingOrThrow(JavaSourceFile.class);
105+
106+
// If referenced anywhere in TypesInUse (e.g., via other classes), keep it.
107+
for (JavaType.Method usedMethodType : cu.getTypesInUse().getUsedMethods()) {
108+
if (methodType.equals(usedMethodType)) {
85109
return m;
86110
}
87-
switch (m.getSimpleName()) {
88-
case "readObject":
89-
case "readObjectNoData":
90-
case "readResolve":
91-
case "writeObject":
92-
case "writeReplace":
93-
return m;
111+
}
112+
113+
// Do not touch if the compilation unit references JUnit's MethodSource (reflective use).
114+
for (JavaType javaType : cu.getTypesInUse().getTypesInUse()) {
115+
if (TypeUtils.isOfClassType(javaType, "org.junit.jupiter.params.provider.MethodSource")) {
116+
return m;
94117
}
118+
}
95119

96-
JavaSourceFile cu = getCursor().firstEnclosingOrThrow(JavaSourceFile.class);
97-
for (JavaType.Method usedMethodType : cu.getTypesInUse().getUsedMethods()) {
98-
if (methodType.getName().equals(usedMethodType.getName()) && methodType.equals(usedMethodType)) {
99-
return m;
100-
}
120+
// Temporary stop-gap until we have DFA:
121+
// If the declared method shows generic type artifacts, be conservative and keep it.
122+
for (JavaType.Method declared : cu.getTypesInUse().getDeclaredMethods()) {
123+
if (methodType.equals(declared) && m.toString().contains("Generic{")) {
124+
return m;
101125
}
126+
}
102127

103-
for (JavaType javaType : cu.getTypesInUse().getTypesInUse()) {
104-
if (TypeUtils.isOfClassType(javaType, "org.junit.jupiter.params.provider.MethodSource")) {
105-
return m;
128+
// Scan the enclosing class for in-class usages (method calls or method references).
129+
// IMPORTANT: self-recursion alone does NOT count as usage.
130+
AtomicBoolean usedInClass = new AtomicBoolean(false);
131+
132+
new JavaIsoVisitor<ExecutionContext>() {
133+
@Override
134+
public J.MethodInvocation visitMethodInvocation(J.MethodInvocation call, ExecutionContext ctx2) {
135+
J.MethodInvocation c = super.visitMethodInvocation(call, ctx2);
136+
JavaType.Method calledType = c.getMethodType();
137+
if (calledType != null && calledType.equals(methodType)) {
138+
// Ignore self-recursion: only count if the enclosing method is NOT the same declaration.
139+
J.MethodDeclaration enclosing = getCursor().firstEnclosing(J.MethodDeclaration.class);
140+
if (enclosing == null || enclosing != method) {
141+
usedInClass.set(true);
142+
}
106143
}
144+
return c;
107145
}
108146

109-
// Temporary stop-gap until we have data flow analysis.
110-
// Do not remove method declarations with generic types since the method invocation in `cu.getTypesInUse` will be bounded with a type.
111-
for (JavaType.Method usedMethodType : cu.getTypesInUse().getDeclaredMethods()) {
112-
if (methodType.getName().equals(usedMethodType.getName()) && methodType.equals(usedMethodType) && m.toString().contains("Generic{")) {
113-
return m;
147+
@Override
148+
public J.MemberReference visitMemberReference(J.MemberReference ref, ExecutionContext ctx2) {
149+
J.MemberReference r = super.visitMemberReference(ref, ctx2);
150+
JavaType.Method refType = r.getMethodType();
151+
if (refType != null && refType.equals(methodType)) {
152+
J.MethodDeclaration enclosing = getCursor().firstEnclosing(J.MethodDeclaration.class);
153+
if (enclosing == null || enclosing != method) {
154+
usedInClass.set(true);
155+
}
114156
}
157+
return r;
115158
}
159+
}.visit(classDeclaration, ctx);
116160

117-
doAfterVisit(new RemoveUnusedImports().getVisitor());
118-
//noinspection ConstantConditions
119-
return null;
161+
if (usedInClass.get()) {
162+
return m;
120163
}
121164

122-
return m;
165+
// No external nor internal usages -> remove it.
166+
doAfterVisit(new RemoveUnusedImports().getVisitor());
167+
//noinspection ConstantConditions
168+
return null;
123169
}
124170
};
171+
125172
return Preconditions.check(new NoMissingTypes(), Repeat.repeatUntilStable(visitor));
126173
}
127174
}

0 commit comments

Comments
 (0)