Skip to content
This repository has been archived by the owner on Aug 7, 2023. It is now read-only.

Incorrectly showing 1 flow error as 2 linter messages #115

Open
MakuraYami opened this issue Jul 8, 2016 · 3 comments
Open

Incorrectly showing 1 flow error as 2 linter messages #115

MakuraYami opened this issue Jul 8, 2016 · 3 comments

Comments

@MakuraYami
Copy link

MakuraYami commented Jul 8, 2016

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.

@nmn
Copy link
Contributor

nmn commented Jul 9, 2016

@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?

@MakuraYami
Copy link
Author

MakuraYami commented Jul 11, 2016

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.

@nmn
Copy link
Contributor

nmn commented Jul 11, 2016

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.

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

No branches or pull requests

2 participants