[clang-tidy objc-property-declaration] New option IncludeDefaultAcronyms

Summary: The existing option objc-property-declaration.Acronyms replaces the built-in set of acronyms. While this behavior is OK for clients that don't want the default behavior, many clients may just want to add their own custom acronyms to the default list. This revision introduces a new option, objc-property-declaration.IncludeDefaultAcronyms, which controls whether the acronyms in objc-property-declaration.Acronyms are appended to the default list (the default behavior) or whether they replace. I also updated the documentation. Test Plan: make -j12 check-clang-tools Reviewers: Wizard, hokein, klimek Reviewed By: hokein Subscribers: Eugene.Zelenko, cfe-commits Differential Revision: https://reviews.llvm.org/D42261 llvm-svn: 323130
2018-01-22 15:45:25 +00:00 · 2018-01-22 15:45:25 +00:00 · f94c10d91a
parent 145d63f1ad
commit f94c10d91a
5 changed files with 120 additions and 70 deletions
--- a/clang-tools-extra/clang-tidy/objc/PropertyDeclarationCheck.cpp
+++ b/clang-tools-extra/clang-tidy/objc/PropertyDeclarationCheck.cpp
@ -11,6 +11,7 @@
 #include "../utils/OptionsUtils.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/Support/Regex.h"
 #include <algorithm>
@ -26,61 +27,62 @@ namespace {
 /// https://developer.apple.com/library/content/documentation/Cocoa/Conceptual/CodingGuidelines/Articles/APIAbbreviations.html#//apple_ref/doc/uid/20001285-BCIHCGAE
 ///
 /// Keep this list sorted.
-constexpr char DefaultSpecialAcronyms[] =
-    "ACL;"
-    "API;"
-    "ARGB;"
-    "ASCII;"
-    "BGRA;"
-    "CMYK;"
-    "DNS;"
-    "FPS;"
-    "FTP;"
-    "GIF;"
-    "GPS;"
-    "HD;"
-    "HDR;"
-    "HTML;"
-    "HTTP;"
-    "HTTPS;"
-    "HUD;"
-    "ID;"
-    "JPG;"
-    "JS;"
-    "LAN;"
-    "LZW;"
-    "MDNS;"
-    "MIDI;"
-    "OS;"
-    "PDF;"
-    "PIN;"
-    "PNG;"
-    "POI;"
-    "PSTN;"
-    "PTR;"
-    "QA;"
-    "QOS;"
-    "RGB;"
-    "RGBA;"
-    "RGBX;"
-    "ROM;"
-    "RPC;"
-    "RTF;"
-    "RTL;"
-    "SDK;"
-    "SSO;"
-    "TCP;"
-    "TIFF;"
-    "TTS;"
-    "UI;"
-    "URI;"
-    "URL;"
-    "VC;"
-    "VOIP;"
-    "VPN;"
-    "VR;"
-    "WAN;"
-    "XML";
+constexpr llvm::StringLiteral DefaultSpecialAcronyms[] = {
+    "ACL",
+    "API",
+    "ARGB",
+    "ASCII",
+    "BGRA",
+    "CMYK",
+    "DNS",
+    "FPS",
+    "FTP",
+    "GIF",
+    "GPS",
+    "HD",
+    "HDR",
+    "HTML",
+    "HTTP",
+    "HTTPS",
+    "HUD",
+    "ID",
+    "JPG",
+    "JS",
+    "LAN",
+    "LZW",
+    "MDNS",
+    "MIDI",
+    "OS",
+    "PDF",
+    "PIN",
+    "PNG",
+    "POI",
+    "PSTN",
+    "PTR",
+    "QA",
+    "QOS",
+    "RGB",
+    "RGBA",
+    "RGBX",
+    "ROM",
+    "RPC",
+    "RTF",
+    "RTL",
+    "SDK",
+    "SSO",
+    "TCP",
+    "TIFF",
+    "TTS",
+    "UI",
+    "URI",
+    "URL",
+    "VC",
+    "VOIP",
+    "VPN",
+    "VR",
+    "WAN",
+    "XML",
+};

 /// For now we will only fix 'CamelCase' property to
 /// 'camelCase'. For other cases the users need to
@ -97,14 +99,7 @@ FixItHint generateFixItHint(const ObjCPropertyDecl *Decl) {
  return FixItHint();
 }

-std::string validPropertyNameRegex(const std::vector<std::string> &Acronyms) {
-  std::vector<std::string> EscapedAcronyms;
-  EscapedAcronyms.reserve(Acronyms.size());
-  // In case someone defines a custom prefix which includes a regex
-  // special character, escape all the prefixes.
-  std::transform(Acronyms.begin(), Acronyms.end(),
-                 std::back_inserter(EscapedAcronyms), [](const std::string& s) {
-                   return llvm::Regex::escape(s); });
+std::string validPropertyNameRegex(const std::vector<std::string> &EscapedAcronyms) {
  // Allow any of these names:
  // foo
  // fooBar
@ -123,15 +118,32 @@ std::string validPropertyNameRegex(const std::vector<std::string> &Acronyms) {
 PropertyDeclarationCheck::PropertyDeclarationCheck(StringRef Name,
                                                   ClangTidyContext *Context)
    : ClangTidyCheck(Name, Context),
-      SpecialAcronyms(utils::options::parseStringList(
-          Options.get("Acronyms", DefaultSpecialAcronyms))) {}
+      SpecialAcronyms(
+          utils::options::parseStringList(Options.get("Acronyms", ""))),
+      IncludeDefaultAcronyms(Options.get("IncludeDefaultAcronyms", true)) {}

 void PropertyDeclarationCheck::registerMatchers(MatchFinder *Finder) {
+  std::vector<std::string> EscapedAcronyms;
+  if (IncludeDefaultAcronyms) {
+    EscapedAcronyms.reserve(llvm::array_lengthof(DefaultSpecialAcronyms) +
+                            SpecialAcronyms.size());
+    // No need to regex-escape the default acronyms.
+    EscapedAcronyms.insert(EscapedAcronyms.end(),
+                           std::begin(DefaultSpecialAcronyms),
+                           std::end(DefaultSpecialAcronyms));
+  } else {
+    EscapedAcronyms.reserve(SpecialAcronyms.size());
+  }
+  // In case someone defines a prefix which includes a regex
+  // special character, regex-escape all the user-defined prefixes.
+  std::transform(SpecialAcronyms.begin(), SpecialAcronyms.end(),
+                 std::back_inserter(EscapedAcronyms),
+                 [](const std::string &s) { return llvm::Regex::escape(s); });
  Finder->addMatcher(
      objcPropertyDecl(
          // the property name should be in Lower Camel Case like
          // 'lowerCamelCase'
-          unless(matchesName(validPropertyNameRegex(SpecialAcronyms))))
+          unless(matchesName(validPropertyNameRegex(EscapedAcronyms))))
          .bind("property"),
      this);
 }
@ -149,6 +161,7 @@ void PropertyDeclarationCheck::check(const MatchFinder::MatchResult &Result) {
 void PropertyDeclarationCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
  Options.store(Opts, "Acronyms",
                utils::options::serializeStringList(SpecialAcronyms));
+  Options.store(Opts, "IncludeDefaultAcronyms", IncludeDefaultAcronyms);
 }

 }  // namespace objc
--- a/clang-tools-extra/clang-tidy/objc/PropertyDeclarationCheck.h
+++ b/clang-tools-extra/clang-tidy/objc/PropertyDeclarationCheck.h
@ -34,7 +34,8 @@ public:
  void storeOptions(ClangTidyOptions::OptionMap &Options) override;

 private:
-    const std::vector<std::string> SpecialAcronyms;
+  const std::vector<std::string> SpecialAcronyms;
+  const bool IncludeDefaultAcronyms;
 };

 } // namespace objc
--- a/clang-tools-extra/docs/clang-tidy/checks/objc-property-declaration.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/objc-property-declaration.rst
@ -37,7 +37,25 @@ Options

 .. option:: Acronyms

-   Semicolon-separated list of acronyms that can be used as a prefix
+   Semicolon-separated list of custom acronyms that can be used as a prefix
   or a suffix of property names.

-   If unset, defaults to "ACL;API;ARGB;ASCII;BGRA;CMYK;DNS;FPS;FTP;GIF;GPS;HD;HDR;HTML;HTTP;HTTPS;HUD;ID;JPG;JS;LAN;LZW;MDNS;MIDI;OS;PDF;PIN;PNG;POI;PSTN;PTR;QA;QOS;RGB;RGBA;RGBX;ROM;RPC;RTF;RTL;SDK;SSO;TCP;TIFF;TTS;UI;URI;URL;VC;VOIP;VPN;VR;WAN;XML".
+   By default, appends to the list of default acronyms (
+   ``IncludeDefaultAcronyms`` set to ``1``).
+   If ``IncludeDefaultAcronyms`` is set to ``0``, instead replaces the
+   default list of acronyms.
+
+.. option:: IncludeDefaultAcronyms
+
+   Integer value (defaults to ``1``) to control whether the default
+   acronyms are included in the list of acronyms.
+
+   If set to ``1``, the value in ``Acronyms`` is appended to the
+   default list of acronyms:
+
+   ``ACL;API;ARGB;ASCII;BGRA;CMYK;DNS;FPS;FTP;GIF;GPS;HD;HDR;HTML;HTTP;HTTPS;
+HUD;ID;JPG;JS;LAN;LZW;MDNS;MIDI;OS;PDF;PIN;PNG;POI;PSTN;PTR;QA;QOS;RGB;RGBA;
+RGBX;ROM;RPC;RTF;RTL;SDK;SSO;TCP;TIFF;TTS;UI;URI;URL;VC;VOIP;VPN;VR;WAN;XML``.
+
+   If set to ``0``, the value in ``Acronyms`` replaces the default list
+   of acronyms.
--- a/clang-tools-extra/test/clang-tidy/objc-property-declaration-additional.m
+++ b/clang-tools-extra/test/clang-tidy/objc-property-declaration-additional.m
@ -0,0 +1,15 @@
+// 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]
+@property(assign, nonatomic) int GIFShouldIncludeStandardAcronym;
+@end
--- a/clang-tools-extra/test/clang-tidy/objc-property-declaration-custom.m
+++ b/clang-tools-extra/test/clang-tidy/objc-property-declaration-custom.m
@ -1,6 +1,7 @@
 // RUN: %check_clang_tidy %s objc-property-declaration %t \
 // RUN: -config='{CheckOptions: \
-// RUN:  [{key: objc-property-declaration.Acronyms, value: "ABC;TGIF"}]}' \
+// RUN:  [{key: objc-property-declaration.Acronyms, value: "ABC;TGIF"}, \
+// RUN:   {key: objc-property-declaration.IncludeDefaultAcronyms, value: 0}]}' \
 // RUN: --
@class NSString;

@ -11,4 +12,6 @@
@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]
+@property(assign, nonatomic) int GIFIgnoreStandardAcronym;
+// CHECK-MESSAGES: :[[@LINE-1]]:34: warning: property name 'GIFIgnoreStandardAcronym' should use lowerCamelCase style, according to the Apple Coding Guidelines [objc-property-declaration]
@end