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

[benhamilton]

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

[benhamilton]

docs/clang-tidy/checks/list.rst

1–11

FYI, I didn't actually change the order here. These were automatically alphabetized when I ran add_new_check.py.

[alexfh]

1. Should we make a separate module for ObjectiveC checks?
2. Is this check actually Google-style-specific?

[hokein]

clang-tidy/google/ObjcForbiddenSubclassingCheck.cpp

24 ↗

(On Diff #119703)

static is redundant as it is in anonymous namespace. How about using constexpr char DefaultForbiddenObjcClassNames[] = ...?

Since this is a long list, using string literal (per class a line) would improve the code readability, I think.

34 ↗

(On Diff #119703)

We can use the utility function utils::options::parseStringList to parse the option list, which would simplify the code.

47 ↗

(On Diff #119703)

nit:: trailing // namespace

clang-tidy/google/ObjcForbiddenSubclassingCheck.h

11 ↗

(On Diff #119703)

The APPLE is not intended?

28 ↗

(On Diff #119703)

Maybe put it into the ObjcForbiddenSubclassingCheck class?

39 ↗

(On Diff #119703)

We also need to overwrite the storeOptions method if the check has its own options.

docs/clang-tidy/checks/google-objc-forbidden-subclassing.rst

9 ↗

(On Diff #119703)

I think the word subclassing should be classes?

10 ↗

(On Diff #119703)

Are they documented in Apple SDK doc? It is worth providing a link, for the users who are interested to know.

14 ↗

(On Diff #119703)

Maybe add more background or context words for this note paragraph.

It took me a while to understand how it is related to the check. If I understand correctly, this means instead of using this check, for the code under your control, you should use __attribute__(...) to prevent your object-c classes which cannot be subclassed?

[hokein]

In D39142#902588, @alexfh wrote:

1. Should we make a separate module for ObjectiveC checks?
2. Is this check actually Google-style-specific?

Copy the email response from @benhamilton (not sure why the response is not in phabricator):

Happy to make a new module, didn't know if that entailed anything special.
This is not google-specific, anyone who writes ObjC should probably use it.

Creating an object-c module seems fine, but I wonder what if we create a google-specific object-c check in the future, which module should we put the check to?

[benhamilton]

• Fix @hokein comments from https://reviews.llvm.org/D39142#903366
• Move from clang-tidy/google -> clang-tidy/objc

[benhamilton]

@hokein: Fixed all those, thanks!

Creating an object-c module seems fine, but I wonder what if we create a google-specific object-c check in the future, which module should we put the check to?

It looks like those would be fine under the google module.

clang-tidy/google/ObjcForbiddenSubclassingCheck.h

11 ↗

(On Diff #119703)

Oops, as you can see I renamed this check a few times.. :)

39 ↗

(On Diff #119703)

Good catch, thank you!

I was surprised that my tests still passed without overriding storeOptions, but now I see that method just improves the UI to explain the options which are valid for the check.

docs/clang-tidy/checks/google-objc-forbidden-subclassing.rst

10 ↗

(On Diff #119703)

The links are in several places. Some are in SDK docs, others are in headers.

I did make a list of URLs to the references in the SDK docs, but it is very long, and Apple tends to break SDK doc URLs every year when they release a new SDK, so I decided it wasn't worth including.

[hokein]

clang-tidy/google/ObjcForbiddenSubclassingCheck.h

39 ↗

(On Diff #119703)

Yeah, the lint test doesn't cover storeOptions. This method is used when dumping the configuration. You can try clang-tidy -dump-config.

clang-tidy/objc/ForbiddenSubclassingCheck.cpp

73

I think we can also do the filtering stuff in the AST matcher. We need to implement a isSubclassOf AST matcher for ObjcInterfaceDecl, the code is roughly like:

AST_MATCHER_P(ObjCInterfaceDecl, isSubClassof,
              ast_matchers::internal::Matcher<ObjCInterfaceDecl>, InnerMatcher) {
  const CXXRecordDecl *Parent = Node.getParent();
 for (const auto*SuperClass = Node->getSuperClass(); SuperClass; SuperClass = SuperClass->getSuperClass) {
   if (InnerMatcher.matches(*SuperClass, Finder, Builder))
      return true;
  }
  return false;
}

And the final matcher would be Finder->addMatcher(objcInterfaceDecl(isSubclassOf(objcInterfaceDecl(hasAnyName(std::vector<StringRef>(ForbiddenSuperclassNames.begin(), ForbiddenSuperclassNames.end())))));.

clang-tidy/objc/ForbiddenSubclassingCheck.h

40

Sorry for missing this in the previous review.

Using a std::vector for the forbidden classes is sufficient -- most of clang-tidy checks are using std::vector for options, and with that, you can get rid of RawStringForbiddenSuperclassNames (just use utils::options::serializeStringList to store the option).

docs/clang-tidy/checks/google-objc-forbidden-subclassing.rst

10 ↗

(On Diff #119703)

ok...:(

[benhamilton]

@hokein:

• Introduce isSubclassOf() AST matcher
• Use std::vector<std::string> to hold forbidden subclass names

[benhamilton]

clang-tidy/objc/ForbiddenSubclassingCheck.cpp

73

OK, done! Easier now with ForbiddenSuperclassNames being a vector.

clang-tidy/objc/ForbiddenSubclassingCheck.h

40

OK. I figured a set made logically more sense since options cannot be duplicated, but the infrastructure doesn't make that trivial, so changed to vector.

[hokein]

LGTM with a few nits.

clang-tidy/objc/ForbiddenSubclassingCheck.cpp

2

nit: This line should align with the last line of this block

96

I'd add an assert(SubClass), the same to the SuperClass.

docs/clang-tidy/checks/objc-forbidden-subclassing.rst

5

nit: remove the extra =.

23

maybe name it ForbiddenSubClassNames? ClassNames is a bit ambiguous.

[benhamilton]

• @hokein: Fix nits from https://reviews.llvm.org/D39142#906232

clang-tidy/objc/ForbiddenSubclassingCheck.cpp

2

Fixed here and in the header.

docs/clang-tidy/checks/objc-forbidden-subclassing.rst

23

I made it ForbiddenSuperClassNames (since it is the superclass which we are listing here).

[benhamilton]

@alexfh: We'd love your thoughts on this diff!

[hokein]

@alexfh is on vacation from today. I think it is fine to submit the patch, committed for you.

clang-tidy/objc/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS support)
 
 add_clang_library(clangTidyObjCModule
+  ForbiddenSubclassingCheck.cpp
   ObjCTidyModule.cpp
 
   LINK_LIBS
   clangAST
   clangASTMatchers
   clangBasic
   clangLex
   clangTidy
   clangTidyReadabilityModule
   clangTidyUtils
   )

clang-tidy/objc/ForbiddenSubclassingCheck.h

+//===--- 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/SmallSet.h"
+#include "llvm/ADT/StringRef.h"
+#include <string>
+
+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;
+
+  /// Container type to hold the names of superclasses which must not be
+  /// subclassed.
+  ///
+  /// Note that this holds std::string and not StringRef, since the
+  /// underlying memory to which a StringRef points might be reclaimed
+  /// after we store it in this container.
+  typedef llvm::SmallSet<std::string, 20> ForbiddenSuperclassNamesSetType;
+
+private:
+  const std::string RawStringForbiddenSuperclassNames;
+  ForbiddenSuperclassNamesSetType ForbiddenSuperclassNames;
+};
+
+} // namespace objc
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_OBJC_FORBIDDEN_SUBCLASSING_CHECK_H

clang-tidy/objc/ForbiddenSubclassingCheck.cpp

+//===--- 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 DefaultForbiddenObjCClassNames[] =
+    "ABNewPersonViewController;"
+    "ABPeoplePickerNavigationController;"
+    "ABPersonViewController;"
+    "ABUnknownPersonViewController;"
+    "NSHashTable;"
+    "NSMapTable;"
+    "NSPointerArray;"
+    "NSPointerFunctions;"
+    "NSTimer;"
+    "UIActionSheet;"
+    "UIAlertView;"
+    "UIImagePickerController;"
+    "UITextInputMode;"
+    "UIWebView";
+
+void parseOptionAndAddForbiddenSuperclassNames(
+    StringRef ForbiddenSuperclassOption,
+    ForbiddenSubclassingCheck::ForbiddenSuperclassNamesSetType
+        &ForbiddenSuperclassNames) {
+  auto ForbiddenSuperclassNamesList =
+      utils::options::parseStringList(ForbiddenSuperclassOption);
+  ForbiddenSuperclassNames.insert(
+      ForbiddenSuperclassNamesList.begin(),
+      ForbiddenSuperclassNamesList.end());
+}
+
+} // namespace
+
+ForbiddenSubclassingCheck::ForbiddenSubclassingCheck(
+    StringRef Name,
+    ClangTidyContext *Context)
+    : ClangTidyCheck(Name, Context),
+      RawStringForbiddenSuperclassNames(
+          Options.get("ClassNames", DefaultForbiddenObjCClassNames)) {
+  parseOptionAndAddForbiddenSuperclassNames(
+      RawStringForbiddenSuperclassNames,
+      ForbiddenSuperclassNames);
+}
+
+void ForbiddenSubclassingCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(objcInterfaceDecl().bind("objc_interface"), this);
+}
+
+void ForbiddenSubclassingCheck::check(
+    const MatchFinder::MatchResult &Result) {
+  const auto *MatchedDecl = Result.Nodes.getNodeAs<ObjCInterfaceDecl>(
+      "objc_interface");
+  for (auto *SuperClass = MatchedDecl->getSuperClass();
+       SuperClass != nullptr;
+       SuperClass = SuperClass->getSuperClass()) {
+    if (ForbiddenSuperclassNames.count(
+            SuperClass->getIdentifier()->getName().str())) {
+      diag(MatchedDecl->getLocation(),
+           "Objective-C interface %0 subclasses %1, which is not "
+           "intended to be subclassed")
+          << MatchedDecl
+          << SuperClass;
+    }
+  }
+}
+
+void ForbiddenSubclassingCheck::storeOptions(
+    ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "ClassNames", RawStringForbiddenSuperclassNames);
+}
+
+} // namespace objc
+} // namespace tidy
+} // namespace clang

clang-tidy/objc/ObjCTidyModule.cpp

 //===--- ObjCTidyModule.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 "../ClangTidy.h"
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
+#include "ForbiddenSubclassingCheck.h"
 
 using namespace clang::ast_matchers;
 
 namespace clang {
 namespace tidy {
 namespace objc {
 
 class ObjCModule : public ClangTidyModule {
 public:
   void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
-    // TODO(D39142): Add checks here.
+    CheckFactories.registerCheck<ForbiddenSubclassingCheck>(
+        "objc-forbidden-subclassing");
   }
 };
 
 // Register the ObjCTidyModule using this statically initialized variable.
 static ClangTidyModuleRegistry::Add<ObjCModule> X(
     "objc-module",
     "Adds Objective-C lint checks.");
 
 } // namespace objc
 
 // This anchor is used to force the linker to link in the generated object file
 // and thus register the ObjCModule.
 volatile int ObjCModuleAnchorSource = 0;
 
 } // namespace tidy
 } // namespace clang

docs/ReleaseNotes.rst

 
 The improvements are...
 
 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:
 
     * **performance-implicit-cast-in-loop** was renamed to
       `performance-implicit-conversion-in-loop
       <http://clang.llvm.org/extra/clang-tidy/checks/performance-implicit-conversion-in-loop.html>`_
     * **readability-implicit-bool-cast** was renamed to
       `readability-implicit-bool-conversion

docs/clang-tidy/checks/list.rst

 .. title:: clang-tidy - Clang-Tidy Checks
 
 Clang-Tidy Checks
 =================
 
 .. toctree::
    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
    android-cloexec-memfd-create
    android-cloexec-open
    android-cloexec-socket
    boost-use-to-string
    bugprone-integer-division
.. toctree::
    cert-err58-cpp
    cert-err60-cpp
    cert-err61-cpp (redirects to misc-throw-by-value-catch-by-reference) <cert-err61-cpp>
    cert-fio38-c (redirects to misc-non-copyable-objects) <cert-fio38-c>
    cert-flp30-c
    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
    cppcoreguidelines-pro-bounds-array-to-pointer-decay
    cppcoreguidelines-pro-bounds-constant-array-index
    cppcoreguidelines-pro-bounds-pointer-arithmetic
    cppcoreguidelines-pro-type-const-cast
    cppcoreguidelines-pro-type-cstyle-cast
.. toctree::
    google-runtime-operator
    google-runtime-references
    hicpp-braces-around-statements (redirects to readability-braces-around-statements) <hicpp-braces-around-statements>
    hicpp-deprecated-headers (redirects to modernize-deprecated-headers) <hicpp-deprecated-headers>
    hicpp-exception-baseclass
    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>
    hicpp-no-assembler
    hicpp-no-malloc (redirects to cppcoreguidelines-no-malloc) <hicpp-no-malloc>
    hicpp-noexcept-move (redirects to misc-noexcept-moveconstructor) <hicpp-noexcept-move>
    hicpp-signed-bitwise
    hicpp-special-member-functions (redirects to cppcoreguidelines-special-member-functions) <hicpp-special-member-functions>
    hicpp-static-assert (redirects to misc-static-assert) <hicpp-static-assert>
    hicpp-undelegated-constructor (redirects to misc-undelegated-constructor) <hicpp-undelegated-constructor>
    hicpp-use-auto (redirects to modernize-use-auto) <hicpp-use-auto>
    hicpp-use-emplace (redirects to modernize-use-emplace) <hicpp-use-emplace>
    hicpp-use-equals-default (redirects to modernize-use-equals-default) <hicpp-use-equals-default>
    hicpp-use-equals-delete (redirects to modernize-use-equals-delete) <hicpp-use-equals-delete>
    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
    llvm-twine-local
    misc-argument-comment
    misc-assert-side-effect
    misc-bool-pointer-implicit-conversion
    misc-dangling-handle
.. toctree::
    performance-faster-string-find
    performance-for-range-copy
    performance-implicit-conversion-in-loop
    performance-inefficient-string-concatenation
    performance-inefficient-vector-operation
    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
    readability-delete-null-pointer
    readability-deleted-default
    readability-else-after-return
    readability-function-size
    readability-identifier-naming

docs/clang-tidy/checks/objc-forbidden-subclassing.rst

+.. 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:: ClassNames
+
+   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``.

test/clang-tidy/objc-forbidden-subclassing-custom.m

+// 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

test/clang-tidy/objc-forbidden-subclassing.m

+// 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

unittests/clang-tidy/ObjCModuleTest.cpp

 #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
 } // namespace clang