dev/core#2814 fix tokenCompat to be consistent with unresolved tokens

Civi/Token/TokenCompatSubscriber.php

@@ -411,29 +411,17 @@ protected function getFieldMetadata(): array {
    * Apply the various CRM_Utils_Token helpers.
    *
    * @param \Civi\Token\Event\TokenRenderEvent $e
-   *
-   * @throws \CRM_Core_Exception
    */
   public function onRender(TokenRenderEvent $e): void {
-    $isHtml = ($e->message['format'] === 'text/html');
     $useSmarty = !empty($e->context['smarty']);
-
-    if (!empty($e->context['contact'])) {
-      // @todo - remove this - it simply removes the last unresolved tokens before
-      // they break smarty.
-      // historically it was only called when context['contact'] so that is
-      // retained but it only works because it's almost always true.
-      $remainingTokens = array_keys(\CRM_Utils_Token::getTokens($e->string));
-      if (!empty($remainingTokens)) {
-        $e->string = \CRM_Utils_Token::replaceHookTokens($e->string, $e->context['contact'], $remainingTokens);
-      }
-    }
+    $e->string = $e->getTokenProcessor()->visitTokens($e->string, function() {
+      // For historical consistency, we filter out unrecognized tokens.
+      return '';
+    });
 
     if ($useSmarty) {
       $smartyVars = [];
       foreach ($e->context['smartyTokenAlias'] ?? [] as $smartyName => $tokenName) {
-        // Note: $e->row->tokens resolves event-based tokens (eg CRM_*_Tokens). But if the target token relies on the
-        // above bits (replaceGreetingTokens=>replaceContactTokens=>replaceHookTokens) then this lookup isn't sufficient.
         $smartyVars[$smartyName] = \CRM_Utils_Array::pathGet($e->row->tokens, explode('.', $tokenName));
       }
       \CRM_Core_Smarty::singleton()->pushScope($smartyVars);

Civi/Token/TokenProcessor.php

@@ -386,15 +386,37 @@ public function render($name, $row) {
     return $event->string;
   }
 
-  private function visitTokens(string $expression, callable $callback): string {
+  /**
+   * Examine a token string and filter each token expression.
+   *
+   * @internal
+   *   This function is only intended for use within civicrm-core. The name/location/callback-signature may change.
+   * @param string $expression
+   *   Ex: 'Hello {foo.bar} and {whiz.bang|filter:"arg"}!'
+   * @param callable $callback
+   *   A function which visits (and substitutes) each token.
+   *   function(?string $fullToken, ?string $entity, ?string $field, ?array $modifier)
+   * @return string
+   */
+  public function visitTokens(string $expression, callable $callback): string {
     // Regex examples: '{foo.bar}', '{foo.bar|whiz}', '{foo.bar|whiz:"bang"}', '{foo.bar|whiz:"bang":"bang"}'
     // Regex counter-examples: '{foobar}', '{foo bar}', '{$foo.bar}', '{$foo.bar|whiz}', '{foo.bar|whiz{bang}}'
     // Key observations: Civi tokens MUST have a `.` and MUST NOT have a `$`. Civi filters MUST NOT have `{}`s or `$`s.
-    $tokRegex = '([\w]+)\.([\w:\.]+)'; /* EX: 'foo.bar' in '{foo.bar|whiz:"bang":"bang"}' */
-    $argRegex = ':[\w": %\-_()\[\]\+/#@!,\.\?]*'; /* EX: ':"bang":"bang"' in '{foo.bar|whiz:"bang":"bang"}' */
-    // Debatable: Maybe relax to this: $argRegex = ':[^{}\n]*'; /* EX: ':"bang":"bang"' in '{foo.bar|whiz:"bang":"bang"}' */
-    $filterRegex = "(\w+(?:$argRegex)?)"; /* EX: 'whiz:"bang"' in '{foo.bar|whiz:"bang"' */
-    return preg_replace_callback(";\{$tokRegex(?:\|$filterRegex)?\};", function($m) use ($callback) {
+
+    static $fullRegex = NULL;
+    if ($fullRegex === NULL) {
+      // The regex is a bit complicated, we so break it down into fragments.
+      // Consider the example '{foo.bar|whiz:"bang":"bang"}'. Each fragment matches the following:
+
+      $tokenRegex = '([\w]+)\.([\w:\.]+)'; /* MATCHES: 'foo.bar' */
+      $filterArgRegex = ':[\w": %\-_()\[\]\+/#@!,\.\?]*'; /* MATCHES: ':"bang":"bang"' */
+      // Key rule of filterArgRegex is to prohibit '{}'s because they may parse ambiguously. So you *might* relax it to:
+      // $filterArgRegex = ':[^{}\n]*'; /* MATCHES: ':"bang":"bang"' */
+      $filterNameRegex = "\w+"; /* MATCHES: 'whiz' */
+      $filterRegex = "\|($filterNameRegex(?:$filterArgRegex)?)"; /* MATCHES: '|whiz:"bang":"bang"' */
+      $fullRegex = ";\{$tokenRegex(?:$filterRegex)?\};";
+    }
+    return preg_replace_callback($fullRegex, function($m) use ($callback) {
       $filterParts = NULL;
       if (isset($m[3])) {
         $filterParts = [];

tests/phpunit/CRM/Activity/Form/Task/PDFLetterCommonTest.php

@@ -140,15 +140,16 @@ public function testCreateDocumentSpecialTokens(): void {
   }
 
   /**
+   * Unknown tokens are removed at the very end.
+   *
    * @throws \CRM_Core_Exception
    * @throws \CiviCRM_API3_Exception
    */
   public function testCreateDocumentUnknownTokens(): void {
     $activity = $this->activityCreate();
-    $html_message = 'Unknown token: {activity.something_unknown}';
+    $html_message = 'Unknown token: ';
     $form = $this->getFormObject('CRM_Activity_Form_Task_PDF');
     $output = $form->createDocument([$activity['id']], $html_message, ['is_unit_test' => TRUE]);
-    // Unknown tokens should be left alone
     $this->assertEquals($html_message, $output[0]);
   }
 

tests/phpunit/CRM/Core/BAO/MessageTemplateTest.php

@@ -397,6 +397,20 @@ public function hookTokenValues(array &$details): void {
     }
   }
 
+  /**
+   * Test that unresolved tokens are not causing a fatal error in smarty.
+   *
+   * @throws \API_Exception
+   * @throws \CRM_Core_Exception
+   */
+  public function testUnresolvedTokens(): void {
+    CRM_Core_BAO_MessageTemplate::renderTemplate([
+      'messageTemplate' => [
+        'msg_text' => '{contact.blah}',
+      ],
+    ])['text'];
+  }
+
   /**
    * Hook to advertise tokens.
    *

tests/phpunit/Civi/Token/TokenProcessorTest.php

@@ -1,7 +1,6 @@
 <?php
 namespace Civi\Token;
 
-use Civi\Test\Invasive;
 use Civi\Token\Event\TokenRegisterEvent;
 use Civi\Token\Event\TokenValueEvent;
 use Symfony\Component\EventDispatcher\EventDispatcher;
@@ -45,19 +44,23 @@ public function testVisitTokens() {
       '{foo.bar}' => ['foo', 'bar', NULL],
       '{foo.bar|whiz}' => ['foo', 'bar', ['whiz']],
       '{foo.bar|whiz:"bang"}' => ['foo', 'bar', ['whiz', 'bang']],
-      '{love.shack|place:"bang":"b@ng, on +he/([do0r])?!"}' => ['love', 'shack', ['place', 'bang', 'b@ng, on +he/([do0r])?!']],
+      '{FoO.bAr|whiz:"bang"}' => ['FoO', 'bAr', ['whiz', 'bang']],
+      '{oo_f.ra_b|b_52:"bang":"b@ng, on +he/([do0r])?!"}' => ['oo_f', 'ra_b', ['b_52', 'bang', 'b@ng, on +he/([do0r])?!']],
+      '{foo.bar.whiz}' => ['foo', 'bar.whiz', NULL],
+      '{foo.bar.whiz|bang}' => ['foo', 'bar.whiz', ['bang']],
+      '{foo.bar:label}' => ['foo', 'bar:label', NULL],
+      '{foo.bar:label|truncate:"10"}' => ['foo', 'bar:label', ['truncate', '10']],
     ];
     foreach ($examples as $input => $expected) {
       array_unshift($expected, $input);
       $log = [];
-      Invasive::call([$p, 'visitTokens'], [
-        $input,
-        function (?string $fullToken, ?string $entity, ?string $field, ?array $modifier) use (&$log) {
-          $log[] = [$fullToken, $entity, $field, $modifier];
-        },
-      ]);
+      $filtered = $p->visitTokens($input, function (?string $fullToken, ?string $entity, ?string $field, ?array $modifier) use (&$log) {
+        $log[] = [$fullToken, $entity, $field, $modifier];
+        return 'Replaced!';
+      });
       $this->assertEquals(1, count($log), "Should receive one callback on expression: $input");
       $this->assertEquals($expected, $log[0]);
+      $this->assertEquals('Replaced!', $filtered);
     }
   }