[clang-tools-extra] r279519 - [clang-tidy] Merge ExtraArgs(Before) instead of overriding them.

[Alexander Kornienko via cfe-commits]

Author: alexfh
Date: Tue Aug 23 09:48:29 2016
New Revision: 279519

URL: http://llvm.org/viewvc/llvm-project?rev=279519&view=rev
Log:
[clang-tidy] Merge ExtraArgs(Before) instead of overriding them.

Added proper tests.

Modified:
    clang-tools-extra/trunk/clang-tidy/ClangTidyOptions.cpp
    clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyOptionsTest.cpp

Modified: clang-tools-extra/trunk/clang-tidy/ClangTidyOptions.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/ClangTidyOptions.cpp?rev=279519&r1=279518&r2=279519&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/ClangTidyOptions.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/ClangTidyOptions.cpp Tue Aug 23 09:48:29 2016
@@ -117,34 +117,40 @@ ClangTidyOptions ClangTidyOptions::getDe
   return Options;
 }
 
+template <typename T>
+static void mergeVectors(Optional<T> &Dest, const Optional<T> &Src) {
+  if (Src) {
+    if (Dest)
+      Dest->insert(Dest->end(), Src->begin(), Src->end());
+    else
+      Dest = Src;
+  }
+}
+
+static void mergeCommaSeparatedLists(Optional<std::string> &Dest,
+                                     const Optional<std::string> &Src) {
+  if (Src)
+    Dest = (Dest && !Dest->empty() ? *Dest + "," : "") + *Src;
+}
+
+template <typename T>
+static void overrideValue(Optional<T> &Dest, const Optional<T> &Src) {
+  if (Src)
+    Dest = Src;
+}
+
 ClangTidyOptions
 ClangTidyOptions::mergeWith(const ClangTidyOptions &Other) const {
   ClangTidyOptions Result = *this;
 
-  // Merge comma-separated glob lists by appending the new value after a comma.
-  if (Other.Checks)
-    Result.Checks =
-        (Result.Checks && !Result.Checks->empty() ? *Result.Checks + "," : "") +
-        *Other.Checks;
-  if (Other.WarningsAsErrors)
-    Result.WarningsAsErrors =
-        (Result.WarningsAsErrors && !Result.WarningsAsErrors->empty()
-             ? *Result.WarningsAsErrors + ","
-             : "") +
-        *Other.WarningsAsErrors;
-
-  if (Other.HeaderFilterRegex)
-    Result.HeaderFilterRegex = Other.HeaderFilterRegex;
-  if (Other.SystemHeaders)
-    Result.SystemHeaders = Other.SystemHeaders;
-  if (Other.AnalyzeTemporaryDtors)
-    Result.AnalyzeTemporaryDtors = Other.AnalyzeTemporaryDtors;
-  if (Other.User)
-    Result.User = Other.User;
-  if (Other.ExtraArgs)
-    Result.ExtraArgs = Other.ExtraArgs;
-  if (Other.ExtraArgsBefore)
-    Result.ExtraArgsBefore = Other.ExtraArgsBefore;
+  mergeCommaSeparatedLists(Result.Checks, Other.Checks);
+  mergeCommaSeparatedLists(Result.WarningsAsErrors, Other.WarningsAsErrors);
+  overrideValue(Result.HeaderFilterRegex, Other.HeaderFilterRegex);
+  overrideValue(Result.SystemHeaders, Other.SystemHeaders);
+  overrideValue(Result.AnalyzeTemporaryDtors, Other.AnalyzeTemporaryDtors);
+  overrideValue(Result.User, Other.User);
+  mergeVectors(Result.ExtraArgs, Other.ExtraArgs);
+  mergeVectors(Result.ExtraArgsBefore, Other.ExtraArgsBefore);
 
   for (const auto &KeyValue : Other.CheckOptions)
     Result.CheckOptions[KeyValue.first] = KeyValue.second;

Modified: clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyOptionsTest.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyOptionsTest.cpp?rev=279519&r1=279518&r2=279519&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyOptionsTest.cpp (original)
+++ clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyOptionsTest.cpp Tue Aug 23 09:48:29 2016
@@ -1,5 +1,6 @@
 #include "ClangTidyOptions.h"
 #include "gtest/gtest.h"
+#include "llvm/ADT/StringExtras.h"
 
 namespace clang {
 namespace tidy {
@@ -66,6 +67,38 @@ TEST(ParseConfiguration, ValidConfigurat
   EXPECT_EQ("some.user", *Options->User);
 }
 
+TEST(ParseConfiguration, MergeConfigurations) {
+  llvm::ErrorOr<ClangTidyOptions> Options1 = parseConfiguration(R"(
+      Checks: "check1,check2"
+      HeaderFilterRegex: "filter1"
+      AnalyzeTemporaryDtors: true
+      User: user1
+      ExtraArgs: ['arg1', 'arg2']
+      ExtraArgsBefore: ['arg-before1', 'arg-before2']
+  )");
+  ASSERT_TRUE(!!Options1);
+  llvm::ErrorOr<ClangTidyOptions> Options2 = parseConfiguration(R"(
+      Checks: "check3,check4"
+      HeaderFilterRegex: "filter2"
+      AnalyzeTemporaryDtors: false
+      User: user2
+      ExtraArgs: ['arg3', 'arg4']
+      ExtraArgsBefore: ['arg-before3', 'arg-before4']
+  )");
+  ASSERT_TRUE(!!Options2);
+  ClangTidyOptions Options = Options1->mergeWith(*Options2);
+  EXPECT_EQ("check1,check2,check3,check4", *Options.Checks);
+  EXPECT_EQ("filter2", *Options.HeaderFilterRegex);
+  EXPECT_FALSE(*Options.AnalyzeTemporaryDtors);
+  EXPECT_EQ("user2", *Options.User);
+  ASSERT_TRUE(Options.ExtraArgs.hasValue());
+  EXPECT_EQ("arg1,arg2,arg3,arg4", llvm::join(Options.ExtraArgs->begin(),
+                                              Options.ExtraArgs->end(), ","));
+  ASSERT_TRUE(Options.ExtraArgsBefore.hasValue());
+  EXPECT_EQ("arg-before1,arg-before2,arg-before3,arg-before4",
+            llvm::join(Options.ExtraArgsBefore->begin(),
+                       Options.ExtraArgsBefore->end(), ","));
+}
 } // namespace test
 } // namespace tidy
 } // namespace clang