From b272aeda2777759955d93c1b599e798b83073779 Mon Sep 17 00:00:00 2001 From: Aman Sharma Date: Mon, 6 Dec 2021 15:09:25 +0100 Subject: [PATCH] fix: prevent importing classes which are already imported via `*` (#4320) --- .../compiler/jdt/JDTImportBuilder.java | 24 ++++++++++++++++--- .../reflect/visitor/ImportCleanerTest.java | 12 +++++++--- .../importCleaner/WildCardImport.java | 5 ++++ 3 files changed, 35 insertions(+), 6 deletions(-) create mode 100644 src/test/resources/importCleaner/WildCardImport.java diff --git a/src/main/java/spoon/support/compiler/jdt/JDTImportBuilder.java b/src/main/java/spoon/support/compiler/jdt/JDTImportBuilder.java index c333934feb2..ebc0e2ead4f 100644 --- a/src/main/java/spoon/support/compiler/jdt/JDTImportBuilder.java +++ b/src/main/java/spoon/support/compiler/jdt/JDTImportBuilder.java @@ -20,8 +20,10 @@ import spoon.reflect.factory.Factory; import spoon.reflect.reference.CtReference; +import java.util.Arrays; import java.util.HashSet; import java.util.List; +import java.util.Optional; import java.util.Set; /** @@ -59,9 +61,8 @@ void build() { int lastDot = importName.lastIndexOf('.'); String packageName = importName.substring(0, lastDot); - // only get package from the model by traversing from rootPackage the model - // it does not use reflection to achieve that - CtPackage ctPackage = this.factory.Package().get(packageName); + // load package by looking up in the class loader or in the model being built + CtPackage ctPackage = loadPackage(packageName); if (ctPackage != null) { this.imports.add(createImportWithPosition(ctPackage.getReference(), importRef)); @@ -138,6 +139,23 @@ private CtImport createUnresolvedImportWithPosition(String ref, boolean isStatic return imprt; } + private CtPackage loadPackage(String packageName) { + // get all packages known for the current class loader and the ones which are accessible from it + Package[] allPackagesInAllClassLoaders = Package.getPackages(); + + Optional requiredPackage = Arrays.stream(allPackagesInAllClassLoaders) + .filter(pkg -> pkg.getName().equals(packageName)) + .findAny(); + if (requiredPackage.isPresent()) { + CtPackage ctPackage = factory.createPackage(); + ctPackage.setSimpleName(requiredPackage.get().getName()); + return ctPackage; + } + + // get package by traversing the model + return factory.Package().get(packageName); + } + private CtType getOrLoadClass(String className) { CtType klass = this.factory.Type().get(className); diff --git a/src/test/java/spoon/reflect/visitor/ImportCleanerTest.java b/src/test/java/spoon/reflect/visitor/ImportCleanerTest.java index d93f7a08c4c..42804328693 100644 --- a/src/test/java/spoon/reflect/visitor/ImportCleanerTest.java +++ b/src/test/java/spoon/reflect/visitor/ImportCleanerTest.java @@ -1,6 +1,6 @@ package spoon.reflect.visitor; -import org.junit.Test; +import org.junit.jupiter.api.Test; import spoon.Launcher; import spoon.reflect.CtModel; import spoon.reflect.declaration.CtCompilationUnit; @@ -16,13 +16,19 @@ public class ImportCleanerTest { @Test - public void testDoesNotDuplicateUnresolvedImports() { + void testDoesNotImportClassesIfAlreadyImportedViaWildCard() { + // contract: The import cleaner should not import classes if they are encompassed in wildcard import. + testImportCleanerDoesNotAlterImports("src/test/resources/importCleaner/WildCardImport.java", "WildCardImport"); + } + + @Test + void testDoesNotDuplicateUnresolvedImports() { // contract: The import cleaner should not duplicate unresolved imports testImportCleanerDoesNotAlterImports("./src/test/resources/unresolved/UnresolvedImport.java", "UnresolvedImport"); } @Test - public void testDoesNotImportInheritedStaticMethod() { + void testDoesNotImportInheritedStaticMethod() { // contract: The import cleaner should not import static attributes that are inherited testImportCleanerDoesNotAlterImports("./src/test/resources/inherit-static-method", "Derived"); } diff --git a/src/test/resources/importCleaner/WildCardImport.java b/src/test/resources/importCleaner/WildCardImport.java new file mode 100644 index 00000000000..3b557f6bf10 --- /dev/null +++ b/src/test/resources/importCleaner/WildCardImport.java @@ -0,0 +1,5 @@ +import java.util.*; + +public class WildCardImport { + private static List x = new ArrayList<>(); +}