Skip to content

Commit

Permalink
Fix for #724: remove missing type bindings when resolving star imports
Browse files Browse the repository at this point in the history 
This prevents LookupEnvironment.createPackage from returning null
because a package was tried as a type and a missing binding was stored
  • Loading branch information
@eric-milles
eric-milles committed Sep 20, 2018
1 parent 079250a commit c423580
Show file tree
Hide file tree
Showing 8 changed files with 1,665 additions and 1,630 deletions.
1,621 changes: 80 additions & 1,541 deletions ...vy.core.tests.compiler/src/org/eclipse/jdt/groovy/core/tests/basic/GroovySimpleTests.java
View file

Large diffs are not rendered by default.

1,528 changes: 1,528 additions & 0 deletions ....groovy.core.tests.compiler/src/org/eclipse/jdt/groovy/core/tests/basic/ImportsTests.java
View file

Large diffs are not rendered by default.

5 changes: 4 additions & 1 deletion base/org.codehaus.groovy24/src/org/codehaus/groovy/control/ResolveVisitor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,8 @@ public class ResolveVisitor extends ClassCodeExpressionTransformer {
private final Set<String> resolutionFailedCache = new HashSet<String>();
// GRECLIPSE end
private boolean checkingVariableTypeInDeclaration = false;
private ImportNode currImportNode = null;
// GRECLIPSE private->protected
protected ImportNode currImportNode;
private MethodNode currentMethod;
private ClassNodeResolver classNodeResolver;

Expand Down Expand Up @@ -1439,6 +1440,7 @@ public void visitClass(ClassNode node) {
*/
for (ImportNode importNode : module.getStarImports()) {
if (importNode.getEnd() > 0) {
currImportNode = importNode;
String importName = importNode.getPackageName();
importName = importName.substring(0, importName.length()-1);
ClassNode type = ClassHelper.makeWithoutCaching(importName);
Expand All @@ -1447,6 +1449,7 @@ public void visitClass(ClassNode node) {
type.setStart(importNode.getStart() + 7);
type.setEnd(type.getStart() + importName.length());
}
currImportNode = null;
}
}
// GRECLIPSE end
Expand Down
5 changes: 4 additions & 1 deletion base/org.codehaus.groovy25/src/org/codehaus/groovy/control/ResolveVisitor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,8 @@ public class ResolveVisitor extends ClassCodeExpressionTransformer {
private final Set<String> resolutionFailedCache = new HashSet<String>();
// GRECLIPSE end
private boolean checkingVariableTypeInDeclaration = false;
private ImportNode currImportNode = null;
// GRECLIPSE private->protected
protected ImportNode currImportNode;
private MethodNode currentMethod;
private ClassNodeResolver classNodeResolver;

Expand Down Expand Up @@ -1505,6 +1506,7 @@ public void visitClass(ClassNode node) {
*/
for (ImportNode importNode : module.getStarImports()) {
if (importNode.getEnd() > 0) {
currImportNode = importNode;
String importName = importNode.getPackageName();
importName = importName.substring(0, importName.length()-1);
ClassNode type = ClassHelper.makeWithoutCaching(importName);
Expand All @@ -1513,6 +1515,7 @@ public void visitClass(ClassNode node) {
type.setStart(importNode.getStart() + 7);
type.setEnd(type.getStart() + importName.length());
}
currImportNode = null;
}
}
// GRECLIPSE end
Expand Down
5 changes: 4 additions & 1 deletion base/org.codehaus.groovy26/src/org/codehaus/groovy/control/ResolveVisitor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,8 @@ public class ResolveVisitor extends ClassCodeExpressionTransformer {
private final Set<String> resolutionFailedCache = new HashSet<String>();
// GRECLIPSE end
private boolean checkingVariableTypeInDeclaration = false;
private ImportNode currImportNode = null;
// GRECLIPSE private->protected
protected ImportNode currImportNode;
private MethodNode currentMethod;
private ClassNodeResolver classNodeResolver;

Expand Down Expand Up @@ -1504,6 +1505,7 @@ public void visitClass(ClassNode node) {
*/
for (ImportNode importNode : module.getStarImports()) {
if (importNode.getEnd() > 0) {
currImportNode = importNode;
String importName = importNode.getPackageName();
importName = importName.substring(0, importName.length()-1);
ClassNode type = ClassHelper.makeWithoutCaching(importName);
Expand All @@ -1512,6 +1514,7 @@ public void visitClass(ClassNode node) {
type.setStart(importNode.getStart() + 7);
type.setEnd(type.getStart() + importName.length());
}
currImportNode = null;
}
}
// GRECLIPSE end
Expand Down
53 changes: 0 additions & 53 deletions ...vy.core/src/org/codehaus/jdt/groovy/internal/compiler/ast/GroovyCompilationUnitScope.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,13 @@
import java.util.Collections;
import java.util.List;

import org.codehaus.groovy.ast.ClassNode;
import org.eclipse.jdt.core.compiler.CharOperation;
import org.eclipse.jdt.groovy.core.util.ArrayUtils;
import org.eclipse.jdt.internal.compiler.ast.CompilationUnitDeclaration;
import org.eclipse.jdt.internal.compiler.ast.ImportReference;
import org.eclipse.jdt.internal.compiler.ast.TypeDeclaration;
import org.eclipse.jdt.internal.compiler.ast.TypeReference;
import org.eclipse.jdt.internal.compiler.env.AccessRestriction;
import org.eclipse.jdt.internal.compiler.lookup.BinaryTypeBinding;
import org.eclipse.jdt.internal.compiler.lookup.Binding;
import org.eclipse.jdt.internal.compiler.lookup.ClassScope;
import org.eclipse.jdt.internal.compiler.lookup.CompilationUnitScope;
Expand All @@ -41,8 +39,6 @@
import org.eclipse.jdt.internal.compiler.lookup.SourceTypeBinding;
import org.eclipse.jdt.internal.compiler.lookup.TypeBinding;
import org.eclipse.jdt.internal.compiler.lookup.TypeConstants;
import org.eclipse.jdt.internal.compiler.problem.AbortCompilation;
import org.eclipse.jdt.internal.core.builder.AbortIncrementalBuildException;
import org.eclipse.jdt.internal.core.builder.NameEnvironment;

/**
Expand Down Expand Up @@ -147,55 +143,6 @@ public void augmentTypeHierarchy() {
}
}

public ClassNode lookupClassNodeForBinary(String typeName, JDTResolver jdtResolver) {
char[][] compoundName = CharOperation.splitOn('.', typeName.toCharArray());
TypeBinding jdtBinding = getType(compoundName, compoundName.length);

if (jdtBinding instanceof ProblemReferenceBinding) {
ProblemReferenceBinding prBinding = (ProblemReferenceBinding) jdtBinding;
if (prBinding.problemId() == ProblemReasons.InternalNameProvided) {
jdtBinding = prBinding.closestMatch();
}
}

if (jdtBinding instanceof BinaryTypeBinding) {
return jdtResolver.convertToClassNode(jdtBinding);
}

return null;
}

/*
* Not quite the right name for this method, because on an incremental build
* it will find BinaryTypeBindings for types that were SourceTypeBindings
* during the full build.
*/
public ClassNode lookupClassNodeForSource(String typeName, JDTResolver jdtResolver) {
char[][] compoundName = CharOperation.splitOn('.', typeName.toCharArray());
TypeBinding jdtBinding = null;
try {
jdtBinding = getType(compoundName, compoundName.length);
} catch (AbortCompilation t) {
if (!(t.silentException instanceof AbortIncrementalBuildException)) {
throw t;
}
}

if (jdtBinding instanceof ProblemReferenceBinding) {
ProblemReferenceBinding prBinding = (ProblemReferenceBinding) jdtBinding;
if (prBinding.problemId() == ProblemReasons.InternalNameProvided) {
jdtBinding = prBinding.closestMatch();
}
}

if ((jdtBinding instanceof SourceTypeBinding || jdtBinding instanceof BinaryTypeBinding) &&
(CharOperation.equals(compoundName, ((ReferenceBinding) jdtBinding).compoundName) || typeName.equals(jdtBinding.debugName()))) {
return jdtResolver.convertToClassNode(jdtBinding);
}

return null;
}

@Override
protected void checkPublicTypeNameMatchesFilename(TypeDeclaration typeDecl) {
}
Expand Down
77 changes: 44 additions & 33 deletions ...clipse.jdt.groovy.core/src/org/codehaus/jdt/groovy/internal/compiler/ast/JDTResolver.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,6 @@
import java.util.Map;
import java.util.Set;

import groovy.lang.GroovyClassLoader;

import org.codehaus.groovy.ast.ClassHelper;
import org.codehaus.groovy.ast.ClassNode;
import org.codehaus.groovy.control.CompilationFailedException;
Expand All @@ -39,8 +37,16 @@
import org.eclipse.jdt.internal.compiler.ast.SingleTypeReference;
import org.eclipse.jdt.internal.compiler.ast.TypeDeclaration;
import org.eclipse.jdt.internal.compiler.env.AccessRestriction;
import org.eclipse.jdt.internal.compiler.lookup.BinaryTypeBinding;
import org.eclipse.jdt.internal.compiler.lookup.MissingTypeBinding;
import org.eclipse.jdt.internal.compiler.lookup.ProblemReasons;
import org.eclipse.jdt.internal.compiler.lookup.ProblemReferenceBinding;
import org.eclipse.jdt.internal.compiler.lookup.ReferenceBinding;
import org.eclipse.jdt.internal.compiler.lookup.SourceTypeBinding;
import org.eclipse.jdt.internal.compiler.lookup.TypeBinding;
import org.eclipse.jdt.internal.compiler.lookup.TypeIds;
import org.eclipse.jdt.internal.compiler.problem.AbortCompilation;
import org.eclipse.jdt.internal.core.builder.AbortIncrementalBuildException;

/**
* An extension to the standard groovy ResolveVisitor that can ask JDT for types when groovy cannot find them. A groovy project in
Expand Down Expand Up @@ -329,21 +335,7 @@ protected boolean resolveFromCompileUnit(ClassNode type) {
if (DEBUG) {
log("resolveFromCompileUnit", type, foundit);
}
if (foundit) {
return true;
}
if (activeScope != null) {
// Ask JDT for a source file, visible from this scope
ClassNode node = activeScope.lookupClassNodeForSource(type.getName(), this);
if (DEBUG) {
log("resolveFromCompileUnit (jdt) ", type, node != null);
}
if (node != null) {
type.setRedirect(node);
return true;
}
}
return false;
return foundit;
}

@Override
Expand All @@ -368,9 +360,34 @@ protected boolean resolveFromStaticInnerClasses(ClassNode type, boolean testStat

@Override
protected boolean resolveToOuter(ClassNode type) {
ClassNode node;
if (activeScope != null) {
node = activeScope.lookupClassNodeForBinary(type.getName(), this);
// ask the JDT for a binary or source type, visible from this scope
/*node = activeScope.lookupClassNodeForBinary(type.getName(), this);*/
char[][] compoundName = CharOperation.splitOn('.', type.getName().toCharArray());
TypeBinding jdtBinding = null;
try {
jdtBinding = activeScope.getType(compoundName, compoundName.length); // TODO: Use getImport(char[][], boolean, boolean) in some cases?
} catch (AbortCompilation t) {
if (!(t.silentException instanceof AbortIncrementalBuildException)) {
throw t;
}
}
if (jdtBinding instanceof ProblemReferenceBinding) {
ProblemReferenceBinding prBinding = (ProblemReferenceBinding) jdtBinding;
if (prBinding.problemId() == ProblemReasons.InternalNameProvided) {
jdtBinding = prBinding.closestMatch();
} else if (prBinding.problemId() == ProblemReasons.NotFound &&
prBinding.closestMatch() instanceof MissingTypeBinding && currImportNode != null && currImportNode.isStar()) {
MissingTypeBinding mtBinding = (MissingTypeBinding) prBinding.closestMatch();
mtBinding.fPackage.knownTypes.put(compoundName[compoundName.length - 1], null);
}
}

ClassNode node = null;
if ((jdtBinding instanceof BinaryTypeBinding || jdtBinding instanceof SourceTypeBinding) &&
(CharOperation.equals(compoundName, ((ReferenceBinding) jdtBinding).compoundName) || type.getName().equals(jdtBinding.debugName()))) {
node = convertToClassNode(jdtBinding);
}
if (DEBUG) {
log("resolveToOuter (jdt)", type, node != null);
}
Expand All @@ -379,23 +396,17 @@ protected boolean resolveToOuter(ClassNode type) {
return true;
}
}

// Rudimentary grab support - if the compilation unit has our special classloader and as grab has occurred, try and find the class through it
GroovyClassLoader loader = compilationUnit.getClassLoader();
if (loader instanceof GrapeAwareGroovyClassLoader) {
GrapeAwareGroovyClassLoader gagcl = (GrapeAwareGroovyClassLoader) loader;
if (gagcl.grabbed) {
Class<?> cls;
if (compilationUnit.getClassLoader() instanceof GrapeAwareGroovyClassLoader) {
GrapeAwareGroovyClassLoader loader = (GrapeAwareGroovyClassLoader) compilationUnit.getClassLoader();
if (loader.grabbed) {
try {
cls = loader.loadClass(type.getName(), false, true);
} catch (ClassNotFoundException | CompilationFailedException e) {
return false;
}
if (cls == null) {
return false;
Class<?> cls = loader.loadClass(type.getName(), false, true);
type.setRedirect(ClassHelper.make(cls));
return true;
} catch (ClassNotFoundException | CompilationFailedException ignore) {
}
node = ClassHelper.make(cls);
type.setRedirect(node);
return true;
}
}
return false;
Expand Down
1 change: 1 addition & 0 deletions ide-test/org.codehaus.groovy.alltests/src/org/codehaus/groovy/alltests/GroovyJDTTests.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import org.junit.runners.Suite
org.eclipse.jdt.groovy.core.tests.basic.GenericsTests,
org.eclipse.jdt.groovy.core.tests.basic.GroovySimpleTests,
org.eclipse.jdt.groovy.core.tests.basic.GroovySimpleTests_Compliance_1_8,
org.eclipse.jdt.groovy.core.tests.basic.ImportsTests,
org.eclipse.jdt.groovy.core.tests.basic.TraitsTests,

// Xform tests
Expand Down

0 comments on commit c423580

Please sign in to comment.