Merge pull request #123 from bigfoot90/fix-comment-parsing

nijel · web-flow · commit ed8d676e2ca0 · 2017-01-20T14:03:28.000+01:00
Fix comment parsing
diff --git a/src/Lexer.php b/src/Lexer.php
@@ -572,7 +572,6 @@ public function parseComment()
             ) {
                 $token .= $this->str[$this->last];
             }
-            $token .= "\n"; // Adding the line ending.
             return new Token($token, Token::TYPE_COMMENT, Token::FLAG_COMMENT_BASH);
         }
 
@@ -641,7 +640,6 @@ public function parseComment()
                     ) {
                         $token .= $this->str[$this->last];
                     }
-                    $token .= "\n"; // Adding the line ending.
                 }
 
                 return new Token($token, Token::TYPE_COMMENT, Token::FLAG_COMMENT_SQL);
diff --git a/src/Utils/Formatter.php b/src/Utils/Formatter.php
@@ -326,14 +326,6 @@ public function formatList($list)
          */
         $prev = null;
 
-        /**
-         * Comments are being formatted separately to maintain the whitespaces
-         * before and after them.
-         *
-         * @var string
-         */
-        $comment = '';
-
         // In order to be able to format the queries correctly, the next token
         // must be taken into consideration. The loop below uses two pointers,
         // `$prev` and `$curr` which store two consecutive tokens.
@@ -342,39 +334,26 @@ public function formatList($list)
             /**
              * Token parsed at this moment.
              *
-             * @var Token
+             * @var Token $curr
              */
             $curr = $list->tokens[$list->idx];
 
             if ($curr->type === Token::TYPE_WHITESPACE) {
                 // Whitespaces are skipped because the formatter adds its own.
                 continue;
-            } elseif ($curr->type === Token::TYPE_COMMENT) {
-                // Whether the comments should be parsed.
-                if (!empty($this->options['remove_comments'])) {
-                    continue;
-                }
-
-                if ($list->tokens[$list->idx - 1]->type === Token::TYPE_WHITESPACE) {
-                    // The whitespaces before and after are preserved for
-                    // formatting reasons.
-                    $comment .= $list->tokens[$list->idx - 1]->token;
-                }
-                $comment .= $this->toString($curr);
-                if (($list->tokens[$list->idx + 1]->type === Token::TYPE_WHITESPACE)
-                    && ($list->tokens[$list->idx + 2]->type !== Token::TYPE_COMMENT)
-                ) {
-                    // Adding the next whitespace only there is no comment that
-                    // follows it immediately which may cause adding a
-                    // whitespace twice.
-                    $comment .= $list->tokens[$list->idx + 1]->token;
-                }
+            }
 
-                // Everything was handled here, no need to continue.
+            if ($curr->type === Token::TYPE_COMMENT && $this->options['remove_comments']) {
+                // Skip Comments if option `remove_comments` is enabled
                 continue;
             }
 
             // Checking if pointers were initialized.
+            /**
+             * Previous Token.
+             *
+             * @var Token $prev
+             */
             if ($prev !== null) {
                 // Checking if a new clause started.
                 if (static::isClause($prev) !== false) {
@@ -453,12 +432,6 @@ public function formatList($list)
                     $shortGroup = false;
                 }
 
-                // Delimiter must be placed on the same line with the last
-                // clause.
-                if ($curr->type === Token::TYPE_DELIMITER) {
-                    $lineEnded = false;
-                }
-
                 // Adding the token.
                 $ret .= $this->toString($prev);
 
@@ -469,32 +442,29 @@ public function formatList($list)
                         $indent = 0;
                     }
 
-                    if ($curr->type !== Token::TYPE_COMMENT) {
-                        $ret .= $this->options['line_ending']
-                            . str_repeat($this->options['indentation'], $indent);
-                    }
+                    $ret .= $this->options['line_ending']
+                        . str_repeat($this->options['indentation'], $indent);
+
                     $lineEnded = false;
                 } else {
                     // If the line ended there is no point in adding whitespaces.
                     // Also, some tokens do not have spaces before or after them.
-                    if (!(($prev->type === Token::TYPE_OPERATOR && ($prev->value === '.' || $prev->value === '('))
-                        // No space after . (
-                        || ($curr->type === Token::TYPE_OPERATOR && ($curr->value === '.' || $curr->value === ',' || $curr->value === '(' || $curr->value === ')'))
-                        // No space before . , ( )
-                        || $curr->type === Token::TYPE_DELIMITER && mb_strlen($curr->value, 'UTF-8') < 2)
+                    if (
                         // A space after delimiters that are longer than 2 characters.
-                        || $prev->value === 'DELIMITER'
+                        $prev->value === 'DELIMITER'
+                        || !(
+                            ($prev->type === Token::TYPE_OPERATOR && ($prev->value === '.' || $prev->value === '('))
+                            // No space after . (
+                            || ($curr->type === Token::TYPE_OPERATOR && ($curr->value === '.' || $curr->value === ',' || $curr->value === '(' || $curr->value === ')'))
+                            // No space before . , ( )
+                            || $curr->type === Token::TYPE_DELIMITER && mb_strlen($curr->value, 'UTF-8') < 2
+                        )
                     ) {
                         $ret .= ' ';
                     }
                 }
             }
 
-            if (!empty($comment)) {
-                $ret .= $comment;
-                $comment = '';
-            }
-
             // Iteration finished, consider current token as previous.
             $prev = $curr;
         }
diff --git a/tests/Utils/CLITest.php b/tests/Utils/CLITest.php
@@ -39,7 +39,7 @@ public function highlightParams()
             ),
             array(
                 array('q' => 'SELECT /* comment */ 1 /* other */', 'f' => 'text'),
-                "SELECT\n     /* comment */ 1 /* other */\n",
+                "SELECT\n    /* comment */ 1 /* other */\n",
                 0,
             ),
             array(
diff --git a/tests/Utils/FormatterTest.php b/tests/Utils/FormatterTest.php
@@ -249,13 +249,36 @@ public function formatQueries()
                 '&nbsp;&nbsp;&nbsp;&nbsp;<span class="sql-number">1</span>',
                 array('type' => 'html'),
             ),
+            array(
+                'SELECT /* Comment */ 1' . "\n" .
+                'FROM tbl # Comment' . "\n" .
+                'WHERE 1 -- Comment',
+                'SELECT' . "\n" .
+                '    /* Comment */ 1' . "\n" .
+                'FROM' . "\n" .
+                '    tbl # Comment' . "\n" .
+                'WHERE' . "\n" .
+                '    1 -- Comment',
+                array('type' => 'text'),
+            ),
             array(
                 'SELECT 1 # Comment',
                 '<span class="sql-reserved">SELECT</span>' . '<br/>' .
-                '&nbsp;&nbsp;&nbsp;&nbsp;<span class="sql-number">1</span> <span class="sql-comment"># Comment' . "\n" .
-                '</span>',
+                '&nbsp;&nbsp;&nbsp;&nbsp;<span class="sql-number">1</span> <span class="sql-comment"># Comment</span>',
+                array('type' => 'html'),
+            ),
+            array(
+                'SELECT 1 -- comment',
+                '<span class="sql-reserved">SELECT</span>' . '<br/>' .
+                '&nbsp;&nbsp;&nbsp;&nbsp;<span class="sql-number">1</span> <span class="sql-comment">-- comment</span>',
                 array('type' => 'html'),
             ),
+            array(
+                'SELECT 1 -- comment',
+                '<span class="sql-reserved">SELECT</span>' . '<br/>' .
+                '&nbsp;&nbsp;&nbsp;&nbsp;<span class="sql-number">1</span>',
+                array('type' => 'html', 'remove_comments' => true),
+            ),
             array(
                 'SELECT HEX("1")',
                 '<span class="sql-reserved">SELECT</span>' . '<br/>' .
@@ -317,19 +340,6 @@ public function formatQueries()
                 '&nbsp;&nbsp;&nbsp;&nbsp;superado = <span class="sql-number">0</span>',
                 array('type' => 'html'),
             ),
-            array(
-                'SELECT 1 -- comment',
-                '<span class="sql-reserved">SELECT</span>' . '<br/>' .
-                '&nbsp;&nbsp;&nbsp;&nbsp;<span class="sql-number">1</span> <span class="sql-comment">-- comment' . "\n" .
-                '</span>',
-                array('type' => 'html'),
-            ),
-            array(
-                'SELECT 1 -- comment',
-                '<span class="sql-reserved">SELECT</span>' . '<br/>' .
-                '&nbsp;&nbsp;&nbsp;&nbsp;<span class="sql-number">1</span>',
-                array('type' => 'html', 'remove_comments' => true),
-            ),
             array(
                 'CREATE TABLE IF NOT EXISTS `pma__bookmark` (' . "\n" .
                 '  `id` int(11) NOT NULL auto_increment,' . "\n" .
diff --git a/tests/data/lexer/lexComment.out b/tests/data/lexer/lexComment.out
@@ -4,11 +4,5 @@ SELECT /*!50000 STRAIGHT_JOIN */ col1 FROM table1, table2 /* select query */
 -- comment 2";s:5:"lexer";O:15:"SqlParser\Lexer":8:{s:6:"strict";b:0;s:3:"str";s:110:"# comment
 SELECT /*!50000 STRAIGHT_JOIN */ col1 FROM table1, table2 /* select query */
 -- comment
--- comment 2";s:3:"len";i:110;s:4:"last";i:111;s:4:"list";O:20:"SqlParser\TokensList":3:{s:6:"tokens";a:23:{i:0;O:15:"SqlParser\Token":5:{s:5:"token";s:10:"# comment
-";s:5:"value";s:10:"# comment
-";s:4:"type";i:4;s:5:"flags";i:1;s:8:"position";i:0;}i:1;O:15:"SqlParser\Token":5:{s:5:"token";s:6:"SELECT";s:5:"value";s:6:"SELECT";s:4:"type";i:1;s:5:"flags";i:3;s:8:"position";i:10;}i:2;O:15:"SqlParser\Token":5:{s:5:"token";s:1:" ";s:5:"value";s:1:" ";s:4:"type";i:3;s:5:"flags";i:0;s:8:"position";i:16;}i:3;O:15:"SqlParser\Token":5:{s:5:"token";s:8:"/*!50000";s:5:"value";s:8:"/*!50000";s:4:"type";i:4;s:5:"flags";i:10;s:8:"position";i:17;}i:4;O:15:"SqlParser\Token":5:{s:5:"token";s:1:" ";s:5:"value";s:1:" ";s:4:"type";i:3;s:5:"flags";i:0;s:8:"position";i:25;}i:5;O:15:"SqlParser\Token":5:{s:5:"token";s:13:"STRAIGHT_JOIN";s:5:"value";s:13:"STRAIGHT_JOIN";s:4:"type";i:1;s:5:"flags";i:3;s:8:"position";i:26;}i:6;O:15:"SqlParser\Token":5:{s:5:"token";s:1:" ";s:5:"value";s:1:" ";s:4:"type";i:3;s:5:"flags";i:0;s:8:"position";i:39;}i:7;O:15:"SqlParser\Token":5:{s:5:"token";s:2:"*/";s:5:"value";s:2:"*/";s:4:"type";i:4;s:5:"flags";i:2;s:8:"position";i:40;}i:8;O:15:"SqlParser\Token":5:{s:5:"token";s:1:" ";s:5:"value";s:1:" ";s:4:"type";i:3;s:5:"flags";i:0;s:8:"position";i:42;}i:9;O:15:"SqlParser\Token":5:{s:5:"token";s:4:"col1";s:5:"value";s:4:"col1";s:4:"type";i:0;s:5:"flags";i:0;s:8:"position";i:43;}i:10;O:15:"SqlParser\Token":5:{s:5:"token";s:1:" ";s:5:"value";s:1:" ";s:4:"type";i:3;s:5:"flags";i:0;s:8:"position";i:47;}i:11;O:15:"SqlParser\Token":5:{s:5:"token";s:4:"FROM";s:5:"value";s:4:"FROM";s:4:"type";i:1;s:5:"flags";i:3;s:8:"position";i:48;}i:12;O:15:"SqlParser\Token":5:{s:5:"token";s:1:" ";s:5:"value";s:1:" ";s:4:"type";i:3;s:5:"flags";i:0;s:8:"position";i:52;}i:13;O:15:"SqlParser\Token":5:{s:5:"token";s:6:"table1";s:5:"value";s:6:"table1";s:4:"type";i:0;s:5:"flags";i:0;s:8:"position";i:53;}i:14;O:15:"SqlParser\Token":5:{s:5:"token";s:1:",";s:5:"value";s:1:",";s:4:"type";i:2;s:5:"flags";i:16;s:8:"position";i:59;}i:15;O:15:"SqlParser\Token":5:{s:5:"token";s:1:" ";s:5:"value";s:1:" ";s:4:"type";i:3;s:5:"flags";i:0;s:8:"position";i:60;}i:16;O:15:"SqlParser\Token":5:{s:5:"token";s:6:"table2";s:5:"value";s:6:"table2";s:4:"type";i:0;s:5:"flags";i:0;s:8:"position";i:61;}i:17;O:15:"SqlParser\Token":5:{s:5:"token";s:1:" ";s:5:"value";s:1:" ";s:4:"type";i:3;s:5:"flags";i:0;s:8:"position";i:67;}i:18;O:15:"SqlParser\Token":5:{s:5:"token";s:18:"/* select query */";s:5:"value";s:18:"/* select query */";s:4:"type";i:4;s:5:"flags";i:2;s:8:"position";i:68;}i:19;O:15:"SqlParser\Token":5:{s:5:"token";s:1:"
-";s:5:"value";s:1:" ";s:4:"type";i:3;s:5:"flags";i:0;s:8:"position";i:86;}i:20;O:15:"SqlParser\Token":5:{s:5:"token";s:11:"-- comment
-";s:5:"value";s:11:"-- comment
-";s:4:"type";i:4;s:5:"flags";i:4;s:8:"position";i:87;}i:21;O:15:"SqlParser\Token":5:{s:5:"token";s:13:"-- comment 2
-";s:5:"value";s:13:"-- comment 2
-";s:4:"type";i:4;s:5:"flags";i:4;s:8:"position";i:98;}i:22;O:15:"SqlParser\Token":5:{s:5:"token";N;s:5:"value";N;s:4:"type";i:9;s:5:"flags";i:0;s:8:"position";N;}}s:5:"count";i:23;s:3:"idx";i:0;}s:9:"delimiter";s:1:";";s:12:"delimiterLen";i:1;s:6:"errors";a:0:{}}s:6:"parser";N;s:6:"errors";a:2:{s:5:"lexer";a:0:{}s:6:"parser";a:0:{}}}
+-- comment 2";s:3:"len";i:110;s:4:"last";i:111;s:4:"list";O:20:"SqlParser\TokensList":3:{s:6:"tokens";a:23:{i:0;O:15:"SqlParser\Token":5:{s:5:"token";s:9:"# comment";s:5:"value";s:9:"# comment";s:4:"type";i:4;s:5:"flags";i:1;s:8:"position";i:0;}i:1;O:15:"SqlParser\Token":5:{s:5:"token";s:6:"SELECT";s:5:"value";s:6:"SELECT";s:4:"type";i:1;s:5:"flags";i:3;s:8:"position";i:10;}i:2;O:15:"SqlParser\Token":5:{s:5:"token";s:1:" ";s:5:"value";s:1:" ";s:4:"type";i:3;s:5:"flags";i:0;s:8:"position";i:16;}i:3;O:15:"SqlParser\Token":5:{s:5:"token";s:8:"/*!50000";s:5:"value";s:8:"/*!50000";s:4:"type";i:4;s:5:"flags";i:10;s:8:"position";i:17;}i:4;O:15:"SqlParser\Token":5:{s:5:"token";s:1:" ";s:5:"value";s:1:" ";s:4:"type";i:3;s:5:"flags";i:0;s:8:"position";i:25;}i:5;O:15:"SqlParser\Token":5:{s:5:"token";s:13:"STRAIGHT_JOIN";s:5:"value";s:13:"STRAIGHT_JOIN";s:4:"type";i:1;s:5:"flags";i:3;s:8:"position";i:26;}i:6;O:15:"SqlParser\Token":5:{s:5:"token";s:1:" ";s:5:"value";s:1:" ";s:4:"type";i:3;s:5:"flags";i:0;s:8:"position";i:39;}i:7;O:15:"SqlParser\Token":5:{s:5:"token";s:2:"*/";s:5:"value";s:2:"*/";s:4:"type";i:4;s:5:"flags";i:2;s:8:"position";i:40;}i:8;O:15:"SqlParser\Token":5:{s:5:"token";s:1:" ";s:5:"value";s:1:" ";s:4:"type";i:3;s:5:"flags";i:0;s:8:"position";i:42;}i:9;O:15:"SqlParser\Token":5:{s:5:"token";s:4:"col1";s:5:"value";s:4:"col1";s:4:"type";i:0;s:5:"flags";i:0;s:8:"position";i:43;}i:10;O:15:"SqlParser\Token":5:{s:5:"token";s:1:" ";s:5:"value";s:1:" ";s:4:"type";i:3;s:5:"flags";i:0;s:8:"position";i:47;}i:11;O:15:"SqlParser\Token":5:{s:5:"token";s:4:"FROM";s:5:"value";s:4:"FROM";s:4:"type";i:1;s:5:"flags";i:3;s:8:"position";i:48;}i:12;O:15:"SqlParser\Token":5:{s:5:"token";s:1:" ";s:5:"value";s:1:" ";s:4:"type";i:3;s:5:"flags";i:0;s:8:"position";i:52;}i:13;O:15:"SqlParser\Token":5:{s:5:"token";s:6:"table1";s:5:"value";s:6:"table1";s:4:"type";i:0;s:5:"flags";i:0;s:8:"position";i:53;}i:14;O:15:"SqlParser\Token":5:{s:5:"token";s:1:",";s:5:"value";s:1:",";s:4:"type";i:2;s:5:"flags";i:16;s:8:"position";i:59;}i:15;O:15:"SqlParser\Token":5:{s:5:"token";s:1:" ";s:5:"value";s:1:" ";s:4:"type";i:3;s:5:"flags";i:0;s:8:"position";i:60;}i:16;O:15:"SqlParser\Token":5:{s:5:"token";s:6:"table2";s:5:"value";s:6:"table2";s:4:"type";i:0;s:5:"flags";i:0;s:8:"position";i:61;}i:17;O:15:"SqlParser\Token":5:{s:5:"token";s:1:" ";s:5:"value";s:1:" ";s:4:"type";i:3;s:5:"flags";i:0;s:8:"position";i:67;}i:18;O:15:"SqlParser\Token":5:{s:5:"token";s:18:"/* select query */";s:5:"value";s:18:"/* select query */";s:4:"type";i:4;s:5:"flags";i:2;s:8:"position";i:68;}i:19;O:15:"SqlParser\Token":5:{s:5:"token";s:1:"
+";s:5:"value";s:1:" ";s:4:"type";i:3;s:5:"flags";i:0;s:8:"position";i:86;}i:20;O:15:"SqlParser\Token":5:{s:5:"token";s:10:"-- comment";s:5:"value";s:10:"-- comment";s:4:"type";i:4;s:5:"flags";i:4;s:8:"position";i:87;}i:21;O:15:"SqlParser\Token":5:{s:5:"token";s:12:"-- comment 2";s:5:"value";s:12:"-- comment 2";s:4:"type";i:4;s:5:"flags";i:4;s:8:"position";i:98;}i:22;O:15:"SqlParser\Token":5:{s:5:"token";N;s:5:"value";N;s:4:"type";i:9;s:5:"flags";i:0;s:8:"position";N;}}s:5:"count";i:23;s:3:"idx";i:0;}s:9:"delimiter";s:1:";";s:12:"delimiterLen";i:1;s:6:"errors";a:0:{}}s:6:"parser";N;s:6:"errors";a:2:{s:5:"lexer";a:0:{}s:6:"parser";a:0:{}}}
diff --git a/tests/data/lexer/lexNumber.out b/tests/data/lexer/lexNumber.out

Original file line number	Diff line number	Diff line change
`@@ -572,7 +572,6 @@ public function parseComment()`
`572`	`572`	`) {`
`573`	`573`	`$token .= $this->str[$this->last];`
`574`	`574`	`}`
`575`		`- $token .= "\n"; // Adding the line ending.`
`576`	`575`	`return new Token($token, Token::TYPE_COMMENT, Token::FLAG_COMMENT_BASH);`
`577`	`576`	`}`
`578`	`577`
`@@ -641,7 +640,6 @@ public function parseComment()`
`641`	`640`	`) {`
`642`	`641`	`$token .= $this->str[$this->last];`
`643`	`642`	`}`
`644`		`- $token .= "\n"; // Adding the line ending.`
`645`	`643`	`}`
`646`	`644`
`647`	`645`	`return new Token($token, Token::TYPE_COMMENT, Token::FLAG_COMMENT_SQL);`