Provide user name in ClangTidyOptions.

Summary:
This adds the ClangTidyOptions::User field and fills it from the USER
or the USERNAME environment variable, if possible. The FileOptionsProvider now
takes "default" options instead of "fallback" options, as it now uses these when
an option is not set in the configuration file (one exception is the checks
list).

Reviewers: bkramer, klimek

Reviewed By: klimek

Subscribers: cfe-commits

Differential Revision: http://reviews.llvm.org/D5440

llvm-svn: 218402

6 files changed, 59 insertions, 42 deletions

diff --git a/clang-tools-extra/clang-tidy/ClangTidyOptions.cpp b/clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
index 148593dabd5..a4bfe0488c1 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
+++ b/clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
@@ -85,6 +85,7 @@ template <> struct MappingTraits<ClangTidyOptions> {
     IO.mapOptional("Checks", Options.Checks);
     IO.mapOptional("HeaderFilterRegex", Options.HeaderFilterRegex);
     IO.mapOptional("AnalyzeTemporaryDtors", Options.AnalyzeTemporaryDtors);
+    IO.mapOptional("User", Options.User);
     IO.mapOptional("CheckOptions", NOpts->Options);
   }
 };
@@ -109,6 +110,8 @@ ClangTidyOptions::mergeWith(const ClangTidyOptions &Other) const {
     Result.HeaderFilterRegex = Other.HeaderFilterRegex;
   if (Other.AnalyzeTemporaryDtors)
     Result.AnalyzeTemporaryDtors = Other.AnalyzeTemporaryDtors;
+  if (Other.User)
+    Result.User = Other.User;
 
   for (const auto &KeyValue : Other.CheckOptions)
     Result.CheckOptions[KeyValue.first] = KeyValue.second;
@@ -118,11 +121,11 @@ ClangTidyOptions::mergeWith(const ClangTidyOptions &Other) const {
 
 FileOptionsProvider::FileOptionsProvider(
     const ClangTidyGlobalOptions &GlobalOptions,
-    const ClangTidyOptions &FallbackOptions,
+    const ClangTidyOptions &DefaultOptions,
     const ClangTidyOptions &OverrideOptions)
-    : DefaultOptionsProvider(GlobalOptions, FallbackOptions),
+    : DefaultOptionsProvider(GlobalOptions, DefaultOptions),
       OverrideOptions(OverrideOptions) {
-  CachedOptions[""] = FallbackOptions.mergeWith(OverrideOptions);
+  CachedOptions[""] = DefaultOptions.mergeWith(OverrideOptions);
 }
 
 static const char ConfigFileName[] = ".clang-tidy";
@@ -177,7 +180,6 @@ llvm::ErrorOr<ClangTidyOptions>
 FileOptionsProvider::TryReadConfigFile(StringRef Directory) {
   assert(!Directory.empty());
 
-  ClangTidyOptions Options = DefaultOptionsProvider::getOptions(Directory);
   if (!llvm::sys::fs::is_directory(Directory))
     return make_error_code(llvm::errc::not_a_directory);
 
@@ -201,9 +203,15 @@ FileOptionsProvider::TryReadConfigFile(StringRef Directory) {
   // 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);
+  llvm::ErrorOr<ClangTidyOptions> ParsedOptions =
+      parseConfiguration((*Text)->getBuffer());
+  if (ParsedOptions) {
+    ClangTidyOptions Defaults = DefaultOptionsProvider::getOptions(Directory);
+    // Only use checks from the config file.
+    Defaults.Checks = None;
+    return Defaults.mergeWith(*ParsedOptions).mergeWith(OverrideOptions);
+  }
+  return ParsedOptions.getError();
 }
 
 /// \brief Parses -line-filter option and stores it to the \c Options.
@@ -214,11 +222,13 @@ std::error_code parseLineFilter(StringRef LineFilter,
   return Input.error();
 }
 
-std::error_code parseConfiguration(StringRef Config,
-                                   clang::tidy::ClangTidyOptions &Options) {
+llvm::ErrorOr<ClangTidyOptions> parseConfiguration(StringRef Config) {
   llvm::yaml::Input Input(Config);
+  ClangTidyOptions Options;
   Input >> Options;
-  return Input.error();
+  if (Input.error())
+    return Input.error();
+  return Options;
 }
 
 std::string configurationAsText(const ClangTidyOptions &Options) {
diff --git a/clang-tools-extra/clang-tidy/ClangTidyOptions.h b/clang-tools-extra/clang-tidy/ClangTidyOptions.h
index 3c2251c4bd3..82a740cec22 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyOptions.h
+++ b/clang-tools-extra/clang-tidy/ClangTidyOptions.h
@@ -55,6 +55,7 @@ struct ClangTidyOptions {
     Options.Checks = "";
     Options.HeaderFilterRegex = "";
     Options.AnalyzeTemporaryDtors = false;
+    Options.User = llvm::None;
     return Options;
   }
 
@@ -72,6 +73,12 @@ struct ClangTidyOptions {
   /// \brief Turns on temporary destructor-based analysis.
   llvm::Optional<bool> AnalyzeTemporaryDtors;
 
+  /// \brief Specifies the name or e-mail of the user running clang-tidy.
+  ///
+  /// This option is used, for example, to place the correct user name in TODO()
+  /// comments in the relevant check.
+  llvm::Optional<std::string> User;
+
   typedef std::pair<std::string, std::string> StringPair;
   typedef std::map<std::string, std::string> OptionMap;
 
@@ -121,19 +128,19 @@ public:
   /// \param GlobalOptions are just stored and returned to the caller of
   /// \c getGlobalOptions.
   ///
-  /// \param FallbackOptions are used in case there's no corresponding
+  /// \param DefaultOptions are used for all settings not specified in a
   /// .clang-tidy file.
   ///
   /// If any of the \param OverrideOptions fields are set, they will override
   /// whatever options are read from the configuration file.
   FileOptionsProvider(const ClangTidyGlobalOptions &GlobalOptions,
-                      const ClangTidyOptions &FallbackOptions,
+                      const ClangTidyOptions &DefaultOptions,
                       const ClangTidyOptions &OverrideOptions);
   const ClangTidyOptions &getOptions(llvm::StringRef FileName) override;
 
 private:
   /// \brief Try to read configuration file from \p Directory. If \p Directory
-  /// is empty, use the fallback value.
+  /// is empty, use the default value.
   llvm::ErrorOr<ClangTidyOptions> TryReadConfigFile(llvm::StringRef Directory);
 
   llvm::StringMap<ClangTidyOptions> CachedOptions;
@@ -144,9 +151,9 @@ private:
 std::error_code parseLineFilter(llvm::StringRef LineFilter,
                                 ClangTidyGlobalOptions &Options);
 
-/// \brief Parses configuration from JSON and stores it to the \p Options.
-std::error_code parseConfiguration(llvm::StringRef Config,
-                                   ClangTidyOptions &Options);
+/// \brief Parses configuration from JSON and returns \c ClangTidyOptions or an
+/// error.
+llvm::ErrorOr<ClangTidyOptions> parseConfiguration(llvm::StringRef Config);
 
 /// \brief Serializes configuration to a YAML-encoded string.
 std::string configurationAsText(const ClangTidyOptions &Options);
diff --git a/clang-tools-extra/clang-tidy/google/TodoCommentCheck.cpp b/clang-tools-extra/clang-tidy/google/TodoCommentCheck.cpp
index 5937f8f1423..26ad6b6c9ad 100644
--- a/clang-tools-extra/clang-tidy/google/TodoCommentCheck.cpp
+++ b/clang-tools-extra/clang-tidy/google/TodoCommentCheck.cpp
@@ -17,8 +17,9 @@ namespace readability {
 
 class TodoCommentCheck::TodoCommentHandler : public CommentHandler {
 public:
-  explicit TodoCommentHandler(TodoCommentCheck &Check)
-      : Check(Check), TodoMatch("^// *TODO(\\(.*\\))?:?( )?(.*)$") {}
+  TodoCommentHandler(TodoCommentCheck &Check, llvm::Optional<std::string> User)
+      : Check(Check), User(User ? *User : "unknown"),
+        TodoMatch("^// *TODO(\\(.*\\))?:?( )?(.*)$") {}
 
   bool HandleComment(Preprocessor &PP, SourceRange Range) override {
     StringRef Text =
@@ -35,12 +36,6 @@ public:
     if (!Username.empty())
       return false;
 
-    // If the username is missing put in the current user's name. Not ideal but
-    // works for running tidy locally.
-    // FIXME: Can we get this from a more reliable source?
-    const char *User = std::getenv("USER");
-    if (!User)
-      User = "unknown";
     std::string NewText = ("// TODO(" + Twine(User) + "): " + Comment).str();
 
     Check.diag(Range.getBegin(), "missing username/bug in TODO")
@@ -51,14 +46,14 @@ public:
 
 private:
   TodoCommentCheck &Check;
+  std::string User;
   llvm::Regex TodoMatch;
 };
 
 TodoCommentCheck::TodoCommentCheck(StringRef Name, ClangTidyContext *Context)
     : ClangTidyCheck(Name, Context),
-      Handler(llvm::make_unique<TodoCommentHandler>(*this)) {}
-
-TodoCommentCheck::~TodoCommentCheck() {}
+      Handler(llvm::make_unique<TodoCommentHandler>(
+          *this, Context->getOptions().User)) {}
 
 void TodoCommentCheck::registerPPCallbacks(CompilerInstance &Compiler) {
   Compiler.getPreprocessor().addCommentHandler(Handler.get());
diff --git a/clang-tools-extra/clang-tidy/google/TodoCommentCheck.h b/clang-tools-extra/clang-tidy/google/TodoCommentCheck.h
index 10e84a7be2e..b3b49fd9d67 100644
--- a/clang-tools-extra/clang-tidy/google/TodoCommentCheck.h
+++ b/clang-tools-extra/clang-tidy/google/TodoCommentCheck.h
@@ -22,7 +22,6 @@ namespace readability {
 class TodoCommentCheck : public ClangTidyCheck {
 public:
   TodoCommentCheck(StringRef Name, ClangTidyContext *Context);
-  ~TodoCommentCheck();
   void registerPPCallbacks(CompilerInstance &Compiler) override;
 
 private:
diff --git a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
index f11cbeec628..a9b29f682ca 100644
--- a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
+++ b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
@@ -17,6 +17,7 @@
 
 #include "../ClangTidy.h"
 #include "clang/Tooling/CommonOptionsParser.h"
+#include "llvm/Support/Process.h"
 
 using namespace clang::ast_matchers;
 using namespace clang::driver;
@@ -143,10 +144,14 @@ int clangTidyMain(int argc, const char **argv) {
     return 1;
   }
 
-  ClangTidyOptions FallbackOptions;
-  FallbackOptions.Checks = DefaultChecks;
-  FallbackOptions.HeaderFilterRegex = HeaderFilter;
-  FallbackOptions.AnalyzeTemporaryDtors = AnalyzeTemporaryDtors;
+  ClangTidyOptions DefaultOptions;
+  DefaultOptions.Checks = DefaultChecks;
+  DefaultOptions.HeaderFilterRegex = HeaderFilter;
+  DefaultOptions.AnalyzeTemporaryDtors = AnalyzeTemporaryDtors;
+  DefaultOptions.User = llvm::sys::Process::GetEnv("USER");
+  // USERNAME is used on Windows.
+  if (!DefaultOptions.User)
+    DefaultOptions.User = llvm::sys::Process::GetEnv("USERNAME");
 
   ClangTidyOptions OverrideOptions;
   if (Checks.getNumOccurrences() > 0)
@@ -157,7 +162,7 @@ int clangTidyMain(int argc, const char **argv) {
     OverrideOptions.AnalyzeTemporaryDtors = AnalyzeTemporaryDtors;
 
   auto OptionsProvider = llvm::make_unique<FileOptionsProvider>(
-      GlobalOptions, FallbackOptions, OverrideOptions);
+      GlobalOptions, DefaultOptions, OverrideOptions);
 
   std::string FileName = OptionsParser.getSourcePathList().front();
   ClangTidyOptions EffectiveOptions = OptionsProvider->getOptions(FileName);
diff --git a/clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp b/clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
index 8b0fbc6ff75..dd2304562be 100644
--- a/clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
+++ b/clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
@@ -55,15 +55,16 @@ TEST(ParseLineFilter, ValidFilter) {
 }
 
 TEST(ParseConfiguration, ValidConfiguration) {
-  ClangTidyOptions Options;
-  std::error_code Error = parseConfiguration("Checks: \"-*,misc-*\"\n"
-                                             "HeaderFilterRegex: \".*\"\n"
-                                             "AnalyzeTemporaryDtors: true\n",
-                                             Options);
-  EXPECT_FALSE(Error);
-  EXPECT_EQ("-*,misc-*", *Options.Checks);
-  EXPECT_EQ(".*", *Options.HeaderFilterRegex);
-  EXPECT_TRUE(*Options.AnalyzeTemporaryDtors);
+  llvm::ErrorOr<ClangTidyOptions> Options =
+      parseConfiguration("Checks: \"-*,misc-*\"\n"
+                         "HeaderFilterRegex: \".*\"\n"
+                         "AnalyzeTemporaryDtors: true\n"
+                         "User: some.user");
+  EXPECT_TRUE(!!Options);
+  EXPECT_EQ("-*,misc-*", *Options->Checks);
+  EXPECT_EQ(".*", *Options->HeaderFilterRegex);
+  EXPECT_TRUE(*Options->AnalyzeTemporaryDtors);
+  EXPECT_EQ("some.user", *Options->User);
 }
 
 } // namespace test