[clang-tidy] fix for NOLINT after macro expansion

[mgehre]

Summary

When having

#define MACRO code-with-warning
MACRO; // NOLINT

clang-tidy would still show the warning, because
it searched for "NOLINT" only in the first line,
not on the second.
This caused e.g. https://llvm.org/bugs/show_bug.cgi?id=29089
(where the macro was defined in a system header). See also
the added test cases.
Now clang-tidy looks at the line of macro invocation and every line
of macro definition for a NOLINT comment.

[alexfh]

Awesome, thanks!

clang-tidy/ClangTidyDiagnosticConsumer.cpp

303 ↗

(On Diff #72214)

Just return false;?

clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp

if (!Invalid) {
     // FIXME: Handle /\bNOLINT\b(\([^)]*\))?/ as cpplint.py does.
     if (RestOfLine.find("NOLINT") != StringRef::npos) {
       return true;
     }
   }
   return false;
 }
 
+static bool LineIsMarkedWithNOLINTinMacro(SourceManager &SM,
+                                          SourceLocation Loc) {
+  while (true) {
+    if (LineIsMarkedWithNOLINT(SM, Loc))
+      return true;
+    if (!Loc.isMacroID())
+      return false;
+    Loc = SM.getImmediateExpansionRange(Loc).first;
+  }
+  return false;
+}
+
 void ClangTidyDiagnosticConsumer::HandleDiagnostic(
     DiagnosticsEngine::Level DiagLevel, const Diagnostic &Info) {
   if (Info.getLocation().isValid() &&
       DiagLevel != DiagnosticsEngine::Error &&
       DiagLevel != DiagnosticsEngine::Fatal &&
-      LineIsMarkedWithNOLINT(Diags->getSourceManager(), Info.getLocation())) {
+      LineIsMarkedWithNOLINTinMacro(Diags->getSourceManager(), Info.getLocation())) {
     ++Context.Stats.ErrorsIgnoredNOLINT;
     return;
   }
   // Count warnings/errors.
   DiagnosticConsumer::HandleDiagnostic(DiagLevel, Info);
 
   if (DiagLevel == DiagnosticsEngine::Note) {
     assert(!Errors.empty() &&

clang-tools-extra/trunk/test/clang-tidy/nolint.cpp

 // RUN: %check_clang_tidy %s google-explicit-constructor,clang-diagnostic-unused-variable %t -- -extra-arg=-Wunused-variable --
 
 class A { A(int i); };
 // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: single-argument constructors must be marked explicit
 
 class B { B(int i); }; // NOLINT
 
 class C { C(int i); }; // NOLINT(we-dont-care-about-categories-yet)
 
 void f() {
   int i;
 // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: unused variable 'i' [clang-diagnostic-unused-variable]
   int j; // NOLINT
 }
 
-// CHECK-MESSAGES: Suppressed 3 warnings (3 NOLINT)
+#define MACRO(X) class X { X(int i); };
+MACRO(D)
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: single-argument constructors must be marked explicit
+MACRO(E) // NOLINT
+
+#define MACRO_NOARG class F { F(int i); };
+MACRO_NOARG // NOLINT
+
+#define MACRO_NOLINT class G { G(int i); }; // NOLINT
+MACRO_NOLINT
+
+#define DOUBLE_MACRO MACRO(H) // NOLINT
+DOUBLE_MACRO
+
+// CHECK-MESSAGES: Suppressed 7 warnings (7 NOLINT)