[clang-tidy] Fix ODR violation in unittests.

[NoQ]

Summary

I debugged the problem from D84453 and D82188. The linker is not at fault - it's an ODR violation: there are two definitions for clang::tidy::test::TestCheck::registerMatchers. One test provides a non-trivial overload for registerMatchers, the other test doesn't overload this function (it only tests whether the check gets enabled when options are tweaked, so it doesn't register any matchers). When linker merges the two classes, the class without the overload may randomly win the race, and in this case the class that expects an overload appears to behave as if the function isn't called at all (because the definition in the superclass does nothing).

Put classes into anonymous namespaces to avoid the ODR violation.

[vsavchenko]

LGTM! Thanks for all the investigative work!

[njames93]

LGTM, Thanks that bug was eating away at me for a good few days.

clang-tools-extra/unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp

 #include "ClangTidy.h"
 #include "ClangTidyTest.h"
 #include "gtest/gtest.h"
 
 namespace clang {
 namespace tidy {
 namespace test {
 
+namespace {
 class TestCheck : public ClangTidyCheck {
 public:
   TestCheck(StringRef Name, ClangTidyContext *Context)
       : ClangTidyCheck(Name, Context) {}
   void registerMatchers(ast_matchers::MatchFinder *Finder) override {
     Finder->addMatcher(ast_matchers::varDecl().bind("var"), this);
   }
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override {
     const auto *Var = Result.Nodes.getNodeAs<VarDecl>("var");
     // Add diagnostics in the wrong order.
     diag(Var->getLocation(), "variable");
     diag(Var->getTypeSpecStartLoc(), "type specifier");
   }
 };
+} // namespace
 
-// FIXME: This test seems to cause a strange linking interference
-// with the ValidConfiguration.ValidEnumOptions test on macOS.
-// If both tests are enabled, this test will fail as if
-// runCheckOnCode() is not invoked at all. Looks like a linker bug.
-// For now both tests are disabled on macOS. It is not sufficient
-// to only disable the other test because this test keeps failing
-// under Address Sanitizer, which may be an indication of more
-// such linking interference with other tests and this test
-// seems to be in the center of it.
-#ifndef __APPLE__
 TEST(ClangTidyDiagnosticConsumer, SortsErrors) {
   std::vector<ClangTidyError> Errors;
   runCheckOnCode<TestCheck>("int a;", &Errors);
   EXPECT_EQ(2ul, Errors.size());
   EXPECT_EQ("type specifier", Errors[0].Message.Message);
   EXPECT_EQ("variable", Errors[1].Message.Message);
 }
-#endif
 
 } // namespace test
 } // namespace tidy
 } // namespace clang

clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp

TEST(ParseConfiguration, MergeConfigurations) {
   ASSERT_TRUE(Options.ExtraArgsBefore.hasValue());
   EXPECT_EQ("arg-before1,arg-before2,arg-before3,arg-before4",
             llvm::join(Options.ExtraArgsBefore->begin(),
                        Options.ExtraArgsBefore->end(), ","));
   ASSERT_TRUE(Options.UseColor.hasValue());
   EXPECT_TRUE(*Options.UseColor);
 }
 
+namespace {
 class TestCheck : public ClangTidyCheck {
 public:
   TestCheck(ClangTidyContext *Context) : ClangTidyCheck("test", Context) {}
 
   template <typename... Args> auto getLocal(Args &&... Arguments) {
     return Options.get(std::forward<Args>(Arguments)...);
   }
 
   template <typename... Args> auto getGlobal(Args &&... Arguments) {
     return Options.getLocalOrGlobal(std::forward<Args>(Arguments)...);
   }
 
   template <typename IntType = int, typename... Args>
   auto getIntLocal(Args &&... Arguments) {
     return Options.get<IntType>(std::forward<Args>(Arguments)...);
   }
 
   template <typename IntType = int, typename... Args>
   auto getIntGlobal(Args &&... Arguments) {
     return Options.getLocalOrGlobal<IntType>(std::forward<Args>(Arguments)...);
   }
 };
+} // namespace
 
 #define CHECK_VAL(Value, Expected)                                             \
   do {                                                                         \
     auto Item = Value;                                                         \
     ASSERT_TRUE(!!Item);                                                       \
     EXPECT_EQ(*Item, Expected);                                                \
   } while (false)
 
CHECK_ERROR_INT(TestCheck.getIntLocal<bool>("BoolUnparseable"),
                   "'test.BoolUnparseable'; expected a bool");
   CHECK_ERROR_INT(TestCheck.getIntLocal<bool>("BoolCaseMismatch"),
                   "invalid configuration value 'True' for option "
                   "'test.BoolCaseMismatch'; expected a bool");
 
 #undef CHECK_ERROR_INT
 }
 
-// FIXME: Figure out why this test causes crashes on mac os.
-// See also comments around the ClangTidyDiagnosticConsumer.SortsErrors test.
-#ifndef __APPLE__
 TEST(ValidConfiguration, ValidEnumOptions) {
 
   ClangTidyOptions Options;
   auto &CheckOptions = Options.CheckOptions;
 
   CheckOptions["test.Valid"] = "Red";
   CheckOptions["test.Invalid"] = "Scarlet";
   CheckOptions["test.ValidWrongCase"] = "rED";
CHECK_ERROR_ENUM(TestCheck.getIntGlobal<Colours>("GlobalValidWrongCase"),
                    "invalid configuration value 'vIOLET' for option "
                    "'GlobalValidWrongCase'; did you mean 'Violet'?");
   CHECK_ERROR_ENUM(TestCheck.getIntGlobal<Colours>("GlobalNearMiss"),
                    "invalid configuration value 'Yelow' for option "
                    "'GlobalNearMiss'; did you mean 'Yellow'?");
 
 #undef CHECK_ERROR_ENUM
 }
-#endif
 
 #undef CHECK_VAL
 #undef CHECK_ERROR
 
 } // namespace test
 } // namespace tidy
 } // namespace clang