This revision was automatically updated to reflect the committed changes.
stephanemoore marked an inline comment as done.
Closed by commit rL372445: [clang-tidy] Add check for classes missing -hash ⚠️
(authored by stephanemoore, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D67737?vs=220823&id=221147#toc
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D67737/new/
https://reviews.llvm.org/D67737
Files:
clang-tools-extra/trunk/clang-tidy/objc/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/objc/MissingHashCheck.cpp
clang-tools-extra/trunk/clang-tidy/objc/MissingHashCheck.h
clang-tools-extra/trunk/clang-tidy/objc/ObjCTidyModule.cpp
clang-tools-extra/trunk/docs/ReleaseNotes.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/objc-missing-hash.rst
clang-tools-extra/trunk/test/clang-tidy/objc-missing-hash.m
Index: clang-tools-extra/trunk/clang-tidy/objc/ObjCTidyModule.cpp
===================================================================
--- clang-tools-extra/trunk/clang-tidy/objc/ObjCTidyModule.cpp
+++ clang-tools-extra/trunk/clang-tidy/objc/ObjCTidyModule.cpp
@@ -12,6 +12,7 @@
#include "AvoidNSErrorInitCheck.h"
#include "AvoidSpinlockCheck.h"
#include "ForbiddenSubclassingCheck.h"
+#include "MissingHashCheck.h"
#include "PropertyDeclarationCheck.h"
#include "SuperSelfCheck.h"
@@ -30,6 +31,8 @@
"objc-avoid-spinlock");
CheckFactories.registerCheck<ForbiddenSubclassingCheck>(
"objc-forbidden-subclassing");
+ CheckFactories.registerCheck<MissingHashCheck>(
+ "objc-missing-hash");
CheckFactories.registerCheck<PropertyDeclarationCheck>(
"objc-property-declaration");
CheckFactories.registerCheck<SuperSelfCheck>(
Index: clang-tools-extra/trunk/clang-tidy/objc/CMakeLists.txt
===================================================================
--- clang-tools-extra/trunk/clang-tidy/objc/CMakeLists.txt
+++ clang-tools-extra/trunk/clang-tidy/objc/CMakeLists.txt
@@ -4,6 +4,7 @@
AvoidNSErrorInitCheck.cpp
AvoidSpinlockCheck.cpp
ForbiddenSubclassingCheck.cpp
+ MissingHashCheck.cpp
ObjCTidyModule.cpp
PropertyDeclarationCheck.cpp
SuperSelfCheck.cpp
Index: clang-tools-extra/trunk/clang-tidy/objc/MissingHashCheck.cpp
===================================================================
--- clang-tools-extra/trunk/clang-tidy/objc/MissingHashCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/objc/MissingHashCheck.cpp
@@ -0,0 +1,62 @@
+//===--- MissingHashCheck.cpp - clang-tidy --------------------------------===//
+//
+// 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 "MissingHashCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace objc {
+
+namespace {
+
+AST_MATCHER_P(ObjCImplementationDecl, hasInterface,
+ ast_matchers::internal::Matcher<ObjCInterfaceDecl>, Base) {
+ const ObjCInterfaceDecl *InterfaceDecl = Node.getClassInterface();
+ return Base.matches(*InterfaceDecl, Finder, Builder);
+}
+
+AST_MATCHER_P(ObjCContainerDecl, hasInstanceMethod,
+ ast_matchers::internal::Matcher<ObjCMethodDecl>, Base) {
+ // Check each instance method against the provided matcher.
+ for (const auto *I : Node.instance_methods()) {
+ if (Base.matches(*I, Finder, Builder))
+ return true;
+ }
+ return false;
+}
+
+} // namespace
+
+void MissingHashCheck::registerMatchers(MatchFinder *Finder) {
+ // This check should only be applied to Objective-C sources.
+ if (!getLangOpts().ObjC)
+ return;
+
+ Finder->addMatcher(
+ objcMethodDecl(
+ hasName("isEqual:"), isInstanceMethod(),
+ hasDeclContext(objcImplementationDecl(
+ hasInterface(isDirectlyDerivedFrom("NSObject")),
+ unless(hasInstanceMethod(hasName("hash"))))
+ .bind("impl"))),
+ this);
+}
+
+void MissingHashCheck::check(const MatchFinder::MatchResult &Result) {
+ const auto *ID = Result.Nodes.getNodeAs<ObjCImplementationDecl>("impl");
+ diag(ID->getLocation(), "%0 implements -isEqual: without implementing -hash")
+ << ID;
+}
+
+} // namespace objc
+} // namespace tidy
+} // namespace clang
Index: clang-tools-extra/trunk/clang-tidy/objc/MissingHashCheck.h
===================================================================
--- clang-tools-extra/trunk/clang-tidy/objc/MissingHashCheck.h
+++ clang-tools-extra/trunk/clang-tidy/objc/MissingHashCheck.h
@@ -0,0 +1,35 @@
+//===--- MissingHashCheck.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 LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_OBJC_MISSINGHASHCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_OBJC_MISSINGHASHCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace objc {
+
+/// Finds Objective-C implementations that implement -isEqual: without also
+/// appropriately implementing -hash.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/objc-missing-hash.html
+class MissingHashCheck : public ClangTidyCheck {
+public:
+ MissingHashCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace objc
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_OBJC_MISSINGHASHCHECK_H
Index: clang-tools-extra/trunk/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/trunk/docs/ReleaseNotes.rst
+++ clang-tools-extra/trunk/docs/ReleaseNotes.rst
@@ -91,6 +91,12 @@
Finds historical use of ``unsigned`` to hold vregs and physregs and rewrites
them to use ``Register``
+- New :doc:`objc-missing-hash
+ <clang-tidy/checks/objc-missing-hash>` check.
+
+ Finds Objective-C implementations that implement ``-isEqual:`` without also
+ appropriately implementing ``-hash``.
+
- Improved :doc:`bugprone-posix-return
<clang-tidy/checks/bugprone-posix-return>` check.
Index: clang-tools-extra/trunk/docs/clang-tidy/checks/objc-missing-hash.rst
===================================================================
--- clang-tools-extra/trunk/docs/clang-tidy/checks/objc-missing-hash.rst
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/objc-missing-hash.rst
@@ -0,0 +1,16 @@
+.. title:: clang-tidy - objc-missing-hash
+
+objc-missing-hash
+=================
+
+Finds Objective-C implementations that implement ``-isEqual:`` without also
+appropriately implementing ``-hash``.
+
+Apple documentation highlights that objects that are equal must have the same
+hash value:
+https://developer.apple.com/documentation/objectivec/1418956-nsobject/1418795-isequal?language=objc
+
+Note that the check only verifies the presence of ``-hash`` in scenarios where
+its omission could result in unexpected behavior. The verification of the
+implementation of ``-hash`` is the responsibility of the developer, e.g.,
+through the addition of unit tests to verify the implementation.
Index: clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
===================================================================
--- clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
@@ -325,6 +325,7 @@
objc-avoid-nserror-init
objc-avoid-spinlock
objc-forbidden-subclassing
+ objc-missing-hash
objc-property-declaration
objc-super-self
openmp-exception-escape
Index: clang-tools-extra/trunk/test/clang-tidy/objc-missing-hash.m
===================================================================
--- clang-tools-extra/trunk/test/clang-tidy/objc-missing-hash.m
+++ clang-tools-extra/trunk/test/clang-tidy/objc-missing-hash.m
@@ -0,0 +1,68 @@
+// RUN: %check_clang_tidy %s objc-missing-hash %t
+
+typedef _Bool BOOL;
+#define YES 1
+#define NO 0
+typedef unsigned int NSUInteger;
+typedef void *id;
+
+@interface NSObject
+- (NSUInteger)hash;
+- (BOOL)isEqual:(id)object;
+@end
+
+@interface MissingHash : NSObject
+@end
+
+@implementation MissingHash
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: 'MissingHash' implements -isEqual: without implementing -hash [objc-missing-hash]
+
+- (BOOL)isEqual:(id)object {
+ return YES;
+}
+
+@end
+
+@interface HasHash : NSObject
+@end
+
+@implementation HasHash
+
+- (NSUInteger)hash {
+ return 0;
+}
+
+- (BOOL)isEqual:(id)object {
+ return YES;
+}
+
+@end
+
+@interface NSArray : NSObject
+@end
+
+@interface MayHaveInheritedHash : NSArray
+@end
+
+@implementation MayHaveInheritedHash
+
+- (BOOL)isEqual:(id)object {
+ return YES;
+}
+
+@end
+
+@interface AnotherRootClass
+@end
+
+@interface NotDerivedFromNSObject : AnotherRootClass
+@end
+
+@implementation NotDerivedFromNSObject
+
+- (BOOL)isEqual:(id)object {
+ return NO;
+}
+
+@end
+
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
