groovy
  1. groovy
  2. GROOVY-2836

Misleading MissingMethodException thrown when this() is called inside a constructor

    Details

    • Number of attachments :
      1

      Description

      class Foo {
          Foo(String s) {}
      
          Foo(Date d) {
              println "call to this() must be the first statement of a ctor"
              this(d.toString())
          }
      }
      
      new Foo(new Date())
      

      In the snippet above, I'm calling this() after a println call.
      But in Java, it is mandatory that calls to this() or super() inside a constructor are the first statements of the block.
      Here I get the following stacktrace:

      groovy.lang.MissingMethodException: No signature of method: Foo.call() is applicable for argument types: (java.lang.String) values: {Fri May 16 23:48:20 CEST 2008}
      	at Foo.invokeMethod(Script35)
      	at Foo.<init>(Script35:6)
      	at Script35.run(Script35:10)
      

      The compiler should check that calls to this() or super() are done as the first statement, and not afterwards.

        Activity

        Hide
        Roshan Dawrani added a comment -

        (This bug was work in progress and also I haven't yet gone through the new guidelines, so just attaching the patches one more time.)

        Attaching the patches for versions 1.5.8, 1.6-RC-1, 1.7-beta-1. One existing testcase also needed to be patched as it was in conflict with the requirement of this JIRA.

        The fix done is "throwing a compilation error when this(..) / super(..) when not on 1st statement"

        "groovy.g" makes sure that constructor calls on first statement (this(..) / super(..)) are captured explicitly by marking their AST node types as CTOR_CALL / SUPER_CTOR_CALL. Later, AntlrParserPlugin converts such captured calls into ConstructorCallExpression.

        But if this(..)/super(..) comes after the first statement, it is turned into a MethodCallExpression representing this.call(..)/super.call(..), which causes failures as noted in the current bug ("No signature of method: Foo.call() is applicable....").

        The change is made to AntlrParserPlugin.methodCallExpression(), which now rejects the this()/super() kind of calls with a compilation error saying "Constructor call must be the first statement in a constructor."

        Here is one testcase to show the new behavior:

        void testThisConstructorCallNotOnFirstStmt() {
            def scriptStr = """
                class ThisConstructorCall {
                    public ThisConstructorCall() {
                        println 'dummy first statement'
                        this(19)
                    }
                    public ThisConstructorCall(int b) {
                        println "another dummy statement"
                    }
                }
                1
            """
            try {
                assertScript scriptStr
                fail("The script compilation should have failed as it calls constructor on 2nd statement")
            } catch(MultipleCompilationErrorsException mcee) {
                def text = mcee.toString();
                assert text.contains("Constructor call must be the first statement in a constructor.")
            }
        }
        
        Show
        Roshan Dawrani added a comment - (This bug was work in progress and also I haven't yet gone through the new guidelines, so just attaching the patches one more time.) Attaching the patches for versions 1.5.8, 1.6-RC-1, 1.7-beta-1. One existing testcase also needed to be patched as it was in conflict with the requirement of this JIRA. The fix done is "throwing a compilation error when this(..) / super(..) when not on 1st statement" "groovy.g" makes sure that constructor calls on first statement (this(..) / super(..)) are captured explicitly by marking their AST node types as CTOR_CALL / SUPER_CTOR_CALL. Later, AntlrParserPlugin converts such captured calls into ConstructorCallExpression. But if this(..)/super(..) comes after the first statement, it is turned into a MethodCallExpression representing this.call(..)/super.call(..), which causes failures as noted in the current bug ("No signature of method: Foo.call() is applicable...."). The change is made to AntlrParserPlugin.methodCallExpression(), which now rejects the this()/super() kind of calls with a compilation error saying "Constructor call must be the first statement in a constructor." Here is one testcase to show the new behavior: void testThisConstructorCallNotOnFirstStmt() { def scriptStr = """ class ThisConstructorCall { public ThisConstructorCall() { println 'dummy first statement' this (19) } public ThisConstructorCall( int b) { println "another dummy statement" } } 1 """ try { assertScript scriptStr fail( "The script compilation should have failed as it calls constructor on 2nd statement" ) } catch (MultipleCompilationErrorsException mcee) { def text = mcee.toString(); assert text.contains( "Constructor call must be the first statement in a constructor." ) } }
        Hide
        blackdrag blackdrag added a comment -

        your test case is trying to test if something does compile or not.. I suggest you put the test instead in gls.invocation and use CompilableTestSupport. This has a method shouldNotCompile and shouldCompile, both taking Strings containing the script

        Show
        blackdrag blackdrag added a comment - your test case is trying to test if something does compile or not.. I suggest you put the test instead in gls.invocation and use CompilableTestSupport. This has a method shouldNotCompile and shouldCompile, both taking Strings containing the script
        Hide
        Roshan Dawrani added a comment -

        Hi,
        I have moved the new tests into gls/invocation/ConstructorDelegationTest.groovy and am attaching the 3 revised patches.
        rgds,
        Roshan

        Show
        Roshan Dawrani added a comment - Hi, I have moved the new tests into gls/invocation/ConstructorDelegationTest.groovy and am attaching the 3 revised patches. rgds, Roshan
        Hide
        blackdrag blackdrag added a comment -

        looks good to me now... one more small thing, that you don't have to do for this one... I prefer the style

        shouldNotCompile """
          ...
        """
        

        without additional local variable.

        Show
        blackdrag blackdrag added a comment - looks good to me now... one more small thing, that you don't have to do for this one... I prefer the style shouldNotCompile """ ... """ without additional local variable.
        Hide
        Roshan Dawrani added a comment -

        Updating the fix versions as issue was found in all 3 branches.

        Show
        Roshan Dawrani added a comment - Updating the fix versions as issue was found in all 3 branches.
        Hide
        Roshan Dawrani added a comment -

        Fixed the issue by throwing a compilation error if constructor is invoked after 1st statement.

        Show
        Roshan Dawrani added a comment - Fixed the issue by throwing a compilation error if constructor is invoked after 1st statement.

          People

          • Assignee:
            Roshan Dawrani
            Reporter:
            Guillaume Laforge
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: