Allow a wider range of characters in macro names, including emoji

Summary:
Fixes T6121. See PHI357.

  - Allow emoji and other unicode (like Chinese characters) as long as you have at least three of them.
  - Disallow macros with only latin symbols. These were previously allowed.

Test Plan: Created a macro for "🐶🐶🐶", then used it in a comment.

Maniphest Tasks: T6121

Differential Revision: https://secure.phabricator.com/D19051

src/__phutil_library_map__.php

@@ -3203,6 +3203,7 @@ phutil_register_library_map(array(
     'PhabricatorMacroQuery' => 'applications/macro/query/PhabricatorMacroQuery.php',
     'PhabricatorMacroReplyHandler' => 'applications/macro/mail/PhabricatorMacroReplyHandler.php',
     'PhabricatorMacroSearchEngine' => 'applications/macro/query/PhabricatorMacroSearchEngine.php',
+    'PhabricatorMacroTestCase' => 'applications/macro/xaction/__tests__/PhabricatorMacroTestCase.php',
     'PhabricatorMacroTransaction' => 'applications/macro/storage/PhabricatorMacroTransaction.php',
     'PhabricatorMacroTransactionComment' => 'applications/macro/storage/PhabricatorMacroTransactionComment.php',
     'PhabricatorMacroTransactionQuery' => 'applications/macro/query/PhabricatorMacroTransactionQuery.php',
@@ -8751,6 +8752,7 @@ phutil_register_library_map(array(
     'PhabricatorMacroQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
     'PhabricatorMacroReplyHandler' => 'PhabricatorApplicationTransactionReplyHandler',
     'PhabricatorMacroSearchEngine' => 'PhabricatorApplicationSearchEngine',
+    'PhabricatorMacroTestCase' => 'PhabricatorTestCase',
     'PhabricatorMacroTransaction' => 'PhabricatorModularTransaction',
     'PhabricatorMacroTransactionComment' => 'PhabricatorApplicationTransactionComment',
     'PhabricatorMacroTransactionQuery' => 'PhabricatorApplicationTransactionQuery',

src/applications/macro/markup/PhabricatorImageMacroRemarkupRule.php

@@ -8,7 +8,7 @@ final class PhabricatorImageMacroRemarkupRule extends PhutilRemarkupRule {
 
   public function apply($text) {
     return preg_replace_callback(
-      '@^\s*([a-zA-Z0-9:_\-]+)$@m',
+      '@^\s*([a-zA-Z0-9:_\x7f-\xff-]+)$@m',
       array($this, 'markupImageMacro'),
       $text);
   }

src/applications/macro/xaction/PhabricatorMacroNameTransaction.php

@@ -52,12 +52,16 @@ final class PhabricatorMacroNameTransaction
           new PhutilNumber($max_length)));
       }
 
-      if (!preg_match('/^[a-z0-9:_-]{3,}\z/', $new_value)) {
+      if (!self::isValidMacroName($new_value)) {
+        // This says "emoji", but the actual rule we implement is "all other
+        // unicode characters are also fine".
         $errors[] = $this->newInvalidError(
-        pht('Macro name "%s" be at least three characters long and contain '.
+          pht(
-            'only lowercase letters, digits, hyphens, colons and '.
+            'Macro name "%s" be: at least three characters long; and contain '.
-            'underscores.',
+            'only lowercase letters, digits, hyphens, colons, underscores, '.
-            $new_value));
+            'and emoji; and not be composed entirely of latin symbols.',
+            $new_value),
+          $xaction);
       }
 
       // Check name is unique when updating / creating
@@ -78,4 +82,35 @@ final class PhabricatorMacroNameTransaction
     return $errors;
   }
 
+  public static function isValidMacroName($name) {
+    if (preg_match('/^[:_-]+\z/', $name)) {
+      return false;
+    }
+
+    // Accept trivial macro names.
+    if (preg_match('/^[a-z0-9:_-]{3,}\z/', $name)) {
+      return true;
+    }
+
+    // Reject names with fewer than 3 glyphs.
+    $length = phutil_utf8v_combined($name);
+    if (count($length) < 3) {
+      return false;
+    }
+
+    // Check character-by-character for any symbols that we don't want.
+    $characters = phutil_utf8v($name);
+    foreach ($characters as $character) {
+      if (ord($character[0]) > 0x7F) {
+        continue;
+      }
+
+      if (preg_match('/^[^a-z0-9:_-]/', $character)) {
+        return false;
+      }
+    }
+
+    return true;
+  }
+
 }

src/applications/macro/xaction/__tests__/PhabricatorMacroTestCase.php

@@ -0,0 +1,46 @@
+<?php
+
+final class PhabricatorMacroTestCase
+  extends PhabricatorTestCase {
+
+  public function testMacroNames() {
+    $lit = "\xF0\x9F\x94\xA5";
+    $combining_diaeresis = "\xCC\x88";
+
+    $cases = array(
+      // Only 2 glyphs long.
+      "u{$combining_diaeresis}n" => false,
+      "{$lit}{$lit}" => false,
+
+      // Too short.
+      'a' => false,
+      '' => false,
+
+      // Bad characters.
+      'yes!' => false,
+      "{$lit} {$lit} {$lit}" => false,
+      "aaa\nbbb" => false,
+      'aaa~' => false,
+      'aaa`' => false,
+
+      // Special rejections for only latin symbols.
+      '---' => false,
+      '___' => false,
+      '-_-' => false,
+      ':::' => false,
+      '-_:' => false,
+
+      "{$lit}{$lit}{$lit}" => true,
+      'bwahahaha' => true,
+      "u{$combining_diaeresis}nt" => true,
+      'a-a-a-a' => true,
+    );
+
+    foreach ($cases as $input => $expect) {
+      $this->assertEqual(
+        $expect,
+        PhabricatorMacroNameTransaction::isValidMacroName($input),
+        pht('Validity of macro "%s"', $input));
+    }
+  }
+}