[PATCH] D80753: [clang-tidy] remove duplicate fixes of alias checkers

[Aaron Ballman via Phabricator via llvm-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM aside from a few small nits, but please wait a bit for @njames93 in case they have final thoughts.



================
Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:758
+
+    // Unique error, we keep it and move along
+    if (Inserted.second) {
----------------
Add full stops to the end of your comments (here and elsewhere).


================
Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:777
+                          "or make sure they are both configured the same. "
+                          "Aliased checkers: '{0}', '{1}'",
+                          ExistingError.DiagnosticName, Error.DiagnosticName)
----------------
Daniel599 wrote:
> njames93 wrote:
> > You don't really need the Aliased checkers note as that is wrapped in the initial diagnostic message. 
> > Also if there was a check that could spew out 3 different fix-its for the same diagnostic, this would result in the duplication note being made twice, maybe the notes should be cleared too?
> regarding your comment about 3 fix-it, as we walked before, there isn't a case like that (I didn't find any) as I wanted to make a test out of it.
> I added the last line as it would show who are the two that conflict (supporting the future case of more than 2 aliased checkers),
> I can remove it if it doesn't help, let me know.
> 
> > maybe the notes should be cleared too?
> I am open for suggestions about how to re-write this message :)
> I also think it could be better
> 
I would reword it to be a run-on sentence: `cannot apply fix-it because an alias checker has suggested a different fix-it; please remove one of the checkers ('{0}', '{1}') or ensure they are both configured the same`

This also nicely removes the nit about terminating punctuation in a diagnostic.


================
Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h:250
   void removeIncompatibleErrors();
+  void removeDuplicatedFixesOfAliasCheckers();
 
----------------
Daniel599 wrote:
> maybe I need to rename this method since now it's removing the errors also, I`ll do it when I get back from work.
I think this should still be renamed. How about `removeDuplicatedDiagnosticsOfAliasCheckers()`?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D80753/new/

https://reviews.llvm.org/D80753