[clang-tidy][objc] Finds and fixes improper usages of XCTAssertEquals and XCTAssertNotEquals.

Using XCTAssertEqual on NSString* objects is almost always  wrong.

Unfortunately, we have seen a lot of tests doing this and reyling on pointer equality for strings with the same values (which happens to work sometimes - depending on the linker, but this assumption is not guaranteed by the language)

These fixes would make tests less brittle.

Differential Revision: https://reviews.llvm.org/D114975

4 files changed, 108 insertions, 0 deletions

diff --git a/clang-tools-extra/clang-tidy/objc/AssertEquals.cpp b/clang-tools-extra/clang-tidy/objc/AssertEquals.cpp
new file mode 100644
index 0000000..198e7f0
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/objc/AssertEquals.cpp
@@ -0,0 +1,65 @@
+//===--- AssertEquals.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 "AssertEquals.h"
+
+#include <map>
+#include <string>
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace objc {
+
+// Mapping from `XCTAssert*Equal` to `XCTAssert*EqualObjects` name.
+static const std::map<std::string, std::string> &NameMap() {
+  static std::map<std::string, std::string> map{
+      {"XCTAssertEqual", "XCTAssertEqualObjects"},
+      {"XCTAssertNotEqual", "XCTAssertNotEqualObjects"},
+
+  };
+  return map;
+}
+
+void AssertEquals::registerMatchers(MatchFinder *finder) {
+  for (const auto &pair : NameMap()) {
+    finder->addMatcher(
+        binaryOperator(anyOf(hasOperatorName("!="), hasOperatorName("==")),
+                       isExpandedFromMacro(pair.first),
+                       anyOf(hasLHS(hasType(qualType(
+                                 hasCanonicalType(asString("NSString *"))))),
+                             hasRHS(hasType(qualType(
+                                 hasCanonicalType(asString("NSString *"))))))
+
+                           )
+            .bind(pair.first),
+        this);
+  }
+}
+
+void AssertEquals::check(const ast_matchers::MatchFinder::MatchResult &result) {
+  for (const auto &pair : NameMap()) {
+    if (const auto *root = result.Nodes.getNodeAs<BinaryOperator>(pair.first)) {
+      SourceManager *sm = result.SourceManager;
+      // The macros are nested two levels, so going up twice.
+      auto macro_callsite = sm->getImmediateMacroCallerLoc(
+          sm->getImmediateMacroCallerLoc(root->getBeginLoc()));
+      diag(macro_callsite, "use " + pair.second + " for comparing objects")
+          << FixItHint::CreateReplacement(
+                 clang::CharSourceRange::getCharRange(
+                     macro_callsite,
+                     macro_callsite.getLocWithOffset(pair.first.length())),
+                 pair.second);
+    }
+  }
+}
+
+} // namespace objc
+} // namespace tidy
+} // namespace clang
diff --git a/clang-tools-extra/clang-tidy/objc/AssertEquals.h b/clang-tools-extra/clang-tidy/objc/AssertEquals.h
new file mode 100644
index 0000000..94389fe
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/objc/AssertEquals.h
@@ -0,0 +1,39 @@
+//===--- AssertEquals.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 THIRD_PARTY_LLVM_LLVM_PROJECT_CLANG_TOOLS_EXTRA_CLANG_TIDY_OBJC_OBJCASSERTEQUALS_H_
+#define THIRD_PARTY_LLVM_LLVM_PROJECT_CLANG_TOOLS_EXTRA_CLANG_TIDY_OBJC_OBJCASSERTEQUALS_H_
+
+#include "../ClangTidyCheck.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+namespace clang {
+namespace tidy {
+namespace objc {
+
+/// Warn if XCTAssertEqual() or XCTAssertNotEqual() is used with at least one
+/// operands of type NSString*.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/objc-assert-equals.html
+class AssertEquals final : public ClangTidyCheck {
+public:
+  AssertEquals(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+    return LangOpts.ObjC;
+  }
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace objc
+} // namespace tidy
+} // namespace clang
+
+#endif // THIRD_PARTY_LLVM_LLVM_PROJECT_CLANG_TOOLS_EXTRA_CLANG_TIDY_OBJC_OBJCASSERTEQUALS_H_
diff --git a/clang-tools-extra/clang-tidy/objc/CMakeLists.txt b/clang-tools-extra/clang-tidy/objc/CMakeLists.txt
index 9a553f5a..1129c6a 100644
--- a/clang-tools-extra/clang-tidy/objc/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/objc/CMakeLists.txt
@@ -4,6 +4,7 @@ set(LLVM_LINK_COMPONENTS
   )
 
 add_clang_library(clangTidyObjCModule
+  AssertEquals.cpp
   AvoidNSErrorInitCheck.cpp
   DeallocInCategoryCheck.cpp
   ForbiddenSubclassingCheck.cpp
diff --git a/clang-tools-extra/clang-tidy/objc/ObjCTidyModule.cpp b/clang-tools-extra/clang-tidy/objc/ObjCTidyModule.cpp
index 56ab0ab..3a5bf7d 100644
--- a/clang-tools-extra/clang-tidy/objc/ObjCTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/objc/ObjCTidyModule.cpp
@@ -9,6 +9,7 @@
 #include "../ClangTidy.h"
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
+#include "AssertEquals.h"
 #include "AvoidNSErrorInitCheck.h"
 #include "DeallocInCategoryCheck.h"
 #include "ForbiddenSubclassingCheck.h"
@@ -28,6 +29,8 @@ public:
   void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
     CheckFactories.registerCheck<AvoidNSErrorInitCheck>(
         "objc-avoid-nserror-init");
+    CheckFactories.registerCheck<AssertEquals>("objc-assert-equals");
+
     CheckFactories.registerCheck<DeallocInCategoryCheck>(
         "objc-dealloc-in-category");
     CheckFactories.registerCheck<ForbiddenSubclassingCheck>(