Skip to content

Commit

Permalink
Merge "Fix ParseError for comments in more places and preserve them i…
Browse files Browse the repository at this point in the history 
…n output"
  • Loading branch information
jenkins-bot authored and Gerrit Code Review committed Apr 23, 2024
2 parents 252a743 + 76b1210 commit 7d05070
Show file tree
Hide file tree
Showing 8 changed files with 161 additions and 170 deletions.
166 changes: 100 additions & 66 deletions lib/Less/Parser.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,12 @@ class Less_Parser {
private $furthest;
private $mb_internal_encoding = ''; // for remember exists value of mbstring.internal_encoding

private $autoCommentAbsorb = true;
/**
* @var array<array{index:int,text:string,isLineComment:bool}>
*/
private $commentStore = [];

/**
* @var Less_Environment
*/
Expand Down Expand Up @@ -659,7 +665,7 @@ private function GetRules( $file_path ) {
}
}
}

$this->skipWhitespace( 0 );
$rules = $this->parsePrimary();

if ( $this->pos < $this->input_len ) {
Expand Down Expand Up @@ -853,8 +859,51 @@ private function peekChar( $tok ) {
*/
private function skipWhitespace( $length ) {
$this->pos += $length;
// Optimization: Skip over irrelevant chars without slow loop
$this->pos += strspn( $this->input, "\n\r\t ", $this->pos );

// we do not increment the $this->pos as upstream, because $this->pos is updated with
// comment length or by amount of whitespaces strsspn finds. What upstream does is ignores
// whitespaces, which makes the for loop keep iterating.
// phpcs:ignore Generic.CodeAnalysis.ForLoopShouldBeWhileLoop.CanSimplify
for ( ; $this->pos < $this->input_len; ) {
$currentChar = $this->input[$this->pos];

if ( $this->autoCommentAbsorb && $currentChar === '/' ) {
$nextChar = $this->input[$this->pos + 1] ?? '';
if ( $nextChar === '/' ) {
$comment = [ 'index' => $this->pos, 'isLineComment' => true ];
$nextNewLine = strpos( $this->input, "\n", $this->pos + 2 );
if ( $nextNewLine === false ) {
$nextNewLine = $this->input_len ?? 0;
}
$this->pos = $nextNewLine;
$comment['text'] = substr( $this->input, $this->pos, $nextNewLine - $this->pos );
$this->commentStore[] = $comment;
continue;
} elseif ( $nextChar === '*' ) {
$nextStarSlash = strpos( $this->input, "*/", $this->pos + 2 );
if ( $nextStarSlash ) {
$comment = [
'text' => substr( $this->input, $this->pos, $nextStarSlash + 2 -
$this->pos ),
'isLineComment' => false,
'index' => $this->pos
];
$this->commentStore[] = $comment;
$this->pos += strlen( $comment['text'] );
continue;
}
}
break;
}

if ( $currentChar !== " " && $currentChar !== "\n"
&& $currentChar !== "\t" && $currentChar !== "\r" ) {
break;
}

// Optimization: Skip over irrelevant chars without slow loop
$this->pos += strspn( $this->input, "\n\r\t ", $this->pos );
}
}

/**
Expand Down Expand Up @@ -941,18 +990,30 @@ private function parsePrimary() {

while ( true ) {

while ( true ) {
$node = $this->parseComment();
if ( !$node ) {
break;
}
$root[] = $node;
}

// always process comments before deciding if finished
if ( $this->pos >= $this->input_len ) {
break;
}

if ( $this->peekChar( '}' ) ) {
break;
}

$node = $this->parseExtend( true );
if ( $node ) {
$root = array_merge( $root, $node );
continue;
}

$node = $this->parseComment()
?? $this->parseMixinDefinition()
$node = $this->parseMixinDefinition()
// Optimisation: NameValue is specific to less.php
?? $this->parseNameValue()
?? $this->parseRule()
Expand All @@ -967,52 +1028,28 @@ private function parsePrimary() {
break;
}

if ( $this->peekChar( '}' ) ) {
break;
}
}

return $root;
}

// We create a Comment node for CSS comments `/* */`,
// but keep the LeSS comments `//` silent, by just skipping
// over them.
/**
* comments are collected by the main parsing mechanism and then assigned to nodes
* where the current structure allows it
* @return Less_Tree_Comment|void
*/
private function parseComment() {
$char = $this->input[$this->pos] ?? null;
if ( $char !== '/' ) {
return;
}

$nextChar = $this->input[$this->pos + 1] ?? null;
if ( $nextChar === '/' ) {
$match = $this->matchReg( '/\\G\/\/.*/' );
return new Less_Tree_Comment( $match, true, $this->pos, $this->env->currentFileInfo );
}

// not the same as less.js to prevent fatal errors
// $this->matchReg( '/\\G\/\*(?:[^*]|\*+[^\/*])*\*+\/\n?/') ;
$comment = $this->matchReg( '/\\G\/\*(?s).*?\*+\/\n?/' );
$comment = array_shift( $this->commentStore );
if ( $comment ) {
return new Less_Tree_Comment( $comment, false, $this->pos, $this->env->currentFileInfo );
return new Less_Tree_Comment(
$comment['text'],
$comment['isLineComment'],
$comment['index'],
$this->env->currentFileInfo
);
}
}

private function parseComments() {
$comments = [];

while ( $this->pos < $this->input_len ) {
$comment = $this->parseComment();
if ( !$comment ) {
break;
}

$comments[] = $comment;
}

return $comments;
}

/**
* A string, which supports escaping " and '
*
Expand Down Expand Up @@ -1194,16 +1231,19 @@ private function parseEntitiesAssignment() {
//
private function parseEntitiesUrl() {
$char = $this->input[$this->pos] ?? null;

$this->autoCommentAbsorb = false;
// Optimisation: 'u' check is specific to less.php
if ( $char !== 'u' || !$this->matchReg( '/\\Gurl\(/' ) ) {
$this->autoCommentAbsorb = true;
return;
}

$value = $this->parseEntitiesQuoted() ?? $this->parseEntitiesVariable() ?? $this->matchReg( '/\\Gdata\:.*?[^\)]+/' ) ?? $this->matchReg( '/\\G(?:(?:\\\\[\(\)\'"])|[^\(\)\'"])+/' ) ?? null;
if ( !$value ) {
$value = '';
}

$this->autoCommentAbsorb = true;
$this->expectChar( ')' );

if ( $value instanceof Less_Tree_Quoted || $value instanceof Less_Tree_Variable ) {
Expand Down Expand Up @@ -1488,7 +1528,7 @@ private function parseMixinArgs( $isCall ) {
if ( $isCall ) {
$arg = $this->parseDetachedRuleset() ?? $this->parseExpression();
} else {
$this->parseComments();
$this->commentStore = [];
if ( $this->input[ $this->pos ] === '.' && $this->matchStr( '...' ) ) {
$returner['variadic'] = true;
if ( $this->matchChar( ";" ) && !$isSemiColonSeperated ) {
Expand Down Expand Up @@ -1651,7 +1691,7 @@ private function parseMixinDefinition() {
return;
}

$this->parseComments();
$this->commentStore = [];

if ( $this->matchStr( 'when' ) ) { // Guard
$cond = $this->expect( 'parseConditions', 'Expected conditions' );
Expand All @@ -1675,13 +1715,13 @@ private function parseMixinDefinition() {
// and can be found inside a rule's value.
//
private function parseEntity() {
return $this->parseEntitiesLiteral() ??
return $this->parseComment() ??
$this->parseEntitiesLiteral() ??
$this->parseEntitiesVariable() ??
$this->parseEntitiesUrl() ??
$this->parseEntitiesCall() ??
$this->parseEntitiesKeyword() ??
$this->parseEntitiesJavascript() ??
$this->parseComment();
$this->parseEntitiesJavascript();
}

//
Expand Down Expand Up @@ -1942,7 +1982,7 @@ private function parseRuleset() {
break;
}
$selectors[] = $s;
$this->parseComments();
$this->commentStore = [];

if ( $s->condition && count( $selectors ) > 1 ) {
$this->Error( 'Guards are only currently allowed on a single selector.' );
Expand All @@ -1954,7 +1994,7 @@ private function parseRuleset() {
if ( $s->condition ) {
$this->Error( 'Guards are only currently allowed on a single selector.' );
}
$this->parseComments();
$this->commentStore = [];
}

if ( $selectors ) {
Expand Down Expand Up @@ -1983,6 +2023,9 @@ private function parseNameValue() {
if ( $match ) {

if ( $match[4] == '}' ) {
// because we will parse all comments after closing }, we need to reset the store as
// we're going to reset the position to closing }
$this->commentStore = [];
$this->pos = $index + strlen( $match[0] ) - 1;
$match[2] = rtrim( $match[2] );
}
Expand Down Expand Up @@ -2019,7 +2062,7 @@ private function parseRule( $tryAnonymous = null ) {
if ( $isVariable ) {
$value = $this->parseDetachedRuleset();
}
$this->parseComments();
$this->commentStore = [];
if ( !$value ) {
// a name returned by this.ruleProperty() is always an array of the form:
// [string-1, ..., string-n, ""] or [string-1, ..., string-n, "+"]
Expand Down Expand Up @@ -2303,8 +2346,8 @@ private function parseDirective() {
$isRooted = false;
break;
}
// TODO: T353132 - differs from less.js - we don't have the ParserInput yet
$this->parseComments();

$this->commentStore = [];

if ( $hasIdentifier ) {
$value = $this->parseEntity();
Expand All @@ -2324,9 +2367,6 @@ private function parseDirective() {
}
}

// TODO: T353132 - differs from less.js - we don't have the ParserInput yet
$this->parseComments();

if ( $hasBlock ) {
$rules = $this->parseBlockRuleset();
}
Expand Down Expand Up @@ -2553,6 +2593,11 @@ private function parseExpression() {
$entities = [];

do {
$e = $this->parseComment();
if ( $e ) {
$entities[] = $e;
continue;
}
$e = $this->parseAddition() ?? $this->parseEntity();
if ( $e ) {
$entities[] = $e;
Expand Down Expand Up @@ -2609,8 +2654,6 @@ 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();
Expand Down Expand Up @@ -2644,15 +2687,6 @@ 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 = '';

Expand Down
34 changes: 22 additions & 12 deletions lib/Less/Tree/Call.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,27 @@ public function accept( $visitor ) {
$this->args = $visitor->visitArray( $this->args );
}

/**
* @see less-2.5.3.js#functionCaller.call
*/
private function functionCaller( $function, array $arguments ) {
// This code is terrible and should be replaced as per this issue...
// https://github.com/less/less.js/issues/2477
$filtered = [];
foreach ( $arguments as $argument ) {
if ( $argument instanceof Less_Tree_Comment ) {
continue;
}
$filtered[] = $argument;
}
foreach ( $filtered as $index => $argument ) {
if ( $argument instanceof Less_Tree_Expression ) {
$filtered[$index] = $argument->mapToFunctionCallArgument();
}
}
return $function( ...$filtered );
}

//
// When evaluating a function call,
// we either find the function in Less_Functions,
Expand All @@ -46,17 +67,6 @@ public function compile( $env ) {
$args[] = $a->compile( $env );
}

foreach ( $args as $key => $arg ) {
if ( $arg instanceof Less_Tree_Expression ) {
$arg->throwAwayComments();

if ( count( $arg->value ) === 1 ) {
$subNode = $arg->value[0];
array_splice( $args, $key, 1, [ $subNode ] );
}
}
}

$env->strictMath = $currentMathContext;

$nameLC = strtolower( $this->name );
Expand Down Expand Up @@ -92,7 +102,7 @@ public function compile( $env ) {
// If the function name isn't known to LESS, output it unchanged as CSS.
if ( $func ) {
try {
$result = $func( ...$args );
$result = $this->functionCaller( $func, $args );
} catch ( Exception $e ) {
// Preserve original trace, especially from custom functions.
// https://github.com/wikimedia/less.php/issues/38
Expand Down
Loading

0 comments on commit 7d05070

Please sign in to comment.