You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by em...@apache.org on 2022/03/08 21:09:59 UTC
[groovy] 01/02: GROOVY-9866, GROOVY-10466: resolve all class headers before class bodies
This is an automated email from the ASF dual-hosted git repository.
emilles pushed a commit to branch GROOVY_4_0_X
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit c838a1dd43a4493f6082dd332cda7232e1de362c
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Fri Mar 4 18:16:04 2022 -0600
GROOVY-9866, GROOVY-10466: resolve all class headers before class bodies
---
.../codehaus/groovy/control/CompilationUnit.java | 22 ++++++-
.../codehaus/groovy/control/ResolveVisitor.java | 70 ++++++++++++----------
src/test/groovy/bugs/Groovy10466.groovy | 65 ++++++++++++++++++++
src/test/groovy/script/RuntimeResolveTests.groovy | 3 +-
4 files changed, 122 insertions(+), 38 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/control/CompilationUnit.java b/src/main/java/org/codehaus/groovy/control/CompilationUnit.java
index a619a2f..0780a21 100644
--- a/src/main/java/org/codehaus/groovy/control/CompilationUnit.java
+++ b/src/main/java/org/codehaus/groovy/control/CompilationUnit.java
@@ -199,9 +199,25 @@ public class CompilationUnit extends ProcessingUnit {
}, Phases.CONVERSION);
addPhaseOperation(source -> {
- resolveVisitor.setClassNodeResolver(classNodeResolver);
- for (ClassNode classNode : source.getAST().getClasses()) {
- resolveVisitor.startResolving(classNode, source);
+ try {
+ resolveVisitor.phase = 1; // resolve head of each class
+ resolveVisitor.setClassNodeResolver(classNodeResolver);
+ for (ClassNode classNode : source.getAST().getClasses()) {
+ resolveVisitor.startResolving(classNode, source);
+ }
+ } finally {
+ resolveVisitor.phase = 0;
+ }
+ }, Phases.SEMANTIC_ANALYSIS);
+
+ addPhaseOperation(source -> {
+ try {
+ resolveVisitor.phase = 2; // resolve body of each class
+ for (ClassNode classNode : source.getAST().getClasses()) {
+ resolveVisitor.startResolving(classNode, source);
+ }
+ } finally {
+ resolveVisitor.phase = 0;
}
}, Phases.SEMANTIC_ANALYSIS);
diff --git a/src/main/java/org/codehaus/groovy/control/ResolveVisitor.java b/src/main/java/org/codehaus/groovy/control/ResolveVisitor.java
index 8780da2..e639d26 100644
--- a/src/main/java/org/codehaus/groovy/control/ResolveVisitor.java
+++ b/src/main/java/org/codehaus/groovy/control/ResolveVisitor.java
@@ -113,6 +113,7 @@ public class ResolveVisitor extends ClassCodeExpressionTransformer {
private boolean checkingVariableTypeInDeclaration;
private boolean inClosure, inPropertyExpression;
private boolean isTopLevelProperty = true;
+ /*package*/ int phase; // sub-divide visit
/**
* A ConstructedNestedClass consists of an outer class and a name part, denoting a
@@ -423,7 +424,6 @@ public class ResolveVisitor extends ClassCodeExpressionTransformer {
// GROOVY-4043: for type "X", try "A$X" with each type in the class hierarchy (except for Object)
for (; cn != null && cycleCheck.add(cn) && !isObjectType(cn); cn = cn.getSuperClass()) {
if (setRedirect(type, cn)) return true;
- // GROOVY-9866: unresolvable interfaces
}
// Another case we want to check here is if we are in a
@@ -1316,46 +1316,50 @@ public class ResolveVisitor extends ClassCodeExpressionTransformer {
genericParameterNames = new HashMap<>();
}
resolveGenericsHeader(node.getGenericsTypes());
+ switch (phase) { // GROOVY-9866, GROOVY-10466
+ case 0:
+ case 1:
+ ClassNode sn = node.getUnresolvedSuperClass();
+ if (sn != null) {
+ resolveOrFail(sn, "", node, true);
+ }
+ for (ClassNode in : node.getInterfaces()) {
+ resolveOrFail(in, "", node, true);
+ }
- ClassNode sn = node.getUnresolvedSuperClass();
- if (sn != null) {
- resolveOrFail(sn, "", node, true);
- }
- for (ClassNode in : node.getInterfaces()) {
- resolveOrFail(in, "", node, true);
- }
-
- if (sn != null) checkCyclicInheritance(node, sn);
- for (ClassNode in : node.getInterfaces()) {
- checkCyclicInheritance(node, in);
- }
- if (node.getGenericsTypes() != null) {
- for (GenericsType gt : node.getGenericsTypes()) {
- if (gt != null && gt.getUpperBounds() != null) {
- for (ClassNode variant : gt.getUpperBounds()) {
- if (variant.isGenericsPlaceHolder()) checkCyclicInheritance(variant, gt.getType());
+ if (sn != null) checkCyclicInheritance(node, sn);
+ for (ClassNode in : node.getInterfaces()) {
+ checkCyclicInheritance(node, in);
+ }
+ if (node.getGenericsTypes() != null) {
+ for (GenericsType gt : node.getGenericsTypes()) {
+ if (gt != null && gt.getUpperBounds() != null) {
+ for (ClassNode variant : gt.getUpperBounds()) {
+ if (variant.isGenericsPlaceHolder()) checkCyclicInheritance(variant, gt.getType());
+ }
}
}
}
- }
-
- // 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
+ case 2:
+ // 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
}
- resolveOrFail(cn.getUnresolvedSuperClass(false), cn); // GROOVY-9642
}
- }
- // initialize scopes/variables now that imports and super types are resolved
- new VariableScopeVisitor(source).visitClass(node);
+ if (phase == 1) break; // resolve other class headers before members, et al.
- visitTypeAnnotations(node);
- super.visitClass(node);
+ // initialize scopes/variables now that imports and super types are resolved
+ new VariableScopeVisitor(source).visitClass(node);
+ visitTypeAnnotations(node);
+ super.visitClass(node);
+ }
currentClass = oldNode;
}
diff --git a/src/test/groovy/bugs/Groovy10466.groovy b/src/test/groovy/bugs/Groovy10466.groovy
new file mode 100644
index 0000000..704682c
--- /dev/null
+++ b/src/test/groovy/bugs/Groovy10466.groovy
@@ -0,0 +1,65 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package groovy.bugs
+
+import org.codehaus.groovy.control.CompilationUnit
+import org.codehaus.groovy.control.CompilerConfiguration
+import org.junit.Test
+
+class Groovy10466 {
+
+ protected field
+
+ @Test
+ void testClassOrder() {
+ def config = new CompilerConfiguration(targetDirectory: File.createTempDir())
+
+ def parentDir = File.createTempDir()
+ try {
+ def a = new File(parentDir, 'A.groovy')
+ a.write '''
+ import org.codehaus.groovy.ast.FieldNode
+
+ class A extends B {
+ @groovy.transform.ASTTest(value={
+ def mce = node.code.statements[0].expression
+ def var = mce.arguments[0] // "field" variable
+ assert var.accessedVariable instanceof FieldNode
+ })
+ void test() {
+ print field
+ }
+ }
+ '''
+ def b = new File(parentDir, 'B.groovy')
+ b.write '''
+ class B extends groovy.bugs.Groovy10466 {
+ }
+ '''
+
+ def loader = new GroovyClassLoader(this.class.classLoader)
+ def cu = new CompilationUnit(config, null, loader)
+ cu.addSources(a, b)
+ cu.compile()
+ } finally {
+ config.targetDirectory.deleteDir()
+ parentDir.deleteDir()
+ }
+ }
+}
diff --git a/src/test/groovy/script/RuntimeResolveTests.groovy b/src/test/groovy/script/RuntimeResolveTests.groovy
index 61462bc..32acab6 100644
--- a/src/test/groovy/script/RuntimeResolveTests.groovy
+++ b/src/test/groovy/script/RuntimeResolveTests.groovy
@@ -18,7 +18,6 @@
*/
package groovy.script
-import org.junit.Ignore
import org.junit.Test
import static groovy.test.GroovyAssert.isAtLeastJdk
@@ -67,7 +66,7 @@ final class RuntimeResolveTests {
runScript('/groovy/bugs/groovy9243/Main.groovy')
}
- @Test @Ignore
+ @Test
void testResolveOuterMemberWithoutAnImport3() {
assumeTrue(isAtLeastJdk('9.0')) // System.Logger
runScript('/groovy/bugs/groovy9866/Main.groovy')