Duplicated comments when analyzer returns dupes

[dpordomingo]

If an analyzer returns twice the same comment, then they're posted twice into the PR.
(same file::line:analyzer::message)

I discovered it working with style-analyzer/typos_checker and posting comments for go, because the bblfsh/go-driver returns the same suggestion twice bblfsh/go-driver#28

[dpordomingo]

I think this issue is also causing that other issue:
src-d/lookout-gometalint-analyzer#27

[se7entyse7en]

@smacker @carlosms I discussed on Slack with @dpordomingo about this link, how do we want to solve this?

[smacker]

Imo, if an analyzer returns duplicate comments it's a bug in an analyzer and should be fixed there.

[dpordomingo]

Should the analyzer remember the previous comments he returned?
Example:

• comment at file level, saying something like: "this file was not formatted"

I think I could find more examples if needed;

Should lookout post that same message every time the analyzer says so?

[smacker]

I don't understand you.

comment at file level, saying something like: "this file was not formatted"

is filtered by lookout if there are rebases/multiple pushes.

[dpordomingo]

If you push two times two different commits, making the analyzer to run twice, it can return the same message for the same file, that lookout will post twice.

Another example is the one described by #263, making surce{d} Lookout to report three times the same comment:

• Improves logging message for failed analysis #442 (comment)
• Improves logging message for failed analysis #442 (comment)
• Improves logging message for failed analysis #442 (comment)

[smacker]

Lookout shouldn't post the same comment twice if it was pushed twice.
We save posted comments in lookout and filter them: https://github.com/src-d/lookout/blob/master/server/server.go#L388

If it happens, it's a bug. And we need a proper test case to reproduce it & fix it.

Though it's not related to the issue. This issue is about an analyzer returning dups when we run it once.

[smacker]

I took a closer look and those comments. They are actually different. They have the same text but posted on different lines.

[dpordomingo]

After talking irl with @smacker we identified different scenarios:

• a) an analyzer returning twice the same comment in one EventResponse,
  It will be posted twice the same comment.
• b) an analyzer returning the same comment in different EventResponse,
  Not a bug; no duplicated comments; It will be posted the first comment, and ignored the following ones
• c) an analyzer returning the same message for different lines of code,
  Not a bug; even expected; It will be posted all of them.

This issue describes a, and during the discussion I mentioned an example of c, that is not a real bug (and it would be difficult to avoid in lookout)

Current behavior is not a bug since it was designed that way.
Avoiding a (and maybe c) could be considered as a feature requests.

c is handled (and avoided) by #455