This is an automated email from the ASF dual-hosted git repository.
emilles pushed a commit to branch GROOVY-11719
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit f54031dcfda0bfe0018687e16bc2a315967d0b05
Author: Eric Milles <eric.mil...@thomsonreuters.com>
AuthorDate: Tue Jul 22 09:23:05 2025 -0500
GROOVY-9526, GROOVY-11719: pop field state on exception
---
.../codehaus/groovy/control/ResolveVisitor.java | 258 +++++++++++----------
.../util/GroovyScriptEngineReloadingTest.groovy | 64 +++--
2 files changed, 183 insertions(+), 139 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/control/ResolveVisitor.java
b/src/main/java/org/codehaus/groovy/control/ResolveVisitor.java
index f8bbf25e17..8e3a08716f 100644
--- a/src/main/java/org/codehaus/groovy/control/ResolveVisitor.java
+++ b/src/main/java/org/codehaus/groovy/control/ResolveVisitor.java
@@ -271,14 +271,15 @@ public class ResolveVisitor extends
ClassCodeExpressionTransformer {
if (!canSeeTypeVars(node.getModifiers(), node.getDeclaringClass())) {
genericParameterNames = Collections.emptyMap();
}
-
- if (!fieldTypesChecked.contains(node)) {
- ClassNode t = node.getOriginType();
- resolveOrFail(t, t);
+ try {
+ if (!fieldTypesChecked.contains(node)) {
+ ClassNode t = node.getOriginType();
+ resolveOrFail(t, t);
+ }
+ super.visitField(node);
+ } finally {
+ genericParameterNames = oldNames;
}
- super.visitField(node);
-
- genericParameterNames = oldNames;
}
@Override
@@ -287,14 +288,15 @@ public class ResolveVisitor extends
ClassCodeExpressionTransformer {
if (!canSeeTypeVars(node.getModifiers(), node.getDeclaringClass())) {
genericParameterNames = Collections.emptyMap();
}
+ try {
+ ClassNode t = node.getOriginType();
+ resolveOrFail(t, t);
+ fieldTypesChecked.add(node.getField());
- ClassNode t = node.getOriginType();
- resolveOrFail(t, t);
- fieldTypesChecked.add(node.getField());
-
- super.visitProperty(node);
-
- genericParameterNames = oldNames;
+ super.visitProperty(node);
+ } finally {
+ genericParameterNames = oldNames;
+ }
}
private static boolean canSeeTypeVars(final int mods, final ClassNode
node) {
@@ -303,39 +305,39 @@ public class ResolveVisitor extends
ClassCodeExpressionTransformer {
@Override
protected void visitConstructorOrMethod(final MethodNode node, final
boolean isConstructor) {
+ MethodNode oldMethod = currentMethod;
VariableScope oldScope = currentScope;
currentScope = node.getVariableScope();
Map<GenericsTypeName, GenericsType> oldNames = genericParameterNames;
genericParameterNames =
canSeeTypeVars(node.getModifiers(), node.getDeclaringClass())
? new HashMap<>(genericParameterNames) : new HashMap<>();
+ try {
+ resolveGenericsHeader(node.getGenericsTypes());
- resolveGenericsHeader(node.getGenericsTypes());
-
- {
- ClassNode t = node.getReturnType();
- resolveOrFail(t, t);
- }
- for (Parameter p : node.getParameters()) {
- p.setInitialExpression(transform(p.getInitialExpression()));
- ClassNode t = p.getOriginType();
- resolveOrFail(t, t);
- visitAnnotations(p);
- }
- if (node.getExceptions() != null) {
- for (ClassNode t : node.getExceptions()) {
+ {
+ ClassNode t = node.getReturnType();
resolveOrFail(t, t);
}
- }
-
- MethodNode oldCurrentMethod = currentMethod;
- currentMethod = node;
-
- super.visitConstructorOrMethod(node, isConstructor);
+ for (Parameter p : node.getParameters()) {
+ p.setInitialExpression(transform(p.getInitialExpression()));
+ ClassNode t = p.getOriginType();
+ resolveOrFail(t, t);
+ visitAnnotations(p);
+ }
+ if (node.getExceptions() != null) {
+ for (ClassNode t : node.getExceptions()) {
+ resolveOrFail(t, t);
+ }
+ }
- currentMethod = oldCurrentMethod;
- genericParameterNames = oldNames;
- currentScope = oldScope;
+ currentMethod = node;
+ super.visitConstructorOrMethod(node, isConstructor);
+ } finally {
+ genericParameterNames = oldNames;
+ currentMethod = oldMethod;
+ currentScope = oldScope;
+ }
}
private void resolveOrFail(final ClassNode type, final ASTNode node) {
@@ -1249,107 +1251,112 @@ public class ResolveVisitor extends
ClassCodeExpressionTransformer {
@Override
public void visitClass(final ClassNode node) {
- ClassNode oldNode = currentClass;
- currentClass = node;
-
- ModuleNode module = node.getModule();
- if (!module.hasImportsResolved()) {
- for (ImportNode importNode : module.getImports()) {
- currentImport = importNode;
- ClassNode type = importNode.getType();
- if (resolve(type, false, false, true)) {
- currentImport = null;
- continue;
- }
- currentImport = null;
- addError("unable to resolve class " + type.getName(), type);
- }
- for (ImportNode importNode : module.getStarImports()) {
- if (importNode.getLineNumber() > 0) {
+ ClassNode oldNode = currentClass; currentClass = node;
+ Map<GenericsTypeName, GenericsType> outerNames = null;
+ try {
+ ModuleNode module = node.getModule();
+ if (!module.hasImportsResolved()) {
+ for (ImportNode importNode : module.getImports()) {
currentImport = importNode;
- String importName = importNode.getPackageName();
- importName = importName.substring(0,
importName.length()-1);
- ClassNode type =
ClassHelper.makeWithoutCaching(importName);
- if (resolve(type, false, false, true)) {
- importNode.setType(type);
+ try {
+ ClassNode type = importNode.getType();
+ if (!resolve(type, false, false, true)) {
+ addError("unable to resolve class " +
type.getName(), type);
+ }
+ } finally {
+ currentImport = null;
}
- currentImport = null;
}
- }
- for (ImportNode importNode : module.getStaticImports().values()) {
- ClassNode type = importNode.getType();
- if (!resolve(type, true, true, true))
- addError("unable to resolve class " + type.getName(),
type);
- }
- for (ImportNode importNode :
module.getStaticStarImports().values()) {
- ClassNode type = importNode.getType();
- if (!resolve(type, true, true, true))
- addError("unable to resolve class " + type.getName(),
type);
- }
+ for (ImportNode importNode : module.getStarImports()) {
+ if (importNode.getLineNumber() > 0) {
+ currentImport = importNode;
+ try {
+ String importName = importNode.getPackageName();
+ importName = importName.substring(0,
importName.length()-1);
+ ClassNode type =
ClassHelper.makeWithoutCaching(importName);
+ if (resolve(type, false, false, true)) {
+ importNode.setType(type);
+ }
+ } finally {
+ currentImport = null;
+ }
+ }
+ }
+ for (ImportNode importNode :
module.getStaticImports().values()) {
+ ClassNode type = importNode.getType();
+ if (!resolve(type, true, true, true))
+ addError("unable to resolve class " + type.getName(),
type);
+ }
+ for (ImportNode importNode :
module.getStaticStarImports().values()) {
+ ClassNode type = importNode.getType();
+ if (!resolve(type, true, true, true))
+ addError("unable to resolve class " + type.getName(),
type);
+ }
- module.setImportsResolved(true);
- }
+ module.setImportsResolved(true);
+ }
- //
+ //
- if (phase < 2) node.putNodeMetaData(AnnotationNode[].class, new
LinkedHashSet<>());
+ if (phase < 2) node.putNodeMetaData(AnnotationNode[].class, new
LinkedHashSet<>());
- Map<GenericsTypeName, GenericsType> outerNames = null;
- if (node instanceof InnerClassNode) {
- outerNames = genericParameterNames;
- genericParameterNames = new HashMap<>();
- if (!Modifier.isStatic(node.getModifiers()))
- genericParameterNames.putAll(outerNames); // outer names
visible
- } else {
- genericParameterNames.clear(); // outer class: new generic
namespace
- }
- 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);
+ if (node instanceof InnerClassNode) {
+ outerNames = genericParameterNames;
+ genericParameterNames = new HashMap<>();
+ if (!Modifier.isStatic(node.getModifiers()))
+ genericParameterNames.putAll(outerNames); // outer names
visible
+ } else {
+ genericParameterNames.clear(); // outer class: new generic
namespace
}
+ 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);
+ }
- if (sn != null) checkCyclicInheritance(node, sn);
- for (ClassNode in : node.getInterfaces()) {
- checkCyclicInheritance(node, in);
- }
- 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
+ if (sn != null) checkCyclicInheritance(node, sn);
+ for (ClassNode in : node.getInterfaces()) {
+ checkCyclicInheritance(node, in);
+ }
+ 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
}
- }
- if (phase == 1) break; // resolve other class headers before
members, et al.
+ if (phase == 1) break; // resolve other class headers before
members, et al.
- // initialize scopes/variables now that imports and super types
are resolved
- new VariableScopeVisitor(source).visitClass(node);
+ // initialize scopes/variables now that imports and super
types are resolved
+ new VariableScopeVisitor(source).visitClass(node);
- visitPackage(node.getPackage());
- visitImports(node.getModule());
- visitAnnotations(node);
+ visitPackage(node.getPackage());
+ visitImports(node.getModule());
+ visitAnnotations(node);
- @SuppressWarnings("unchecked") // grab the collected annotations
and stop collecting
- var headerAnnotations = (Set<AnnotationNode>)
node.putNodeMetaData(AnnotationNode[].class, null);
+ @SuppressWarnings("unchecked") // grab the collected
annotations and stop collecting
+ var headerAnnotations = (Set<AnnotationNode>)
node.putNodeMetaData(AnnotationNode[].class, null);
- node.visitContents(this);
- visitObjectInitializerStatements(node);
- // GROOVY-10750, GROOVY-11179: resolve and inline
- headerAnnotations.forEach(this::visitAnnotation);
+ node.visitContents(this);
+ visitObjectInitializerStatements(node);
+ // GROOVY-10750, GROOVY-11179: resolve and inline
+ headerAnnotations.forEach(this::visitAnnotation);
+ }
+ } finally {
+ if (outerNames != null) genericParameterNames = outerNames;
+ currentClass = oldNode;
}
- if (outerNames != null) genericParameterNames = outerNames;
- currentClass = oldNode;
}
private void addFatalError(final String text, final ASTNode node) {
@@ -1401,8 +1408,11 @@ public class ResolveVisitor extends
ClassCodeExpressionTransformer {
public void visitBlockStatement(final BlockStatement block) {
VariableScope oldScope = currentScope;
currentScope = block.getVariableScope();
- super.visitBlockStatement(block);
- currentScope = oldScope;
+ try {
+ super.visitBlockStatement(block);
+ } finally {
+ currentScope = oldScope;
+ }
}
private boolean resolveGenericsTypes(final GenericsType[] types) {
diff --git a/src/test/groovy/groovy/util/GroovyScriptEngineReloadingTest.groovy
b/src/test/groovy/groovy/util/GroovyScriptEngineReloadingTest.groovy
index 660f4ddcf0..dc8653ca81 100644
--- a/src/test/groovy/groovy/util/GroovyScriptEngineReloadingTest.groovy
+++ b/src/test/groovy/groovy/util/GroovyScriptEngineReloadingTest.groovy
@@ -21,9 +21,9 @@ package groovy.util
import groovy.transform.AutoFinal
import groovy.transform.Canonical
import groovy.transform.TupleConstructor
-import org.junit.Before
-import org.junit.BeforeClass
-import org.junit.Test
+import org.junit.jupiter.api.BeforeAll
+import org.junit.jupiter.api.BeforeEach
+import org.junit.jupiter.api.Test
import static groovy.test.GroovyAssert.shouldFail
@@ -32,7 +32,7 @@ final class GroovyScriptEngineReloadingTest {
private GroovyScriptEngine gse
- @BeforeClass
+ @BeforeAll
static void setUpTestSuite() {
URL.setURLStreamHandlerFactory(protocol -> {
if (protocol == MapUrlConnection.PROTOCOL) {
@@ -41,7 +41,7 @@ final class GroovyScriptEngineReloadingTest {
})
}
- @Before
+ @BeforeEach
void setUpTestCase() {
makeGSE()
}
@@ -72,9 +72,9 @@ final class GroovyScriptEngineReloadingTest {
gse.@time += i
}
- private void execute(intervall, sleepTime, expected) {
- gse.config.minimumRecompilationInterval = intervall
- sleep intervall
+ private void execute(interval, sleepTime, expected) {
+ gse.config.minimumRecompilationInterval = interval
+ sleep interval
Binding binding = new Binding()
int val = 0
@@ -152,8 +152,8 @@ final class GroovyScriptEngineReloadingTest {
execute(1000, 10000, 2)
}
- @Test // ensures new source is ignored till minimumRecompilationIntervall
is passed
- void testRecompilationIntervall() {
+ @Test // ensures new source is ignored till minimumRecompilationInterval
is passed
+ void testRecompilationInterval() {
execute(100000, 10000, 1)
execute(100000, 10000, 1)
execute(100000, 200000, 2)
@@ -178,8 +178,37 @@ final class GroovyScriptEngineReloadingTest {
// make a change to the sub-class so that it gets recompiled
MapFileSystem.instance.modFile('SubClass.groovy', subClassText + '\n',
gse.@time)
+
gse.loadScriptByName('SubClass.groovy')
+ }
+
+ // GROOVY-9526, GROOVY-11719
+ @Test
+ void testRecompilingWithGenerics2() {
+ MapFileSystem.instance.modFile('BaseClass.groovy', 'class BaseClass<T>
{}', gse.@time)
+
+ MapFileSystem.instance.modFile('SomeClass.groovy', '''
+ class SomeClass {
+ public static final String CONSTANT = String.valueOf("")
+ }
+ ''', gse.@time)
+
+ def subClassText = '''
+ class SubClass extends BaseClass<String> {
+ public static final String CONSTANT = SomeClass.CONSTANT
+ }
+ '''
+ MapFileSystem.instance.modFile('SubClass.groovy', subClassText,
gse.@time)
+ MapFileSystem.instance.modFile('subClassUsage.groovy',
'SubClass.CONSTANT', gse.@time)
+
+ gse.loadScriptByName('subClassUsage.groovy')
+ sleep 1000
+
+ // make a change to the sub-class so that it gets recompiled
+ MapFileSystem.instance.modFile('SubClass.groovy', subClassText + '\n',
gse.@time)
+
+ gse.loadScriptByName('subClassUsage.groovy')
}
@Test
@@ -380,7 +409,8 @@ final class GroovyScriptEngineReloadingTest {
assert aScript instanceof CustomBaseClass
}
- @Test // GROOVY-3893
+ // GROOVY-3893
+ @Test
void testGSEWithNoScriptRoots() {
shouldFail ResourceException, {
String[] emptyScriptRoots = []
@@ -389,7 +419,8 @@ final class GroovyScriptEngineReloadingTest {
}
}
- @Test // GROOVY-6203
+ // GROOVY-6203
+ @Test
void testGSEBaseClass() {
gse.config = new
org.codehaus.groovy.control.CompilerConfiguration(scriptBaseClass:
CustomBaseClass.name)
@@ -401,7 +432,8 @@ final class GroovyScriptEngineReloadingTest {
assert script instanceof CustomBaseClass
}
- @Test // GROOVY-4013
+ // GROOVY-4013
+ @Test
void testGSENoCachingOfInnerClasses() {
MapFileSystem.instance.modFile('Groovy4013Helper.groovy', '''
import java.awt.event.*
@@ -424,7 +456,8 @@ final class GroovyScriptEngineReloadingTest {
assert klazz.name == 'Groovy4013Helper' // we should still get the
outer class, not inner one
}
- @Test // GROOVY-4234
+ // GROOVY-4234
+ @Test
void testGSERunningAScriptThatHasMultipleClasses() {
MapFileSystem.instance.modFile('Groovy4234Helper.groovy', '''
class Foo4234 {
@@ -443,7 +476,8 @@ final class GroovyScriptEngineReloadingTest {
gse.run('Groovy4234Helper.groovy', new Binding())
}
- @Test // GROOVY-2811, GROOVY-4286
+ // GROOVY-2811, GROOVY-4286
+ @Test
void testReloadingInterval() {
gse.config.minimumRecompilationInterval = 1500
def binding = new Binding([:])
