Skip to content

Commit

Permalink
single-pass resolution
Browse files Browse the repository at this point in the history 
GROOVY-4386, GROOVY-5961, GROOVY-6977, GROOVY-9642

~GROOVY-4287, GROOVY-8947~
  • Loading branch information
@eric-milles
eric-milles committed Aug 21, 2021
1 parent f3e2092 commit e9b3d70
Show file tree
Hide file tree
Showing 8 changed files with 206 additions and 96 deletions.
36 changes: 29 additions & 7 deletions ...vy.core.tests.compiler/src/org/eclipse/jdt/groovy/core/tests/basic/GroovySimpleTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -788,19 +788,41 @@ public void testStaticProperty3() {
public void testStaticProperty4() {
//@formatter:off
String[] sources = {
"Super.groovy",
"class Super {\n" +
" def static getSql() { 'sql' }\n" +
"Script.groovy",
"abstract class A {\n" +
" static getB() { 'bee' }\n" +
"}\n" +
"class Sub extends Super {\n" +
" def static m() {\n" +
" sql.charAt(0)\n" +
"class C extends A {\n" +
" static m() {\n" +
" print b.charAt(0)\n" +
" }\n" +
"}\n" +
"new C().m()\n",
};
//@formatter:on

runConformTest(sources, "b");
}

@Test
public void testStaticProperty4a() {
//@formatter:off
String[] sources = {
"C.groovy",
"class C extends A {\n" +
" static main(args) {\n" +
" print b.charAt(0)\n" +
" }\n" +
"}\n",

"A.groovy",
"abstract class A {\n" +
" static getB() { 'bee' }\n" +
"}\n",
};
//@formatter:on

runNegativeTest(sources, "");
runConformTest(sources, "b");
}

@Test // GRECLIPSE-364
Expand Down
92 changes: 88 additions & 4 deletions ...oovy.core.tests.compiler/src/org/eclipse/jdt/groovy/core/tests/basic/InnerClassTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/
package org.eclipse.jdt.groovy.core.tests.basic;

import org.junit.Ignore;
import org.junit.Test;

public final class InnerClassTests extends GroovyCompilerTestSuite {
Expand Down Expand Up @@ -213,7 +214,7 @@ public void testInnerClass2() {
}

@Test
public void testInnerClass2a() {
public void testInnerClass3() {
//@formatter:off
String[] sources = {
"Outer.groovy",
Expand All @@ -234,7 +235,7 @@ public void testInnerClass2a() {
}

@Test
public void testInnerClass3() {
public void testInnerClass4() {
//@formatter:off
String[] sources = {
"WithInnerClass.groovy",
Expand All @@ -253,8 +254,28 @@ public void testInnerClass3() {
runNegativeTest(sources, "");
}

@Test // GROOVY-4287
public void testInnerClass5() {
//@formatter:off
String[] sources = {
"Script.groovy",
"import static p.Outer.Inner\n" +
"new Inner()\n",

"p/Outer.groovy",
"package p\n" +
"class Outer {\n" +
" static class Inner {\n" +
" }\n" +
"}\n",
};
//@formatter:on

runConformTest(sources);
}

@Test // https://github.com/groovy/groovy-eclipse/issues/708
public void testInnerClass4() {
public void testInnerClass6() {
//@formatter:off
String[] sources = {
"Script.groovy",
Expand All @@ -281,7 +302,51 @@ public void testInnerClass4() {
}

@Test
public void testInnerClass5() {
public void testInnerClass7() {
//@formatter:off
String[] sources = {
"Script.groovy",
"class Outer {\n" +
" @groovy.transform.TupleConstructor(defaults=false)\n" +
" class Inner {\n" +
" int p\n" +
" }\n" +
" static m(int n) {\n" +
" new Inner(new Outer(), n)\n" +
" }\n" +
"}\n" +
"print Outer.m(4).p\n" +
"print new Outer.Inner(new Outer(), 2).p\n",
};
//@formatter:on

runConformTest(sources, "42");
}

@Ignore @Test // GROOVY-8947
public void testInnerClass8() {
//@formatter:off
String[] sources = {
"Script.groovy",
"class Outer {\n" +
" @groovy.transform.TupleConstructor(defaults=false)\n" +
" class Inner {\n" +
" int p\n" +
" }\n" +
" static m(int n) {\n" +
" new Outer().(new Inner(n))\n" + // TODO: no parens
" }\n" +
"}\n" +
"print Outer.m(4).p\n" +
"print new Outer().(new Outer.Inner(2)).p\n", // TODO: no parens, no qualifier
};
//@formatter:on

runConformTest(sources, "42");
}

@Test
public void testInnerClass9() {
//@formatter:off
String[] sources = {
"Script.groovy",
Expand Down Expand Up @@ -1595,6 +1660,25 @@ public void testAnonymousInnerClass32() {
runConformTest(sources);
}

@Test // GROOVY-6977
public void testAnonymousInnerClass33() {
//@formatter:off
String[] sources = {
"Script.groovy",
"class C {\n" +
" def <T> List<T> foo() {\n" +
" new ArrayList<T>() {}\n" +
" }\n" +
"}\n" +
"def longList = new C().<Long>foo()\n" +
"assert longList != null\n" +
"assert longList.empty\n",
};
//@formatter:on

runConformTest(sources);
}

@Test
public void testMixedModeInnerProperties_GRE597() {
//@formatter:off
Expand Down
8 changes: 4 additions & 4 deletions base/org.codehaus.groovy25/src/org/codehaus/groovy/control/CompilationUnit.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@
import org.codehaus.groovy.classgen.GeneratorContext;
import org.codehaus.groovy.classgen.InnerClassCompletionVisitor;
import org.codehaus.groovy.classgen.InnerClassVisitor;
import org.codehaus.groovy.classgen.VariableScopeVisitor;
import org.codehaus.groovy.classgen.Verifier;
import org.codehaus.groovy.control.customizers.CompilationCustomizer;
import org.codehaus.groovy.control.io.InputStreamReaderSource;
Expand Down Expand Up @@ -582,12 +581,12 @@ public void compile(int throughPhase) throws CompilationFailedException {
throughPhase = Math.min(throughPhase, Phases.ALL);

while (throughPhase >= phase && phase <= Phases.ALL) {

/* GRECLIPSE edit -- GROOVY-4386, et al.
if (phase == Phases.SEMANTIC_ANALYSIS) {
doPhaseOperation(resolve);
if (dequeued()) continue;
}

*/
processPhaseOperations(phase);
// Grab processing may have brought in new AST transforms into various phases, process them as well
processNewPhaseOperations(phase);
Expand Down Expand Up @@ -689,9 +688,10 @@ protected boolean dequeued() throws CompilationFailedException {
public void call(SourceUnit source) throws CompilationFailedException {
List<ClassNode> classes = source.ast.getClasses();
for (ClassNode node : classes) {
/* GRECLIPSE edit -- GROOVY-4386, et al.
VariableScopeVisitor scopeVisitor = new VariableScopeVisitor(source);
scopeVisitor.visitClass(node);

*/
resolveVisitor.setClassNodeResolver(classNodeResolver);
resolveVisitor.startResolving(node, source);
}
Expand Down
23 changes: 22 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 @@ -61,6 +61,7 @@
import org.codehaus.groovy.ast.stmt.CatchStatement;
import org.codehaus.groovy.ast.stmt.ForStatement;
import org.codehaus.groovy.ast.stmt.Statement;
import org.codehaus.groovy.classgen.VariableScopeVisitor;
import org.codehaus.groovy.control.ClassNodeResolver.LookupResult;
import org.codehaus.groovy.syntax.Types;
import org.codehaus.groovy.transform.trait.Traits;
Expand All @@ -70,6 +71,7 @@
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Iterator;
import java.util.LinkedHashMap;
import java.util.LinkedList;
import java.util.List;
Expand Down Expand Up @@ -1330,10 +1332,15 @@ protected Expression transformClosureExpression(ClosureExpression ce) {
}

protected Expression transformConstructorCallExpression(ConstructorCallExpression cce) {
/* GRECLIPSE edit -- GROOVY-4386, et al.
ClassNode type = cce.getType();
if (cce.isUsingAnonymousInnerClass()) { // GROOVY-9642
resolveOrFail(type.getUnresolvedSuperClass(false), type);
} else {
*/
if (!cce.isUsingAnonymousInnerClass()) {
ClassNode type = cce.getType();
// GRECLIPSE end
resolveOrFail(type, cce);
if (type.isAbstract()) {
addError("You cannot create an instance from the abstract " + getDescription(type) + ".", cce);
Expand Down Expand Up @@ -1506,14 +1513,15 @@ public void visitClass(ClassNode node) {
if (Modifier.isStatic(node.getModifiers())) {
genericParameterNames = new HashMap<GenericsTypeName, GenericsType>();
}

/* GRECLIPSE edit -- GROOVY-4386, et al.
InnerClassNode innerClassNode = (InnerClassNode) node;
if (innerClassNode.isAnonymous()) {
MethodNode enclosingMethod = innerClassNode.getEnclosingMethod();
if (null != enclosingMethod) {
resolveGenericsHeader(enclosingMethod.getGenericsTypes());
}
}
*/
} else {
genericParameterNames = new HashMap<GenericsTypeName, GenericsType>();
}
Expand Down Expand Up @@ -1609,6 +1617,19 @@ public void visitClass(ClassNode node) {
}
}
}
// VariableScopeVisitor visits anon. inner class body inline, so resolve now
for (Iterator<InnerClassNode> it = node.getInnerClasses(); it.hasNext(); ) {
InnerClassNode cn = it.next();
if (cn.isAnonymous()) {
MethodNode enclosingMethod = cn.getEnclosingMethod();
if (enclosingMethod != null) {
resolveGenericsHeader(enclosingMethod.getGenericsTypes()); // GROOVY-6977
}
resolveOrFail(cn.getUnresolvedSuperClass(false), cn); // GROOVY-9642
}
}
// initialize scopes/variables now that imports and super types are resolved
new VariableScopeVisitor(source).visitClass(node);
// GRECLIPSE end
super.visitClass(node);

Expand Down
22 changes: 9 additions & 13 deletions base/org.codehaus.groovy30/src/org/codehaus/groovy/control/CompilationUnit.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@
import org.codehaus.groovy.classgen.GeneratorContext;
import org.codehaus.groovy.classgen.InnerClassCompletionVisitor;
import org.codehaus.groovy.classgen.InnerClassVisitor;
import org.codehaus.groovy.classgen.VariableScopeVisitor;
import org.codehaus.groovy.classgen.Verifier;
import org.codehaus.groovy.control.customizers.CompilationCustomizer;
import org.codehaus.groovy.control.io.InputStreamReaderSource;
Expand Down Expand Up @@ -629,25 +628,21 @@ public void compile(int throughPhase) throws CompilationFailedException {
throughPhase = Math.min(throughPhase, Phases.ALL);

while (throughPhase >= phase && phase <= Phases.ALL) {
// GRECLIPSE add
if (phase == Phases.CONVERSION) {
if (sources.size() > 1 && Boolean.TRUE.equals(configuration.getOptimizationOptions().get(CompilerConfiguration.PARALLEL_PARSE))) {
sources.values().parallelStream().forEach(SourceUnit::buildAST);
} else {
sources.values().forEach(SourceUnit::buildAST);
}
} else
// GRECLIPSE end
/* GRECLIPSE edit -- GROOVY-4386, et al.
if (phase == Phases.SEMANTIC_ANALYSIS) {
resolve.doPhaseOperation(this);
if (dequeued()) continue;
}
/* GRECLIPSE edit
if (phase == Phases.CONVERSION) {
buildASTs();
}
*/

if (phase == Phases.CONVERSION) {
for (SourceUnit source : sources.values()) {
source.buildAST();
}
}
// GRECLIPSE end
processPhaseOperations(phase);
// Grab processing may have brought in new AST transforms into various phases, process them as well
processNewPhaseOperations(phase);
Expand Down Expand Up @@ -749,9 +744,10 @@ protected boolean dequeued() throws CompilationFailedException {
*/
private final ISourceUnitOperation resolve = (final SourceUnit source) -> {
for (ClassNode classNode : source.getAST().getClasses()) {
/* GRECLIPSE edit -- GROOVY-4386, et al.
GroovyClassVisitor visitor = new VariableScopeVisitor(source);
visitor.visitClass(classNode);

*/
resolveVisitor.setClassNodeResolver(classNodeResolver);
resolveVisitor.startResolving(classNode, source);
}
Expand Down
Loading

0 comments on commit e9b3d70

Please sign in to comment.