[PATCH] D42261: [clang-tidy objc-property-declaration] New option AdditionalAcronyms

2018-01-19 Thread Ben Hamilton via Phabricator via cfe-commits 
benhamilton updated this revision to Diff 130657.
benhamilton marked 2 inline comments as done.
benhamilton added a comment.

- Switch to IncludeDefaultAcronyms option (defaults to 1).
- Use array for default acronyms, since we no longer need to parse it.
- Don't regex-escape default acronyms, since we control them.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D42261

Files:
  clang-tidy/objc/PropertyDeclarationCheck.cpp
  clang-tidy/objc/PropertyDeclarationCheck.h
  docs/clang-tidy/checks/objc-property-declaration.rst
  test/clang-tidy/objc-property-declaration-additional.m
  test/clang-tidy/objc-property-declaration-custom.m

Index: test/clang-tidy/objc-property-declaration-custom.m
===
--- test/clang-tidy/objc-property-declaration-custom.m
+++ 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
Index: test/clang-tidy/objc-property-declaration-additional.m
===
--- test/clang-tidy/objc-property-declaration-additional.m
+++ test/clang-tidy/objc-property-declaration-additional.m
@@ -11,4 +11,5 @@
 @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
Index: docs/clang-tidy/checks/objc-property-declaration.rst
===
--- docs/clang-tidy/checks/objc-property-declaration.rst
+++ docs/clang-tidy/checks/objc-property-declaration.rst
@@ -37,7 +37,25 @@
 
 .. 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.
Index: clang-tidy/objc/PropertyDeclarationCheck.h
===
--- clang-tidy/objc/PropertyDeclarationCheck.h
+++ clang-tidy/objc/PropertyDeclarationCheck.h
@@ -34,7 +34,8 @@
   void storeOptions(ClangTidyOptions::OptionMap ) override;
 
 private:
-const std::vector SpecialAcronyms;
+  const std::vector SpecialAcronyms;
+  const bool IncludeDefaultAcronyms;
 };
 
 } // namespace objc
Index: clang-tidy/objc/PropertyDeclarationCheck.cpp
===
--- clang-tidy/objc/PropertyDeclarationCheck.cpp
+++ 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 
@@ -26,61 +27,62 @@
 ///

[PATCH] D42261: [clang-tidy objc-property-declaration] New option AdditionalAcronyms

2018-01-19 Thread Haojian Wu via Phabricator via cfe-commits 
hokein added inline comments.



Comment at: clang-tidy/objc/PropertyDeclarationCheck.h:38
 const std::vector SpecialAcronyms;
+const std::vector AdditionalAcronyms;
 };

nit: code indent



Comment at: docs/clang-tidy/checks/objc-property-declaration.rst:47
+
+.. option:: AdditionalAcronyms
+

It seems to me the `Acronyms` and `AdditionalAcronyms` play most same role 
here. 

If we want to keep the long default list, how about using a bool flag 
`IncludeDefaultList` + the existing `Acronyms` option?

* if `IncludeDefaultList` is on, the acronyms will be "default" + "Acronyms".
* if `IncludeDefaultList` is off, the acronyms will be only "Acronyms".


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D42261



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D42261: [clang-tidy objc-property-declaration] New option AdditionalAcronyms

2018-01-18 Thread Eugene Zelenko via Phabricator via cfe-commits 
Eugene.Zelenko added inline comments.



Comment at: docs/clang-tidy/checks/objc-property-declaration.rst:45
+
+   If set, replaces the default list. (If you want to append to the default 
list, set AdditionalAcronyms instead.)
+

Please limit string length to 80 symbols.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D42261



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D42261: [clang-tidy objc-property-declaration] New option AdditionalAcronyms

2018-01-18 Thread Ben Hamilton via Phabricator via cfe-commits 
benhamilton created this revision.
benhamilton added reviewers: Wizard, hokein, klimek.
Herald added a subscriber: cfe-commits.

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.AdditionalAcronyms, which appends custom
acronyms to the default list (instead of replacing the default list).

I also updated the documentation.

Test Plan: make -j12 check-clang-tools


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D42261

Files:
  clang-tidy/objc/PropertyDeclarationCheck.cpp
  clang-tidy/objc/PropertyDeclarationCheck.h
  docs/clang-tidy/checks/objc-property-declaration.rst
  test/clang-tidy/objc-property-declaration-additional.m
  test/clang-tidy/objc-property-declaration-custom.m


Index: test/clang-tidy/objc-property-declaration-custom.m
===
--- test/clang-tidy/objc-property-declaration-custom.m
+++ test/clang-tidy/objc-property-declaration-custom.m
@@ -11,4 +11,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
Index: test/clang-tidy/objc-property-declaration-additional.m
===
--- test/clang-tidy/objc-property-declaration-additional.m
+++ test/clang-tidy/objc-property-declaration-additional.m
@@ -1,6 +1,6 @@
 // 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.AdditionalAcronyms, value: 
"ABC;TGIF"}]}' \
 // RUN: --
 @class NSString;
 
@@ -11,4 +11,5 @@
 @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
Index: docs/clang-tidy/checks/objc-property-declaration.rst
===
--- docs/clang-tidy/checks/objc-property-declaration.rst
+++ docs/clang-tidy/checks/objc-property-declaration.rst
@@ -41,3 +41,12 @@
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".
+
+   If set, replaces the default list. (If you want to append to the default 
list, set AdditionalAcronyms instead.)
+
+.. option:: AdditionalAcronyms
+
+   Semicolon-separated list of additional acronyms that can be used as a prefix
+   or a suffix of property names.
+
+   Appends to the list in Acronyms.
Index: clang-tidy/objc/PropertyDeclarationCheck.h
===
--- clang-tidy/objc/PropertyDeclarationCheck.h
+++ clang-tidy/objc/PropertyDeclarationCheck.h
@@ -35,6 +35,7 @@
 
 private:
 const std::vector SpecialAcronyms;
+const std::vector AdditionalAcronyms;
 };
 
 } // namespace objc
Index: clang-tidy/objc/PropertyDeclarationCheck.cpp
===
--- clang-tidy/objc/PropertyDeclarationCheck.cpp
+++ clang-tidy/objc/PropertyDeclarationCheck.cpp
@@ -124,14 +124,22 @@
ClangTidyContext *Context)
 : ClangTidyCheck(Name, Context),
   SpecialAcronyms(utils::options::parseStringList(
-  Options.get("Acronyms", DefaultSpecialAcronyms))) {}
+  Options.get("Acronyms", DefaultSpecialAcronyms))),
+  AdditionalAcronyms(utils::options::parseStringList(
+  Options.get("AdditionalAcronyms", ""))) {}
 
 void PropertyDeclarationCheck::registerMatchers(MatchFinder *Finder) {
+  std::vector Acronyms;
+  Acronyms.reserve(SpecialAcronyms.size() + AdditionalAcronyms.size());
+  Acronyms.insert(Acronyms.end(), SpecialAcronyms.begin(),
+  SpecialAcronyms.end());
+  Acronyms.insert(Acronyms.end(), AdditionalAcronyms.begin(),
+  AdditionalAcronyms.end());
   Finder->addMatcher(