linter-flow icon indicating copy to clipboard operation
linter-flow copied to clipboard

Incorrectly showing 1 flow error as 2 linter messages

Open MakuraYami opened this issue 9 years ago • 3 comments

I got a flow errors that splits up in 3 messages (blame + comment + blame), this is a single error. but the way linter-flow handles it is it displays an error for both blames, which give exactly the same output

[warning] identifier `EventEmitter` Expected polymorphic type instead of module `events`at line 20 col 24

here is my error for reference

{"flowVersion":"0.28.0","errors":[{"kind":"infer","level":"error","message":[{"context":"class MyClass extends EventEmitter {","descr":"identifier `EventEmitter`","type":"Blame","loc":{"source":"/path/to/index.js","type":"SourceFile","start":{"line":20,"column":24,"offset":407},"end":{"line":20,"column":35,"offset":419}},"path":"/path/to/index.js","line":20,"endline":20,"start":24,"end":35},{"context":null,"descr":"Expected polymorphic type instead of","type":"Comment","path":"","line":0,"endline":0,"start":1,"end":0},{"context":"class MyClass extends EventEmitter {","descr":"module `events`","type":"Blame","loc":{"source":"/path/to/index.js","type":"SourceFile","start":{"line":20,"column":24,"offset":407},"end":{"line":20,"column":35,"offset":419}},"path":"/path/to/index.js","line":20,"endline":20,"start":24,"end":35}]}],"passed":false}

I tried myself and changed messages.js:26

const blameMessages = flowMessages.filter(m => m.type === 'Blame');

to

const blameMessages = flowMessages.reduce((msgs, m) => {
  if (m.type === 'Blame' && msgs.filter((m2) => m.context === m2.context).length === 0)
    msgs.push(m);
  return msgs;
},[]);

and that solves the issue.

MakuraYami avatar Jul 08 '16 09:07 MakuraYami

@MakuraYami This behavior has been included on purpose. It may be a little more noisy, but it makes it easier to see errors as they happen.

Specially in React, if you fail to pass some required props, the error originates at the Component definition. This way, there is also an error where you use the component where the real error usually lies.

Does that make sense?

nmn avatar Jul 09 '16 04:07 nmn

Ah, I did not know it would give two different contexts on the same error. I have not tried flow with react all that much, it seemed logical since flow has a array of errors that each one represents one line. Doesn't my separating context's still solve that? if it gives two different context, it will still be 2 Linter messages. There is just no point displaying 2 exactly the same messages when its not these edge cases. If not at the location I showed then later in the process before passing them off to atom, filter out so the messages are unique.

MakuraYami avatar Jul 11 '16 08:07 MakuraYami

I'll look into it. There is no perfect solution here. Even in non-React code. You can often call a function with the wrong arguments and the error can originate at the function definition. If the function is defined in a different module, you won't even know you called the function wrong till much later.

So, showing the multiple errors still helps. It can get noisy though, and maybe we can use some heuristic to make it less noisy.

nmn avatar Jul 11 '16 18:07 nmn