Index: clang-tools-extra/clang-tidy/google/AvoidNSObjectNewCheck.h =================================================================== --- /dev/null +++ clang-tools-extra/clang-tidy/google/AvoidNSObjectNewCheck.h @@ -0,0 +1,38 @@ +//===--- AvoidNSObjectNewCheck.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_GOOGLE_AVOIDNSOBJECTNEWCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_GOOGLE_AVOIDNSOBJECTNEWCHECK_H + +#include "../ClangTidyCheck.h" + +namespace clang { +namespace tidy { +namespace google { +namespace objc { + +/// This check finds Objective-C code that uses +new to create object instances, +/// or overrides +new in classes. Both are forbidden by Google's Objective-C +/// style guide. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/google-avoid-nsobject-new.html +class AvoidNSObjectNewCheck : public ClangTidyCheck { +public: + AvoidNSObjectNewCheck(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 google +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_GOOGLE_AVOIDNSOBJECTNEWCHECK_H Index: clang-tools-extra/clang-tidy/google/AvoidNSObjectNewCheck.cpp =================================================================== --- /dev/null +++ clang-tools-extra/clang-tidy/google/AvoidNSObjectNewCheck.cpp @@ -0,0 +1,130 @@ +//===--- AvoidNSObjectNewCheck.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 "AvoidNSObjectNewCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Basic/LangOptions.h" +#include "clang/Basic/SourceLocation.h" +#include "clang/Basic/SourceManager.h" +#include "llvm/Support/FormatVariadic.h" +#include +#include + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace google { +namespace objc { + +static bool isMessageExpressionInsideMacro(const ObjCMessageExpr *Expr) { + SourceLocation ReceiverLocation = Expr->getReceiverRange().getBegin(); + if (ReceiverLocation.isMacroID()) + return true; + + SourceLocation SelectorLocation = Expr->getSelectorStartLoc(); + if (SelectorLocation.isMacroID()) + return true; + + return false; +} + +// Walk up the class hierarchy looking for an -init method, returning true +// if one is found and has not been marked unavailable. +static bool isInitMethodAvailable(const ObjCInterfaceDecl *ClassDecl) { + while (ClassDecl != nullptr) { + for (const auto *MethodDecl : ClassDecl->instance_methods()) { + if (MethodDecl->getSelector().getAsString() == "init") + return !MethodDecl->isUnavailable(); + } + ClassDecl = ClassDecl->getSuperClass(); + } + + // No -init method found in the class hierarchy. This should occur only rarely + // in Objective-C code, and only really applies to classes not derived from + // NSObject. + return false; +} + +// Returns the string for the Objective-C message receiver. Keeps any generics +// included in the receiver class type, which are stripped if the class type is +// used. While the generics arguments will not make any difference to the +// returned code at this time, the style guide allows them and they should be +// left in any fix-it hint. +static StringRef getReceiverString(SourceRange ReceiverRange, + const SourceManager &SM, + const LangOptions &LangOpts) { + CharSourceRange CharRange = Lexer::makeFileCharRange( + CharSourceRange::getTokenRange(ReceiverRange), SM, LangOpts); + return Lexer::getSourceText(CharRange, SM, LangOpts); +} + +static FixItHint getCallFixItHint(const ObjCMessageExpr *Expr, + const SourceManager &SM, + const LangOptions &LangOpts) { + // Check whether the messaged class has a known factory method to use instead + // of -init. + StringRef Receiver = + getReceiverString(Expr->getReceiverRange(), SM, LangOpts); + // Some classes should use standard factory methods instead of alloc/init. + std::map ClassToFactoryMethodMap = {{"NSDate", "date"}, + {"NSNull", "null"}}; + auto FoundClassFactory = ClassToFactoryMethodMap.find(Receiver); + if (FoundClassFactory != ClassToFactoryMethodMap.end()) { + StringRef ClassName = FoundClassFactory->first; + StringRef FactorySelector = FoundClassFactory->second; + std::string NewCall = + llvm::formatv("[{0} {1}]", ClassName, FactorySelector); + return FixItHint::CreateReplacement(Expr->getSourceRange(), NewCall); + } + + if (isInitMethodAvailable(Expr->getReceiverInterface())) { + std::string NewCall = llvm::formatv("[[{0} alloc] init]", Receiver); + return FixItHint::CreateReplacement(Expr->getSourceRange(), NewCall); + } + + return {}; // No known replacement available. +} + +void AvoidNSObjectNewCheck::registerMatchers(MatchFinder *Finder) { + if (!getLangOpts().ObjC) + return; + + // Add two matchers, to catch calls to +new and implementations of +new. + Finder->addMatcher( + objcMessageExpr(isClassMessage(), hasSelector("new")).bind("new_call"), + this); + Finder->addMatcher( + objcMethodDecl(isClassMethod(), isDefinition(), hasName("new")) + .bind("new_override"), + this); +} + +void AvoidNSObjectNewCheck::check(const MatchFinder::MatchResult &Result) { + if (const auto *CallExpr = + Result.Nodes.getNodeAs("new_call")) { + // Don't warn if the call expression originates from a macro expansion. + if (isMessageExpressionInsideMacro(CallExpr)) + return; + + diag(CallExpr->getExprLoc(), "do not create objects with +new") + << getCallFixItHint(CallExpr, *Result.SourceManager, + Result.Context->getLangOpts()); + } + + if (const auto *DeclExpr = + Result.Nodes.getNodeAs("new_override")) { + diag(DeclExpr->getBeginLoc(), "classes should not override +new"); + } +} + +} // namespace objc +} // namespace google +} // namespace tidy +} // namespace clang Index: clang-tools-extra/clang-tidy/google/CMakeLists.txt =================================================================== --- clang-tools-extra/clang-tidy/google/CMakeLists.txt +++ clang-tools-extra/clang-tidy/google/CMakeLists.txt @@ -2,6 +2,7 @@ add_clang_library(clangTidyGoogleModule AvoidCStyleCastsCheck.cpp + AvoidNSObjectNewCheck.cpp AvoidThrowingObjCExceptionCheck.cpp AvoidUnderscoreInGoogletestNameCheck.cpp DefaultArgumentsCheck.cpp Index: clang-tools-extra/clang-tidy/google/GoogleTidyModule.cpp =================================================================== --- clang-tools-extra/clang-tidy/google/GoogleTidyModule.cpp +++ clang-tools-extra/clang-tidy/google/GoogleTidyModule.cpp @@ -13,6 +13,7 @@ #include "../readability/FunctionSizeCheck.h" #include "../readability/NamespaceCommentCheck.h" #include "AvoidCStyleCastsCheck.h" +#include "AvoidNSObjectNewCheck.h" #include "AvoidThrowingObjCExceptionCheck.h" #include "AvoidUnderscoreInGoogletestNameCheck.h" #include "DefaultArgumentsCheck.h" @@ -49,6 +50,8 @@ "google-explicit-constructor"); CheckFactories.registerCheck( "google-global-names-in-headers"); + CheckFactories.registerCheck( + "google-objc-avoid-nsobject-new"); CheckFactories.registerCheck( "google-objc-avoid-throwing-exception"); CheckFactories.registerCheck( Index: clang-tools-extra/docs/ReleaseNotes.rst =================================================================== --- clang-tools-extra/docs/ReleaseNotes.rst +++ clang-tools-extra/docs/ReleaseNotes.rst @@ -101,6 +101,12 @@ Finds and fixes ``absl::Time`` subtraction expressions to do subtraction in the Time domain instead of the numeric domain. +- New :doc:`google-objc-avoid-nsobject-new + ` check. + + Checks for calls to ``+new`` or overrides of it, which are prohibited by the + Google Objective-C style guide. + - New :doc:`google-readability-avoid-underscore-in-googletest-name ` check. Index: clang-tools-extra/docs/clang-tidy/checks/google-objc-avoid-nsobject-new.rst =================================================================== --- /dev/null +++ clang-tools-extra/docs/clang-tidy/checks/google-objc-avoid-nsobject-new.rst @@ -0,0 +1,29 @@ +.. title:: clang-tidy - google-objc-avoid-nsobject-new + +google-objc-avoid-nsobject-new +============================== + +Finds calls to ``+new`` or overrides of it, which are prohibited by the +Google Objective-C style guide. + +The Google Objective-C style guide forbids calling ``+new`` or overriding it in +class implementations, preferring ``+alloc`` and ``-init`` methods to +instantiate objects. + +An example: + +.. code-block:: objc + + NSDate *now = [NSDate new]; + Foo *bar = [Foo new]; + +Instead, code should use ``+alloc``/``-init`` or class factory methods. + +.. code-block:: objc + + NSDate *now = [NSDate date]; + Foo *bar = [[Foo alloc] init]; + +This check corresponds to the Google Objective-C Style Guide rule +`Do Not Use +new +`_. 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 @@ -133,6 +133,7 @@ google-default-arguments google-explicit-constructor google-global-names-in-headers + google-objc-avoid-nsobject-new google-objc-avoid-throwing-exception google-objc-function-naming google-objc-global-variable-declaration Index: clang-tools-extra/test/clang-tidy/google-objc-avoid-nsobject-new.m =================================================================== --- /dev/null +++ clang-tools-extra/test/clang-tidy/google-objc-avoid-nsobject-new.m @@ -0,0 +1,79 @@ +// RUN: %check_clang_tidy %s google-objc-avoid-nsobject-new %t + +@interface NSObject ++ (instancetype)new; ++ (instancetype)alloc; +- (instancetype)init; +@end + +@interface NSProxy // Root class with no -init method. +@end + +// NSDate provides a specific factory method. +@interface NSDate : NSObject ++ (instancetype)date; +@end + +// For testing behavior with Objective-C Generics. +@interface NSMutableDictionary<__covariant KeyType, __covariant ObjectType> : NSObject +@end + +@class NSString; + +#define ALLOCATE_OBJECT(_Type) [_Type new] + +void CheckSpecificInitRecommendations(void) { + NSObject *object = [NSObject new]; + // CHECK-MESSAGES: [[@LINE-1]]:22: warning: do not create objects with +new [google-objc-avoid-nsobject-new] + // CHECK-FIXES: [NSObject alloc] init]; + + NSDate *correctDate = [NSDate date]; + NSDate *incorrectDate = [NSDate new]; + // CHECK-MESSAGES: [[@LINE-1]]:27: warning: do not create objects with +new [google-objc-avoid-nsobject-new] + // CHECK-FIXES: [NSDate date]; + + NSObject *macroCreated = ALLOCATE_OBJECT(NSObject); // Shouldn't warn on macros. + + NSMutableDictionary *dict = [NSMutableDictionary new]; + // CHECK-MESSAGES: [[@LINE-1]]:31: warning: do not create objects with +new [google-objc-avoid-nsobject-new] + // CHECK-FIXES: [NSMutableDictionary alloc] init]; +} + +@interface Foo : NSObject ++ (instancetype)new; // Declare again to suppress warning. +- (instancetype)initWithInt:(int)anInt; +- (instancetype)init __attribute__((unavailable)); + +- (id)new; +@end + +@interface Baz : Foo // Check unavailable -init through inheritance. +@end + +@interface ProxyFoo : NSProxy +@end + +void CallNewWhenInitUnavailable(void) { + Foo *foo = [Foo new]; + // CHECK-MESSAGES: [[@LINE-1]]:14: warning: do not create objects with +new [google-objc-avoid-nsobject-new] + + Baz *baz = [Baz new]; + // CHECK-MESSAGES: [[@LINE-1]]:14: warning: do not create objects with +new [google-objc-avoid-nsobject-new] + + // Instance method -new calls may be weird, but are not strictly forbidden. + Foo *bar = [[Foo alloc] initWithInt:4]; + [bar new]; + + ProxyFoo *proxy = [ProxyFoo new]; + // CHECK-MESSAGES: [[@LINE-1]]:21: warning: do not create objects with +new [google-objc-avoid-nsobject-new] +} + +@interface HasNewOverride : NSObject +@end + +@implementation HasNewOverride ++ (instancetype)new { + return [[self alloc] init]; +} +// CHECK-MESSAGES: [[@LINE-3]]:1: warning: classes should not override +new [google-objc-avoid-nsobject-new] +@end