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,23 @@ 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. +/// The `CheckMacroExpansion` parameter determines whether the function should +/// handle the case where the diagnostic is inside a macro expansion. A degree +/// of control over this is needed because handling this case can require +/// examining source files other than the one in which the diagnostic is +/// located, and in some use cases we cannot rely on such other files being +/// mapped in the SourceMapper. +bool ShouldSuppressDiagnostic(DiagnosticsEngine::Level DiagLevel, + const Diagnostic &Info, ClangTidyContext &Context, + bool CheckMacroExpansion = true); + /// \brief A diagnostic consumer that turns each \c Diagnostic into a /// \c SourceManager-independent \c ClangTidyError. // @@ -246,7 +263,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,30 @@ return false; } +namespace clang { +namespace tidy { + +bool ShouldSuppressDiagnostic(DiagnosticsEngine::Level DiagLevel, + const Diagnostic &Info, ClangTidyContext &Context, + bool CheckMacroExpansion) { + return Info.getLocation().isValid() && + DiagLevel != DiagnosticsEngine::Error && + DiagLevel != DiagnosticsEngine::Fatal && + (CheckMacroExpansion ? LineIsMarkedWithNOLINTinMacro + : LineIsMarkedWithNOLINT)(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,40 @@ 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: + ClangdDiagnosticConsumer() { + filterDiagnostics([this](DiagnosticsEngine::Level DiagLevel, + const clang::Diagnostic &Info, + bool IsInsideMainFile) { + if (CTContext) { + bool isClangTidyDiag = !CTContext->getCheckName(Info.getID()).empty(); + // Skip the ShouldSuppressDiagnostic check for diagnostics not in + // the main file, because we don't want that function to query the + // source buffer for preamble files. For the same reason, we ask + // ShouldSuppressDiagnostic not to follow macro expansions, since those + // might take us into a preamble file as well. + if (isClangTidyDiag && IsInsideMainFile && + tidy::ShouldSuppressDiagnostic(DiagLevel, Info, *CTContext, + /* CheckMacroExpansion = */ false)) { + return false; + } + } + return true; + }); + } + + void setClangTidyContext(tidy::ClangTidyContext *CTContext) { + this->CTContext = CTContext; + } + +private: + tidy::ClangTidyContext *CTContext = nullptr; +}; + } // namespace void dumpAST(ParsedAST &AST, llvm::raw_ostream &OS) { @@ -256,7 +290,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 +328,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. diff --git a/clang-tools-extra/clangd/Diagnostics.h b/clang-tools-extra/clangd/Diagnostics.h --- a/clang-tools-extra/clangd/Diagnostics.h +++ b/clang-tools-extra/clangd/Diagnostics.h @@ -113,16 +113,23 @@ using DiagFixer = std::function(DiagnosticsEngine::Level, const clang::Diagnostic &)>; + using DiagFilter = + std::function; /// If set, possibly adds fixes for diagnostics using \p Fixer. void contributeFixes(DiagFixer Fixer) { this->Fixer = Fixer; } + /// If set, ignore diagnostics for which \p Filter returns false. + void filterDiagnostics(DiagFilter Filter) { this->Filter = Filter; } private: void flushLastDiag(); DiagFixer Fixer = nullptr; + DiagFilter Filter = nullptr; std::vector Output; llvm::Optional LangOpts; llvm::Optional LastDiag; + bool LastErrorWasIgnored = false; }; } // namespace clangd diff --git a/clang-tools-extra/clangd/Diagnostics.cpp b/clang-tools-extra/clangd/Diagnostics.cpp --- a/clang-tools-extra/clangd/Diagnostics.cpp +++ b/clang-tools-extra/clangd/Diagnostics.cpp @@ -311,8 +311,17 @@ return; } + if (LastErrorWasIgnored && DiagLevel == DiagnosticsEngine::Note) + return; + bool InsideMainFile = isInsideMainFile(Info); + if (Filter && !Filter(DiagLevel, Info, InsideMainFile)) { + LastErrorWasIgnored = true; + return; + } + LastErrorWasIgnored = false; + auto FillDiagBase = [&](DiagBase &D) { D.Range = diagnosticRange(Info, *LangOpts); llvm::SmallString<64> Message;