diff --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h --- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h +++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h @@ -217,6 +217,17 @@ bool AllowEnablingAnalyzerAlphaCheckers; }; +/// Check whether a given diagnostic should be suppressed due to the presence +/// of a "NOLINT" suppression comment. +/// This is exposed so that other tools that present clang-tidy diagnostics +/// (such as clangd) can respect the same suppression rules as clang-tidy. +/// This does not handle suppression of notes following a suppressed diagnostic; +/// that is left to the caller is it requires maintaining state in between calls +/// to this function. +bool ShouldSuppressDiagnostic(DiagnosticsEngine::Level DiagLevel, + const Diagnostic &Info, + ClangTidyContext &Context); + /// \brief A diagnostic consumer that turns each \c Diagnostic into a /// \c SourceManager-independent \c ClangTidyError. // @@ -246,7 +257,7 @@ /// \brief Updates \c LastErrorRelatesToUserCode and LastErrorPassesLineFilter /// according to the diagnostic \p Location. - void checkFilters(SourceLocation Location, const SourceManager& Sources); + void checkFilters(SourceLocation Location, const SourceManager &Sources); bool passesLineFilter(StringRef FileName, unsigned LineNumber) const; ClangTidyContext &Context; diff --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp --- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp +++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp @@ -160,11 +160,13 @@ bool contains(StringRef S) { switch (auto &Result = Cache[S]) { - case Yes: return true; - case No: return false; - case None: - Result = Globs.contains(S) ? Yes : No; - return Result == Yes; + case Yes: + return true; + case No: + return false; + case None: + Result = Globs.contains(S) ? Yes : No; + return Result == Yes; } llvm_unreachable("invalid enum"); } @@ -381,16 +383,29 @@ return false; } +namespace clang { +namespace tidy { + +bool ShouldSuppressDiagnostic(DiagnosticsEngine::Level DiagLevel, + const Diagnostic &Info, + ClangTidyContext &Context) { + return Info.getLocation().isValid() && + DiagLevel != DiagnosticsEngine::Error && + DiagLevel != DiagnosticsEngine::Fatal && + LineIsMarkedWithNOLINTinMacro(Info.getSourceManager(), + Info.getLocation(), Info.getID(), + Context); +} + +} // namespace tidy +} // namespace clang + void ClangTidyDiagnosticConsumer::HandleDiagnostic( DiagnosticsEngine::Level DiagLevel, const Diagnostic &Info) { if (LastErrorWasIgnored && DiagLevel == DiagnosticsEngine::Note) return; - if (Info.getLocation().isValid() && DiagLevel != DiagnosticsEngine::Error && - DiagLevel != DiagnosticsEngine::Fatal && - LineIsMarkedWithNOLINTinMacro(Info.getSourceManager(), - Info.getLocation(), Info.getID(), - Context)) { + if (ShouldSuppressDiagnostic(DiagLevel, Info, Context)) { ++Context.Stats.ErrorsIgnoredNOLINT; // Ignored a warning, should ignore related notes as well LastErrorWasIgnored = true; diff --git a/clang-tools-extra/clangd/ClangdUnit.cpp b/clang-tools-extra/clangd/ClangdUnit.cpp --- a/clang-tools-extra/clangd/ClangdUnit.cpp +++ b/clang-tools-extra/clangd/ClangdUnit.cpp @@ -237,6 +237,39 @@ const LangOptions &LangOpts; }; +// A wrapper around StoreDiags to handle suppression comments for +// clang-tidy diagnostics (and possibly other clang-tidy customizations in the +// future). +class ClangdDiagnosticConsumer : public StoreDiags { +public: + void HandleDiagnostic(DiagnosticsEngine::Level DiagLevel, + const clang::Diagnostic &Info) override { + if (LastErrorWasIgnored && DiagLevel == DiagnosticsEngine::Note) + return; + + if (CTContext) { + bool isClangTidyDiag = !CTContext->getCheckName(Info.getID()).empty(); + if (isClangTidyDiag && + tidy::ShouldSuppressDiagnostic(DiagLevel, Info, *CTContext)) { + // Ignored a warning, should ignore related notes as well + LastErrorWasIgnored = true; + return; + } + } + + LastErrorWasIgnored = false; + StoreDiags::HandleDiagnostic(DiagLevel, Info); + } + + void setClangTidyContext(tidy::ClangTidyContext *CTContext) { + this->CTContext = CTContext; + } + +private: + bool LastErrorWasIgnored = false; + tidy::ClangTidyContext *CTContext = nullptr; +}; + } // namespace void dumpAST(ParsedAST &AST, llvm::raw_ostream &OS) { @@ -256,7 +289,7 @@ const PrecompiledPreamble *PreamblePCH = Preamble ? &Preamble->Preamble : nullptr; - StoreDiags ASTDiags; + ClangdDiagnosticConsumer ASTDiags; std::string Content = Buffer->getBuffer(); auto Clang = prepareCompilerInstance(std::move(CI), PreamblePCH, @@ -294,6 +327,7 @@ CTContext->setASTContext(&Clang->getASTContext()); CTContext->setCurrentFile(MainInput.getFile()); CTFactories.createChecks(CTContext.getPointer(), CTChecks); + ASTDiags.setClangTidyContext(CTContext.getPointer()); Preprocessor *PP = &Clang->getPreprocessor(); for (const auto &Check : CTChecks) { // FIXME: the PP callbacks skip the entire preamble.