Merge pull request #74 from mzeis/6-LineLength-UnitTest

#6: Fix line length sniff when using translations

Author: lenaorobei

Magento2/Sniffs/Files/LineLengthSniff.php

@@ -5,6 +5,7 @@
  */
 namespace Magento2\Sniffs\Files;
 
+use PHP_CodeSniffer\Files\File;
 use PHP_CodeSniffer\Standards\Generic\Sniffs\Files\LineLengthSniff as FilesLineLengthSniff;
 
 /**
@@ -13,11 +14,32 @@
 class LineLengthSniff extends FilesLineLengthSniff
 {
     /**
-     * Having previous line content allows to ignore long lines in case of multi-line declaration.
+     * Having previous line content allows to ignore long lines in case of translations.
      *
      * @var string
      */
-    protected $previousLineContent = '';
+    protected $lastLineContent = '';
+
+    /**
+     * Regular expression for finding a translation in the last line.
+     *
+     * @var string
+     */
+    protected $lastLineRegExp = '~__\(.+\)|\bPhrase\(.+\)~';
+
+    /**
+     * Having the next-to-last line content allows to ignore long lines in case of translations.
+     *
+     * @var string
+     */
+    protected $nextToLastLineContent = '';
+
+    /**
+     * Regular expression for finding a translation in the next-to-last line.
+     *
+     * @var string
+     */
+    protected $nextToLastLineRegexp = '~__\($|\bPhrase\($~';
 
     /**
      * @inheritdoc
@@ -32,15 +54,79 @@ class LineLengthSniff extends FilesLineLengthSniff
     /**
      * @inheritdoc
      */
-    protected function checkLineLength($phpcsFile, $stackPtr, $lineContent)
+    protected function checkLineLength($phpcsFile, $tokens, $stackPtr)
+    {
+        /*
+         * The parent sniff checks the length of the previous line, so we have to inspect the previous line instead of
+         * the current one.
+         */
+        if (!$this->doesPreviousLineContainTranslationString()) {
+            parent::checkLineLength($phpcsFile, $tokens, $stackPtr);
+        }
+
+        $this->updateLineBuffer($phpcsFile, $tokens, $stackPtr);
+    }
+
+    /**
+     * Checks whether the previous line is part of a translation.
+     *
+     * The generic line sniff (which we are falling back to if there is no translation) always checks the length of the
+     * last line, so we have to check the last and next-to-last line for translations.
+     *
+     * @return bool
+     */
+    protected function doesPreviousLineContainTranslationString()
     {
-        $previousLineRegexp = '~__\($|\bPhrase\($~';
-        $currentLineRegexp = '~__\(.+\)|\bPhrase\(.+\)~';
-        $currentLineMatch = preg_match($currentLineRegexp, $lineContent) !== 0;
-        $previousLineMatch = preg_match($previousLineRegexp, $this->previousLineContent) !== 0;
-        $this->previousLineContent = $lineContent;
-        if (! $currentLineMatch && !$previousLineMatch) {
-            parent::checkLineLength($phpcsFile, $stackPtr, $lineContent);
+        $lastLineMatch       = preg_match($this->lastLineRegExp, $this->lastLineContent) !== 0;
+        $nextToLastLineMatch = preg_match($this->nextToLastLineRegexp, $this->nextToLastLineContent) !== 0;
+
+        return $lastLineMatch || $nextToLastLineMatch;
+    }
+
+    /**
+     * Assembles and returns the content for the code line of the provided stack pointer.
+     *
+     * @param File $phpcsFile
+     * @param array $tokens
+     * @param int $stackPtr
+     * @return string
+     */
+    protected function getLineContent(File $phpcsFile, array $tokens, $stackPtr)
+    {
+        $lineContent = '';
+
+        /*
+         * Avoid out of range error at the end of the file
+         */
+        if (!array_key_exists($stackPtr, $tokens)) {
+            return $lineContent;
         }
+
+        $codeLine = $tokens[$stackPtr]['line'];
+
+        /*
+         * Concatenate the string until we jump to the next line or reach the end of line character.
+         */
+        while (array_key_exists($stackPtr, $tokens) &&
+               $tokens[$stackPtr]['line'] === $codeLine &&
+               $tokens[$stackPtr]['content'] !== $phpcsFile->eolChar) {
+            $lineContent .= $tokens[$stackPtr]['content'];
+            $stackPtr++;
+        }
+
+        return $lineContent;
+    }
+
+    /**
+     * Pre-fills the line buffer for the next iteration.
+     *
+     * @param File $phpcsFile
+     * @param array $tokens
+     * @param int $stackPtr
+     */
+    protected function updateLineBuffer(File $phpcsFile, array $tokens, $stackPtr)
+    {
+        $this->nextToLastLineContent = $this->lastLineContent;
+        $this->lastLineContent       = $this->getLineContent($phpcsFile, $tokens, $stackPtr);
     }
 }

Magento2/Tests/Files/LineLengthUnitTest.inc

@@ -1,5 +1,7 @@
 <?php
 
+use Magento\Framework\Phrase;
+
 // allowed
 $shortString = 'This is a short line';
 
@@ -10,6 +12,19 @@ $tooLongString = 'This is a pretty long line. The code sniffer will report an er
 $longStringBrokenUp = 'This is a pretty long line. The code sniffer would report an error if this was written as a ' .
                       'single line. You have to insert a line break to avoid this error.';
 
+// allowed: long lines for translations
+$phraseObjects = [
+    Phrase('This is a short line'),
+    Phrase('Lines which include translations are excluded from this check. You can exceed the maximum character count without receiving a warning or an error.'),
+    'This is no translation string, so let us make sure this throws an error even when we use the word Phrase inside the string',
+    __(
+        'Also if we use this syntax and we put the actual translation string into a new line, there should be no error if this is used in a translation.'
+    ),
+    Phrase(
+        'The same goes for the Phrase class when we put the actual translation string into a new line, also here we should not receive an error.'
+    )
+];
+
 // allowed by Magento Standard (Generic ruleset would trigger warning / error)
 $test = '012344567890123445678901234456789012344567890123445678901234456789';
 $test1 = '01234456789012344567890123445678901234456789012344567890123445678901234456789';

Magento2/Tests/Files/LineLengthUnitTest.php

@@ -18,7 +18,8 @@ class LineLengthUnitTest extends AbstractSniffUnitTest
     public function getErrorList()
     {
         return [
-            7 => 1
+            9 => 1,
+            19 => 1,
         ];
     }