Skip to content

shortenFullyQualifiedTypes: preserve unqualified type resolution #3037

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
maxandersen wants to merge 1 commit into
base: main
Choose a base branch
from
Open

shortenFullyQualifiedTypes: preserve unqualified type resolution #3037

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 20 additions & 11 deletions ...Parser/java/com/diffplug/spotless/glue/javaparser/ShortenQualifiedTypesFormatterFunc.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,13 @@ public String apply(String rawUnix) throws Exception {
cu.findAll(EnumDeclaration.class).forEach(c -> declaredTypeNames.add(c.getNameAsString()));
cu.findAll(RecordDeclaration.class).forEach(c -> declaredTypeNames.add(c.getNameAsString()));

// 4. Walk the AST to find outermost fully-qualified type nodes
// 4. Collect unqualified type references, which may resolve to types in the same package
Set<String> unqualifiedTypeNames = new LinkedHashSet<>();
cu.findAll(ClassOrInterfaceType.class).stream()
.filter(type -> type.getScope().isEmpty())
.forEach(type -> unqualifiedTypeNames.add(type.getNameAsString()));

// 5. Walk the AST to find outermost fully-qualified type nodes
Map<String, Set<String>> simpleToFqns = new LinkedHashMap<>();
List<QualifiedTypeRef> qualifiedRefs = new ArrayList<>();

Expand All @@ -96,7 +102,7 @@ public String apply(String rawUnix) throws Exception {
return rawUnix;
}

// 5. Determine which FQNs are safe to shorten
// 6. Determine which FQNs are safe to shorten
Set<String> safeToShorten = new LinkedHashSet<>();
for (Map.Entry<String, Set<String>> entry : simpleToFqns.entrySet()) {
String simple = entry.getKey();
Expand All @@ -109,8 +115,9 @@ public String apply(String rawUnix) throws Exception {
if (existing != null && !existing.equals(fqn)) {
continue;
}
// Skip if simple name clashes with a type declared in this file
if (declaredTypeNames.contains(simple)) {
// Skip if simple name clashes with a type declared or already referenced in this file
if (declaredTypeNames.contains(simple)
|| (existing == null && unqualifiedTypeNames.contains(simple) && !isImplicitlyImported(fqn, packageName))) {
continue;
}
safeToShorten.add(fqn);
Expand All @@ -120,7 +127,7 @@ public String apply(String rawUnix) throws Exception {
return rawUnix;
}

// 6. Convert line/column positions to string offsets and replace
// 7. Convert line/column positions to string offsets and replace
// Build line-start offset table
int[] lineOffsets = buildLineOffsets(rawUnix);

Expand All @@ -147,14 +154,10 @@ public String apply(String rawUnix) throws Exception {
sb.delete(removal[0], removal[1]);
}

// 7. Add missing imports
// 8. Add missing imports
Set<String> newImports = new TreeSet<>();
for (String fqn : safeToShorten) {
if (fqn.startsWith("java.lang.") && fqn.indexOf('.', 10) == -1) {
continue;
}
if (!packageName.isEmpty() && fqn.startsWith(packageName + ".")
&& fqn.indexOf('.', packageName.length() + 1) == -1) {
if (isImplicitlyImported(fqn, packageName)) {
continue;
}
if (existingImportFqns.contains(fqn)) {
Expand Down Expand Up @@ -242,6 +245,12 @@ private static boolean startsWithPackage(String rawName) {
return !rawName.isEmpty() && Character.isLowerCase(rawName.charAt(0));
}

private static boolean isImplicitlyImported(String fqn, String packageName) {
return fqn.startsWith("java.lang.") && fqn.indexOf('.', 10) == -1
|| !packageName.isEmpty() && fqn.startsWith(packageName + ".")
&& fqn.indexOf('.', packageName.length() + 1) == -1;
}

/** Builds an array where lineOffsets[line] is the char offset of the start of that line (1-indexed). */
private static int[] buildLineOffsets(String text) {
List<Integer> offsets = new ArrayList<>();
Expand Down
15 changes: 15 additions & 0 deletions testlib/src/test/java/com/diffplug/spotless/java/ShortenFullyQualifiedTypesStepTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ void alreadyImportedNotDuplicated() throws Exception {
"",
"public class Foo {",
" java.util.List<String> a;",
" List<String> b;",
"}",
"");
String result = apply(before);
Expand Down Expand Up @@ -329,6 +330,20 @@ void fqnCollisionWithInnerClassName() throws Exception {
assertEquals(code, apply(code));
}

@Test
void fqnCollisionWithUnqualifiedSamePackageType() throws Exception {
// RandomAccessFile is declared in another file in this package; importing java.io.RandomAccessFile
// would silently change which type the unqualified superclass name resolves to
String code = String.join("\n",
"package test.reprod1;",
"",
"public class ClassA extends RandomAccessFile {",
" final java.io.RandomAccessFile file;",
"}",
"");
assertEquals(code, apply(code));
}

@Test
void fqnNoCollisionWithDifferentSimpleName() throws Exception {
// FQN whose simple name does NOT match the enclosing class — should still shorten
Expand Down
Loading