Skip to content

Commit 12b3696

Browse files
shs96cshs96c
authored
[gazelle]: preserve same-package Java type refs so test deps aren’t dropped (#433)
We were missing dependencies when code referenced classes in the same java package without imports, which is common when dealing with split source roots (src/main vs src/test). This caused generated test targets to omit required production-library deps and fail builds. This change ensures those same-package references are retained for downstream dependency resolution. This change also broadens the pool of places we look for types.
1 parent 9db31b8 commit 12b3696

10 files changed

Lines changed: 258 additions & 0 deletions

File tree

‎java/gazelle/testdata/maven/README.md‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,9 @@
11
Copied from
22
[bazelbuild/examples/java-maven](https://github.com/bazelbuild/examples/tree/b29794fb55f6714442dd86946c77f8908321a430/java-maven).
3+
4+
This fixture also covers same-package class references without imports in split source roots.
5+
`src/test/java/com/example/myproject/AppTest.java` references `App` as a bare type (`App app = ...`) with no import, which is valid because both files are in `com.example.myproject`.
6+
7+
`src/sample/java/com/example/myproject/SampleOnly.java` is intentionally present to create a second provider of `com.example.myproject` (in addition to `src/main`). That forces ambiguous package-level resolution and requires class-level disambiguation using `imported_classes`.
8+
9+
If the parser drops same-package bare type references, class-level input is missing and the test target can no longer reliably resolve to `//src/main/java/com/example/myproject` (the target that actually provides `App`). With the fix, `com.example.myproject.App` is retained and resolution succeeds.

‎java/gazelle/testdata/maven/src/sample/java/com/example/myproject/BUILD.in‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+

‎java/gazelle/testdata/maven/src/sample/java/com/example/myproject/BUILD.out‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
load("@rules_java//java:defs.bzl", "java_library")
2+
3+
java_library(
4+
name = "myproject",
5+
srcs = ["SampleOnly.java"],
6+
visibility = ["//:__subpackages__"],
7+
)

‎java/gazelle/testdata/maven/src/sample/java/com/example/myproject/SampleOnly.java‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
package com.example.myproject;
2+
3+
/** Test-only class used by the maven fixture. See this fixture's README for details. */
4+
public class SampleOnly {
5+
6+
public static String sourceSet() {
7+
return "sample";
8+
}
9+
}

‎java/src/com/github/bazel_contrib/contrib_rules_jvm/javaparser/generators/ClasspathParser.java‎

Lines changed: 145 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,105 @@
5050
public class ClasspathParser {
5151
private static final Logger logger = LoggerFactory.getLogger(ClasspathParser.class);
5252

53+
private static final Set<String> JAVA_LANG_TYPES =
54+
Set.of(
55+
// Primitive wrappers
56+
"Boolean",
57+
"Byte",
58+
"Character",
59+
"Double",
60+
"Float",
61+
"Integer",
62+
"Long",
63+
"Short",
64+
"Void",
65+
// Core types
66+
"CharSequence",
67+
"Class",
68+
"ClassLoader",
69+
"Comparable",
70+
"Enum",
71+
"Iterable",
72+
"Math",
73+
"Number",
74+
"Object",
75+
"Package",
76+
"Process",
77+
"ProcessBuilder",
78+
"Record",
79+
"Runtime",
80+
"SecurityManager",
81+
"StackTraceElement",
82+
"StrictMath",
83+
"String",
84+
"StringBuffer",
85+
"StringBuilder",
86+
"System",
87+
"Thread",
88+
"ThreadGroup",
89+
"ThreadLocal",
90+
// Interfaces
91+
"Appendable",
92+
"AutoCloseable",
93+
"Cloneable",
94+
"Readable",
95+
"Runnable",
96+
// Throwable hierarchy (commonly referenced without import)
97+
"Throwable",
98+
"Error",
99+
"Exception",
100+
"RuntimeException",
101+
"ArithmeticException",
102+
"ArrayIndexOutOfBoundsException",
103+
"ArrayStoreException",
104+
"ClassCastException",
105+
"ClassNotFoundException",
106+
"CloneNotSupportedException",
107+
"EnumConstantNotPresentException",
108+
"IllegalAccessException",
109+
"IllegalArgumentException",
110+
"IllegalMonitorStateException",
111+
"IllegalStateException",
112+
"IllegalThreadStateException",
113+
"IndexOutOfBoundsException",
114+
"InstantiationException",
115+
"InterruptedException",
116+
"NegativeArraySizeException",
117+
"NoSuchFieldException",
118+
"NoSuchMethodException",
119+
"NullPointerException",
120+
"NumberFormatException",
121+
"ReflectiveOperationException",
122+
"SecurityException",
123+
"StringIndexOutOfBoundsException",
124+
"TypeNotPresentException",
125+
"UnsupportedOperationException",
126+
"AbstractMethodError",
127+
"AssertionError",
128+
"BootstrapMethodError",
129+
"ClassCircularityError",
130+
"ClassFormatError",
131+
"ExceptionInInitializerError",
132+
"IncompatibleClassChangeError",
133+
"InternalError",
134+
"LinkageError",
135+
"NoClassDefFoundError",
136+
"NoSuchFieldError",
137+
"NoSuchMethodError",
138+
"OutOfMemoryError",
139+
"StackOverflowError",
140+
"UnknownError",
141+
"UnsatisfiedLinkError",
142+
"UnsupportedClassVersionError",
143+
"VerifyError",
144+
"VirtualMachineError",
145+
// Annotations
146+
"Deprecated",
147+
"FunctionalInterface",
148+
"Override",
149+
"SafeVarargs",
150+
"SuppressWarnings");
151+
53152
private final ParsedPackageData data = new ParsedPackageData();
54153

55154
// get the system java compiler instance
@@ -138,6 +237,8 @@ class ClassScanner extends TreeScanner<Void, Void> {
138237
private final Deque<Tree> stack = new ArrayDeque<>();
139238

140239
@Nullable private Map<String, String> currentFileImports;
240+
private Set<String> locallyDefinedClassNames;
241+
private Set<String> typeParameterNames;
141242

142243
void popOrThrow(Tree expected) {
143244
Tree popped = stack.removeLast();
@@ -152,10 +253,30 @@ public Void visitCompilationUnit(CompilationUnitTree t, Void v) {
152253
compileUnit = t;
153254
fileName = Paths.get(compileUnit.getSourceFile().toUri()).getFileName().toString();
154255
currentFileImports = new HashMap<>();
256+
locallyDefinedClassNames = new TreeSet<>();
257+
typeParameterNames = new TreeSet<>();
258+
259+
// Pre-scan to collect all class names defined in this compilation unit.
260+
// This prevents inner/nested class references from being treated as
261+
// same-package cross-target dependencies.
262+
collectLocalClassNames(t);
155263

156264
return super.visitCompilationUnit(t, v);
157265
}
158266

267+
private void collectLocalClassNames(CompilationUnitTree compilationUnit) {
268+
new TreeScanner<Void, Void>() {
269+
@Override
270+
public Void visitClass(ClassTree t, Void v) {
271+
String simpleName = t.getSimpleName().toString();
272+
if (!simpleName.isEmpty()) {
273+
locallyDefinedClassNames.add(simpleName);
274+
}
275+
return super.visitClass(t, v);
276+
}
277+
}.scan(compilationUnit, null);
278+
}
279+
159280
@Override
160281
public Void visitPackage(PackageTree p, Void v) {
161282
logger.debug("JavaTools: Got package {} for class: {}", p.getPackageName(), fileName);
@@ -195,6 +316,12 @@ public Void visitImport(ImportTree i, Void v) {
195316
@Override
196317
public Void visitClass(ClassTree t, Void v) {
197318
stack.addLast(t);
319+
for (com.sun.source.tree.TypeParameterTree typeParam : t.getTypeParameters()) {
320+
typeParameterNames.add(typeParam.getName().toString());
321+
for (Tree bound : typeParam.getBounds()) {
322+
checkFullyQualifiedType(bound);
323+
}
324+
}
198325
checkFullyQualifiedType(t.getExtendsClause());
199326
for (Tree implement : t.getImplementsClause()) {
200327
checkFullyQualifiedType(implement);
@@ -217,6 +344,12 @@ public Void visitClass(ClassTree t, Void v) {
217344
@Override
218345
public Void visitMethod(com.sun.source.tree.MethodTree m, Void v) {
219346
stack.addLast(m);
347+
for (com.sun.source.tree.TypeParameterTree typeParam : m.getTypeParameters()) {
348+
typeParameterNames.add(typeParam.getName().toString());
349+
for (Tree bound : typeParam.getBounds()) {
350+
checkFullyQualifiedType(bound);
351+
}
352+
}
220353
boolean isVoidReturn = false;
221354

222355
// Check the return type on the method.
@@ -360,6 +493,18 @@ private Set<String> checkFullyQualifiedType(Tree identifier) {
360493
} else if (components.size() > 1) {
361494
data.usedTypes.add(typeName);
362495
types.add(typeName);
496+
} else if (currentPackage != null
497+
&& !typeName.isEmpty()
498+
&& !locallyDefinedClassNames.contains(typeName)
499+
&& !JAVA_LANG_TYPES.contains(typeName)
500+
&& !typeParameterNames.contains(typeName)) {
501+
// Bare class name, not imported, not locally defined — resolve against
502+
// current package. This handles same-package references like
503+
// "extends AbstractIdentifier" where the referenced class is in the
504+
// same Java package but potentially a different Bazel package.
505+
String qualifiedName = currentPackage + "." + typeName;
506+
data.usedTypes.add(qualifiedName);
507+
types.add(qualifiedName);
363508
}
364509
} else if (identifier.getKind() == Tree.Kind.PARAMETERIZED_TYPE) {
365510
Tree baseType = ((ParameterizedTypeTree) identifier).getType();

‎java/test/com/github/bazel_contrib/contrib_rules_jvm/javaparser/generators/ClasspathParserTest.java‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -390,6 +390,65 @@ public void testClassExports() throws IOException {
390390
assertEquals(expected, parser.getExportedTypes());
391391
}
392392

393+
@Test
394+
public void testSamePackageBareClassReference() throws IOException {
395+
List<? extends JavaFileObject> files =
396+
List.of(
397+
testFiles.get(
398+
"/workspace/com/gazelle/java/javaparser/generators/SamePackageReference.java"));
399+
parser.parseClasses(files);
400+
assertEquals(
401+
Set.of(
402+
"workspace.com.gazelle.java.javaparser.generators.AbstractIdentifier",
403+
"workspace.com.gazelle.java.javaparser.generators.SomeHelper",
404+
"workspace.com.gazelle.java.javaparser.generators.SomeInput",
405+
"workspace.com.gazelle.java.javaparser.generators.SomeInterface"),
406+
parser.getUsedTypes());
407+
}
408+
409+
@Test
410+
public void testSamePackageFiltersInnerClasses() throws IOException {
411+
List<? extends JavaFileObject> files =
412+
List.of(
413+
testFiles.get(
414+
"/workspace/com/gazelle/java/javaparser/generators/SamePackageWithInnerClass.java"));
415+
parser.parseClasses(files);
416+
assertEquals(
417+
Set.of("workspace.com.gazelle.java.javaparser.generators.ExternalHelper"),
418+
parser.getUsedTypes());
419+
}
420+
421+
@Test
422+
public void testSamePackageFiltersTypeParameters() throws IOException {
423+
List<? extends JavaFileObject> files =
424+
List.of(
425+
testFiles.get(
426+
"/workspace/com/gazelle/java/javaparser/generators/SamePackageWithGenerics.java"));
427+
parser.parseClasses(files);
428+
assertEquals(
429+
Set.of("workspace.com.gazelle.java.javaparser.generators.SomeBound"),
430+
parser.getUsedTypes());
431+
}
432+
433+
@Test
434+
public void testSamePackageAllTypePositions() throws IOException {
435+
// Exercises many code paths where checkFullyQualifiedType is called:
436+
// field type, method annotation, method type param bound, throws clause.
437+
// Also verifies filtering of type parameters (R) and java.lang types (String).
438+
List<? extends JavaFileObject> files =
439+
List.of(
440+
testFiles.get(
441+
"/workspace/com/gazelle/java/javaparser/generators/SamePackageAllPositions.java"));
442+
parser.parseClasses(files);
443+
assertEquals(
444+
Set.of(
445+
"workspace.com.gazelle.java.javaparser.generators.SomeFieldType",
446+
"workspace.com.gazelle.java.javaparser.generators.SomeMethodAnnotation",
447+
"workspace.com.gazelle.java.javaparser.generators.SomeMethodBound",
448+
"workspace.com.gazelle.java.javaparser.generators.SomeCheckedException"),
449+
parser.getUsedTypes());
450+
}
451+
393452
private <T> TreeSet<T> treeSet(T... values) {
394453
return new TreeSet<>(Arrays.asList(values));
395454
}

‎java/test/com/github/bazel_contrib/contrib_rules_jvm/javaparser/generators/workspace/com/gazelle/java/javaparser/generators/SamePackageAllPositions.java‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
package workspace.com.gazelle.java.javaparser.generators;
2+
3+
@SomeClassAnnotation
4+
public class SamePackageAllPositions {
5+
SomeFieldType field;
6+
7+
@SomeMethodAnnotation
8+
public <R extends SomeMethodBound> R transform(String input) throws SomeCheckedException {
9+
return null;
10+
}
11+
}

‎java/test/com/github/bazel_contrib/contrib_rules_jvm/javaparser/generators/workspace/com/gazelle/java/javaparser/generators/SamePackageReference.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
package workspace.com.gazelle.java.javaparser.generators;
2+
3+
public class SamePackageReference extends AbstractIdentifier implements SomeInterface {
4+
public SomeHelper getHelper() { return null; }
5+
public void process(SomeInput input) {}
6+
}

‎java/test/com/github/bazel_contrib/contrib_rules_jvm/javaparser/generators/workspace/com/gazelle/java/javaparser/generators/SamePackageWithGenerics.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
package workspace.com.gazelle.java.javaparser.generators;
2+
3+
public class SamePackageWithGenerics<T extends SomeBound> {
4+
public <R> R convert(T input) { return null; }
5+
}

‎java/test/com/github/bazel_contrib/contrib_rules_jvm/javaparser/generators/workspace/com/gazelle/java/javaparser/generators/SamePackageWithInnerClass.java‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
package workspace.com.gazelle.java.javaparser.generators;
2+
3+
public class SamePackageWithInnerClass {
4+
public InnerHelper createHelper() { return new InnerHelper(); }
5+
public ExternalHelper getExternal() { return null; }
6+
7+
public static class InnerHelper {}
8+
}

0 commit comments

Comments
 (0)