[jira] [Updated] (GROOVY-8096) setScriptBaseClass with Java base class breaks @Field initialization from Binding due to wrong constructor

["Christoffer Hammarström (JIRA)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/GROOVY-8096?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Christoffer Hammarström updated GROOVY-8096:
--------------------------------------------
    Description: 
I created a pull request on GitHub with a failing test showing the problem: [https://github.com/apache/groovy/pull/502]

This test fails because {{ModuleNode.createStatementsClass()}} calls {{.getSuperClass().getDeclaredConstructor(SCRIPT_CONTEXT_CTOR)}} and gets {{null}} back, though the constructor does exist!

{{ModuleNode.setScriptBaseClassFromConfig(ClassNode)}}
 calls {{.setSuperClass(ClassHelper.make(baseClassName))}} on the {{scriptDummy ClassNode}}.

The {{ClassNode}} created for this script's base class has {{.lazyInitDone = true}} and {{.constructors = null}}

{{ModuleNode.createStatementsClass()}} calls {{.getSuperClass().getDeclaredConstructor(SCRIPT_CONTEXT_CTOR)}}

Then {{ClassNode.constructors}} is set to an empty ArrayList in {{ClassNode.getDeclaredConstructors()}}, insteaf of looking them up from the Java class.

The script constructor is then generated in {{ModuleNode.createStatementsClass()}} as:
{code:java}
     Constructor(Binding context) {
         super();             // Fields are initialized after the call to super()
                              // Fields are initialized here with new Binding() instead of context
         setBinding(context); // Fields are initialized before setBinding
     }
{code}
instead of
{code:java}
     Constructor(Binding context) {
         super(context); // Fields are initialized after the call to super(context)
     }
{code}
We're calling the default constructor in the base class with {{super()}}, instead of passing along the {{Binding context}} with {{super(context)}}

This breaks initialization of Fields that depend on the {{Binding context}}, because Fields are initialized between the call to {{super()}} and the {{setBinding(context)}}: [http://stackoverflow.com/a/14806340/233014]

This leads to {{MissingPropertyException}} because we're trying to look up variables from the {{new Binding()}} created in the default constructor, instead of the binding we passed in.

For convenience, here is the failing test:
{code:java|title=GroovyShellTest2.groovy}
    void testBindingsInFieldInitializersWithConfigJavaBaseScript() {
        def config = new org.codehaus.groovy.control.CompilerConfiguration()
        config.scriptBaseClass = BindingScript.class.name

        def shell = new GroovyShell(config);
        def scriptText = '''
        @groovy.transform.Field def script_args = getProperty('args')    // Will get MissingPropertyException here 

        assert script_args[0] == 'Hello Groovy'
        script_args[0]
'''

        def arg0 = 'Hello Groovy'
        def result = shell.run scriptText, 'TestBindingsInFieldInitializersWithConfigJavaBaseScript.groovy', [arg0]
        assert result == arg0
    }
{code}
and the Java script base class:
{code:java|title=BindingScript.java}
package groovy.lang;

/**
 * A Script which requires a Binding passed in the constructor and disallows calling the default constructor.
 */
public abstract class BindingScript extends Script {
    // Making the default constructor private instead gives IllegalAccessError
    // Removing the default constructor instead gives NoSuchMethodError
    // Removing both constructors just calls to the default constructor in groovy.lang.Script giving MissingPropertyException on field initialization
    protected BindingScript() {
        // This constructor erroneously gets called instead of the other one
    }
    
    protected BindingScript(Binding binding) {
        super(binding);
    }
}
{code}

  was:
I created a pull request on GitHub with a failing test showing the problem: https://github.com/apache/groovy/pull/502

This test fails because {{ModuleNode.createStatementsClass()}} calls {{.getSuperClass().getDeclaredConstructor(SCRIPT_CONTEXT_CTOR)}} and gets {{null}} back, though the constructor does exist!

{{ModuleNode.setScriptBaseClassFromConfig(ClassNode)}}
calls {{.setSuperClass(ClassHelper.make(baseClassName))}} on the {{scriptDummy ClassNode}}.

The {{ClassNode}} created for this script's base class has {{.lazyInitDone = true}} and {{.constructors = null}}

{{ModuleNode.createStatementsClass()}} calls {{.getSuperClass().getDeclaredConstructor(SCRIPT_CONTEXT_CTOR)}} 

Then {{ClassNode.constructors}} is set to an empty ArrayList in {{ClassNode.getDeclaredConstructors()}}, insteaf of looking them up from the Java class.

The script constructor is then generated in {{ModuleNode.createStatementsClass()}} as:

{code:java}
     Constructor(Binding context) {
         super();             // Fields are initialized after the call to super()
                              // Fields are initialized here with new Binding() instead of context
         setBinding(context); // Fields are initialized before setBinding
     }
{code}

instead of

{code:java}
     Constructor(Binding context) {
         super(context); // Fields are initialized after the call to super(context)
     }
{code}

We're calling the default constructor in the base class with {{super()}}, instead of passing along the {{Binding context}} with {{super(context)}} 

This breaks initialization of Fields that depend on the {{Binding context}}, because Fields are initialized between the call to {{super()}} and the {{setBinding(context)}}: http://stackoverflow.com/a/14806340/233014

This leads to {{MissingPropertyException}} because we're trying to look up variables from the {{new Binding()}} created in the default constructor, instead of the binding we passed in.

For convenience, here is the failing test:
{code:title=GroovyShellTest2.groovy}
    void testBindingsInFieldInitializersWithConfigJavaBaseScript() {
        def config = new org.codehaus.groovy.control.CompilerConfiguration()
        config.scriptBaseClass = BindingScript.class.name

        def shell = new GroovyShell(config);
        def scriptText = '''
        @groovy.transform.Field def script_args = getProperty('args')    // Will get MissingPropertyException here 
                                                                         // if we don't throw UnsupportedOperationException in the default constructor

        assert script_args[0] == 'Hello Groovy'
        script_args[0]
'''

        def arg0 = 'Hello Groovy'
        def result = shell.run scriptText, 'TestBindingsInFieldInitializersWithConfigJavaBaseScript.groovy', [arg0]
        assert result == arg0
    }
{code}
and the Java script base class:
{code:title=BindingScript.java}
package groovy.lang;

/**
 * A Script which requires a Binding passed in the constructor and disallows calling the default constructor.
 */
public abstract class BindingScript extends Script {
    // Making the default constructor private instead gives IllegalAccessError
    // Removing the default constructor instead gives NoSuchMethodError
    // Removing both constructors just calls to the default constructor in groovy.lang.Script giving MissingPropertyException on field initialization
    protected BindingScript() {
        throw new UnsupportedOperationException("\n\t*******\n\tBindingScript() should not be called! Should be calling BindingScript(Binding)!\n\t*******");
    }

    protected BindingScript(Binding binding) {
        super(binding);
    }
}
{code}


> setScriptBaseClass with Java base class breaks @Field initialization from Binding due to wrong constructor
> ----------------------------------------------------------------------------------------------------------
>
>                 Key: GROOVY-8096
>                 URL: https://issues.apache.org/jira/browse/GROOVY-8096
>             Project: Groovy
>          Issue Type: Bug
>          Components: Compiler, GroovyScriptEngine
>    Affects Versions: 2.4.8
>            Reporter: Christoffer Hammarström
>            Priority: Major
>              Labels: test
>
> I created a pull request on GitHub with a failing test showing the problem: [https://github.com/apache/groovy/pull/502]
> This test fails because {{ModuleNode.createStatementsClass()}} calls {{.getSuperClass().getDeclaredConstructor(SCRIPT_CONTEXT_CTOR)}} and gets {{null}} back, though the constructor does exist!
> {{ModuleNode.setScriptBaseClassFromConfig(ClassNode)}}
>  calls {{.setSuperClass(ClassHelper.make(baseClassName))}} on the {{scriptDummy ClassNode}}.
> The {{ClassNode}} created for this script's base class has {{.lazyInitDone = true}} and {{.constructors = null}}
> {{ModuleNode.createStatementsClass()}} calls {{.getSuperClass().getDeclaredConstructor(SCRIPT_CONTEXT_CTOR)}}
> Then {{ClassNode.constructors}} is set to an empty ArrayList in {{ClassNode.getDeclaredConstructors()}}, insteaf of looking them up from the Java class.
> The script constructor is then generated in {{ModuleNode.createStatementsClass()}} as:
> {code:java}
>      Constructor(Binding context) {
>          super();             // Fields are initialized after the call to super()
>                               // Fields are initialized here with new Binding() instead of context
>          setBinding(context); // Fields are initialized before setBinding
>      }
> {code}
> instead of
> {code:java}
>      Constructor(Binding context) {
>          super(context); // Fields are initialized after the call to super(context)
>      }
> {code}
> We're calling the default constructor in the base class with {{super()}}, instead of passing along the {{Binding context}} with {{super(context)}}
> This breaks initialization of Fields that depend on the {{Binding context}}, because Fields are initialized between the call to {{super()}} and the {{setBinding(context)}}: [http://stackoverflow.com/a/14806340/233014]
> This leads to {{MissingPropertyException}} because we're trying to look up variables from the {{new Binding()}} created in the default constructor, instead of the binding we passed in.
> For convenience, here is the failing test:
> {code:java|title=GroovyShellTest2.groovy}
>     void testBindingsInFieldInitializersWithConfigJavaBaseScript() {
>         def config = new org.codehaus.groovy.control.CompilerConfiguration()
>         config.scriptBaseClass = BindingScript.class.name
>         def shell = new GroovyShell(config);
>         def scriptText = '''
>         @groovy.transform.Field def script_args = getProperty('args')    // Will get MissingPropertyException here 
>         assert script_args[0] == 'Hello Groovy'
>         script_args[0]
> '''
>         def arg0 = 'Hello Groovy'
>         def result = shell.run scriptText, 'TestBindingsInFieldInitializersWithConfigJavaBaseScript.groovy', [arg0]
>         assert result == arg0
>     }
> {code}
> and the Java script base class:
> {code:java|title=BindingScript.java}
> package groovy.lang;
> /**
>  * A Script which requires a Binding passed in the constructor and disallows calling the default constructor.
>  */
> public abstract class BindingScript extends Script {
>     // Making the default constructor private instead gives IllegalAccessError
>     // Removing the default constructor instead gives NoSuchMethodError
>     // Removing both constructors just calls to the default constructor in groovy.lang.Script giving MissingPropertyException on field initialization
>     protected BindingScript() {
>         // This constructor erroneously gets called instead of the other one
>     }
>     
>     protected BindingScript(Binding binding) {
>         super(binding);
>     }
> }
> {code}



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)