oontvoo updated this revision to Diff 391472.
oontvoo added a comment.
doc
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D114975/new/
https://reviews.llvm.org/D114975
Files:
clang-tools-extra/clang-tidy/objc/AssertEquals.cpp
clang-tools-extra/clang-tidy/objc/AssertEquals.h
clang-tools-extra/clang-tidy/objc/CMakeLists.txt
clang-tools-extra/clang-tidy/objc/ObjCTidyModule.cpp
clang-tools-extra/docs/clang-tidy/checks/list.rst
clang-tools-extra/docs/clang-tidy/checks/objc-assert-equals.rst
clang-tools-extra/test/clang-tidy/checkers/Inputs/objc-assert/XCTestAssertions.h
clang-tools-extra/test/clang-tidy/checkers/objc-assert-equals.m
Index: clang-tools-extra/test/clang-tidy/checkers/objc-assert-equals.m
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/objc-assert-equals.m
@@ -0,0 +1,25 @@
+// RUN: %check_clang_tidy %s objc-assert-equals %t -- -- -I %S/Inputs/objc-assert
+#include "XCTestAssertions.h"
+// Can't reference NSString directly so we use this getStr() instead.
+__typeof(@"abc") getStr() {
+ return @"abc";
+}
+void foo() {
+ XCTAssertEqual(getStr(), @"abc");
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use XCTAssertEqualObjects for comparing objects
+ // CHECK-FIXES: XCTAssertEqualObjects(getStr(), @"abc");
+ XCTAssertEqual(@"abc", @"abc");
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use XCTAssertEqualObjects for comparing objects
+ // CHECK-FIXES: XCTAssertEqualObjects(@"abc", @"abc");
+ XCTAssertEqual(@"abc", getStr());
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use XCTAssertEqualObjects for comparing objects
+ // CHECK-FIXES: XCTAssertEqualObjects(@"abc", getStr());
+ XCTAssertEqual(getStr(), getStr());
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use XCTAssertEqualObjects for comparing objects
+ // CHECK-FIXES: XCTAssertEqualObjects(getStr(), getStr());
+ // Primitive types should be ok
+ XCTAssertEqual(123, 123);
+ XCTAssertEqual(123.0, 123.45);
+ // FIXME: This is the case where we don't diagnose properly.
+ // XCTAssertEqual(@"abc" != @"abc", @"xyz" != @"xyz")
+}
Index: clang-tools-extra/test/clang-tidy/checkers/Inputs/objc-assert/XCTestAssertions.h
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/Inputs/objc-assert/XCTestAssertions.h
@@ -0,0 +1,30 @@
+#ifndef THIRD_PARTY_LLVM_LLVM_PROJECT_CLANG_TOOLS_EXTRA_TEST_CLANG_TIDY_CHECKERS_INPUTS_OBJC_ASSERT_XCTESTASSERTIONS_H_
+#define THIRD_PARTY_LLVM_LLVM_PROJECT_CLANG_TOOLS_EXTRA_TEST_CLANG_TIDY_CHECKERS_INPUTS_OBJC_ASSERT_XCTESTASSERTIONS_H_
+
+#define _XCTPrimitiveAssertEqual(test, expression1, expressionStr1, \
+ expression2, expressionStr2, ...) \
+ ({ \
+ __typeof__(expression1) expressionValue1 = (expression1); \
+ __typeof__(expression2) expressionValue2 = (expression2); \
+ if (expressionValue1 != expressionValue2) { \
+ } \
+ })
+
+#define _XCTPrimitiveAssertEqualObjects(test, expression1, expressionStr1, \
+ expression2, expressionStr2, ...) \
+ ({ \
+ __typeof__(expression1) expressionValue1 = (expression1); \
+ __typeof__(expression2) expressionValue2 = (expression2); \
+ if (expressionValue1 != expressionValue2) { \
+ } \
+ })
+
+#define XCTAssertEqual(expression1, expression2, ...) \
+ _XCTPrimitiveAssertEqual(nil, expression1, @ #expression1, expression2, \
+ @ #expression2, __VA_ARGS__)
+
+#define XCTAssertEqualObjects(expression1, expression2, ...) \
+ _XCTPrimitiveAssertEqualObjects(nil, expression1, @ #expression1, \
+ expression2, @ #expression2, __VA_ARGS__)
+
+#endif // THIRD_PARTY_LLVM_LLVM_PROJECT_CLANG_TOOLS_EXTRA_TEST_CLANG_TIDY_CHECKERS_INPUTS_OBJC_ASSERT_XCTESTASSERTIONS_H_
Index: clang-tools-extra/docs/clang-tidy/checks/objc-assert-equals.rst
===================================================================
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/objc-assert-equals.rst
@@ -0,0 +1,11 @@
+.. title:: clang-tidy - objc-assert-equals
+
+objc-assert-equals
+==================
+
+Finds improper usages of `XCTAssertEqual` and `XCTAssertNotEqual` and replaces
+them with `XCTAssertEqualObjects` or `XCTAssertNotEqualObjects`.
+
+This makes tests less fragile, as many improperly rely on pointer equality for
+strings that have equal values. This assumption is not guarantted by the
+language.
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===================================================================
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -261,6 +261,7 @@
`mpi-buffer-deref <mpi-buffer-deref.html>`_, "Yes"
`mpi-type-mismatch <mpi-type-mismatch.html>`_, "Yes"
`objc-avoid-nserror-init <objc-avoid-nserror-init.html>`_,
+ `objc-assert-equals <objc-assert-equals.html>`_, "Yes"
`objc-dealloc-in-category <objc-dealloc-in-category.html>`_,
`objc-forbidden-subclassing <objc-forbidden-subclassing.html>`_,
`objc-missing-hash <objc-missing-hash.html>`_,
Index: clang-tools-extra/clang-tidy/objc/ObjCTidyModule.cpp
===================================================================
--- clang-tools-extra/clang-tidy/objc/ObjCTidyModule.cpp
+++ 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 @@
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>(
Index: clang-tools-extra/clang-tidy/objc/CMakeLists.txt
===================================================================
--- clang-tools-extra/clang-tidy/objc/CMakeLists.txt
+++ clang-tools-extra/clang-tidy/objc/CMakeLists.txt
@@ -4,6 +4,7 @@
)
add_clang_library(clangTidyObjCModule
+ AssertEquals.cpp
AvoidNSErrorInitCheck.cpp
DeallocInCategoryCheck.cpp
ForbiddenSubclassingCheck.cpp
Index: clang-tools-extra/clang-tidy/objc/AssertEquals.h
===================================================================
--- /dev/null
+++ 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_
Index: clang-tools-extra/clang-tidy/objc/AssertEquals.cpp
===================================================================
--- /dev/null
+++ 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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
