groovy
  1. groovy
  2. GROOVY-3675

MetaClassImpl invokes call() on binding variable even if it is not a Closure

    Details

    • Type: Bug Bug
    • Status: Closed Closed
    • Priority: Major Major
    • Resolution: Won't Fix
    • Affects Version/s: 1.6.4
    • Fix Version/s: None
    • Component/s: None
    • Labels:
      None
    • Number of attachments :
      0

      Description

      The following:

      foo = 5
      foo()
      

      results in:

      Caught: groovy.lang.MissingMethodException: No signature of method: java.lang.Integer.call() is applicable for argument types: () values: []
      	at MyScript.run(MyScript.groovy:2)
      

      This is bad for at least two reasons:

      • The error message is hard to understand (should be: No signature of method: MyScript.foo() is applicable for argument types: () values: [])
      • invokeMethod and methodMissing don't get a chance to intercept the method call

      Likely solution: Change line 1093 of MetaClassImpl.java to: if(bindingVar instanceof Closure) {

        Issue Links

          Activity

          Hide
          blackdrag blackdrag added a comment -

          so it is not about an existing method or variable, but for the case it does not exist... ok...

          (1) if there is a local varibale f, do f.call().
          (2) if there is a method f(), invoke it
          (3) if there is a property f, invoke f.call()
          (4) call invokeMethod()/methodMissing

          Accroding to this scheme it is ok for the script variable to win over invokeMethod.

          Show
          blackdrag blackdrag added a comment - so it is not about an existing method or variable, but for the case it does not exist... ok... (1) if there is a local varibale f, do f.call(). (2) if there is a method f(), invoke it (3) if there is a property f, invoke f.call() (4) call invokeMethod()/methodMissing Accroding to this scheme it is ok for the script variable to win over invokeMethod.
          Hide
          Peter Niederwieser added a comment -

          >so it is not about an existing method or variable, but for the case it does not exist... ok...
          It's about the race between an existing script variable and Script.invokeMethod().

          >Accroding to this scheme it is ok for the script variable to win over invokeMethod.
          I don't see script variables mentioned in your logic, and intuitively I'd put them after invokeMethod(), just like the author of Script.invokeMethod(). Anyway, if a script variable should win over invokeMethod(), then the implementation of Script.invokeMethod() must change. Because right now, Script.invokeMethod() wins over a script variable as soon as the call occurs from within a closure (and for all other calls that don't go through MetaClassImpl).

          Show
          Peter Niederwieser added a comment - >so it is not about an existing method or variable, but for the case it does not exist... ok... It's about the race between an existing script variable and Script.invokeMethod(). >Accroding to this scheme it is ok for the script variable to win over invokeMethod. I don't see script variables mentioned in your logic, and intuitively I'd put them after invokeMethod(), just like the author of Script.invokeMethod(). Anyway, if a script variable should win over invokeMethod(), then the implementation of Script.invokeMethod() must change. Because right now, Script.invokeMethod() wins over a script variable as soon as the call occurs from within a closure (and for all other calls that don't go through MetaClassImpl).
          Hide
          blackdrag blackdrag added a comment -

          Script variables are properties, so this goes in (3).

          Show
          blackdrag blackdrag added a comment - Script variables are properties, so this goes in (3).
          Hide
          Peter Niederwieser added a comment -

          OK, then let's change Script.invokeMethod().

          Show
          Peter Niederwieser added a comment - OK, then let's change Script.invokeMethod().
          Hide
          blackdrag blackdrag added a comment -

          After rereading the issue I think we are in the state that we can close the issue already. If

          (1) if there is a local varibale f, do f.call().
          (2) if there is a method f(), invoke it
          (3) if there is a property f, invoke f.call()
          (4) call invokeMethod()/methodMissing

          is acceptable, then the important point here is that f is found in step (3) in the binding. Script's invokeMethod is not asked, because that would have happened in (4), but because a property was found we do the call for it which fails, because Integer does not provide a call() method in this example. What if it would provide one? Wouldn't it be expected to be called? The only way to fix it in the way that you may think is right is to react to the missing method there.

          If the method call is in a Closure, then yes, it behaves different. And that is the reason I don't close this issue. But this is because of the way the MOP is designed. So this will become a target for 2.0 instead

          Show
          blackdrag blackdrag added a comment - After rereading the issue I think we are in the state that we can close the issue already. If (1) if there is a local varibale f, do f.call(). (2) if there is a method f(), invoke it (3) if there is a property f, invoke f.call() (4) call invokeMethod()/methodMissing is acceptable, then the important point here is that f is found in step (3) in the binding. Script's invokeMethod is not asked, because that would have happened in (4), but because a property was found we do the call for it which fails, because Integer does not provide a call() method in this example. What if it would provide one? Wouldn't it be expected to be called? The only way to fix it in the way that you may think is right is to react to the missing method there. If the method call is in a Closure, then yes, it behaves different. And that is the reason I don't close this issue. But this is because of the way the MOP is designed. So this will become a target for 2.0 instead

            People

            • Assignee:
              blackdrag blackdrag
              Reporter:
              Peter Niederwieser
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: