Skip to content

Commit 68bddde

Browse files
authored
shortenFullyQualifiedTypes: skip FQNs whose simple name clashes with types declared in the same file (#3031)
2 parents 6f73ed3 + cd525f2 commit 68bddde

2 files changed

Lines changed: 60 additions & 4 deletions

File tree

lib/src/javaParser/java/com/diffplug/spotless/glue/javaparser/ShortenQualifiedTypesFormatterFunc.java

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,9 @@
3333
import com.github.javaparser.ast.CompilationUnit;
3434
import com.github.javaparser.ast.ImportDeclaration;
3535
import com.github.javaparser.ast.PackageDeclaration;
36+
import com.github.javaparser.ast.body.ClassOrInterfaceDeclaration;
37+
import com.github.javaparser.ast.body.EnumDeclaration;
38+
import com.github.javaparser.ast.body.RecordDeclaration;
3639
import com.github.javaparser.ast.type.ClassOrInterfaceType;
3740
import com.github.javaparser.ast.visitor.VoidVisitorAdapter;
3841

@@ -77,7 +80,13 @@ public String apply(String rawUnix) throws Exception {
7780
existingImportsBySimple.put(simple, fqn);
7881
}
7982

80-
// 3. Walk the AST to find outermost fully-qualified type nodes
83+
// 3. Collect type names declared in this file (top-level + nested)
84+
Set<String> declaredTypeNames = new LinkedHashSet<>();
85+
cu.findAll(ClassOrInterfaceDeclaration.class).forEach(c -> declaredTypeNames.add(c.getNameAsString()));
86+
cu.findAll(EnumDeclaration.class).forEach(c -> declaredTypeNames.add(c.getNameAsString()));
87+
cu.findAll(RecordDeclaration.class).forEach(c -> declaredTypeNames.add(c.getNameAsString()));
88+
89+
// 4. Walk the AST to find outermost fully-qualified type nodes
8190
Map<String, Set<String>> simpleToFqns = new LinkedHashMap<>();
8291
List<QualifiedTypeRef> qualifiedRefs = new ArrayList<>();
8392

@@ -87,7 +96,7 @@ public String apply(String rawUnix) throws Exception {
8796
return rawUnix;
8897
}
8998

90-
// 4. Determine which FQNs are safe to shorten
99+
// 5. Determine which FQNs are safe to shorten
91100
Set<String> safeToShorten = new LinkedHashSet<>();
92101
for (Map.Entry<String, Set<String>> entry : simpleToFqns.entrySet()) {
93102
String simple = entry.getKey();
@@ -100,14 +109,18 @@ public String apply(String rawUnix) throws Exception {
100109
if (existing != null && !existing.equals(fqn)) {
101110
continue;
102111
}
112+
// Skip if simple name clashes with a type declared in this file
113+
if (declaredTypeNames.contains(simple)) {
114+
continue;
115+
}
103116
safeToShorten.add(fqn);
104117
}
105118

106119
if (safeToShorten.isEmpty()) {
107120
return rawUnix;
108121
}
109122

110-
// 5. Convert line/column positions to string offsets and replace
123+
// 6. Convert line/column positions to string offsets and replace
111124
// Build line-start offset table
112125
int[] lineOffsets = buildLineOffsets(rawUnix);
113126

@@ -134,7 +147,7 @@ public String apply(String rawUnix) throws Exception {
134147
sb.delete(removal[0], removal[1]);
135148
}
136149

137-
// 6. Add missing imports
150+
// 7. Add missing imports
138151
Set<String> newImports = new TreeSet<>();
139152
for (String fqn : safeToShorten) {
140153
if (fqn.startsWith("java.lang.") && fqn.indexOf('.', 10) == -1) {

testlib/src/test/java/com/diffplug/spotless/java/ShortenFullyQualifiedTypesStepTest.java

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -301,6 +301,49 @@ void lambdaParameterTypes() throws Exception {
301301
assertTrue(result.contains("import java.util.List;"), "should import List");
302302
}
303303

304+
@Test
305+
void fqnCollisionWithEnclosingClassName() throws Exception {
306+
// dev.jbang.cli.Alias intentionally uses dev.jbang.catalog.Alias as FQN
307+
// because the simple name "Alias" would clash with the enclosing class
308+
String code = String.join("\n",
309+
"package dev.jbang.cli;",
310+
"",
311+
"public class Alias {",
312+
" dev.jbang.catalog.Alias catalogAlias;",
313+
"}",
314+
"");
315+
assertEquals(code, apply(code));
316+
}
317+
318+
@Test
319+
void fqnCollisionWithInnerClassName() throws Exception {
320+
// FQN whose simple name matches an inner class declared in the same file
321+
String code = String.join("\n",
322+
"package com.example;",
323+
"",
324+
"public class Outer {",
325+
" static class Conflict {}",
326+
" com.other.Conflict externalConflict;",
327+
"}",
328+
"");
329+
assertEquals(code, apply(code));
330+
}
331+
332+
@Test
333+
void fqnNoCollisionWithDifferentSimpleName() throws Exception {
334+
// FQN whose simple name does NOT match the enclosing class — should still shorten
335+
String before = String.join("\n",
336+
"package dev.jbang.cli;",
337+
"",
338+
"public class Alias {",
339+
" java.util.List<String> items;",
340+
"}",
341+
"");
342+
String result = apply(before);
343+
assertFalse(codeBody(result).contains("java.util.List"), "non-conflicting FQN should be shortened");
344+
assertTrue(result.contains("import java.util.List;"), "should import List");
345+
}
346+
304347
@Test
305348
void multipleAnnotationsWithFqn() throws Exception {
306349
// FQNs used as annotation types should NOT be treated as type references

0 commit comments

Comments
 (0)