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 @@ -6,6 +6,7 @@ ForbiddenSubclassingCheck.cpp ObjCTidyModule.cpp PropertyDeclarationCheck.cpp + SuperSelfCheck.cpp LINK_LIBS clangAST 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 @@ -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( "objc-property-declaration"); + CheckFactories.registerCheck( + "objc-super-self"); } }; Index: clang-tools-extra/trunk/clang-tidy/objc/SuperSelfCheck.h =================================================================== --- clang-tools-extra/trunk/clang-tidy/objc/SuperSelfCheck.h +++ clang-tools-extra/trunk/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/trunk/clang-tidy/objc/SuperSelfCheck.cpp =================================================================== --- clang-tools-extra/trunk/clang-tidy/objc/SuperSelfCheck.cpp +++ clang-tools-extra/trunk/clang-tidy/objc/SuperSelfCheck.cpp @@ -0,0 +1,127 @@ +//===--- 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, + 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("message"); + + auto Diag = diag(Message->getExprLoc(), "suspicious invocation of %0 in " + "initializer; did you mean to " + "invoke a superclass initializer?") + << Message->getMethodDecl(); + + SourceLocation ReceiverLoc = Message->getReceiverRange().getBegin(); + if (ReceiverLoc.isMacroID() || ReceiverLoc.isInvalid()) + return; + + SourceLocation SelectorLoc = Message->getSelectorStartLoc(); + if (SelectorLoc.isMacroID() || SelectorLoc.isInvalid()) + return; + + Diag << FixItHint::CreateReplacement(Message->getSourceRange(), + StringRef("[super init]")); +} + +} // namespace objc +} // namespace tidy +} // namespace clang Index: clang-tools-extra/trunk/docs/ReleaseNotes.rst =================================================================== --- clang-tools-extra/trunk/docs/ReleaseNotes.rst +++ clang-tools-extra/trunk/docs/ReleaseNotes.rst @@ -108,6 +108,12 @@ 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 ` 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 ` to :doc:`modernize-use-override 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 @@ -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/trunk/docs/clang-tidy/checks/objc-super-self.rst =================================================================== --- clang-tools-extra/trunk/docs/clang-tidy/checks/objc-super-self.rst +++ clang-tools-extra/trunk/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/trunk/test/clang-tidy/objc-super-self.m =================================================================== --- clang-tools-extra/trunk/test/clang-tidy/objc-super-self.m +++ clang-tools-extra/trunk/test/clang-tidy/objc-super-self.m @@ -0,0 +1,86 @@ +// 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 '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 'self' in initializer; did you mean to invoke a superclass initializer? [objc-super-self] +// CHECK-FIXES: self = [super init]; + if (self) { + // ... + } + return self; +} + +#define INITIALIZE() [super self] + +- (instancetype)initWithObject:(NSObject *)objc a:(int)a { + return INITIALIZE(); +// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious invocation of 'self' in initializer; did you mean to invoke a superclass initializer? [objc-super-self] +// CHECK-FIXES: return INITIALIZE(); +} + +#define INITIALIZER_IMPL() return [super self] + +- (instancetype)initWithObject:(NSObject *)objc b:(int)b { + INITIALIZER_IMPL(); +// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: suspicious invocation of 'self' in initializer; did you mean to invoke a superclass initializer? [objc-super-self] +// CHECK-FIXES: INITIALIZER_IMPL(); +} + +#define INITIALIZER_METHOD self + +- (instancetype)initWithObject:(NSObject *)objc c:(int)c { + return [super INITIALIZER_METHOD]; +// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious invocation of 'self' in initializer; did you mean to invoke a superclass initializer? [objc-super-self] +// CHECK-FIXES: return [super INITIALIZER_METHOD]; +} + +#define RECEIVER super + +- (instancetype)initWithObject:(NSObject *)objc d:(int)d { + return [RECEIVER self]; +// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious invocation of 'self' in initializer; did you mean to invoke a superclass initializer? [objc-super-self] +// CHECK-FIXES: return [RECEIVER 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 +