This is an automated email from the ASF dual-hosted git repository.
emilles pushed a commit to branch GROOVY_5_0_X
in repository https://gitbox.apache.org/repos/asf/groovy.git
The following commit(s) were added to refs/heads/GROOVY_5_0_X by this push:
new 381fae0373 GROOVY-11800: logging transform within closure adds
`getThisObject()`
381fae0373 is described below
commit 381fae03734a739eb816196637404c285dbc181a
Author: Eric Milles <[email protected]>
AuthorDate: Thu Nov 13 11:28:02 2025 -0600
GROOVY-11800: logging transform within closure adds `getThisObject()`
5_0_X backport
---
.../groovy/transform/LogASTTransformation.java | 69 ++++++++++++----------
.../groovy/groovy/util/logging/Slf4jTest.groovy | 32 +++++++---
2 files changed, 62 insertions(+), 39 deletions(-)
diff --git
a/src/main/java/org/codehaus/groovy/transform/LogASTTransformation.java
b/src/main/java/org/codehaus/groovy/transform/LogASTTransformation.java
index 1a1d925507..6ccfd5a5c9 100644
--- a/src/main/java/org/codehaus/groovy/transform/LogASTTransformation.java
+++ b/src/main/java/org/codehaus/groovy/transform/LogASTTransformation.java
@@ -47,6 +47,8 @@ import java.lang.reflect.Method;
import java.lang.reflect.Modifier;
import static org.apache.groovy.ast.tools.VisibilityUtils.getVisibility;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.callThisX;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.propX;
import static org.objectweb.asm.Opcodes.ACC_FINAL;
import static org.objectweb.asm.Opcodes.ACC_PRIVATE;
import static org.objectweb.asm.Opcodes.ACC_STATIC;
@@ -98,7 +100,8 @@ public class LogASTTransformation extends
AbstractASTTransformation implements C
final ClassNode classNode = (ClassNode) targetClass;
- ClassCodeExpressionTransformer transformer = new
ClassCodeExpressionTransformer() {
+ var transformer = new ClassCodeExpressionTransformer() {
+ private boolean inClosure;
private FieldNode logNode;
@Override
@@ -108,12 +111,23 @@ public class LogASTTransformation extends
AbstractASTTransformation implements C
@Override
public Expression transform(final Expression exp) {
- if (exp == null) return null;
if (exp instanceof MethodCallExpression) {
- return transformMethodCallExpression(exp);
- }
- if (exp instanceof ClosureExpression) {
- return transformClosureExpression((ClosureExpression) exp);
+ var call = (MethodCallExpression) exp;
+ var modifiedCall = addGuard(call);
+ if (modifiedCall != null) {
+ return modifiedCall;
+ }
+ } else if (exp instanceof ClosureExpression) {
+ var code = ((ClosureExpression) exp).getCode();
+ if (code instanceof BlockStatement) {
+ boolean previousValue = inClosure; inClosure = true;
+ try {
+ super.visitBlockStatement((BlockStatement) code);
+ } finally {
+ inClosure = previousValue;
+ }
+ }
+ return exp;
}
return super.transform(exp);
}
@@ -137,46 +151,41 @@ public class LogASTTransformation extends
AbstractASTTransformation implements C
super.visitClass(node);
}
- private Expression transformClosureExpression(final
ClosureExpression exp) {
- if (exp.getCode() instanceof BlockStatement) {
- BlockStatement code = (BlockStatement) exp.getCode();
- super.visitBlockStatement(code);
- }
- return exp;
- }
-
- private Expression transformMethodCallExpression(final Expression
exp) {
- Expression modifiedCall = addGuard((MethodCallExpression) exp);
- return modifiedCall == null ? super.transform(exp) :
modifiedCall;
- }
-
private Expression addGuard(final MethodCallExpression mce) {
- // only add guard to methods of the form:
logVar.logMethod(params)
+ // only add guard to methods of the form:
logVar.logMethod(arguments)
if (!(mce.getObjectExpression() instanceof
VariableExpression)) {
return null;
}
- VariableExpression variableExpression = (VariableExpression)
mce.getObjectExpression();
+ var variableExpression = (VariableExpression)
mce.getObjectExpression();
if (!variableExpression.getName().equals(logFieldName)
|| !(variableExpression.getAccessedVariable()
instanceof DynamicVariable)) {
return null;
}
-
String methodName = mce.getMethodAsString();
- if (methodName == null) return null;
- if (!loggingStrategy.isLoggingMethod(methodName)) return null;
- // also don't bother with guard if we have "simple" method args
- // since there is no saving
+ if (methodName == null ||
!loggingStrategy.isLoggingMethod(methodName)) return null;
+
+ Expression receiver;
+ if (inClosure) {
+ var pe = propX(callThisX("getThisObject"), logFieldName);
// GROOVY-11800
+ pe.getProperty().setSourcePosition(variableExpression);
+ pe.setType(logNode.getType());
+ mce.setObjectExpression(pe);
+ receiver = pe;
+ } else {
+ receiver = variableExpression;
+ variableExpression.setAccessedVariable(logNode);
+ }
+
+ // do not bother with guard if we have "simple" args since
there are no savings
if (usesSimpleMethodArgumentsOnly(mce)) return null;
- variableExpression.setAccessedVariable(logNode);
- return
loggingStrategy.wrapLoggingMethodCall(variableExpression, methodName, mce);
+ return loggingStrategy.wrapLoggingMethodCall(receiver,
methodName, mce);
}
private boolean usesSimpleMethodArgumentsOnly(final
MethodCallExpression mce) {
Expression arguments = mce.getArguments();
if (arguments instanceof TupleExpression) {
- TupleExpression tuple = (TupleExpression) arguments;
- for (Expression exp : tuple.getExpressions()) {
+ for (Expression exp : (TupleExpression) arguments) {
if (!isSimpleExpression(exp)) return false;
}
return true;
diff --git a/src/test/groovy/groovy/util/logging/Slf4jTest.groovy
b/src/test/groovy/groovy/util/logging/Slf4jTest.groovy
index cb1c071d24..d5199334ca 100644
--- a/src/test/groovy/groovy/util/logging/Slf4jTest.groovy
+++ b/src/test/groovy/groovy/util/logging/Slf4jTest.groovy
@@ -394,6 +394,24 @@ final class Slf4jTest {
"""
}
+ // GROOVY-11800
+ @Test
+ void testLogOwner() {
+ Class clazz = new GroovyClassLoader().parseClass('''
+ @groovy.util.logging.Slf4j
+ class MyClass {
+ def loggingMethod() {
+ [].with {
+ log.info('info')
+ }
+ }
+ }
+ ''')
+ clazz.newInstance().loggingMethod()
+
+ assert appender.events.size() == 1
+ }
+
@Test
void testLogGuard() {
Class clazz = new GroovyClassLoader().parseClass('''
@@ -402,15 +420,14 @@ final class Slf4jTest {
def loggingMethod() {
def isSet = false
log.setLevel(ch.qos.logback.classic.Level.ERROR)
- log.trace (isSet = true)
+ log.trace(isSet = true)
return isSet
}
}
- new MyClass().loggingMethod()
''')
+ boolean result = clazz.newInstance().loggingMethod()
- Script s = (Script) clazz.newInstance()
- assert s.run() == false
+ assert result == false
}
@Test
@@ -423,9 +440,7 @@ final class Slf4jTest {
}
}
''')
-
- def s = clazz.newInstance()
- s.loggingMethod()
+ clazz.newInstance().loggingMethod()
assert appender.events.size() == 1
}
@@ -448,8 +463,7 @@ final class Slf4jTest {
}
}
''')
- def s = clazz.newInstance()
- s.loggingMethod()
+ clazz.newInstance().loggingMethod()
assert appenderForCustomCategory.events.size() == 1
assert appender.events.isEmpty()
