[clang-tidy ObjC] [3/3] New check objc-forbidden-subclassing

Summary:
This is part 3 of 3 of a series of changes to improve Objective-C
linting in clang-tidy.

This adds a new clang-tidy check `objc-forbidden-subclassing` which
ensures clients do not create subclasses of Objective-C classes which
are not designed to be subclassed.

(Note that for code under your control, you should use
__attribute__((objc_subclassing_restricted)) instead -- this
is intended for third-party APIs which cannot be modified.)

By default, the following classes (which are publicly documented
as not supporting subclassing) are forbidden from subclassing:

ABNewPersonViewController
ABPeoplePickerNavigationController
ABPersonViewController
ABUnknownPersonViewController
NSHashTable
NSMapTable
NSPointerArray
NSPointerFunctions
NSTimer
UIActionSheet
UIAlertView
UIImagePickerController
UITextInputMode
UIWebView

Clients can set a CheckOption
`objc-forbidden-subclassing.ClassNames` to a semicolon-separated
list of class names, which overrides this list.

Test Plan: `ninja check-clang-tools`

Patch by Ben Hamilton!

Reviewers: hokein, alexfh

Reviewed By: hokein

Subscribers: saidinwot, Wizard, srhines, mgorny, xazax.hun

Differential Revision: https://reviews.llvm.org/D39142

llvm-svn: 316744
This commit is contained in:
Haojian Wu 2017-10-27 07:41:36 +00:00
parent 363afa3b25
commit e010406e28
10 changed files with 291 additions and 6 deletions

View File

@ -1,6 +1,7 @@
set(LLVM_LINK_COMPONENTS support)
add_clang_library(clangTidyObjCModule
ForbiddenSubclassingCheck.cpp
ObjCTidyModule.cpp
LINK_LIBS

View File

@ -0,0 +1,118 @@
//===--- ForbiddenSubclassingCheck.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 "ForbiddenSubclassingCheck.h"
#include "clang/AST/ASTContext.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
#include "llvm/ADT/Hashing.h"
#include "llvm/ADT/SmallVector.h"
#include "../utils/OptionsUtils.h"
using namespace clang::ast_matchers;
namespace clang {
namespace tidy {
namespace objc {
namespace {
constexpr char DefaultForbiddenSuperClassNames[] =
"ABNewPersonViewController;"
"ABPeoplePickerNavigationController;"
"ABPersonViewController;"
"ABUnknownPersonViewController;"
"NSHashTable;"
"NSMapTable;"
"NSPointerArray;"
"NSPointerFunctions;"
"NSTimer;"
"UIActionSheet;"
"UIAlertView;"
"UIImagePickerController;"
"UITextInputMode;"
"UIWebView";
/// \brief Matches Objective-C classes that directly or indirectly
/// have a superclass matching \c Base.
///
/// Note that a class is not considered to be a subclass of itself.
///
/// Example matches Y, Z
/// (matcher = objcInterfaceDecl(hasName("X")))
/// \code
/// @interface X
/// @end
/// @interface Y : X // directly derived
/// @end
/// @interface Z : Y // indirectly derived
/// @end
/// \endcode
AST_MATCHER_P(ObjCInterfaceDecl, isSubclassOf,
ast_matchers::internal::Matcher<ObjCInterfaceDecl>, Base) {
for (const auto *SuperClass = Node.getSuperClass();
SuperClass != nullptr;
SuperClass = SuperClass->getSuperClass()) {
if (Base.matches(*SuperClass, Finder, Builder)) {
return true;
}
}
return false;
}
} // namespace
ForbiddenSubclassingCheck::ForbiddenSubclassingCheck(
StringRef Name,
ClangTidyContext *Context)
: ClangTidyCheck(Name, Context),
ForbiddenSuperClassNames(
utils::options::parseStringList(
Options.get("ClassNames", DefaultForbiddenSuperClassNames))) {
}
void ForbiddenSubclassingCheck::registerMatchers(MatchFinder *Finder) {
Finder->addMatcher(
objcInterfaceDecl(
isSubclassOf(
objcInterfaceDecl(
hasAnyName(
std::vector<StringRef>(
ForbiddenSuperClassNames.begin(),
ForbiddenSuperClassNames.end())))
.bind("superclass")))
.bind("subclass"),
this);
}
void ForbiddenSubclassingCheck::check(
const MatchFinder::MatchResult &Result) {
const auto *SubClass = Result.Nodes.getNodeAs<ObjCInterfaceDecl>(
"subclass");
assert(SubClass != nullptr);
const auto *SuperClass = Result.Nodes.getNodeAs<ObjCInterfaceDecl>(
"superclass");
assert(SuperClass != nullptr);
diag(SubClass->getLocation(),
"Objective-C interface %0 subclasses %1, which is not "
"intended to be subclassed")
<< SubClass
<< SuperClass;
}
void ForbiddenSubclassingCheck::storeOptions(
ClangTidyOptions::OptionMap &Opts) {
Options.store(
Opts,
"ForbiddenSuperClassNames",
utils::options::serializeStringList(ForbiddenSuperClassNames));
}
} // namespace objc
} // namespace tidy
} // namespace clang

View File

@ -0,0 +1,42 @@
//===--- ForbiddenSubclassingCheck.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_FORBIDDEN_SUBCLASSING_CHECK_H
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_OBJC_FORBIDDEN_SUBCLASSING_CHECK_H
#include "../ClangTidy.h"
#include "llvm/ADT/StringRef.h"
#include <string>
#include <vector>
namespace clang {
namespace tidy {
namespace objc {
/// Finds Objective-C classes which have a superclass which is
/// documented to not support subclassing.
///
/// For the user-facing documentation see:
/// http://clang.llvm.org/extra/clang-tidy/checks/objc-forbidden-subclassing.html
class ForbiddenSubclassingCheck : public ClangTidyCheck {
public:
ForbiddenSubclassingCheck(StringRef Name, ClangTidyContext *Context);
void registerMatchers(ast_matchers::MatchFinder *Finder) override;
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
void storeOptions(ClangTidyOptions::OptionMap &Options) override;
private:
const std::vector<std::string> ForbiddenSuperClassNames;
};
} // namespace objc
} // namespace tidy
} // namespace clang
#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_OBJC_FORBIDDEN_SUBCLASSING_CHECK_H

View File

@ -10,6 +10,7 @@
#include "../ClangTidy.h"
#include "../ClangTidyModule.h"
#include "../ClangTidyModuleRegistry.h"
#include "ForbiddenSubclassingCheck.h"
using namespace clang::ast_matchers;
@ -20,7 +21,8 @@ namespace objc {
class ObjCModule : public ClangTidyModule {
public:
void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
// TODO(D39142): Add checks here.
CheckFactories.registerCheck<ForbiddenSubclassingCheck>(
"objc-forbidden-subclassing");
}
};

View File

@ -59,6 +59,12 @@ Improvements to clang-tidy
- New module `objc` for Objective-C style checks.
- New `objc-forbidden-subclassing
<http://clang.llvm.org/extra/clang-tidy/checks/objc-forbidden-subclassing.html>`_ check
Ensures Objective-C classes do not subclass any classes which are
not intended to be subclassed.
- Renamed checks to use correct term "implicit conversion" instead of "implicit
cast" and modified messages and option names accordingly:

View File

@ -7,9 +7,9 @@ Clang-Tidy Checks
android-cloexec-accept
android-cloexec-accept4
android-cloexec-creat
android-cloexec-dup
android-cloexec-epoll-create
android-cloexec-epoll-create1
android-cloexec-dup
android-cloexec-fopen
android-cloexec-inotify-init
android-cloexec-inotify-init1
@ -38,7 +38,7 @@ Clang-Tidy Checks
cert-msc30-c (redirects to cert-msc50-cpp) <cert-msc30-c>
cert-msc50-cpp
cert-oop11-cpp (redirects to misc-move-constructor-init) <cert-oop11-cpp>
cppcoreguidelines-c-copy-assignment-signature
cppcoreguidelines-c-copy-assignment-signature (redirects to misc-unconventional-assign-operator) <cppcoreguidelines-c-copy-assignment-signature>
cppcoreguidelines-interfaces-global-init
cppcoreguidelines-no-malloc
cppcoreguidelines-owning-memory
@ -76,8 +76,8 @@ Clang-Tidy Checks
hicpp-explicit-conversions (redirects to google-explicit-constructor) <hicpp-explicit-conversions>
hicpp-function-size (redirects to readability-function-size) <hicpp-function-size>
hicpp-invalid-access-moved (redirects to misc-use-after-move) <hicpp-invalid-access-moved>
hicpp-move-const-arg (redirects to misc-move-const-arg) <hicpp-move-const-arg>
hicpp-member-init (redirects to cppcoreguidelines-pro-type-member-init) <hicpp-member-init>
hicpp-move-const-arg (redirects to misc-move-const-arg) <hicpp-move-const-arg>
hicpp-named-parameter (redirects to readability-named-parameter) <hicpp-named-parameter>
hicpp-new-delete-operators (redirects to misc-new-delete-overloads) <hicpp-new-delete-operators>
hicpp-no-array-decay (redirects to cppcoreguidelines-pro-bounds-array-to-pointer-decay) <hicpp-no-array-decay>
@ -95,7 +95,7 @@ Clang-Tidy Checks
hicpp-use-noexcept (redirects to modernize-use-noexcept) <hicpp-use-noexcept>
hicpp-use-nullptr (redirects to modernize-use-nullptr) <hicpp-use-nullptr>
hicpp-use-override (redirects to modernize-use-override) <hicpp-use-override>
hicpp-vararg (redirects to cppcoreguidelines-pro-type-varg) <hicpp-vararg>
hicpp-vararg (redirects to cppcoreguidelines-pro-type-vararg) <hicpp-vararg>
llvm-header-guard
llvm-include-order
llvm-namespace-comment
@ -180,6 +180,7 @@ Clang-Tidy Checks
performance-type-promotion-in-math-fn
performance-unnecessary-copy-initialization
performance-unnecessary-value-param
objc-forbidden-subclassing
readability-avoid-const-params-in-decls
readability-braces-around-statements
readability-container-size-empty

View File

@ -0,0 +1,27 @@
.. title:: clang-tidy - objc-forbidden-subclassing
objc-forbidden-subclassing
==================================
Finds Objective-C classes which are subclasses of classes which are
not designed to be subclassed.
By default, includes a list of Objective-C classes which
are publicly documented as not supporting subclassing.
.. note::
Instead of using this check, for code under your control, you should add
``__attribute__((objc_subclassing_restricted))`` before your ``@interface`` declarations
to ensure the compiler prevents others from subclassing your Objective-C classes.
See https://clang.llvm.org/docs/AttributeReference.html#objc-subclassing-restricted
Options
-------
.. option:: ForbiddenSuperClassNames
Semicolon-separated list of names of Objective-C classes which
do not support subclassing.
Defaults to ``ABNewPersonViewController;ABPeoplePickerNavigationController;ABPersonViewController;ABUnknownPersonViewController;NSHashTable;NSMapTable;NSPointerArray;NSPointerFunctions;NSTimer;UIActionSheet;UIAlertView;UIImagePickerController;UITextInputMode;UIWebView``.

View File

@ -0,0 +1,39 @@
// RUN: %check_clang_tidy %s objc-forbidden-subclassing %t \
// RUN: -config='{CheckOptions: \
// RUN: [{key: objc-forbidden-subclassing.ClassNames, value: "Foo;Quux"}]}' \
// RUN: --
@interface UIImagePickerController
@end
// Make sure custom config options replace (not add to) the default list.
@interface Waldo : UIImagePickerController
// CHECK-MESSAGES-NOT: :[[@LINE-1]]:12: warning: Objective-C interface 'Waldo' subclasses 'UIImagePickerController', which is not intended to be subclassed [objc-forbidden-subclassing]
@end
@interface Foo
@end
@interface Bar : Foo
// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: Objective-C interface 'Bar' subclasses 'Foo', which is not intended to be subclassed [objc-forbidden-subclassing]
@end
// Check subclasses of subclasses.
@interface Baz : Bar
// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: Objective-C interface 'Baz' subclasses 'Foo', which is not intended to be subclassed [objc-forbidden-subclassing]
@end
@interface Quux
@end
// Check that more than one forbidden superclass can be specified.
@interface Xyzzy : Quux
// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: Objective-C interface 'Xyzzy' subclasses 'Quux', which is not intended to be subclassed [objc-forbidden-subclassing]
@end
@interface Plugh
@end
@interface Corge : Plugh
// CHECK-MESSAGES-NOT: :[[@LINE-1]]:12: warning: Objective-C interface 'Corge' subclasses 'Plugh', which is not intended to be subclassed [objc-forbidden-subclassing]
@end

View File

@ -0,0 +1,21 @@
// RUN: %check_clang_tidy %s objc-forbidden-subclassing %t
@interface UIImagePickerController
@end
@interface Foo : UIImagePickerController
// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: Objective-C interface 'Foo' subclasses 'UIImagePickerController', which is not intended to be subclassed [objc-forbidden-subclassing]
@end
// Check subclasses of subclasses.
@interface Bar : Foo
// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: Objective-C interface 'Bar' subclasses 'UIImagePickerController', which is not intended to be subclassed [objc-forbidden-subclassing]
@end
@interface Baz
@end
// Make sure innocent subclasses aren't caught by the check.
@interface Blech : Baz
// CHECK-MESSAGES-NOT: :[[@LINE-1]]:12: warning: Objective-C interface 'Blech' subclasses 'Baz', which is not intended to be subclassed [objc-forbidden-subclassing]
@end

View File

@ -9,12 +9,40 @@
#include "ClangTidyTest.h"
#include "gtest/gtest.h"
#include "objc/ForbiddenSubclassingCheck.h"
using namespace clang::tidy::objc;
namespace clang {
namespace tidy {
namespace test {
// TODO(D39142) Add unit tests for the ObjC module here once a check lands.
TEST(ObjCForbiddenSubclassing, AllowedSubclass) {
std::vector<ClangTidyError> Errors;
runCheckOnCode<ForbiddenSubclassingCheck>(
"@interface Foo\n"
"@end\n"
"@interface Bar : Foo\n"
"@end\n",
&Errors,
"input.m");
EXPECT_EQ(0ul, Errors.size());
}
TEST(ObjCForbiddenSubclassing, ForbiddenSubclass) {
std::vector<ClangTidyError> Errors;
runCheckOnCode<ForbiddenSubclassingCheck>(
"@interface UIImagePickerController\n"
"@end\n"
"@interface Foo : UIImagePickerController\n"
"@end\n",
&Errors,
"input.m");
EXPECT_EQ(1ul, Errors.size());
EXPECT_EQ(
"Objective-C interface 'Foo' subclasses 'UIImagePickerController', which is not intended to be subclassed",
Errors[0].Message.Message);
}
} // namespace test
} // namespace tidy