https://github.com/voyager-jhk updated https://github.com/llvm/llvm-project/pull/202575
>From 9bc8e9f423d2fab686c15f1bcc2ac21817442f4d Mon Sep 17 00:00:00 2001 From: voyager-jhk <[email protected]> Date: Tue, 9 Jun 2026 18:32:50 +0800 Subject: [PATCH] [clang-tidy] Support LineFilter in .clang-tidy configuration files Previously, the `LineFilter` option was strictly read from the global command-line arguments. This made it completely inaccessible to Language Servers like `clangd`, which rely exclusively on `.clang-tidy` configuration files. This patch adds `LineFilter` to the `ClangTidyOptions` struct, implements YAML serialization, and defines merge semantics. The `DiagnosticConsumer` is updated to fallback to the local configuration only if the global command-line filter is empty, preserving the expected CLI-override behavior. Fixes #59263 --- .../ClangTidyDiagnosticConsumer.cpp | 19 +--- .../clang-tidy/ClangTidyOptions.cpp | 19 ++++ .../clang-tidy/ClangTidyOptions.h | 10 ++ clang-tools-extra/clangd/ParsedAST.cpp | 107 +++++++++++------- .../clangd/unittests/DiagnosticsTests.cpp | 19 ++++ clang-tools-extra/docs/ReleaseNotes.rst | 4 + .../infrastructure/line-filter-config.cpp | 18 +++ .../clang-tidy/ClangTidyOptionsTest.cpp | 15 +++ 8 files changed, 155 insertions(+), 56 deletions(-) create mode 100644 clang-tools-extra/test/clang-tidy/infrastructure/line-filter-config.cpp diff --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp index 88d0a433bc7fb..8b78a60ffff41 100644 --- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp +++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp @@ -482,19 +482,12 @@ void ClangTidyDiagnosticConsumer::HandleDiagnostic( bool ClangTidyDiagnosticConsumer::passesLineFilter(StringRef FileName, unsigned LineNumber) const { - if (Context.getGlobalOptions().LineFilter.empty()) - return true; - for (const FileFilter &Filter : Context.getGlobalOptions().LineFilter) { - if (FileName.ends_with(Filter.Name)) { - if (Filter.LineRanges.empty()) - return true; - return llvm::any_of( - Filter.LineRanges, [&](const FileFilter::LineRange &Range) { - return Range.first <= LineNumber && LineNumber <= Range.second; - }); - } - } - return false; + const std::vector<FileFilter> *Filters = + &Context.getGlobalOptions().LineFilter; + if (Filters->empty() && Context.getOptions().LineFilter) + Filters = &*Context.getOptions().LineFilter; + + return tidy::passesLineFilter(*Filters, FileName, LineNumber); } void ClangTidyDiagnosticConsumer::forwardDiagnostic(const Diagnostic &Info) { diff --git a/clang-tools-extra/clang-tidy/ClangTidyOptions.cpp b/clang-tools-extra/clang-tidy/ClangTidyOptions.cpp index 0a0f392346f6d..9111f2a7ccd7b 100644 --- a/clang-tools-extra/clang-tidy/ClangTidyOptions.cpp +++ b/clang-tools-extra/clang-tidy/ClangTidyOptions.cpp @@ -11,6 +11,7 @@ #include "clang/Basic/DiagnosticIDs.h" #include "clang/Basic/LLVM.h" #include "llvm/ADT/SmallString.h" +#include "llvm/ADT/STLExtras.h" #include "llvm/ADT/StringExtras.h" #include "llvm/Support/Debug.h" #include "llvm/Support/ErrorOr.h" @@ -243,6 +244,7 @@ template <> struct MappingTraits<ClangTidyOptions> { IO.mapOptional("UseColor", Options.UseColor); IO.mapOptional("SystemHeaders", Options.SystemHeaders); IO.mapOptional("CustomChecks", Options.CustomChecks); + IO.mapOptional("LineFilter", Options.LineFilter); } }; @@ -303,6 +305,7 @@ ClangTidyOptions &ClangTidyOptions::mergeWith(const ClangTidyOptions &Other, overrideValue(FormatStyle, Other.FormatStyle); overrideValue(User, Other.User); overrideValue(UseColor, Other.UseColor); + overrideValue(LineFilter, Other.LineFilter); mergeVectors(ExtraArgs, Other.ExtraArgs); mergeVectors(ExtraArgsBefore, Other.ExtraArgsBefore); mergeVectors(RemovedArgs, Other.RemovedArgs); @@ -526,6 +529,22 @@ FileOptionsBaseProvider::tryReadConfigFile(StringRef Directory) { return std::nullopt; } +bool passesLineFilter(ArrayRef<FileFilter> LineFilter, StringRef FileName, + unsigned LineNumber) { + if (LineFilter.empty()) + return true; + for (const FileFilter &Filter : LineFilter) { + if (!FileName.ends_with(Filter.Name)) + continue; + if (Filter.LineRanges.empty()) + return true; + return llvm::any_of(Filter.LineRanges, [LineNumber](const auto &Range) { + return Range.first <= LineNumber && LineNumber <= Range.second; + }); + } + return false; +} + /// Parses -line-filter option and stores it to the \c Options. std::error_code parseLineFilter(StringRef LineFilter, clang::tidy::ClangTidyGlobalOptions &Options) { diff --git a/clang-tools-extra/clang-tidy/ClangTidyOptions.h b/clang-tools-extra/clang-tidy/ClangTidyOptions.h index 73fdbabd5bdba..2873e03e9abbd 100644 --- a/clang-tools-extra/clang-tidy/ClangTidyOptions.h +++ b/clang-tools-extra/clang-tidy/ClangTidyOptions.h @@ -10,6 +10,7 @@ #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CLANGTIDYOPTIONS_H #include "clang/Basic/DiagnosticIDs.h" +#include "llvm/ADT/ArrayRef.h" #include "llvm/ADT/IntrusiveRefCntPtr.h" #include "llvm/ADT/SmallString.h" #include "llvm/ADT/StringMap.h" @@ -70,6 +71,9 @@ struct ClangTidyOptions { /// Checks filter. std::optional<std::string> Checks; + /// Line filter. + std::optional<std::vector<FileFilter>> LineFilter; + /// WarningsAsErrors filter. std::optional<std::string> WarningsAsErrors; @@ -338,6 +342,12 @@ class FileOptionsProvider : public FileOptionsBaseProvider { std::vector<OptionsSource> getRawOptions(StringRef FileName) override; }; +/// Returns true if a diagnostic at \p LineNumber in \p FileName should be +/// displayed according to \p LineFilter. An empty filter allows all +/// diagnostics. +bool passesLineFilter(ArrayRef<FileFilter> LineFilter, StringRef FileName, + unsigned LineNumber); + /// Parses LineFilter from JSON and stores it to the \p Options. std::error_code parseLineFilter(StringRef LineFilter, ClangTidyGlobalOptions &Options); diff --git a/clang-tools-extra/clangd/ParsedAST.cpp b/clang-tools-extra/clangd/ParsedAST.cpp index e2a49f384a3e9..4265d5cd41f3f 100644 --- a/clang-tools-extra/clangd/ParsedAST.cpp +++ b/clang-tools-extra/clangd/ParsedAST.cpp @@ -361,6 +361,23 @@ void applyWarningOptions(llvm::ArrayRef<std::string> ExtraArgs, } } +bool passesTidyLineFilter(const tidy::ClangTidyOptions &Options, + SourceLocation Loc, const SourceManager &SM) { + if (!Options.LineFilter) + return true; + + if (!Loc.isValid()) + return true; + + FileID FID = SM.getDecomposedExpansionLoc(Loc).first; + OptionalFileEntryRef File = SM.getFileEntryRefForID(FID); + if (!File) + return true; + + return tidy::passesLineFilter(*Options.LineFilter, File->getName(), + SM.getExpansionLineNumber(Loc)); +} + std::vector<Diag> getIncludeCleanerDiags(ParsedAST &AST, llvm::StringRef Code, const ThreadsafeFS &TFS) { auto &Cfg = Config::current(); @@ -583,50 +600,54 @@ ParsedAST::build(llvm::StringRef Filename, const ParseInputs &Inputs, SourceLocation()); } - ASTDiags.setLevelAdjuster([&](DiagnosticsEngine::Level DiagLevel, - const clang::Diagnostic &Info) { - auto It = OverriddenSeverity.find(Info.getID()); - if (It != OverriddenSeverity.end()) - DiagLevel = It->second; - - if (!CTChecks.empty()) { - std::string CheckName = CTContext->getCheckName(Info.getID()); - bool IsClangTidyDiag = !CheckName.empty(); - if (IsClangTidyDiag) { - if (Cfg.Diagnostics.Suppress.contains(CheckName)) - return DiagnosticsEngine::Ignored; - // Check for suppression comment. Skip the 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 to avoid I/O. - // We let suppression comments take precedence over warning-as-error - // to match clang-tidy's behaviour. - bool IsInsideMainFile = - Info.hasSourceManager() && - isInsideMainFile(Info.getLocation(), Info.getSourceManager()); - SmallVector<tooling::Diagnostic, 1> TidySuppressedErrors; - if (IsInsideMainFile && CTContext->shouldSuppressDiagnostic( - DiagLevel, Info, TidySuppressedErrors, - /*AllowIO=*/false, - /*EnableNolintBlocks=*/true)) { - // FIXME: should we expose the suppression error (invalid use of - // NOLINT comments)? - return DiagnosticsEngine::Ignored; - } - if (!CTContext->getOptions().SystemHeaders.value_or(false) && - Info.hasSourceManager() && - Info.getSourceManager().isInSystemMacro(Info.getLocation())) - return DiagnosticsEngine::Ignored; - - // Check for warning-as-error. - if (DiagLevel == DiagnosticsEngine::Warning && - CTContext->treatAsError(CheckName)) { - return DiagnosticsEngine::Error; + ASTDiags.setLevelAdjuster( + [&](DiagnosticsEngine::Level DiagLevel, const clang::Diagnostic &Info) { + auto It = OverriddenSeverity.find(Info.getID()); + if (It != OverriddenSeverity.end()) + DiagLevel = It->second; + + if (!CTChecks.empty()) { + std::string CheckName = CTContext->getCheckName(Info.getID()); + bool IsClangTidyDiag = !CheckName.empty(); + if (IsClangTidyDiag) { + if (Cfg.Diagnostics.Suppress.contains(CheckName)) + return DiagnosticsEngine::Ignored; + if (Info.hasSourceManager() && + !passesTidyLineFilter(ClangTidyOpts, Info.getLocation(), + Info.getSourceManager())) + return DiagnosticsEngine::Ignored; + // Check for suppression comment. Skip the 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 to avoid I/O. We let + // suppression comments take precedence over warning-as-error to + // match clang-tidy's behaviour. + bool IsInsideMainFile = + Info.hasSourceManager() && + isInsideMainFile(Info.getLocation(), Info.getSourceManager()); + SmallVector<tooling::Diagnostic, 1> TidySuppressedErrors; + if (IsInsideMainFile && CTContext->shouldSuppressDiagnostic( + DiagLevel, Info, TidySuppressedErrors, + /*AllowIO=*/false, + /*EnableNolintBlocks=*/true)) { + // FIXME: should we expose the suppression error (invalid use of + // NOLINT comments)? + return DiagnosticsEngine::Ignored; + } + if (!CTContext->getOptions().SystemHeaders.value_or(false) && + Info.hasSourceManager() && + Info.getSourceManager().isInSystemMacro(Info.getLocation())) + return DiagnosticsEngine::Ignored; + + // Check for warning-as-error. + if (DiagLevel == DiagnosticsEngine::Warning && + CTContext->treatAsError(CheckName)) { + return DiagnosticsEngine::Error; + } + } } - } - } - return DiagLevel; - }); + return DiagLevel; + }); // Add IncludeFixer which can recover diagnostics caused by missing includes // (e.g. incomplete type) and attach include insertion fixes to diagnostics. diff --git a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp index 4258f7faf34fd..4d06ad4ac41a8 100644 --- a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp +++ b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp @@ -2055,6 +2055,25 @@ TEST(Diagnostics, TidyDiagsArentAffectedFromWerror) { diagSeverity(DiagnosticsEngine::Error))))); } +TEST(Diagnostics, TidyLineFilter) { + Annotations Test(R"cpp( + $skip[[typedef int Skip]]; + $keep[[typedef int Keep]]; + $after[[typedef int After]]; + )cpp"); + TestTU TU = TestTU::withCode(Test.code()); + unsigned KeepLine = Test.range("keep").start.line + 1; + TU.ClangTidyProvider = [KeepLine](tidy::ClangTidyOptions &Opts, + llvm::StringRef) { + Opts.Checks = "modernize-use-using"; + Opts.LineFilter = + std::vector<tidy::FileFilter>{{"TestTU.cpp", {{KeepLine, KeepLine}}}}; + }; + EXPECT_THAT(TU.build().getDiagnostics(), + ifTidyChecks(ElementsAre(Diag( + Test.range("keep"), "use 'using' instead of 'typedef'")))); +} + TEST(Diagnostics, DeprecatedDiagsAreHints) { ClangdDiagnosticOptions Opts; std::optional<clangd::Diagnostic> Diag; diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index c369b1fd8b373..e1883ae3b1fcc 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -200,6 +200,10 @@ Improvements to clang-tidy - Improved :program:`clang-tidy` ``-store-check-profile`` by generating valid JSON when the source file path contains characters that require JSON escaping. +- Added support for specifying ``LineFilter`` in :program:`clang-tidy` + configuration files. This is particularly useful for tools such as + :program:`clangd` that consume ``.clang-tidy`` files directly. + - Ensured that :program:`clang-tidy` and the clang compiler uses the same logic for the suppression of compiler diagnostics in system headers and expansions of macros defined in system headers. Previously the default setting of tidy diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/line-filter-config.cpp b/clang-tools-extra/test/clang-tidy/infrastructure/line-filter-config.cpp new file mode 100644 index 0000000000000..5a79ba2f39012 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/infrastructure/line-filter-config.cpp @@ -0,0 +1,18 @@ +// RUN: clang-tidy -checks='-*,modernize-use-using' -config="{LineFilter: [{name: 'line-filter-config.cpp', lines: [[8, 8]]}]}" %s -- 2>&1 | FileCheck --check-prefix=CONFIG %s +// RUN: clang-tidy -checks='-*,modernize-use-using' -config="{LineFilter: [{name: 'line-filter-config.cpp', lines: [[8, 8]]}]}" -line-filter="[{name: 'line-filter-config.cpp', lines: [[12, 12]]}]" %s -- 2>&1 | FileCheck --check-prefix=CLI %s + +typedef int BeforeLineFilter; +// CONFIG-NOT: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef' +// CLI-NOT: :[[@LINE-2]]:1: warning: use 'using' instead of 'typedef' + +typedef int ConfigWarn; +// CONFIG: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef' [modernize-use-using] +// CLI-NOT: :[[@LINE-2]]:1: warning: use 'using' instead of 'typedef' + +typedef int CliWarn; +// CONFIG-NOT: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef' +// CLI: :[[@LINE-2]]:1: warning: use 'using' instead of 'typedef' [modernize-use-using] + +typedef int AfterLineFilter; +// CONFIG-NOT: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef' +// CLI-NOT: :[[@LINE-2]]:1: warning: use 'using' instead of 'typedef' diff --git a/clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp b/clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp index 3f86f65c1ce65..78e3f7abd2d70 100644 --- a/clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp +++ b/clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp @@ -75,6 +75,21 @@ TEST(ParseLineFilter, ValidFilter) { EXPECT_EQ(1000u, Options.LineFilter[2].LineRanges[0].second); } +TEST(ClangTidyOptions, PassesLineFilter) { + EXPECT_TRUE(passesLineFilter({}, "a.cpp", 1)); + + std::vector<FileFilter> Filters = {{"file.cpp", {{10, 12}, {20, 20}}}}; + EXPECT_TRUE(passesLineFilter(Filters, "/path/file.cpp", 10)); + EXPECT_TRUE(passesLineFilter(Filters, "/path/file.cpp", 12)); + EXPECT_TRUE(passesLineFilter(Filters, "/path/file.cpp", 20)); + EXPECT_FALSE(passesLineFilter(Filters, "/path/file.cpp", 13)); + EXPECT_FALSE(passesLineFilter(Filters, "/path/other.cpp", 10)); + + Filters = {{"header.h", {}}}; + EXPECT_TRUE(passesLineFilter(Filters, "/path/header.h", 999)); + EXPECT_FALSE(passesLineFilter(Filters, "/path/file.cpp", 1)); +} + TEST(ParseConfiguration, ValidConfiguration) { llvm::ErrorOr<ClangTidyOptions> Options = parseConfiguration(llvm::MemoryBufferRef( _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
