Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Flow 0.97 LSP causes most code to be marked as uncovered, even if the server reports (correctly) that it's 100% covered #7654

Closed
lxe opened this issue Apr 19, 2019 · 12 comments

Comments

@lxe
Copy link

lxe commented Apr 19, 2019

Update:

Issue was introduced in 313d6c7


Tracking in flow-for-vscode as well: flow/flow-for-vscode#332

Some LSP-related change between 0.96 and 0.97 causes VSCode listening to flow-bin/flow-osx-v0.97.0/flow lsp --from vscode to mark most code as uncovered:

With flowtype.flow-for-vscode-1.1.1 :

image

Observe that the task bar shows coverage 100%, but every line is marked as uncovered.

Flow 0.96 does not cause flow-for-vscode to exhibit this behavior:

Screen Shot 2019-04-19 at 1 25 11 PM

@lxe lxe changed the title Flow 0.97 LSP causes most code to be marked as uncovered, even if the server report that it's 100% covered Flow 0.97 LSP causes most code to be marked as uncovered, even if the server reports (correctly) that it's 100% covered Apr 19, 2019
@lxe
Copy link
Author

lxe commented Apr 19, 2019

v0.96.0...v0.97.0 bet it's something to do with trusted types

@lxe
Copy link
Author

lxe commented Apr 19, 2019

Looks like 3801a02 - v0.96.0 (3 weeks ago) <avik@fb.com> is not the same as flow-bin@0.96.0, as this problem also happens there, but flow-bin@0.96.0 is working fine.

@lxe
Copy link
Author

lxe commented Apr 19, 2019

This issue appeared in this commit: 313d6c7

cc @dsainati1

@dsainati1
Copy link
Contributor

Hmmmm. Is this limited to VSCode?

@lxe
Copy link
Author

lxe commented Apr 19, 2019

I tried with

[options]
trust_mode=none
trust_mode=check
trust_mode=silent

With no difference

@lxe
Copy link
Author

lxe commented Apr 19, 2019

Hmmmm. Is this limited to VSCode?

@dsainati1 I haven't tried any other editor or plugin. I guess maybe the flow lsp extension needs to update to take 313d6c7#diff-84d0c6a799d463d1e58a368fdfb53bf5L1009 into the account

@dsainati1
Copy link
Contributor

I can reproduce with my editor. I think this has to do with the fact that I left trust-coverage mode off for the LSP's persistent coverage command, but I suppose that didn't have the effect I thought it did. Unsure why our tests didn't catch this, but I can fix it for the 0.98 release of Flow.

@lxe
Copy link
Author

lxe commented Apr 19, 2019

Thanks @dsainati1

I'm trying to debug the flow-for-vscode side of things. Getting coveredPercent 100 with a bunch of uncoveredRanges

{
  "coveredPercent": 100,
  "uncoveredRanges": [
    {
      "range": {
        "start": {
          "line": 8,
          "character": 7
        },
        "end": {
          "line": 8,
          "character": 15
        }
      }
    },
    ...
  ],
  "defaultMessage": "Un-type checked code. Consider adding type annotations."
}

@dsainati1
Copy link
Contributor

Problem is this line: 313d6c7#diff-84d0c6a799d463d1e58a368fdfb53bf5R1024

No idea how our tests didn't catch this. Thanks for the bug report!

@dsainati1
Copy link
Contributor

Cool I have a fix for this. Should land by Monday and make it into the next release.

@dsainati1 dsainati1 reopened this Apr 19, 2019
facebook-github-bot pushed a commit that referenced this issue Apr 20, 2019
Summary:
#7654 reported that coverage highlighting in the IDE was marking every location as uncovered, even though the total coverage percentage was correct. This was a result of a bug in a previous commit that enabled trust coverage for the CLI; I had accidentally removed a `None` case from a match statement.

I also improved our test coverage for this feature, since the tests should have failed as a result of the mistake but they didn't.

Fixes #7654

Reviewed By: panagosg7

Differential Revision: D15021980

fbshipit-source-id: ff80a5e29fb4d37bb95f72aa6164045eefc298be
@vicapow
Copy link
Contributor

vicapow commented Apr 22, 2019

Thanks for the quick fix!

@lxe
Copy link
Author

lxe commented Apr 22, 2019

Nice! Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants