groovy
  1. groovy
  2. GROOVY-2849

A property name in a nested closure interferes with a class property when refering with "this.prop"

    Details

    • Type: Bug Bug
    • Status: Closed Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.6-beta-1
    • Fix Version/s: 1.6-rc-1, 1.5.8, 1.7-beta-1
    • Component/s: parser
    • Labels:
      None
    • Testcase included:
      yes
    • Number of attachments :
      4

      Description

      I guess this may be a parser bug (I'm not sure).

      This class is working fine:

      private class TestClosure {
      def test = 1
      def c1 = {
      def test = 2
      def c2 =

      { println this.test this.test+=10 println this.test }

      c2()
      }
      }

      new TestClosure().c1()

      But when we change:

      def c2 =

      { println this.test this.test+=10 println this.test test = 3 }

      then thing is screwed up.
      'this.test' is resolved to be 'test' of the c1 closure, which is not correct.

      1. Groovy2849Bug_v2.groovy
        0.8 kB
        Roshan Dawrani
      2. Groovy2849Bug.groovy
        0.3 kB
        Chanwit Kaewkasi
      3. Groovy2849BugAST.groovy.xml
        43 kB
        Roshan Dawrani

        Issue Links

          Activity

          Hide
          Roshan Dawrani added a comment -

          Thank you.

          With so much on your plate, I am sure it is only natural to forget some details as their origin moves farther into past.

          I am glad that I'll probably now have a tiny fingerprint on ACG.

          I will make the negation related changes, test it with all 3 versions (1.5.8, 1.6, and 1.7) and re-attach the patches.

          Shall I also now include the fix for this same issue for method calls (ACG -> visitMethodCallExpression())?

          rgds,
          Roshan

          Show
          Roshan Dawrani added a comment - Thank you. With so much on your plate, I am sure it is only natural to forget some details as their origin moves farther into past. I am glad that I'll probably now have a tiny fingerprint on ACG. I will make the negation related changes, test it with all 3 versions (1.5.8, 1.6, and 1.7) and re-attach the patches. Shall I also now include the fix for this same issue for method calls (ACG -> visitMethodCallExpression())? rgds, Roshan
          Hide
          blackdrag blackdrag added a comment -

          Since the fix for this does not fix the other issue automatically we should probably split both issues and make a new one for the method call problem. We may also need further discussion on the fix... depends on the fix, so having a new fix should make things a bit more easy and allows us to close this issue here without thinking about the other issue.

          Show
          blackdrag blackdrag added a comment - Since the fix for this does not fix the other issue automatically we should probably split both issues and make a new one for the method call problem. We may also need further discussion on the fix... depends on the fix, so having a new fix should make things a bit more easy and allows us to close this issue here without thinking about the other issue.
          Hide
          Roshan Dawrani added a comment -

          I have re-attached the fix patches for this issue as per the latest discussions till this point - moving the condition negation inside the helper method and reversing the order of conditions.

          I have tested the fix with all 1.5.8, 1.6-RC1 and 1.7-beta-1 versions.

          Also, from isNotExplicitThisInClosure(), I have removed the use of isThisExpression() as this condition is already met in visitAttributeOrProperty().

          Also, as suggested, for the similar issue found with method calls, I have opened a separate bug (GROOVY-3156).

          rgds,
          Roshan

          Show
          Roshan Dawrani added a comment - I have re-attached the fix patches for this issue as per the latest discussions till this point - moving the condition negation inside the helper method and reversing the order of conditions. I have tested the fix with all 1.5.8, 1.6-RC1 and 1.7-beta-1 versions. Also, from isNotExplicitThisInClosure(), I have removed the use of isThisExpression() as this condition is already met in visitAttributeOrProperty(). Also, as suggested, for the similar issue found with method calls, I have opened a separate bug ( GROOVY-3156 ). rgds, Roshan
          Hide
          Roshan Dawrani added a comment -

          Re-attaching the test case too - removed the testing of method call interference from it as it has its JIRA item now. Moved that test in its JIRA.

          Show
          Roshan Dawrani added a comment - Re-attaching the test case too - removed the testing of method call interference from it as it has its JIRA item now. Moved that test in its JIRA.
          Hide
          Roshan Dawrani added a comment -

          Fixed by correcting the issue related to property resolution inside a nested closure.

          Show
          Roshan Dawrani added a comment - Fixed by correcting the issue related to property resolution inside a nested closure.

            People

            • Assignee:
              Roshan Dawrani
              Reporter:
              Chanwit Kaewkasi
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: