instrumentation breaks JavaScript labels

[sourcehunter]

When placing a JavaScript label in CoffeeScript in front of a loop like this:

`loopLabel: //`
for item in someArray
    ...

the instrumentation will insert a line similar to

_$jscoverage[".\\somefile.coffee"][12]++;

between the label and the loop head, thus generating invalid JavaScript:

loopLabel: //;
_$jscoverage[".\\somefile.coffee"][12]++;
for (_i = 0, _len = someArray.length; _i < _len; _i++) {
    item = someArray[_i];
    ...

[jwalton]

I didn't even know you could do this. :P

[jwalton]

Some thoughts about fixing this:

In compiler terms, the embedded javascript label becomes a "value". e.g. this:

arr = [1,2,3,4]
`foo: //`
for item in arr
  console.log item

Becomes an AST that looks like:

Block
  Assign
    Value "arr"
    Value
      Arr
        Value "1"
        Value "2"
        Value "3"
        Value "4"
  Value "foo: //"
  For
    Block
      Call
        Value "console"
          Access "log"
        Value "item"
    Value "arr"

So we could say "Whenever there's a Value that's a direct descendant of a Block, don't annotate the next line." This sort of works, although of course there are cases where you might WANT to annotate the next line, where your embedded JS is just a statement.

Another alternative would be to use something along the lines of Microsoft C++ style pragmas, so we could do:

arr = [1,2,3,4]
###
#pragma coverage off
###
`foo: //`
for item in arr
  console.log item
###
#pragma coverage on
###

So the pragmas would disable and enable coverage selectively. This is perhaps a more flexible solution, which applications outside of this particular problem. It might be desirable to allow the "off" pragma to be inside the loop, although it might be tricky to allow the pragmas to span multiple scopes in such a fashion.

[sourcehunter]

Your #pragma idea sounds intriguing. I'm not sure how much effort this would be and how much effort one should spend on the above issue. You can always rewrite code using labels into code using no labels. Thats what we have done for now to continue working with coffee-coverage.

Another way would be to parse the embedded JavaScript and try to get a deeper understanding of it.

[silkentrance]

While pragma sounds intriguing, it will lead to code for which no coverage data was collected for. As such I would call this a bad practice. And bad programmers tend to fall back to a pragma(tic) solution whenever they feel necessary.

http://stackoverflow.com/questions/46586/goto-still-considered-harmful

[jwalton]

We have this new pragma feature in 0.5.0, which might do what you want using jscoverage, but it still instruments the line for Istanbul and just marks the statement as skip in Istanbul's statementMap. We could add another pragma which says "Don't even instrument this line."