facebookarchive / ide-flowtype

Flow support for Atom IDE
Other
178 stars 17 forks source link

Doesn't display lint warnings #38

Open AriaFallah opened 6 years ago

AriaFallah commented 6 years ago

Take any example from the flow linting blog post, for example:

// @flow

function square(x: ?number) {
  if (x) {
    return x * x;
  } else {
    return NaN;
  }
}

Invoking flow from the command line shows a warning but ide-flowtype will not

CLI (error)

flow
Warning: test.js:4
  4:   if (x) {
           ^ sketchy-null-number: Sketchy null check on number value. Perhaps you meant to check for null instead of for existence?
    3: function square(x: ?number) {
                          ^^^^^^^ Potentially null/undefined value.
    3: function square(x: ?number) {
                           ^^^^^^ Potentially 0 value.

Found 1 warning

IDE (no error)

screen shot 2017-10-13 at 12 49 57 pm

ballardrog commented 6 years ago

Flow lints were initially implemented with Nuclide, and we added a feature to only send Nuclide warnings for files that were currently open, to keep the system from choking on large projects with many warnings. (See this commit on the Nuclide repository and this commit on the Flow repository.) It looks like either ide-flowtype doesn't send file open/close events to the Flow server, or the Flow server isn't recognizing the way ide-flowtype is sending the events. Either of those things would cause the Flow server to think that there are no open files, and therefore not send any warnings.

I don't have a working development setup right now; do any of the active developers have any thoughts on this?

ballardrog commented 6 years ago

Update: It looks like ide-flowtype is sending "textDocument/didOpen" and "textDocument/didClose" events, (looking here) while the Flow server is expecting "didOpen" and "didClose" events. (looking here)

Arcanemagus commented 6 years ago

It sounds like the Flow server needs to be updated to follow the standard for textDocument/didOpen and textDocument/didClose.

hansonw commented 6 years ago

There's no problem with the Flow server here - ide-flowtype's requests go through the https://github.com/flowtype/flow-language-server server which does the LSP -> Flow API translation in JavaScript.

Native Flow LSP support is going to come in a (very near) future Flow version (which should address this issue), and we should be able to deprecate flow-language-server altogether.

flow-language-server isn't sending anything through the Flow connection right now on file open / file close, which probably explains this. https://github.com/flowtype/flow-language-server/blob/master/src/index.js#L100