Fix ParseError for comments after rule name or in @keyframes

This commit ports required logic from an upstream patch
for Less.js 1.7:
less/less.js@e045362c901e6bb5

This simply eats up comments and is easy to backport.
Less.js 2.5.3 actually takes a different approach, it uses a
ParserInput class and stores comments in a commentStore.

This first change will allow the parser to eat up comments
and prevent syntax errors. As we don't store the comments yet
the generated CSS won't contain all comments because we have
no way to re-inject those later, yet. The comments.css override
allows us to make sure that we properly compile this Less
without errors.

The comments2.less tests comments between rule names
and block definitions. Those are not only parsed properly,
but also should be put in the generated CSS.

What Less.js 2.0 does, they parse comments when handling
spaces in skipWhitespace, instead of manually consuming comments
in specific places during parsing, and comments are pushed to an
array. Then, they either empty the comments array,
or get the first one and add it to the output.

In T353132 I'm going to do a follow-up on this work and
match Less.js 2.0 implementation - introducing the ParserInput
class with commentStore. As ParserInput change is going
to affect overall parsing, I prefer to do it in a separate
task.

Bug: T353131
Change-Id: I95db673e90726909eaa108870ee600fa4eca351e

lib/Less/Parser.php

@@ -990,8 +990,9 @@ private function parseComment() {
 			return new Less_Tree_Comment( $match, true, $this->pos, $this->env->currentFileInfo );
 		}
 
-		// $comment = $this->matchReg('/\\G\/\*(?:[^*]|\*+[^\/*])*\*+\/\n?/');
-		$comment = $this->matchReg( '/\\G\/\*(?s).*?\*+\/\n?/' );// not the same as less.js to prevent fatal errors
+		// not the same as less.js to prevent fatal errors
+		//         $this->matchReg( '/\\G\/\*(?:[^*]|\*+[^\/*])*\*+\/\n?/') ;
+		$comment = $this->matchReg( '/\\G\/\*(?s).*?\*+\/\n?/' );
 		if ( $comment ) {
 			return new Less_Tree_Comment( $comment, false, $this->pos, $this->env->currentFileInfo );
 		}
@@ -2018,7 +2019,7 @@ private function parseRule( $tryAnonymous = null ) {
 			if ( $isVariable ) {
 				$value = $this->parseDetachedRuleset();
 			}
-
+			$this->parseComments();
 			if ( !$value ) {
 				// a name returned by this.ruleProperty() is always an array of the form:
 				// [string-1, ..., string-n, ""] or [string-1, ..., string-n, "+"]
@@ -2302,6 +2303,8 @@ private function parseDirective() {
 				$isRooted = false;
 				break;
 		}
+		// TODO: T353132 - differs from less.js - we don't have the ParserInput yet
+		$this->parseComments();
 
 		if ( $hasIdentifier ) {
 			$value = $this->parseEntity();
@@ -2321,6 +2324,9 @@ private function parseDirective() {
 			}
 		}
 
+		// TODO: T353132 - differs from less.js - we don't have the ParserInput yet
+		$this->parseComments();
+
 		if ( $hasBlock ) {
 			$rules = $this->parseBlockRuleset();
 		}
@@ -2581,6 +2587,7 @@ private function parseProperty() {
 	 * eg: 'color', 'width', 'height', etc
 	 *
 	 * @return array<Less_Tree_Keyword|Less_Tree_Variable>
+	 * @see less-2.5.3.js#parsers.ruleProperty
 	 */
 	private function parseRuleProperty() {
 		$name = [];
@@ -2600,6 +2607,8 @@ private function parseRuleProperty() {
 		// Consume!
 		// @phan-suppress-next-line PhanPluginEmptyStatementWhileLoop
 		while ( $this->rulePropertyMatch( '/\\G((?:[\w-]+)|(?:@\{[\w-]+\}))/', $index, $name ) );
+		// @phan-suppress-next-line PhanPluginEmptyStatementWhileLoop
+		while ( $this->rulePropertyCutOutBlockComments() );
 
 		if ( ( count( $name ) > 1 ) && $this->rulePropertyMatch( '/\\G((?:\+_|\+)?)\s*:/', $index, $name ) ) {
 			$this->forget();
@@ -2633,6 +2642,15 @@ private function rulePropertyMatch( $re, &$index, &$name ) {
 		}
 	}
 
+	private function rulePropertyCutOutBlockComments() {
+		// not the same as less.js to prevent fatal errors
+		// similar to parseComment()
+		//    '/\\G\s*\/\*(?:[^*]|\*+[^\/*])*\*+\//'
+		$re = '/\\G\s*\/\*(?s).*?\*+\//';
+		$chunk = $this->matchReg( $re );
+		return $chunk != null;
+	}
+
 	public static function serializeVars( $vars ) {
 		$s = '';
 

test/Fixtures/lessjs-2.5.3/override/comments.css

@@ -0,0 +1,82 @@
+/******************\
+*                  *
+*  Comment Header  *
+*                  *
+\******************/
+/*
+
+    Comment
+
+*/
+/*
+ * Comment Test
+ *
+ * - cloudhead (http://cloudhead.net)
+ *
+ */
+/* Colors
+ * ------
+ *   #EDF8FC (background blue)
+ *   #166C89 (darkest blue)
+ *
+ * Text:
+ *   #333 (standard text) // A comment within a comment!
+ *   #1F9EC9 (standard link)
+ *
+ */
+/* @group Variables
+------------------- */
+#comments,
+.comments {
+  /**/
+  color: red;
+  /* A C-style comment */
+  /* A C-style comment */
+  background-color: orange;
+  font-size: 12px;
+  /* lost comment */
+  content: "content";
+  border: 1px solid black;
+  padding: 0;
+  margin: 2em;
+}
+/* commented out
+  #more-comments {
+    color: grey;
+  }
+*/
+.selector,
+.lots,
+.comments {
+  color: grey, /* blue */ orange;
+  -webkit-border-radius: 2px /* webkit only */;
+  -moz-border-radius: 8px /* moz only with operation */;
+}
+.test {
+  color: 1px;
+}
+.sr-only-focusable {
+  clip: auto;
+}
+@-webkit-keyframes hover {
+  0% {
+    color: red;
+  }
+}
+#last {
+  color: blue;
+}
+/*  */
+/* { */
+/*  */
+/*  */
+/*  */
+#div {
+  color: #A33;
+}
+/* } */
+/*by block */
+#output-block {
+  comment: /* // Not commented out // */;
+}
+/*comment on last line*/

test/phpunit/FixturesTest.php

@@ -26,8 +26,7 @@ class phpunit_FixturesTest extends phpunit_bootstrap {
 			'parens' => true,
 
 			// Temporary disabled
-			'comments' => true, // T353131 & T353132
-			'comments2' => true, // T353131 & T353132
+			'comments2' => true, // T353132
 			'css' => true, // T352911 & T352866
 			'css-guards' => true, // T353144
 			'import' => true, // T353146