groovy
  1. groovy
  2. GROOVY-2643

Delegate property is ignored by closure

    Details

    • Type: Bug Bug
    • Status: Closed Closed
    • Priority: Blocker Blocker
    • Resolution: Fixed
    • Affects Version/s: 1.5.4, 1.6-beta-1
    • Fix Version/s: 1.5.5, 1.6-beta-1
    • Component/s: None
    • Labels:
      None
    • Testcase included:
      yes
    • Number of attachments :
      3

      Description

      I have attached the following test case. It is interesting to see that the testDelegateFileMethodAccess fails, whereas the other test where delegate methods are accessed succeed. All tests with property access fail. This is a serious bug which for example prevents me to offer important DSL functionality for my build tool.

      import org.apache.tools.ant.Project
      
      class ClosureBug extends GroovyTestCase {
          String expectedFileResult
      
          void setUp() {
              expectedFileResult = 'path'
          }
      
          void testDelegateAntPropertyAccess() {
              assert script(antScriptText('antProject')).run() instanceof Project
          }
      
          void testDelegateAntMethodAccess() {
              assert script(antScriptText('getAntProject()')).run() instanceof Project
          }
      
          void testDelegateFilePropertyAccess() {
              assert script(fileScriptText('path')).run() == expectedFileResult
          }
      
          void testDelegateFileMethodAccess() {
              assert script(fileScriptText('getPath()')).run() == expectedFileResult
          }
      
          void testDelegateCircleAccessGetterWithoutField() {
              assert script(circleScriptText('getProp3()')).run() == 'value3'
          }
      
          void testDelegateCircleAccessGetterWithField() {
              assert script(circleScriptText('getProp2()')).run() == 'value2'
          }
      
          void testDelegateCircleAccessDynamicGetter() {
              assert script(circleScriptText('getProp1()')).run() == 'value1'
          }
      
          void testDelegateCircleAccessPropertyWithoutGetter() {
              assert script(circleScriptText('prop1')).run() == 'value1'
          }
      
          void testDelegateCircleAccessPropertyWithGetter() {
              assert script(circleScriptText('prop2')).run() == 'value2'
          }
      
          void testDelegateCircleAccessPropertyWithoutField() {
              assert script(circleScriptText('prop3')).run() == 'value3'
          }
      
      
          def replaceMetaclass(Script script) {
              ExpandoMetaClass emc = new ExpandoMetaClass(script.class, false)
              emc.methodMissing = {String name, args ->
                  throw new MissingMethodException(name, Script, args)
              }
              emc.propertyMissing = {String name ->
                  throw new MissingPropertyException(name, Script)
              }
              emc.initialize()
              script.metaClass = emc
          }
      
          Script script(String text) {
              GroovyShell groovyShell = new GroovyShell()
              Script script = groovyShell.parse(text)
              replaceMetaclass(script)
              script
          }
      
          String antScriptText(String accessDirective) {
              """          
      AntBuilder ant = new AntBuilder()
      Closure cl = {
           $accessDirective
      }
      cl.delegate = ant
      cl.call()"""
          }
      
          String fileScriptText(String accessDirective) {
              """
      File file = new File('$expectedFileResult')
      Closure cl = {
           $accessDirective
      }
      cl.delegate = file
      cl.call()"""
          }
      
          String circleScriptText(String accessDirective) {
              """
      Circle circle = new Circle()
      Closure cl = {
           $accessDirective
      }
      cl.delegate = circle
      cl.call()"""
          }
      }
      
      class Circle {
          String prop1 = 'value1'
          String prop2 = 'value2'
      
          String getProp2() {
              prop2
          }
      
          String getProp3() {
              'value3'
          }
      
      }
      
      1. groovy2643.groovy
        3 kB
        Paul King
      2. groovy2643-passing.groovy
        3 kB
        Paul King
      3. Groovy2643ShortTest.groovy
        2 kB
        Hans Dockter

        Issue Links

          Activity

          Hide
          Hans Dockter added a comment -

          I forgot to mention that if I don't touch the metaclass of the Script class, everything works fine.

          Show
          Hans Dockter added a comment - I forgot to mention that if I don't touch the metaclass of the Script class, everything works fine.
          Hide
          Hans Dockter added a comment -

          There is a small bug in the unit test. Unfortunately I can't edit the issue.

          The line: File file = new File($expectedFileResult)
          should be: File file = new File('$expectedFileResult')

          Now the testDelegateFileMethodAccess succeeds and we have a simple rule. Method access works, property access not.

          Show
          Hans Dockter added a comment - There is a small bug in the unit test. Unfortunately I can't edit the issue. The line: File file = new File($expectedFileResult) should be: File file = new File('$expectedFileResult') Now the testDelegateFileMethodAccess succeeds and we have a simple rule. Method access works, property access not.
          Hide
          Hans Dockter added a comment -

          For me this is a very serious bug. Actually I wanted to release my build tool this week, but I have to wait until this is fixed. To use ant from my build scripts I have to specify the delegate to make things work:

          ant.copy(toDir: distSamplesDir)

          {delegate.fileset(dir: new File(srcRoot, 'samples'))}

          Constructs like this don't work at all:

          ant.copy(toDir: distSrcDir) {
          [srcDirs + resourceDirs + groovySrcDirs].each

          {dir -> delegate.fileset(dir: dir)}

          }

          Show
          Hans Dockter added a comment - For me this is a very serious bug. Actually I wanted to release my build tool this week, but I have to wait until this is fixed. To use ant from my build scripts I have to specify the delegate to make things work: ant.copy(toDir: distSamplesDir) {delegate.fileset(dir: new File(srcRoot, 'samples'))} Constructs like this don't work at all: ant.copy(toDir: distSrcDir) { [srcDirs + resourceDirs + groovySrcDirs] .each {dir -> delegate.fileset(dir: dir)} }
          Hide
          Hans Dockter added a comment -

          Any idea when someone has time to look into this?

          Show
          Hans Dockter added a comment - Any idea when someone has time to look into this?
          Hide
          Jim White added a comment -

          I cut down the number of errors from 9 of 10 tests to 2 of 10 tests by:

          1) Not using .metaClass and .class (using the setter/getter instead) on classes where I'm not totally sure your might not be dealing with a map.

          2) Compiling the script so that the Circle class can be found by Class.forName (putting Circle.groovy in a separate file also works). There is already a JIRA for this problem, but I forget the number. But currently a script cannot find classes that are defined in it by Class.forName.

          The remaining 2 failing tests try to access a 'path' property on g.l.Script which doesn't exist. (in 1.5.5 anyhow)

          Show
          Jim White added a comment - I cut down the number of errors from 9 of 10 tests to 2 of 10 tests by: 1) Not using .metaClass and .class (using the setter/getter instead) on classes where I'm not totally sure your might not be dealing with a map. 2) Compiling the script so that the Circle class can be found by Class.forName (putting Circle.groovy in a separate file also works). There is already a JIRA for this problem, but I forget the number. But currently a script cannot find classes that are defined in it by Class.forName. The remaining 2 failing tests try to access a 'path' property on g.l.Script which doesn't exist. (in 1.5.5 anyhow)
          Hide
          Hans Dockter added a comment -

          There was also a discussion on the mailing list about this.

          http://www.nabble.com/Delegate-property-is-ignored-by-closure-to15733032.html#a15733032

          Would the script mentioned there run for you (Here I have replaced .metaclass with the setter):

          def scriptText = '''
          AntBuilder ant = new AntBuilder()
          Closure cl = {
              // access property of AntBuilder from closure
              antProject
          }
          cl.delegate = ant
          cl.call()
          '''
          
          GroovyShell groovyShell = new GroovyShell()
          Script script = groovyShell.parse(scriptText)
          replaceMetaclass(script)
          script.run()
          
          
          def replaceMetaclass(Script script) {
              ExpandoMetaClass emc = new ExpandoMetaClass(script.class, false)
              emc.methodMissing = {String name, args ->
                  throw new MissingMethodException(name, Script, args)
              }
              emc.propertyMissing = {String name ->
                  throw new MissingPropertyException(name, Script)
              }
              emc.initialize()
              script.setMetaClass(emc)
          }
          
          Show
          Hans Dockter added a comment - There was also a discussion on the mailing list about this. http://www.nabble.com/Delegate-property-is-ignored-by-closure-to15733032.html#a15733032 Would the script mentioned there run for you (Here I have replaced .metaclass with the setter): def scriptText = ''' AntBuilder ant = new AntBuilder() Closure cl = { // access property of AntBuilder from closure antProject } cl.delegate = ant cl.call() ''' GroovyShell groovyShell = new GroovyShell() Script script = groovyShell.parse(scriptText) replaceMetaclass(script) script.run() def replaceMetaclass(Script script) { ExpandoMetaClass emc = new ExpandoMetaClass(script.class, false ) emc.methodMissing = { String name, args -> throw new MissingMethodException(name, Script, args) } emc.propertyMissing = { String name -> throw new MissingPropertyException(name, Script) } emc.initialize() script.setMetaClass(emc) }
          Hide
          Hans Dockter added a comment -

          Hi Jim,

          with all the changes you have mentioned I still have 5 failing tests (with the latest build from trunk).

          I'm wondering why this is the case.

          Could you attach your more successful testcase to this issue?

          Show
          Hans Dockter added a comment - Hi Jim, with all the changes you have mentioned I still have 5 failing tests (with the latest build from trunk). I'm wondering why this is the case. Could you attach your more successful testcase to this issue?
          Hide
          Paul King added a comment -

          made the edit to the script

          Show
          Paul King added a comment - made the edit to the script
          Hide
          Paul King added a comment -

          I attached the script I used for testing.
          It doesn't have the issue with using groovyc.
          It shows 5 errors for 1.5.5-SNAPSHOT and HEAD for me.

          Show
          Paul King added a comment - I attached the script I used for testing. It doesn't have the issue with using groovyc. It shows 5 errors for 1.5.5-SNAPSHOT and HEAD for me.
          Hide
          Paul King added a comment - - edited

          If I comment out the three lines overriding propertyMissing (file attached), then it works fine on 1.5.5-SNAPSHOT and HEAD. That would seem to indicate that propertyMissing is being triggered prior to looking for get/set methods.

          Show
          Paul King added a comment - - edited If I comment out the three lines overriding propertyMissing (file attached), then it works fine on 1.5.5-SNAPSHOT and HEAD. That would seem to indicate that propertyMissing is being triggered prior to looking for get/set methods.
          Hide
          blackdrag blackdrag added a comment -

          is there a way to reduce this test case to something that does not use AntBuilder? That would give us a more solid base to find the problem

          Show
          blackdrag blackdrag added a comment - is there a way to reduce this test case to something that does not use AntBuilder? That would give us a more solid base to find the problem
          Hide
          Hans Dockter added a comment -

          I have attached a shortened version of the testcases which includes only the test methods that do not use AntBuilder.

          Show
          Hans Dockter added a comment - I have attached a shortened version of the testcases which includes only the test methods that do not use AntBuilder.
          Hide
          blackdrag blackdrag added a comment -

          looks like in Closure:202 a MissingPropertyException should have been thrown, but the Exception really is an InvocationTargetException with the correct MissingPropertyException nested. I think this problem is related to GROOVY-2666

          Show
          blackdrag blackdrag added a comment - looks like in Closure:202 a MissingPropertyException should have been thrown, but the Exception really is an InvocationTargetException with the correct MissingPropertyException nested. I think this problem is related to GROOVY-2666
          Hide
          Hans Dockter added a comment -

          But GROOVY-2643 happens also for 1.5.4. GROOVY-2666 is only for > 1.5.4

          Show
          Hans Dockter added a comment - But GROOVY-2643 happens also for 1.5.4. GROOVY-2666 is only for > 1.5.4
          Hide
          blackdrag blackdrag added a comment -

          I see... but still, the exception I debugged was incorrect, so it is related to 2666 in 1.6 and fixing that bug is needed to make fixing this bug possible in 1.6

          Show
          blackdrag blackdrag added a comment - I see... but still, the exception I debugged was incorrect, so it is related to 2666 in 1.6 and fixing that bug is needed to make fixing this bug possible in 1.6
          Hide
          blackdrag blackdrag added a comment -

          I used your short test with the small change where GroovyShell is used "GroovyShell groovyShell = new GroovyShell(this.class.classLoader)". After this change 3 of the 6 tests do fail and that is the same for 1.5.4 and 1.6 in all these cases Groovy claims that propX does not exist for the script. I very much think this happens, because the real MissingPropertyException is wrapped in a InvokerInvocatationException. But this is nothing new, Groovy 1.0 behaved that way already. So I do guess that construction above never worked before.

          Show
          blackdrag blackdrag added a comment - I used your short test with the small change where GroovyShell is used "GroovyShell groovyShell = new GroovyShell(this.class.classLoader)". After this change 3 of the 6 tests do fail and that is the same for 1.5.4 and 1.6 in all these cases Groovy claims that propX does not exist for the script. I very much think this happens, because the real MissingPropertyException is wrapped in a InvokerInvocatationException. But this is nothing new, Groovy 1.0 behaved that way already. So I do guess that construction above never worked before.
          Hide
          Hans Dockter added a comment -

          But it should, shouldn't it?

          Show
          Hans Dockter added a comment - But it should, shouldn't it?
          Hide
          Hans Dockter added a comment -

          Before EMC was introduces this problem was probably more hidden. What I do is nothing exotic. Things you need to do for many DSL scenarios.

          Show
          Hans Dockter added a comment - Before EMC was introduces this problem was probably more hidden. What I do is nothing exotic. Things you need to do for many DSL scenarios.
          Hide
          blackdrag blackdrag added a comment -

          without EMC this problem does happen, because it is special to closures... even if missing method is used from the current class this will not happen. But a fix is to be expected in a matter of hours.

          Show
          blackdrag blackdrag added a comment - without EMC this problem does happen, because it is special to closures... even if missing method is used from the current class this will not happen. But a fix is to be expected in a matter of hours.
          Hide
          blackdrag blackdrag added a comment -

          ok, both 1.5.x and 1.6 now have fixes for this issue. It would be nice of you to test this to be sure it fixes your original issue

          Show
          blackdrag blackdrag added a comment - ok, both 1.5.x and 1.6 now have fixes for this issue. It would be nice of you to test this to be sure it fixes your original issue
          Hide
          Hans Dockter added a comment -

          The tests run now, but in my real code I still have a problem. I've used the GROOVY_1_5_X branch from this morning. I could not build HEAD, as I got test failures.

          Here is a script to reproduce my problem. It gives the following error:

          [echo] closureCall
          testprop
          testMeth
               [echo] directCall
          Caught: groovy.lang.MissingMethodException: No signature of method: Parent.echo() is applicable for argument types: (java.util.LinkedHashMap) values: {["message":"closureCall"]}
          groovy.lang.MissingMethodException: No signature of method: Parent.echo() is applicable for argument types: (java.util.LinkedHashMap) values: {["message":"closureCall"]}
          

          What is also confusing! When I uncomment: cl.resolveStrategy(Closure.DELEGATE_ONLY), I get the exception:

          Caught: groovy.lang.MissingMethodException: No signature of method: MyScript.resolveStrategy() is applicable for argument types: (java.lang.Integer) values: {3}
          groovy.lang.MissingMethodException: No signature of method: MyScript.resolveStrategy() is applicable for argument types: (java.lang.Integer) values: {3}
          
          class BuildScriptProcessor {
              void evaluate(def parent, String text) {
                  GroovyShell groovyShell = new GroovyShell()
                  Script script = groovyShell.parse(text)
                  replaceMetaclass(script, parent)
                  script.run()
              }
          
              private void replaceMetaclass(Script script, def parent) {
                  ExpandoMetaClass projectScriptExpandoMetaclass = new ExpandoMetaClass(script.class, false)
                  projectScriptExpandoMetaclass.methodMissing = {String name, args ->
                      parent.invokeMethod(name, args)
                  }
                  projectScriptExpandoMetaclass.propertyMissing = {String name ->
                      if (name == 'out') {
                          return System.out
                      }
                      parent."$name"
                  }
                  projectScriptExpandoMetaclass.initialize()
                  script.metaClass = projectScriptExpandoMetaclass
              }
          }
          
          class Parent {
              AntBuilder ant = new AntBuilder()
              String testProp = 'testprop'
          
              String testMeth() {
                  'testMeth'
              }
          
              void ant(Closure cl) {
          //        cl.resolveStrategy(Closure.DELEGATE_ONLY)
                  cl.delegate = ant
                  cl.call()
              }
          }
          
          Parent parent = new Parent()
          
          parent.ant {
             echo(message: 'closureCall')  
          }
          
          String script = """
          println testProp
          println testMeth()
          ant.echo(message: 'directCall')
          ant {
              echo(message: 'closureCall') 
          }
          """
          
          new BuildScriptProcessor().evaluate(parent, script)
          
          Show
          Hans Dockter added a comment - The tests run now, but in my real code I still have a problem. I've used the GROOVY_1_5_X branch from this morning. I could not build HEAD, as I got test failures. Here is a script to reproduce my problem. It gives the following error: [echo] closureCall testprop testMeth [echo] directCall Caught: groovy.lang.MissingMethodException: No signature of method: Parent.echo() is applicable for argument types: (java.util.LinkedHashMap) values: {["message":"closureCall"]} groovy.lang.MissingMethodException: No signature of method: Parent.echo() is applicable for argument types: (java.util.LinkedHashMap) values: {["message":"closureCall"]} What is also confusing! When I uncomment: cl.resolveStrategy(Closure.DELEGATE_ONLY), I get the exception: Caught: groovy.lang.MissingMethodException: No signature of method: MyScript.resolveStrategy() is applicable for argument types: (java.lang.Integer) values: {3} groovy.lang.MissingMethodException: No signature of method: MyScript.resolveStrategy() is applicable for argument types: (java.lang.Integer) values: {3} class BuildScriptProcessor { void evaluate(def parent, String text) { GroovyShell groovyShell = new GroovyShell() Script script = groovyShell.parse(text) replaceMetaclass(script, parent) script.run() } private void replaceMetaclass(Script script, def parent) { ExpandoMetaClass projectScriptExpandoMetaclass = new ExpandoMetaClass(script.class, false ) projectScriptExpandoMetaclass.methodMissing = { String name, args -> parent.invokeMethod(name, args) } projectScriptExpandoMetaclass.propertyMissing = { String name -> if (name == 'out') { return System .out } parent. "$name" } projectScriptExpandoMetaclass.initialize() script.metaClass = projectScriptExpandoMetaclass } } class Parent { AntBuilder ant = new AntBuilder() String testProp = 'testprop' String testMeth() { 'testMeth' } void ant(Closure cl) { // cl.resolveStrategy(Closure.DELEGATE_ONLY) cl.delegate = ant cl.call() } } Parent parent = new Parent() parent.ant { echo(message: 'closureCall') } String script = """ println testProp println testMeth() ant.echo(message: 'directCall') ant { echo(message: 'closureCall') } """ new BuildScriptProcessor().evaluate(parent, script)
          Hide
          blackdrag blackdrag added a comment -

          the issue was resolved the day after, but I let it open to see if there are more problems. I merged the change now into 1.6 and since it looks like there are no problems left I close it

          Show
          blackdrag blackdrag added a comment - the issue was resolved the day after, but I let it open to see if there are more problems. I merged the change now into 1.6 and since it looks like there are no problems left I close it

            People

            • Assignee:
              blackdrag blackdrag
              Reporter:
              Hans Dockter
            • Votes:
              1 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: