Hi klimek, bkramer,
Each check can implement readOptions and storeOptions methods to read
and store custom options. Each check's options are stored in a local namespace
to avoid name collisions and provide some sort of context to the user.
http://reviews.llvm.org/D5296
Files:
clang-tidy/ClangTidy.cpp
clang-tidy/ClangTidy.h
clang-tidy/ClangTidyOptions.cpp
clang-tidy/ClangTidyOptions.h
clang-tidy/llvm/NamespaceCommentCheck.cpp
clang-tidy/llvm/NamespaceCommentCheck.h
clang-tidy/tool/ClangTidyMain.cpp
unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp
unittests/clang-tidy/ClangTidyTest.h
Index: clang-tidy/ClangTidy.cpp
===================================================================
--- clang-tidy/ClangTidy.cpp
+++ clang-tidy/ClangTidy.cpp
@@ -251,9 +251,9 @@
std::move(Consumers), std::move(Finder), std::move(Checks));
}
-std::vector<std::string>
-ClangTidyASTConsumerFactory::getCheckNames(GlobList &Filter) {
+std::vector<std::string> ClangTidyASTConsumerFactory::getCheckNames() {
std::vector<std::string> CheckNames;
+ GlobList &Filter = Context.getChecksFilter();
for (const auto &CheckFactory : *CheckFactories) {
if (Filter.contains(CheckFactory.first))
CheckNames.push_back(CheckFactory.first);
@@ -266,6 +266,18 @@
return CheckNames;
}
+ClangTidyOptions::OptionMap ClangTidyASTConsumerFactory::getCheckOptions() {
+ ClangTidyOptions::OptionMap Options;
+ std::vector<std::unique_ptr<ClangTidyCheck>> Checks;
+ GlobList &Filter = Context.getChecksFilter();
+ CheckFactories->createChecks(Filter, Checks);
+ for (const auto &Check : Checks) {
+ Check->setContext(&Context);
+ Check->storeOptions(Options);
+ }
+ return Options;
+}
+
ClangTidyASTConsumerFactory::CheckersList
ClangTidyASTConsumerFactory::getCheckersControlList(GlobList &Filter) {
CheckersList List;
@@ -312,12 +324,25 @@
CheckName = Name.str();
}
+void ClangTidyCheck::setContext(ClangTidyContext *Ctx) {
+ assert(Context == nullptr);
+ Context = Ctx;
+ readOptions();
+}
+
std::vector<std::string> getCheckNames(const ClangTidyOptions &Options) {
clang::tidy::ClangTidyContext Context(
llvm::make_unique<DefaultOptionsProvider>(ClangTidyGlobalOptions(),
Options));
ClangTidyASTConsumerFactory Factory(Context);
- return Factory.getCheckNames(Context.getChecksFilter());
+ return Factory.getCheckNames();
+}
+ClangTidyOptions::OptionMap getCheckOptions(const ClangTidyOptions &Options) {
+ clang::tidy::ClangTidyContext Context(
+ llvm::make_unique<DefaultOptionsProvider>(ClangTidyGlobalOptions(),
+ Options));
+ ClangTidyASTConsumerFactory Factory(Context);
+ return Factory.getCheckOptions();
}
ClangTidyStats
Index: clang-tidy/ClangTidy.h
===================================================================
--- clang-tidy/ClangTidy.h
+++ clang-tidy/ClangTidy.h
@@ -16,6 +16,8 @@
#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 <memory>
#include <vector>
@@ -49,6 +51,7 @@
/// useful/necessary.
class ClangTidyCheck : public ast_matchers::MatchFinder::MatchCallback {
public:
+ ClangTidyCheck() : Context(nullptr) {}
virtual ~ClangTidyCheck() {}
/// \brief Overwrite this to register \c PPCallbacks with \c Compiler.
@@ -76,17 +79,91 @@
virtual void check(const ast_matchers::MatchFinder::MatchResult &Result) {}
/// \brief The infrastructure sets the context to \p Ctx with this function.
- void setContext(ClangTidyContext *Ctx) { Context = Ctx; }
+ ///
+ /// This function can only be called once.
+ void setContext(ClangTidyContext *Ctx);
/// \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 \c storeOption() to store each option it supports
+ /// whether it has the default value or has been overridden.
+ virtual void storeOptions(ClangTidyOptions::OptionMap &Options) {}
+
/// \brief Sets the check name. Intended to be used by the clang-tidy
/// framework. Can be called only once.
void setName(StringRef Name);
+protected:
+ /// \brief Implement this method to read check options.
+ ///
+ /// Options can be accessed via getOption() method below or directly using
+ /// Context->getOptions().CheckOptions.
+ virtual void readOptions() {}
+
+ /// \brief Read a named option from the \c Context.
+ ///
+ /// Reads the option named <code>CheckName + "." +</code> \p LocalName
+ /// from the current options in the \c Context to \p Result if the
+ /// corresponding key is present. Otherwise leaves the \p Result unchanged.
+ ///
+ /// \returns \c true, if the value has been found and read to \p Result.
+ bool getOption(StringRef LocalName, std::string &Result) const {
+ assert(Context != nullptr);
+ const auto &Options = Context->getOptions().CheckOptions;
+ const auto &Iter = Options.find(getQualifiedOptionName(LocalName));
+ if (Iter != Options.end()) {
+ Result = Iter->second;
+ return true;
+ }
+ return false;
+ }
+
+ /// \brief Read a named option from the \c Context.
+ ///
+ /// Reads the option named <code>CheckName + "." +</code> \p LocalName
+ /// from the current options in the \c Context to \p Result if the
+ /// corresponding key is present. Otherwise leaves the \p Result unchanged.
+ ///
+ /// \returns \c true, if the value has been found, successfully converted
+ /// to the target type \c T and stored to \p Result.
+ template <typename T> bool getOption(StringRef LocalName, T &Result) const {
+ std::string Value;
+ if (!getOption(LocalName, Value))
+ return false;
+ return StringRef(Value).getAsInteger(10, Result);
+ }
+
+ /// \brief Stores an option named \p LocalName with string value \p Value to
+ /// \p Options.
+ ///
+ /// The function calls \c getQualifiedOptionName(LocalName) to get the global
+ /// option name by prepending the check name.
+ void storeOption(ClangTidyOptions::OptionMap &Options, StringRef LocalName,
+ StringRef Value) const {
+ Options[getQualifiedOptionName(LocalName)] = Value;
+ }
+
+ /// \brief Stores an option named \p LocalName with \c int64_t value \p Value
+ /// to \p Options.
+ ///
+ /// The function calls \c getQualifiedOptionName(LocalName) to get the global
+ /// option name by prepending the check name.
+ void storeOption(ClangTidyOptions::OptionMap &Options, StringRef LocalName,
+ int64_t Value) const {
+ storeOption(Options, LocalName, llvm::itostr(Value));
+ }
+
private:
+ std::string getQualifiedOptionName(StringRef OptionName) const {
+ assert(!CheckName.empty());
+ return CheckName + "." + OptionName.str();
+ }
+
void run(const ast_matchers::MatchFinder::MatchResult &Result) override;
ClangTidyContext *Context;
std::string CheckName;
@@ -103,10 +180,13 @@
CreateASTConsumer(clang::CompilerInstance &Compiler, StringRef File);
/// \brief Get the list of enabled checks.
- std::vector<std::string> getCheckNames(GlobList &Filter);
+ std::vector<std::string> getCheckNames();
+
+ /// \brief Get the union of options from all checks.
+ ClangTidyOptions::OptionMap getCheckOptions();
private:
- typedef std::vector<std::pair<std::string, bool> > CheckersList;
+ typedef std::vector<std::pair<std::string, bool>> CheckersList;
CheckersList getCheckersControlList(GlobList &Filter);
ClangTidyContext &Context;
@@ -117,6 +197,14 @@
/// filters are applied.
std::vector<std::string> getCheckNames(const ClangTidyOptions &Options);
+/// \brief Returns the effective check-specific options.
+///
+/// The method configures ClangTidy with the specified \p Options and collects
+/// effective options from all created checks. The returned set of options
+/// includes default check-specific options for all keys not overridden by \p
+/// Options.
+ClangTidyOptions::OptionMap getCheckOptions(const ClangTidyOptions &Options);
+
/// \brief Run a set of clang-tidy checks on a set of files.
ClangTidyStats
runClangTidy(std::unique_ptr<ClangTidyOptionsProvider> OptionsProvider,
Index: clang-tidy/ClangTidyOptions.cpp
===================================================================
--- clang-tidy/ClangTidyOptions.cpp
+++ clang-tidy/ClangTidyOptions.cpp
@@ -25,6 +25,7 @@
LLVM_YAML_IS_FLOW_SEQUENCE_VECTOR(FileFilter)
LLVM_YAML_IS_FLOW_SEQUENCE_VECTOR(FileFilter::LineRange)
+LLVM_YAML_IS_SEQUENCE_VECTOR(ClangTidyOptions::StringPair);
namespace llvm {
namespace yaml {
@@ -57,11 +58,34 @@
}
};
+template <> struct MappingTraits<ClangTidyOptions::StringPair> {
+ static void mapping(IO &IO, ClangTidyOptions::StringPair &KeyValue) {
+ IO.mapRequired("key", KeyValue.first);
+ IO.mapRequired("value", KeyValue.second);
+ }
+};
+
+struct NOptionMap {
+ NOptionMap(IO &) {}
+ NOptionMap(IO &, const ClangTidyOptions::OptionMap &OptionMap)
+ : Options(OptionMap.begin(), OptionMap.end()) {}
+ ClangTidyOptions::OptionMap denormalize(IO &) {
+ ClangTidyOptions::OptionMap Map;
+ for (const auto &KeyValue : Options)
+ Map[KeyValue.first] = KeyValue.second;
+ return Map;
+ }
+ std::vector<ClangTidyOptions::StringPair> Options;
+};
+
template <> struct MappingTraits<ClangTidyOptions> {
static void mapping(IO &IO, ClangTidyOptions &Options) {
+ MappingNormalization<NOptionMap, ClangTidyOptions::OptionMap> NOpts(
+ IO, Options.CheckOptions);
IO.mapOptional("Checks", Options.Checks);
IO.mapOptional("HeaderFilterRegex", Options.HeaderFilterRegex);
IO.mapOptional("AnalyzeTemporaryDtors", Options.AnalyzeTemporaryDtors);
+ IO.mapOptional("CheckOptions", NOpts->Options);
}
};
@@ -85,6 +109,10 @@
Result.HeaderFilterRegex = Other.HeaderFilterRegex;
if (Other.AnalyzeTemporaryDtors)
Result.AnalyzeTemporaryDtors = Other.AnalyzeTemporaryDtors;
+
+ for (const auto &KeyValue : Other.CheckOptions)
+ Result.CheckOptions[KeyValue.first] = KeyValue.second;
+
return Result;
}
@@ -169,6 +197,10 @@
llvm::MemoryBuffer::getFile(ConfigFile.c_str());
if (std::error_code EC = Text.getError())
return EC;
+ // Skip empty files, e.g. files opened for writing via shell output
+ // redirection.
+ if ((*Text)->getBuffer().empty())
+ return make_error_code(llvm::errc::no_such_file_or_directory);
if (std::error_code EC = parseConfiguration((*Text)->getBuffer(), Options))
return EC;
return Options.mergeWith(OverrideOptions);
Index: clang-tidy/ClangTidyOptions.h
===================================================================
--- clang-tidy/ClangTidyOptions.h
+++ clang-tidy/ClangTidyOptions.h
@@ -14,6 +14,7 @@
#include "llvm/ADT/StringMap.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/ErrorOr.h"
+#include <map>
#include <string>
#include <system_error>
#include <utility>
@@ -70,6 +71,12 @@
/// \brief Turns on temporary destructor-based analysis.
llvm::Optional<bool> AnalyzeTemporaryDtors;
+
+ typedef std::pair<std::string, std::string> StringPair;
+ typedef std::map<std::string, std::string> OptionMap;
+
+ /// \brief Key-value mapping used to store check-specific options.
+ OptionMap CheckOptions;
};
/// \brief Abstract interface for retrieving various ClangTidy options.
Index: clang-tidy/llvm/NamespaceCommentCheck.cpp
===================================================================
--- clang-tidy/llvm/NamespaceCommentCheck.cpp
+++ clang-tidy/llvm/NamespaceCommentCheck.cpp
@@ -11,9 +11,7 @@
#include "clang/AST/ASTContext.h"
#include "clang/ASTMatchers/ASTMatchers.h"
#include "clang/Lex/Lexer.h"
-
-
-#include "llvm/Support/raw_ostream.h"
+#include "llvm/ADT/StringExtras.h"
using namespace clang::ast_matchers;
@@ -24,7 +22,17 @@
: NamespaceCommentPattern("^/[/*] *(end (of )?)? *(anonymous|unnamed)? *"
"namespace( +([a-zA-Z0-9_]+))? *(\\*/)?$",
llvm::Regex::IgnoreCase),
- ShortNamespaceLines(1) {}
+ ShortNamespaceLines(1u), SpacesBeforeComments(1u) {}
+
+void NamespaceCommentCheck::storeOptions(ClangTidyOptions::OptionMap &Options) {
+ storeOption(Options, "ShortNamespaceLines", ShortNamespaceLines);
+ storeOption(Options, "SpacesBeforeComments", SpacesBeforeComments);
+}
+
+void NamespaceCommentCheck::readOptions() {
+ getOption("ShortNamespaceLines", ShortNamespaceLines);
+ getOption("SpacesBeforeComments", SpacesBeforeComments);
+}
void NamespaceCommentCheck::registerMatchers(MatchFinder *Finder) {
Finder->addMatcher(namespaceDecl().bind("namespace"), this);
@@ -36,10 +44,12 @@
Sources.getFileID(Loc1) == Sources.getFileID(Loc2);
}
-std::string getNamespaceComment(const NamespaceDecl *ND, bool InsertLineBreak) {
+std::string getNamespaceComment(const NamespaceDecl *ND, bool InsertLineBreak,
+ unsigned SpacesBeforeComments) {
std::string Fix = "// namespace";
if (!ND->isAnonymousNamespace())
- Fix.append(" ").append(ND->getNameAsString());
+ Fix.append(std::string(SpacesBeforeComments, ' '))
+ .append(ND->getNameAsString());
if (InsertLineBreak)
Fix.append("\n");
return Fix;
@@ -97,7 +107,8 @@
diag(Loc, "namespace closing comment refers to a wrong namespace '%0'")
<< NamespaceNameInComment
<< FixItHint::CreateReplacement(
- OldCommentRange, getNamespaceComment(ND, NeedLineBreak));
+ OldCommentRange,
+ getNamespaceComment(ND, NeedLineBreak, SpacesBeforeComments));
return;
}
@@ -110,7 +121,8 @@
diag(ND->getLocation(), "namespace not terminated with a closing comment")
<< FixItHint::CreateInsertion(
- AfterRBrace, " " + getNamespaceComment(ND, NeedLineBreak));
+ AfterRBrace,
+ " " + getNamespaceComment(ND, NeedLineBreak, SpacesBeforeComments));
}
} // namespace tidy
Index: clang-tidy/llvm/NamespaceCommentCheck.h
===================================================================
--- clang-tidy/llvm/NamespaceCommentCheck.h
+++ clang-tidy/llvm/NamespaceCommentCheck.h
@@ -26,8 +26,12 @@
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
private:
+ void storeOptions(ClangTidyOptions::OptionMap &Options) override;
+ void readOptions() override;
+
llvm::Regex NamespaceCommentPattern;
- const unsigned ShortNamespaceLines;
+ unsigned ShortNamespaceLines;
+ unsigned SpacesBeforeComments;
};
} // namespace tidy
Index: clang-tidy/tool/ClangTidyMain.cpp
===================================================================
--- clang-tidy/tool/ClangTidyMain.cpp
+++ clang-tidy/tool/ClangTidyMain.cpp
@@ -173,6 +173,7 @@
}
if (DumpConfig) {
+ EffectiveOptions.CheckOptions = getCheckOptions(EffectiveOptions);
llvm::outs() << configurationAsText(ClangTidyOptions::getDefaults()
.mergeWith(EffectiveOptions))
<< "\n";
Index: unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp
===================================================================
--- unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp
+++ unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp
@@ -23,9 +23,8 @@
std::vector<ClangTidyError> Errors;
runCheckOnCode<TestCheck>("int a;", &Errors);
EXPECT_EQ(2ul, Errors.size());
- // FIXME: Remove " []" once the check name is removed from the message text.
- EXPECT_EQ("type specifier []", Errors[0].Message.Message);
- EXPECT_EQ("variable []", Errors[1].Message.Message);
+ EXPECT_EQ("type specifier", Errors[0].Message.Message);
+ EXPECT_EQ("variable", Errors[1].Message.Message);
}
TEST(GlobList, Empty) {
Index: unittests/clang-tidy/ClangTidyTest.h
===================================================================
--- unittests/clang-tidy/ClangTidyTest.h
+++ unittests/clang-tidy/ClangTidyTest.h
@@ -50,6 +50,7 @@
ClangTidyContext Context(llvm::make_unique<DefaultOptionsProvider>(
ClangTidyGlobalOptions(), Options));
ClangTidyDiagnosticConsumer DiagConsumer(Context);
+ Check.setName("test-check");
Check.setContext(&Context);
std::vector<std::string> ArgCXX11(1, "-std=c++11");
ArgCXX11.insert(ArgCXX11.end(), ExtraArgs.begin(), ExtraArgs.end());
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
