add new check for property declaration

Summary:
This check finds property declarations in Objective-C files that do not follow the pattern of property names in Apple's programming guide. The property name should be in the format of Lower Camel Case or with some particular acronyms as prefix.

Example:
@Property(nonatomic, assign) int lowerCamelCase;

@Property(nonatomic, strong) NSString *URLString;

Test plan:  ninja check-clang-tools

Reviewers: benhamilton, hokein

Reviewed By: hokein

Subscribers: cfe-commits, mgorny

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

llvm-svn: 318117

clang-tools-extra/clang-tidy/objc/CMakeLists.txt

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

clang-tools-extra/clang-tidy/objc/ObjCTidyModule.cpp

@@ -11,6 +11,7 @@
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
 #include "ForbiddenSubclassingCheck.h"
+#include "PropertyDeclarationCheck.h"
 
 using namespace clang::ast_matchers;
 
@@ -23,6 +24,8 @@ class ObjCModule : public ClangTidyModule {
   void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
     CheckFactories.registerCheck<ForbiddenSubclassingCheck>(
         "objc-forbidden-subclassing");
+    CheckFactories.registerCheck<PropertyDeclarationCheck>(
+        "objc-property-declaration");
   }
 };
 

clang-tools-extra/clang-tidy/objc/PropertyDeclarationCheck.cpp

@@ -0,0 +1,115 @@
+//===--- PropertyDeclarationCheck.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 "PropertyDeclarationCheck.h"
+#include "../utils/OptionsUtils.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "llvm/ADT/StringExtras.h"
+#include "llvm/Support/Regex.h"
+#include <algorithm>
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace objc {
+
+namespace {
+/// The acronyms are from
+/// https://developer.apple.com/library/content/documentation/Cocoa/Conceptual/CodingGuidelines/Articles/APIAbbreviations.html#//apple_ref/doc/uid/20001285-BCIHCGAE
+constexpr char DefaultSpecialAcronyms[] =
+    "ASCII;"
+    "PDF;"
+    "XML;"
+    "HTML;"
+    "URL;"
+    "RTF;"
+    "HTTP;"
+    "TIFF;"
+    "JPG;"
+    "PNG;"
+    "GIF;"
+    "LZW;"
+    "ROM;"
+    "RGB;"
+    "CMYK;"
+    "MIDI;"
+    "FTP";
+
+/// For now we will only fix 'CamelCase' property to
+/// 'camelCase'. For other cases the users need to
+/// come up with a proper name by their own.
+/// FIXME: provide fix for snake_case to snakeCase
+FixItHint generateFixItHint(const ObjCPropertyDecl *Decl) {
+  if (isupper(Decl->getName()[0])) {
+    auto NewName = Decl->getName().str();
+    NewName[0] = tolower(NewName[0]);
+    return FixItHint::CreateReplacement(
+        CharSourceRange::getTokenRange(SourceRange(Decl->getLocation())),
+        llvm::StringRef(NewName));
+  }
+  return FixItHint();
+}
+
+std::string validPropertyNameRegex(const std::vector<std::string> &Prefixes) {
+  std::vector<std::string> EscapedPrefixes;
+  EscapedPrefixes.reserve(Prefixes.size());
+  // In case someone defines a custom prefix which includes a regex
+  // special character, escape all the prefixes.
+  std::transform(Prefixes.begin(), Prefixes.end(),
+                 std::back_inserter(EscapedPrefixes), [](const std::string& s) {
+                   return llvm::Regex::escape(s); });
+  // Allow any of these names:
+  // foo
+  // fooBar
+  // url
+  // urlString
+  // URL
+  // URLString
+  return std::string("::((") +
+      llvm::join(EscapedPrefixes.begin(), EscapedPrefixes.end(), "|") +
+      ")[A-Z]?)?[a-z]+[a-z0-9]*([A-Z][a-z0-9]+)*$";
+}
+}  // namespace
+
+PropertyDeclarationCheck::PropertyDeclarationCheck(StringRef Name,
+                                                   ClangTidyContext *Context)
+    : ClangTidyCheck(Name, Context),
+      SpecialAcronyms(utils::options::parseStringList(
+          Options.get("Acronyms", DefaultSpecialAcronyms))) {}
+
+void PropertyDeclarationCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(
+      objcPropertyDecl(
+          // the property name should be in Lower Camel Case like
+          // 'lowerCamelCase'
+          unless(matchesName(validPropertyNameRegex(SpecialAcronyms))))
+          .bind("property"),
+      this);
+}
+
+void PropertyDeclarationCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto *MatchedDecl =
+      Result.Nodes.getNodeAs<ObjCPropertyDecl>("property");
+  assert(MatchedDecl->getName().size() > 0);
+  diag(MatchedDecl->getLocation(),
+       "property name '%0' should use lowerCamelCase style, according to "
+       "the Apple Coding Guidelines")
+      << MatchedDecl->getName() << generateFixItHint(MatchedDecl);
+}
+
+void PropertyDeclarationCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "Acronyms",
+                utils::options::serializeStringList(SpecialAcronyms));
+}
+
+}  // namespace objc
+}  // namespace tidy
+}  // namespace clang

clang-tools-extra/clang-tidy/objc/PropertyDeclarationCheck.h

@@ -0,0 +1,44 @@
+//===--- PropertyDeclarationCheck.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_PROPERTY_DECLARATION_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_OBJC_PROPERTY_DECLARATION_H
+
+#include "../ClangTidy.h"
+#include <string>
+#include <vector>
+
+namespace clang {
+namespace tidy {
+namespace objc {
+
+/// Finds Objective-C property declarations which
+/// are not in Lower Camel Case.
+///
+/// The format of property should look like:
+/// @property(nonatomic) NSString *lowerCamelCase;
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/objc-property-declaration.html
+class PropertyDeclarationCheck : public ClangTidyCheck {
+public:
+  PropertyDeclarationCheck(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> SpecialAcronyms;
+};
+
+} // namespace objc
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_OBJC_PROPERTY_DECLARATION_H

clang-tools-extra/docs/ReleaseNotes.rst

@@ -57,6 +57,13 @@ The improvements are...
 Improvements to clang-tidy
 --------------------------
 
+- New `objc-property-declaration
+  <http://clang.llvm.org/extra/clang-tidy/checks/objc-property-declaration.html>`_ check
+
+  Add new check for Objective-C code to ensure property
+  names follow the naming convention of Apple's programming
+  guide.
+
 - New `google-objc-global-variable-declaration
   <http://clang.llvm.org/extra/clang-tidy/checks/google-global-variable-declaration.html>`_ check
 

clang-tools-extra/docs/clang-tidy/checks/google-objc-global-variable-declaration.rst

@@ -3,7 +3,7 @@
 google-objc-global-variable-declaration
 =======================================
 
-Finds global variable declarations in Objective-C files that are not follow the pattern
+Finds global variable declarations in Objective-C files that do not follow the pattern
 of variable names in Google's Objective-C Style Guide.
 
 The corresponding style guide rule:
@@ -16,26 +16,31 @@ if it can be inferred from the original name.
 For code:
 
 .. code-block:: objc
+
   static NSString* myString = @"hello";
 
 The fix will be:
 
 .. code-block:: objc
+
   static NSString* gMyString = @"hello";
 
 Another example of constant:
 
 .. code-block:: objc
+
   static NSString* const myConstString = @"hello";
 
 The fix will be:
 
 .. code-block:: objc
+
   static NSString* const kMyConstString = @"hello";
 
 However for code that prefixed with non-alphabetical characters like:
 
 .. code-block:: objc
+
   static NSString* __anotherString = @"world";
 
 The check will give a warning message but will not be able to suggest a fix. The user

clang-tools-extra/docs/clang-tidy/checks/list.rst

@@ -173,6 +173,8 @@ Clang-Tidy Checks
    modernize-use-using
    mpi-buffer-deref
    mpi-type-mismatch
+   objc-forbidden-subclassing
+   objc-property-declaration
    performance-faster-string-find
    performance-for-range-copy
    performance-implicit-conversion-in-loop
@@ -181,7 +183,6 @@ 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

clang-tools-extra/docs/clang-tidy/checks/objc-property-declaration.rst

@@ -0,0 +1,43 @@
+.. title:: clang-tidy - objc-property-declaration
+
+objc-property-declaration
+=========================
+
+Finds property declarations in Objective-C files that do not follow the pattern
+of property names in Apple's programming guide. The property name should be
+in the format of Lower Camel Case.
+
+For code:
+
+.. code-block:: objc
+
+@property(nonatomic, assign) int LowerCamelCase;
+
+The fix will be:
+
+.. code-block:: objc
+
+@property(nonatomic, assign) int lowerCamelCase;
+
+The check will only fix 'CamelCase' to 'camelCase'. In some other cases we will
+only provide warning messages since the property name could be complicated.
+Users will need to come up with a proper name by their own.
+
+This check also accepts special acronyms as prefix. Such prefix will suppress
+the check of Lower Camel Case according to the guide:
+https://developer.apple.com/library/content/documentation/Cocoa/Conceptual/CodingGuidelines/Articles/NamingBasics.html#//apple_ref/doc/uid/20001281-1002931-BBCFHEAB
+
+For a full list of well-known acronyms:
+https://developer.apple.com/library/content/documentation/Cocoa/Conceptual/CodingGuidelines/Articles/APIAbbreviations.html#//apple_ref/doc/uid/20001285-BCIHCGAE
+
+The corresponding style rule: https://developer.apple.com/library/content/documentation/Cocoa/Conceptual/CodingGuidelines/Articles/NamingIvarsAndTypes.html#//apple_ref/doc/uid/20001284-1001757
+
+Options
+-------
+
+.. option:: Acronyms
+
+   Semicolon-separated list of acronyms that can be used as prefix
+   of property names.
+
+   Defaults to ``ASCII;PDF;XML;HTML;URL;RTF;HTTP;TIFF;JPG;PNG;GIF;LZW;ROM;RGB;CMYK;MIDI;FTP``.

clang-tools-extra/test/clang-tidy/objc-property-declaration-custom.m

@@ -0,0 +1,14 @@
+// RUN: %check_clang_tidy %s objc-property-declaration %t \
+// RUN: -config='{CheckOptions: \
+// RUN:  [{key: objc-property-declaration.Acronyms, value: "ABC;TGIF"}]}' \
+// RUN: --
+@class NSString;
+
+@interface Foo
+@property(assign, nonatomic) int AbcNotRealPrefix;
+// CHECK-MESSAGES: :[[@LINE-1]]:34: warning: property name 'AbcNotRealPrefix' should use lowerCamelCase style, according to the Apple Coding Guidelines [objc-property-declaration]
+// CHECK-FIXES: @property(assign, nonatomic) int abcNotRealPrefix;
+@property(assign, nonatomic) int ABCCustomPrefix;
+@property(strong, nonatomic) NSString *ABC_custom_prefix;
+// CHECK-MESSAGES: :[[@LINE-1]]:40: warning: property name 'ABC_custom_prefix' should use lowerCamelCase style, according to the Apple Coding Guidelines [objc-property-declaration]
+@end

clang-tools-extra/test/clang-tidy/objc-property-declaration.m

@@ -0,0 +1,12 @@
+// RUN: %check_clang_tidy %s objc-property-declaration %t
+@class NSString;
+
+@interface Foo
+@property(assign, nonatomic) int NotCamelCase;
+// CHECK-MESSAGES: :[[@LINE-1]]:34: warning: property name 'NotCamelCase' should use lowerCamelCase style, according to the Apple Coding Guidelines [objc-property-declaration]
+// CHECK-FIXES: @property(assign, nonatomic) int notCamelCase;
+@property(assign, nonatomic) int camelCase;
+@property(strong, nonatomic) NSString *URLString;
+@property(strong, nonatomic) NSString *URL_string;
+// CHECK-MESSAGES: :[[@LINE-1]]:40: warning: property name 'URL_string' should use lowerCamelCase style, according to the Apple Coding Guidelines [objc-property-declaration]
+@end