search

rpl / flow-coverage-report

Generate flow coverage reports in JSON, HTML and in the console.
MIT License
505 stars 42 forks source link

fix: Support '@flow strict' pragma #150

Closed zeorin closed 6 years ago

zeorin commented 6 years ago

Trying to run coverage on a file that has the new @flow strict pragma results in:

Error while generating Flow Coverage Report: Error: Unexpected missing flow annotation on index.js Error: Unexpected missing flow annotation on index.js
    at /home/zeorin/code/my-framework/node_modules/flow-coverage-report/dist/lib/flow.js:336:15
    at Array.forEach (<anonymous>)
    at summarizeAnnotations (/home/zeorin/code/my-framework/node_modules/flow-coverage-report/dist/lib/flow.js:324:13)
    at /home/zeorin/code/my-framework/node_modules/flow-coverage-report/dist/lib/flow.js:577:45
    at <anonymous>
    at process._tickCallback (internal/process/next_tick.js:188:7)

This fixes that by treating it the same as the regular @flow pragma for coverage purposes.

benfjohnson commented 6 years ago

@zeorin I was halfway through a nearly identical PR when I saw this had been opened 😆. Hopefully this can be merged soon!

rpl commented 6 years ago

@zeorin Thanks! This looks great (and thanks a lot for taking care of adding some additional test case for it, it is very much appreciated), during the weekend I've tried this locally a bit and prepared to rebase it on top on #154.

I'd like to apply some minimal changes to it (I'm going to add some comments to this pull request to highlight them), but don't worry about applying them in this PR, if we agree on them I'm going to just open a new pull request which contains those changes and the rebase of this one on top of #154 (so that I can spare you to look into any of the conflicts that I already solved locally).

rpl commented 6 years ago

I'm closing this PR in favor of the rebased version part of #155 (which already have the conflicts resolved, and some additional small change we agreed on).