[jira] [Created] (GROOVY-9651) VariableScopeVisitor does not check object expression resp. MethodCallExpression#isImplicitThis returns unhelpful true

["Björn Kautler (Jira)" <ji...@apache.org>]

Björn Kautler created GROOVY-9651:
-------------------------------------

             Summary: VariableScopeVisitor does not check object expression resp. MethodCallExpression#isImplicitThis returns unhelpful true
                 Key: GROOVY-9651
                 URL: https://issues.apache.org/jira/browse/GROOVY-9651
             Project: Groovy
          Issue Type: Bug
    Affects Versions: 2.5.12, 3.0.4
            Reporter: Björn Kautler


We got this bug reported in Spock: [https://github.com/spockframework/spock/issues/1200]

The issue is, that Spock now uses the {{VariableScopeVisitor}} which misbehaves.
In {{VariableScopeVisitor#visitMethodCallExpression}}, the code assumes, that if {{isImplicitThis}} returns {{true}}, the method call actually targets {{this}}. But this is not necessarily the case, as {{isImplicitThis}} defaults to {{true}}, no matter what receiver you supply in the constructor.

I also found this old conversation where a related problem was discussed: [http://groovy.329449.n5.nabble.com/Problem-with-latest-2-0-beta-3-snapshot-and-Spock-td5496353.html]

Summarized the outcome of this thread is, that the behaviour is confusing and error prone, but that Jochen doesn't want to change it because it could break things out there and that if the object part of a {{MethodCallExpression}} is not "{{this}}", then the use of {{implicitThis}} is undefined.

Here is a simple reproducer, just paste it in a Groovy Console and execute it:
{code:java}
import groovyjarjarasm.asm.Opcodes;
import groovy.inspect.swingui.*;
import org.codehaus.groovy.ast.*;
import org.codehaus.groovy.ast.expr.*;
import org.codehaus.groovy.ast.stmt.*;
import org.codehaus.groovy.classgen.VariableScopeVisitor;

MethodNode method = new MethodNode(
  "foo",
  Opcodes.ACC_PUBLIC,
  ClassHelper.OBJECT_TYPE,
  [
    new Parameter(ClassHelper.OBJECT_TYPE, "bar"),
    new Parameter(ClassHelper.OBJECT_TYPE, "build")
  ] as Parameter[],
  ClassNode.EMPTY_ARRAY,
  new BlockStatement([new ExpressionStatement(new MethodCallExpression(new VariableExpression(new Parameter(ClassHelper.OBJECT_TYPE, "bar")), "build", MethodCallExpression.NO_ARGUMENTS))], null));
StringWriter sw = new StringWriter();
new AstNodeToScriptVisitor(sw).visitMethod(method);
println(sw.toString());
new VariableScopeVisitor(null).visitMethod(method);
sw = new StringWriter();
new AstNodeToScriptVisitor(sw).visitMethod(method);
println(sw.toString()); {code}
The result will be
{code:java}
public java.lang.Object foo(java.lang.Object bar, java.lang.Object build) {
    bar.build()
}

public java.lang.Object foo(java.lang.Object bar, java.lang.Object build) {
    build.call()
} {code}
 

A work-around on consumer side is, to do {{setImplicitThis(false);}} on the created {{MethodCallExpression}}.

A proper fix of this instance of {{isImplicitThis}} misinterpretation would be to check in the {{VariableScopeVisitor#visitMethodCallExpression}} whether the {{objectExpression}} actually is referring to {{this}} or not.

A maybe better fix that also fixes other misinterpretations like this and also future ones, would be to default to {{false}} if the object expression is not referring to {{this}} and to change the trivial implementation of {{isImplicitThis}} that just returns the member value to always return {{false}} if the {{objectExpression}} is not referring to {{this}} and only if it does to return the value of the member.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)