Skip to content

Commit

Permalink
Fix a number of bugs related to handling of unnamed packages (#705)
Browse files Browse the repository at this point in the history
  • Loading branch information
leventov authored and GerardPaligot committed Jun 17, 2016
1 parent f17381a commit 540b260
Show file tree
Hide file tree
Showing 18 changed files with 186 additions and 64 deletions.
6 changes: 6 additions & 0 deletions src/main/java/spoon/reflect/declaration/CtPackage.java
Original file line number Diff line number Diff line change
Expand Up @@ -127,4 +127,10 @@ public interface CtPackage extends CtNamedElement, CtShadowable {

@Override
CtPackage clone();

/**
* Returns {@code true} if this is an <i>unnamed</i> Java package.
* See JLS §7.4.2. Unnamed Packages.
*/
boolean isUnnamedPackage();
}
3 changes: 3 additions & 0 deletions src/main/java/spoon/reflect/factory/PackageFactory.java
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,9 @@ public CtPackage create(CtPackage parent, String simpleName) {
* the full name of the package
*/
public CtPackage getOrCreate(String qualifiedName) {
if (qualifiedName.isEmpty()) {
return factory.getModel().getRootPackage();
}
StringTokenizer token = new StringTokenizer(qualifiedName, CtPackage.PACKAGE_SEPARATOR);
CtPackage last = factory.getModel().getRootPackage();

Expand Down
14 changes: 12 additions & 2 deletions src/main/java/spoon/reflect/factory/TypeFactory.java
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,9 @@
import spoon.support.visitor.java.JavaReflectionTreeBuilder;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.Date;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import java.util.TreeSet;
Expand All @@ -42,6 +44,12 @@
*/
public class TypeFactory extends SubFactory {

private static final Set<String> NULL_PACKAGE_CLASSES = new HashSet<String>(
Arrays.asList("void", "boolean", "byte", "short", "char", "int", "float", "long",
"double",
// TODO (leventov) it is questionable to me that nulltype should also be here
CtTypeReference.NULL_TYPE_NAME));

public final CtTypeReference<?> NULL_TYPE = createReference(CtTypeReference.NULL_TYPE_NAME);
public final CtTypeReference<Void> VOID = createReference(Void.class);
public final CtTypeReference<String> STRING = createReference(String.class);
Expand Down Expand Up @@ -319,6 +327,8 @@ public <T> CtTypeReference<T> createReference(String qualifiedName) {
ref.setDeclaringType(createReference(getDeclaringTypeName(qualifiedName)));
} else if (hasPackage(qualifiedName) > 0) {
ref.setPackage(factory.Package().createReference(getPackageName(qualifiedName)));
} else if (!NULL_PACKAGE_CLASSES.contains(qualifiedName)) {
ref.setPackage(factory.Package().topLevel());
}
ref.setSimpleName(getSimpleName(qualifiedName));
return ref;
Expand Down Expand Up @@ -377,7 +387,7 @@ public boolean matches(CtNewClass element) {
if (packageIndex > 0) {
pack = factory.Package().get(qualifiedName.substring(0, packageIndex));
} else {
pack = factory.Package().get(CtPackage.TOP_LEVEL_PACKAGE_NAME);
pack = factory.Package().getRootPackage();
}

if (pack == null) {
Expand Down Expand Up @@ -462,7 +472,7 @@ protected String getPackageName(String qualifiedName) {
if (hasPackage(qualifiedName) >= 0) {
return qualifiedName.substring(0, hasPackage(qualifiedName));
}
return CtPackage.TOP_LEVEL_PACKAGE_NAME;
return "";
}

/**
Expand Down
6 changes: 6 additions & 0 deletions src/main/java/spoon/reflect/reference/CtPackageReference.java
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,12 @@ public interface CtPackageReference extends CtReference {
*/
void replace(CtPackageReference packageReference);

/**
* Returns {@code true} if this is a reference to an <i>unnamed</i>
* Java package. See JLS §7.4.2. Unnamed Packages.
*/
boolean isUnnamedPackage();

@Override
CtPackageReference clone();
}
31 changes: 24 additions & 7 deletions src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
import java.util.Objects;

/**
* A visitor for generating Java code from the program compile-time model.
Expand Down Expand Up @@ -299,7 +300,7 @@ public String getPackageDeclaration() {
a.accept(this);
}

if (!context.currentTopLevel.getPackage().getQualifiedName().equals(CtPackage.TOP_LEVEL_PACKAGE_NAME)) {
if (!context.currentTopLevel.getPackage().isUnnamedPackage()) {
write("package " + context.currentTopLevel.getPackage().getQualifiedName() + ";");
}
String ret = sbf.toString();
Expand Down Expand Up @@ -1916,7 +1917,7 @@ public <T, A extends T> void visitCtOperatorAssignment(CtOperatorAssignment<T, A
}

public void visitCtPackage(CtPackage ctPackage) {
if (!ctPackage.getQualifiedName().equals(CtPackage.TOP_LEVEL_PACKAGE_NAME)) {
if (!ctPackage.isUnnamedPackage()) {
write("package " + ctPackage.getQualifiedName() + ";");
} else {
write("// default package (CtPackage.TOP_LEVEL_PACKAGE_NAME in Spoon= unnamed package)\n");
Expand Down Expand Up @@ -2042,10 +2043,10 @@ public void visitCtTypeParameterReference(CtTypeParameterReference ref) {
return;
}
writeAnnotations(ref);
if (importsContext.isImported(ref)) {
write(ref.getSimpleName());
} else {
if (printQualified(ref)) {
write(ref.getQualifiedName());
} else {
write(ref.getSimpleName());
}
if ((!context.isInvocation || "?".equals(ref.getSimpleName())) && ref.getBoundingType() != null) {
if (ref.isUpper()) {
Expand All @@ -2057,6 +2058,22 @@ public void visitCtTypeParameterReference(CtTypeParameterReference ref) {
}
}

private boolean printQualified(CtTypeReference<?> ref) {
if (importsContext.isImported(ref)) {
// If my.pkg.Something is imported, but we are in the context of a class which is
// also called "Something", we should still use qualified version my.pkg.Something
for (CtTypeReference<?> enclosingClassRef : context.currentThis) {
if (enclosingClassRef.getSimpleName().equals(ref.getSimpleName())
&& !Objects.equals(enclosingClassRef.getPackage(), ref.getPackage())) {
return true;
}
}
return false;
} else {
return true;
}
}

@Override
public <T> void visitCtIntersectionTypeReference(CtIntersectionTypeReference<T> reference) {
for (CtTypeReference<?> bound : reference.getBounds()) {
Expand Down Expand Up @@ -2115,8 +2132,8 @@ private void visitCtTypeReference(CtTypeReference<?> ref, boolean withGenerics)
write(ref.getSimpleName());
}
} else {
if (ref.getPackage() != null && !importsContext.isImported(ref)) {
if (!CtPackage.TOP_LEVEL_PACKAGE_NAME.equals(ref.getPackage().getSimpleName())) {
if (ref.getPackage() != null && printQualified(ref)) {
if (!ref.getPackage().isUnnamedPackage()) {
scan(ref.getPackage()).write(CtPackage.PACKAGE_SEPARATOR);
}
}
Expand Down
51 changes: 48 additions & 3 deletions src/main/java/spoon/reflect/visitor/ImportScannerImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,10 @@

import java.lang.annotation.Annotation;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.TreeMap;
Expand All @@ -44,7 +46,14 @@
* A scanner that calculates the imports for a given model.
*/
public class ImportScannerImpl extends CtScanner implements ImportScanner {

private static final Collection<String> namesPresentInJavaLang8 =
Collections.singletonList("FunctionalInterface");
private static final Collection<String> namesPresentInJavaLang9 = Arrays.asList(
"ProcessHandle", "StackWalker", "StackFramePermission");

private Map<String, CtTypeReference<?>> imports = new TreeMap<>();
private Map<String, Boolean> namesPresentInJavaLang = new HashMap<>();

@Override
public <T> void visitCtFieldRead(CtFieldRead<T> fieldRead) {
Expand Down Expand Up @@ -192,12 +201,11 @@ private Collection<CtTypeReference<?>> getImports(
if (imports.isEmpty()) {
return Collections.EMPTY_LIST;
}
CtPackageReference pack = ((CtTypeReference<?>) imports
.get(simpleType.getSimpleName())).getPackage();
CtPackageReference pack = simpleType.getPackage().getReference();
List<CtTypeReference<?>> refs = new ArrayList<>();
for (CtTypeReference<?> ref : imports.values()) {
// ignore non-top-level type
if (ref.getPackage() != null) {
if (ref.getPackage() != null && !ref.getPackage().isUnnamedPackage()) {
// ignore java.lang package
if (!ref.getPackage().getSimpleName().equals("java.lang")) {
// ignore type in same package
Expand All @@ -218,7 +226,44 @@ private boolean addImport(CtTypeReference<?> ref) {
if (imports.containsKey(ref.getSimpleName())) {
return isImported(ref);
}
// don't import unnamed package elements
if (ref.getPackage() == null || ref.getPackage().isUnnamedPackage()) {
return false;
}
if (!ref.getPackage().getSimpleName().equals("java.lang")) {
if (classNamePresentInJavaLang(ref)) {
// Don't import class with names clashing with some classes present in java.lang,
// because it leads to undecidability and compilation errors. I. e. always leave
// com.mycompany.String fully-qualified.
return false;
}
}
imports.put(ref.getSimpleName(), ref);
return true;
}

private boolean classNamePresentInJavaLang(CtTypeReference<?> ref) {
Boolean presentInJavaLang = namesPresentInJavaLang.get(ref.getSimpleName());
if (presentInJavaLang == null) {
// The following procedure of determining if the handle is present in Java Lang or
// not produces "false positives" if the analyzed source complianceLevel is > 6.
// For example, it reports that FunctionalInterface is present in java.lang even
// for compliance levels 6, 7. But this is not considered a bad thing, in opposite,
// it makes generated code a little more compatible with future versions of Java.
if (namesPresentInJavaLang8.contains(ref.getSimpleName())
|| namesPresentInJavaLang9.contains(ref.getSimpleName())) {
presentInJavaLang = true;
} else {
// Assuming Spoon's own runtime environment is Java 7+
try {
Class.forName("java.lang." + ref.getSimpleName());
presentInJavaLang = true;
} catch (ClassNotFoundException e) {
presentInJavaLang = false;
}
}
namesPresentInJavaLang.put(ref.getSimpleName(), presentInJavaLang);
}
return presentInJavaLang;
}
}
2 changes: 1 addition & 1 deletion src/main/java/spoon/support/JavaOutputProcessor.java
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ public void createJavaFile(CtType<?> element) {

// create package directory
File packageDir;
if (pack.getQualifiedName().equals(CtPackage.TOP_LEVEL_PACKAGE_NAME)) {
if (pack.isUnnamedPackage()) {
packageDir = new File(directory.getAbsolutePath());
} else {
// Create current package dir
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -550,7 +550,7 @@ protected void generateProcessedSourceFilesUsingCUs() {

// create package directory
File packageDir;
if (pack.getQualifiedName().equals(CtPackage.TOP_LEVEL_PACKAGE_NAME)) {
if (pack.isUnnamedPackage()) {
packageDir = new File(outputDirectory.getAbsolutePath());
} else {
// Create current package directory
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ private void addVirtualJavaFile(List<CompilationUnit> unitList, CtType<?> ctType

// create package directory
File packageDir;
if (CtPackage.TOP_LEVEL_PACKAGE_NAME.equals(pack.getQualifiedName())) {
if (pack.isUnnamedPackage()) {
packageDir = new File(directory.getAbsolutePath());
} else {
packageDir = new File(directory.getAbsolutePath() + File.separatorChar + pack.getQualifiedName().replace('.', File.separatorChar));
Expand Down
Loading

0 comments on commit 540b260

Please sign in to comment.