[clang-tools-extra] r361487 - [clang-tidy] New check calling out uses of +new in Objective-C code
Dmitri Gribenko via cfe-commits
cfe-commits at lists.llvm.org
Thu May 23 05:01:26 PDT 2019
Author: gribozavr
Date: Thu May 23 05:01:26 2019
New Revision: 361487
URL: http://llvm.org/viewvc/llvm-project?rev=361487&view=rev
Log:
[clang-tidy] New check calling out uses of +new in Objective-C code
Summary:
Google's Objective-C style guide forbids calling or overriding +new to instantiate objects. This check warns on violations.
Style guide reference: https://google.github.io/styleguide/objcguide.html#do-not-use-new
Patch by Michael Wyman.
Reviewers: benhamilton, aaron.ballman, JonasToth, gribozavr, ilya-biryukov, stephanemoore, mwyman
Reviewed By: aaron.ballman, gribozavr, stephanemoore, mwyman
Subscribers: stephanemoore, xazax.hun, Eugene.Zelenko, mgorny, cfe-commits
Tags: #clang, #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D61350
Added:
clang-tools-extra/trunk/clang-tidy/google/AvoidNSObjectNewCheck.cpp
clang-tools-extra/trunk/clang-tidy/google/AvoidNSObjectNewCheck.h
clang-tools-extra/trunk/docs/clang-tidy/checks/google-objc-avoid-nsobject-new.rst
clang-tools-extra/trunk/test/clang-tidy/google-objc-avoid-nsobject-new.m
Modified:
clang-tools-extra/trunk/clang-tidy/google/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/google/GoogleTidyModule.cpp
clang-tools-extra/trunk/docs/ReleaseNotes.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
Added: clang-tools-extra/trunk/clang-tidy/google/AvoidNSObjectNewCheck.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/google/AvoidNSObjectNewCheck.cpp?rev=361487&view=auto
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/google/AvoidNSObjectNewCheck.cpp (added)
+++ clang-tools-extra/trunk/clang-tidy/google/AvoidNSObjectNewCheck.cpp Thu May 23 05:01:26 2019
@@ -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 <map>
+#include <string>
+
+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<StringRef, StringRef> 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<ObjCMessageExpr>("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<ObjCMethodDecl>("new_override")) {
+ diag(DeclExpr->getBeginLoc(), "classes should not override +new");
+ }
+}
+
+} // namespace objc
+} // namespace google
+} // namespace tidy
+} // namespace clang
Added: clang-tools-extra/trunk/clang-tidy/google/AvoidNSObjectNewCheck.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/google/AvoidNSObjectNewCheck.h?rev=361487&view=auto
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/google/AvoidNSObjectNewCheck.h (added)
+++ clang-tools-extra/trunk/clang-tidy/google/AvoidNSObjectNewCheck.h Thu May 23 05:01:26 2019
@@ -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
Modified: clang-tools-extra/trunk/clang-tidy/google/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/google/CMakeLists.txt?rev=361487&r1=361486&r2=361487&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/google/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/google/CMakeLists.txt Thu May 23 05:01:26 2019
@@ -2,6 +2,7 @@ set(LLVM_LINK_COMPONENTS support)
add_clang_library(clangTidyGoogleModule
AvoidCStyleCastsCheck.cpp
+ AvoidNSObjectNewCheck.cpp
AvoidThrowingObjCExceptionCheck.cpp
AvoidUnderscoreInGoogletestNameCheck.cpp
DefaultArgumentsCheck.cpp
Modified: clang-tools-extra/trunk/clang-tidy/google/GoogleTidyModule.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/google/GoogleTidyModule.cpp?rev=361487&r1=361486&r2=361487&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/google/GoogleTidyModule.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/google/GoogleTidyModule.cpp Thu May 23 05:01:26 2019
@@ -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 @@ class GoogleModule : public ClangTidyMod
"google-explicit-constructor");
CheckFactories.registerCheck<readability::GlobalNamesInHeadersCheck>(
"google-global-names-in-headers");
+ CheckFactories.registerCheck<objc::AvoidNSObjectNewCheck>(
+ "google-objc-avoid-nsobject-new");
CheckFactories.registerCheck<objc::AvoidThrowingObjCExceptionCheck>(
"google-objc-avoid-throwing-exception");
CheckFactories.registerCheck<objc::FunctionNamingCheck>(
Modified: clang-tools-extra/trunk/docs/ReleaseNotes.rst
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/ReleaseNotes.rst?rev=361487&r1=361486&r2=361487&view=diff
==============================================================================
--- clang-tools-extra/trunk/docs/ReleaseNotes.rst (original)
+++ clang-tools-extra/trunk/docs/ReleaseNotes.rst Thu May 23 05:01:26 2019
@@ -122,6 +122,12 @@ Improvements to clang-tidy
Checks whether there are underscores in googletest test and test case names in
test macros, which is prohibited by the Googletest FAQ.
+- New :doc:`google-objc-avoid-nsobject-new
+ <clang-tidy/checks/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:`objc-super-self <clang-tidy/checks/objc-super-self>` check.
Finds invocations of ``-self`` on super instances in initializers of
Added: clang-tools-extra/trunk/docs/clang-tidy/checks/google-objc-avoid-nsobject-new.rst
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/google-objc-avoid-nsobject-new.rst?rev=361487&view=auto
==============================================================================
--- clang-tools-extra/trunk/docs/clang-tidy/checks/google-objc-avoid-nsobject-new.rst (added)
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/google-objc-avoid-nsobject-new.rst Thu May 23 05:01:26 2019
@@ -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
+<https://google.github.io/styleguide/objcguide.html#do-not-use-new>`_.
Modified: clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst?rev=361487&r1=361486&r2=361487&view=diff
==============================================================================
--- clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst (original)
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst Thu May 23 05:01:26 2019
@@ -135,6 +135,7 @@ Clang-Tidy Checks
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
Added: clang-tools-extra/trunk/test/clang-tidy/google-objc-avoid-nsobject-new.m
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/google-objc-avoid-nsobject-new.m?rev=361487&view=auto
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/google-objc-avoid-nsobject-new.m (added)
+++ clang-tools-extra/trunk/test/clang-tidy/google-objc-avoid-nsobject-new.m Thu May 23 05:01:26 2019
@@ -0,0 +1,80 @@
+// RUN: %check_clang_tidy %s google-objc-avoid-nsobject-new %t
+
+ at interface NSObject
++ (instancetype)new;
++ (instancetype)alloc;
+- (instancetype)init;
+ at end
+
+ at interface NSProxy // Root class with no -init method.
+ at end
+
+// NSDate provides a specific factory method.
+ at interface NSDate : NSObject
++ (instancetype)date;
+ at end
+
+// For testing behavior with Objective-C Generics.
+ at interface NSMutableDictionary<__covariant KeyType, __covariant ObjectType> : NSObject
+ at end
+
+ at 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<NSString *, NSString *> new];
+ // CHECK-MESSAGES: [[@LINE-1]]:31: warning: do not create objects with +new [google-objc-avoid-nsobject-new]
+ // CHECK-FIXES: [NSMutableDictionary<NSString *, NSString *> alloc] init];
+}
+
+ at interface Foo : NSObject
++ (instancetype)new; // Declare again to suppress warning.
+- (instancetype)initWithInt:(int)anInt;
+- (instancetype)init __attribute__((unavailable));
+
+- (id)new;
+ at end
+
+ at interface Baz : Foo // Check unavailable -init through inheritance.
+ at end
+
+ at interface ProxyFoo : NSProxy
++ (instancetype)new;
+ at 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]
+}
+
+ at interface HasNewOverride : NSObject
+ at end
+
+ at implementation HasNewOverride
++ (instancetype)new {
+ return [[self alloc] init];
+}
+// CHECK-MESSAGES: [[@LINE-3]]:1: warning: classes should not override +new [google-objc-avoid-nsobject-new]
+ at end
More information about the cfe-commits
mailing list