From e9087fe75c8652a442bada0a7e02cba0df781074 Mon Sep 17 00:00:00 2001 From: Alexander Kornienko Date: Mon, 25 Mar 2019 12:36:30 +0000 Subject: [PATCH] [clang-tidy] Separate the check-facing interface Summary: Move ClangTidyCheck to a separate header/.cpp Switch checks to #include "ClangTidyCheck.h" Mention ClangTidyCheck.h in the docs Reviewers: hokein, gribozavr, aaron.ballman Reviewed By: hokein Subscribers: mgorny, javed.absar, xazax.hun, arphaman, jdoerfert, llvm-commits, cfe-commits Tags: #clang, #llvm Differential Revision: https://reviews.llvm.org/D59714 llvm-svn: 356890 --- clang-tools-extra/clang-tidy/CMakeLists.txt | 1 + clang-tools-extra/clang-tidy/ClangTidy.cpp | 45 ----- clang-tools-extra/clang-tidy/ClangTidy.h | 181 +------------------ clang-tools-extra/clang-tidy/ClangTidyCheck.cpp | 71 ++++++++ clang-tools-extra/clang-tidy/ClangTidyCheck.h | 197 +++++++++++++++++++++ clang-tools-extra/docs/clang-tidy/Contributing.rst | 5 +- .../clang-tools-extra/clang-tidy/BUILD.gn | 1 + 7 files changed, 274 insertions(+), 227 deletions(-) create mode 100644 clang-tools-extra/clang-tidy/ClangTidyCheck.cpp create mode 100644 clang-tools-extra/clang-tidy/ClangTidyCheck.h diff --git a/clang-tools-extra/clang-tidy/CMakeLists.txt b/clang-tools-extra/clang-tidy/CMakeLists.txt index 1cdd021..84328ba 100644 --- a/clang-tools-extra/clang-tidy/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/CMakeLists.txt @@ -4,6 +4,7 @@ set(LLVM_LINK_COMPONENTS add_clang_library(clangTidy ClangTidy.cpp + ClangTidyCheck.cpp ClangTidyModule.cpp ClangTidyDiagnosticConsumer.cpp ClangTidyOptions.cpp diff --git a/clang-tools-extra/clang-tidy/ClangTidy.cpp b/clang-tools-extra/clang-tidy/ClangTidy.cpp index dd5faab..42ab826 100644 --- a/clang-tools-extra/clang-tidy/ClangTidy.cpp +++ b/clang-tools-extra/clang-tidy/ClangTidy.cpp @@ -449,51 +449,6 @@ ClangTidyOptions::OptionMap ClangTidyASTConsumerFactory::getCheckOptions() { return Options; } -DiagnosticBuilder ClangTidyCheck::diag(SourceLocation Loc, StringRef Message, - DiagnosticIDs::Level Level) { - return Context->diag(CheckName, Loc, Message, Level); -} - -void ClangTidyCheck::run(const ast_matchers::MatchFinder::MatchResult &Result) { - // For historical reasons, checks don't implement the MatchFinder run() - // callback directly. We keep the run()/check() distinction to avoid interface - // churn, and to allow us to add cross-cutting logic in the future. - check(Result); -} - -OptionsView::OptionsView(StringRef CheckName, - const ClangTidyOptions::OptionMap &CheckOptions) - : NamePrefix(CheckName.str() + "."), CheckOptions(CheckOptions) {} - -std::string OptionsView::get(StringRef LocalName, StringRef Default) const { - const auto &Iter = CheckOptions.find(NamePrefix + LocalName.str()); - if (Iter != CheckOptions.end()) - return Iter->second; - return Default; -} - -std::string OptionsView::getLocalOrGlobal(StringRef LocalName, - StringRef Default) const { - auto Iter = CheckOptions.find(NamePrefix + LocalName.str()); - if (Iter != CheckOptions.end()) - return Iter->second; - // Fallback to global setting, if present. - Iter = CheckOptions.find(LocalName.str()); - if (Iter != CheckOptions.end()) - return Iter->second; - return Default; -} - -void OptionsView::store(ClangTidyOptions::OptionMap &Options, - StringRef LocalName, StringRef Value) const { - Options[NamePrefix + LocalName.str()] = Value; -} - -void OptionsView::store(ClangTidyOptions::OptionMap &Options, - StringRef LocalName, int64_t Value) const { - store(Options, LocalName, llvm::itostr(Value)); -} - std::vector getCheckNames(const ClangTidyOptions &Options, bool AllowEnablingAnalyzerAlphaCheckers) { diff --git a/clang-tools-extra/clang-tidy/ClangTidy.h b/clang-tools-extra/clang-tidy/ClangTidy.h index af315ca..a9433f6 100644 --- a/clang-tools-extra/clang-tidy/ClangTidy.h +++ b/clang-tools-extra/clang-tidy/ClangTidy.h @@ -9,16 +9,11 @@ #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CLANGTIDY_H #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CLANGTIDY_H +#include "ClangTidyCheck.h" #include "ClangTidyDiagnosticConsumer.h" #include "ClangTidyOptions.h" -#include "clang/ASTMatchers/ASTMatchFinder.h" -#include "clang/Basic/Diagnostic.h" -#include "clang/Basic/SourceManager.h" -#include "clang/Tooling/Refactoring.h" -#include "llvm/ADT/StringExtras.h" #include "llvm/Support/raw_ostream.h" #include -#include #include namespace clang { @@ -30,180 +25,6 @@ class CompilationDatabase; namespace tidy { -/// \brief Provides access to the ``ClangTidyCheck`` options via check-local -/// names. -/// -/// Methods of this class prepend ``CheckName + "."`` to translate check-local -/// option names to global option names. -class OptionsView { -public: - /// \brief Initializes the instance using \p CheckName + "." as a prefix. - OptionsView(StringRef CheckName, - const ClangTidyOptions::OptionMap &CheckOptions); - - /// \brief Read a named option from the ``Context``. - /// - /// Reads the option with the check-local name \p LocalName from the - /// ``CheckOptions``. If the corresponding key is not present, returns - /// \p Default. - std::string get(StringRef LocalName, StringRef Default) const; - - /// \brief Read a named option from the ``Context``. - /// - /// Reads the option with the check-local name \p LocalName from local or - /// global ``CheckOptions``. Gets local option first. If local is not present, - /// falls back to get global option. If global option is not present either, - /// returns Default. - std::string getLocalOrGlobal(StringRef LocalName, StringRef Default) const; - - /// \brief Read a named option from the ``Context`` and parse it as an - /// integral type ``T``. - /// - /// Reads the option with the check-local name \p LocalName from the - /// ``CheckOptions``. If the corresponding key is not present, returns - /// \p Default. - template - typename std::enable_if::value, T>::type - get(StringRef LocalName, T Default) const { - std::string Value = get(LocalName, ""); - T Result = Default; - if (!Value.empty()) - StringRef(Value).getAsInteger(10, Result); - return Result; - } - - /// \brief Read a named option from the ``Context`` and parse it as an - /// integral type ``T``. - /// - /// Reads the option with the check-local name \p LocalName from local or - /// global ``CheckOptions``. Gets local option first. If local is not present, - /// falls back to get global option. If global option is not present either, - /// returns Default. - template - typename std::enable_if::value, T>::type - getLocalOrGlobal(StringRef LocalName, T Default) const { - std::string Value = getLocalOrGlobal(LocalName, ""); - T Result = Default; - if (!Value.empty()) - StringRef(Value).getAsInteger(10, Result); - return Result; - } - - /// \brief Stores an option with the check-local name \p LocalName with string - /// value \p Value to \p Options. - void store(ClangTidyOptions::OptionMap &Options, StringRef LocalName, - StringRef Value) const; - - /// \brief Stores an option with the check-local name \p LocalName with - /// ``int64_t`` value \p Value to \p Options. - void store(ClangTidyOptions::OptionMap &Options, StringRef LocalName, - int64_t Value) const; - -private: - std::string NamePrefix; - const ClangTidyOptions::OptionMap &CheckOptions; -}; - -/// \brief Base class for all clang-tidy checks. -/// -/// To implement a ``ClangTidyCheck``, write a subclass and override some of the -/// base class's methods. E.g. to implement a check that validates namespace -/// declarations, override ``registerMatchers``: -/// -/// ~~~{.cpp} -/// void registerMatchers(ast_matchers::MatchFinder *Finder) override { -/// Finder->addMatcher(namespaceDecl().bind("namespace"), this); -/// } -/// ~~~ -/// -/// and then override ``check(const MatchResult &Result)`` to do the actual -/// check for each match. -/// -/// A new ``ClangTidyCheck`` instance is created per translation unit. -/// -/// FIXME: Figure out whether carrying information from one TU to another is -/// useful/necessary. -class ClangTidyCheck : public ast_matchers::MatchFinder::MatchCallback { -public: - /// \brief Initializes the check with \p CheckName and \p Context. - /// - /// Derived classes must implement the constructor with this signature or - /// delegate it. If a check needs to read options, it can do this in the - /// constructor using the Options.get() methods below. - ClangTidyCheck(StringRef CheckName, ClangTidyContext *Context) - : CheckName(CheckName), Context(Context), - Options(CheckName, Context->getOptions().CheckOptions) { - assert(Context != nullptr); - assert(!CheckName.empty()); - } - - /// \brief Override this to register ``PPCallbacks`` with ``Compiler``. - /// - /// This should be used for clang-tidy checks that analyze preprocessor- - /// dependent properties, e.g. the order of include directives. - virtual void registerPPCallbacks(CompilerInstance &Compiler) {} - - /// \brief Override this to register ``PPCallbacks`` in the preprocessor. - /// - /// This should be used for clang-tidy checks that analyze preprocessor- - /// dependent properties, e.g. include directives and macro definitions. - /// - /// There are two Preprocessors to choose from that differ in how they handle - /// modular #includes: - /// - PP is the real Preprocessor. It doesn't walk into modular #includes and - /// thus doesn't generate PPCallbacks for their contents. - /// - ModuleExpanderPP preprocesses the whole translation unit in the - /// non-modular mode, which allows it to generate PPCallbacks not only for - /// the main file and textual headers, but also for all transitively - /// included modular headers when the analysis runs with modules enabled. - /// When modules are not enabled ModuleExpanderPP just points to the real - /// preprocessor. - virtual void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP, - Preprocessor *ModuleExpanderPP) {} - - /// \brief Override this to register AST matchers with \p Finder. - /// - /// This should be used by clang-tidy checks that analyze code properties that - /// dependent on AST knowledge. - /// - /// You can register as many matchers as necessary with \p Finder. Usually, - /// "this" will be used as callback, but you can also specify other callback - /// classes. Thereby, different matchers can trigger different callbacks. - /// - /// If you need to merge information between the different matchers, you can - /// store these as members of the derived class. However, note that all - /// matches occur in the order of the AST traversal. - virtual void registerMatchers(ast_matchers::MatchFinder *Finder) {} - - /// \brief ``ClangTidyChecks`` that register ASTMatchers should do the actual - /// work in here. - virtual void check(const ast_matchers::MatchFinder::MatchResult &Result) {} - - /// \brief Add a diagnostic with the check's name. - DiagnosticBuilder diag(SourceLocation Loc, StringRef Description, - DiagnosticIDs::Level Level = DiagnosticIDs::Warning); - - /// \brief Should store all options supported by this check with their - /// current values or default values for options that haven't been overridden. - /// - /// The check should use ``Options.store()`` to store each option it supports - /// whether it has the default value or it has been overridden. - virtual void storeOptions(ClangTidyOptions::OptionMap &Options) {} - -private: - void run(const ast_matchers::MatchFinder::MatchResult &Result) override; - StringRef getID() const override { return CheckName; } - std::string CheckName; - ClangTidyContext *Context; - -protected: - OptionsView Options; - /// \brief Returns the main file name of the current translation unit. - StringRef getCurrentMainFile() const { return Context->getCurrentFile(); } - /// \brief Returns the language options from the context. - LangOptions getLangOpts() const { return Context->getLangOpts(); } -}; - class ClangTidyCheckFactories; class ClangTidyASTConsumerFactory { diff --git a/clang-tools-extra/clang-tidy/ClangTidyCheck.cpp b/clang-tools-extra/clang-tidy/ClangTidyCheck.cpp new file mode 100644 index 0000000..fbf1176 --- /dev/null +++ b/clang-tools-extra/clang-tidy/ClangTidyCheck.cpp @@ -0,0 +1,71 @@ +//===--- ClangTidyCheck.cpp - clang-tidy ------------------------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "ClangTidyCheck.h" + +namespace clang { +namespace tidy { + +ClangTidyCheck::ClangTidyCheck(StringRef CheckName, ClangTidyContext *Context) + : CheckName(CheckName), Context(Context), + Options(CheckName, Context->getOptions().CheckOptions) { + assert(Context != nullptr); + assert(!CheckName.empty()); +} + +DiagnosticBuilder ClangTidyCheck::diag(SourceLocation Loc, StringRef Message, + DiagnosticIDs::Level Level) { + return Context->diag(CheckName, Loc, Message, Level); +} + +void ClangTidyCheck::run(const ast_matchers::MatchFinder::MatchResult &Result) { + // For historical reasons, checks don't implement the MatchFinder run() + // callback directly. We keep the run()/check() distinction to avoid interface + // churn, and to allow us to add cross-cutting logic in the future. + check(Result); +} + +ClangTidyCheck::OptionsView::OptionsView(StringRef CheckName, + const ClangTidyOptions::OptionMap &CheckOptions) + : NamePrefix(CheckName.str() + "."), CheckOptions(CheckOptions) {} + +std::string ClangTidyCheck::OptionsView::get(StringRef LocalName, + StringRef Default) const { + const auto &Iter = CheckOptions.find(NamePrefix + LocalName.str()); + if (Iter != CheckOptions.end()) + return Iter->second; + return Default; +} + +std::string +ClangTidyCheck::OptionsView::getLocalOrGlobal(StringRef LocalName, + StringRef Default) const { + auto Iter = CheckOptions.find(NamePrefix + LocalName.str()); + if (Iter != CheckOptions.end()) + return Iter->second; + // Fallback to global setting, if present. + Iter = CheckOptions.find(LocalName.str()); + if (Iter != CheckOptions.end()) + return Iter->second; + return Default; +} + +void ClangTidyCheck::OptionsView::store(ClangTidyOptions::OptionMap &Options, + StringRef LocalName, + StringRef Value) const { + Options[NamePrefix + LocalName.str()] = Value; +} + +void ClangTidyCheck::OptionsView::store(ClangTidyOptions::OptionMap &Options, + StringRef LocalName, + int64_t Value) const { + store(Options, LocalName, llvm::itostr(Value)); +} + +} // namespace tidy +} // namespace clang diff --git a/clang-tools-extra/clang-tidy/ClangTidyCheck.h b/clang-tools-extra/clang-tidy/ClangTidyCheck.h new file mode 100644 index 0000000..bfb57e3 --- /dev/null +++ b/clang-tools-extra/clang-tidy/ClangTidyCheck.h @@ -0,0 +1,197 @@ +//===--- ClangTidyCheck.h - clang-tidy --------------------------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CLANGTIDYCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CLANGTIDYCHECK_H + +#include "ClangTidyDiagnosticConsumer.h" +#include "ClangTidyOptions.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Basic/Diagnostic.h" +#include "clang/Basic/SourceManager.h" +#include "llvm/ADT/StringExtras.h" +#include +#include +#include + +namespace clang { + +class CompilerInstance; + +namespace tidy { + +/// \brief Base class for all clang-tidy checks. +/// +/// To implement a ``ClangTidyCheck``, write a subclass and override some of the +/// base class's methods. E.g. to implement a check that validates namespace +/// declarations, override ``registerMatchers``: +/// +/// ~~~{.cpp} +/// void registerMatchers(ast_matchers::MatchFinder *Finder) override { +/// Finder->addMatcher(namespaceDecl().bind("namespace"), this); +/// } +/// ~~~ +/// +/// and then override ``check(const MatchResult &Result)`` to do the actual +/// check for each match. +/// +/// A new ``ClangTidyCheck`` instance is created per translation unit. +/// +/// FIXME: Figure out whether carrying information from one TU to another is +/// useful/necessary. +class ClangTidyCheck : public ast_matchers::MatchFinder::MatchCallback { +public: + /// \brief Initializes the check with \p CheckName and \p Context. + /// + /// Derived classes must implement the constructor with this signature or + /// delegate it. If a check needs to read options, it can do this in the + /// constructor using the Options.get() methods below. + ClangTidyCheck(StringRef CheckName, ClangTidyContext *Context); + + /// DEPRECATED: Use the other overload. + virtual void registerPPCallbacks(CompilerInstance &Compiler) {} + + /// \brief Override this to register ``PPCallbacks`` in the preprocessor. + /// + /// This should be used for clang-tidy checks that analyze preprocessor- + /// dependent properties, e.g. include directives and macro definitions. + /// + /// There are two Preprocessors to choose from that differ in how they handle + /// modular #includes: + /// - PP is the real Preprocessor. It doesn't walk into modular #includes and + /// thus doesn't generate PPCallbacks for their contents. + /// - ModuleExpanderPP preprocesses the whole translation unit in the + /// non-modular mode, which allows it to generate PPCallbacks not only for + /// the main file and textual headers, but also for all transitively + /// included modular headers when the analysis runs with modules enabled. + /// When modules are not enabled ModuleExpanderPP just points to the real + /// preprocessor. + virtual void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP, + Preprocessor *ModuleExpanderPP) {} + + /// \brief Override this to register AST matchers with \p Finder. + /// + /// This should be used by clang-tidy checks that analyze code properties that + /// dependent on AST knowledge. + /// + /// You can register as many matchers as necessary with \p Finder. Usually, + /// "this" will be used as callback, but you can also specify other callback + /// classes. Thereby, different matchers can trigger different callbacks. + /// + /// If you need to merge information between the different matchers, you can + /// store these as members of the derived class. However, note that all + /// matches occur in the order of the AST traversal. + virtual void registerMatchers(ast_matchers::MatchFinder *Finder) {} + + /// \brief ``ClangTidyChecks`` that register ASTMatchers should do the actual + /// work in here. + virtual void check(const ast_matchers::MatchFinder::MatchResult &Result) {} + + /// \brief Add a diagnostic with the check's name. + DiagnosticBuilder diag(SourceLocation Loc, StringRef Description, + DiagnosticIDs::Level Level = DiagnosticIDs::Warning); + + /// \brief Should store all options supported by this check with their + /// current values or default values for options that haven't been overridden. + /// + /// The check should use ``Options.store()`` to store each option it supports + /// whether it has the default value or it has been overridden. + virtual void storeOptions(ClangTidyOptions::OptionMap &Options) {} + +private: + void run(const ast_matchers::MatchFinder::MatchResult &Result) override; + StringRef getID() const override { return CheckName; } + std::string CheckName; + ClangTidyContext *Context; + +protected: + /// \brief Provides access to the ``ClangTidyCheck`` options via check-local + /// names. + /// + /// Methods of this class prepend ``CheckName + "."`` to translate check-local + /// option names to global option names. + class OptionsView { + public: + /// \brief Initializes the instance using \p CheckName + "." as a prefix. + OptionsView(StringRef CheckName, + const ClangTidyOptions::OptionMap &CheckOptions); + + /// \brief Read a named option from the ``Context``. + /// + /// Reads the option with the check-local name \p LocalName from the + /// ``CheckOptions``. If the corresponding key is not present, returns + /// \p Default. + std::string get(StringRef LocalName, StringRef Default) const; + + /// \brief Read a named option from the ``Context``. + /// + /// Reads the option with the check-local name \p LocalName from local or + /// global ``CheckOptions``. Gets local option first. If local is not + /// present, falls back to get global option. If global option is not + /// present either, returns Default. + std::string getLocalOrGlobal(StringRef LocalName, StringRef Default) const; + + /// \brief Read a named option from the ``Context`` and parse it as an + /// integral type ``T``. + /// + /// Reads the option with the check-local name \p LocalName from the + /// ``CheckOptions``. If the corresponding key is not present, returns + /// \p Default. + template + typename std::enable_if::value, T>::type + get(StringRef LocalName, T Default) const { + std::string Value = get(LocalName, ""); + T Result = Default; + if (!Value.empty()) + StringRef(Value).getAsInteger(10, Result); + return Result; + } + + /// \brief Read a named option from the ``Context`` and parse it as an + /// integral type ``T``. + /// + /// Reads the option with the check-local name \p LocalName from local or + /// global ``CheckOptions``. Gets local option first. If local is not + /// present, falls back to get global option. If global option is not + /// present either, returns Default. + template + typename std::enable_if::value, T>::type + getLocalOrGlobal(StringRef LocalName, T Default) const { + std::string Value = getLocalOrGlobal(LocalName, ""); + T Result = Default; + if (!Value.empty()) + StringRef(Value).getAsInteger(10, Result); + return Result; + } + + /// \brief Stores an option with the check-local name \p LocalName with + /// string value \p Value to \p Options. + void store(ClangTidyOptions::OptionMap &Options, StringRef LocalName, + StringRef Value) const; + + /// \brief Stores an option with the check-local name \p LocalName with + /// ``int64_t`` value \p Value to \p Options. + void store(ClangTidyOptions::OptionMap &Options, StringRef LocalName, + int64_t Value) const; + + private: + std::string NamePrefix; + const ClangTidyOptions::OptionMap &CheckOptions; + }; + + OptionsView Options; + /// \brief Returns the main file name of the current translation unit. + StringRef getCurrentMainFile() const { return Context->getCurrentFile(); } + /// \brief Returns the language options from the context. + LangOptions getLangOpts() const { return Context->getLangOpts(); } +}; + +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CLANGTIDYCHECK_H diff --git a/clang-tools-extra/docs/clang-tidy/Contributing.rst b/clang-tools-extra/docs/clang-tidy/Contributing.rst index 719bed3..09ff1f6 100644 --- a/clang-tools-extra/docs/clang-tidy/Contributing.rst +++ b/clang-tools-extra/docs/clang-tidy/Contributing.rst @@ -80,7 +80,8 @@ The Directory Structure :: clang-tidy/ # Clang-tidy core. - |-- ClangTidy.h # Interfaces for users and checks. + |-- ClangTidy.h # Interfaces for users. + |-- ClangTidyCheck.h # Interfaces for checks. |-- ClangTidyModule.h # Interface for clang-tidy modules. |-- ClangTidyModuleRegistry.h # Interface for registering of modules. ... @@ -157,7 +158,7 @@ Let's see in more detail at the check class definition: ... - #include "../ClangTidy.h" + #include "../ClangTidyCheck.h" namespace clang { namespace tidy { diff --git a/llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/BUILD.gn b/llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/BUILD.gn index 2b0bb2b..415e0fc 100644 --- a/llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/BUILD.gn +++ b/llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/BUILD.gn @@ -28,6 +28,7 @@ static_library("clang-tidy") { sources = [ "ClangTidy.cpp", + "ClangTidyCheck.cpp", "ClangTidyDiagnosticConsumer.cpp", "ClangTidyModule.cpp", "ClangTidyOptions.cpp", -- 2.7.4