From 52161a5abd45461b8b6984e1a639f29f85ee12cc Mon Sep 17 00:00:00 2001 From: Ben Hamilton Date: Mon, 13 Nov 2017 23:54:31 +0000 Subject: [PATCH] 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-tidy/objc/CMakeLists.txt | 1 + .../clang-tidy/objc/ObjCTidyModule.cpp | 3 + .../objc/PropertyDeclarationCheck.cpp | 115 ++++++++++++++++++ .../objc/PropertyDeclarationCheck.h | 44 +++++++ clang-tools-extra/docs/ReleaseNotes.rst | 7 ++ ...oogle-objc-global-variable-declaration.rst | 7 +- .../docs/clang-tidy/checks/list.rst | 3 +- .../checks/objc-property-declaration.rst | 43 +++++++ .../objc-property-declaration-custom.m | 14 +++ .../clang-tidy/objc-property-declaration.m | 12 ++ 10 files changed, 247 insertions(+), 2 deletions(-) create mode 100644 clang-tools-extra/clang-tidy/objc/PropertyDeclarationCheck.cpp create mode 100644 clang-tools-extra/clang-tidy/objc/PropertyDeclarationCheck.h create mode 100644 clang-tools-extra/docs/clang-tidy/checks/objc-property-declaration.rst create mode 100644 clang-tools-extra/test/clang-tidy/objc-property-declaration-custom.m create mode 100644 clang-tools-extra/test/clang-tidy/objc-property-declaration.m diff --git a/clang-tools-extra/clang-tidy/objc/CMakeLists.txt b/clang-tools-extra/clang-tidy/objc/CMakeLists.txt index d445f6e1b1c1..daec7a1d6483 100644 --- a/clang-tools-extra/clang-tidy/objc/CMakeLists.txt +++ b/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 diff --git a/clang-tools-extra/clang-tidy/objc/ObjCTidyModule.cpp b/clang-tools-extra/clang-tidy/objc/ObjCTidyModule.cpp index 515407399701..6fe12c3a53f5 100644 --- a/clang-tools-extra/clang-tidy/objc/ObjCTidyModule.cpp +++ b/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 @@ public: void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override { CheckFactories.registerCheck( "objc-forbidden-subclassing"); + CheckFactories.registerCheck( + "objc-property-declaration"); } }; diff --git a/clang-tools-extra/clang-tidy/objc/PropertyDeclarationCheck.cpp b/clang-tools-extra/clang-tidy/objc/PropertyDeclarationCheck.cpp new file mode 100644 index 000000000000..e47b2411b05c --- /dev/null +++ b/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 + +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 &Prefixes) { + std::vector 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("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 diff --git a/clang-tools-extra/clang-tidy/objc/PropertyDeclarationCheck.h b/clang-tools-extra/clang-tidy/objc/PropertyDeclarationCheck.h new file mode 100644 index 000000000000..ab65f0609d2b --- /dev/null +++ b/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 +#include + +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 SpecialAcronyms; +}; + +} // namespace objc +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_OBJC_PROPERTY_DECLARATION_H diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index eed180107517..1d70bffdab31 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -57,6 +57,13 @@ The improvements are... Improvements to clang-tidy -------------------------- +- New `objc-property-declaration + `_ 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 `_ check diff --git a/clang-tools-extra/docs/clang-tidy/checks/google-objc-global-variable-declaration.rst b/clang-tools-extra/docs/clang-tidy/checks/google-objc-global-variable-declaration.rst index ae2b1ee37951..e850cc4133cd 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/google-objc-global-variable-declaration.rst +++ b/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 diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index 1b74c00752ab..1ad7166dab80 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/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 diff --git a/clang-tools-extra/docs/clang-tidy/checks/objc-property-declaration.rst b/clang-tools-extra/docs/clang-tidy/checks/objc-property-declaration.rst new file mode 100644 index 000000000000..c3721243a930 --- /dev/null +++ b/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``. diff --git a/clang-tools-extra/test/clang-tidy/objc-property-declaration-custom.m b/clang-tools-extra/test/clang-tidy/objc-property-declaration-custom.m new file mode 100644 index 000000000000..641834bb09e4 --- /dev/null +++ b/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 diff --git a/clang-tools-extra/test/clang-tidy/objc-property-declaration.m b/clang-tools-extra/test/clang-tidy/objc-property-declaration.m new file mode 100644 index 000000000000..a8a31e4695d7 --- /dev/null +++ b/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