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:51:44 UTC
(groovy) branch GROOVY_3_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_3_0_X
in repository https://gitbox.apache.org/repos/asf/groovy.git
The following commit(s) were added to refs/heads/GROOVY_3_0_X by this push:
new a26cf5b414 GROOVY-9526, GROOVY-11719: pop field state on exception
a26cf5b414 is described below
commit a26cf5b414c0cfa3c87d756f97f99d7468aa35ad
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Tue Jul 22 09:23:05 2025 -0500
GROOVY-9526, GROOVY-11719: pop field state on exception
3_0_X backport
---
.../codehaus/groovy/control/ResolveVisitor.java | 229 +++++++++++----------
.../util/GroovyScriptEngineReloadingTest.groovy | 229 +++++++++++----------
2 files changed, 235 insertions(+), 223 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/control/ResolveVisitor.java b/src/main/java/org/codehaus/groovy/control/ResolveVisitor.java
index 541cde99b8..05ad3e3b0b 100644
--- a/src/main/java/org/codehaus/groovy/control/ResolveVisitor.java
+++ b/src/main/java/org/codehaus/groovy/control/ResolveVisitor.java
@@ -102,10 +102,8 @@ 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."};
- private static final String BIGINTEGER_STR = "BigInteger";
- private static final String BIGDECIMAL_STR = "BigDecimal";
- public static final String QUESTION_MARK = "?";
- public static final String[] EMPTY_STRING_ARRAY = new String[0];
+ public static final String[] EMPTY_STRING_ARRAY = {};
+ public static final String QUESTION_MARK = "?";
private ClassNode currentClass;
private final CompilationUnit compilationUnit;
@@ -273,13 +271,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
@@ -288,13 +287,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) {
@@ -303,36 +303,36 @@ 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);
+ }
}
- }
-
- MethodNode oldCurrentMethod = currentMethod;
- currentMethod = node;
-
- super.visitConstructorOrMethod(node, isConstructor);
- 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) {
@@ -650,11 +650,11 @@ public class ResolveVisitor extends ClassCodeExpressionTransformer {
if (resolveFromDefaultImports(type, DEFAULT_IMPORTS)) {
return true;
}
- if (BIGINTEGER_STR.equals(typeName)) {
+ if ("BigInteger".equals(typeName)) {
type.setRedirect(ClassHelper.BigInteger_TYPE);
return true;
}
- if (BIGDECIMAL_STR.equals(typeName)) {
+ if ("BigDecimal".equals(typeName)) {
type.setRedirect(ClassHelper.BigDecimal_TYPE);
return true;
}
@@ -1155,7 +1155,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));
}
@@ -1410,92 +1410,98 @@ public class ResolveVisitor extends ClassCodeExpressionTransformer {
public void visitClass(final ClassNode node) {
ClassNode oldNode = currentClass; currentClass = node;
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
- }
- InnerClassNode innerClass = (InnerClassNode) node;
- if (innerClass.isAnonymous()) {
- MethodNode enclosingMethod = innerClass.getEnclosingMethod();
- if (enclosingMethod != null) {
- resolveGenericsHeader(enclosingMethod.getGenericsTypes());
+ try {
+ if (node instanceof InnerClassNode) {
+ outerNames = genericParameterNames;
+ genericParameterNames = new HashMap<>();
+ if (!Modifier.isStatic(node.getModifiers())) {
+ genericParameterNames.putAll(outerNames); // outer names visible
}
+ InnerClassNode innerClass = (InnerClassNode) node;
+ if (innerClass.isAnonymous()) {
+ MethodNode enclosingMethod = innerClass.getEnclosingMethod();
+ if (enclosingMethod != null) {
+ resolveGenericsHeader(enclosingMethod.getGenericsTypes());
+ }
+ }
+ } else {
+ genericParameterNames.clear(); // outer class: new generic namespace
}
- } else {
- genericParameterNames.clear(); // outer class: new generic namespace
- }
- resolveGenericsHeader(node.getGenericsTypes());
+ resolveGenericsHeader(node.getGenericsTypes());
- ModuleNode module = node.getModule();
- if (!module.hasImportsResolved()) {
- for (ImportNode importNode : module.getImports()) {
- currImportNode = importNode;
- ClassNode type = importNode.getType();
- if (resolve(type, false, false, true)) {
- currImportNode = null;
- continue;
- }
- currImportNode = null;
- addError("unable to resolve class " + type.getName(), type);
- }
- for (ImportNode importNode : module.getStarImports()) {
- if (importNode.getLineNumber() > 0) {
+ ModuleNode module = node.getModule();
+ if (!module.hasImportsResolved()) {
+ for (ImportNode importNode : module.getImports()) {
currImportNode = 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 {
+ currImportNode = null;
}
- currImportNode = null;
}
+ for (ImportNode importNode : module.getStarImports()) {
+ if (importNode.getLineNumber() > 0) {
+ currImportNode = 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 {
+ currImportNode = 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);
}
- for (ImportNode importNode : module.getStaticImports().values()) {
- ClassNode type = importNode.getType();
- if (!resolve(type, true, true, true))
- addError("unable to resolve class " + type.getName(), type);
+
+ ClassNode sn = node.getUnresolvedSuperClass();
+ if (sn != null) {
+ resolveOrFail(sn, "", node, true);
}
- 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 (ClassNode in : node.getInterfaces()) {
+ resolveOrFail(in, "", node, true);
}
- module.setImportsResolved(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());
+ }
}
}
}
- }
-
- super.visitClass(node);
- resolveOuterNestedClassFurther(node);
+ super.visitClass(node);
- if (outerNames != null) // GROOVY-11711
- genericParameterNames = outerNames;
+ resolveOuterNestedClassFurther(node);
+ } finally {
+ if (outerNames != null) // GROOVY-11711
+ genericParameterNames = outerNames;
- currentClass = oldNode;
+ currentClass = oldNode;
+ }
}
private void checkCyclicInheritance(final ClassNode node, final ClassNode type) {
@@ -1574,8 +1580,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/util/GroovyScriptEngineReloadingTest.groovy b/src/test/groovy/util/GroovyScriptEngineReloadingTest.groovy
index cf00a99497..b4709f2ab2 100644
--- a/src/test/groovy/util/GroovyScriptEngineReloadingTest.groovy
+++ b/src/test/groovy/util/GroovyScriptEngineReloadingTest.groovy
@@ -19,6 +19,8 @@
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
@@ -70,24 +72,24 @@ 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
- binding.setVariable("val", val)
- MapFileSystem.instance.modFile("s_1", "val = 1", gse.@time)
- gse.run("s_1", binding)
+ binding.setVariable('val', val)
+ MapFileSystem.instance.modFile('s_1', 'val = 1', gse.@time)
+ gse.run('s_1', binding)
- assert binding.getVariable("val") == 1
+ assert binding.getVariable('val') == 1
sleep sleepTime
- MapFileSystem.instance.modFile("s_1", "val = 2", gse.@time)
- gse.run("s_1", binding)
+ MapFileSystem.instance.modFile('s_1', 'val = 2', gse.@time)
+ gse.run('s_1', binding)
- assert binding.getVariable("val") == expected
+ assert binding.getVariable('val') == expected
}
/**
@@ -100,16 +102,16 @@ final class GroovyScriptEngineReloadingTest {
* job.
*/
private Object instantiate(String className, ClassLoader classLoader) {
- Class clazz = null;
+ Class clazz = null
try {
- clazz = Class.forName(className, true, classLoader);
- } catch (ClassNotFoundException ex) {
- throw new RuntimeException("Class.forName failed for " + className, ex);
+ clazz = Class.forName(className, true, classLoader)
+ } catch (ClassNotFoundException e) {
+ throw new RuntimeException("Class.forName failed for $className", e)
}
try {
return clazz.newInstance();
- } catch (Exception ex) {
- throw new RuntimeException("Could not instantiate object of class " + className, ex);
+ } catch (Exception e) {
+ throw new RuntimeException("Could not instantiate object of class $className", e)
}
}
@@ -150,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)
@@ -176,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
@@ -297,63 +328,60 @@ final class GroovyScriptEngineReloadingTest {
@Test
void testDynamicInstantiation() {
MapFileSystem.instance.modFile('script.groovy', '''
- def obj = dynaInstantiate.instantiate(className, getClass().getClassLoader())
- obj.modifyWidth(dim, addThis)
- returnedMessage = obj.message
+ def obj = dynaInstantiate.instantiate(className, this.class.classLoader)
+ obj.modifyWidth(dim, addThis)
+ returnedMessage = obj.message
''', 0)
MapFileSystem.instance.modFile('com/company/MakeMeSuper.groovy', '''
package com.company
import com.company.util.*
class MakeMeSuper{
- private HelperIntf helper = new Helper()
- def getMessage() {
- helper.getMessage()
- }
+ private HelperIntf helper = new Helper()
+ def getMessage() {
+ helper.getMessage()
+ }
}
- ''', 0)
+ ''', 0)
- MapFileSystem.instance.modFile('com/company/MakeMe.groovy',
- '''
- package com.company
- class MakeMe extends MakeMeSuper{
- def modifyWidth(dim, addThis){
+ MapFileSystem.instance.modFile('com/company/MakeMe.groovy', '''
+ package com.company
+ class MakeMe extends MakeMeSuper{
+ def modifyWidth(dim, addThis){
dim.width += addThis
- }
- }
- ''', 0)
-
- MapFileSystem.instance.modFile('com/company/util/HelperIntf.groovy',
- '''
- package com.company.util
- interface HelperIntf{
+ }
+ }
+ ''', 0)
+
+ MapFileSystem.instance.modFile('com/company/util/HelperIntf.groovy', '''
+ package com.company.util
+ interface HelperIntf {
public String getMessage();
- }
- ''', 0)
-
- MapFileSystem.instance.modFile('com/company/util/Helper.groovy',
- '''
- package com.company.util
- class Helper implements HelperIntf{
- public String getMessage(){
- 'worked'
- }
- }
- ''', 0)
+ }
+ ''', 0)
+
+ MapFileSystem.instance.modFile('com/company/util/Helper.groovy', '''
+ package com.company.util
+ class Helper implements HelperIntf {
+ public String getMessage() {
+ 'worked'
+ }
+ }
+ ''', 0)
//Code run in the script will modify this dimension object.
- MyDimension dim = new MyDimension();
+ MyDimension dim = new MyDimension()
- Binding binding = new Binding();
- binding.setVariable('dim', dim);
- binding.setVariable('dynaInstantiate', this);
+ Binding binding = new Binding()
+ binding.setVariable('dim', dim)
+ binding.setVariable('dynaInstantiate', this)
- binding.setVariable('className', 'com.company.MakeMe');
+ binding.setVariable('className', 'com.company.MakeMe')
- int addThis = 3;
- binding.setVariable('addThis', addThis);
+ int addThis = 3
+ binding.setVariable('addThis', addThis)
- gse.run('script.groovy', binding);
+ gse.run('script.groovy', binding)
//The script instantiated com.company.MakeMe via our own
//instantiate method. The instantiated object modified the
@@ -381,7 +409,7 @@ final class GroovyScriptEngineReloadingTest {
assert aScript instanceof CustomBaseClass
}
- /** GROOVY-3893 */
+ // GROOVY-3893
@Test
void testGSEWithNoScriptRoots() {
shouldFail ResourceException, {
@@ -391,7 +419,7 @@ final class GroovyScriptEngineReloadingTest {
}
}
- /** GROOVY-6203 */
+ // GROOVY-6203
@Test
void testGSEBaseClass() {
gse.config = new org.codehaus.groovy.control.CompilerConfiguration(scriptBaseClass: CustomBaseClass.name)
@@ -404,24 +432,22 @@ final class GroovyScriptEngineReloadingTest {
assert script instanceof CustomBaseClass
}
- /** GROOVY-4013 */
+ // GROOVY-4013
@Test
void testGSENoCachingOfInnerClasses() {
- MapFileSystem.instance.modFile('Groovy4013Helper.groovy',
- '''
- import java.awt.event.*
- import java.awt.*
- class Groovy4013Helper
- {
- def initPanel()
- {
- def b = new Button('click me')
- b.addActionListener( new ActionListener(){
- public void actionPerformed(ActionEvent e) {}
- })
- }
- }
- ''', 0)
+ MapFileSystem.instance.modFile('Groovy4013Helper.groovy', '''
+ import java.awt.event.*
+ import java.awt.*
+ class Groovy4013Helper {
+ def initPanel() {
+ def b = new Button('click me')
+ b.addActionListener(new ActionListener() {
+ public void actionPerformed(ActionEvent e) {
+ }
+ })
+ }
+ }
+ ''', 0)
def klazz = gse.loadScriptByName('Groovy4013Helper.groovy')
assert klazz.name == 'Groovy4013Helper'
@@ -430,18 +456,18 @@ final class GroovyScriptEngineReloadingTest {
assert klazz.name == 'Groovy4013Helper' // we should still get the outer class, not inner one
}
- /** GROOVY-4234 */
+ // GROOVY-4234
@Test
void testGSERunningAScriptThatHasMultipleClasses() {
- MapFileSystem.instance.modFile('Groovy4234Helper.groovy',
- '''
- class Foo4234 {
- static main(args){
- //println 'Running Foo4234 -> main()'
- }
- }
- class Bar4234 { }
- ''', 0)
+ MapFileSystem.instance.modFile('Groovy4234Helper.groovy', '''
+ class Foo4234 {
+ static main(args) {
+ //println 'Running Foo4234 -> main()'
+ }
+ }
+ class Bar4234 {
+ }
+ ''', 0)
//println 'testGSELoadingAScriptThatHasMultipleClasses - Run 1'
gse.run('Groovy4234Helper.groovy', new Binding())
@@ -450,7 +476,7 @@ final class GroovyScriptEngineReloadingTest {
gse.run('Groovy4234Helper.groovy', new Binding())
}
- /** GROOVY-2811 and GROOVY-4286 */
+ // GROOVY-2811, GROOVY-4286
@Test
void testReloadingInterval() {
gse.config.minimumRecompilationInterval = 1500
@@ -479,14 +505,10 @@ final class GroovyScriptEngineReloadingTest {
//--------------------------------------------------------------------------
+ @TupleConstructor
static class MapFileEntry {
String content
long lutime
-
- MapFileEntry(String content, long lutime) {
- this.content = content
- this.lutime = lutime
- }
}
@Singleton
@@ -585,29 +607,10 @@ final class GroovyScriptEngineReloadingTest {
void connect() throws IOException {}
}
+ @Canonical
static class MyDimension {
int width
int height
-
- MyDimension(int x, int y) {
- width = x
- height = y
- }
-
- MyDimension() {
- width = 0
- height = 0
- }
-
- @Override
- boolean equals(o) {
- o.width == width && o.height == height
- }
-
- @Override
- int hashCode() {
- width + 13 * height
- }
}
static abstract class CustomBaseClass extends Script {