[lldb][ObjC][NFC] Rewrite IsPossibleObjCMethodName in terms of llvm::StringRef

[Michael137]

We've seen some crashes around this area (particularly around checking/handling raw C-strings). Dealing with StringRefs makes it a bit easier to reason about.

This doesn't fix anything per se, but is an improvement in readability.

rdar://164519648

[llvmbot]

@llvm/pr-subscribers-lldb

Author: Michael Buch (Michael137)

Changes

We've seen some crashes around this area (particularly around checking/handling raw C-strings). Dealing with StringRefs makes it a bit easier to reason about.

This doesn't fix anything per se, but is an improvement in readability.

rdar://164519648

---

Full diff: https://github.com/llvm/llvm-project/pull/167660.diff

3 Files Affected:

• (modified) lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp (+7)
• (modified) lldb/source/Plugins/Language/ObjC/ObjCLanguage.h (+1-7)
• (modified) lldb/unittests/Language/ObjC/ObjCLanguageTest.cpp (+43)

diff --git a/lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp b/lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp
index 3b8e21cbb9269..6d31c96ded601 100644
--- a/lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp
+++ b/lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp
@@ -1065,3 +1065,10 @@ ObjCLanguage::GetBooleanFromString(llvm::StringRef str) const {
       .Case("NO", {false})
       .Default({});
 }
+
+bool ObjCLanguage::IsPossibleObjCMethodName(llvm::StringRef name) {
+  if (!name.starts_with("-[") && !name.starts_with("+["))
+    return false;
+
+  return name.ends_with("]");
+}
diff --git a/lldb/source/Plugins/Language/ObjC/ObjCLanguage.h b/lldb/source/Plugins/Language/ObjC/ObjCLanguage.h
index a68ea41c723de..aed1a68c2aea6 100644
--- a/lldb/source/Plugins/Language/ObjC/ObjCLanguage.h
+++ b/lldb/source/Plugins/Language/ObjC/ObjCLanguage.h
@@ -175,13 +175,7 @@ class ObjCLanguage : public Language {
 
   static llvm::StringRef GetPluginNameStatic() { return "objc"; }
 
-  static bool IsPossibleObjCMethodName(const char *name) {
-    if (!name)
-      return false;
-    bool starts_right = (name[0] == '+' || name[0] == '-') && name[1] == '[';
-    bool ends_right = (name[strlen(name) - 1] == ']');
-    return (starts_right && ends_right);
-  }
+  static bool IsPossibleObjCMethodName(llvm::StringRef name);
 
   static bool IsPossibleObjCSelector(const char *name) {
     if (!name)
diff --git a/lldb/unittests/Language/ObjC/ObjCLanguageTest.cpp b/lldb/unittests/Language/ObjC/ObjCLanguageTest.cpp
index 70baa7e6bc135..4b018a29f3587 100644
--- a/lldb/unittests/Language/ObjC/ObjCLanguageTest.cpp
+++ b/lldb/unittests/Language/ObjC/ObjCLanguageTest.cpp
@@ -112,3 +112,46 @@ TEST(ObjCLanguage, InvalidMethodNameParsing) {
     EXPECT_FALSE(lax_method.has_value());
   }
 }
+
+struct ObjCMethodTestCase {
+  llvm::StringRef name;
+  bool is_valid;
+};
+
+struct ObjCMethodNameTextFiture
+    : public testing::TestWithParam<ObjCMethodTestCase> {};
+
+static ObjCMethodTestCase g_objc_method_name_test_cases[] = {
+    {"", false},
+    {"+[Uh oh!", false},
+    {"-[Definitely not...", false},
+    {"[Nice try ] :)", false},
+    {"+MaybeIfYouSquintYourEyes]", false},
+    {"?[Tricky]", false},
+    {"[]", false},
+    {"-[a", false},
+    {"+[a", false},
+    {"-]a]", false},
+    {"+]a]", false},
+
+    // FIXME: should these count as valid?
+    {"+[]", true},
+    {"-[]", true},
+    {"-[[]", true},
+    {"+[[]", true},
+    {"+[a ]", true},
+    {"-[a ]", true},
+
+    // Valid names
+    {"+[a a]", true},
+    {"-[a a]", true},
+};
+
+TEST_P(ObjCMethodNameTextFiture, TestIsPossibleObjCMethodName) {
+  // Tests ObjCLanguage::IsPossibleObjCMethodName
+  auto [name, expect_valid] = GetParam();
+  EXPECT_EQ(ObjCLanguage::IsPossibleObjCMethodName(name), expect_valid);
+}
+
+INSTANTIATE_TEST_SUITE_P(ObjCMethodNameTests, ObjCMethodNameTextFiture,
+                         testing::ValuesIn(g_objc_method_name_test_cases));

[felipepiovezan]

nice tests!