https://github.com/voyager-jhk updated https://github.com/llvm/llvm-project/pull/202575
>From 37e868e73ad8fbc77de797e9089899307c570f39 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 ++++ .../infrastructure/line-filter-config.cpp | 18 +++ .../clang-tidy/ClangTidyOptionsTest.cpp | 15 +++ 7 files changed, 151 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 48d8c76bd4db6..4f8bc0fdd7726 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 2ef23ede09972..582e276bbcf56 100644 --- a/clang-tools-extra/clang-tidy/ClangTidyOptions.cpp +++ b/clang-tools-extra/clang-tidy/ClangTidyOptions.cpp @@ -10,6 +10,7 @@ #include "ClangTidyModule.h" #include "clang/Basic/DiagnosticIDs.h" #include "clang/Basic/LLVM.h" +#include "llvm/ADT/STLExtras.h" #include "llvm/ADT/SmallString.h" #include "llvm/ADT/StringExtras.h" #include "llvm/Support/Debug.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(llvm::ArrayRef<FileFilter> LineFilter, + llvm::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..65eb6032a87e7 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(llvm::ArrayRef<FileFilter> LineFilter, + llvm::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 df56420cd7f24..1a1bc05b0632e 100644 --- a/clang-tools-extra/clangd/ParsedAST.cpp +++ b/clang-tools-extra/clangd/ParsedAST.cpp @@ -383,6 +383,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(); @@ -605,50 +622,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 6d91ac1ef1e8e..b8703a8384bcc 100644 --- a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp +++ b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp @@ -2078,6 +2078,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/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
