[PATCH] D46922: [checks/property-decls] Fix comment in clang-tidy/objc/PropertyDeclarationCheck.cpp ✍️

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore created this revision.
Herald added a subscriber: cfe-commits.

The comment incorrectly claims that the listed acronyms are all extracted from 
the linked Apple documentation.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D46922

Files:
  clang-tidy/objc/PropertyDeclarationCheck.cpp


Index: clang-tidy/objc/PropertyDeclarationCheck.cpp
===
--- clang-tidy/objc/PropertyDeclarationCheck.cpp
+++ clang-tidy/objc/PropertyDeclarationCheck.cpp
@@ -34,7 +34,7 @@
   CategoryProperty = 2,
 };
 
-/// The acronyms are from
+/// The acronyms are aggregated from multiple sources including
 /// 
https://developer.apple.com/library/content/documentation/Cocoa/Conceptual/CodingGuidelines/Articles/APIAbbreviations.html#//apple_ref/doc/uid/20001285-BCIHCGAE
 ///
 /// Keep this list sorted.


Index: clang-tidy/objc/PropertyDeclarationCheck.cpp
===
--- clang-tidy/objc/PropertyDeclarationCheck.cpp
+++ clang-tidy/objc/PropertyDeclarationCheck.cpp
@@ -34,7 +34,7 @@
   CategoryProperty = 2,
 };
 
-/// The acronyms are from
+/// The acronyms are aggregated from multiple sources including
 /// https://developer.apple.com/library/content/documentation/Cocoa/Conceptual/CodingGuidelines/Articles/APIAbbreviations.html#//apple_ref/doc/uid/20001285-BCIHCGAE
 ///
 /// Keep this list sorted.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46659: [clang-tidy/google-readability-casting] Disable check for Objective-C++

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore accepted this revision.
stephanemoore added inline comments.



Comment at: clang-tidy/google/AvoidCStyleCastsCheck.cpp:103-104
   // The rest of this check is only relevant to C++.
-  if (!getLangOpts().CPlusPlus)
+  // We also disable it for Objective-C++.
+  if (!getLangOpts().CPlusPlus || getLangOpts().ObjC1 || getLangOpts().ObjC2)
 return;

In the future it would probably be good to allow configuring whether or not 
this is disabled but I think that disabling it for Objective-C++ completely in 
the interim is a positive change.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D46659



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

[PATCH] D47393: [clang-format] Disable AlwaysBreakBeforeMultilineStrings in Google style for Objective-C 📜

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore created this revision.
Herald added subscribers: cfe-commits, klimek.

Repository:
  rC Clang

https://reviews.llvm.org/D47393

Files:
  lib/Format/Format.cpp
  unittests/Format/FormatTestObjC.cpp


Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -1193,6 +1193,22 @@
"}");
 }
 
+TEST_F(FormatTestObjC, AlwaysBreakBeforeMultilineStrings) {
+  FormatStyle Style = getGoogleStyle();
+  verifyFormat(" = @\"\"\n"
+   "   @\"\";",
+   Style);
+  verifyFormat("(@\"\"\n"
+   " @\"\");",
+   Style);
+  verifyFormat("(qqq, @\"\"\n"
+   "  @\"\");",
+   Style);
+  verifyFormat("NSString *const kString = @\"\"\n"
+   "  @\"\");",
+   Style);
+}
+
 } // end namespace
 } // end namespace format
 } // end namespace clang
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -799,6 +799,7 @@
 // has been implemented.
 GoogleStyle.BreakStringLiterals = false;
   } else if (Language == FormatStyle::LK_ObjC) {
+GoogleStyle.AlwaysBreakBeforeMultilineStrings = false;
 GoogleStyle.ColumnLimit = 100;
   }
 


Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -1193,6 +1193,22 @@
"}");
 }
 
+TEST_F(FormatTestObjC, AlwaysBreakBeforeMultilineStrings) {
+  FormatStyle Style = getGoogleStyle();
+  verifyFormat(" = @\"\"\n"
+   "   @\"\";",
+   Style);
+  verifyFormat("(@\"\"\n"
+   " @\"\");",
+   Style);
+  verifyFormat("(qqq, @\"\"\n"
+   "  @\"\");",
+   Style);
+  verifyFormat("NSString *const kString = @\"\"\n"
+   "  @\"\");",
+   Style);
+}
+
 } // end namespace
 } // end namespace format
 } // end namespace clang
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -799,6 +799,7 @@
 // has been implemented.
 GoogleStyle.BreakStringLiterals = false;
   } else if (Language == FormatStyle::LK_ObjC) {
+GoogleStyle.AlwaysBreakBeforeMultilineStrings = false;
 GoogleStyle.ColumnLimit = 100;
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47393: [clang-format] Disable AlwaysBreakBeforeMultilineStrings in Google style for Objective-C 📜

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore updated this revision to Diff 148672.

Repository:
  rC Clang

https://reviews.llvm.org/D47393

Files:
  lib/Format/Format.cpp
  unittests/Format/FormatTestObjC.cpp


Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -1193,6 +1193,18 @@
"}");
 }
 
+TEST_F(FormatTestObjC, AlwaysBreakBeforeMultilineStrings) {
+  Style = getGoogleStyle();
+  verifyFormat(" = @\"\"\n"
+   "   @\"\";");
+  verifyFormat("(@\"\"\n"
+   " @\"\");");
+  verifyFormat("(qqq, @\"\"\n"
+   "  @\"\");");
+  verifyFormat("NSString *const kString = @\"\"\n"
+   "  @\"\");");
+}
+
 } // end namespace
 } // end namespace format
 } // end namespace clang
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -799,6 +799,7 @@
 // has been implemented.
 GoogleStyle.BreakStringLiterals = false;
   } else if (Language == FormatStyle::LK_ObjC) {
+GoogleStyle.AlwaysBreakBeforeMultilineStrings = false;
 GoogleStyle.ColumnLimit = 100;
   }
 


Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -1193,6 +1193,18 @@
"}");
 }
 
+TEST_F(FormatTestObjC, AlwaysBreakBeforeMultilineStrings) {
+  Style = getGoogleStyle();
+  verifyFormat(" = @\"\"\n"
+   "   @\"\";");
+  verifyFormat("(@\"\"\n"
+   " @\"\");");
+  verifyFormat("(qqq, @\"\"\n"
+   "  @\"\");");
+  verifyFormat("NSString *const kString = @\"\"\n"
+   "  @\"\");");
+}
+
 } // end namespace
 } // end namespace format
 } // end namespace clang
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -799,6 +799,7 @@
 // has been implemented.
 GoogleStyle.BreakStringLiterals = false;
   } else if (Language == FormatStyle::LK_ObjC) {
+GoogleStyle.AlwaysBreakBeforeMultilineStrings = false;
 GoogleStyle.ColumnLimit = 100;
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46895: add AR to acronyms of clang-tidy property check

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore added inline comments.



Comment at: clang-tidy/objc/PropertyDeclarationCheck.cpp:37-38
 
 /// The acronyms are from
 /// 
https://developer.apple.com/library/content/documentation/Cocoa/Conceptual/CodingGuidelines/Articles/APIAbbreviations.html#//apple_ref/doc/uid/20001285-BCIHCGAE
 ///

This comment does not seem to be accurate anymore. The acronyms in this list 
are not all from the linked Apple documentation.

Sent out https://reviews.llvm.org/D46922.



Comment at: clang-tidy/objc/PropertyDeclarationCheck.cpp:45
 "API",
+"AR",
 "ARGB",

Are we sure it's sustainable to keep growing this list of acronyms? I suspect 
there may be enough domain-specific acronyms that it might be better to accept 
any sequence of capitalized alphanumeric characters as an acronym.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D46895



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

[PATCH] D47393: [clang-format] Disable AlwaysBreakBeforeMultilineStrings in Google style for Objective-C 📜

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore updated this revision to Diff 148705.

Repository:
  rC Clang

https://reviews.llvm.org/D47393

Files:
  lib/Format/Format.cpp
  unittests/Format/FormatTestObjC.cpp


Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -1193,6 +1193,18 @@
"}");
 }
 
+TEST_F(FormatTestObjC, AlwaysBreakBeforeMultilineStrings) {
+  Style = getGoogleStyle(FormatStyle::LK_ObjC);
+  verifyFormat(" = @\"\"\n"
+   "   @\"\";");
+  verifyFormat("(@\"\"\n"
+   " @\"\");");
+  verifyFormat("(qqq, @\"\"\n"
+   "  @\"\");");
+  verifyFormat("NSString *const kString = @\"\"\n"
+   "  @\"\");");
+}
+
 } // end namespace
 } // end namespace format
 } // end namespace clang
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -799,6 +799,7 @@
 // has been implemented.
 GoogleStyle.BreakStringLiterals = false;
   } else if (Language == FormatStyle::LK_ObjC) {
+GoogleStyle.AlwaysBreakBeforeMultilineStrings = false;
 GoogleStyle.ColumnLimit = 100;
   }
 


Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -1193,6 +1193,18 @@
"}");
 }
 
+TEST_F(FormatTestObjC, AlwaysBreakBeforeMultilineStrings) {
+  Style = getGoogleStyle(FormatStyle::LK_ObjC);
+  verifyFormat(" = @\"\"\n"
+   "   @\"\";");
+  verifyFormat("(@\"\"\n"
+   " @\"\");");
+  verifyFormat("(qqq, @\"\"\n"
+   "  @\"\");");
+  verifyFormat("NSString *const kString = @\"\"\n"
+   "  @\"\");");
+}
+
 } // end namespace
 } // end namespace format
 } // end namespace clang
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -799,6 +799,7 @@
 // has been implemented.
 GoogleStyle.BreakStringLiterals = false;
   } else if (Language == FormatStyle::LK_ObjC) {
+GoogleStyle.AlwaysBreakBeforeMultilineStrings = false;
 GoogleStyle.ColumnLimit = 100;
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51575: [clang-tidy] Implement a clang-tidy check to verify Google Objective-C function naming conventions 📜

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore created this revision.
Herald added subscribers: cfe-commits, xazax.hun, mgorny.

§1 Description

This check finds function names in function definitions in Objective-C files 
that do not follow the naming pattern described in the Google Objective-C Style 
Guide. Function names should be in UpperCamelCase and functions that are not of 
static storage class should have an appropriate prefix as described in the 
Google Objective-C Style Guide. The function `main` is a notable exception.

Example conforming function definitions:

  static bool IsPositive(int i) { return i > 0; }
  static bool ABIsPositive(int i) { return i > 0; }
  bool ABIsNegative(int i) { return i < 0; }

§2 Test Notes

- Verified clang-tidy tests pass successfully.
- Used check_clang_tidy.py to verify expected output of processing 
google-objc-function-naming.m


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51575

Files:
  clang-tidy/google/CMakeLists.txt
  clang-tidy/google/FunctionNamingCheck.cpp
  clang-tidy/google/FunctionNamingCheck.h
  clang-tidy/google/GoogleTidyModule.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/google-objc-function-naming.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/google-objc-function-naming.m
  unittests/clang-tidy/GoogleModuleTest.cpp

Index: unittests/clang-tidy/GoogleModuleTest.cpp
===
--- unittests/clang-tidy/GoogleModuleTest.cpp
+++ unittests/clang-tidy/GoogleModuleTest.cpp
@@ -1,6 +1,7 @@
 #include "ClangTidyTest.h"
 #include "google/ExplicitConstructorCheck.h"
 #include "google/GlobalNamesInHeadersCheck.h"
+#include "google/FunctionNamingCheck.h"
 #include "gtest/gtest.h"
 
 using namespace clang::tidy::google;
@@ -105,6 +106,28 @@
   EXPECT_FALSE(runCheckOnCode("namespace {}", "foo.h"));
 }
 
+TEST(ObjCFunctionNaming, AllowedStaticFunctionName) {
+  std::vector Errors;
+  runCheckOnCode(
+  "static void FooBar(void) {}",
+  &Errors,
+  "input.m");
+  EXPECT_EQ(0ul, Errors.size());
+}
+
+TEST(ObjCFunctionNaming, LowerCamelCaseStaticFunctionName) {
+  std::vector Errors;
+  runCheckOnCode(
+  "static void fooBar(void) {}\n",
+  &Errors,
+  "input.m");
+  EXPECT_EQ(1ul, Errors.size());
+  EXPECT_EQ(
+  "function name 'fooBar' not using function naming conventions described by "
+   "Google Objective-C style guide",
+  Errors[0].Message.Message);
+}
+
 } // namespace test
 } // namespace tidy
 } // namespace clang
Index: test/clang-tidy/google-objc-function-naming.m
===
--- /dev/null
+++ test/clang-tidy/google-objc-function-naming.m
@@ -0,0 +1,43 @@
+// RUN: %check_clang_tidy %s google-objc-function-naming %t
+
+typedef _Bool bool;
+
+static bool ispositive(int a) { return a > 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'ispositive' not using function naming conventions described by Google Objective-C style guide
+
+static bool is_positive(int a) { return a > 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'is_positive' not using function naming conventions described by Google Objective-C style guide
+
+static bool isPositive(int a) { return a > 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'isPositive' not using function naming conventions described by Google Objective-C style guide
+
+static bool Is_Positive(int a) { return a > 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'Is_Positive' not using function naming conventions described by Google Objective-C style guide
+
+static bool IsPositive(int a) { return a > 0; }
+
+static const char *md5(const char *str) { return 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: function name 'md5' not using function naming conventions described by Google Objective-C style guide
+
+static const char *MD5(const char *str) { return 0; }
+
+static const char *URL(void) { return "https://clang.llvm.org/";; }
+
+static const char *DEFURL(void) { return "https://clang.llvm.org/";; }
+
+static const char *DEFFooURL(void) { return "https://clang.llvm.org/";; }
+
+static const char *StringFromNSString(id str) { return ""; }
+
+bool ispalindrome(const char *str);
+
+void ABLog_String(const char *str) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function name 'ABLog_String' not using function naming conventions described by Google Objective-C style guide
+
+void ABLogString(const char *str) {}
+
+const char *ABURL(void) { return "https://clang.llvm.org/";; }
+
+const char *ABFooURL(void) { return "https://clang.llvm.org/";; }
+
+int main(int argc, const char **argv) { return 0; }
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -118,6 +118,7 @@
google-explicit-constructor
google-global-names-in-headers
google-objc-avoid-throwing-exception
+   g

[PATCH] D51575: [clang-tidy] Implement a clang-tidy check to verify Google Objective-C function naming conventions 📜

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore updated this revision to Diff 163638.
stephanemoore added a comment.

Minor fixes:

- Fixed header guard.
- Removed unnecessary imports in header.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51575

Files:
  clang-tidy/google/CMakeLists.txt
  clang-tidy/google/FunctionNamingCheck.cpp
  clang-tidy/google/FunctionNamingCheck.h
  clang-tidy/google/GoogleTidyModule.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/google-objc-function-naming.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/google-objc-function-naming.m
  unittests/clang-tidy/GoogleModuleTest.cpp

Index: unittests/clang-tidy/GoogleModuleTest.cpp
===
--- unittests/clang-tidy/GoogleModuleTest.cpp
+++ unittests/clang-tidy/GoogleModuleTest.cpp
@@ -1,6 +1,7 @@
 #include "ClangTidyTest.h"
 #include "google/ExplicitConstructorCheck.h"
 #include "google/GlobalNamesInHeadersCheck.h"
+#include "google/FunctionNamingCheck.h"
 #include "gtest/gtest.h"
 
 using namespace clang::tidy::google;
@@ -105,6 +106,28 @@
   EXPECT_FALSE(runCheckOnCode("namespace {}", "foo.h"));
 }
 
+TEST(ObjCFunctionNaming, AllowedStaticFunctionName) {
+  std::vector Errors;
+  runCheckOnCode(
+  "static void FooBar(void) {}",
+  &Errors,
+  "input.m");
+  EXPECT_EQ(0ul, Errors.size());
+}
+
+TEST(ObjCFunctionNaming, LowerCamelCaseStaticFunctionName) {
+  std::vector Errors;
+  runCheckOnCode(
+  "static void fooBar(void) {}\n",
+  &Errors,
+  "input.m");
+  EXPECT_EQ(1ul, Errors.size());
+  EXPECT_EQ(
+  "function name 'fooBar' not using function naming conventions described by "
+   "Google Objective-C style guide",
+  Errors[0].Message.Message);
+}
+
 } // namespace test
 } // namespace tidy
 } // namespace clang
Index: test/clang-tidy/google-objc-function-naming.m
===
--- /dev/null
+++ test/clang-tidy/google-objc-function-naming.m
@@ -0,0 +1,43 @@
+// RUN: %check_clang_tidy %s google-objc-function-naming %t
+
+typedef _Bool bool;
+
+static bool ispositive(int a) { return a > 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'ispositive' not using function naming conventions described by Google Objective-C style guide
+
+static bool is_positive(int a) { return a > 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'is_positive' not using function naming conventions described by Google Objective-C style guide
+
+static bool isPositive(int a) { return a > 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'isPositive' not using function naming conventions described by Google Objective-C style guide
+
+static bool Is_Positive(int a) { return a > 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'Is_Positive' not using function naming conventions described by Google Objective-C style guide
+
+static bool IsPositive(int a) { return a > 0; }
+
+static const char *md5(const char *str) { return 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: function name 'md5' not using function naming conventions described by Google Objective-C style guide
+
+static const char *MD5(const char *str) { return 0; }
+
+static const char *URL(void) { return "https://clang.llvm.org/";; }
+
+static const char *DEFURL(void) { return "https://clang.llvm.org/";; }
+
+static const char *DEFFooURL(void) { return "https://clang.llvm.org/";; }
+
+static const char *StringFromNSString(id str) { return ""; }
+
+bool ispalindrome(const char *str);
+
+void ABLog_String(const char *str) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function name 'ABLog_String' not using function naming conventions described by Google Objective-C style guide
+
+void ABLogString(const char *str) {}
+
+const char *ABURL(void) { return "https://clang.llvm.org/";; }
+
+const char *ABFooURL(void) { return "https://clang.llvm.org/";; }
+
+int main(int argc, const char **argv) { return 0; }
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -118,6 +118,7 @@
google-explicit-constructor
google-global-names-in-headers
google-objc-avoid-throwing-exception
+   google-objc-function-naming
google-objc-global-variable-declaration
google-readability-braces-around-statements (redirects to readability-braces-around-statements) 
google-readability-casting
Index: docs/clang-tidy/checks/google-objc-function-naming.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/google-objc-function-naming.rst
@@ -0,0 +1,29 @@
+.. title:: clang-tidy - google-objc-function-naming
+
+google-objc-function-naming
+===
+
+Finds function definitions in Objective-C files that do not follow the pattern
+described in the Google Objective-C Style Guide.
+
+The corresponding style

[PATCH] D51575: [clang-tidy] Implement a clang-tidy check to verify Google Objective-C function naming conventions 📜

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore updated this revision to Diff 163647.
stephanemoore added a comment.

Fixed alphabetical ordering of clang-tidy improvements in release notes.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51575

Files:
  clang-tidy/google/CMakeLists.txt
  clang-tidy/google/FunctionNamingCheck.cpp
  clang-tidy/google/FunctionNamingCheck.h
  clang-tidy/google/GoogleTidyModule.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/google-objc-function-naming.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/google-objc-function-naming.m
  unittests/clang-tidy/GoogleModuleTest.cpp

Index: unittests/clang-tidy/GoogleModuleTest.cpp
===
--- unittests/clang-tidy/GoogleModuleTest.cpp
+++ unittests/clang-tidy/GoogleModuleTest.cpp
@@ -1,6 +1,7 @@
 #include "ClangTidyTest.h"
 #include "google/ExplicitConstructorCheck.h"
 #include "google/GlobalNamesInHeadersCheck.h"
+#include "google/FunctionNamingCheck.h"
 #include "gtest/gtest.h"
 
 using namespace clang::tidy::google;
@@ -105,6 +106,28 @@
   EXPECT_FALSE(runCheckOnCode("namespace {}", "foo.h"));
 }
 
+TEST(ObjCFunctionNaming, AllowedStaticFunctionName) {
+  std::vector Errors;
+  runCheckOnCode(
+  "static void FooBar(void) {}",
+  &Errors,
+  "input.m");
+  EXPECT_EQ(0ul, Errors.size());
+}
+
+TEST(ObjCFunctionNaming, LowerCamelCaseStaticFunctionName) {
+  std::vector Errors;
+  runCheckOnCode(
+  "static void fooBar(void) {}\n",
+  &Errors,
+  "input.m");
+  EXPECT_EQ(1ul, Errors.size());
+  EXPECT_EQ(
+  "function name 'fooBar' not using function naming conventions described by "
+   "Google Objective-C style guide",
+  Errors[0].Message.Message);
+}
+
 } // namespace test
 } // namespace tidy
 } // namespace clang
Index: test/clang-tidy/google-objc-function-naming.m
===
--- /dev/null
+++ test/clang-tidy/google-objc-function-naming.m
@@ -0,0 +1,43 @@
+// RUN: %check_clang_tidy %s google-objc-function-naming %t
+
+typedef _Bool bool;
+
+static bool ispositive(int a) { return a > 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'ispositive' not using function naming conventions described by Google Objective-C style guide
+
+static bool is_positive(int a) { return a > 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'is_positive' not using function naming conventions described by Google Objective-C style guide
+
+static bool isPositive(int a) { return a > 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'isPositive' not using function naming conventions described by Google Objective-C style guide
+
+static bool Is_Positive(int a) { return a > 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'Is_Positive' not using function naming conventions described by Google Objective-C style guide
+
+static bool IsPositive(int a) { return a > 0; }
+
+static const char *md5(const char *str) { return 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: function name 'md5' not using function naming conventions described by Google Objective-C style guide
+
+static const char *MD5(const char *str) { return 0; }
+
+static const char *URL(void) { return "https://clang.llvm.org/";; }
+
+static const char *DEFURL(void) { return "https://clang.llvm.org/";; }
+
+static const char *DEFFooURL(void) { return "https://clang.llvm.org/";; }
+
+static const char *StringFromNSString(id str) { return ""; }
+
+bool ispalindrome(const char *str);
+
+void ABLog_String(const char *str) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function name 'ABLog_String' not using function naming conventions described by Google Objective-C style guide
+
+void ABLogString(const char *str) {}
+
+const char *ABURL(void) { return "https://clang.llvm.org/";; }
+
+const char *ABFooURL(void) { return "https://clang.llvm.org/";; }
+
+int main(int argc, const char **argv) { return 0; }
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -118,6 +118,7 @@
google-explicit-constructor
google-global-names-in-headers
google-objc-avoid-throwing-exception
+   google-objc-function-naming
google-objc-global-variable-declaration
google-readability-braces-around-statements (redirects to readability-braces-around-statements) 
google-readability-casting
Index: docs/clang-tidy/checks/google-objc-function-naming.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/google-objc-function-naming.rst
@@ -0,0 +1,29 @@
+.. title:: clang-tidy - google-objc-function-naming
+
+google-objc-function-naming
+===
+
+Finds function definitions in Objective-C files that do not follow the pattern
+described in the Google Objective-C Style Guide.
+
+The corresponding style gui

[PATCH] D51575: [clang-tidy] Implement a clang-tidy check to verify Google Objective-C function naming conventions 📜

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore marked an inline comment as done.
stephanemoore added inline comments.



Comment at: docs/ReleaseNotes.rst:60
 
+- New :doc:`google-objc-function-naming
+  ` check.

Eugene.Zelenko wrote:
> Please use alphabetical order.
Good catch. Fixed.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51575



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

[PATCH] D51575: [clang-tidy] Implement a clang-tidy check to verify Google Objective-C function naming conventions 📜

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore updated this revision to Diff 163656.
stephanemoore marked an inline comment as done.
stephanemoore added a comment.

Updated with changes:

- Removed unit tests as other tests have been indicated to provide adequate 
coverage.
- Added a comment explaining why only function definitions are evaluated.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51575

Files:
  clang-tidy/google/CMakeLists.txt
  clang-tidy/google/FunctionNamingCheck.cpp
  clang-tidy/google/FunctionNamingCheck.h
  clang-tidy/google/GoogleTidyModule.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/google-objc-function-naming.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/google-objc-function-naming.m

Index: test/clang-tidy/google-objc-function-naming.m
===
--- /dev/null
+++ test/clang-tidy/google-objc-function-naming.m
@@ -0,0 +1,43 @@
+// RUN: %check_clang_tidy %s google-objc-function-naming %t
+
+typedef _Bool bool;
+
+static bool ispositive(int a) { return a > 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'ispositive' not using function naming conventions described by Google Objective-C style guide
+
+static bool is_positive(int a) { return a > 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'is_positive' not using function naming conventions described by Google Objective-C style guide
+
+static bool isPositive(int a) { return a > 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'isPositive' not using function naming conventions described by Google Objective-C style guide
+
+static bool Is_Positive(int a) { return a > 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'Is_Positive' not using function naming conventions described by Google Objective-C style guide
+
+static bool IsPositive(int a) { return a > 0; }
+
+static const char *md5(const char *str) { return 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: function name 'md5' not using function naming conventions described by Google Objective-C style guide
+
+static const char *MD5(const char *str) { return 0; }
+
+static const char *URL(void) { return "https://clang.llvm.org/";; }
+
+static const char *DEFURL(void) { return "https://clang.llvm.org/";; }
+
+static const char *DEFFooURL(void) { return "https://clang.llvm.org/";; }
+
+static const char *StringFromNSString(id str) { return ""; }
+
+bool ispalindrome(const char *str);
+
+void ABLog_String(const char *str) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function name 'ABLog_String' not using function naming conventions described by Google Objective-C style guide
+
+void ABLogString(const char *str) {}
+
+const char *ABURL(void) { return "https://clang.llvm.org/";; }
+
+const char *ABFooURL(void) { return "https://clang.llvm.org/";; }
+
+int main(int argc, const char **argv) { return 0; }
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -118,6 +118,7 @@
google-explicit-constructor
google-global-names-in-headers
google-objc-avoid-throwing-exception
+   google-objc-function-naming
google-objc-global-variable-declaration
google-readability-braces-around-statements (redirects to readability-braces-around-statements) 
google-readability-casting
Index: docs/clang-tidy/checks/google-objc-function-naming.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/google-objc-function-naming.rst
@@ -0,0 +1,29 @@
+.. title:: clang-tidy - google-objc-function-naming
+
+google-objc-function-naming
+===
+
+Finds function definitions in Objective-C files that do not follow the pattern
+described in the Google Objective-C Style Guide.
+
+The corresponding style guide rule can be found here:
+https://google.github.io/styleguide/objcguide.html#function-names
+
+All function names should be in upper camel case. Functions whose storage class
+is not static should have an appropriate prefix.
+
+The following code sample does not follow this pattern:
+
+.. code-block:: objc
+
+  static bool is_positive(int i) { return i > 0; }
+  bool IsNegative(int i) { return i < 0; }
+
+The sample above might be corrected to the following code:
+
+.. code-block:: objc
+
+  static bool IsPositive(int i) { return i > 0; }
+  bool *ABCIsNegative(int i) { return i < 0; }
+
+The check does not currently recommend any fixes.
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -93,6 +93,12 @@
   Flags uses of ``absl::StrCat()`` to append to a ``std::string``. Suggests 
   ``absl::StrAppend()`` should be used instead.
 
+- New :doc:`google-objc-function-naming
+  ` check.
+
+  Checks that function names in function definitions comply with the naming
+  conventi

[PATCH] D51575: [clang-tidy] Implement a clang-tidy check to verify Google Objective-C function naming conventions 📜

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore marked an inline comment as done.
stephanemoore added inline comments.



Comment at: clang-tidy/google/FunctionNamingCheck.cpp:57
+  functionDecl(
+  isDefinition(),
+  unless(anyOf(isMain(), matchesName(validFunctionNameRegex(true)),

hokein wrote:
> any reason why we restrict to definitions only? I think we can consider 
> declarations too.
I restricted the check to function definitions because function declarations 
are sometimes associated with functions outside the control of the author. I 
have personally observed unfortunate cases where functions declared in the iOS 
SDK had incorrect—or seemingly incorrect—availability attributes that caused 
fatal dyld assertions during application startup. The check currently intends 
to avoid flagging function declarations because of the rare circumstances where 
an inflexible function declaration without a corresponding function definition 
is required.

I have added a comment explaining why only function definitions are flagged.

I am still open to discussion though.



Comment at: unittests/clang-tidy/GoogleModuleTest.cpp:109
 
+TEST(ObjCFunctionNaming, AllowedStaticFunctionName) {
+  std::vector Errors;

hokein wrote:
> nit: we don't need unittest for the check here, as it is well covered in the 
> littest.
Removed the unit tests.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51575



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

[PATCH] D51575: [clang-tidy] Implement a clang-tidy check to verify Google Objective-C function naming conventions 📜

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore updated this revision to Diff 164568.
stephanemoore marked 3 inline comments as done.
stephanemoore edited the summary of this revision.
stephanemoore added a comment.

Implemented the following suggested changes:
• Restricted matching to function declarations that are not in expansions in 
system headers.
• Extended the check to all function declarations rather than just function 
definitions.
• Implemented a fixit hint for functions of static storage class.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51575

Files:
  clang-tidy/google/CMakeLists.txt
  clang-tidy/google/FunctionNamingCheck.cpp
  clang-tidy/google/FunctionNamingCheck.h
  clang-tidy/google/GoogleTidyModule.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/google-objc-function-naming.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/google-objc-function-naming.m

Index: test/clang-tidy/google-objc-function-naming.m
===
--- /dev/null
+++ test/clang-tidy/google-objc-function-naming.m
@@ -0,0 +1,49 @@
+// RUN: %check_clang_tidy %s google-objc-function-naming %t
+
+typedef _Bool bool;
+
+static bool ispositive(int a) { return a > 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'ispositive' not using function naming conventions described by Google Objective-C style guide
+// CHECK-FIXES: static bool Ispositive(int a) { return a > 0; }
+
+static bool is_positive(int a) { return a > 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'is_positive' not using function naming conventions described by Google Objective-C style guide
+// CHECK-FIXES: static bool IsPositive(int a) { return a > 0; }
+
+static bool isPositive(int a) { return a > 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'isPositive' not using function naming conventions described by Google Objective-C style guide
+// CHECK-FIXES: static bool IsPositive(int a) { return a > 0; }
+
+static bool Is_Positive(int a) { return a > 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'Is_Positive' not using function naming conventions described by Google Objective-C style guide
+// CHECK-FIXES: static bool IsPositive(int a) { return a > 0; }
+
+static bool IsPositive(int a) { return a > 0; }
+
+bool ispalindrome(const char *str);
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function name 'ispalindrome' not using function naming conventions described by Google Objective-C style guide
+
+static const char *md5(const char *str) { return 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: function name 'md5' not using function naming conventions described by Google Objective-C style guide
+// CHECK-FIXES: static const char *Md5(const char *str) { return 0; }
+
+static const char *MD5(const char *str) { return 0; }
+
+static const char *URL(void) { return "https://clang.llvm.org/";; }
+
+static const char *DEFURL(void) { return "https://clang.llvm.org/";; }
+
+static const char *DEFFooURL(void) { return "https://clang.llvm.org/";; }
+
+static const char *StringFromNSString(id str) { return ""; }
+
+void ABLog_String(const char *str);
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function name 'ABLog_String' not using function naming conventions described by Google Objective-C style guide
+
+void ABLogString(const char *str);
+
+const char *ABURL(void) { return "https://clang.llvm.org/";; }
+
+const char *ABFooURL(void) { return "https://clang.llvm.org/";; }
+
+int main(int argc, const char **argv) { return 0; }
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -118,6 +118,7 @@
google-explicit-constructor
google-global-names-in-headers
google-objc-avoid-throwing-exception
+   google-objc-function-naming
google-objc-global-variable-declaration
google-readability-braces-around-statements (redirects to readability-braces-around-statements) 
google-readability-casting
Index: docs/clang-tidy/checks/google-objc-function-naming.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/google-objc-function-naming.rst
@@ -0,0 +1,27 @@
+.. title:: clang-tidy - google-objc-function-naming
+
+google-objc-function-naming
+===
+
+Finds function declarations in Objective-C files that do not follow the pattern
+described in the Google Objective-C Style Guide.
+
+The corresponding style guide rule can be found here:
+https://google.github.io/styleguide/objcguide.html#function-names
+
+All function names should be in upper camel case. Functions whose storage class
+is not static should have an appropriate prefix.
+
+The following code sample does not follow this pattern:
+
+.. code-block:: objc
+
+  static bool is_positive(int i) { return i > 0; }
+  bool IsNegative(int i) { return i < 0; }
+
+The sample above might be corre

[PATCH] D51575: [clang-tidy] Implement a clang-tidy check to verify Google Objective-C function naming conventions 📜

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore updated this revision to Diff 164569.
stephanemoore added a comment.

Implemented the following suggested changes:
• Added a note that FunctionNamingCheck does not apply to Objective-C method 
name or property declarations.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51575

Files:
  clang-tidy/google/CMakeLists.txt
  clang-tidy/google/FunctionNamingCheck.cpp
  clang-tidy/google/FunctionNamingCheck.h
  clang-tidy/google/GoogleTidyModule.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/google-objc-function-naming.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/google-objc-function-naming.m

Index: test/clang-tidy/google-objc-function-naming.m
===
--- /dev/null
+++ test/clang-tidy/google-objc-function-naming.m
@@ -0,0 +1,49 @@
+// RUN: %check_clang_tidy %s google-objc-function-naming %t
+
+typedef _Bool bool;
+
+static bool ispositive(int a) { return a > 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'ispositive' not using function naming conventions described by Google Objective-C style guide
+// CHECK-FIXES: static bool Ispositive(int a) { return a > 0; }
+
+static bool is_positive(int a) { return a > 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'is_positive' not using function naming conventions described by Google Objective-C style guide
+// CHECK-FIXES: static bool IsPositive(int a) { return a > 0; }
+
+static bool isPositive(int a) { return a > 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'isPositive' not using function naming conventions described by Google Objective-C style guide
+// CHECK-FIXES: static bool IsPositive(int a) { return a > 0; }
+
+static bool Is_Positive(int a) { return a > 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'Is_Positive' not using function naming conventions described by Google Objective-C style guide
+// CHECK-FIXES: static bool IsPositive(int a) { return a > 0; }
+
+static bool IsPositive(int a) { return a > 0; }
+
+bool ispalindrome(const char *str);
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function name 'ispalindrome' not using function naming conventions described by Google Objective-C style guide
+
+static const char *md5(const char *str) { return 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: function name 'md5' not using function naming conventions described by Google Objective-C style guide
+// CHECK-FIXES: static const char *Md5(const char *str) { return 0; }
+
+static const char *MD5(const char *str) { return 0; }
+
+static const char *URL(void) { return "https://clang.llvm.org/";; }
+
+static const char *DEFURL(void) { return "https://clang.llvm.org/";; }
+
+static const char *DEFFooURL(void) { return "https://clang.llvm.org/";; }
+
+static const char *StringFromNSString(id str) { return ""; }
+
+void ABLog_String(const char *str);
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function name 'ABLog_String' not using function naming conventions described by Google Objective-C style guide
+
+void ABLogString(const char *str);
+
+const char *ABURL(void) { return "https://clang.llvm.org/";; }
+
+const char *ABFooURL(void) { return "https://clang.llvm.org/";; }
+
+int main(int argc, const char **argv) { return 0; }
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -118,6 +118,7 @@
google-explicit-constructor
google-global-names-in-headers
google-objc-avoid-throwing-exception
+   google-objc-function-naming
google-objc-global-variable-declaration
google-readability-braces-around-statements (redirects to readability-braces-around-statements) 
google-readability-casting
Index: docs/clang-tidy/checks/google-objc-function-naming.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/google-objc-function-naming.rst
@@ -0,0 +1,27 @@
+.. title:: clang-tidy - google-objc-function-naming
+
+google-objc-function-naming
+===
+
+Finds function declarations in Objective-C files that do not follow the pattern
+described in the Google Objective-C Style Guide.
+
+The corresponding style guide rule can be found here:
+https://google.github.io/styleguide/objcguide.html#function-names
+
+All function names should be in upper camel case. Functions whose storage class
+is not static should have an appropriate prefix.
+
+The following code sample does not follow this pattern:
+
+.. code-block:: objc
+
+  static bool is_positive(int i) { return i > 0; }
+  bool IsNegative(int i) { return i < 0; }
+
+The sample above might be corrected to the following code:
+
+.. code-block:: objc
+
+  static bool IsPositive(int i) { return i > 0; }
+  bool *ABCIsNegative(int i) { return i < 0; }
Index: docs/ReleaseNotes.rst
==

[PATCH] D51575: [clang-tidy] Implement a clang-tidy check to verify Google Objective-C function naming conventions 📜

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore updated this revision to Diff 164570.
stephanemoore marked an inline comment as done.
stephanemoore added a comment.

Cleaned up comment about FunctionNamingCheck not applying to Objective-C method 
or property declarations.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51575

Files:
  clang-tidy/google/CMakeLists.txt
  clang-tidy/google/FunctionNamingCheck.cpp
  clang-tidy/google/FunctionNamingCheck.h
  clang-tidy/google/GoogleTidyModule.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/google-objc-function-naming.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/google-objc-function-naming.m

Index: test/clang-tidy/google-objc-function-naming.m
===
--- /dev/null
+++ test/clang-tidy/google-objc-function-naming.m
@@ -0,0 +1,49 @@
+// RUN: %check_clang_tidy %s google-objc-function-naming %t
+
+typedef _Bool bool;
+
+static bool ispositive(int a) { return a > 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'ispositive' not using function naming conventions described by Google Objective-C style guide
+// CHECK-FIXES: static bool Ispositive(int a) { return a > 0; }
+
+static bool is_positive(int a) { return a > 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'is_positive' not using function naming conventions described by Google Objective-C style guide
+// CHECK-FIXES: static bool IsPositive(int a) { return a > 0; }
+
+static bool isPositive(int a) { return a > 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'isPositive' not using function naming conventions described by Google Objective-C style guide
+// CHECK-FIXES: static bool IsPositive(int a) { return a > 0; }
+
+static bool Is_Positive(int a) { return a > 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'Is_Positive' not using function naming conventions described by Google Objective-C style guide
+// CHECK-FIXES: static bool IsPositive(int a) { return a > 0; }
+
+static bool IsPositive(int a) { return a > 0; }
+
+bool ispalindrome(const char *str);
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function name 'ispalindrome' not using function naming conventions described by Google Objective-C style guide
+
+static const char *md5(const char *str) { return 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: function name 'md5' not using function naming conventions described by Google Objective-C style guide
+// CHECK-FIXES: static const char *Md5(const char *str) { return 0; }
+
+static const char *MD5(const char *str) { return 0; }
+
+static const char *URL(void) { return "https://clang.llvm.org/";; }
+
+static const char *DEFURL(void) { return "https://clang.llvm.org/";; }
+
+static const char *DEFFooURL(void) { return "https://clang.llvm.org/";; }
+
+static const char *StringFromNSString(id str) { return ""; }
+
+void ABLog_String(const char *str);
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function name 'ABLog_String' not using function naming conventions described by Google Objective-C style guide
+
+void ABLogString(const char *str);
+
+const char *ABURL(void) { return "https://clang.llvm.org/";; }
+
+const char *ABFooURL(void) { return "https://clang.llvm.org/";; }
+
+int main(int argc, const char **argv) { return 0; }
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -118,6 +118,7 @@
google-explicit-constructor
google-global-names-in-headers
google-objc-avoid-throwing-exception
+   google-objc-function-naming
google-objc-global-variable-declaration
google-readability-braces-around-statements (redirects to readability-braces-around-statements) 
google-readability-casting
Index: docs/clang-tidy/checks/google-objc-function-naming.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/google-objc-function-naming.rst
@@ -0,0 +1,27 @@
+.. title:: clang-tidy - google-objc-function-naming
+
+google-objc-function-naming
+===
+
+Finds function declarations in Objective-C files that do not follow the pattern
+described in the Google Objective-C Style Guide.
+
+The corresponding style guide rule can be found here:
+https://google.github.io/styleguide/objcguide.html#function-names
+
+All function names should be in upper camel case. Functions whose storage class
+is not static should have an appropriate prefix.
+
+The following code sample does not follow this pattern:
+
+.. code-block:: objc
+
+  static bool is_positive(int i) { return i > 0; }
+  bool IsNegative(int i) { return i < 0; }
+
+The sample above might be corrected to the following code:
+
+.. code-block:: objc
+
+  static bool IsPositive(int i) { return i > 0; }
+  bool *ABCIsNegative(int i) { return i < 0; }
Index: docs/ReleaseNotes.rst
=

[PATCH] D51575: [clang-tidy] Implement a clang-tidy check to verify Google Objective-C function naming conventions 📜

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore added inline comments.



Comment at: clang-tidy/google/FunctionNamingCheck.cpp:57
+  functionDecl(
+  isDefinition(),
+  unless(anyOf(isMain(), matchesName(validFunctionNameRegex(true)),

hokein wrote:
> stephanemoore wrote:
> > hokein wrote:
> > > any reason why we restrict to definitions only? I think we can consider 
> > > declarations too.
> > I restricted the check to function definitions because function 
> > declarations are sometimes associated with functions outside the control of 
> > the author. I have personally observed unfortunate cases where functions 
> > declared in the iOS SDK had incorrect—or seemingly incorrect—availability 
> > attributes that caused fatal dyld assertions during application startup. 
> > The check currently intends to avoid flagging function declarations because 
> > of the rare circumstances where an inflexible function declaration without 
> > a corresponding function definition is required.
> > 
> > I have added a comment explaining why only function definitions are flagged.
> > 
> > I am still open to discussion though.
> Thanks for the explanations.
> 
> I have a concern about the heuristic using here, it seems fragile -- if there 
> is an inline function defined in a base header, the check will still give a 
> warning to it if the source file `.m` #includes this header; it also limits 
> the scope of the check, I think this check is flagged mostly on file-local 
> functions (e.g. static functions, functions defined in anonymous namespace).
> 
> Flagging on function declaration doesn't seem too problematic (we already 
> have a similar check `objc-property-declaration` does the same thing) -- our 
> internal review system shows check warnings on changed code, if user's code 
> includes a common header which violates the check, warnings on the header 
> will not be shown; for violations in iOS SDK, we can use some filtering 
> matchers (`isExpansionInSystemHeader` maybe work) to ignore all functions 
> from these files.
> 
> 
Good idea to use isExpansionInSystemHeader. I wasn't aware of that particular 
matcher. I have incorporated that into the matching.

I am still wary about flagging function declarations but I think that false 
positives should generally be marginal and we can monitor for them. I have 
extended the check to also include function declarations.



Comment at: clang-tidy/google/FunctionNamingCheck.cpp:35
+  // non-standard capitalized character sequences including acronyms,
+  // initialisms, and prefixes of symbols (e.g., UIColorFromNSString). For this
+  // reason, the regex only verifies that the function name after the prefix

benhamilton wrote:
> benhamilton wrote:
> > Any reason why this is different from the implementation in the property 
> > name checker? Either we should allow both of:
> > 
> > ```
> > void ARBiTraRilyNameDFuncTioN();
> > // ...
> > @property (...) id arBItrArIlyNameD;
> > ```
> > 
> > or we should require that acronyms in the middle of the name be 
> > registered/known acronyms for both properties and functions.
> > 
> > I believe this diff allows arbitrary capitalization for functions, but we 
> > disallowed that for property names, so I think we should be consistent.
> (And, just to be clear, I don't feel strongly which direction we go — it's 
> certainly less work to allow arbitrarily-named functions and properties than 
> to maintain the acronym list.)
> 
I have been meaning to send out a proposal to change the 
`objc-property-declaration` check to allow arbitrary capitalization but I ended 
up sending this out first. Let me prep my proposal for that change 
simultaneously.



Comment at: clang-tidy/google/FunctionNamingCheck.cpp:50
+
+void FunctionNamingCheck::registerMatchers(MatchFinder *Finder) {
+  // This check should only be applied to Objective-C sources.

benhamilton wrote:
> Wizard wrote:
> > Can we do some simple check to see if some easy fix can be provided just 
> > like `objc-property-declaration` check?
> > Something like `static bool isPositive` to `static bool IsPositive` and 
> > `static bool is_upper_camel` to `IsUpperCamel`. Such check can help provide 
> > code fix for a lot of  very common mistake at a low cost (i.e. if the 
> > naming pattern cannot be simply recognized, just provide no fix).
> +1, I think the two checks should be substantially similar.
Implemented a fixit hint for functions of static storage class.



Comment at: clang-tidy/google/FunctionNamingCheck.h:21
+
+/// Finds function names that do not conform to the recommendations of the
+/// Google Objective-C Style Guide. Function names should be in upper camel 
case

benhamilton wrote:
> Worth mentioning this does not apply to Objective-C method names, nor 
> Objective-C properties.
> 
Added a note that this check does not apply to Objective-C methods or 
properties.

[PATCH] D51832: [clang-tidy/checks] Update objc-property-declaration check to allow arbitrary acronyms and initialisms 🔧

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore created this revision.
stephanemoore added reviewers: benhamilton, Wizard.
Herald added subscribers: cfe-commits, jfb.

§1 Description

This changes the objc-property-declaration check to allow arbitrary acronyms 
and initialisms instead of using whitelisted acronyms. In Objective-C it is 
relatively common to use project prefixes in property names for the purposes of 
disambiguation. For example, the CIColor¹ and CGColor² properties on UIColor 
both represent symbol prefixes being used in proeprty names outside of Apple's 
accepted acronyms³. The union of Apple's accepted acronyms and all symbol 
prefixes that might be used for disambiguation in property declarations 
effectively allows for any arbitrary sequence of capital alphanumeric 
characters to be acceptable in property declarations. This change updates the 
check accordingly.

The test variants with custom configurations are deleted as part of this change 
because their configurations no longer impact behavior. The acronym 
configurations are currently preserved for backwards compatibility of check 
configuration.

[1] 
https://developer.apple.com/documentation/uikit/uicolor/1621951-cicolor?language=objc
[2] 
https://developer.apple.com/documentation/uikit/uicolor/1621954-cgcolor?language=objc
[3] 
https://developer.apple.com/library/archive/documentation/Cocoa/Conceptual/CodingGuidelines/Articles/APIAbbreviations.html#//apple_ref/doc/uid/20001285-BCIHCGAE

§2 Test Notes

Changes verified by:
• Running clang-tidy unit tests.
• Used check_clang_tidy.py to verify expected output of processing 
objc-property-declaration.m


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51832

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

Index: test/clang-tidy/objc-property-declaration.m
===
--- test/clang-tidy/objc-property-declaration.m
+++ test/clang-tidy/objc-property-declaration.m
@@ -2,6 +2,9 @@
 @class NSData;
 @class NSString;
 @class UIViewController;
+@class CIColor;
+
+typedef void *CGColorRef;
 
 @interface Foo
 @property(assign, nonatomic) int NotCamelCase;
@@ -23,6 +26,9 @@
 @property(assign, nonatomic) int enableGLAcceleration;
 @property(assign, nonatomic) int ID;
 @property(assign, nonatomic) int hasADog;
+@property(nonatomic, readonly) CGColorRef CGColor;
+@property(nonatomic, readonly) CIColor *CIColor;
+@property(nonatomic, copy) NSArray *IDs;
 @end
 
 @interface Foo (Bar)
Index: test/clang-tidy/objc-property-declaration-custom.m
===
--- test/clang-tidy/objc-property-declaration-custom.m
+++ /dev/null
@@ -1,18 +0,0 @@
-// 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.IncludeDefaultAcronyms, value: 0}]}' \
-// RUN: --
-@class NSString;
-
-@interface Foo
-@property(assign, nonatomic) int AbcNotRealPrefix;
-// CHECK-MESSAGES: :[[@LINE-1]]:34: warning: property name 'AbcNotRealPrefix' not using lowerCamelCase style or not prefixed in a category, 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' not using lowerCamelCase style or not prefixed in a category, according to the Apple Coding Guidelines [objc-property-declaration]
-@property(assign, nonatomic) int GIFIgnoreStandardAcronym;
-// CHECK-MESSAGES: :[[@LINE-1]]:34: warning: property name 'GIFIgnoreStandardAcronym' not using lowerCamelCase style or not prefixed in a category, according to the Apple Coding Guidelines [objc-property-declaration]
-@property(strong, nonatomic) NSString *TGIF;
-@end
Index: test/clang-tidy/objc-property-declaration-additional.m
===
--- test/clang-tidy/objc-property-declaration-additional.m
+++ /dev/null
@@ -1,15 +0,0 @@
-// 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' not using lowerCamelCase style or not prefixed in a category, 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_cust

[PATCH] D51832: [clang-tidy/checks] Update objc-property-declaration check to allow arbitrary acronyms and initialisms 🔧

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore updated this revision to Diff 164582.
stephanemoore added a comment.

Fix a couple issues:
• Forward declare `NSArray` in `objc-property-declaration.m`.
• Remove unnecessary lightweight generics declaration from `IDs` property in 
`objc-property-declaration.m` to fix compilation error.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51832

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

Index: test/clang-tidy/objc-property-declaration.m
===
--- test/clang-tidy/objc-property-declaration.m
+++ test/clang-tidy/objc-property-declaration.m
@@ -1,7 +1,11 @@
 // RUN: %check_clang_tidy %s objc-property-declaration %t
 @class NSData;
+@class NSArray;
 @class NSString;
 @class UIViewController;
+@class CIColor;
+
+typedef void *CGColorRef;
 
 @interface Foo
 @property(assign, nonatomic) int NotCamelCase;
@@ -23,6 +27,9 @@
 @property(assign, nonatomic) int enableGLAcceleration;
 @property(assign, nonatomic) int ID;
 @property(assign, nonatomic) int hasADog;
+@property(nonatomic, readonly) CGColorRef CGColor;
+@property(nonatomic, readonly) CIColor *CIColor;
+@property(nonatomic, copy) NSArray *IDs;
 @end
 
 @interface Foo (Bar)
Index: test/clang-tidy/objc-property-declaration-custom.m
===
--- test/clang-tidy/objc-property-declaration-custom.m
+++ /dev/null
@@ -1,18 +0,0 @@
-// 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.IncludeDefaultAcronyms, value: 0}]}' \
-// RUN: --
-@class NSString;
-
-@interface Foo
-@property(assign, nonatomic) int AbcNotRealPrefix;
-// CHECK-MESSAGES: :[[@LINE-1]]:34: warning: property name 'AbcNotRealPrefix' not using lowerCamelCase style or not prefixed in a category, 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' not using lowerCamelCase style or not prefixed in a category, according to the Apple Coding Guidelines [objc-property-declaration]
-@property(assign, nonatomic) int GIFIgnoreStandardAcronym;
-// CHECK-MESSAGES: :[[@LINE-1]]:34: warning: property name 'GIFIgnoreStandardAcronym' not using lowerCamelCase style or not prefixed in a category, according to the Apple Coding Guidelines [objc-property-declaration]
-@property(strong, nonatomic) NSString *TGIF;
-@end
Index: test/clang-tidy/objc-property-declaration-additional.m
===
--- test/clang-tidy/objc-property-declaration-additional.m
+++ /dev/null
@@ -1,15 +0,0 @@
-// 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' not using lowerCamelCase style or not prefixed in a category, 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' not using lowerCamelCase style or not prefixed in a category, according to the Apple Coding Guidelines [objc-property-declaration]
-@property(assign, nonatomic) int GIFShouldIncludeStandardAcronym;
-@end
Index: clang-tidy/objc/PropertyDeclarationCheck.cpp
===
--- clang-tidy/objc/PropertyDeclarationCheck.cpp
+++ clang-tidy/objc/PropertyDeclarationCheck.cpp
@@ -34,91 +34,6 @@
   CategoryProperty = 2,
 };
 
-/// The acronyms are aggregated from multiple sources including
-/// https://developer.apple.com/library/content/documentation/Cocoa/Conceptual/CodingGuidelines/Articles/APIAbbreviations.html#//apple_ref/doc/uid/20001285-BCIHCGAE
-///
-/// Keep this list sorted.
-constexpr llvm::StringLiteral DefaultSpecialAcronyms[] = {
-"[2-9]G",
-"ACL",
-"API",
-"AR",
-"ARGB",
-"ASCII",
-"AV",
-"BGRA",
-"CA",
-"CF",
-"CG",
-"CI",
-"CRC",
-"CV",
-"CMYK",
-"DNS",
-"FPS",
-"FTP",
-"GIF",
-"GL",
-"GPS",
-"GUID",
-"HD",
-"HDR",
-"HMAC",
-"HTML",
-"HTTP",
-"HTTPS",
-"HUD",
-

[PATCH] D51832: [clang-tidy/checks] Update objc-property-declaration check to allow arbitrary acronyms and initialisms 🔧

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore updated this revision to Diff 164583.
stephanemoore added a comment.

Sorted forward declared classes in `objc-property-declaration.m`.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51832

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

Index: test/clang-tidy/objc-property-declaration.m
===
--- test/clang-tidy/objc-property-declaration.m
+++ test/clang-tidy/objc-property-declaration.m
@@ -1,8 +1,12 @@
 // RUN: %check_clang_tidy %s objc-property-declaration %t
+@class CIColor;
+@class NSArray;
 @class NSData;
 @class NSString;
 @class UIViewController;
 
+typedef void *CGColorRef;
+
 @interface Foo
 @property(assign, nonatomic) int NotCamelCase;
 // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: property name 'NotCamelCase' not using lowerCamelCase style or not prefixed in a category, according to the Apple Coding Guidelines [objc-property-declaration]
@@ -23,6 +27,9 @@
 @property(assign, nonatomic) int enableGLAcceleration;
 @property(assign, nonatomic) int ID;
 @property(assign, nonatomic) int hasADog;
+@property(nonatomic, readonly) CGColorRef CGColor;
+@property(nonatomic, readonly) CIColor *CIColor;
+@property(nonatomic, copy) NSArray *IDs;
 @end
 
 @interface Foo (Bar)
Index: test/clang-tidy/objc-property-declaration-custom.m
===
--- test/clang-tidy/objc-property-declaration-custom.m
+++ /dev/null
@@ -1,18 +0,0 @@
-// 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.IncludeDefaultAcronyms, value: 0}]}' \
-// RUN: --
-@class NSString;
-
-@interface Foo
-@property(assign, nonatomic) int AbcNotRealPrefix;
-// CHECK-MESSAGES: :[[@LINE-1]]:34: warning: property name 'AbcNotRealPrefix' not using lowerCamelCase style or not prefixed in a category, 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' not using lowerCamelCase style or not prefixed in a category, according to the Apple Coding Guidelines [objc-property-declaration]
-@property(assign, nonatomic) int GIFIgnoreStandardAcronym;
-// CHECK-MESSAGES: :[[@LINE-1]]:34: warning: property name 'GIFIgnoreStandardAcronym' not using lowerCamelCase style or not prefixed in a category, according to the Apple Coding Guidelines [objc-property-declaration]
-@property(strong, nonatomic) NSString *TGIF;
-@end
Index: test/clang-tidy/objc-property-declaration-additional.m
===
--- test/clang-tidy/objc-property-declaration-additional.m
+++ /dev/null
@@ -1,15 +0,0 @@
-// 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' not using lowerCamelCase style or not prefixed in a category, 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' not using lowerCamelCase style or not prefixed in a category, according to the Apple Coding Guidelines [objc-property-declaration]
-@property(assign, nonatomic) int GIFShouldIncludeStandardAcronym;
-@end
Index: clang-tidy/objc/PropertyDeclarationCheck.cpp
===
--- clang-tidy/objc/PropertyDeclarationCheck.cpp
+++ clang-tidy/objc/PropertyDeclarationCheck.cpp
@@ -34,91 +34,6 @@
   CategoryProperty = 2,
 };
 
-/// The acronyms are aggregated from multiple sources including
-/// https://developer.apple.com/library/content/documentation/Cocoa/Conceptual/CodingGuidelines/Articles/APIAbbreviations.html#//apple_ref/doc/uid/20001285-BCIHCGAE
-///
-/// Keep this list sorted.
-constexpr llvm::StringLiteral DefaultSpecialAcronyms[] = {
-"[2-9]G",
-"ACL",
-"API",
-"AR",
-"ARGB",
-"ASCII",
-"AV",
-"BGRA",
-"CA",
-"CF",
-"CG",
-"CI",
-"CRC",
-"CV",
-"CMYK",
-"DNS",
-"FPS",
-"FTP",
-"GIF",
-"GL",
-"GPS",
-"GUID",
-"HD",
-"HDR",
-"HMAC"

[PATCH] D42704: [clang-format] Do not break Objective-C string literals inside array literals

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore added a comment.

The summary states that -Wobjc-string-concatenation is enabled by default? I 
looked through `include/clang/Basic/DiagnosticGroups.td` and did not see 
evidence that `ObjCStringConcatenation` is under `All`. Am I missing something? 
Is -Wobjc-string-concatenation enabled by default through some other mechanism?


Repository:
  rC Clang

https://reviews.llvm.org/D42704



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

[PATCH] D42708: [clang-format] Set ObjCBinPackProtocolList to Never for google style

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore added a comment.

I believe that I have consensus from Google to be able to approve this. I sent 
out a last call for dissenting opinions. If I don't hear anything I plan on 
approving tomorrow.


Repository:
  rC Clang

https://reviews.llvm.org/D42708



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

[PATCH] D42864: [clang-format] Add more tests for Objective-C 2.0 generic alignment

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore added a comment.

Looks reasonable 👌


Repository:
  rL LLVM

https://reviews.llvm.org/D42864



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

[PATCH] D42708: [clang-format] Set ObjCBinPackProtocolList to Never for google style

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore accepted this revision.
stephanemoore added a comment.

Pretty unanimous from the Google style arbitration side. Looks good to me.


Repository:
  rC Clang

https://reviews.llvm.org/D42708



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

[PATCH] D42704: [clang-format] Do not break Objective-C string literals inside array literals

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore added a comment.

Thanks for the explanation 👌 I'm still learning the inner workings of clang 😅

The change looks good to me.


Repository:
  rC Clang

https://reviews.llvm.org/D42704



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

[PATCH] D43581: [clang-tidy/google] Fix the Objective-C global variable declaration check 🔧

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore created this revision.
Herald added a subscriber: cfe-commits.

The current Objective-C global variable declaration check restricts naming that 
is permitted by the Objective-C style guide.

The Objective-C style guide states the following:
"Global and file scope constants should have an appropriate prefix. [...] 
Constants may use a lowercase k prefix when appropriate"
http://google.github.io/styleguide/objcguide#constants

This change fixes the check to allow two or more capital letters as an 
appropriate prefix. This change intentionally avoids making a decision 
regarding whether to flag constants that use a two letter prefix (two letter 
prefixes are reserved by Apple¹ but many projects seem to violate this 
guideline).

❧

(1)
"Two-letter prefixes like these are reserved by Apple for use in framework 
classes."
https://developer.apple.com/library/content/documentation/Cocoa/Conceptual/ProgrammingWithObjectiveC/Conventions/Conventions.html


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D43581

Files:
  clang-tidy/google/GlobalVariableDeclarationCheck.cpp
  test/clang-tidy/google-objc-global-variable-declaration.m


Index: test/clang-tidy/google-objc-global-variable-declaration.m
===
--- test/clang-tidy/google-objc-global-variable-declaration.m
+++ test/clang-tidy/google-objc-global-variable-declaration.m
@@ -30,6 +30,7 @@
 // CHECK-FIXES: static NSString* const k_Alpha = @"SecondNotAlpha";
 
 static NSString* const kGood = @"hello";
+static NSString* const XYGood = @"hello";
 static NSString* gMyIntGood = 0;
 
 @implementation Foo
Index: clang-tidy/google/GlobalVariableDeclarationCheck.cpp
===
--- clang-tidy/google/GlobalVariableDeclarationCheck.cpp
+++ clang-tidy/google/GlobalVariableDeclarationCheck.cpp
@@ -72,7 +72,7 @@
   this);
   Finder->addMatcher(varDecl(hasGlobalStorage(), hasType(isConstQualified()),
  unless(isLocalVariable()),
- unless(matchesName("::k[A-Z]")))
+ unless(matchesName("::(k[A-Z]|[A-Z]{2,})")))
  .bind("global_const"),
  this);
 }


Index: test/clang-tidy/google-objc-global-variable-declaration.m
===
--- test/clang-tidy/google-objc-global-variable-declaration.m
+++ test/clang-tidy/google-objc-global-variable-declaration.m
@@ -30,6 +30,7 @@
 // CHECK-FIXES: static NSString* const k_Alpha = @"SecondNotAlpha";
 
 static NSString* const kGood = @"hello";
+static NSString* const XYGood = @"hello";
 static NSString* gMyIntGood = 0;
 
 @implementation Foo
Index: clang-tidy/google/GlobalVariableDeclarationCheck.cpp
===
--- clang-tidy/google/GlobalVariableDeclarationCheck.cpp
+++ clang-tidy/google/GlobalVariableDeclarationCheck.cpp
@@ -72,7 +72,7 @@
   this);
   Finder->addMatcher(varDecl(hasGlobalStorage(), hasType(isConstQualified()),
  unless(isLocalVariable()),
- unless(matchesName("::k[A-Z]")))
+ unless(matchesName("::(k[A-Z]|[A-Z]{2,})")))
  .bind("global_const"),
  this);
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D43581: [clang-tidy/google] Fix the Objective-C global variable declaration check 🔧

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore updated this revision to Diff 135291.
stephanemoore added a comment.

Added excerpts from the Google Objective-C style guide section on constant 
naming (http://google.github.io/styleguide/objcguide#constants) as additional 
test cases.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D43581

Files:
  clang-tidy/google/GlobalVariableDeclarationCheck.cpp
  test/clang-tidy/google-objc-global-variable-declaration.m


Index: test/clang-tidy/google-objc-global-variable-declaration.m
===
--- test/clang-tidy/google-objc-global-variable-declaration.m
+++ test/clang-tidy/google-objc-global-variable-declaration.m
@@ -30,8 +30,16 @@
 // CHECK-FIXES: static NSString* const k_Alpha = @"SecondNotAlpha";
 
 static NSString* const kGood = @"hello";
+static NSString* const XYGood = @"hello";
 static NSString* gMyIntGood = 0;
 
+extern NSString* const GTLServiceErrorDomain;
+
+typedef NS_ENUM(NSInteger, GTLServiceError) {
+  GTLServiceErrorQueryResultMissing = -3000,
+  GTLServiceErrorWaitTimedOut   = -3001,
+};
+
 @implementation Foo
 - (void)f {
 int x = 0;
Index: clang-tidy/google/GlobalVariableDeclarationCheck.cpp
===
--- clang-tidy/google/GlobalVariableDeclarationCheck.cpp
+++ clang-tidy/google/GlobalVariableDeclarationCheck.cpp
@@ -72,7 +72,7 @@
   this);
   Finder->addMatcher(varDecl(hasGlobalStorage(), hasType(isConstQualified()),
  unless(isLocalVariable()),
- unless(matchesName("::k[A-Z]")))
+ unless(matchesName("::(k[A-Z]|[A-Z]{2,})")))
  .bind("global_const"),
  this);
 }


Index: test/clang-tidy/google-objc-global-variable-declaration.m
===
--- test/clang-tidy/google-objc-global-variable-declaration.m
+++ test/clang-tidy/google-objc-global-variable-declaration.m
@@ -30,8 +30,16 @@
 // CHECK-FIXES: static NSString* const k_Alpha = @"SecondNotAlpha";
 
 static NSString* const kGood = @"hello";
+static NSString* const XYGood = @"hello";
 static NSString* gMyIntGood = 0;
 
+extern NSString* const GTLServiceErrorDomain;
+
+typedef NS_ENUM(NSInteger, GTLServiceError) {
+  GTLServiceErrorQueryResultMissing = -3000,
+  GTLServiceErrorWaitTimedOut   = -3001,
+};
+
 @implementation Foo
 - (void)f {
 int x = 0;
Index: clang-tidy/google/GlobalVariableDeclarationCheck.cpp
===
--- clang-tidy/google/GlobalVariableDeclarationCheck.cpp
+++ clang-tidy/google/GlobalVariableDeclarationCheck.cpp
@@ -72,7 +72,7 @@
   this);
   Finder->addMatcher(varDecl(hasGlobalStorage(), hasType(isConstQualified()),
  unless(isLocalVariable()),
- unless(matchesName("::k[A-Z]")))
+ unless(matchesName("::(k[A-Z]|[A-Z]{2,})")))
  .bind("global_const"),
  this);
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D43581: [clang-tidy/google] Fix the Objective-C global variable declaration check 🔧

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore marked an inline comment as done.
stephanemoore added a comment.

In https://reviews.llvm.org/D43581#1014664, @aaron.ballman wrote:

> LGTM with a few additional test cases.
>
> It'd be nice if the style guide linked actually described what a "good" 
> prefix is rather than make the reader guess.


I will work on updating the Google Objective-C style guide to provide more 
clarity regarding what constitutes an appropriate prefix.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D43581



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

[PATCH] D43640: add support for constants with appropriate prefix. Start with 'k' is not the only option. This is according to http://google.github.io/styleguide/objcguide.html#constants

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore added a comment.

I have this change out for review as well:
https://reviews.llvm.org/D43581


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D43640



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

[PATCH] D43581: [clang-tidy/google] Fix the Objective-C global variable declaration check 🔧

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore updated this revision to Diff 135489.
stephanemoore added a comment.

I forgot to update the diagnostic message per the change.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D43581

Files:
  clang-tidy/google/GlobalVariableDeclarationCheck.cpp
  test/clang-tidy/google-objc-global-variable-declaration.m


Index: test/clang-tidy/google-objc-global-variable-declaration.m
===
--- test/clang-tidy/google-objc-global-variable-declaration.m
+++ test/clang-tidy/google-objc-global-variable-declaration.m
@@ -30,8 +30,16 @@
 // CHECK-FIXES: static NSString* const k_Alpha = @"SecondNotAlpha";
 
 static NSString* const kGood = @"hello";
+static NSString* const XYGood = @"hello";
 static NSString* gMyIntGood = 0;
 
+extern NSString* const GTLServiceErrorDomain;
+
+typedef NS_ENUM(NSInteger, GTLServiceError) {
+  GTLServiceErrorQueryResultMissing = -3000,
+  GTLServiceErrorWaitTimedOut   = -3001,
+};
+
 @implementation Foo
 - (void)f {
 int x = 0;
Index: clang-tidy/google/GlobalVariableDeclarationCheck.cpp
===
--- clang-tidy/google/GlobalVariableDeclarationCheck.cpp
+++ clang-tidy/google/GlobalVariableDeclarationCheck.cpp
@@ -72,7 +72,7 @@
   this);
   Finder->addMatcher(varDecl(hasGlobalStorage(), hasType(isConstQualified()),
  unless(isLocalVariable()),
- unless(matchesName("::k[A-Z]")))
+ unless(matchesName("::(k[A-Z]|[A-Z]{2,})")))
  .bind("global_const"),
  this);
 }
@@ -88,7 +88,7 @@
   if (const auto *Decl = Result.Nodes.getNodeAs("global_const")) {
 diag(Decl->getLocation(),
  "const global variable '%0' must have a name which starts with "
- "'k[A-Z]'")
+ "an appropriate prefix matching '(k[A-Z]|[A-Z]{2,})'.")
 << Decl->getName() << generateFixItHint(Decl, true);
   }
 }


Index: test/clang-tidy/google-objc-global-variable-declaration.m
===
--- test/clang-tidy/google-objc-global-variable-declaration.m
+++ test/clang-tidy/google-objc-global-variable-declaration.m
@@ -30,8 +30,16 @@
 // CHECK-FIXES: static NSString* const k_Alpha = @"SecondNotAlpha";
 
 static NSString* const kGood = @"hello";
+static NSString* const XYGood = @"hello";
 static NSString* gMyIntGood = 0;
 
+extern NSString* const GTLServiceErrorDomain;
+
+typedef NS_ENUM(NSInteger, GTLServiceError) {
+  GTLServiceErrorQueryResultMissing = -3000,
+  GTLServiceErrorWaitTimedOut   = -3001,
+};
+
 @implementation Foo
 - (void)f {
 int x = 0;
Index: clang-tidy/google/GlobalVariableDeclarationCheck.cpp
===
--- clang-tidy/google/GlobalVariableDeclarationCheck.cpp
+++ clang-tidy/google/GlobalVariableDeclarationCheck.cpp
@@ -72,7 +72,7 @@
   this);
   Finder->addMatcher(varDecl(hasGlobalStorage(), hasType(isConstQualified()),
  unless(isLocalVariable()),
- unless(matchesName("::k[A-Z]")))
+ unless(matchesName("::(k[A-Z]|[A-Z]{2,})")))
  .bind("global_const"),
  this);
 }
@@ -88,7 +88,7 @@
   if (const auto *Decl = Result.Nodes.getNodeAs("global_const")) {
 diag(Decl->getLocation(),
  "const global variable '%0' must have a name which starts with "
- "'k[A-Z]'")
+ "an appropriate prefix matching '(k[A-Z]|[A-Z]{2,})'.")
 << Decl->getName() << generateFixItHint(Decl, true);
   }
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D43581: [clang-tidy/google] Fix the Objective-C global variable declaration check 🔧

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore added a comment.

In https://reviews.llvm.org/D43581#1016300, @Wizard wrote:

> Please update the warning info to indicate that prefix 'k' is not the only 
> option for constants. Something like:
>  "const global variable '%0' must have an appropriate prefix or a name which 
> starts with 'k[A-Z]'"


Is the latest warning info satisfactory? I believe I updated it after adding 
you as a reviewer.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D43581



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

[PATCH] D43581: [clang-tidy/google] Fix the Objective-C global variable declaration check 🔧

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore updated this revision to Diff 135497.
stephanemoore added a comment.

Update the diagnostic information to link to the Google Objective-C style guide 
for guidance on an appropriate prefix.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D43581

Files:
  clang-tidy/google/GlobalVariableDeclarationCheck.cpp
  test/clang-tidy/google-objc-global-variable-declaration.m


Index: test/clang-tidy/google-objc-global-variable-declaration.m
===
--- test/clang-tidy/google-objc-global-variable-declaration.m
+++ test/clang-tidy/google-objc-global-variable-declaration.m
@@ -30,8 +30,16 @@
 // CHECK-FIXES: static NSString* const k_Alpha = @"SecondNotAlpha";
 
 static NSString* const kGood = @"hello";
+static NSString* const XYGood = @"hello";
 static NSString* gMyIntGood = 0;
 
+extern NSString* const GTLServiceErrorDomain;
+
+typedef NS_ENUM(NSInteger, GTLServiceError) {
+  GTLServiceErrorQueryResultMissing = -3000,
+  GTLServiceErrorWaitTimedOut   = -3001,
+};
+
 @implementation Foo
 - (void)f {
 int x = 0;
Index: clang-tidy/google/GlobalVariableDeclarationCheck.cpp
===
--- clang-tidy/google/GlobalVariableDeclarationCheck.cpp
+++ clang-tidy/google/GlobalVariableDeclarationCheck.cpp
@@ -72,7 +72,7 @@
   this);
   Finder->addMatcher(varDecl(hasGlobalStorage(), hasType(isConstQualified()),
  unless(isLocalVariable()),
- unless(matchesName("::k[A-Z]")))
+ unless(matchesName("::(k[A-Z]|[A-Z]{2,})")))
  .bind("global_const"),
  this);
 }
@@ -88,7 +88,8 @@
   if (const auto *Decl = Result.Nodes.getNodeAs("global_const")) {
 diag(Decl->getLocation(),
  "const global variable '%0' must have a name which starts with "
- "'k[A-Z]'")
+ "an appropriate prefix (see "
+ "http://google.github.io/styleguide/objcguide#constants).")
 << Decl->getName() << generateFixItHint(Decl, true);
   }
 }


Index: test/clang-tidy/google-objc-global-variable-declaration.m
===
--- test/clang-tidy/google-objc-global-variable-declaration.m
+++ test/clang-tidy/google-objc-global-variable-declaration.m
@@ -30,8 +30,16 @@
 // CHECK-FIXES: static NSString* const k_Alpha = @"SecondNotAlpha";
 
 static NSString* const kGood = @"hello";
+static NSString* const XYGood = @"hello";
 static NSString* gMyIntGood = 0;
 
+extern NSString* const GTLServiceErrorDomain;
+
+typedef NS_ENUM(NSInteger, GTLServiceError) {
+  GTLServiceErrorQueryResultMissing = -3000,
+  GTLServiceErrorWaitTimedOut   = -3001,
+};
+
 @implementation Foo
 - (void)f {
 int x = 0;
Index: clang-tidy/google/GlobalVariableDeclarationCheck.cpp
===
--- clang-tidy/google/GlobalVariableDeclarationCheck.cpp
+++ clang-tidy/google/GlobalVariableDeclarationCheck.cpp
@@ -72,7 +72,7 @@
   this);
   Finder->addMatcher(varDecl(hasGlobalStorage(), hasType(isConstQualified()),
  unless(isLocalVariable()),
- unless(matchesName("::k[A-Z]")))
+ unless(matchesName("::(k[A-Z]|[A-Z]{2,})")))
  .bind("global_const"),
  this);
 }
@@ -88,7 +88,8 @@
   if (const auto *Decl = Result.Nodes.getNodeAs("global_const")) {
 diag(Decl->getLocation(),
  "const global variable '%0' must have a name which starts with "
- "'k[A-Z]'")
+ "an appropriate prefix (see "
+ "http://google.github.io/styleguide/objcguide#constants).")
 << Decl->getName() << generateFixItHint(Decl, true);
   }
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D43581: [clang-tidy/google] Fix the Objective-C global variable declaration check 🔧

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore added a comment.

In https://reviews.llvm.org/D43581#1016327, @Wizard wrote:

> In https://reviews.llvm.org/D43581#1016318, @stephanemoore wrote:
>
> > In https://reviews.llvm.org/D43581#1016300, @Wizard wrote:
> >
> > > Please update the warning info to indicate that prefix 'k' is not the 
> > > only option for constants. Something like:
> > >  "const global variable '%0' must have an appropriate prefix or a name 
> > > which starts with 'k[A-Z]'"
> >
> >
> > Is the latest warning info satisfactory? I believe I updated it after 
> > adding you as a reviewer.
>
>
> Hmm I feel it is a bit unfriendly to show users a rather complicated regex in 
> the info, but I will leave it to you. Not a big problem.


At a glance this seems unconventional but could we have the warning link to the 
Google Objective-C style guide for guidance on an appropriate prefix? Is that 
allowed for diagnostic messages? I can revert if that is considered 
inappropriate.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D43581



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

[PATCH] D43581: [clang-tidy/google] Fix the Objective-C global variable declaration check 🔧

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore updated this revision to Diff 135510.
stephanemoore added a comment.

Revert the inclusion of the link to the Google Objective-C style guide in the 
diagnostic message.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D43581

Files:
  clang-tidy/google/GlobalVariableDeclarationCheck.cpp
  test/clang-tidy/google-objc-global-variable-declaration.m


Index: test/clang-tidy/google-objc-global-variable-declaration.m
===
--- test/clang-tidy/google-objc-global-variable-declaration.m
+++ test/clang-tidy/google-objc-global-variable-declaration.m
@@ -30,8 +30,16 @@
 // CHECK-FIXES: static NSString* const k_Alpha = @"SecondNotAlpha";
 
 static NSString* const kGood = @"hello";
+static NSString* const XYGood = @"hello";
 static NSString* gMyIntGood = 0;
 
+extern NSString* const GTLServiceErrorDomain;
+
+typedef NS_ENUM(NSInteger, GTLServiceError) {
+  GTLServiceErrorQueryResultMissing = -3000,
+  GTLServiceErrorWaitTimedOut   = -3001,
+};
+
 @implementation Foo
 - (void)f {
 int x = 0;
Index: clang-tidy/google/GlobalVariableDeclarationCheck.cpp
===
--- clang-tidy/google/GlobalVariableDeclarationCheck.cpp
+++ clang-tidy/google/GlobalVariableDeclarationCheck.cpp
@@ -72,7 +72,7 @@
   this);
   Finder->addMatcher(varDecl(hasGlobalStorage(), hasType(isConstQualified()),
  unless(isLocalVariable()),
- unless(matchesName("::k[A-Z]")))
+ unless(matchesName("::(k[A-Z]|[A-Z]{2,})")))
  .bind("global_const"),
  this);
 }
@@ -88,7 +88,7 @@
   if (const auto *Decl = Result.Nodes.getNodeAs("global_const")) {
 diag(Decl->getLocation(),
  "const global variable '%0' must have a name which starts with "
- "'k[A-Z]'")
+ "an appropriate prefix matching '(k[A-Z]|[A-Z]{2,})'.")
 << Decl->getName() << generateFixItHint(Decl, true);
   }
 }


Index: test/clang-tidy/google-objc-global-variable-declaration.m
===
--- test/clang-tidy/google-objc-global-variable-declaration.m
+++ test/clang-tidy/google-objc-global-variable-declaration.m
@@ -30,8 +30,16 @@
 // CHECK-FIXES: static NSString* const k_Alpha = @"SecondNotAlpha";
 
 static NSString* const kGood = @"hello";
+static NSString* const XYGood = @"hello";
 static NSString* gMyIntGood = 0;
 
+extern NSString* const GTLServiceErrorDomain;
+
+typedef NS_ENUM(NSInteger, GTLServiceError) {
+  GTLServiceErrorQueryResultMissing = -3000,
+  GTLServiceErrorWaitTimedOut   = -3001,
+};
+
 @implementation Foo
 - (void)f {
 int x = 0;
Index: clang-tidy/google/GlobalVariableDeclarationCheck.cpp
===
--- clang-tidy/google/GlobalVariableDeclarationCheck.cpp
+++ clang-tidy/google/GlobalVariableDeclarationCheck.cpp
@@ -72,7 +72,7 @@
   this);
   Finder->addMatcher(varDecl(hasGlobalStorage(), hasType(isConstQualified()),
  unless(isLocalVariable()),
- unless(matchesName("::k[A-Z]")))
+ unless(matchesName("::(k[A-Z]|[A-Z]{2,})")))
  .bind("global_const"),
  this);
 }
@@ -88,7 +88,7 @@
   if (const auto *Decl = Result.Nodes.getNodeAs("global_const")) {
 diag(Decl->getLocation(),
  "const global variable '%0' must have a name which starts with "
- "'k[A-Z]'")
+ "an appropriate prefix matching '(k[A-Z]|[A-Z]{2,})'.")
 << Decl->getName() << generateFixItHint(Decl, true);
   }
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D43581: [clang-tidy/google] Improve the Objective-C global variable declaration check 🔧

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore updated this revision to Diff 135595.
stephanemoore marked 2 inline comments as done.
stephanemoore retitled this revision from "[clang-tidy/google] Fix the 
Objective-C global variable declaration check 🔧" to "[clang-tidy/google] 
Improve the Objective-C global variable declaration check 🔧".
stephanemoore edited the summary of this revision.
stephanemoore added a comment.

I had to make two more fixes to get the tests working:
• Updated the CHECK-MESSAGES in 
`test/clang-tidy/google-objc-global-variable-declaration.m` to the revised 
diagnostic message.
• Removed NS_ENUM and NSInteger from the additions to 
`test/clang-tidy/google-objc-global-variable-declaration.m` because their 
declarations were missing and I assumed that replication of the Foundation 
macro and type alias was not meaningfully important to this test case.

I verified the changes with the following commands:

  $ ${LLVM_ROOT}/tools/clang/tools/extra/test/clang-tidy/check_clang_tidy.py 
${LLVM_ROOT}/tools/clang/tools/extra/test/clang-tidy/google-objc-global-variable-declaration.m
 google-objc-global-variable-declaration /tmp/test && make check-all

Is that satisfactory verification of these changes?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D43581

Files:
  clang-tidy/google/GlobalVariableDeclarationCheck.cpp
  test/clang-tidy/google-objc-global-variable-declaration.m


Index: test/clang-tidy/google-objc-global-variable-declaration.m
===
--- test/clang-tidy/google-objc-global-variable-declaration.m
+++ test/clang-tidy/google-objc-global-variable-declaration.m
@@ -2,7 +2,7 @@
 
 @class NSString;
 static NSString* const myConstString = @"hello";
-// CHECK-MESSAGES: :[[@LINE-1]]:24: warning: const global variable 
'myConstString' must have a name which starts with 'k[A-Z]' 
[google-objc-global-variable-declaration]
+// CHECK-MESSAGES: :[[@LINE-1]]:24: warning: const global variable 
'myConstString' must have a name which starts with an appropriate prefix 
matching '(k[A-Z]|[A-Z]{2,})' [google-objc-global-variable-declaration]
 // CHECK-FIXES: static NSString* const kMyConstString = @"hello";
 
 static NSString* MyString = @"hi";
@@ -22,16 +22,24 @@
 // CHECK-FIXES: static NSString* gNoDef;
 
 static NSString* const _notAlpha = @"NotBeginWithAlpha";
-// CHECK-MESSAGES: :[[@LINE-1]]:24: warning: const global variable '_notAlpha' 
must have a name which starts with 'k[A-Z]' 
[google-objc-global-variable-declaration]
+// CHECK-MESSAGES: :[[@LINE-1]]:24: warning: const global variable '_notAlpha' 
must have a name which starts with an appropriate prefix matching 
'(k[A-Z]|[A-Z]{2,})' [google-objc-global-variable-declaration]
 // CHECK-FIXES: static NSString* const _notAlpha = @"NotBeginWithAlpha";
 
 static NSString* const k_Alpha = @"SecondNotAlpha";
-// CHECK-MESSAGES: :[[@LINE-1]]:24: warning: const global variable 'k_Alpha' 
must have a name which starts with 'k[A-Z]' 
[google-objc-global-variable-declaration]
+// CHECK-MESSAGES: :[[@LINE-1]]:24: warning: const global variable 'k_Alpha' 
must have a name which starts with an appropriate prefix matching 
'(k[A-Z]|[A-Z]{2,})' [google-objc-global-variable-declaration]
 // CHECK-FIXES: static NSString* const k_Alpha = @"SecondNotAlpha";
 
 static NSString* const kGood = @"hello";
+static NSString* const XYGood = @"hello";
 static NSString* gMyIntGood = 0;
 
+extern NSString* const GTLServiceErrorDomain;
+
+enum GTLServiceError {
+  GTLServiceErrorQueryResultMissing = -3000,
+  GTLServiceErrorWaitTimedOut   = -3001,
+};
+
 @implementation Foo
 - (void)f {
 int x = 0;
Index: clang-tidy/google/GlobalVariableDeclarationCheck.cpp
===
--- clang-tidy/google/GlobalVariableDeclarationCheck.cpp
+++ clang-tidy/google/GlobalVariableDeclarationCheck.cpp
@@ -72,7 +72,7 @@
   this);
   Finder->addMatcher(varDecl(hasGlobalStorage(), hasType(isConstQualified()),
  unless(isLocalVariable()),
- unless(matchesName("::k[A-Z]")))
+ unless(matchesName("::(k[A-Z]|[A-Z]{2,})")))
  .bind("global_const"),
  this);
 }
@@ -88,7 +88,7 @@
   if (const auto *Decl = Result.Nodes.getNodeAs("global_const")) {
 diag(Decl->getLocation(),
  "const global variable '%0' must have a name which starts with "
- "'k[A-Z]'")
+ "an appropriate prefix matching '(k[A-Z]|[A-Z]{2,})'")
 << Decl->getName() << generateFixItHint(Decl, true);
   }
 }


Index: test/clang-tidy/google-objc-global-variable-declaration.m
===
--- test/clang-tidy/google-objc-global-variable-declaration.m
+++ test/clang-tidy/google-objc-global-variable-declaration.m
@@ -2,7 +2,7 @@
 
 @class NSString;
 static NSString* const myConstString = @"hello";
-// CHECK-MESSAGES: :[[@L

[PATCH] D43581: [clang-tidy/google] Improve the Objective-C global variable declaration check 🔧

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore added inline comments.



Comment at: clang-tidy/google/GlobalVariableDeclarationCheck.cpp:92
+ "an appropriate prefix (see "
+ "http://google.github.io/styleguide/objcguide#constants).")
 << Decl->getName() << generateFixItHint(Decl, true);

Wizard wrote:
> aaron.ballman wrote:
> > We don't usually put hyperlinks in the diagnostic messages, so please 
> > remove this.
> > 
> > My suggestion about describing what constitutes an appropriate prefix was 
> > with regards to the style guide wording itself. For instance, that document 
> > doesn't mention that two capital letters is good. That's not on you to fix 
> > before this patch goes in, of course.
> Btw it is actually "2 or more" characters for prefix. I think it makes sense 
> because we need at least 2 or more characters to call it a "prefix" :-)
Reverted the inclusion of the hyperlink in the message. I agree that embedding 
the hyperlink in the message is indirect and inconsistent with other diagnostic 
messages in LLVM projecta.



Comment at: clang-tidy/google/GlobalVariableDeclarationCheck.cpp:92
+ "an appropriate prefix (see "
+ "http://google.github.io/styleguide/objcguide#constants).")
 << Decl->getName() << generateFixItHint(Decl, true);

stephanemoore wrote:
> Wizard wrote:
> > aaron.ballman wrote:
> > > We don't usually put hyperlinks in the diagnostic messages, so please 
> > > remove this.
> > > 
> > > My suggestion about describing what constitutes an appropriate prefix was 
> > > with regards to the style guide wording itself. For instance, that 
> > > document doesn't mention that two capital letters is good. That's not on 
> > > you to fix before this patch goes in, of course.
> > Btw it is actually "2 or more" characters for prefix. I think it makes 
> > sense because we need at least 2 or more characters to call it a "prefix" 
> > :-)
> Reverted the inclusion of the hyperlink in the message. I agree that 
> embedding the hyperlink in the message is indirect and inconsistent with 
> other diagnostic messages in LLVM projecta.
§1 Regarding Prefixes And Character Count

Strictly speaking, a single character prefix (e.g., `extern const int ALimit;` 
❌) is possible but not allowed for constants in Google Objective-C style except 
for lowercase 'k' as a prefix for file scope constants (e.g., `static const int 
kLimit = 3;` ✔️). As hinted in the commit description, Google currently 
enforces a minimum two character prefix (e.g., `extern const int ABPrefix;` ✔️) 
or exceptionally 'k' where appropriate.

Technically this modified regex allows authored code to include inadvisable 
constant names with a single character prefix (e.g., `extern const int 
APrefix;` ❌). In this respect I would say that this change eliminates an 
important category of false positives (constants prefixed with '[A-Z]{2,}') 
while introducing a less important category of false negatives (constants 
prefixed with only '[A-Z]'). The false positives are observed in standard 
recommended code while the false negatives occur in non-standard unrecommended 
code. Without a reliable mechanism to separate the constant prefix from the 
constant name (e.g., splitting "FOOURL" into "FOO" and "URL" or "HTTP2Header" 
into "HTTP2" and "Header"), I cannot immediately think of a way to eliminate 
the introduced category of false negatives without introducing other false 
positives. I intend to continue thinking about alternative methods that might 
eliminate these false negatives without introducing other compromises. If it is 
acceptable to the reviewers I would propose to move forward with this proposed 
change to eliminate the observed false positives and then consider addressing 
the false negatives in a followup.

§2 Regarding the Google Objective-C Style Guide's Description of Appropriate 
Prefixes

I agree that the Google Objective-C style guide should be clearer on this topic 
and will pursue clarifying this promptly.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D43581



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

[PATCH] D43581: [clang-tidy/google] Improve the Objective-C global variable declaration check 🔧

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore marked an inline comment as not done.
stephanemoore added inline comments.



Comment at: test/clang-tidy/google-objc-global-variable-declaration.m:33
 static NSString* const kGood = @"hello";
+static NSString* const XYGood = @"hello";
 static NSString* gMyIntGood = 0;

aaron.ballman wrote:
> Can you also add the code from the style guide as a test case?
> ```
> extern NSString *const GTLServiceErrorDomain;
> 
> typedef NS_ENUM(NSInteger, GTLServiceError) {
>   GTLServiceErrorQueryResultMissing = -3000,
>   GTLServiceErrorWaitTimedOut   = -3001,
> };
> ```
NS_ENUM and NSInteger are not defined in this implementation file.

I can either (1) Omit the macro and the type alias; or (2) reasonably replicate 
the macro and type alias in this source file.

Which option is preferred? For the time being, I have pursued option (1) on the 
assumption that NS_ENUM and NSInteger are not critical to this test case.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D43581



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

[PATCH] D43581: [clang-tidy/google] Improve the Objective-C global variable declaration check 🔧

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore updated this revision to Diff 135745.
stephanemoore marked 3 inline comments as done.
stephanemoore added a comment.

Removed the regex from the diagnostic ✂️


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D43581

Files:
  clang-tidy/google/GlobalVariableDeclarationCheck.cpp
  test/clang-tidy/google-objc-global-variable-declaration.m


Index: test/clang-tidy/google-objc-global-variable-declaration.m
===
--- test/clang-tidy/google-objc-global-variable-declaration.m
+++ test/clang-tidy/google-objc-global-variable-declaration.m
@@ -2,7 +2,7 @@
 
 @class NSString;
 static NSString* const myConstString = @"hello";
-// CHECK-MESSAGES: :[[@LINE-1]]:24: warning: const global variable 
'myConstString' must have a name which starts with 'k[A-Z]' 
[google-objc-global-variable-declaration]
+// CHECK-MESSAGES: :[[@LINE-1]]:24: warning: const global variable 
'myConstString' must have a name which starts with an appropriate prefix 
[google-objc-global-variable-declaration]
 // CHECK-FIXES: static NSString* const kMyConstString = @"hello";
 
 static NSString* MyString = @"hi";
@@ -22,16 +22,24 @@
 // CHECK-FIXES: static NSString* gNoDef;
 
 static NSString* const _notAlpha = @"NotBeginWithAlpha";
-// CHECK-MESSAGES: :[[@LINE-1]]:24: warning: const global variable '_notAlpha' 
must have a name which starts with 'k[A-Z]' 
[google-objc-global-variable-declaration]
+// CHECK-MESSAGES: :[[@LINE-1]]:24: warning: const global variable '_notAlpha' 
must have a name which starts with an appropriate prefix 
[google-objc-global-variable-declaration]
 // CHECK-FIXES: static NSString* const _notAlpha = @"NotBeginWithAlpha";
 
 static NSString* const k_Alpha = @"SecondNotAlpha";
-// CHECK-MESSAGES: :[[@LINE-1]]:24: warning: const global variable 'k_Alpha' 
must have a name which starts with 'k[A-Z]' 
[google-objc-global-variable-declaration]
+// CHECK-MESSAGES: :[[@LINE-1]]:24: warning: const global variable 'k_Alpha' 
must have a name which starts with an appropriate prefix 
[google-objc-global-variable-declaration]
 // CHECK-FIXES: static NSString* const k_Alpha = @"SecondNotAlpha";
 
 static NSString* const kGood = @"hello";
+static NSString* const XYGood = @"hello";
 static NSString* gMyIntGood = 0;
 
+extern NSString* const GTLServiceErrorDomain;
+
+enum GTLServiceError {
+  GTLServiceErrorQueryResultMissing = -3000,
+  GTLServiceErrorWaitTimedOut   = -3001,
+};
+
 @implementation Foo
 - (void)f {
 int x = 0;
Index: clang-tidy/google/GlobalVariableDeclarationCheck.cpp
===
--- clang-tidy/google/GlobalVariableDeclarationCheck.cpp
+++ clang-tidy/google/GlobalVariableDeclarationCheck.cpp
@@ -72,7 +72,7 @@
   this);
   Finder->addMatcher(varDecl(hasGlobalStorage(), hasType(isConstQualified()),
  unless(isLocalVariable()),
- unless(matchesName("::k[A-Z]")))
+ unless(matchesName("::(k[A-Z]|[A-Z]{2,})")))
  .bind("global_const"),
  this);
 }
@@ -88,7 +88,7 @@
   if (const auto *Decl = Result.Nodes.getNodeAs("global_const")) {
 diag(Decl->getLocation(),
  "const global variable '%0' must have a name which starts with "
- "'k[A-Z]'")
+ "an appropriate prefix")
 << Decl->getName() << generateFixItHint(Decl, true);
   }
 }


Index: test/clang-tidy/google-objc-global-variable-declaration.m
===
--- test/clang-tidy/google-objc-global-variable-declaration.m
+++ test/clang-tidy/google-objc-global-variable-declaration.m
@@ -2,7 +2,7 @@
 
 @class NSString;
 static NSString* const myConstString = @"hello";
-// CHECK-MESSAGES: :[[@LINE-1]]:24: warning: const global variable 'myConstString' must have a name which starts with 'k[A-Z]' [google-objc-global-variable-declaration]
+// CHECK-MESSAGES: :[[@LINE-1]]:24: warning: const global variable 'myConstString' must have a name which starts with an appropriate prefix [google-objc-global-variable-declaration]
 // CHECK-FIXES: static NSString* const kMyConstString = @"hello";
 
 static NSString* MyString = @"hi";
@@ -22,16 +22,24 @@
 // CHECK-FIXES: static NSString* gNoDef;
 
 static NSString* const _notAlpha = @"NotBeginWithAlpha";
-// CHECK-MESSAGES: :[[@LINE-1]]:24: warning: const global variable '_notAlpha' must have a name which starts with 'k[A-Z]' [google-objc-global-variable-declaration]
+// CHECK-MESSAGES: :[[@LINE-1]]:24: warning: const global variable '_notAlpha' must have a name which starts with an appropriate prefix [google-objc-global-variable-declaration]
 // CHECK-FIXES: static NSString* const _notAlpha = @"NotBeginWithAlpha";
 
 static NSString* const k_Alpha = @"SecondNotAlpha";
-// CHECK-MESSAGES: :[[@LINE-1]]:24: warning: const global variable 'k_Alpha' must have a name which starts with 'k[A-Z

[PATCH] D43581: [clang-tidy/google] Improve the Objective-C global variable declaration check 🔧

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore marked 2 inline comments as done.
stephanemoore added inline comments.



Comment at: clang-tidy/google/GlobalVariableDeclarationCheck.cpp:92
+ "an appropriate prefix (see "
+ "http://google.github.io/styleguide/objcguide#constants).")
 << Decl->getName() << generateFixItHint(Decl, true);

aaron.ballman wrote:
> stephanemoore wrote:
> > stephanemoore wrote:
> > > Wizard wrote:
> > > > aaron.ballman wrote:
> > > > > We don't usually put hyperlinks in the diagnostic messages, so please 
> > > > > remove this.
> > > > > 
> > > > > My suggestion about describing what constitutes an appropriate prefix 
> > > > > was with regards to the style guide wording itself. For instance, 
> > > > > that document doesn't mention that two capital letters is good. 
> > > > > That's not on you to fix before this patch goes in, of course.
> > > > Btw it is actually "2 or more" characters for prefix. I think it makes 
> > > > sense because we need at least 2 or more characters to call it a 
> > > > "prefix" :-)
> > > Reverted the inclusion of the hyperlink in the message. I agree that 
> > > embedding the hyperlink in the message is indirect and inconsistent with 
> > > other diagnostic messages in LLVM projecta.
> > §1 Regarding Prefixes And Character Count
> > 
> > Strictly speaking, a single character prefix (e.g., `extern const int 
> > ALimit;` ❌) is possible but not allowed for constants in Google Objective-C 
> > style except for lowercase 'k' as a prefix for file scope constants (e.g., 
> > `static const int kLimit = 3;` ✔️). As hinted in the commit description, 
> > Google currently enforces a minimum two character prefix (e.g., `extern 
> > const int ABPrefix;` ✔️) or exceptionally 'k' where appropriate.
> > 
> > Technically this modified regex allows authored code to include inadvisable 
> > constant names with a single character prefix (e.g., `extern const int 
> > APrefix;` ❌). In this respect I would say that this change eliminates an 
> > important category of false positives (constants prefixed with '[A-Z]{2,}') 
> > while introducing a less important category of false negatives (constants 
> > prefixed with only '[A-Z]'). The false positives are observed in standard 
> > recommended code while the false negatives occur in non-standard 
> > unrecommended code. Without a reliable mechanism to separate the constant 
> > prefix from the constant name (e.g., splitting "FOOURL" into "FOO" and 
> > "URL" or "HTTP2Header" into "HTTP2" and "Header"), I cannot immediately 
> > think of a way to eliminate the introduced category of false negatives 
> > without introducing other false positives. I intend to continue thinking 
> > about alternative methods that might eliminate these false negatives 
> > without introducing other compromises. If it is acceptable to the reviewers 
> > I would propose to move forward with this proposed change to eliminate the 
> > observed false positives and then consider addressing the false negatives 
> > in a followup.
> > 
> > §2 Regarding the Google Objective-C Style Guide's Description of 
> > Appropriate Prefixes
> > 
> > I agree that the Google Objective-C style guide should be clearer on this 
> > topic and will pursue clarifying this promptly.
> I don't think we should have the regex as part of the diagnostic either -- 
> it's a distraction and it's not something we typically do. While it's nice 
> for the diagnostic to explain how to fix the issue, the critical bit is 
> diagnosing what's wrong with the code. The diagnostic without the regex does 
> that and users have a place to find that information (the style guide docs).
Sounds good to me 👌 Removed the regex from the diagnostic.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D43581



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

[PATCH] D43581: [clang-tidy/google] Improve the Objective-C global variable declaration check 🔧

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore marked an inline comment as done.
stephanemoore added a comment.

In https://reviews.llvm.org/D43581#1018499, @aaron.ballman wrote:

> This LGTM! Do you need someone to commit on your behalf?


I would be happy to commit assuming that I am able to and can meet submission 
requirements.

I see a "Submit" button in Phabricator that I assume will land the commit if I 
press it?

I found some submission guidelines in the LLVM Developer Policy 
. Are there any other submission 
guidelines I should follow?

I ran the LLVM and Clang regression tests (by running `make check-all` from my 
LLVM build directory) and I encountered a failure in "Bindings/Go/go.test":

   TEST 'LLVM :: Bindings/Go/go.test' FAILED 

  Script:
  --
  /Users/mog/projects/llvm-build/bin/llvm-go go=/usr/local/go/bin/go test 
llvm.org/llvm/bindings/go/llvm
  --
  Exit Code: 1
  
  Command Output (stdout):
  --
  FAIL  llvm.org/llvm/bindings/go/llvm [build failed]
  
  --
  Command Output (stderr):
  --
  # llvm.org/llvm/bindings/go/llvm
  In file included from 
/var/folders/qh/4y215hgd4zqg30v9q04czw58005k3k/T/lit_tmp_7sZYWR/gopath735545420/src/llvm.org/llvm/bindings/go/llvm/analysis.go:17:
  In file included from /Users/mog/projects/llvm/include/llvm-c/Analysis.h:22:
  In file included from /Users/mog/projects/llvm/include/llvm-c/Types.h:17:
  /Users/mog/projects/llvm-build/include/llvm/Support/DataTypes.h:35:10: fatal 
error: 'math.h' file not found
  #include 
   ^~~~
  1 error generated.
  
  --
  
  

I reran these tests on a clean checkout and I still encountered the failure so 
I presume that this failure is unrelated?

I have also been trying to run the `test-suite` but I have been encountering 
Python exceptions while trying to run `lnt` 😕.

If someone wants to submit on my behalf that works for me. If not, I can also 
continue trying to drive this myself (though verification of submission 
requirements would be helpful in that case).


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D43581



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

[PATCH] D47393: [clang-format] Disable AlwaysBreakBeforeMultilineStrings in Google style for Objective-C 📜

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore updated this revision to Diff 149338.
stephanemoore added a comment.

Removed the last format from the tests. That particular scenario was failing 
and it might need additional changes to pass. I also think that particular 
scenario is not critical to the change and can be considered in a followup.


Repository:
  rC Clang

https://reviews.llvm.org/D47393

Files:
  lib/Format/Format.cpp
  unittests/Format/FormatTestObjC.cpp


Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -1193,6 +1193,17 @@
"}");
 }
 
+TEST_F(FormatTestObjC, AlwaysBreakBeforeMultilineStrings) {
+  Style = getGoogleStyle(FormatStyle::LK_ObjC);
+  Style.ColumnLimit = 40;
+  verifyFormat(" = @\"\"\n"
+   "   @\"\";");
+  verifyFormat("(@\"\"\n"
+   " @\"\");");
+  verifyFormat("(qqq, @\"\"\n"
+   "  @\"\");");
+}
+
 } // end namespace
 } // end namespace format
 } // end namespace clang
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -799,6 +799,7 @@
 // has been implemented.
 GoogleStyle.BreakStringLiterals = false;
   } else if (Language == FormatStyle::LK_ObjC) {
+GoogleStyle.AlwaysBreakBeforeMultilineStrings = false;
 GoogleStyle.ColumnLimit = 100;
   }
 


Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -1193,6 +1193,17 @@
"}");
 }
 
+TEST_F(FormatTestObjC, AlwaysBreakBeforeMultilineStrings) {
+  Style = getGoogleStyle(FormatStyle::LK_ObjC);
+  Style.ColumnLimit = 40;
+  verifyFormat(" = @\"\"\n"
+   "   @\"\";");
+  verifyFormat("(@\"\"\n"
+   " @\"\");");
+  verifyFormat("(qqq, @\"\"\n"
+   "  @\"\");");
+}
+
 } // end namespace
 } // end namespace format
 } // end namespace clang
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -799,6 +799,7 @@
 // has been implemented.
 GoogleStyle.BreakStringLiterals = false;
   } else if (Language == FormatStyle::LK_ObjC) {
+GoogleStyle.AlwaysBreakBeforeMultilineStrings = false;
 GoogleStyle.ColumnLimit = 100;
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47393: [clang-format] Disable AlwaysBreakBeforeMultilineStrings in Google style for Objective-C 📜

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore added a comment.

Polling the Google Objective-C community to see if anyone objects to this 
change. So far no concerns have been raised.


Repository:
  rC Clang

https://reviews.llvm.org/D47393



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

[PATCH] D47393: [clang-format] Disable AlwaysBreakBeforeMultilineStrings in Google style for Objective-C 📜

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore added reviewers: benhamilton, jolesiak, djasper.
stephanemoore added a comment.

I believe I have consensus that this is the correct change for Google 
Objective-C style.


Repository:
  rC Clang

https://reviews.llvm.org/D47393



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

[PATCH] D46922: [checks/property-decls] Fix comment in clang-tidy/objc/PropertyDeclarationCheck.cpp ✍️

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore added a comment.

Is it possible to get someone to land this for me? I don't believe I have 
access to land it myself.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D46922



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

[PATCH] D46922: [checks/property-decls] Fix comment in clang-tidy/objc/PropertyDeclarationCheck.cpp ✍️

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore added a comment.

In https://reviews.llvm.org/D46922#1125689, @benhamilton wrote:

> > Is it possible to get someone to land this for me? I don't believe I have 
> > access to land it myself.
>
> Done.


Thank you, kind sir 🙇


Repository:
  rL LLVM

https://reviews.llvm.org/D46922



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

[PATCH] D47393: [clang-format] Disable AlwaysBreakBeforeMultilineStrings in Google style for Objective-C 📜

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore added a comment.

Are additional approvals needed to land this? I pinged jolesiak earlier this 
week asking if he had any comments but have not heard back yet.

(also, once necessary approvals have been granted can I request help from 
someone to land it for me? 😇)


Repository:
  rC Clang

https://reviews.llvm.org/D47393



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

[PATCH] D48432: [clang-format] Add AlwaysBreakBeforeMultilineString tests

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore accepted this revision.
stephanemoore added a comment.
This revision is now accepted and ready to land.

Thanks!


Repository:
  rC Clang

https://reviews.llvm.org/D48432



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

[PATCH] D56945: [clang-tidy] Delete obsolete objc-property-declaration options ✂️

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore created this revision.
stephanemoore added reviewers: benhamilton, aaron.ballman.
Herald added subscribers: cfe-commits, xazax.hun.

The `Acronyms` and `IncludeDefaultAcronyms` options were deprecated in
https://reviews.llvm.org/D51832. These options can be removed.

Tested by running the clang-tidy tests.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D56945

Files:
  clang-tidy/objc/PropertyDeclarationCheck.cpp
  clang-tidy/objc/PropertyDeclarationCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/objc-property-declaration.rst


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
@@ -40,15 +40,3 @@
@property(nonatomic, assign) int abc_lowerCamelCase;
 
 The corresponding style rule: 
https://developer.apple.com/library/content/qa/qa1908/_index.html
-
-
-Options

-
-.. option:: Acronyms
-
-   This option is deprecated and ignored.
-
-.. option:: IncludeDefaultAcronyms
-
-   This option is deprecated and ignored.
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -245,6 +245,10 @@
   suffix, and suggests to make the suffix uppercase. The list of destination
   suffixes can be optionally provided.
 
+- The `Acronyms` and `IncludeDefaultAcronyms` options for the
+  :doc:`objc-property-declaration`
+  check have been removed.
+
 - New alias :doc:`cert-dcl16-c
   ` to 
:doc:`readability-uppercase-literal-suffix
   `
Index: clang-tidy/objc/PropertyDeclarationCheck.h
===
--- clang-tidy/objc/PropertyDeclarationCheck.h
+++ clang-tidy/objc/PropertyDeclarationCheck.h
@@ -11,8 +11,6 @@
 #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_OBJC_PROPERTY_DECLARATION_H
 
 #include "../ClangTidy.h"
-#include 
-#include 
 
 namespace clang {
 namespace tidy {
@@ -28,15 +26,10 @@
 /// 
http://clang.llvm.org/extra/clang-tidy/checks/objc-property-declaration.html
 class PropertyDeclarationCheck : public ClangTidyCheck {
 public:
-  PropertyDeclarationCheck(StringRef Name, ClangTidyContext *Context);
+  PropertyDeclarationCheck(StringRef Name, ClangTidyContext *Context)
+  : ClangTidyCheck(Name, 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;
-  const bool IncludeDefaultAcronyms;
-  std::vector EscapedAcronyms;
 };
 
 } // namespace objc
Index: clang-tidy/objc/PropertyDeclarationCheck.cpp
===
--- clang-tidy/objc/PropertyDeclarationCheck.cpp
+++ clang-tidy/objc/PropertyDeclarationCheck.cpp
@@ -98,14 +98,6 @@
 }
 }  // namespace
 
-PropertyDeclarationCheck::PropertyDeclarationCheck(StringRef Name,
-   ClangTidyContext *Context)
-: ClangTidyCheck(Name, Context),
-  SpecialAcronyms(
-  utils::options::parseStringList(Options.get("Acronyms", ""))),
-  IncludeDefaultAcronyms(Options.get("IncludeDefaultAcronyms", true)),
-  EscapedAcronyms() {}
-
 void PropertyDeclarationCheck::registerMatchers(MatchFinder *Finder) {
   // this check should only be applied to ObjC sources.
   if (!getLangOpts().ObjC) return;
@@ -146,12 +138,6 @@
   << generateFixItHint(MatchedDecl, StandardProperty);
 }
 
-void PropertyDeclarationCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) 
{
-  Options.store(Opts, "Acronyms",
-utils::options::serializeStringList(SpecialAcronyms));
-  Options.store(Opts, "IncludeDefaultAcronyms", IncludeDefaultAcronyms);
-}
-
 }  // namespace objc
 }  // namespace tidy
 }  // namespace clang


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
@@ -40,15 +40,3 @@
@property(nonatomic, assign) int abc_lowerCamelCase;
 
 The corresponding style rule: https://developer.apple.com/library/content/qa/qa1908/_index.html
-
-
-Options

-
-.. option:: Acronyms
-
-   This option is deprecated and ignored.
-
-.. option:: IncludeDefaultAcronyms
-
-   This option is deprecated and ignored.
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -245,6 +245,10 @@
   suffix, and suggests to make the suffix uppercase. The list of destination
   suffixes can be optionally provided.
 
+- The `Acronyms` and `IncludeDefaultAcronyms` options for the
+  :doc:`objc-property-declaratio

[PATCH] D56945: [clang-tidy] Delete obsolete objc-property-declaration options ✂️

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore updated this revision to Diff 182660.
stephanemoore marked 2 inline comments as done.
stephanemoore added a comment.

Use :option: prefix for options in release notes.
Add missing space in check reference in release notes.


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56945/new/

https://reviews.llvm.org/D56945

Files:
  clang-tidy/objc/PropertyDeclarationCheck.cpp
  clang-tidy/objc/PropertyDeclarationCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/objc-property-declaration.rst


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
@@ -40,15 +40,3 @@
@property(nonatomic, assign) int abc_lowerCamelCase;
 
 The corresponding style rule: 
https://developer.apple.com/library/content/qa/qa1908/_index.html
-
-
-Options

-
-.. option:: Acronyms
-
-   This option is deprecated and ignored.
-
-.. option:: IncludeDefaultAcronyms
-
-   This option is deprecated and ignored.
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -73,6 +73,10 @@
   Checks for casts of ``absl::Duration`` conversion functions, and recommends
   the right conversion function instead.
 
+- The :option:`Acronyms` and :option:`IncludeDefaultAcronyms` options for the
+  :doc:`objc-property-declaration 
`
+  check have been removed.
+
 Improvements to include-fixer
 -
 
Index: clang-tidy/objc/PropertyDeclarationCheck.h
===
--- clang-tidy/objc/PropertyDeclarationCheck.h
+++ clang-tidy/objc/PropertyDeclarationCheck.h
@@ -11,8 +11,6 @@
 #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_OBJC_PROPERTY_DECLARATION_H
 
 #include "../ClangTidy.h"
-#include 
-#include 
 
 namespace clang {
 namespace tidy {
@@ -28,15 +26,10 @@
 /// 
http://clang.llvm.org/extra/clang-tidy/checks/objc-property-declaration.html
 class PropertyDeclarationCheck : public ClangTidyCheck {
 public:
-  PropertyDeclarationCheck(StringRef Name, ClangTidyContext *Context);
+  PropertyDeclarationCheck(StringRef Name, ClangTidyContext *Context)
+  : ClangTidyCheck(Name, 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;
-  const bool IncludeDefaultAcronyms;
-  std::vector EscapedAcronyms;
 };
 
 } // namespace objc
Index: clang-tidy/objc/PropertyDeclarationCheck.cpp
===
--- clang-tidy/objc/PropertyDeclarationCheck.cpp
+++ clang-tidy/objc/PropertyDeclarationCheck.cpp
@@ -98,14 +98,6 @@
 }
 }  // namespace
 
-PropertyDeclarationCheck::PropertyDeclarationCheck(StringRef Name,
-   ClangTidyContext *Context)
-: ClangTidyCheck(Name, Context),
-  SpecialAcronyms(
-  utils::options::parseStringList(Options.get("Acronyms", ""))),
-  IncludeDefaultAcronyms(Options.get("IncludeDefaultAcronyms", true)),
-  EscapedAcronyms() {}
-
 void PropertyDeclarationCheck::registerMatchers(MatchFinder *Finder) {
   // this check should only be applied to ObjC sources.
   if (!getLangOpts().ObjC) return;
@@ -146,12 +138,6 @@
   << generateFixItHint(MatchedDecl, StandardProperty);
 }
 
-void PropertyDeclarationCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) 
{
-  Options.store(Opts, "Acronyms",
-utils::options::serializeStringList(SpecialAcronyms));
-  Options.store(Opts, "IncludeDefaultAcronyms", IncludeDefaultAcronyms);
-}
-
 }  // namespace objc
 }  // namespace tidy
 }  // namespace clang


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
@@ -40,15 +40,3 @@
@property(nonatomic, assign) int abc_lowerCamelCase;
 
 The corresponding style rule: https://developer.apple.com/library/content/qa/qa1908/_index.html
-
-
-Options

-
-.. option:: Acronyms
-
-   This option is deprecated and ignored.
-
-.. option:: IncludeDefaultAcronyms
-
-   This option is deprecated and ignored.
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -73,6 +73,10 @@
   Checks for casts of ``absl::Duration`` conversion functions, and recommends
   the right conversion function instead.
 
+- The :option:`Acronyms` and :option:`IncludeDefaultAcronyms` options for the
+  :doc:`objc-property-declaration `
+  c

[PATCH] D56945: [clang-tidy] Delete obsolete objc-property-declaration options ✂️

[Stephane Moore via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL351921: [clang-tidy] Delete obsolete 
objc-property-declaration options ✂️ (authored by stephanemoore, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D56945?vs=182660&id=183015#toc

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56945/new/

https://reviews.llvm.org/D56945

Files:
  clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.cpp
  clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.h
  clang-tools-extra/trunk/docs/ReleaseNotes.rst
  clang-tools-extra/trunk/docs/clang-tidy/checks/objc-property-declaration.rst


Index: clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.h
===
--- clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.h
+++ clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.h
@@ -10,8 +10,6 @@
 #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_OBJC_PROPERTY_DECLARATION_H
 
 #include "../ClangTidy.h"
-#include 
-#include 
 
 namespace clang {
 namespace tidy {
@@ -27,15 +25,10 @@
 /// 
http://clang.llvm.org/extra/clang-tidy/checks/objc-property-declaration.html
 class PropertyDeclarationCheck : public ClangTidyCheck {
 public:
-  PropertyDeclarationCheck(StringRef Name, ClangTidyContext *Context);
+  PropertyDeclarationCheck(StringRef Name, ClangTidyContext *Context)
+  : ClangTidyCheck(Name, 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;
-  const bool IncludeDefaultAcronyms;
-  std::vector EscapedAcronyms;
 };
 
 } // namespace objc
Index: clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.cpp
@@ -97,14 +97,6 @@
 }
 }  // namespace
 
-PropertyDeclarationCheck::PropertyDeclarationCheck(StringRef Name,
-   ClangTidyContext *Context)
-: ClangTidyCheck(Name, Context),
-  SpecialAcronyms(
-  utils::options::parseStringList(Options.get("Acronyms", ""))),
-  IncludeDefaultAcronyms(Options.get("IncludeDefaultAcronyms", true)),
-  EscapedAcronyms() {}
-
 void PropertyDeclarationCheck::registerMatchers(MatchFinder *Finder) {
   // this check should only be applied to ObjC sources.
   if (!getLangOpts().ObjC) return;
@@ -145,12 +137,6 @@
   << generateFixItHint(MatchedDecl, StandardProperty);
 }
 
-void PropertyDeclarationCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) 
{
-  Options.store(Opts, "Acronyms",
-utils::options::serializeStringList(SpecialAcronyms));
-  Options.store(Opts, "IncludeDefaultAcronyms", IncludeDefaultAcronyms);
-}
-
 }  // namespace objc
 }  // namespace tidy
 }  // namespace clang
Index: clang-tools-extra/trunk/docs/ReleaseNotes.rst
===
--- clang-tools-extra/trunk/docs/ReleaseNotes.rst
+++ clang-tools-extra/trunk/docs/ReleaseNotes.rst
@@ -73,6 +73,10 @@
   Checks for casts of ``absl::Duration`` conversion functions, and recommends
   the right conversion function instead.
 
+- The :option:`Acronyms` and :option:`IncludeDefaultAcronyms` options for the
+  :doc:`objc-property-declaration 
`
+  check have been removed.
+
 Improvements to include-fixer
 -
 
Index: 
clang-tools-extra/trunk/docs/clang-tidy/checks/objc-property-declaration.rst
===
--- clang-tools-extra/trunk/docs/clang-tidy/checks/objc-property-declaration.rst
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/objc-property-declaration.rst
@@ -40,15 +40,3 @@
@property(nonatomic, assign) int abc_lowerCamelCase;
 
 The corresponding style rule: 
https://developer.apple.com/library/content/qa/qa1908/_index.html
-
-
-Options

-
-.. option:: Acronyms
-
-   This option is deprecated and ignored.
-
-.. option:: IncludeDefaultAcronyms
-
-   This option is deprecated and ignored.


Index: clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.h
===
--- clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.h
+++ clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.h
@@ -10,8 +10,6 @@
 #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_OBJC_PROPERTY_DECLARATION_H
 
 #include "../ClangTidy.h"
-#include 
-#include 
 
 namespace clang {
 namespace tidy {
@@ -27,15 +25,10 @@
 /// http://clang.llvm.org/extra/clang-tidy/checks

[PATCH] D56945: [clang-tidy] Delete obsolete objc-property-declaration options ✂️

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore marked an inline comment as done.
stephanemoore added inline comments.



Comment at: docs/ReleaseNotes.rst:248
 
+- The `Acronyms` and `IncludeDefaultAcronyms` options for the
+  :doc:`objc-property-declaration`

Eugene.Zelenko wrote:
> Please rebase from trunk and use :option: prefix for options..
Could using the :option: prefix cause issues if I am removing the option?

The documentation generation seems to emit a warning for an unknown option:
"llvm/src/tools/clang/tools/extra/docs/ReleaseNotes.rst:76: WARNING: unknown 
option: Acronyms"
http://lab.llvm.org:8011/builders/clang-tools-sphinx-docs/builds/36425/steps/docs-clang-tools-html/logs/stdio


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56945/new/

https://reviews.llvm.org/D56945



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

[PATCH] D57080: [clang-tidy] Delete obsolete objc-property-declaration options ✂️

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore created this revision.
Herald added subscribers: cfe-commits, xazax.hun.
stephanemoore edited the summary of this revision.

The Acronyms and IncludeDefaultAcronyms options were deprecated in
https://reviews.llvm.org/D51832. These options can be removed.

Tested by running the clang-tidy tests.

This is an amended resubmission of https://reviews.llvm.org/D56945.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D57080

Files:
  clang-tidy/objc/PropertyDeclarationCheck.cpp
  clang-tidy/objc/PropertyDeclarationCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/objc-property-declaration.rst


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
@@ -40,15 +40,3 @@
@property(nonatomic, assign) int abc_lowerCamelCase;
 
 The corresponding style rule: 
https://developer.apple.com/library/content/qa/qa1908/_index.html
-
-
-Options

-
-.. option:: Acronyms
-
-   This option is deprecated and ignored.
-
-.. option:: IncludeDefaultAcronyms
-
-   This option is deprecated and ignored.
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -73,6 +73,10 @@
   Checks for casts of ``absl::Duration`` conversion functions, and recommends
   the right conversion function instead.
 
+- The `Acronyms` and `IncludeDefaultAcronyms` options for the
+  :doc:`objc-property-declaration 
`
+  check have been removed.
+
 Improvements to include-fixer
 -
 
Index: clang-tidy/objc/PropertyDeclarationCheck.h
===
--- clang-tidy/objc/PropertyDeclarationCheck.h
+++ clang-tidy/objc/PropertyDeclarationCheck.h
@@ -10,8 +10,6 @@
 #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_OBJC_PROPERTY_DECLARATION_H
 
 #include "../ClangTidy.h"
-#include 
-#include 
 
 namespace clang {
 namespace tidy {
@@ -27,15 +25,10 @@
 /// 
http://clang.llvm.org/extra/clang-tidy/checks/objc-property-declaration.html
 class PropertyDeclarationCheck : public ClangTidyCheck {
 public:
-  PropertyDeclarationCheck(StringRef Name, ClangTidyContext *Context);
+  PropertyDeclarationCheck(StringRef Name, ClangTidyContext *Context)
+  : ClangTidyCheck(Name, 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;
-  const bool IncludeDefaultAcronyms;
-  std::vector EscapedAcronyms;
 };
 
 } // namespace objc
Index: clang-tidy/objc/PropertyDeclarationCheck.cpp
===
--- clang-tidy/objc/PropertyDeclarationCheck.cpp
+++ clang-tidy/objc/PropertyDeclarationCheck.cpp
@@ -97,14 +97,6 @@
 }
 }  // namespace
 
-PropertyDeclarationCheck::PropertyDeclarationCheck(StringRef Name,
-   ClangTidyContext *Context)
-: ClangTidyCheck(Name, Context),
-  SpecialAcronyms(
-  utils::options::parseStringList(Options.get("Acronyms", ""))),
-  IncludeDefaultAcronyms(Options.get("IncludeDefaultAcronyms", true)),
-  EscapedAcronyms() {}
-
 void PropertyDeclarationCheck::registerMatchers(MatchFinder *Finder) {
   // this check should only be applied to ObjC sources.
   if (!getLangOpts().ObjC) return;
@@ -145,12 +137,6 @@
   << generateFixItHint(MatchedDecl, StandardProperty);
 }
 
-void PropertyDeclarationCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) 
{
-  Options.store(Opts, "Acronyms",
-utils::options::serializeStringList(SpecialAcronyms));
-  Options.store(Opts, "IncludeDefaultAcronyms", IncludeDefaultAcronyms);
-}
-
 }  // namespace objc
 }  // namespace tidy
 }  // namespace clang


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
@@ -40,15 +40,3 @@
@property(nonatomic, assign) int abc_lowerCamelCase;
 
 The corresponding style rule: https://developer.apple.com/library/content/qa/qa1908/_index.html
-
-
-Options

-
-.. option:: Acronyms
-
-   This option is deprecated and ignored.
-
-.. option:: IncludeDefaultAcronyms
-
-   This option is deprecated and ignored.
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -73,6 +73,10 @@
   Checks for casts of ``absl::Duration`` conversion functions, and recommends
   the right conversion function instead.
 
+- The `Acronyms` and `IncludeDefaultAcronyms` options for t

[PATCH] D57080: [clang-tidy] Delete obsolete objc-property-declaration options ✂️

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore marked an inline comment as done.
stephanemoore added inline comments.



Comment at: docs/ReleaseNotes.rst:76
 
+- The `Acronyms` and `IncludeDefaultAcronyms` options for the
+  :doc:`objc-property-declaration 
`

In https://reviews.llvm.org/D56945, I implemented the recommendation to use the 
:option: prefix for these options but the documentation generator produced a 
warning for an unknown option which I imagine is due to the deletion of the 
options. I imagine that means we can't use the :option: prefix for deleted 
options? Please let me know if you have other suggestions.


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57080/new/

https://reviews.llvm.org/D57080



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

[PATCH] D57207: [clang-tidy] Make google-objc-function-naming ignore implicit functions 🙈

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore created this revision.
Herald added subscribers: cfe-commits, xazax.hun.

Implicit functions are outside the control of source authors and should
be exempt from style restrictions.

Tested via running clang tools tests.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D57207

Files:
  clang-tidy/google/FunctionNamingCheck.cpp
  test/clang-tidy/google-objc-function-naming.m


Index: test/clang-tidy/google-objc-function-naming.m
===
--- test/clang-tidy/google-objc-function-naming.m
+++ test/clang-tidy/google-objc-function-naming.m
@@ -1,5 +1,11 @@
 // RUN: %check_clang_tidy %s google-objc-function-naming %t
 
+#import 
+
+static void TestImplicitlyDefinedFunction(int a) {
+  printf("%d", a);
+}
+
 typedef _Bool bool;
 
 static bool ispositive(int a) { return a > 0; }
Index: clang-tidy/google/FunctionNamingCheck.cpp
===
--- clang-tidy/google/FunctionNamingCheck.cpp
+++ clang-tidy/google/FunctionNamingCheck.cpp
@@ -98,7 +98,7 @@
   Finder->addMatcher(
   functionDecl(
   unless(anyOf(isExpansionInSystemHeader(), cxxMethodDecl(),
-   hasAncestor(namespaceDecl()), isMain(),
+   hasAncestor(namespaceDecl()), isMain(), isImplicit(),
matchesName(validFunctionNameRegex(true)),
allOf(isStaticStorageClass(),
  matchesName(validFunctionNameRegex(false))


Index: test/clang-tidy/google-objc-function-naming.m
===
--- test/clang-tidy/google-objc-function-naming.m
+++ test/clang-tidy/google-objc-function-naming.m
@@ -1,5 +1,11 @@
 // RUN: %check_clang_tidy %s google-objc-function-naming %t
 
+#import 
+
+static void TestImplicitlyDefinedFunction(int a) {
+  printf("%d", a);
+}
+
 typedef _Bool bool;
 
 static bool ispositive(int a) { return a > 0; }
Index: clang-tidy/google/FunctionNamingCheck.cpp
===
--- clang-tidy/google/FunctionNamingCheck.cpp
+++ clang-tidy/google/FunctionNamingCheck.cpp
@@ -98,7 +98,7 @@
   Finder->addMatcher(
   functionDecl(
   unless(anyOf(isExpansionInSystemHeader(), cxxMethodDecl(),
-   hasAncestor(namespaceDecl()), isMain(),
+   hasAncestor(namespaceDecl()), isMain(), isImplicit(),
matchesName(validFunctionNameRegex(true)),
allOf(isStaticStorageClass(),
  matchesName(validFunctionNameRegex(false))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57207: [clang-tidy] Make google-objc-function-naming ignore implicit functions 🙈

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore updated this revision to Diff 183646.
stephanemoore added a comment.

Updated the comment describing the matching behavior.


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57207/new/

https://reviews.llvm.org/D57207

Files:
  clang-tidy/google/FunctionNamingCheck.cpp
  test/clang-tidy/google-objc-function-naming.m


Index: test/clang-tidy/google-objc-function-naming.m
===
--- test/clang-tidy/google-objc-function-naming.m
+++ test/clang-tidy/google-objc-function-naming.m
@@ -1,5 +1,11 @@
 // RUN: %check_clang_tidy %s google-objc-function-naming %t
 
+#import 
+
+static void TestImplicitlyDefinedFunction(int a) {
+  printf("%d", a);
+}
+
 typedef _Bool bool;
 
 static bool ispositive(int a) { return a > 0; }
Index: clang-tidy/google/FunctionNamingCheck.cpp
===
--- clang-tidy/google/FunctionNamingCheck.cpp
+++ clang-tidy/google/FunctionNamingCheck.cpp
@@ -93,12 +93,16 @@
   if (!getLangOpts().ObjC)
 return;
 
-  // Match function declarations that are not in system headers and are not
-  // main.
+  // Enforce Objective-C function naming conventions on all functions except:
+  // • Functions defined in system headers.
+  // • C++ member functions.
+  // • Namespaced functions.
+  // • Implicitly defined functions.
+  // • The main function.
   Finder->addMatcher(
   functionDecl(
   unless(anyOf(isExpansionInSystemHeader(), cxxMethodDecl(),
-   hasAncestor(namespaceDecl()), isMain(),
+   hasAncestor(namespaceDecl()), isMain(), isImplicit(),
matchesName(validFunctionNameRegex(true)),
allOf(isStaticStorageClass(),
  matchesName(validFunctionNameRegex(false))


Index: test/clang-tidy/google-objc-function-naming.m
===
--- test/clang-tidy/google-objc-function-naming.m
+++ test/clang-tidy/google-objc-function-naming.m
@@ -1,5 +1,11 @@
 // RUN: %check_clang_tidy %s google-objc-function-naming %t
 
+#import 
+
+static void TestImplicitlyDefinedFunction(int a) {
+  printf("%d", a);
+}
+
 typedef _Bool bool;
 
 static bool ispositive(int a) { return a > 0; }
Index: clang-tidy/google/FunctionNamingCheck.cpp
===
--- clang-tidy/google/FunctionNamingCheck.cpp
+++ clang-tidy/google/FunctionNamingCheck.cpp
@@ -93,12 +93,16 @@
   if (!getLangOpts().ObjC)
 return;
 
-  // Match function declarations that are not in system headers and are not
-  // main.
+  // Enforce Objective-C function naming conventions on all functions except:
+  // • Functions defined in system headers.
+  // • C++ member functions.
+  // • Namespaced functions.
+  // • Implicitly defined functions.
+  // • The main function.
   Finder->addMatcher(
   functionDecl(
   unless(anyOf(isExpansionInSystemHeader(), cxxMethodDecl(),
-   hasAncestor(namespaceDecl()), isMain(),
+   hasAncestor(namespaceDecl()), isMain(), isImplicit(),
matchesName(validFunctionNameRegex(true)),
allOf(isStaticStorageClass(),
  matchesName(validFunctionNameRegex(false))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57207: [clang-tidy] Make google-objc-function-naming ignore implicit functions 🙈

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore marked 2 inline comments as done.
stephanemoore added inline comments.



Comment at: clang-tidy/google/FunctionNamingCheck.cpp:96-97
 
   // Match function declarations that are not in system headers and are not
   // main.
   Finder->addMatcher(

aaron.ballman wrote:
> This comment is drifting more and more out of sync with the code.
Good catch; updated the comment.



Comment at: test/clang-tidy/google-objc-function-naming.m:5-8
+static void TestImplicitlyDefinedFunction(int a) {
+  printf("%d", a);
+}
+

aaron.ballman wrote:
> Excuse my ignorance, but I don't see what about this function definition is 
> testing an implicitly-defined function. Doesn't the `#import` above bring in 
> the declaration for `printf()`, while `TestImplicitlyDefinedFunction()` 
> itself is explicitly defined?
**tl;dr**: Yes, we have an explicit function declaration but clang generates an 
implicit function declaration as well.

I found this surprising as well but the AST generated from this includes an 
implicit function declaration. Even though we have an explicit function 
declaration for `printf` from , clang still generates an implicit 
declaration of `printf` 🤨 Similar behavior occurs with other functions, e.g., 
`memset` and various C11 atomic functions.

I have been spelunking in search of the reason but I do not have a full 
understanding yet. I believe I have insight into the underlying mechanism 
though: the behavior seems linked to builtin functions (e.g., `printf` is 
declared as a builtin function 
[here](https://github.com/llvm/llvm-project/blob/2946cd7/clang/include/clang/Basic/Builtins.def#L889)).
 I have attached a small sample program and an associated AST dump below to 
demonstrate the behavior.

❧

```
$ cat /tmp/test.c
extern int printf(const char *format, ...);

int main(int argc, const char **argv) {
  printf("%d", argc);
  return 0;
}
```

```
$ clang -cc1 -ast-dump /tmp/test.c 
TranslationUnitDecl 0x561b9f7fe570 <> 
|-TypedefDecl 0x561b9f7feac0 <>  implicit 
__int128_t '__int128'
| `-BuiltinType 0x561b9f7fe7e0 '__int128'
|-TypedefDecl 0x561b9f7feb28 <>  implicit 
__uint128_t 'unsigned __int128'
| `-BuiltinType 0x561b9f7fe800 'unsigned __int128'
|-TypedefDecl 0x561b9f7fedf8 <>  implicit 
__NSConstantString 'struct __NSConstantString_tag'
| `-RecordType 0x561b9f7fec00 'struct __NSConstantString_tag'
|   `-Record 0x561b9f7feb78 '__NSConstantString_tag'
|-TypedefDecl 0x561b9f7fee90 <>  implicit 
__builtin_ms_va_list 'char *'
| `-PointerType 0x561b9f7fee50 'char *'
|   `-BuiltinType 0x561b9f7fe600 'char'
|-TypedefDecl 0x561b9f7ff158 <>  implicit 
__builtin_va_list 'struct __va_list_tag [1]'
| `-ConstantArrayType 0x561b9f7ff100 'struct __va_list_tag [1]' 1 
|   `-RecordType 0x561b9f7fef70 'struct __va_list_tag'
| `-Record 0x561b9f7feee0 '__va_list_tag'
|-FunctionDecl 0x561b9f851860  col:12 implicit used printf 
'int (const char *, ...)' extern
| |-ParmVarDecl 0x561b9f8518f8 <>  'const char *'
| `-FormatAttr 0x561b9f851960  Implicit printf 1 2
|-FunctionDecl 0x561b9f8519b8 prev 0x561b9f851860  col:12 used 
printf 'int (const char *, ...)' extern
| |-ParmVarDecl 0x561b9f7ff1c0  col:31 format 'const char *'
| `-FormatAttr 0x561b9f851a90  Inherited printf 1 2
`-FunctionDecl 0x561b9f851c48  line:3:5 main 'int (int, 
const char **)'
  |-ParmVarDecl 0x561b9f851ac8  col:14 used argc 'int'
  |-ParmVarDecl 0x561b9f851b70  col:33 argv 'const char **'
  `-CompoundStmt 0x561b9f851f08 
|-CallExpr 0x561b9f851e50  'int'
| |-ImplicitCastExpr 0x561b9f851e38  'int (*)(const char *, ...)' 

| | `-DeclRefExpr 0x561b9f851d58  'int (const char *, ...)' Function 
0x561b9f8519b8 'printf' 'int (const char *, ...)'
| |-ImplicitCastExpr 0x561b9f851ea0  'const char *' 
| | `-ImplicitCastExpr 0x561b9f851e88  'char *' 

| |   `-StringLiteral 0x561b9f851db8  'char [3]' lvalue "%d"
| `-ImplicitCastExpr 0x561b9f851eb8  'int' 
|   `-DeclRefExpr 0x561b9f851de8  'int' lvalue ParmVar 
0x561b9f851ac8 'argc' 'int'
`-ReturnStmt 0x561b9f851ef0 
  `-IntegerLiteral 0x561b9f851ed0  'int' 0
```

Note the presence of implicit and explicit declarations of `printf` in the AST 
dump 🤔


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57207/new/

https://reviews.llvm.org/D57207



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

[PATCH] D56945: [clang-tidy] Delete obsolete objc-property-declaration options ✂️

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore marked an inline comment as done.
stephanemoore added inline comments.



Comment at: docs/ReleaseNotes.rst:248
 
+- The `Acronyms` and `IncludeDefaultAcronyms` options for the
+  :doc:`objc-property-declaration`

stephanemoore wrote:
> Eugene.Zelenko wrote:
> > Please rebase from trunk and use :option: prefix for options..
> Could using the :option: prefix cause issues if I am removing the option?
> 
> The documentation generation seems to emit a warning for an unknown option:
> "llvm/src/tools/clang/tools/extra/docs/ReleaseNotes.rst:76: WARNING: unknown 
> option: Acronyms"
> http://lab.llvm.org:8011/builders/clang-tools-sphinx-docs/builds/36425/steps/docs-clang-tools-html/logs/stdio
FYI:
I rolled this change back due to the documentation generation failure and sent 
out https://reviews.llvm.org/D57080 with amended changes.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56945/new/

https://reviews.llvm.org/D56945



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

[PATCH] D55640: [clang-tidy] Implement a check for large Objective-C type encodings 🔍

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore planned changes to this revision.
stephanemoore added a comment.

Reiterating outstanding action items:
• Evaluate using hasDeclContext instead of hasAncestor.
• Include type encoding size information in diagnostic messages.

I've been observing some unexpected behaviors with matching in the proposed 
test sample that I am still investigating (as I recall the issue pertained to 
interfaces/protocols without interfaces).

Including the type encoding size information in diagnostic messages shouldn't 
be too hard (assuming we can get precise type encoding size information by 
analyzing the AST).


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55640/new/

https://reviews.llvm.org/D55640



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

[PATCH] D57207: [clang-tidy] Make google-objc-function-naming ignore implicit functions 🙈

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore marked 3 inline comments as done.
stephanemoore added a comment.

Thanks for the review!


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57207/new/

https://reviews.llvm.org/D57207



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

[PATCH] D57207: [clang-tidy] Make google-objc-function-naming ignore implicit functions 🙈

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore updated this revision to Diff 184553.
stephanemoore marked an inline comment as done.
stephanemoore added a comment.

Add comment explaining that builtin function call generates implicit function 
declaration and update calling function name for improved clarity.


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57207/new/

https://reviews.llvm.org/D57207

Files:
  clang-tidy/google/FunctionNamingCheck.cpp
  test/clang-tidy/google-objc-function-naming.m


Index: test/clang-tidy/google-objc-function-naming.m
===
--- test/clang-tidy/google-objc-function-naming.m
+++ test/clang-tidy/google-objc-function-naming.m
@@ -1,5 +1,13 @@
 // RUN: %check_clang_tidy %s google-objc-function-naming %t
 
+#import 
+
+static void TestImplicitFunctionDeclaration(int a) {
+  // Call a builtin function so that the compiler generates an implicit
+  // function declaration.
+  printf("%d", a);
+}
+
 typedef _Bool bool;
 
 static bool ispositive(int a) { return a > 0; }
Index: clang-tidy/google/FunctionNamingCheck.cpp
===
--- clang-tidy/google/FunctionNamingCheck.cpp
+++ clang-tidy/google/FunctionNamingCheck.cpp
@@ -93,12 +93,16 @@
   if (!getLangOpts().ObjC)
 return;
 
-  // Match function declarations that are not in system headers and are not
-  // main.
+  // Enforce Objective-C function naming conventions on all functions except:
+  // • Functions defined in system headers.
+  // • C++ member functions.
+  // • Namespaced functions.
+  // • Implicitly defined functions.
+  // • The main function.
   Finder->addMatcher(
   functionDecl(
   unless(anyOf(isExpansionInSystemHeader(), cxxMethodDecl(),
-   hasAncestor(namespaceDecl()), isMain(),
+   hasAncestor(namespaceDecl()), isMain(), isImplicit(),
matchesName(validFunctionNameRegex(true)),
allOf(isStaticStorageClass(),
  matchesName(validFunctionNameRegex(false))


Index: test/clang-tidy/google-objc-function-naming.m
===
--- test/clang-tidy/google-objc-function-naming.m
+++ test/clang-tidy/google-objc-function-naming.m
@@ -1,5 +1,13 @@
 // RUN: %check_clang_tidy %s google-objc-function-naming %t
 
+#import 
+
+static void TestImplicitFunctionDeclaration(int a) {
+  // Call a builtin function so that the compiler generates an implicit
+  // function declaration.
+  printf("%d", a);
+}
+
 typedef _Bool bool;
 
 static bool ispositive(int a) { return a > 0; }
Index: clang-tidy/google/FunctionNamingCheck.cpp
===
--- clang-tidy/google/FunctionNamingCheck.cpp
+++ clang-tidy/google/FunctionNamingCheck.cpp
@@ -93,12 +93,16 @@
   if (!getLangOpts().ObjC)
 return;
 
-  // Match function declarations that are not in system headers and are not
-  // main.
+  // Enforce Objective-C function naming conventions on all functions except:
+  // • Functions defined in system headers.
+  // • C++ member functions.
+  // • Namespaced functions.
+  // • Implicitly defined functions.
+  // • The main function.
   Finder->addMatcher(
   functionDecl(
   unless(anyOf(isExpansionInSystemHeader(), cxxMethodDecl(),
-   hasAncestor(namespaceDecl()), isMain(),
+   hasAncestor(namespaceDecl()), isMain(), isImplicit(),
matchesName(validFunctionNameRegex(true)),
allOf(isStaticStorageClass(),
  matchesName(validFunctionNameRegex(false))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51832: [clang-tidy/checks] Update objc-property-declaration check to allow arbitrary acronyms and initialisms 🔧

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore updated this revision to Diff 172285.
stephanemoore marked an inline comment as done.
stephanemoore added a comment.

Updated comments, release notes, and documentation to be consistent with the 
implemented changes.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51832

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

Index: test/clang-tidy/objc-property-declaration.m
===
--- test/clang-tidy/objc-property-declaration.m
+++ test/clang-tidy/objc-property-declaration.m
@@ -1,8 +1,12 @@
 // RUN: %check_clang_tidy %s objc-property-declaration %t
+@class CIColor;
+@class NSArray;
 @class NSData;
 @class NSString;
 @class UIViewController;
 
+typedef void *CGColorRef;
+
 @interface Foo
 @property(assign, nonatomic) int NotCamelCase;
 // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: property name 'NotCamelCase' not using lowerCamelCase style or not prefixed in a category, according to the Apple Coding Guidelines [objc-property-declaration]
@@ -23,6 +27,9 @@
 @property(assign, nonatomic) int enableGLAcceleration;
 @property(assign, nonatomic) int ID;
 @property(assign, nonatomic) int hasADog;
+@property(nonatomic, readonly) CGColorRef CGColor;
+@property(nonatomic, readonly) CIColor *CIColor;
+@property(nonatomic, copy) NSArray *IDs;
 @end
 
 @interface Foo (Bar)
Index: test/clang-tidy/objc-property-declaration-custom.m
===
--- test/clang-tidy/objc-property-declaration-custom.m
+++ /dev/null
@@ -1,18 +0,0 @@
-// 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.IncludeDefaultAcronyms, value: 0}]}' \
-// RUN: --
-@class NSString;
-
-@interface Foo
-@property(assign, nonatomic) int AbcNotRealPrefix;
-// CHECK-MESSAGES: :[[@LINE-1]]:34: warning: property name 'AbcNotRealPrefix' not using lowerCamelCase style or not prefixed in a category, 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' not using lowerCamelCase style or not prefixed in a category, according to the Apple Coding Guidelines [objc-property-declaration]
-@property(assign, nonatomic) int GIFIgnoreStandardAcronym;
-// CHECK-MESSAGES: :[[@LINE-1]]:34: warning: property name 'GIFIgnoreStandardAcronym' not using lowerCamelCase style or not prefixed in a category, according to the Apple Coding Guidelines [objc-property-declaration]
-@property(strong, nonatomic) NSString *TGIF;
-@end
Index: test/clang-tidy/objc-property-declaration-additional.m
===
--- test/clang-tidy/objc-property-declaration-additional.m
+++ /dev/null
@@ -1,15 +0,0 @@
-// 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' not using lowerCamelCase style or not prefixed in a category, 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' not using lowerCamelCase style or not prefixed in a category, 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
@@ -47,23 +47,8 @@
 
 .. option:: Acronyms
 
-   Semicolon-separated list of custom acronyms that can be used as a prefix
-   or a suffix of property names.
-
-   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.
+   This option is deprecated and ignored.
 
 .. option:: IncludeDefaultAcronyms
 
-   Integer value (defaults to ``1``) to 

[PATCH] D51832: [clang-tidy/checks] Update objc-property-declaration check to allow arbitrary acronyms and initialisms 🔧

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore added a comment.

I believe that it should be safe to remove the `Acronyms` and 
`IncludeDefaultAcronyms` options but if it's alright with you, I would prefer 
to separate that into a followup change.




Comment at: clang-tidy/objc/PropertyDeclarationCheck.cpp:28-31
 // For StandardProperty the naming style is 'lowerCamelCase'.
 // For CategoryProperty especially in categories of system class,
 // to avoid naming conflict, the suggested naming style is
 // 'abc_lowerCamelCase' (adding lowercase prefix followed by '_').

benhamilton wrote:
> These comments are no longer accurate.
I think the comments are still more or less accurate? I think we have just 
changed the level of enforcement that we are applying.

We allow aRbITRaRyCapS now but that's based on the concession that it's 
fundamentally hard to identify appropriate lowerCamelCase with high precision 
(i.e., it is hard to reject "aRbITRaRyCapS" as a representation of "a rb IT ra 
ry cap S" without human judgment). I added a note that acronyms and initialisms 
are capitalized regardless of style though.

❧

Sidenote:
A spark of curiosity led to me realizing that the previous approach would also 
have allowed aRbITRaRyCapS if all letters of the alphabet were to be 
whitelisted as acronyms. The previous check already included several letters in 
the default special acronyms and provided special treatment for "A" and "I" in 
the matching regex. I can't say for certain that every letter of the alphabet 
would have ended up being whitelisted but I audited the letters of the alphabet 
and I think I managed to construct a list of terms that could conceivably have 
been used in code and might have led to the whitelisting of each individual 
letter of the alphabet (attached below¹).

[1] List of Letters in English Alphabet and Terms Containing Them in Isolation
* A: https://en.wikipedia.org/wiki/A_value
* B: https://en.wikipedia.org/wiki/B_vitamins
* C: https://en.wikipedia.org/wiki/C_major
* D: https://en.wikipedia.org/wiki/D_major
* E: https://en.wikipedia.org/wiki/E_major
* F: https://en.wikipedia.org/wiki/F-number
* G: https://en.wikipedia.org/wiki/G_major
* H: https://en.wikipedia.org/wiki/H-index
* I: https://en.wikipedia.org/wiki/I_band_(NATO)
* J: https://en.wikipedia.org/wiki/J_operator
* K: https://en.wikipedia.org/wiki/Vitamin_K
* L: https://en.wikipedia.org/wiki/L-notation
* M: https://en.wikipedia.org/wiki/M_postcode_area
* N: https://en.wikipedia.org/wiki/N_ray
* O: https://en.wikipedia.org/wiki/Big_O_notation
* P: https://en.wikipedia.org/wiki/Extrinsic_semiconductor#P-type_semiconductors
* Q: https://en.wikipedia.org/wiki/Q_Score
* R: https://en.wikipedia.org/wiki/R-value_(insulation)
* S: https://en.wikipedia.org/wiki/S_postcode_area
* T: https://en.wikipedia.org/wiki/F-number#T-stop
* U: https://en.wikipedia.org/wiki/U_language
* V: https://en.wikipedia.org/wiki/V_band
* W: https://en.wikipedia.org/wiki/W_and_Z_bosons
* X: https://en.wikipedia.org/wiki/X-ray
* Y: https://en.wikipedia.org/wiki/Y_chromosome
* Z: https://en.wikipedia.org/wiki/Z-buffering




Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51832



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

[PATCH] D51575: [clang-tidy] Implement a clang-tidy check to verify Google Objective-C function naming conventions 📜

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore marked an inline comment as done.
stephanemoore added a comment.

https://reviews.llvm.org/D51832 is in review to update the 
objc-property-declaration check to allow arbitrary acronyms and initialisms.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51575



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

[PATCH] D51575: [clang-tidy] Implement a clang-tidy check to verify Google Objective-C function naming conventions 📜

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore updated this revision to Diff 172296.
stephanemoore added a comment.

Updated diff after pulling and merging.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51575

Files:
  clang-tidy/google/CMakeLists.txt
  clang-tidy/google/FunctionNamingCheck.cpp
  clang-tidy/google/FunctionNamingCheck.h
  clang-tidy/google/GoogleTidyModule.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/google-objc-function-naming.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/google-objc-function-naming.m

Index: test/clang-tidy/google-objc-function-naming.m
===
--- /dev/null
+++ test/clang-tidy/google-objc-function-naming.m
@@ -0,0 +1,49 @@
+// RUN: %check_clang_tidy %s google-objc-function-naming %t
+
+typedef _Bool bool;
+
+static bool ispositive(int a) { return a > 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'ispositive' not using function naming conventions described by Google Objective-C style guide
+// CHECK-FIXES: static bool Ispositive(int a) { return a > 0; }
+
+static bool is_positive(int a) { return a > 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'is_positive' not using function naming conventions described by Google Objective-C style guide
+// CHECK-FIXES: static bool IsPositive(int a) { return a > 0; }
+
+static bool isPositive(int a) { return a > 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'isPositive' not using function naming conventions described by Google Objective-C style guide
+// CHECK-FIXES: static bool IsPositive(int a) { return a > 0; }
+
+static bool Is_Positive(int a) { return a > 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'Is_Positive' not using function naming conventions described by Google Objective-C style guide
+// CHECK-FIXES: static bool IsPositive(int a) { return a > 0; }
+
+static bool IsPositive(int a) { return a > 0; }
+
+bool ispalindrome(const char *str);
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function name 'ispalindrome' not using function naming conventions described by Google Objective-C style guide
+
+static const char *md5(const char *str) { return 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: function name 'md5' not using function naming conventions described by Google Objective-C style guide
+// CHECK-FIXES: static const char *Md5(const char *str) { return 0; }
+
+static const char *MD5(const char *str) { return 0; }
+
+static const char *URL(void) { return "https://clang.llvm.org/";; }
+
+static const char *DEFURL(void) { return "https://clang.llvm.org/";; }
+
+static const char *DEFFooURL(void) { return "https://clang.llvm.org/";; }
+
+static const char *StringFromNSString(id str) { return ""; }
+
+void ABLog_String(const char *str);
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function name 'ABLog_String' not using function naming conventions described by Google Objective-C style guide
+
+void ABLogString(const char *str);
+
+const char *ABURL(void) { return "https://clang.llvm.org/";; }
+
+const char *ABFooURL(void) { return "https://clang.llvm.org/";; }
+
+int main(int argc, const char **argv) { return 0; }
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -120,6 +120,7 @@
google-explicit-constructor
google-global-names-in-headers
google-objc-avoid-throwing-exception
+   google-objc-function-naming
google-objc-global-variable-declaration
google-readability-braces-around-statements (redirects to readability-braces-around-statements) 
google-readability-casting
Index: docs/clang-tidy/checks/google-objc-function-naming.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/google-objc-function-naming.rst
@@ -0,0 +1,27 @@
+.. title:: clang-tidy - google-objc-function-naming
+
+google-objc-function-naming
+===
+
+Finds function declarations in Objective-C files that do not follow the pattern
+described in the Google Objective-C Style Guide.
+
+The corresponding style guide rule can be found here:
+https://google.github.io/styleguide/objcguide.html#function-names
+
+All function names should be in upper camel case. Functions whose storage class
+is not static should have an appropriate prefix.
+
+The following code sample does not follow this pattern:
+
+.. code-block:: objc
+
+  static bool is_positive(int i) { return i > 0; }
+  bool IsNegative(int i) { return i < 0; }
+
+The sample above might be corrected to the following code:
+
+.. code-block:: objc
+
+  static bool IsPositive(int i) { return i > 0; }
+  bool *ABCIsNegative(int i) { return i < 0; }
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -103,6 +103,12 @@
   Flags uses of ``absl::StrCat

[PATCH] D51575: [clang-tidy/checks] Implement a clang-tidy check to verify Google Objective-C function naming conventions 📜

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore updated this revision to Diff 173244.
stephanemoore marked an inline comment as done.
stephanemoore added a comment.

Added a test case for an extern function without an appropriate prefix 
(`IsPrime`).


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51575

Files:
  clang-tidy/google/CMakeLists.txt
  clang-tidy/google/FunctionNamingCheck.cpp
  clang-tidy/google/FunctionNamingCheck.h
  clang-tidy/google/GoogleTidyModule.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/google-objc-function-naming.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/google-objc-function-naming.m

Index: test/clang-tidy/google-objc-function-naming.m
===
--- /dev/null
+++ test/clang-tidy/google-objc-function-naming.m
@@ -0,0 +1,52 @@
+// RUN: %check_clang_tidy %s google-objc-function-naming %t
+
+typedef _Bool bool;
+
+static bool ispositive(int a) { return a > 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'ispositive' not using function naming conventions described by Google Objective-C style guide
+// CHECK-FIXES: static bool Ispositive(int a) { return a > 0; }
+
+static bool is_positive(int a) { return a > 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'is_positive' not using function naming conventions described by Google Objective-C style guide
+// CHECK-FIXES: static bool IsPositive(int a) { return a > 0; }
+
+static bool isPositive(int a) { return a > 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'isPositive' not using function naming conventions described by Google Objective-C style guide
+// CHECK-FIXES: static bool IsPositive(int a) { return a > 0; }
+
+static bool Is_Positive(int a) { return a > 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'Is_Positive' not using function naming conventions described by Google Objective-C style guide
+// CHECK-FIXES: static bool IsPositive(int a) { return a > 0; }
+
+static bool IsPositive(int a) { return a > 0; }
+
+bool ispalindrome(const char *str);
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function name 'ispalindrome' not using function naming conventions described by Google Objective-C style guide
+
+static const char *md5(const char *str) { return 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: function name 'md5' not using function naming conventions described by Google Objective-C style guide
+// CHECK-FIXES: static const char *Md5(const char *str) { return 0; }
+
+static const char *MD5(const char *str) { return 0; }
+
+static const char *URL(void) { return "https://clang.llvm.org/";; }
+
+static const char *DEFURL(void) { return "https://clang.llvm.org/";; }
+
+static const char *DEFFooURL(void) { return "https://clang.llvm.org/";; }
+
+static const char *StringFromNSString(id str) { return ""; }
+
+void ABLog_String(const char *str);
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function name 'ABLog_String' not using function naming conventions described by Google Objective-C style guide
+
+void ABLogString(const char *str);
+
+bool IsPrime(int a);
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function name 'IsPrime' not using function naming conventions described by Google Objective-C style guide
+
+const char *ABURL(void) { return "https://clang.llvm.org/";; }
+
+const char *ABFooURL(void) { return "https://clang.llvm.org/";; }
+
+int main(int argc, const char **argv) { return 0; }
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -120,6 +120,7 @@
google-explicit-constructor
google-global-names-in-headers
google-objc-avoid-throwing-exception
+   google-objc-function-naming
google-objc-global-variable-declaration
google-readability-braces-around-statements (redirects to readability-braces-around-statements) 
google-readability-casting
Index: docs/clang-tidy/checks/google-objc-function-naming.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/google-objc-function-naming.rst
@@ -0,0 +1,27 @@
+.. title:: clang-tidy - google-objc-function-naming
+
+google-objc-function-naming
+===
+
+Finds function declarations in Objective-C files that do not follow the pattern
+described in the Google Objective-C Style Guide.
+
+The corresponding style guide rule can be found here:
+https://google.github.io/styleguide/objcguide.html#function-names
+
+All function names should be in upper camel case. Functions whose storage class
+is not static should have an appropriate prefix.
+
+The following code sample does not follow this pattern:
+
+.. code-block:: objc
+
+  static bool is_positive(int i) { return i > 0; }
+  bool IsNegative(int i) { return i < 0; }
+
+The sample above might be corrected to the following code:
+
+.. code-block:: objc
+
+  static bool IsPositive(int i)

[PATCH] D51575: [clang-tidy/checks] Implement a clang-tidy check to verify Google Objective-C function naming conventions 📜

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore added inline comments.



Comment at: docs/clang-tidy/checks/google-objc-function-naming.rst:20
+  static bool is_positive(int i) { return i > 0; }
+  bool IsNegative(int i) { return i < 0; }
+

benhamilton wrote:
> This is not actually handled by the check, right? (You even have a test which 
> confirms this.)
> 
> As far as I can tell, this check essentially looks for at least one capital 
> letter and no `_` in function names, right?
It does flag this declaration because it's missing an appropriate prefix.

It looks like I neglected to add a test case for that. I have done so now.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51575



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

[PATCH] D51575: [clang-tidy/checks] Implement a clang-tidy check to verify Google Objective-C function naming conventions 📜

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore updated this revision to Diff 173544.
stephanemoore marked 9 inline comments as done.
stephanemoore added a comment.

Updated with the following changes:
• Elided conditional braces to comply with LLVM style.
• Converted conditional loop break to loop condition in generateFixItHint.
• Fixed Objective-C language option check.
• Removed unnecessary assertions.
• Use `MatchedDecl` directly for formatting the diagnostic message.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51575

Files:
  clang-tidy/google/CMakeLists.txt
  clang-tidy/google/FunctionNamingCheck.cpp
  clang-tidy/google/FunctionNamingCheck.h
  clang-tidy/google/GoogleTidyModule.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/google-objc-function-naming.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/google-objc-function-naming.m

Index: test/clang-tidy/google-objc-function-naming.m
===
--- /dev/null
+++ test/clang-tidy/google-objc-function-naming.m
@@ -0,0 +1,52 @@
+// RUN: %check_clang_tidy %s google-objc-function-naming %t
+
+typedef _Bool bool;
+
+static bool ispositive(int a) { return a > 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'ispositive' not using function naming conventions described by Google Objective-C style guide
+// CHECK-FIXES: static bool Ispositive(int a) { return a > 0; }
+
+static bool is_positive(int a) { return a > 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'is_positive' not using function naming conventions described by Google Objective-C style guide
+// CHECK-FIXES: static bool IsPositive(int a) { return a > 0; }
+
+static bool isPositive(int a) { return a > 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'isPositive' not using function naming conventions described by Google Objective-C style guide
+// CHECK-FIXES: static bool IsPositive(int a) { return a > 0; }
+
+static bool Is_Positive(int a) { return a > 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'Is_Positive' not using function naming conventions described by Google Objective-C style guide
+// CHECK-FIXES: static bool IsPositive(int a) { return a > 0; }
+
+static bool IsPositive(int a) { return a > 0; }
+
+bool ispalindrome(const char *str);
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function name 'ispalindrome' not using function naming conventions described by Google Objective-C style guide
+
+static const char *md5(const char *str) { return 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: function name 'md5' not using function naming conventions described by Google Objective-C style guide
+// CHECK-FIXES: static const char *Md5(const char *str) { return 0; }
+
+static const char *MD5(const char *str) { return 0; }
+
+static const char *URL(void) { return "https://clang.llvm.org/";; }
+
+static const char *DEFURL(void) { return "https://clang.llvm.org/";; }
+
+static const char *DEFFooURL(void) { return "https://clang.llvm.org/";; }
+
+static const char *StringFromNSString(id str) { return ""; }
+
+void ABLog_String(const char *str);
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function name 'ABLog_String' not using function naming conventions described by Google Objective-C style guide
+
+void ABLogString(const char *str);
+
+bool IsPrime(int a);
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function name 'IsPrime' not using function naming conventions described by Google Objective-C style guide
+
+const char *ABURL(void) { return "https://clang.llvm.org/";; }
+
+const char *ABFooURL(void) { return "https://clang.llvm.org/";; }
+
+int main(int argc, const char **argv) { return 0; }
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -122,6 +122,7 @@
google-explicit-constructor
google-global-names-in-headers
google-objc-avoid-throwing-exception
+   google-objc-function-naming
google-objc-global-variable-declaration
google-readability-braces-around-statements (redirects to readability-braces-around-statements) 
google-readability-casting
Index: docs/clang-tidy/checks/google-objc-function-naming.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/google-objc-function-naming.rst
@@ -0,0 +1,27 @@
+.. title:: clang-tidy - google-objc-function-naming
+
+google-objc-function-naming
+===
+
+Finds function declarations in Objective-C files that do not follow the pattern
+described in the Google Objective-C Style Guide.
+
+The corresponding style guide rule can be found here:
+https://google.github.io/styleguide/objcguide.html#function-names
+
+All function names should be in upper camel case. Functions whose storage class
+is not static should have an appropriate prefix.
+
+The following code sample does not follow this pattern:
+
+.. code-block

[PATCH] D51575: [clang-tidy/checks] Implement a clang-tidy check to verify Google Objective-C function naming conventions 📜

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore updated this revision to Diff 173936.
stephanemoore added a comment.

Reworded "upper camel case" to "Pascal case".


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51575

Files:
  clang-tidy/google/CMakeLists.txt
  clang-tidy/google/FunctionNamingCheck.cpp
  clang-tidy/google/FunctionNamingCheck.h
  clang-tidy/google/GoogleTidyModule.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/google-objc-function-naming.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/google-objc-function-naming.m

Index: test/clang-tidy/google-objc-function-naming.m
===
--- /dev/null
+++ test/clang-tidy/google-objc-function-naming.m
@@ -0,0 +1,52 @@
+// RUN: %check_clang_tidy %s google-objc-function-naming %t
+
+typedef _Bool bool;
+
+static bool ispositive(int a) { return a > 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'ispositive' not using function naming conventions described by Google Objective-C style guide
+// CHECK-FIXES: static bool Ispositive(int a) { return a > 0; }
+
+static bool is_positive(int a) { return a > 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'is_positive' not using function naming conventions described by Google Objective-C style guide
+// CHECK-FIXES: static bool IsPositive(int a) { return a > 0; }
+
+static bool isPositive(int a) { return a > 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'isPositive' not using function naming conventions described by Google Objective-C style guide
+// CHECK-FIXES: static bool IsPositive(int a) { return a > 0; }
+
+static bool Is_Positive(int a) { return a > 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'Is_Positive' not using function naming conventions described by Google Objective-C style guide
+// CHECK-FIXES: static bool IsPositive(int a) { return a > 0; }
+
+static bool IsPositive(int a) { return a > 0; }
+
+bool ispalindrome(const char *str);
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function name 'ispalindrome' not using function naming conventions described by Google Objective-C style guide
+
+static const char *md5(const char *str) { return 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: function name 'md5' not using function naming conventions described by Google Objective-C style guide
+// CHECK-FIXES: static const char *Md5(const char *str) { return 0; }
+
+static const char *MD5(const char *str) { return 0; }
+
+static const char *URL(void) { return "https://clang.llvm.org/";; }
+
+static const char *DEFURL(void) { return "https://clang.llvm.org/";; }
+
+static const char *DEFFooURL(void) { return "https://clang.llvm.org/";; }
+
+static const char *StringFromNSString(id str) { return ""; }
+
+void ABLog_String(const char *str);
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function name 'ABLog_String' not using function naming conventions described by Google Objective-C style guide
+
+void ABLogString(const char *str);
+
+bool IsPrime(int a);
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function name 'IsPrime' not using function naming conventions described by Google Objective-C style guide
+
+const char *ABURL(void) { return "https://clang.llvm.org/";; }
+
+const char *ABFooURL(void) { return "https://clang.llvm.org/";; }
+
+int main(int argc, const char **argv) { return 0; }
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -122,6 +122,7 @@
google-explicit-constructor
google-global-names-in-headers
google-objc-avoid-throwing-exception
+   google-objc-function-naming
google-objc-global-variable-declaration
google-readability-braces-around-statements (redirects to readability-braces-around-statements) 
google-readability-casting
Index: docs/clang-tidy/checks/google-objc-function-naming.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/google-objc-function-naming.rst
@@ -0,0 +1,27 @@
+.. title:: clang-tidy - google-objc-function-naming
+
+google-objc-function-naming
+===
+
+Finds function declarations in Objective-C files that do not follow the pattern
+described in the Google Objective-C Style Guide.
+
+The corresponding style guide rule can be found here:
+https://google.github.io/styleguide/objcguide.html#function-names
+
+All function names should be in Pascal case. Functions whose storage class is
+not static should have an appropriate prefix.
+
+The following code sample does not follow this pattern:
+
+.. code-block:: objc
+
+  static bool is_positive(int i) { return i > 0; }
+  bool IsNegative(int i) { return i < 0; }
+
+The sample above might be corrected to the following code:
+
+.. code-block:: objc
+
+  static bool IsPositive(int i) { return i > 0; }
+  bool *ABCIsNegative(int i) { return i < 0; }
Index: docs/ReleaseNotes.

[PATCH] D51575: [clang-tidy/checks] Implement a clang-tidy check to verify Google Objective-C function naming conventions 📜

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore added a comment.

Thanks for the review everyone!

Let me know if there are any further changes that you want me to make or any 
further action required on my part to land this 😁


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51575



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

[PATCH] D51575: [clang-tidy/checks] Implement a clang-tidy check to verify Google Objective-C function naming conventions 📜

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore added a comment.

I received commit access yesterday so I believe I should be able to land this 
myself (after I get myself set up and land a test commit).


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51575



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

[PATCH] D51575: [clang-tidy/checks] Implement a clang-tidy check to verify Google Objective-C function naming conventions 📜

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore added inline comments.



Comment at: clang-tidy/google/FunctionNamingCheck.cpp:63
+  bool AtWordBoundary = true;
+  while (Index >= NewName.size()) {
+char ch = NewName[Index];

I forgot to invert this condition when I refactored the early return into the 
loop condition 😓


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51575



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

[PATCH] D51575: [clang-tidy/checks] Implement a clang-tidy check to verify Google Objective-C function naming conventions 📜

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore updated this revision to Diff 174465.
stephanemoore added a comment.

Fixed loop condition in generateFixItHint.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51575

Files:
  clang-tidy/google/CMakeLists.txt
  clang-tidy/google/FunctionNamingCheck.cpp
  clang-tidy/google/FunctionNamingCheck.h
  clang-tidy/google/GoogleTidyModule.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/google-objc-function-naming.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/google-objc-function-naming.m

Index: test/clang-tidy/google-objc-function-naming.m
===
--- /dev/null
+++ test/clang-tidy/google-objc-function-naming.m
@@ -0,0 +1,52 @@
+// RUN: %check_clang_tidy %s google-objc-function-naming %t
+
+typedef _Bool bool;
+
+static bool ispositive(int a) { return a > 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'ispositive' not using function naming conventions described by Google Objective-C style guide
+// CHECK-FIXES: static bool Ispositive(int a) { return a > 0; }
+
+static bool is_positive(int a) { return a > 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'is_positive' not using function naming conventions described by Google Objective-C style guide
+// CHECK-FIXES: static bool IsPositive(int a) { return a > 0; }
+
+static bool isPositive(int a) { return a > 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'isPositive' not using function naming conventions described by Google Objective-C style guide
+// CHECK-FIXES: static bool IsPositive(int a) { return a > 0; }
+
+static bool Is_Positive(int a) { return a > 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'Is_Positive' not using function naming conventions described by Google Objective-C style guide
+// CHECK-FIXES: static bool IsPositive(int a) { return a > 0; }
+
+static bool IsPositive(int a) { return a > 0; }
+
+bool ispalindrome(const char *str);
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function name 'ispalindrome' not using function naming conventions described by Google Objective-C style guide
+
+static const char *md5(const char *str) { return 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: function name 'md5' not using function naming conventions described by Google Objective-C style guide
+// CHECK-FIXES: static const char *Md5(const char *str) { return 0; }
+
+static const char *MD5(const char *str) { return 0; }
+
+static const char *URL(void) { return "https://clang.llvm.org/";; }
+
+static const char *DEFURL(void) { return "https://clang.llvm.org/";; }
+
+static const char *DEFFooURL(void) { return "https://clang.llvm.org/";; }
+
+static const char *StringFromNSString(id str) { return ""; }
+
+void ABLog_String(const char *str);
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function name 'ABLog_String' not using function naming conventions described by Google Objective-C style guide
+
+void ABLogString(const char *str);
+
+bool IsPrime(int a);
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function name 'IsPrime' not using function naming conventions described by Google Objective-C style guide
+
+const char *ABURL(void) { return "https://clang.llvm.org/";; }
+
+const char *ABFooURL(void) { return "https://clang.llvm.org/";; }
+
+int main(int argc, const char **argv) { return 0; }
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -124,6 +124,7 @@
google-explicit-constructor
google-global-names-in-headers
google-objc-avoid-throwing-exception
+   google-objc-function-naming
google-objc-global-variable-declaration
google-readability-braces-around-statements (redirects to readability-braces-around-statements) 
google-readability-casting
Index: docs/clang-tidy/checks/google-objc-function-naming.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/google-objc-function-naming.rst
@@ -0,0 +1,27 @@
+.. title:: clang-tidy - google-objc-function-naming
+
+google-objc-function-naming
+===
+
+Finds function declarations in Objective-C files that do not follow the pattern
+described in the Google Objective-C Style Guide.
+
+The corresponding style guide rule can be found here:
+https://google.github.io/styleguide/objcguide.html#function-names
+
+All function names should be in Pascal case. Functions whose storage class is
+not static should have an appropriate prefix.
+
+The following code sample does not follow this pattern:
+
+.. code-block:: objc
+
+  static bool is_positive(int i) { return i > 0; }
+  bool IsNegative(int i) { return i < 0; }
+
+The sample above might be corrected to the following code:
+
+.. code-block:: objc
+
+  static bool IsPositive(int i) { return i > 0; }
+  bool *ABCIsNegative(int i) { return i < 0; }
Index: docs/ReleaseNotes.rst

[PATCH] D51575: [clang-tidy/checks] Implement a clang-tidy check to verify Google Objective-C function naming conventions 📜

[Stephane Moore via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL347132: [clang-tidy/checks] Implement a clang-tidy check to 
verify Google Objective-C… (authored by stephanemoore, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D51575?vs=174465&id=174493#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D51575

Files:
  clang-tools-extra/trunk/clang-tidy/google/CMakeLists.txt
  clang-tools-extra/trunk/clang-tidy/google/FunctionNamingCheck.cpp
  clang-tools-extra/trunk/clang-tidy/google/FunctionNamingCheck.h
  clang-tools-extra/trunk/clang-tidy/google/GoogleTidyModule.cpp
  clang-tools-extra/trunk/docs/ReleaseNotes.rst
  clang-tools-extra/trunk/docs/clang-tidy/checks/google-objc-function-naming.rst
  clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
  clang-tools-extra/trunk/test/clang-tidy/google-objc-function-naming.m

Index: clang-tools-extra/trunk/clang-tidy/google/FunctionNamingCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/google/FunctionNamingCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/google/FunctionNamingCheck.cpp
@@ -0,0 +1,121 @@
+//===--- FunctionNamingCheck.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 "FunctionNamingCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "llvm/Support/Regex.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace google {
+namespace objc {
+
+namespace {
+
+std::string validFunctionNameRegex(bool RequirePrefix) {
+  // Allow the following name patterns for all functions:
+  // • ABFoo (prefix + UpperCamelCase)
+  // • ABURL (prefix + capitalized acronym/initialism)
+  //
+  // If no prefix is required, additionally allow the following name patterns:
+  // • Foo (UpperCamelCase)
+  // • URL (capitalized acronym/initialism)
+  //
+  // The function name following the prefix can contain standard and
+  // non-standard capitalized character sequences including acronyms,
+  // initialisms, and prefixes of symbols (e.g., UIColorFromNSString). For this
+  // reason, the regex only verifies that the function name after the prefix
+  // begins with a capital letter followed by an arbitrary sequence of
+  // alphanumeric characters.
+  //
+  // If a prefix is required, the regex checks for a capital letter followed by
+  // another capital letter or number that is part of the prefix and another
+  // capital letter or number that begins the name following the prefix.
+  std::string FunctionNameMatcher =
+  std::string(RequirePrefix ? "[A-Z][A-Z0-9]+" : "") + "[A-Z][a-zA-Z0-9]*";
+  return std::string("::(") + FunctionNameMatcher + ")$";
+}
+
+/// For now we will only fix functions of static storage class with names like
+/// 'functionName' or 'function_name' and convert them to 'FunctionName'. For
+/// other cases the user must determine an appropriate name on their own.
+FixItHint generateFixItHint(const FunctionDecl *Decl) {
+  // A fixit can be generated for functions of static storage class but
+  // otherwise the check cannot determine the appropriate function name prefix
+  // to use.
+  if (Decl->getStorageClass() != SC_Static)
+return FixItHint();
+
+  StringRef Name = Decl->getName();
+  std::string NewName = Decl->getName().str();
+
+  size_t Index = 0;
+  bool AtWordBoundary = true;
+  while (Index < NewName.size()) {
+char ch = NewName[Index];
+if (isalnum(ch)) {
+  // Capitalize the first letter after every word boundary.
+  if (AtWordBoundary) {
+NewName[Index] = toupper(NewName[Index]);
+AtWordBoundary = false;
+  }
+
+  // Advance the index after every alphanumeric character.
+  Index++;
+} else {
+  // Strip out any characters other than alphanumeric characters.
+  NewName.erase(Index, 1);
+  AtWordBoundary = true;
+}
+  }
+
+  // Generate a fixit hint if the new name is different.
+  if (NewName != Name)
+return FixItHint::CreateReplacement(
+CharSourceRange::getTokenRange(SourceRange(Decl->getLocation())),
+llvm::StringRef(NewName));
+
+  return FixItHint();
+}
+
+} // namespace
+
+void FunctionNamingCheck::registerMatchers(MatchFinder *Finder) {
+  // This check should only be applied to Objective-C sources.
+  if (!getLangOpts().ObjC)
+return;
+
+  // Match function declarations that are not in system headers and are not
+  // main.
+  Finder->addMatcher(
+  functionDecl(
+  unless(isExpansionInSystemHeader()),
+  unless(anyOf(isMain(), matchesName(validFunctionNameRegex(true)),
+   al

[PATCH] D55101: [clang-tidy] Ignore namespaced and C++ member functions in google-objc-function-naming check 🙈

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore created this revision.
stephanemoore added reviewers: benhamilton, aaron.ballman.
Herald added subscribers: cfe-commits, xazax.hun.

The google-objc-function-naming check applies to functions that are not 
namespaced and should not be applied to C++ member functions. Such function 
declarations should be ignored by the check to avoid false positives in 
Objective-C++ sources.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D55101

Files:
  clang-tidy/google/FunctionNamingCheck.cpp
  test/clang-tidy/google-objc-function-naming.mm


Index: test/clang-tidy/google-objc-function-naming.mm
===
--- /dev/null
+++ test/clang-tidy/google-objc-function-naming.mm
@@ -0,0 +1,28 @@
+// RUN: %check_clang_tidy %s google-objc-function-naming %t
+
+void printSomething() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function name 'printSomething' not 
using function naming conventions described by Google Objective-C style guide
+
+void PrintSomething() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function name 'PrintSomething' not 
using function naming conventions described by Google Objective-C style guide
+
+void ABCBad_Name() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function name 'ABCBad_Name' not 
using function naming conventions described by Google Objective-C style guide
+
+namespace {
+
+int foo() { return 0; }
+
+}
+
+namespace bar {
+
+int convert() { return 0; }
+
+}
+
+class Baz {
+  public:
+int value() { return 0; }
+};
+
Index: clang-tidy/google/FunctionNamingCheck.cpp
===
--- clang-tidy/google/FunctionNamingCheck.cpp
+++ clang-tidy/google/FunctionNamingCheck.cpp
@@ -98,8 +98,9 @@
   // main.
   Finder->addMatcher(
   functionDecl(
-  unless(isExpansionInSystemHeader()),
-  unless(anyOf(isMain(), matchesName(validFunctionNameRegex(true)),
+  unless(anyOf(isExpansionInSystemHeader(), cxxMethodDecl(),
+   hasAncestor(namespaceDecl()), isMain(),
+   matchesName(validFunctionNameRegex(true)),
allOf(isStaticStorageClass(),
  matchesName(validFunctionNameRegex(false))
   .bind("function"),


Index: test/clang-tidy/google-objc-function-naming.mm
===
--- /dev/null
+++ test/clang-tidy/google-objc-function-naming.mm
@@ -0,0 +1,28 @@
+// RUN: %check_clang_tidy %s google-objc-function-naming %t
+
+void printSomething() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function name 'printSomething' not using function naming conventions described by Google Objective-C style guide
+
+void PrintSomething() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function name 'PrintSomething' not using function naming conventions described by Google Objective-C style guide
+
+void ABCBad_Name() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function name 'ABCBad_Name' not using function naming conventions described by Google Objective-C style guide
+
+namespace {
+
+int foo() { return 0; }
+
+}
+
+namespace bar {
+
+int convert() { return 0; }
+
+}
+
+class Baz {
+  public:
+int value() { return 0; }
+};
+
Index: clang-tidy/google/FunctionNamingCheck.cpp
===
--- clang-tidy/google/FunctionNamingCheck.cpp
+++ clang-tidy/google/FunctionNamingCheck.cpp
@@ -98,8 +98,9 @@
   // main.
   Finder->addMatcher(
   functionDecl(
-  unless(isExpansionInSystemHeader()),
-  unless(anyOf(isMain(), matchesName(validFunctionNameRegex(true)),
+  unless(anyOf(isExpansionInSystemHeader(), cxxMethodDecl(),
+   hasAncestor(namespaceDecl()), isMain(),
+   matchesName(validFunctionNameRegex(true)),
allOf(isStaticStorageClass(),
  matchesName(validFunctionNameRegex(false))
   .bind("function"),
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55101: [clang-tidy] Ignore namespaced and C++ member functions in google-objc-function-naming check 🙈

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore updated this revision to Diff 176206.
stephanemoore marked an inline comment as done.
stephanemoore added a comment.

Re-formatted google-objc-function-naming.mm to LLVM style.


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55101/new/

https://reviews.llvm.org/D55101

Files:
  clang-tidy/google/FunctionNamingCheck.cpp
  test/clang-tidy/google-objc-function-naming.mm


Index: test/clang-tidy/google-objc-function-naming.mm
===
--- /dev/null
+++ test/clang-tidy/google-objc-function-naming.mm
@@ -0,0 +1,30 @@
+// RUN: %check_clang_tidy %s google-objc-function-naming %t
+
+void printSomething() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function name 'printSomething' not
+// using function naming conventions described by Google Objective-C style 
guide
+
+void PrintSomething() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function name 'PrintSomething' not
+// using function naming conventions described by Google Objective-C style 
guide
+
+void ABCBad_Name() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function name 'ABCBad_Name' not
+// using function naming conventions described by Google Objective-C style 
guide
+
+namespace {
+
+int foo() { return 0; }
+
+}
+
+namespace bar {
+
+int convert() { return 0; }
+
+}
+
+class Baz {
+public:
+  int value() { return 0; }
+};
Index: clang-tidy/google/FunctionNamingCheck.cpp
===
--- clang-tidy/google/FunctionNamingCheck.cpp
+++ clang-tidy/google/FunctionNamingCheck.cpp
@@ -98,8 +98,9 @@
   // main.
   Finder->addMatcher(
   functionDecl(
-  unless(isExpansionInSystemHeader()),
-  unless(anyOf(isMain(), matchesName(validFunctionNameRegex(true)),
+  unless(anyOf(isExpansionInSystemHeader(), cxxMethodDecl(),
+   hasAncestor(namespaceDecl()), isMain(),
+   matchesName(validFunctionNameRegex(true)),
allOf(isStaticStorageClass(),
  matchesName(validFunctionNameRegex(false))
   .bind("function"),


Index: test/clang-tidy/google-objc-function-naming.mm
===
--- /dev/null
+++ test/clang-tidy/google-objc-function-naming.mm
@@ -0,0 +1,30 @@
+// RUN: %check_clang_tidy %s google-objc-function-naming %t
+
+void printSomething() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function name 'printSomething' not
+// using function naming conventions described by Google Objective-C style guide
+
+void PrintSomething() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function name 'PrintSomething' not
+// using function naming conventions described by Google Objective-C style guide
+
+void ABCBad_Name() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function name 'ABCBad_Name' not
+// using function naming conventions described by Google Objective-C style guide
+
+namespace {
+
+int foo() { return 0; }
+
+}
+
+namespace bar {
+
+int convert() { return 0; }
+
+}
+
+class Baz {
+public:
+  int value() { return 0; }
+};
Index: clang-tidy/google/FunctionNamingCheck.cpp
===
--- clang-tidy/google/FunctionNamingCheck.cpp
+++ clang-tidy/google/FunctionNamingCheck.cpp
@@ -98,8 +98,9 @@
   // main.
   Finder->addMatcher(
   functionDecl(
-  unless(isExpansionInSystemHeader()),
-  unless(anyOf(isMain(), matchesName(validFunctionNameRegex(true)),
+  unless(anyOf(isExpansionInSystemHeader(), cxxMethodDecl(),
+   hasAncestor(namespaceDecl()), isMain(),
+   matchesName(validFunctionNameRegex(true)),
allOf(isStaticStorageClass(),
  matchesName(validFunctionNameRegex(false))
   .bind("function"),
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55101: [clang-tidy] Ignore namespaced and C++ member functions in google-objc-function-naming check 🙈

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore added a comment.

In D55101#1314716 , @benhamilton wrote:

> IMHO we should just disable this check entirely for C++ files (including 
> Objective-C++).
>
> Since Objective-C++ files will have a mix of the Google Objective-C and 
> Google C++ styles, I think there will be too many places where we'll hit 
> conflicts like this (there are likely more).


Would you be okay with landing this fix and if we get any further reports for 
Objective-C++ sources then we can suppress it in all C++/Objective-C++ sources? 
I think there is enough value to enforcing the naming conventions on 
non-namespaced C functions in Objective-C++ to justify a simple followup fix. 
If other issues are reported after this then I also agree that enforcement in 
Objective-C++ sources may incur more overhead than it's worth.


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55101/new/

https://reviews.llvm.org/D55101



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

[PATCH] D55101: [clang-tidy] Ignore namespaced and C++ member functions in google-objc-function-naming check 🙈

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore added a comment.

In D55101#1315294 , @benhamilton wrote:

> > Would you be okay with landing this fix and if we get any further reports 
> > for Objective-C++ sources then we can suppress it in all C++/Objective-C++ 
> > sources? I think there is enough value to enforcing the naming conventions 
> > on non-namespaced C functions in Objective-C++ to justify a simple followup 
> > fix. If other issues are reported after this then I also agree that 
> > enforcement in Objective-C++ sources may incur more overhead than it's 
> > worth.
>
> I'm not against it, but we've already disabled the majority of Objective-C 
> checks for Objective-C++ code, so I don't think this one should apply either.


Within the module named "google-module", I only see that AvoidCStyleCastsCheck 
is disabled for Objective-C++. That check is probably disabled because C-style 
casts on Objective-C types in Objective-C++ code are not seen as a major issue 
and the cost of refactoring code to use or not use C-style casts as code is 
refactored between Objective-C and Objective-C++ sources might be considered 
high.
Within the module named "objc-module", it appears that all checks are currently 
enabled for Objective-C++ code.
Looking across all the clang-tidy modules, AvoidCStyleCastsCheck might actually 
be the only check that is specifically disabled for Objective-C++ (although the 
`grep` I used to posit this assumes that any check that is suppressed in 
Objective-C++ must contain the string `getLangOpts().ObjC`: `grep -r 
"getLangOpts().ObjC" clang-tidy/`).

There are certainly cases where the heterogeneity of Objective-C++ encourages 
disabling a check due to inability to distinguish between whether C++ or 
Objective-C practices should be contextually dominant. I think that 
google-objc-function-naming should be able to cleanly separate declarations 
that should follow C++ or Objective-C patterns. If further bugs are reported 
after this is submitted then I am more willing to concede that I was overly 
optimistic. If it's alright with you, I would prefer to land this fix to give 
google-objc-function-naming another attempt at proper enforcement in 
Objective-C++ and consider disabling it in all C++ variants as a followup to 
further bug reports in Objective-C++.


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55101/new/

https://reviews.llvm.org/D55101



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

[PATCH] D55101: [clang-tidy] Ignore namespaced and C++ member functions in google-objc-function-naming check 🙈

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore added a comment.

Chatted offline with Ben and confirmed that he's okay with proceeding.


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55101/new/

https://reviews.llvm.org/D55101



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

[PATCH] D55101: [clang-tidy] Ignore namespaced and C++ member functions in google-objc-function-naming check 🙈

[Stephane Moore via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL348317: [clang-tidy] Ignore namespaced and C++ member 
functions in google-objc-function… (authored by stephanemoore, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D55101?vs=176206&id=176726#toc

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55101/new/

https://reviews.llvm.org/D55101

Files:
  clang-tools-extra/trunk/clang-tidy/google/FunctionNamingCheck.cpp
  clang-tools-extra/trunk/test/clang-tidy/google-objc-function-naming.mm


Index: clang-tools-extra/trunk/test/clang-tidy/google-objc-function-naming.mm
===
--- clang-tools-extra/trunk/test/clang-tidy/google-objc-function-naming.mm
+++ clang-tools-extra/trunk/test/clang-tidy/google-objc-function-naming.mm
@@ -0,0 +1,30 @@
+// RUN: %check_clang_tidy %s google-objc-function-naming %t
+
+void printSomething() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function name 'printSomething' not
+// using function naming conventions described by Google Objective-C style 
guide
+
+void PrintSomething() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function name 'PrintSomething' not
+// using function naming conventions described by Google Objective-C style 
guide
+
+void ABCBad_Name() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function name 'ABCBad_Name' not
+// using function naming conventions described by Google Objective-C style 
guide
+
+namespace {
+
+int foo() { return 0; }
+
+}
+
+namespace bar {
+
+int convert() { return 0; }
+
+}
+
+class Baz {
+public:
+  int value() { return 0; }
+};
Index: clang-tools-extra/trunk/clang-tidy/google/FunctionNamingCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/google/FunctionNamingCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/google/FunctionNamingCheck.cpp
@@ -98,8 +98,9 @@
   // main.
   Finder->addMatcher(
   functionDecl(
-  unless(isExpansionInSystemHeader()),
-  unless(anyOf(isMain(), matchesName(validFunctionNameRegex(true)),
+  unless(anyOf(isExpansionInSystemHeader(), cxxMethodDecl(),
+   hasAncestor(namespaceDecl()), isMain(),
+   matchesName(validFunctionNameRegex(true)),
allOf(isStaticStorageClass(),
  matchesName(validFunctionNameRegex(false))
   .bind("function"),


Index: clang-tools-extra/trunk/test/clang-tidy/google-objc-function-naming.mm
===
--- clang-tools-extra/trunk/test/clang-tidy/google-objc-function-naming.mm
+++ clang-tools-extra/trunk/test/clang-tidy/google-objc-function-naming.mm
@@ -0,0 +1,30 @@
+// RUN: %check_clang_tidy %s google-objc-function-naming %t
+
+void printSomething() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function name 'printSomething' not
+// using function naming conventions described by Google Objective-C style guide
+
+void PrintSomething() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function name 'PrintSomething' not
+// using function naming conventions described by Google Objective-C style guide
+
+void ABCBad_Name() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function name 'ABCBad_Name' not
+// using function naming conventions described by Google Objective-C style guide
+
+namespace {
+
+int foo() { return 0; }
+
+}
+
+namespace bar {
+
+int convert() { return 0; }
+
+}
+
+class Baz {
+public:
+  int value() { return 0; }
+};
Index: clang-tools-extra/trunk/clang-tidy/google/FunctionNamingCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/google/FunctionNamingCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/google/FunctionNamingCheck.cpp
@@ -98,8 +98,9 @@
   // main.
   Finder->addMatcher(
   functionDecl(
-  unless(isExpansionInSystemHeader()),
-  unless(anyOf(isMain(), matchesName(validFunctionNameRegex(true)),
+  unless(anyOf(isExpansionInSystemHeader(), cxxMethodDecl(),
+   hasAncestor(namespaceDecl()), isMain(),
+   matchesName(validFunctionNameRegex(true)),
allOf(isStaticStorageClass(),
  matchesName(validFunctionNameRegex(false))
   .bind("function"),
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51832: [clang-tidy/checks] Update objc-property-declaration check to allow arbitrary acronyms and initialisms 🔧

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore added a comment.

Chatted with Ben offline and he's okay with proceeding.


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D51832/new/

https://reviews.llvm.org/D51832



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

[PATCH] D51832: [clang-tidy/checks] Update objc-property-declaration check to allow arbitrary acronyms and initialisms 🔧

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore updated this revision to Diff 176756.
stephanemoore added a comment.

Rebased changes and resolved merge conflicts.
Moved release notes update next to other release notes regarding updates to 
existing checks.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D51832/new/

https://reviews.llvm.org/D51832

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

Index: test/clang-tidy/objc-property-declaration.m
===
--- test/clang-tidy/objc-property-declaration.m
+++ test/clang-tidy/objc-property-declaration.m
@@ -1,8 +1,12 @@
 // RUN: %check_clang_tidy %s objc-property-declaration %t
+@class CIColor;
+@class NSArray;
 @class NSData;
 @class NSString;
 @class UIViewController;
 
+typedef void *CGColorRef;
+
 @interface Foo
 @property(assign, nonatomic) int NotCamelCase;
 // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: property name 'NotCamelCase' not using lowerCamelCase style or not prefixed in a category, according to the Apple Coding Guidelines [objc-property-declaration]
@@ -23,6 +27,9 @@
 @property(assign, nonatomic) int enableGLAcceleration;
 @property(assign, nonatomic) int ID;
 @property(assign, nonatomic) int hasADog;
+@property(nonatomic, readonly) CGColorRef CGColor;
+@property(nonatomic, readonly) CIColor *CIColor;
+@property(nonatomic, copy) NSArray *IDs;
 @end
 
 @interface Foo (Bar)
Index: test/clang-tidy/objc-property-declaration-custom.m
===
--- test/clang-tidy/objc-property-declaration-custom.m
+++ /dev/null
@@ -1,18 +0,0 @@
-// 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.IncludeDefaultAcronyms, value: 0}]}' \
-// RUN: --
-@class NSString;
-
-@interface Foo
-@property(assign, nonatomic) int AbcNotRealPrefix;
-// CHECK-MESSAGES: :[[@LINE-1]]:34: warning: property name 'AbcNotRealPrefix' not using lowerCamelCase style or not prefixed in a category, 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' not using lowerCamelCase style or not prefixed in a category, according to the Apple Coding Guidelines [objc-property-declaration]
-@property(assign, nonatomic) int GIFIgnoreStandardAcronym;
-// CHECK-MESSAGES: :[[@LINE-1]]:34: warning: property name 'GIFIgnoreStandardAcronym' not using lowerCamelCase style or not prefixed in a category, according to the Apple Coding Guidelines [objc-property-declaration]
-@property(strong, nonatomic) NSString *TGIF;
-@end
Index: test/clang-tidy/objc-property-declaration-additional.m
===
--- test/clang-tidy/objc-property-declaration-additional.m
+++ /dev/null
@@ -1,15 +0,0 @@
-// 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' not using lowerCamelCase style or not prefixed in a category, 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' not using lowerCamelCase style or not prefixed in a category, 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
@@ -47,23 +47,8 @@
 
 .. option:: Acronyms
 
-   Semicolon-separated list of custom acronyms that can be used as a prefix
-   or a suffix of property names.
-
-   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.
+   This option is deprecated and ignored.
 
 .. option:: IncludeDefaultAcronyms
 
-   Integer value (d

[PATCH] D51832: [clang-tidy/checks] Update objc-property-declaration check to allow arbitrary acronyms and initialisms 🔧

[Stephane Moore via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL348331: [clang-tidy/checks] Update objc-property-declaration 
check to allow arbitrary… (authored by stephanemoore, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D51832?vs=176756&id=176759#toc

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D51832/new/

https://reviews.llvm.org/D51832

Files:
  clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.cpp
  clang-tools-extra/trunk/docs/ReleaseNotes.rst
  clang-tools-extra/trunk/docs/clang-tidy/checks/objc-property-declaration.rst
  clang-tools-extra/trunk/test/clang-tidy/objc-property-declaration-additional.m
  clang-tools-extra/trunk/test/clang-tidy/objc-property-declaration-custom.m
  clang-tools-extra/trunk/test/clang-tidy/objc-property-declaration.m

Index: clang-tools-extra/trunk/test/clang-tidy/objc-property-declaration.m
===
--- clang-tools-extra/trunk/test/clang-tidy/objc-property-declaration.m
+++ clang-tools-extra/trunk/test/clang-tidy/objc-property-declaration.m
@@ -1,8 +1,12 @@
 // RUN: %check_clang_tidy %s objc-property-declaration %t
+@class CIColor;
+@class NSArray;
 @class NSData;
 @class NSString;
 @class UIViewController;
 
+typedef void *CGColorRef;
+
 @interface Foo
 @property(assign, nonatomic) int NotCamelCase;
 // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: property name 'NotCamelCase' not using lowerCamelCase style or not prefixed in a category, according to the Apple Coding Guidelines [objc-property-declaration]
@@ -23,6 +27,9 @@
 @property(assign, nonatomic) int enableGLAcceleration;
 @property(assign, nonatomic) int ID;
 @property(assign, nonatomic) int hasADog;
+@property(nonatomic, readonly) CGColorRef CGColor;
+@property(nonatomic, readonly) CIColor *CIColor;
+@property(nonatomic, copy) NSArray *IDs;
 @end
 
 @interface Foo (Bar)
Index: clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.cpp
@@ -29,104 +29,12 @@
 // For CategoryProperty especially in categories of system class,
 // to avoid naming conflict, the suggested naming style is
 // 'abc_lowerCamelCase' (adding lowercase prefix followed by '_').
+// Regardless of the style, all acronyms and initialisms should be capitalized.
 enum NamingStyle {
   StandardProperty = 1,
   CategoryProperty = 2,
 };
 
-/// The acronyms are aggregated from multiple sources including
-/// https://developer.apple.com/library/content/documentation/Cocoa/Conceptual/CodingGuidelines/Articles/APIAbbreviations.html#//apple_ref/doc/uid/20001285-BCIHCGAE
-///
-/// Keep this list sorted.
-constexpr llvm::StringLiteral DefaultSpecialAcronyms[] = {
-"[2-9]G",
-"ACL",
-"API",
-"APN",
-"APNS",
-"AR",
-"ARGB",
-"ASCII",
-"AV",
-"BGRA",
-"CA",
-"CDN",
-"CF",
-"CG",
-"CI",
-"CRC",
-"CV",
-"CMYK",
-"DNS",
-"FPS",
-"FTP",
-"GIF",
-"GL",
-"GPS",
-"GUID",
-"HD",
-"HDR",
-"HMAC",
-"HTML",
-"HTTP",
-"HTTPS",
-"HUD",
-"ID",
-"JPG",
-"JS",
-"JSON",
-"LAN",
-"LZW",
-"LTR",
-"MAC",
-"MD",
-"MDNS",
-"MIDI",
-"NS",
-"OS",
-"P2P",
-"PDF",
-"PIN",
-"PNG",
-"POI",
-"PSTN",
-"PTR",
-"QA",
-"QOS",
-"RGB",
-"RGBA",
-"RGBX",
-"RIPEMD",
-"ROM",
-"RPC",
-"RTF",
-"RTL",
-"SC",
-"SDK",
-"SHA",
-"SQL",
-"SSO",
-"TCP",
-"TIFF",
-"TOS",
-"TTS",
-"UI",
-"URI",
-"URL",
-"UUID",
-"VC",
-"VO",
-"VOIP",
-"VPN",
-"VR",
-"W",
-"WAN",
-"X",
-"XML",
-"Y",
-"Z",
-};
-
 /// For now we will only fix 'CamelCase' or 'abc_CamelCase' property to
 /// 'camelCase' or 'abc_camelCase'. For other cases the users need to
 /// come up with a proper name by their own.
@@ -150,26 +58,26 @@
   return FixItHint();
 }
 
-std::string AcronymsGroupRegex(llvm::ArrayRef EscapedAcronyms) {
-  return "(" +
- llvm::join(EscapedAcronyms.begin(), EscapedAcronyms.end(), "s?|") +
- "s?)";
-}
-
-std::string validPropertyNameRegex(llvm::ArrayRef EscapedAcronyms,
-   bool UsedInMatcher) {
+std::string validPropertyNameRegex(bool UsedInMatcher) {
   // Allow any of these names:
   // foo
   // fooBar
   // url
   // urlString
+  // ID
+  // IDs
   // URL
   // URLString
   // bundleID
+  // CIColor
+  //
+  // Disallow names of this form:
+  // LongString
+  //
+  // aRbITRaRyCapS is allowed to avoid generating false positives for names
+  // like isVitaminBSupplement, CProgrammingLanguage, and isBeforeM.
   std::st

[PATCH] D55482: [clang-tidy] Improve google-objc-function-naming diagnostics 📙

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore created this revision.
stephanemoore added reviewers: benhamilton, aaron.ballman.
Herald added subscribers: cfe-commits, xazax.hun.

The diagnostics from google-objc-function-naming check will be more
actionable if they provide a brief description of the requirements from
the Google Objective-C style guide. The more descriptive diagnostics may
help clarify that functions in the global namespace must have an
appropriate prefix followed by Pascal case (engineers working previously
with static functions might not immediately understand the different
requirements of static and non-static functions).

Test Notes:
Verified against the clang-tidy tests.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D55482

Files:
  clang-tidy/google/FunctionNamingCheck.cpp
  test/clang-tidy/google-objc-function-naming.m
  test/clang-tidy/google-objc-function-naming.mm

Index: test/clang-tidy/google-objc-function-naming.mm
===
--- test/clang-tidy/google-objc-function-naming.mm
+++ test/clang-tidy/google-objc-function-naming.mm
@@ -1,16 +1,19 @@
 // RUN: %check_clang_tidy %s google-objc-function-naming %t
 
 void printSomething() {}
-// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function name 'printSomething' not
-// using function naming conventions described by Google Objective-C style guide
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: global function name
+// 'printSomething' must have an appropriate prefix followed by Pascal case as
+// required by Google Objective-C style guide
 
 void PrintSomething() {}
-// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function name 'PrintSomething' not
-// using function naming conventions described by Google Objective-C style guide
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: global function name
+// 'PrintSomething' must have an appropriate prefix followed by Pascal case as
+// required by Google Objective-C style guide
 
 void ABCBad_Name() {}
-// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function name 'ABCBad_Name' not
-// using function naming conventions described by Google Objective-C style guide
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: global function name 'ABCBad_Name'
+// must have an appropriate prefix followed by Pascal case as required by Google
+// Objective-C style guide
 
 namespace {
 
Index: test/clang-tidy/google-objc-function-naming.m
===
--- test/clang-tidy/google-objc-function-naming.m
+++ test/clang-tidy/google-objc-function-naming.m
@@ -3,28 +3,35 @@
 typedef _Bool bool;
 
 static bool ispositive(int a) { return a > 0; }
-// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'ispositive' not using function naming conventions described by Google Objective-C style guide
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: static function name 'ispositive'
+// must be in Pascal case as required by Google Objective-C style guide
 // CHECK-FIXES: static bool Ispositive(int a) { return a > 0; }
 
 static bool is_positive(int a) { return a > 0; }
-// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'is_positive' not using function naming conventions described by Google Objective-C style guide
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: static function name 'is_positive'
+// must be in Pascal case as required by Google Objective-C style guide
 // CHECK-FIXES: static bool IsPositive(int a) { return a > 0; }
 
 static bool isPositive(int a) { return a > 0; }
-// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'isPositive' not using function naming conventions described by Google Objective-C style guide
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: static function name 'isPositive'
+// must be in Pascal case as required by Google Objective-C style guide
 // CHECK-FIXES: static bool IsPositive(int a) { return a > 0; }
 
 static bool Is_Positive(int a) { return a > 0; }
-// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'Is_Positive' not using function naming conventions described by Google Objective-C style guide
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: static function name 'Is_Positive'
+// must be in Pascal case as required by Google Objective-C style guide
 // CHECK-FIXES: static bool IsPositive(int a) { return a > 0; }
 
 static bool IsPositive(int a) { return a > 0; }
 
 bool ispalindrome(const char *str);
-// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function name 'ispalindrome' not using function naming conventions described by Google Objective-C style guide
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: global function name 'ispalindrome'
+// must have an appropriate prefix followed by Pascal case as required by Google
+// Objective-C style guide
 
 static const char *md5(const char *str) { return 0; }
-// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: function name 'md5' not using function naming conventions described by Google Objective-C style guide
+// CHECK-MESSAGES: :[[@LINE-1]]:20: warning

[PATCH] D55482: [clang-tidy] Improve google-objc-function-naming diagnostics 📙

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore updated this revision to Diff 177609.
stephanemoore marked 4 inline comments as done.
stephanemoore added a comment.

Use the select modifier to customize diagnostics for static and non-static 
function instead of branching diagnostic code paths for static and non-static 
functions.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55482/new/

https://reviews.llvm.org/D55482

Files:
  clang-tidy/google/FunctionNamingCheck.cpp
  test/clang-tidy/google-objc-function-naming.m
  test/clang-tidy/google-objc-function-naming.mm

Index: test/clang-tidy/google-objc-function-naming.mm
===
--- test/clang-tidy/google-objc-function-naming.mm
+++ test/clang-tidy/google-objc-function-naming.mm
@@ -1,16 +1,19 @@
 // RUN: %check_clang_tidy %s google-objc-function-naming %t
 
 void printSomething() {}
-// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function name 'printSomething' not
-// using function naming conventions described by Google Objective-C style guide
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: global function name
+// 'printSomething' must have an appropriate prefix followed by Pascal case as
+// required by Google Objective-C style guide
 
 void PrintSomething() {}
-// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function name 'PrintSomething' not
-// using function naming conventions described by Google Objective-C style guide
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: global function name
+// 'PrintSomething' must have an appropriate prefix followed by Pascal case as
+// required by Google Objective-C style guide
 
 void ABCBad_Name() {}
-// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function name 'ABCBad_Name' not
-// using function naming conventions described by Google Objective-C style guide
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: global function name 'ABCBad_Name'
+// must have an appropriate prefix followed by Pascal case as required by Google
+// Objective-C style guide
 
 namespace {
 
Index: test/clang-tidy/google-objc-function-naming.m
===
--- test/clang-tidy/google-objc-function-naming.m
+++ test/clang-tidy/google-objc-function-naming.m
@@ -3,28 +3,35 @@
 typedef _Bool bool;
 
 static bool ispositive(int a) { return a > 0; }
-// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'ispositive' not using function naming conventions described by Google Objective-C style guide
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: static function name 'ispositive'
+// must be in Pascal case as required by Google Objective-C style guide
 // CHECK-FIXES: static bool Ispositive(int a) { return a > 0; }
 
 static bool is_positive(int a) { return a > 0; }
-// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'is_positive' not using function naming conventions described by Google Objective-C style guide
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: static function name 'is_positive'
+// must be in Pascal case as required by Google Objective-C style guide
 // CHECK-FIXES: static bool IsPositive(int a) { return a > 0; }
 
 static bool isPositive(int a) { return a > 0; }
-// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'isPositive' not using function naming conventions described by Google Objective-C style guide
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: static function name 'isPositive'
+// must be in Pascal case as required by Google Objective-C style guide
 // CHECK-FIXES: static bool IsPositive(int a) { return a > 0; }
 
 static bool Is_Positive(int a) { return a > 0; }
-// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'Is_Positive' not using function naming conventions described by Google Objective-C style guide
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: static function name 'Is_Positive'
+// must be in Pascal case as required by Google Objective-C style guide
 // CHECK-FIXES: static bool IsPositive(int a) { return a > 0; }
 
 static bool IsPositive(int a) { return a > 0; }
 
 bool ispalindrome(const char *str);
-// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function name 'ispalindrome' not using function naming conventions described by Google Objective-C style guide
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: global function name 'ispalindrome'
+// must have an appropriate prefix followed by Pascal case as required by Google
+// Objective-C style guide
 
 static const char *md5(const char *str) { return 0; }
-// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: function name 'md5' not using function naming conventions described by Google Objective-C style guide
+// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: static function name 'md5' must be
+// in Pascal case as required by Google Objective-C style guide
 // CHECK-FIXES: static const char *Md5(const char *str) { return 0; }
 
 static const char *MD5(const char *str) { return 0; }
@@ -38,12 +45,16 @@
 static const char *StringFromNSString(id str) { return ""; }
 
 void ABLog_String(const char

[PATCH] D55482: [clang-tidy] Improve google-objc-function-naming diagnostics 📙

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore added inline comments.



Comment at: clang-tidy/google/FunctionNamingCheck.cpp:114-125
+  if (MatchedDecl->getStorageClass() == SC_Static) {
+diag(MatchedDecl->getLocation(),
+ "static function name %0 must be in Pascal case as required by "
+ "Google Objective-C style guide")
+<< MatchedDecl << generateFixItHint(MatchedDecl);
+return;
+  }

aaron.ballman wrote:
> I'd rather see these diagnostics combined: `%select{static|global}1 function 
> name %0 must %select{be in |have an appropriate prefixed followed by "}1 
> Pascal case as required by the Google Objective-C style guide` to simplify 
> the logic (since `generateFixItHint()` already does the right thing).
I had no idea about the select modifier! Good idea!



Comment at: test/clang-tidy/google-objc-function-naming.m:8
+// must be in Pascal case as required by Google Objective-C style guide
 // CHECK-FIXES: static bool Ispositive(int a) { return a > 0; }
 

MyDeveloperDay wrote:
> I realize there are words that begin with 'is...' but you could detect if the 
> function returned a boolean and started with "is,has,does" and could this 
> extrapolate to  IsPositive()  HasSomething(), DoesHaveSomething(),  this 
> might generate a better fixit candidate? (just a suggestion feel free to 
> ignore)
Sounds like something worth investigating.

Filed https://bugs.llvm.org/show_bug.cgi?id=39941


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55482/new/

https://reviews.llvm.org/D55482



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

[PATCH] D55482: [clang-tidy] Improve google-objc-function-naming diagnostics 📙

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore marked 4 inline comments as done.
stephanemoore added inline comments.



Comment at: clang-tidy/google/FunctionNamingCheck.cpp:114-118
   diag(MatchedDecl->getLocation(),
-   "function name %0 not using function naming conventions described by "
-   "Google Objective-C style guide")
-  << MatchedDecl << generateFixItHint(MatchedDecl);
+   "%select{static|global}1 function name %0 must %select{be in|have an "
+   "appropriate prefix followed by}1 Pascal case as required by Google "
+   "Objective-C style guide")
+  << MatchedDecl << IsGlobal << generateFixItHint(MatchedDecl);

benhamilton wrote:
> Should we suggest making the function static when it fails this check? (I 
> assume the vast majority of functions which fail this check should really be 
> static.)
Are you asking whether we should suggest making global functions static when 
they are missing appropriate prefixes? If so, I think that would lead to 
undesired changes in various cases.

One common cause of this style violation is when someone endeavors to take a 
static function that is used in one translation unit and share it with other 
translation units by migrating it to a header. The Easy™ thing to do is migrate 
that static function to a header and convert it directly to a global function, 
avoiding the overhead of refactoring the function name and all call sites. The 
effort of the described refactoring approach is low compared to renaming the 
function to avoid symbol collisions in the global namespace as is generally 
recommended.

If a function's definition and declaration (if it exists) are both in an 
implementation file we can probably recommend making the function static 
though. Filed https://bugs.llvm.org/show_bug.cgi?id=39945.



Comment at: clang-tidy/google/FunctionNamingCheck.cpp:115
   diag(MatchedDecl->getLocation(),
-   "function name %0 not using function naming conventions described by "
-   "Google Objective-C style guide")
-  << MatchedDecl << generateFixItHint(MatchedDecl);
+   "%select{static|global}1 function name %0 must %select{be in|have an "
+   "appropriate prefix followed by}1 Pascal case as required by Google "

benhamilton wrote:
> I know "global" is the correct name, but I feel like "non-static" might be 
> clearer to folks dealing with these error messages.
> 
> WDYT?
> 
I'm wary of saying "non-static" because namespaced functions in Objective-C++ 
are not subject to the cited rules. The term "non-static" seems like it could 
be interpreted to extend to namespaced functions which could potentially 
mislead someone into adding prefixes to namespaced functions in Objective-C++.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55482/new/

https://reviews.llvm.org/D55482



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

[PATCH] D55544: Warning: objc-encodings-larger-than=

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore added a comment.

FYI:
I have a clang-tidy check almost ready for review that aims to flag large 
Objective-C type encodings.




Comment at: include/clang/Basic/LangOptions.def:103
+BENIGN_LANGOPT(ObjCLargeEncodingSize, 32, 0,
+  "if non-zero, warn about Objective C encodings larger in bytes 
than this setting. 0 is no check.")
 LANGOPT(AppExt , 1, 0, "Objective-C App Extension")

Objective-C


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55544/new/

https://reviews.llvm.org/D55544



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

[PATCH] D55546: [clang] Add AST matcher for block expressions 🔍

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore created this revision.
Herald added a subscriber: cfe-commits.

This change adds a new AST matcher for block expressions.

Test Notes:
Ran the clang unit tests.


Repository:
  rC Clang

https://reviews.llvm.org/D55546

Files:
  docs/LibASTMatchersReference.html
  include/clang/ASTMatchers/ASTMatchers.h
  lib/ASTMatchers/ASTMatchersInternal.cpp
  lib/ASTMatchers/Dynamic/Registry.cpp
  unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp


Index: unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -1386,6 +1386,10 @@
 hasDeclaration(namedDecl(hasName("y"));
 }
 
+TEST(BlockExprMatcher, BlockExpr) {
+  EXPECT_TRUE(matchesObjC("void f() { ^{}(); }", blockExpr()));
+}
+
 TEST(StatementCountIs, FindsNoStatementsInAnEmptyCompoundStatement) {
   EXPECT_TRUE(matches("void f() { }",
   compoundStmt(statementCountIs(0;
Index: lib/ASTMatchers/Dynamic/Registry.cpp
===
--- lib/ASTMatchers/Dynamic/Registry.cpp
+++ lib/ASTMatchers/Dynamic/Registry.cpp
@@ -139,6 +139,7 @@
   REGISTER_MATCHER(binaryConditionalOperator);
   REGISTER_MATCHER(binaryOperator);
   REGISTER_MATCHER(blockDecl);
+  REGISTER_MATCHER(blockExpr);
   REGISTER_MATCHER(blockPointerType);
   REGISTER_MATCHER(booleanType);
   REGISTER_MATCHER(breakStmt);
Index: lib/ASTMatchers/ASTMatchersInternal.cpp
===
--- lib/ASTMatchers/ASTMatchersInternal.cpp
+++ lib/ASTMatchers/ASTMatchersInternal.cpp
@@ -688,6 +688,7 @@
 const internal::VariadicDynCastAllOfMatcher expr;
 const internal::VariadicDynCastAllOfMatcher declRefExpr;
 const internal::VariadicDynCastAllOfMatcher 
objcIvarRefExpr;
+const internal::VariadicDynCastAllOfMatcher blockExpr;
 const internal::VariadicDynCastAllOfMatcher ifStmt;
 const internal::VariadicDynCastAllOfMatcher forStmt;
 const internal::VariadicDynCastAllOfMatcher
Index: include/clang/ASTMatchers/ASTMatchers.h
===
--- include/clang/ASTMatchers/ASTMatchers.h
+++ include/clang/ASTMatchers/ASTMatchers.h
@@ -1776,6 +1776,14 @@
 extern const internal::VariadicDynCastAllOfMatcher
 objcIvarRefExpr;
 
+/// Matches a reference to a block.
+///
+/// Example: matches "^{}":
+/// \code
+///   void f() { ^{}(); }
+/// \endcode
+extern const internal::VariadicDynCastAllOfMatcher blockExpr;
+
 /// Matches if statements.
 ///
 /// Example matches 'if (x) {}'
Index: docs/LibASTMatchersReference.html
===
--- docs/LibASTMatchersReference.html
+++ docs/LibASTMatchersReference.html
@@ -707,6 +707,14 @@
 
 
 
+MatcherStmt>blockExprMatcherBlockExpr>...
+MAtches a reference to a 
block.
+
+Example: matches "^{}":
+  void f() { ^{}(); }
+
+
+
 MatcherStmt>breakStmtMatcherBreakStmt>...
 Matches break statements.
 


Index: unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -1386,6 +1386,10 @@
 hasDeclaration(namedDecl(hasName("y"));
 }
 
+TEST(BlockExprMatcher, BlockExpr) {
+  EXPECT_TRUE(matchesObjC("void f() { ^{}(); }", blockExpr()));
+}
+
 TEST(StatementCountIs, FindsNoStatementsInAnEmptyCompoundStatement) {
   EXPECT_TRUE(matches("void f() { }",
   compoundStmt(statementCountIs(0;
Index: lib/ASTMatchers/Dynamic/Registry.cpp
===
--- lib/ASTMatchers/Dynamic/Registry.cpp
+++ lib/ASTMatchers/Dynamic/Registry.cpp
@@ -139,6 +139,7 @@
   REGISTER_MATCHER(binaryConditionalOperator);
   REGISTER_MATCHER(binaryOperator);
   REGISTER_MATCHER(blockDecl);
+  REGISTER_MATCHER(blockExpr);
   REGISTER_MATCHER(blockPointerType);
   REGISTER_MATCHER(booleanType);
   REGISTER_MATCHER(breakStmt);
Index: lib/ASTMatchers/ASTMatchersInternal.cpp
===
--- lib/ASTMatchers/ASTMatchersInternal.cpp
+++ lib/ASTMatchers/ASTMatchersInternal.cpp
@@ -688,6 +688,7 @@
 const internal::VariadicDynCastAllOfMatcher expr;
 const internal::VariadicDynCastAllOfMatcher declRefExpr;
 const internal::VariadicDynCastAllOfMatcher objcIvarRefExpr;
+const internal::VariadicDynCastAllOfMatcher blockExpr;
 const internal::VariadicDynCastAllOfMatcher ifStmt;
 const internal::VariadicDynCastAllOfMatcher forStmt;
 const internal::VariadicDynCastAllOfMatche

[PATCH] D55482: [clang-tidy] Improve google-objc-function-naming diagnostics 📙

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore updated this revision to Diff 177795.
stephanemoore marked 5 inline comments as done.
stephanemoore added a comment.

Changes:
• Drop const on local bool variable.
• Adopt the term "function in global namespace" in diagnostic messages.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55482/new/

https://reviews.llvm.org/D55482

Files:
  clang-tidy/google/FunctionNamingCheck.cpp
  test/clang-tidy/google-objc-function-naming.m
  test/clang-tidy/google-objc-function-naming.mm

Index: test/clang-tidy/google-objc-function-naming.mm
===
--- test/clang-tidy/google-objc-function-naming.mm
+++ test/clang-tidy/google-objc-function-naming.mm
@@ -1,16 +1,19 @@
 // RUN: %check_clang_tidy %s google-objc-function-naming %t
 
 void printSomething() {}
-// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function name 'printSomething' not
-// using function naming conventions described by Google Objective-C style guide
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function in global namespace named
+// 'printSomething' must have an appropriate prefix followed by Pascal case as
+// required by Google Objective-C style guide
 
 void PrintSomething() {}
-// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function name 'PrintSomething' not
-// using function naming conventions described by Google Objective-C style guide
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function in global namespace named
+// 'PrintSomething' must have an appropriate prefix followed by Pascal case as
+// required by Google Objective-C style guide
 
 void ABCBad_Name() {}
-// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function name 'ABCBad_Name' not
-// using function naming conventions described by Google Objective-C style guide
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function in global namespace named
+// 'ABCBad_Name' must have an appropriate prefix followed by Pascal case as
+// required by Google Objective-C style guide
 
 namespace {
 
Index: test/clang-tidy/google-objc-function-naming.m
===
--- test/clang-tidy/google-objc-function-naming.m
+++ test/clang-tidy/google-objc-function-naming.m
@@ -3,28 +3,35 @@
 typedef _Bool bool;
 
 static bool ispositive(int a) { return a > 0; }
-// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'ispositive' not using function naming conventions described by Google Objective-C style guide
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: static function named 'ispositive'
+// must be in Pascal case as required by Google Objective-C style guide
 // CHECK-FIXES: static bool Ispositive(int a) { return a > 0; }
 
 static bool is_positive(int a) { return a > 0; }
-// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'is_positive' not using function naming conventions described by Google Objective-C style guide
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: static function named 'is_positive'
+// must be in Pascal case as required by Google Objective-C style guide
 // CHECK-FIXES: static bool IsPositive(int a) { return a > 0; }
 
 static bool isPositive(int a) { return a > 0; }
-// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'isPositive' not using function naming conventions described by Google Objective-C style guide
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: static function named 'isPositive'
+// must be in Pascal case as required by Google Objective-C style guide
 // CHECK-FIXES: static bool IsPositive(int a) { return a > 0; }
 
 static bool Is_Positive(int a) { return a > 0; }
-// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'Is_Positive' not using function naming conventions described by Google Objective-C style guide
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: static function named 'Is_Positive'
+// must be in Pascal case as required by Google Objective-C style guide
 // CHECK-FIXES: static bool IsPositive(int a) { return a > 0; }
 
 static bool IsPositive(int a) { return a > 0; }
 
 bool ispalindrome(const char *str);
-// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function name 'ispalindrome' not using function naming conventions described by Google Objective-C style guide
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function in global namespace named
+// 'ispalindrome' must have an appropriate prefix followed by Pascal case as
+// required by Google Objective-C style guide
 
 static const char *md5(const char *str) { return 0; }
-// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: function name 'md5' not using function naming conventions described by Google Objective-C style guide
+// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: static function named 'md5' must be
+// in Pascal case as required by Google Objective-C style guide
 // CHECK-FIXES: static const char *Md5(const char *str) { return 0; }
 
 static const char *MD5(const char *str) { return 0; }
@@ -38,12 +45,16 @@
 static const char *StringFromNSString(id str) { return ""; }
 
 void ABLog_Stri

[PATCH] D59336: [clang-tidy] Disable google-runtime-int in Objective-C++ 🔓

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore created this revision.
Herald added subscribers: cfe-commits, jdoerfert, xazax.hun.
Herald added a project: clang.

In contrast to Google C++, Objective-C often uses built-in integer types
other than `int`. In fact, the Objective-C runtime itself defines the
types NSInteger¹ and NSUInteger² which are variant types depending on
the target architecture. The Objective-C style guide indicates that
usage of system types with variant sizes is appropriate when handling
values provided by system interfaces³. Objective-C++ is commonly the
result of conversion from Objective-C to Objective-C++ for the purpose
of integrating C++ functionality. The opposite of Objective-C++ being
used to expose Objective-C functionality to C++ is less common,
potentially because Objective-C has a signficantly more uneven presence
on different platforms compared to C++. This generally predisposes
Objective-C++ to commonly being more Objective-C than C++. Forcing
Objective-C++ developers to perform conversions between variant system types
and fixed size integer types depending on target architecture when
Objective-C++ commonly uses variant system types from Objective-C is
likely to lead to more bugs and overhead than benefit. For that reason,
this change proposes to disable google-runtime-int in Objective-C++.

[1] https://developer.apple.com/documentation/objectivec/nsinteger?language=objc
[2] 
https://developer.apple.com/documentation/objectivec/nsuinteger?language=objc
[3] "Types long, NSInteger, NSUInteger, and CGFloat vary in size between
32- and 64-bit builds. Use of these types is appropriate when handling
values exposed by system interfaces, but they should be avoided for most
other computations."
https://github.com/google/styleguide/blob/gh-pages/objcguide.md#types-with-inconsistent-sizes


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D59336

Files:
  clang-tools-extra/clang-tidy/google/IntegerTypesCheck.cpp


Index: clang-tools-extra/clang-tidy/google/IntegerTypesCheck.cpp
===
--- clang-tools-extra/clang-tidy/google/IntegerTypesCheck.cpp
+++ clang-tools-extra/clang-tidy/google/IntegerTypesCheck.cpp
@@ -54,7 +54,9 @@
 
 void IntegerTypesCheck::registerMatchers(MatchFinder *Finder) {
   // Find all TypeLocs. The relevant Style Guide rule only applies to C++.
-  if (!getLangOpts().CPlusPlus)
+  // This check is also not applied in Objective-C++ sources as Objective-C
+  // often uses built-in integer types other than `int`.
+  if (!getLangOpts().CPlusPlus || getLangOpts().ObjC)
 return;
   // Match any integer types, unless they are passed to a printf-based API:
   //


Index: clang-tools-extra/clang-tidy/google/IntegerTypesCheck.cpp
===
--- clang-tools-extra/clang-tidy/google/IntegerTypesCheck.cpp
+++ clang-tools-extra/clang-tidy/google/IntegerTypesCheck.cpp
@@ -54,7 +54,9 @@
 
 void IntegerTypesCheck::registerMatchers(MatchFinder *Finder) {
   // Find all TypeLocs. The relevant Style Guide rule only applies to C++.
-  if (!getLangOpts().CPlusPlus)
+  // This check is also not applied in Objective-C++ sources as Objective-C
+  // often uses built-in integer types other than `int`.
+  if (!getLangOpts().CPlusPlus || getLangOpts().ObjC)
 return;
   // Match any integer types, unless they are passed to a printf-based API:
   //
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59336: [clang-tidy] Disable google-runtime-int in Objective-C++ 🔓

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore updated this revision to Diff 190520.
stephanemoore added a comment.

Document change in release notes.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59336/new/

https://reviews.llvm.org/D59336

Files:
  clang-tools-extra/clang-tidy/google/IntegerTypesCheck.cpp
  clang-tools-extra/docs/ReleaseNotes.rst


Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -115,6 +115,9 @@
   `CommentUserDefiniedLiterals`, `CommentStringLiterals`,
   `CommentCharacterLiterals` & `CommentNullPtrs` options.
 
+- The :doc:`google-runtime-int `
+  check has been disabled in Objective-C++.
+
 - The `Acronyms` and `IncludeDefaultAcronyms` options for the
   :doc:`objc-property-declaration 
`
   check have been removed.
Index: clang-tools-extra/clang-tidy/google/IntegerTypesCheck.cpp
===
--- clang-tools-extra/clang-tidy/google/IntegerTypesCheck.cpp
+++ clang-tools-extra/clang-tidy/google/IntegerTypesCheck.cpp
@@ -54,7 +54,9 @@
 
 void IntegerTypesCheck::registerMatchers(MatchFinder *Finder) {
   // Find all TypeLocs. The relevant Style Guide rule only applies to C++.
-  if (!getLangOpts().CPlusPlus)
+  // This check is also not applied in Objective-C++ sources as Objective-C
+  // often uses built-in integer types other than `int`.
+  if (!getLangOpts().CPlusPlus || getLangOpts().ObjC)
 return;
   // Match any integer types, unless they are passed to a printf-based API:
   //


Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -115,6 +115,9 @@
   `CommentUserDefiniedLiterals`, `CommentStringLiterals`,
   `CommentCharacterLiterals` & `CommentNullPtrs` options.
 
+- The :doc:`google-runtime-int `
+  check has been disabled in Objective-C++.
+
 - The `Acronyms` and `IncludeDefaultAcronyms` options for the
   :doc:`objc-property-declaration `
   check have been removed.
Index: clang-tools-extra/clang-tidy/google/IntegerTypesCheck.cpp
===
--- clang-tools-extra/clang-tidy/google/IntegerTypesCheck.cpp
+++ clang-tools-extra/clang-tidy/google/IntegerTypesCheck.cpp
@@ -54,7 +54,9 @@
 
 void IntegerTypesCheck::registerMatchers(MatchFinder *Finder) {
   // Find all TypeLocs. The relevant Style Guide rule only applies to C++.
-  if (!getLangOpts().CPlusPlus)
+  // This check is also not applied in Objective-C++ sources as Objective-C
+  // often uses built-in integer types other than `int`.
+  if (!getLangOpts().CPlusPlus || getLangOpts().ObjC)
 return;
   // Match any integer types, unless they are passed to a printf-based API:
   //
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59336: [clang-tidy] Disable google-runtime-int in Objective-C++ 🔓

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore added a comment.

Let me know if you think this change would benefit from added tests.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59336/new/

https://reviews.llvm.org/D59336



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

[PATCH] D59336: [clang-tidy] Disable google-runtime-int in Objective-C++ 🔓

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore marked 2 inline comments as done.
stephanemoore added a comment.

Thanks for the review!




Comment at: clang-tools-extra/clang-tidy/google/IntegerTypesCheck.cpp:57
   // Find all TypeLocs. The relevant Style Guide rule only applies to C++.
-  if (!getLangOpts().CPlusPlus)
+  // This check is also not applied in Objective-C++ sources as Objective-C
+  // often uses built-in integer types other than `int`.

gribozavr wrote:
> gribozavr wrote:
> > "Objective-C and Objective-C++"?  Similarly in release notes.
> I see -- should have read the previous sentence, which you are not changing.  
> Never mind.
Yup, the previous sentence indicates that the check targets C++ and this added 
sentence indicates that Objective-C++ is considered an exception.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59336/new/

https://reviews.llvm.org/D59336



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

[PATCH] D59336: [clang-tidy] Disable google-runtime-int in Objective-C++ 🔓

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore updated this revision to Diff 190742.
stephanemoore added a comment.

Add google-runtime-int tests for Objective-C and Objective-C++.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59336/new/

https://reviews.llvm.org/D59336

Files:
  clang-tools-extra/clang-tidy/google/IntegerTypesCheck.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/test/clang-tidy/google-runtime-int.m
  clang-tools-extra/test/clang-tidy/google-runtime-int.mm

Index: clang-tools-extra/test/clang-tidy/google-runtime-int.mm
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/google-runtime-int.mm
@@ -0,0 +1,31 @@
+// RUN: clang-tidy -checks=-*,google-runtime-int %s -- -x objective-c++ 2>&1 | not grep 'warning:\|error:'
+
+typedef long NSInteger;
+typedef unsigned long NSUInteger;
+
+@interface NSString
+@property(readonly) NSInteger integerValue;
+@property(readonly) long long longLongValue;
+@property(readonly) NSUInteger length;
+@end
+
+NSInteger Foo(NSString *s) {
+  return [s integerValue];
+}
+
+long long Bar(NSString *s) {
+  return [s longLongValue];
+}
+
+NSUInteger Baz(NSString *s) {
+  return [s length];
+}
+
+unsigned short NSSwapShort(unsigned short inv);
+
+long DoSomeMath(long a, short b) {
+  short c = NSSwapShort(b);
+  long a2 = a * 5L;
+  return a2 + c;
+}
+
Index: clang-tools-extra/test/clang-tidy/google-runtime-int.m
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/google-runtime-int.m
@@ -0,0 +1,31 @@
+// RUN: clang-tidy -checks=-*,google-runtime-int %s 2>&1 -- -x objective-c | not grep 'warning:\|error:'
+
+typedef long NSInteger;
+typedef unsigned long NSUInteger;
+
+@interface NSString
+@property(readonly) NSInteger integerValue;
+@property(readonly) long long longLongValue;
+@property(readonly) NSUInteger length;
+@end
+
+NSInteger Foo(NSString *s) {
+  return [s integerValue];
+}
+
+long long Bar(NSString *s) {
+  return [s longLongValue];
+}
+
+NSUInteger Baz(NSString *s) {
+  return [s length];
+}
+
+unsigned short NSSwapShort(unsigned short inv);
+
+long DoSomeMath(long a, short b) {
+  short c = NSSwapShort(b);
+  long a2 = a * 5L;
+  return a2 + c;
+}
+
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -115,6 +115,9 @@
   `CommentUserDefiniedLiterals`, `CommentStringLiterals`,
   `CommentCharacterLiterals` & `CommentNullPtrs` options.
 
+- The :doc:`google-runtime-int `
+  check has been disabled in Objective-C++.
+
 - The `Acronyms` and `IncludeDefaultAcronyms` options for the
   :doc:`objc-property-declaration `
   check have been removed.
Index: clang-tools-extra/clang-tidy/google/IntegerTypesCheck.cpp
===
--- clang-tools-extra/clang-tidy/google/IntegerTypesCheck.cpp
+++ clang-tools-extra/clang-tidy/google/IntegerTypesCheck.cpp
@@ -54,7 +54,9 @@
 
 void IntegerTypesCheck::registerMatchers(MatchFinder *Finder) {
   // Find all TypeLocs. The relevant Style Guide rule only applies to C++.
-  if (!getLangOpts().CPlusPlus)
+  // This check is also not applied in Objective-C++ sources as Objective-C
+  // often uses built-in integer types other than `int`.
+  if (!getLangOpts().CPlusPlus || getLangOpts().ObjC)
 return;
   // Match any integer types, unless they are passed to a printf-based API:
   //
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59336: [clang-tidy] Disable google-runtime-int in Objective-C++ 🔓

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore added a comment.

In D59336#1429564 , @aaron.ballman 
wrote:

> This is missing test cases that demonstrate the behavior is what we expect it 
> to be for ObjC++ code vs C++ code.


Added tests for Objective-C and Objective-C++.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59336/new/

https://reviews.llvm.org/D59336



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

[PATCH] D59336: [clang-tidy] Disable google-runtime-int in Objective-C++ 🔓

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore updated this revision to Diff 191198.
stephanemoore marked 2 inline comments as done.
stephanemoore added a comment.

Updated with changes:
• Switched to using `| count 0` instead of grep'ing for warnings.
• Merged Objective-C and Objective-C++ test cases into single file with 
multiple runs.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59336/new/

https://reviews.llvm.org/D59336

Files:
  clang-tools-extra/clang-tidy/google/IntegerTypesCheck.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/test/clang-tidy/google-runtime-int.m


Index: clang-tools-extra/test/clang-tidy/google-runtime-int.m
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/google-runtime-int.m
@@ -0,0 +1,32 @@
+// RUN: clang-tidy -checks=-*,google-runtime-int %s 2>&1 -- -x objective-c | 
count 0
+// RUN: clang-tidy -checks=-*,google-runtime-int %s 2>&1 -- -x objective-c++ | 
count 0
+
+typedef long NSInteger;
+typedef unsigned long NSUInteger;
+
+@interface NSString
+@property(readonly) NSInteger integerValue;
+@property(readonly) long long longLongValue;
+@property(readonly) NSUInteger length;
+@end
+
+NSInteger Foo(NSString *s) {
+  return [s integerValue];
+}
+
+long long Bar(NSString *s) {
+  return [s longLongValue];
+}
+
+NSUInteger Baz(NSString *s) {
+  return [s length];
+}
+
+unsigned short NSSwapShort(unsigned short inv);
+
+long DoSomeMath(long a, short b) {
+  short c = NSSwapShort(b);
+  long a2 = a * 5L;
+  return a2 + c;
+}
+
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -115,6 +115,9 @@
   `CommentUserDefiniedLiterals`, `CommentStringLiterals`,
   `CommentCharacterLiterals` & `CommentNullPtrs` options.
 
+- The :doc:`google-runtime-int `
+  check has been disabled in Objective-C++.
+
 - The `Acronyms` and `IncludeDefaultAcronyms` options for the
   :doc:`objc-property-declaration 
`
   check have been removed.
Index: clang-tools-extra/clang-tidy/google/IntegerTypesCheck.cpp
===
--- clang-tools-extra/clang-tidy/google/IntegerTypesCheck.cpp
+++ clang-tools-extra/clang-tidy/google/IntegerTypesCheck.cpp
@@ -54,7 +54,9 @@
 
 void IntegerTypesCheck::registerMatchers(MatchFinder *Finder) {
   // Find all TypeLocs. The relevant Style Guide rule only applies to C++.
-  if (!getLangOpts().CPlusPlus)
+  // This check is also not applied in Objective-C++ sources as Objective-C
+  // often uses built-in integer types other than `int`.
+  if (!getLangOpts().CPlusPlus || getLangOpts().ObjC)
 return;
   // Match any integer types, unless they are passed to a printf-based API:
   //


Index: clang-tools-extra/test/clang-tidy/google-runtime-int.m
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/google-runtime-int.m
@@ -0,0 +1,32 @@
+// RUN: clang-tidy -checks=-*,google-runtime-int %s 2>&1 -- -x objective-c | count 0
+// RUN: clang-tidy -checks=-*,google-runtime-int %s 2>&1 -- -x objective-c++ | count 0
+
+typedef long NSInteger;
+typedef unsigned long NSUInteger;
+
+@interface NSString
+@property(readonly) NSInteger integerValue;
+@property(readonly) long long longLongValue;
+@property(readonly) NSUInteger length;
+@end
+
+NSInteger Foo(NSString *s) {
+  return [s integerValue];
+}
+
+long long Bar(NSString *s) {
+  return [s longLongValue];
+}
+
+NSUInteger Baz(NSString *s) {
+  return [s length];
+}
+
+unsigned short NSSwapShort(unsigned short inv);
+
+long DoSomeMath(long a, short b) {
+  short c = NSSwapShort(b);
+  long a2 = a * 5L;
+  return a2 + c;
+}
+
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -115,6 +115,9 @@
   `CommentUserDefiniedLiterals`, `CommentStringLiterals`,
   `CommentCharacterLiterals` & `CommentNullPtrs` options.
 
+- The :doc:`google-runtime-int `
+  check has been disabled in Objective-C++.
+
 - The `Acronyms` and `IncludeDefaultAcronyms` options for the
   :doc:`objc-property-declaration `
   check have been removed.
Index: clang-tools-extra/clang-tidy/google/IntegerTypesCheck.cpp
===
--- clang-tools-extra/clang-tidy/google/IntegerTypesCheck.cpp
+++ clang-tools-extra/clang-tidy/google/IntegerTypesCheck.cpp
@@ -54,7 +54,9 @@
 
 void IntegerTypesCheck::registerMatchers(MatchFinder *Finder) {
   // Find all TypeLocs. The relevant Style Guide rule only applies to C++.
-  if (!getLangOpts().CPlusPlus)
+  // This check is also not applied in Objective-C++ sources as Objective-C
+  // often uses built-in integer types other than `int`.
+  if (!getLangOpts().CPl

[PATCH] D59336: [clang-tidy] Disable google-runtime-int in Objective-C++ 🔓

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore added inline comments.



Comment at: clang-tools-extra/test/clang-tidy/google-runtime-int.m:1
+// RUN: clang-tidy -checks=-*,google-runtime-int %s 2>&1 -- -x objective-c | 
not grep 'warning:\|error:'
+

aaron.ballman wrote:
> We typically use `| count 0` instead of an explicit grep. Also, do you really 
> need to pass `-x objective-c`?
I had chosen to use `grep` and had explicitly specified the language to be 
consistent with 
[google-runtime-int.c](https://github.com/llvm/llvm-project/blob/master/clang-tools-extra/test/clang-tidy/google-runtime-int.c#L1).
 I agree that `| count 0` is more common and have switched to that approach.

I thought that I should be able to remove `-x objective-c` but when I try to I 
get an error loading the compilation database:
```
Error while trying to load a compilation database:
Could not auto-detect compilation database for file 
"path_to_llvm_project/clang-tools-extra/test/clang-tidy/google-runtime-int.m"
No compilation database found in 
path_to_llvm_project/clang-tools-extra/test/clang-tidy or any parent directory
fixed-compilation-database: Error while opening fixed database: No such file or 
directory
json-compilation-database: Error while opening JSON database: No such file or 
directory
Running without flags.
```
I am still investigating the underlying reason.



Comment at: clang-tools-extra/test/clang-tidy/google-runtime-int.mm:1
+// RUN: clang-tidy -checks=-*,google-runtime-int %s -- -x objective-c++ 2>&1 | 
not grep 'warning:\|error:'
+

aaron.ballman wrote:
> Given that the contents of the file are the same, I would just add this RUN 
> line to the previous test case (changing to `| count 0`). You will still need 
> the `-x objective-c++` in that case.
Makes sense. Done.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59336/new/

https://reviews.llvm.org/D59336



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

[PATCH] D59336: [clang-tidy] Disable google-runtime-int in Objective-C++ 🔓

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore updated this revision to Diff 191219.
stephanemoore added a comment.

Remove redundant specification of `-x objective-c`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59336/new/

https://reviews.llvm.org/D59336

Files:
  clang-tools-extra/clang-tidy/google/IntegerTypesCheck.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/test/clang-tidy/google-runtime-int.m


Index: clang-tools-extra/test/clang-tidy/google-runtime-int.m
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/google-runtime-int.m
@@ -0,0 +1,32 @@
+// RUN: clang-tidy -checks=-*,google-runtime-int %s 2>&1 -- | count 0
+// RUN: clang-tidy -checks=-*,google-runtime-int %s 2>&1 -- -x objective-c++ | 
count 0
+
+typedef long NSInteger;
+typedef unsigned long NSUInteger;
+
+@interface NSString
+@property(readonly) NSInteger integerValue;
+@property(readonly) long long longLongValue;
+@property(readonly) NSUInteger length;
+@end
+
+NSInteger Foo(NSString *s) {
+  return [s integerValue];
+}
+
+long long Bar(NSString *s) {
+  return [s longLongValue];
+}
+
+NSUInteger Baz(NSString *s) {
+  return [s length];
+}
+
+unsigned short NSSwapShort(unsigned short inv);
+
+long DoSomeMath(long a, short b) {
+  short c = NSSwapShort(b);
+  long a2 = a * 5L;
+  return a2 + c;
+}
+
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -115,6 +115,9 @@
   `CommentUserDefiniedLiterals`, `CommentStringLiterals`,
   `CommentCharacterLiterals` & `CommentNullPtrs` options.
 
+- The :doc:`google-runtime-int `
+  check has been disabled in Objective-C++.
+
 - The `Acronyms` and `IncludeDefaultAcronyms` options for the
   :doc:`objc-property-declaration 
`
   check have been removed.
Index: clang-tools-extra/clang-tidy/google/IntegerTypesCheck.cpp
===
--- clang-tools-extra/clang-tidy/google/IntegerTypesCheck.cpp
+++ clang-tools-extra/clang-tidy/google/IntegerTypesCheck.cpp
@@ -54,7 +54,9 @@
 
 void IntegerTypesCheck::registerMatchers(MatchFinder *Finder) {
   // Find all TypeLocs. The relevant Style Guide rule only applies to C++.
-  if (!getLangOpts().CPlusPlus)
+  // This check is also not applied in Objective-C++ sources as Objective-C
+  // often uses built-in integer types other than `int`.
+  if (!getLangOpts().CPlusPlus || getLangOpts().ObjC)
 return;
   // Match any integer types, unless they are passed to a printf-based API:
   //


Index: clang-tools-extra/test/clang-tidy/google-runtime-int.m
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/google-runtime-int.m
@@ -0,0 +1,32 @@
+// RUN: clang-tidy -checks=-*,google-runtime-int %s 2>&1 -- | count 0
+// RUN: clang-tidy -checks=-*,google-runtime-int %s 2>&1 -- -x objective-c++ | count 0
+
+typedef long NSInteger;
+typedef unsigned long NSUInteger;
+
+@interface NSString
+@property(readonly) NSInteger integerValue;
+@property(readonly) long long longLongValue;
+@property(readonly) NSUInteger length;
+@end
+
+NSInteger Foo(NSString *s) {
+  return [s integerValue];
+}
+
+long long Bar(NSString *s) {
+  return [s longLongValue];
+}
+
+NSUInteger Baz(NSString *s) {
+  return [s length];
+}
+
+unsigned short NSSwapShort(unsigned short inv);
+
+long DoSomeMath(long a, short b) {
+  short c = NSSwapShort(b);
+  long a2 = a * 5L;
+  return a2 + c;
+}
+
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -115,6 +115,9 @@
   `CommentUserDefiniedLiterals`, `CommentStringLiterals`,
   `CommentCharacterLiterals` & `CommentNullPtrs` options.
 
+- The :doc:`google-runtime-int `
+  check has been disabled in Objective-C++.
+
 - The `Acronyms` and `IncludeDefaultAcronyms` options for the
   :doc:`objc-property-declaration `
   check have been removed.
Index: clang-tools-extra/clang-tidy/google/IntegerTypesCheck.cpp
===
--- clang-tools-extra/clang-tidy/google/IntegerTypesCheck.cpp
+++ clang-tools-extra/clang-tidy/google/IntegerTypesCheck.cpp
@@ -54,7 +54,9 @@
 
 void IntegerTypesCheck::registerMatchers(MatchFinder *Finder) {
   // Find all TypeLocs. The relevant Style Guide rule only applies to C++.
-  if (!getLangOpts().CPlusPlus)
+  // This check is also not applied in Objective-C++ sources as Objective-C
+  // often uses built-in integer types other than `int`.
+  if (!getLangOpts().CPlusPlus || getLangOpts().ObjC)
 return;
   // Match any integer types, unless they are passed to a printf-based API:
   //
___
cfe-commits mailing list
cfe-

[PATCH] D59336: [clang-tidy] Disable google-runtime-int in Objective-C++ 🔓

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore marked 3 inline comments as done.
stephanemoore added inline comments.



Comment at: clang-tools-extra/test/clang-tidy/google-runtime-int.m:1
+// RUN: clang-tidy -checks=-*,google-runtime-int %s 2>&1 -- -x objective-c | 
not grep 'warning:\|error:'
+

stephanemoore wrote:
> aaron.ballman wrote:
> > We typically use `| count 0` instead of an explicit grep. Also, do you 
> > really need to pass `-x objective-c`?
> I had chosen to use `grep` and had explicitly specified the language to be 
> consistent with 
> [google-runtime-int.c](https://github.com/llvm/llvm-project/blob/master/clang-tools-extra/test/clang-tidy/google-runtime-int.c#L1).
>  I agree that `| count 0` is more common and have switched to that approach.
> 
> I thought that I should be able to remove `-x objective-c` but when I try to 
> I get an error loading the compilation database:
> ```
> Error while trying to load a compilation database:
> Could not auto-detect compilation database for file 
> "path_to_llvm_project/clang-tools-extra/test/clang-tidy/google-runtime-int.m"
> No compilation database found in 
> path_to_llvm_project/clang-tools-extra/test/clang-tidy or any parent directory
> fixed-compilation-database: Error while opening fixed database: No such file 
> or directory
> json-compilation-database: Error while opening JSON database: No such file or 
> directory
> Running without flags.
> ```
> I am still investigating the underlying reason.
Apparently it was because I removed the `--` as well 😅 I have now removed `-x 
objective-c`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59336/new/

https://reviews.llvm.org/D59336



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

[PATCH] D59336: [clang-tidy] Disable google-runtime-int in Objective-C++ 🔓

[Stephane Moore via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE356627: [clang-tidy] Disable google-runtime-int in 
Objective-C++ 🔓 (authored by stephanemoore, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D59336?vs=191219&id=191599#toc

Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59336/new/

https://reviews.llvm.org/D59336

Files:
  clang-tidy/google/IntegerTypesCheck.cpp
  docs/ReleaseNotes.rst
  test/clang-tidy/google-runtime-int.m


Index: clang-tidy/google/IntegerTypesCheck.cpp
===
--- clang-tidy/google/IntegerTypesCheck.cpp
+++ clang-tidy/google/IntegerTypesCheck.cpp
@@ -54,7 +54,9 @@
 
 void IntegerTypesCheck::registerMatchers(MatchFinder *Finder) {
   // Find all TypeLocs. The relevant Style Guide rule only applies to C++.
-  if (!getLangOpts().CPlusPlus)
+  // This check is also not applied in Objective-C++ sources as Objective-C
+  // often uses built-in integer types other than `int`.
+  if (!getLangOpts().CPlusPlus || getLangOpts().ObjC)
 return;
   // Match any integer types, unless they are passed to a printf-based API:
   //
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -115,6 +115,9 @@
   `CommentUserDefiniedLiterals`, `CommentStringLiterals`,
   `CommentCharacterLiterals` & `CommentNullPtrs` options.
 
+- The :doc:`google-runtime-int `
+  check has been disabled in Objective-C++.
+
 - The `Acronyms` and `IncludeDefaultAcronyms` options for the
   :doc:`objc-property-declaration 
`
   check have been removed.
Index: test/clang-tidy/google-runtime-int.m
===
--- test/clang-tidy/google-runtime-int.m
+++ test/clang-tidy/google-runtime-int.m
@@ -0,0 +1,32 @@
+// RUN: clang-tidy -checks=-*,google-runtime-int %s 2>&1 -- | count 0
+// RUN: clang-tidy -checks=-*,google-runtime-int %s 2>&1 -- -x objective-c++ | 
count 0
+
+typedef long NSInteger;
+typedef unsigned long NSUInteger;
+
+@interface NSString
+@property(readonly) NSInteger integerValue;
+@property(readonly) long long longLongValue;
+@property(readonly) NSUInteger length;
+@end
+
+NSInteger Foo(NSString *s) {
+  return [s integerValue];
+}
+
+long long Bar(NSString *s) {
+  return [s longLongValue];
+}
+
+NSUInteger Baz(NSString *s) {
+  return [s length];
+}
+
+unsigned short NSSwapShort(unsigned short inv);
+
+long DoSomeMath(long a, short b) {
+  short c = NSSwapShort(b);
+  long a2 = a * 5L;
+  return a2 + c;
+}
+


Index: clang-tidy/google/IntegerTypesCheck.cpp
===
--- clang-tidy/google/IntegerTypesCheck.cpp
+++ clang-tidy/google/IntegerTypesCheck.cpp
@@ -54,7 +54,9 @@
 
 void IntegerTypesCheck::registerMatchers(MatchFinder *Finder) {
   // Find all TypeLocs. The relevant Style Guide rule only applies to C++.
-  if (!getLangOpts().CPlusPlus)
+  // This check is also not applied in Objective-C++ sources as Objective-C
+  // often uses built-in integer types other than `int`.
+  if (!getLangOpts().CPlusPlus || getLangOpts().ObjC)
 return;
   // Match any integer types, unless they are passed to a printf-based API:
   //
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -115,6 +115,9 @@
   `CommentUserDefiniedLiterals`, `CommentStringLiterals`,
   `CommentCharacterLiterals` & `CommentNullPtrs` options.
 
+- The :doc:`google-runtime-int `
+  check has been disabled in Objective-C++.
+
 - The `Acronyms` and `IncludeDefaultAcronyms` options for the
   :doc:`objc-property-declaration `
   check have been removed.
Index: test/clang-tidy/google-runtime-int.m
===
--- test/clang-tidy/google-runtime-int.m
+++ test/clang-tidy/google-runtime-int.m
@@ -0,0 +1,32 @@
+// RUN: clang-tidy -checks=-*,google-runtime-int %s 2>&1 -- | count 0
+// RUN: clang-tidy -checks=-*,google-runtime-int %s 2>&1 -- -x objective-c++ | count 0
+
+typedef long NSInteger;
+typedef unsigned long NSUInteger;
+
+@interface NSString
+@property(readonly) NSInteger integerValue;
+@property(readonly) long long longLongValue;
+@property(readonly) NSUInteger length;
+@end
+
+NSInteger Foo(NSString *s) {
+  return [s integerValue];
+}
+
+long long Bar(NSString *s) {
+  return [s longLongValue];
+}
+
+NSUInteger Baz(NSString *s) {
+  return [s length];
+}
+
+unsigned short NSSwapShort(unsigned short inv);
+
+long DoSomeMath(long a, short b) {
+  short c = NSSwapShort(b);
+  long a2 = a * 5L;
+  return a2 + c;
+}
+
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59336: [clang-tidy] Disable google-runtime-int in Objective-C++ 🔓

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore marked an inline comment as done.
stephanemoore added a comment.

Thanks for the review!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59336/new/

https://reviews.llvm.org/D59336



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