This is an automated email from the ASF dual-hosted git repository.
sunlan pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/groovy.git
The following commit(s) were added to refs/heads/master by this push:
new e6b7fc83a2 GROOVY-4721: variable declared in try block is in scope in
finally block (#2172)
e6b7fc83a2 is described below
commit e6b7fc83a279a12399df3cbb8755dd21bc64504e
Author: Daniel Sun <[email protected]>
AuthorDate: Thu Apr 10 23:22:18 2025 +0900
GROOVY-4721: variable declared in try block is in scope in finally block
(#2172)
* GROOVY-4721: variable declared in try block is in scope in finally block
* GROOVY-4721: Failed to find bound variables in Groovysh
Co-authored-by: Paul King <[email protected]>
---------
Co-authored-by: Paul King <[email protected]>
---
.../groovy/classgen/asm/StatementWriter.java | 16 ++-
src/test/groovy/bugs/Groovy4721.groovy | 117 +++++++++++++++++++++
.../org/apache/groovy/groovysh/Groovysh.groovy | 2 +-
3 files changed, 131 insertions(+), 4 deletions(-)
diff --git
a/src/main/java/org/codehaus/groovy/classgen/asm/StatementWriter.java
b/src/main/java/org/codehaus/groovy/classgen/asm/StatementWriter.java
index bd7387661a..eb9f08000a 100644
--- a/src/main/java/org/codehaus/groovy/classgen/asm/StatementWriter.java
+++ b/src/main/java/org/codehaus/groovy/classgen/asm/StatementWriter.java
@@ -20,6 +20,7 @@ package org.codehaus.groovy.classgen.asm;
import org.codehaus.groovy.ast.ClassHelper;
import org.codehaus.groovy.ast.ClassNode;
+import org.codehaus.groovy.ast.VariableScope;
import org.codehaus.groovy.ast.expr.BinaryExpression;
import org.codehaus.groovy.ast.expr.BooleanExpression;
import org.codehaus.groovy.ast.expr.ClosureListExpression;
@@ -36,6 +37,7 @@ import org.codehaus.groovy.ast.stmt.CaseStatement;
import org.codehaus.groovy.ast.stmt.CatchStatement;
import org.codehaus.groovy.ast.stmt.ContinueStatement;
import org.codehaus.groovy.ast.stmt.DoWhileStatement;
+import org.codehaus.groovy.ast.stmt.EmptyStatement;
import org.codehaus.groovy.ast.stmt.ExpressionStatement;
import org.codehaus.groovy.ast.stmt.ForStatement;
import org.codehaus.groovy.ast.stmt.IfStatement;
@@ -413,12 +415,20 @@ public class StatementWriter {
private BlockRecorder makeBlockRecorder(final Statement finallyStatement) {
BlockRecorder recorder = new BlockRecorder();
+ final CompileStack compileStack = controller.getCompileStack();
+
recorder.excludedStatement = () -> {
- controller.getCompileStack().pushBlockRecorderVisit(recorder);
+ if (finallyStatement == null || finallyStatement instanceof
EmptyStatement) return;
+
+ final VariableScope originalScope = compileStack.getScope();
+ compileStack.pop();
+ compileStack.pushBlockRecorderVisit(recorder);
finallyStatement.visit(controller.getAcg());
- controller.getCompileStack().popBlockRecorderVisit(recorder);
+ compileStack.popBlockRecorderVisit(recorder);
+ compileStack.pushVariableScope(originalScope);
};
- controller.getCompileStack().pushBlockRecorder(recorder);
+
+ compileStack.pushBlockRecorder(recorder);
return recorder;
}
diff --git a/src/test/groovy/bugs/Groovy4721.groovy
b/src/test/groovy/bugs/Groovy4721.groovy
new file mode 100644
index 0000000000..8e720f6f83
--- /dev/null
+++ b/src/test/groovy/bugs/Groovy4721.groovy
@@ -0,0 +1,117 @@
+/*
+ * 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 bugs
+
+import org.junit.jupiter.api.Test
+
+import static groovy.test.GroovyAssert.assertScript
+import static groovy.test.GroovyAssert.shouldFail
+
+final class Groovy4721 {
+ @Test
+ void testAccessingVariableInFinallyBlock_1() {
+ def err = shouldFail '''\
+ class MyClass {
+ def myMethod() {
+ try {
+ def x = 'foo'
+ }
+ finally {
+ println 'x: ' + x
+ }
+ }
+ }
+ println new MyClass().myMethod()
+ '''
+ assert err =~ /No such property: x for class: MyClass/
+ }
+
+ @Test
+ void testAccessingVariableInFinallyBlock_2() {
+ def err = shouldFail '''\
+ class MyClass {
+ def myMethod() {
+ try {
+ def x = 'foo'
+ return x
+ }
+ finally {
+ println 'x: ' + x
+ }
+ }
+ }
+ println new MyClass().myMethod()
+ '''
+ assert err =~ /No such property: x for class: MyClass/
+ }
+
+ @Test
+ void testAccessingVariableInFinallyBlock_3() {
+ def err = shouldFail '''\
+ class MyClass {
+ def myMethod() {
+ try {
+ def x = 'foo'
+ return
+ }
+ finally {
+ println 'x: ' + x
+ }
+ }
+ }
+ println new MyClass().myMethod()
+ '''
+ assert err =~ /No such property: x for class: MyClass/
+ }
+
+ @Test
+ void testAccessingVariableInFinallyBlock_4() {
+ assertScript '''\
+ class MyClass {
+ def myMethod() {
+ try {
+ def x = 'foo'
+ }
+ finally {
+ assert true
+ }
+ }
+ }
+ assert 'foo' == new MyClass().myMethod()
+ '''
+ }
+
+ @Test
+ void testAccessingVariableInFinallyBlock_5() {
+ assertScript '''\
+ class MyClass {
+ def myMethod() {
+ try {
+ def x = 'foo'
+ return x
+ }
+ finally {
+ assert true
+ }
+ }
+ }
+ assert 'foo' == new MyClass().myMethod()
+ '''
+ }
+}
diff --git
a/subprojects/groovy-groovysh/src/main/groovy/org/apache/groovy/groovysh/Groovysh.groovy
b/subprojects/groovy-groovysh/src/main/groovy/org/apache/groovy/groovysh/Groovysh.groovy
index 8be4ba6795..6f1553c77d 100644
---
a/subprojects/groovy-groovysh/src/main/groovy/org/apache/groovy/groovysh/Groovysh.groovy
+++
b/subprojects/groovy-groovysh/src/main/groovy/org/apache/groovy/groovysh/Groovysh.groovy
@@ -291,7 +291,7 @@ try {$COLLECTED_BOUND_VARS_MAP_VARNAME[\"$varname\"] =
$varname;
// Evaluate the current buffer w/imports and dummy statement
List<String> buff
if (variableBlocks) {
- buff = [importsSpec] + ['try {', 'true'] + current + ['} finally
{' + variableBlocks + '}']
+ buff = [importsSpec] + ['try {', 'true'] + current +
[variableBlocks, '} finally {' + variableBlocks + '}']
} else {
buff = [importsSpec] + ['true'] + current
}
