Line numbers sometimes incorrect in transformed code

[sdudley]

After running some code through the sandbox transformer, I found that the source line numbers are not always correctly rendered in stack traces. For example, given this input:

public class MyClass
{
    int foo()
    {
    }
}

MyClass myObj;

def cl =  {
    int bar = 0;

    System.out.println(myObj.foo());
}

cl();

When running this, the expected null dereference when calling myObj.foo() happens on line 13. Running this with groovy-sandbox 1.7 yields:

Caused by: java.lang.NullPointerException: Cannot invoke method foo() on null object
    at org.codehaus.groovy.runtime.NullObject.invokeMethod(NullObject.java:88)
    at org.codehaus.groovy.runtime.callsite.PogoMetaClassSite.call(PogoMetaClassSite.java:45)
    at org.codehaus.groovy.runtime.callsite.CallSiteArray.defaultCall(CallSiteArray.java:45)
    at org.codehaus.groovy.runtime.callsite.NullCallSite.call(NullCallSite.java:32)
    at org.codehaus.groovy.runtime.callsite.CallSiteArray.defaultCall(CallSiteArray.java:45)
    at org.codehaus.groovy.runtime.callsite.AbstractCallSite.call(AbstractCallSite.java:108)
    at org.kohsuke.groovy.sandbox.impl.Checker.checkedCall(Checker.java:92)
    at org.kohsuke.groovy.sandbox.impl.Checker$checkedCall.callStatic(Unknown Source)
    at org.codehaus.groovy.runtime.callsite.CallSiteArray.defaultCallStatic(CallSiteArray.java:53)
    at org.codehaus.groovy.runtime.callsite.AbstractCallSite.callStatic(AbstractCallSite.java:157)
    at MyGroovyScript$_run_closure1.doCall(MyGroovyScript.groovy:11)
    at MyGroovyScript$_run_closure1.doCall(MyGroovyScript.groovy)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:606)
    at org.codehaus.groovy.reflection.CachedMethod.invoke(CachedMethod.java:90)
    at groovy.lang.MetaMethod.doMethodInvoke(MetaMethod.java:233)
    at org.codehaus.groovy.runtime.metaclass.ClosureMetaClass.invokeMethod(ClosureMetaClass.java:278)
    at groovy.lang.MetaClassImpl.invokeMethod(MetaClassImpl.java:909)
    at org.codehaus.groovy.runtime.callsite.PogoMetaClassSite.call(PogoMetaClassSite.java:39)
    at org.codehaus.groovy.runtime.callsite.CallSiteArray.defaultCall(CallSiteArray.java:45)
    at org.codehaus.groovy.runtime.callsite.AbstractCallSite.call(AbstractCallSite.java:108)
    at org.kohsuke.groovy.sandbox.impl.Checker$1.call(Checker.java:113)
    at org.kohsuke.groovy.sandbox.GroovyInterceptor.onMethodCall(GroovyInterceptor.java:21)
    at org.kohsuke.groovy.sandbox.GroovyValueFilter.onMethodCall(GroovyValueFilter.java:58)
    at org.kohsuke.groovy.sandbox.impl.Checker$1.call(Checker.java:111)
    at org.kohsuke.groovy.sandbox.impl.Checker.checkedCall(Checker.java:108)
    at org.kohsuke.groovy.sandbox.impl.Checker$checkedCall.callStatic(Unknown Source)
    at org.codehaus.groovy.runtime.callsite.CallSiteArray.defaultCallStatic(CallSiteArray.java:53)
    at org.codehaus.groovy.runtime.callsite.AbstractCallSite.callStatic(AbstractCallSite.java:157)
    at MyGroovyScript.run(MyGroovyScript.groovy:10)
    at com.mycompany.GroovyManager.executeScript(GroovyManager.java:325)
    ... 32 more

We walk the stack trace to find the line number as the source of the error to report to the user, which means that we extract this line from the above:

    at MyGroovyScript$_run_closure1.doCall(MyGroovyScript.groovy:11)

This should have been line 13 (and indeed it is shown as line 13 if we do not enable the sandbox transformer).

Applying the following patch to SandboxTransformer.groovy seems to yield the correct line numbers. I'm not a Groovy/AST expert though, so I'm not sure if this is the right way to solve the problem.

index 65c845c..ef5e112 100644
--- a/src/main/groovy/org/kohsuke/groovy/sandbox/SandboxTransformer.groovy
+++ b/src/main/groovy/org/kohsuke/groovy/sandbox/SandboxTransformer.groovy
@@ -179,9 +179,19 @@ class SandboxTransformer extends CompilationCustomizer {
             return new StaticMethodCallExpression(checkerClass,name,
                 new ArgumentListExpression(arguments as Expression[]))
         }
-    
+
         @Override
         Expression transform(Expression exp) {
+            Expression rc = innerTransform(exp);
+
+            if (rc != null && exp != null) {
+                rc.setSourcePosition(exp);
+            }
+
+            return rc;
+        }
+
+        Expression innerTransform(Expression exp) {
             if (exp instanceof ClosureExpression) {
                 // ClosureExpression.transformExpression doesn't visit the code inside
                 ClosureExpression ce = (ClosureExpression)exp;

[kohsuke]

Fixed toward 1.8