Uploaded image for project: 'Clover'
  1. Clover
  2. CLOV-1341

Implicit return in Groovy switch statement is instrumented incorrectly

XMLWordPrintable

    • Icon: Bug Bug
    • Resolution: Fixed
    • Icon: High High
    • 3.2.0
    • None
    • Instrumentation
    • None

      Problem:

      Consider the following code:

      def create(boolean b) {
        switch (b) {
            case true:
                new Integer(10)
                break
            case false:
                new String("abc")
                break
        }
}
      • switch statement is the last statement in function, so it's value is being returned from a function call
      • new Integer() / new String() are last statements before 'break' which is a function's exit point
      • thus 10 or "abc" are returned

      Clover instruments code as follows:

      def create(boolean b) { recorder.inc(0);
        switch (b) {
            case true:
                recorder.inc(1); new Integer(10)
                recorder.inc(2); break
            case false:
                recorder.inc(3); new String("abc")
                recorder.inc(4); break
        }
}
      • as a consequence, the last statement before 'break' becomes recorder.inc(), which returns void; as a result the create() function returns null

      Workaround:

      Use return instead of break in a switch statement, e.g.:

                  case true:
                return new Integer(10)
                // no break

      Fix:

      • do not add recorder.inc() before 'break'; drawback of the solution: we loose tracking of empty breaks, see below.
      def foo(int i) {
  switch (i) {
    case 1:
      recorder.inc(1); println("one")
      /*recorder.inc(2); not added*/ break;
    case 2:
    case 3: 
      /*recorder.inc(3); not added*/ break;
  }
}

foo(1)
foo(2)

      in the code above it would be nice to have "case2: case 3: recorder.inc(3); break;" colored in green, despite that code makes nothing ...

            [CLOV-1341] Implicit return in Groovy switch statement is instrumented incorrectly

            Owen made changes -
            Workflow Original: New Clover Workflow [ 897614 ] New: New Clover Workflow - Restricted [ 1474308 ]
            Piotr Swiecicki made changes -
            Workflow Original: Clover Workflow [ 895320 ] New: New Clover Workflow [ 897614 ]
            Piotr Swiecicki made changes -
            Workflow Original: reviewflow [ 550664 ] New: Clover Workflow [ 895320 ]
            Marek Parfianowicz made changes -
            Status Original: Resolved [ 5 ] New: Closed [ 6 ]
            Marek Parfianowicz made changes -
            Resolution New: Fixed [ 1 ]
            Status Original: To be reviewed [ 10026 ] New: Resolved [ 5 ]

            Marek Parfianowicz added a comment -

            NOTE: This works with Groovy 1.6.5 or higher, because the "switch as an expression" feature was implemented in Groovy 1.6.5:

            Marek Parfianowicz added a comment - NOTE: This works with Groovy 1.6.5 or higher, because the "switch as an expression" feature was implemented in Groovy 1.6.5: http://jira.codehaus.org/browse/GROOVY-3789
            Marek Parfianowicz made changes -
            Description Original: *Problem:*

            Consider the following code:

            {code:java}
            def create(boolean b) {
                    switch (b) {
                        case true:
                            new Integer(10)
                            break
                        case false:
                            new String("abc")
                            break
                    }
            }
            {code}

             * switch statement is the last statement in function, so it's value is being returned from a function call
             * new Integer() / new String() are last statements before 'break' which is a function's exit point
             * thus 10 or "abc" are returned

            Clover instruments code as follows:

            {code:java}
            def create(boolean b) { recorder.inc(0);
                    switch (b) {
                        case true:
                            recorder.inc(1); new Integer(10)
                            recorder.inc(2); break
                        case false:
                            recorder.inc(3); new String("abc")
                            recorder.inc(4); break
                    }
            }
            {code}

             * as a consequence, the last statement before 'break' becomes recorder.inc(), which returns void; as a result the create() function returns void

            *Workaround:*

            Use return instead of break in a switch statement, e.g.:

            {code:java}
                        case true:
                            return new Integer(10)
                            // no break
            {code}


            *Fix:*

             * do not add recorder.inc() before 'break'; drawback of the solution: we loose tracking of empty breaks, see below.

            {code:java}
            def foo(int i) {
              switch (i) {
                case 1:
                  recorder.inc(1); println("one")
                  /*recorder.inc(2); not added*/ break;
                case 2:
                case 3:
                  /*recorder.inc(3); not added*/ break;
              }
            }

            foo(1)
            foo(2)
            {code}

            in the code above it would be nice to have "case2: case 3: recorder.inc(3); break;" colored in green, despite that code makes nothing ...
            		New: *Problem:*

            Consider the following code:

            {code:java}
            def create(boolean b) {
                    switch (b) {
                        case true:
                            new Integer(10)
                            break
                        case false:
                            new String("abc")
                            break
                    }
            }
            {code}

             * switch statement is the last statement in function, so it's value is being returned from a function call
             * new Integer() / new String() are last statements before 'break' which is a function's exit point
             * thus 10 or "abc" are returned

            Clover instruments code as follows:

            {code:java}
            def create(boolean b) { recorder.inc(0);
                    switch (b) {
                        case true:
                            recorder.inc(1); new Integer(10)
                            recorder.inc(2); break
                        case false:
                            recorder.inc(3); new String("abc")
                            recorder.inc(4); break
                    }
            }
            {code}

             * as a consequence, the last statement before 'break' becomes recorder.inc(), which returns void; as a result the create() function returns null

            *Workaround:*

            Use return instead of break in a switch statement, e.g.:

            {code:java}
                        case true:
                            return new Integer(10)
                            // no break
            {code}


            *Fix:*

             * do not add recorder.inc() before 'break'; drawback of the solution: we loose tracking of empty breaks, see below.

            {code:java}
            def foo(int i) {
              switch (i) {
                case 1:
                  recorder.inc(1); println("one")
                  /*recorder.inc(2); not added*/ break;
                case 2:
                case 3:
                  /*recorder.inc(3); not added*/ break;
              }
            }

            foo(1)
            foo(2)
            {code}

            in the code above it would be nice to have "case2: case 3: recorder.inc(3); break;" colored in green, despite that code makes nothing ...
            Marek Parfianowicz made changes -
            Status Original: Implemented [ 10025 ] New: To be reviewed [ 10026 ]
            Marek Parfianowicz made changes -
            Status Original: In Progress [ 3 ] New: Implemented [ 10025 ]

            Marek Parfianowicz added a comment -

            Fix is implemented. It available in 3.1.12.1-SNAPSHOT:
            https://maven.atlassian.com/public-snapshot/com/cenqua/clover/clover/3.1.12.1-SNAPSHOT/

            and will be available in 3.2.0.

            Marek Parfianowicz added a comment - Fix is implemented. It available in 3.1.12.1-SNAPSHOT: https://maven.atlassian.com/public-snapshot/com/cenqua/clover/clover/3.1.12.1-SNAPSHOT/ and will be available in 3.2.0.

              mparfianowicz Marek Parfianowicz
              mparfianowicz Marek Parfianowicz
              Affected customers:
              0 This affects my team
              Watchers:
              2 Start watching this issue

                Created:
                Updated:
                Resolved: