stephanemoore updated this revision to Diff 192315.
stephanemoore marked 5 inline comments as done.
stephanemoore added a comment.
Fix diagnostic format string to actually use the message's method declaration.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D59806/new/
https://reviews.llvm.org/D59806
Files:
clang-tools-extra/clang-tidy/objc/CMakeLists.txt
clang-tools-extra/clang-tidy/objc/ObjCTidyModule.cpp
clang-tools-extra/clang-tidy/objc/SuperSelfCheck.cpp
clang-tools-extra/clang-tidy/objc/SuperSelfCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
clang-tools-extra/docs/clang-tidy/checks/objc-super-self.rst
clang-tools-extra/test/clang-tidy/objc-super-self.m
Index: clang-tools-extra/test/clang-tidy/objc-super-self.m
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/objc-super-self.m
@@ -0,0 +1,54 @@
+// RUN: %check_clang_tidy %s objc-super-self %t
+
+@interface NSObject
+- (instancetype)init;
+- (instancetype)self;
+@end
+
+@interface NSObjectDerivedClass : NSObject
+@end
+
+@implementation NSObjectDerivedClass
+
+- (instancetype)init {
+ return [super self];
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious invocation of -[NSObject self] in initializer; did you mean to invoke a superclass initializer? [objc-super-self]
+// CHECK-FIXES: return [super init];
+}
+
+- (instancetype)initWithObject:(NSObject *)obj {
+ self = [super self];
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious invocation of -[NSObject self] in initializer; did you mean to invoke a superclass initializer? [objc-super-self]
+// CHECK-FIXES: self = [super init];
+ if (self) {
+ // ...
+ }
+ return self;
+}
+
+- (instancetype)foo {
+ return [super self];
+}
+
+- (instancetype)bar {
+ return [self self];
+}
+
+@end
+
+@interface RootClass
+- (instancetype)init;
+- (instancetype)self;
+@end
+
+@interface NotNSObjectDerivedClass : RootClass
+@end
+
+@implementation NotNSObjectDerivedClass
+
+- (instancetype)init {
+ return [super self];
+}
+
+@end
+
Index: clang-tools-extra/docs/clang-tidy/checks/objc-super-self.rst
===================================================================
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/objc-super-self.rst
@@ -0,0 +1,13 @@
+.. title:: clang-tidy - objc-super-self
+
+objc-super-self
+===============
+
+Finds invocations of -self on super instances in initializers of subclasses of
+`NSObject` and recommends calling a superclass initializer instead.
+
+Invoking -self on super instances in initializers is a common programmer error
+when the programmer's original intent is to call a superclass initializer.
+Failing to call a superclass initializer breaks initializer chaining and can
+result in invalid object initialization.
+
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
@@ -227,6 +227,7 @@
objc-avoid-spinlock
objc-forbidden-subclassing
objc-property-declaration
+ objc-super-self
openmp-exception-escape
openmp-use-default-none
performance-faster-string-find
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -108,6 +108,11 @@
Checks whether there are underscores in googletest test and test case names in
test macros, which is prohibited by the Googletest FAQ.
+- New :doc:`objc-super-self <clang-tidy/checks/objc-super-self>` check.
+
+ Finds invocations of -self on super instances in initializers of subclasses
+ of `NSObject` and recommends calling a superclass initializer instead.
+
- New alias :doc:`cppcoreguidelines-explicit-virtual-functions
<clang-tidy/checks/cppcoreguidelines-explicit-virtual-functions>` to
:doc:`modernize-use-override
Index: clang-tools-extra/clang-tidy/objc/SuperSelfCheck.h
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/objc/SuperSelfCheck.h
@@ -0,0 +1,36 @@
+//===--- SuperSelfCheck.h - clang-tidy --------------------------*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_OBJC_SUPERSELFCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_OBJC_SUPERSELFCHECK_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace objc {
+
+/// Finds invocations of -self on super instances in initializers of subclasses
+/// of NSObject and recommends calling a superclass initializer instead.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/objc-super-self.html
+class SuperSelfCheck : public ClangTidyCheck {
+public:
+ SuperSelfCheck(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_SUPERSELFCHECK_H
Index: clang-tools-extra/clang-tidy/objc/SuperSelfCheck.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/objc/SuperSelfCheck.cpp
@@ -0,0 +1,118 @@
+//===--- SuperSelfCheck.cpp - clang-tidy ----------------------------------===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "SuperSelfCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace objc {
+
+namespace {
+
+/// \brief Matches Objective-C methods in the initializer family.
+///
+/// Example matches -init and -initWithInt:.
+/// (matcher = objcMethodDecl(isInitializer()))
+/// \code
+/// @interface Foo
+/// - (instancetype)init;
+/// - (instancetype)initWithInt:(int)i;
+/// + (instancetype)init;
+/// - (void)bar;
+/// @end
+/// \endcode
+AST_MATCHER(ObjCMethodDecl, isInitializer) {
+ return Node.getMethodFamily() == OMF_init;
+}
+
+/// \brief Matches Objective-C implementations of classes that directly or
+/// indirectly have a superclass matching \c InterfaceDecl.
+///
+/// Note that a class is not considered to be a subclass of itself.
+///
+/// Example matches implementation declarations for Y and Z.
+/// (matcher = objcInterfaceDecl(isSubclassOf(hasName("X"))))
+/// \code
+/// @interface X
+/// @end
+/// @interface Y : X
+/// @end
+/// @implementation Y // directly derived
+/// @end
+/// @interface Z : Y
+/// @end
+/// @implementation Z // indirectly derived
+/// @end
+/// \endcode
+AST_MATCHER_P(ObjCImplementationDecl, isSubclassOf,
+ ast_matchers::internal::Matcher<ObjCInterfaceDecl>,
+ InterfaceDecl) {
+ // Check if any of the superclasses of the class match.
+ for (const ObjCInterfaceDecl *SuperClass =
+ Node.getClassInterface()->getSuperClass();
+ SuperClass != nullptr; SuperClass = SuperClass->getSuperClass()) {
+ if (InterfaceDecl.matches(*SuperClass, Finder, Builder))
+ return true;
+ }
+
+ // No matches found.
+ return false;
+}
+
+/// \brief Matches Objective-C message expressions where the receiver is the
+/// super instance.
+///
+/// Example matches the invocations of -banana and -orange.
+/// (matcher = objcMessageExpr(isMessagingSuperInstance()))
+/// \code
+/// - (void)banana {
+/// [self apple]
+/// [super banana];
+/// [super orange];
+/// }
+/// \endcode
+AST_MATCHER(ObjCMessageExpr, isMessagingSuperInstance) {
+ return Node.getReceiverKind() == ObjCMessageExpr::SuperInstance;
+}
+
+} // namespace
+
+void SuperSelfCheck::registerMatchers(MatchFinder *Finder) {
+ // This check should only be applied to Objective-C sources.
+ if (!getLangOpts().ObjC)
+ return;
+
+ Finder->addMatcher(
+ objcMessageExpr(
+ hasSelector("self"), isMessagingSuperInstance(),
+ hasAncestor(objcMethodDecl(isInitializer(),
+ hasDeclContext(objcImplementationDecl(
+ isSubclassOf(hasName("NSObject")))))))
+ .bind("message"),
+ this);
+}
+
+void SuperSelfCheck::check(const MatchFinder::MatchResult &Result) {
+ const auto *Message = Result.Nodes.getNodeAs<ObjCMessageExpr>("message");
+
+ diag(Message->getBeginLoc(), "suspicious invocation of %0 in "
+ "initializer; did you mean to "
+ "invoke a superclass initializer?")
+ << Message->getMethodDecl()
+ << FixItHint::CreateReplacement(Message->getSourceRange(),
+ StringRef("[super init]"));
+}
+
+} // namespace objc
+} // namespace tidy
+} // namespace clang
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
@@ -13,6 +13,7 @@
#include "AvoidSpinlockCheck.h"
#include "ForbiddenSubclassingCheck.h"
#include "PropertyDeclarationCheck.h"
+#include "SuperSelfCheck.h"
using namespace clang::ast_matchers;
@@ -31,6 +32,8 @@
"objc-forbidden-subclassing");
CheckFactories.registerCheck<PropertyDeclarationCheck>(
"objc-property-declaration");
+ CheckFactories.registerCheck<SuperSelfCheck>(
+ "objc-super-self");
}
};
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
@@ -6,6 +6,7 @@
ForbiddenSubclassingCheck.cpp
ObjCTidyModule.cpp
PropertyDeclarationCheck.cpp
+ SuperSelfCheck.cpp
LINK_LIBS
clangAST
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
