Fixes #4823 and #6005: yii message accuracy and error handling were improved by using PHP tokenizer instead of regular expressions. Removed eval() as well.

framework/CHANGELOG.md

@@ -5,6 +5,7 @@ Yii Framework 2 Change Log
 -----------------------
 
 - Bug #4471: `yii\caching\ApcCache::getValues()` now returns array in case of APC is installed but not enabled in CLI mode (samdark, cebe)
+- Bug #4823: `yii message` accuracy and error handling were improved (samdark)
 - Bug #4889: Application was getting into redirect loop when user wasn't allowed accessing login page. Now shows 403 (samdark)
 - Bug #5402: Debugger was not loading when there were closures in asset classes (samdark)
 - Bug #5452: Errors occurring after the response is sent are not displayed (qiangxue) 

framework/console/controllers/MessageController.php

@@ -244,39 +244,102 @@ class MessageController extends Controller
      */
     protected function extractMessages($fileName, $translator)
     {
-        echo "Extracting messages from $fileName...\n";
+        $coloredFileName = Console::ansiFormat($fileName, [Console::FG_CYAN]);
+        $this->stdout("Extracting messages from $coloredFileName...\n");
         $subject = file_get_contents($fileName);
         $messages = [];
         if (!is_array($translator)) {
             $translator = [$translator];
         }
         foreach ($translator as $currentTranslator) {
-            $n = preg_match_all(
+            $translatorTokens = token_get_all('<?php ' . $currentTranslator);
-                '/\b' . $currentTranslator . '\s*\(\s*(\'.*?(?<!\\\\)\'|".*?(?<!\\\\)")\s*,\s*(\'.*?(?<!\\\\)\'|".*?(?<!\\\\)")\s*[,\)]/s',
+            array_shift($translatorTokens);
-                $subject,
-                $matches,
-                PREG_SET_ORDER
-            );
-            for ($i = 0; $i < $n; ++$i) {
-                $category = substr($matches[$i][1], 1, -1);
-                $message = $matches[$i][2];
-                try {
-                    $messages[$category][] = eval("return {$message};"); // use eval to eliminate quote escape
-                } catch (ErrorException $e) {
-                    $category = Console::ansiFormat($category, [Console::FG_CYAN]);
-                    $message = Console::ansiFormat($message, [Console::FG_CYAN]);
-                    $fileName = Console::ansiFormat($fileName, [Console::FG_CYAN]);
-                    $error = Console::ansiFormat($e->getMessage(), [Console::FG_RED]);
 
-                    $this->stdout("Failed parsing $fileName, $message in $category category:\n" . $error . "\n");
+            $translatorTokensCount = count($translatorTokens);
-                    Yii::$app->end(self::EXIT_CODE_ERROR);
+            $matchedTokensCount = 0;
+            $buffer = [];
+
+            $tokens = token_get_all($subject);
+            foreach ($tokens as $token) {
+                // finding out translator call
+                if ($matchedTokensCount < $translatorTokensCount) {
+                    if ($this->tokensEqual($token, $translatorTokens[$matchedTokensCount])) {
+                        $matchedTokensCount++;
+                    } else {
+                        $matchedTokensCount = 0;
+                    }
+                } elseif ($matchedTokensCount === $translatorTokensCount) {
+                    // translator found
+
+                    // end of translator call or end of something that we can't extract
+                    if ($this->tokensEqual(')', $token)) {
+                        if (isset($buffer[0][0], $buffer[1], $buffer[2][0]) && $buffer[0][0] === T_CONSTANT_ENCAPSED_STRING && $buffer[1] === ',' && $buffer[2][0] === T_CONSTANT_ENCAPSED_STRING) {
+                            // is valid call we can extract
+
+                            $category = stripcslashes($buffer[0][1]);
+                            $category = mb_substr($category, 1, mb_strlen($category) - 2);
+
+                            $message = stripcslashes($buffer[2][1]);
+                            $message = mb_substr($message, 1, mb_strlen($message) - 2);
+
+                            $messages[$category][] = $message;
+                        } else {
+                            // invalid call or dynamic call we can't extract
+
+                            $line = Console::ansiFormat($this->getLine($buffer), [Console::FG_CYAN]);
+                            $skipping = Console::ansiFormat('Skipping line', [Console::FG_YELLOW]);
+                            $this->stdout("$skipping $line. Make sure both category and message are static strings.\n");
+                        }
+
+                        // prepare for the next match
+                        $matchedTokensCount = 0;
+                        $buffer = [];
+                    } elseif ($token !== '(' && isset($token[0]) && !in_array($token[0], [T_WHITESPACE, T_COMMENT])) {
+                        // ignore comments, whitespaces and beginning of function call
+                        $buffer[] = $token;
                     }
                 }
             }
+        }
+
+        $this->stdout("\n");
 
         return $messages;
     }
 
+    /**
+     * Finds out if two PHP tokens are equal
+     *
+     * @param array|string $a
+     * @param array|string $b
+     * @return boolean
+     */
+    protected function tokensEqual($a, $b)
+    {
+        if (is_string($a) && is_string($b)) {
+            return $a === $b;
+        } elseif (isset($a[0], $a[1], $b[0], $b[1])) {
+            return $a[0] === $b[0] && $a[1] == $b[1];
+        }
+        return false;
+    }
+
+    /**
+     * Finds out a line of the first non-char PHP token found
+     *
+     * @param array $tokens
+     * @return int|string
+     */
+    protected function getLine($tokens)
+    {
+        foreach ($tokens as $token) {
+            if (isset($token[2])) {
+                return $token[2];
+            }
+        }
+        return 'unknown';
+    }
+
     /**
      * Writes messages into PHP files
      *
@@ -293,7 +356,8 @@ class MessageController extends Controller
             $path = dirname($file);
             FileHelper::createDirectory($path);
             $msgs = array_values(array_unique($msgs));
-            echo "Saving messages to $file...\n";
+            $coloredFileName = Console::ansiFormat($file, [Console::FG_CYAN]);
+            echo "Saving messages to $coloredFileName...\n";
             $this->saveMessagesCategoryToPHP($msgs, $file, $overwrite, $removeUnused, $sort, $category);
         }
     }
@@ -315,7 +379,7 @@ class MessageController extends Controller
             sort($messages);
             ksort($existingMessages);
             if (array_keys($existingMessages) == $messages) {
-                echo "Nothing new in \"$category\" category... Nothing to save.\n";
+                echo "Nothing new in \"$category\" category... Nothing to save.\n\n";
                 return;
             }
             $merged = [];
@@ -385,7 +449,7 @@ return $array;
 EOD;
 
         file_put_contents($fileName, $content);
-        echo "Saved.\n";
+        echo "Saved.\n\n";
     }
 
     /**

tests/unit/framework/console/controllers/BaseMessageControllerTest.php

@@ -86,7 +86,7 @@ abstract class BaseMessageControllerTest extends TestCase
     protected function createSourceFile($content)
     {
         $fileName = $this->sourcePath . DIRECTORY_SEPARATOR . md5(uniqid()) . '.php';
-        file_put_contents($fileName, $content);
+        file_put_contents($fileName, "<?php\n" . $content);
         return $fileName;
     }