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 2025/07/25 19:19:48 UTC
(groovy) branch GROOVY_4_0_X updated: GROOVY-9526, GROOVY-11719: pop field state on exception
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
The following commit(s) were added to refs/heads/GROOVY_4_0_X by this push:
new 7aec64d3de GROOVY-9526, GROOVY-11719: pop field state on exception
7aec64d3de is described below
commit 7aec64d3de2eafc2e57f2c5c549402a9cd87e806
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Tue Jul 22 09:23:05 2025 -0500
GROOVY-9526, GROOVY-11719: pop field state on exception
4_0_X backport
---
.../codehaus/groovy/control/ResolveVisitor.java | 249 +++++++++++----------
.../util/GroovyScriptEngineReloadingTest.groovy | 54 ++++-
2 files changed, 173 insertions(+), 130 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/control/ResolveVisitor.java b/src/main/java/org/codehaus/groovy/control/ResolveVisitor.java
index 383c0bffa7..d91aaf3216 100644
--- a/src/main/java/org/codehaus/groovy/control/ResolveVisitor.java
+++ b/src/main/java/org/codehaus/groovy/control/ResolveVisitor.java
@@ -95,7 +95,7 @@ public class ResolveVisitor extends ClassCodeExpressionTransformer {
// note: BigInteger and BigDecimal are also imported by default
// `java.util` is used much frequently than other two java packages(`java.io` and `java.net`), so place java.util before the two packages
public static final String[] DEFAULT_IMPORTS = {"java.lang.", "java.util.", "java.io.", "java.net.", "groovy.lang.", "groovy.util."};
- public static final String[] EMPTY_STRING_ARRAY = new String[0];
+ public static final String[] EMPTY_STRING_ARRAY = {};
public static final String QUESTION_MARK = "?";
private final CompilationUnit compilationUnit;
@@ -269,13 +269,14 @@ public class ResolveVisitor extends ClassCodeExpressionTransformer {
if (!canSeeTypeVars(node.getModifiers(), node.getDeclaringClass())) {
genericParameterNames = Collections.emptyMap();
}
-
- if (!fieldTypesChecked.contains(node)) {
- resolveOrFail(node.getType(), node);
+ try {
+ if (!fieldTypesChecked.contains(node)) {
+ resolveOrFail(node.getType(), node);
+ }
+ super.visitField(node);
+ } finally {
+ genericParameterNames = oldNames;
}
- super.visitField(node);
-
- genericParameterNames = oldNames;
}
@Override
@@ -284,13 +285,14 @@ public class ResolveVisitor extends ClassCodeExpressionTransformer {
if (!canSeeTypeVars(node.getModifiers(), node.getDeclaringClass())) {
genericParameterNames = Collections.emptyMap();
}
+ try {
+ resolveOrFail(node.getType(), node);
+ fieldTypesChecked.add(node.getField());
- resolveOrFail(node.getType(), node);
- 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) {
@@ -299,38 +301,38 @@ 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());
-
- resolveOrFail(node.getReturnType(), node);
- for (Parameter p : node.getParameters()) {
- p.setInitialExpression(transform(p.getInitialExpression()));
- ClassNode t = p.getType();
- resolveOrFail(t, t);
- visitAnnotations(p);
- }
- if (node.getExceptions() != null) {
- for (ClassNode t : node.getExceptions()) {
+ resolveOrFail(node.getReturnType(), node);
+ for (Parameter p : node.getParameters()) {
+ p.setInitialExpression(transform(p.getInitialExpression()));
+ ClassNode t = p.getType();
resolveOrFail(t, t);
+ visitAnnotations(p);
+ }
+ if (node.getExceptions() != null) {
+ for (ClassNode t : node.getExceptions()) {
+ resolveOrFail(t, t);
+ }
}
- }
-
- checkGenericsCyclicInheritance(node.getGenericsTypes());
-
- MethodNode oldCurrentMethod = currentMethod;
- currentMethod = node;
- super.visitConstructorOrMethod(node, isConstructor);
+ checkGenericsCyclicInheritance(node.getGenericsTypes());
- 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) {
@@ -1047,7 +1049,7 @@ public class ResolveVisitor extends ClassCodeExpressionTransformer {
}
private static boolean testVanillaNameForClass(final String name) {
- if (name == null || name.length() == 0) return false;
+ if (name == null || name.isEmpty()) return false;
return !Character.isLowerCase(name.charAt(0));
}
@@ -1229,106 +1231,110 @@ 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);
+ }
- //
+ //
- 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);
- }
- checkGenericsCyclicInheritance(node.getGenericsTypes());
- 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);
+ }
+ checkGenericsCyclicInheritance(node.getGenericsTypes());
+ 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());
+ visitPackage(node.getPackage());
+ visitImports(node.getModule());
- node.visitContents(this);
- visitObjectInitializerStatements(node);
- visitAnnotations(node); // GROOVY-10750, GROOVY-11206
+ node.visitContents(this);
+ visitObjectInitializerStatements(node);
+ visitAnnotations(node); // GROOVY-10750, GROOVY-11206
+ }
+ } finally {
+ if (outerNames != null) genericParameterNames = outerNames;
+ currentClass = oldNode;
}
- if (outerNames != null) genericParameterNames = outerNames;
- currentClass = oldNode;
}
- private void checkGenericsCyclicInheritance(GenericsType[] genericsTypes) {
+ private void checkGenericsCyclicInheritance(final GenericsType[] genericsTypes) {
if (genericsTypes == null) return;
-
for (GenericsType gt : genericsTypes) {
if (gt == null) continue;
ClassNode[] upperBounds = gt.getUpperBounds();
@@ -1390,8 +1396,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..bf0791e7f8 100644
--- a/src/test/groovy/groovy/util/GroovyScriptEngineReloadingTest.groovy
+++ b/src/test/groovy/groovy/util/GroovyScriptEngineReloadingTest.groovy
@@ -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([:])