Don't consider function definitions as "executable statements"

[p-bakker]

See some of the last comments on #111.

Istanbul reports function and variable declarations as executable statements. This is unwanted behavior, as it makes it seem that files that are not covered by tests are partially covered and in files that are partially covered by tests there are green annotations on declarations that are not covered.

From the discussion on #111 I get that this was done for backwards compatibility with older tools, but I think it is not correct behavior.

If it is desired to not break this backwards compatibility, please provide an option to turn off this backwards compatibility feature

[gotwarlost]

Can only do this for function declarations. Variable declarations are executable statements in the general case:

    var a = 'b', c = d(), e;

Processing the variable declaration with an assignment can execute code.

[p-bakker]

Agree, as long as the variables are only reported as executed when the file is actually loaded. This is currently the case already for variables, but functions statements are already marked as executed once in the initial coverage JSON that Istanbul generates when instrumenting the .js file.

We post-process the instrumented files before running our tests, to extract the init logic istanbul puts at the top of each .js file and copy it to another file which we force-load, so in the report that we generate after running, all .js files are mentioned, even the ones that were never loaded while running our unit tests. We do this, because otherwise you look at the report and you might think that you're doing pretty well, but in fact some files are just never touched upon and thus not reported.

A slightly related question: The instrumentation code that increments the statement count for variables when the file gets loaded is placed in front of every variable declaration. This causes a problem in our specific environment, which parses .js files, extracts all variable and function declarations and then dynamically builds JavaScript scopes at runtime. In this process the bits of instrumentation code increments statement counts for variable declarations gets lost. Would it be possible to hoist them up to the top of the file, just below the Istanbul initialization code at the top of each file (we already preproces that to make sure it doesn't get lost in our environment)

And on a side note: while digging into this, I saw statement call counts are recorded in a JavaScript object with keys being incremental numbers stored as strings. AFAIK it would improve performance to use an Array instead of an Object here. This would also decrease the size of the instrumentation code (no more (double) quotes needed)

[idmillington]

I think this is correct behavior. Functions are first class objects, to be created, passed, mangled, whatever. As such if a line would be a valid statement when creating (say) an integer or date object, it should be a valid statement when creating a function.

Some coding standards require:

var foo = function foo() { ... }:

to make this clear. And since this is equivalent to

function foo() { ... };

it seems correct to not treat them differently in terms of coverage.

Very happy for this to be an option, but the current behavior seems correct to me and my use cases.

[abelmokadem]

Any updates on this topic? Otherwise I will try to make this an option and submit a pull request.

[p-bakker]

I did look into it at the time, couldn't quickly figure it out and then never got around to it anymore.

Still think it is a needed option though, so if you can figure it out and provide a PR that would be great, I think