Skip to content

Commit

Permalink
Less_Parser: Remove dead code in getVariables() and improve test cove…
Browse files Browse the repository at this point in the history
…rage

Test:
* It doesn't actually return native float or int for whole numbers.
  The test was fooling itself because it used arrayEquals() instead
  of arraySame(), which allows '123' to equal 123.0.

  The number is actually returned as a string in this case.
* Add test where the value is really returned as a native number.
* Add test for array.

Code:
* This method is part of our own API, and not ported from Less.js,
  so we have a bit more freedom over its implementation to make
  sense for us.

* Remove "else if instanceof Less_Tree_Comment" branch,
  because it is impossible to reach. The in_array check already
  excludes where get_class() === Less_Tree_Comment.

  This would also explain the lack of code coverage
  https://doc.wikimedia.org/cover/mediawiki-libs-less.php/Parser.php.html#285

* Remove redundant "not_variable_type" check because the loop
  already does `if instanceof Less_Tree_Declaration" which thus
  excludes all others. None of the entries in not_variable_type
  where subclasses of Less_Tree_Declaration, so it's not there for
  that reason, either.

Change-Id: I42fc406e64157432e99f822ca15c8d9eb8986e3f
  • Loading branch information
Krinkle committed Aug 12, 2024
1 parent 29e1451 commit 9e047b4
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 22 deletions.
23 changes: 3 additions & 20 deletions lib/Less/Parser.php
Original file line number Diff line number Diff line change
Expand Up @@ -276,36 +276,18 @@ public function findValueOf( $varName ) {
}

/**
* Gets the private rules variable and returns an array of the found variables
* it uses a helper method getVariableValue() that contains the logic ot fetch the value
* from the rule object
* Get an array of the found variables in the parsed input.
*
* @return array
* @phan-return array<string,string|float|array>
*/
public function getVariables() {
$variables = [];

$not_variable_type = [
Less_Tree_Comment::class, // this include less comments ( // ) and css comments (/* */)
Less_Tree_Import::class, // do not search variables in included files @import
Less_Tree_Ruleset::class, // selectors (.someclass, #someid, …)
Less_Tree_Operation::class,
];

$rules = $this->cachedEvaldRules ?? $this->rules;

foreach ( $rules as $key => $rule ) {
if ( in_array( get_class( $rule ), $not_variable_type ) ) {
continue;
}

// Note: it seems $rule is always Less_Tree_Rule when variable = true
if ( $rule instanceof Less_Tree_Declaration && $rule->variable ) {
$variables[$rule->name] = $this->getVariableValue( $rule );
} else {
if ( $rule instanceof Less_Tree_Comment ) {
$variables[] = $this->getVariableValue( $rule );
}
}
}
return $variables;
Expand All @@ -330,6 +312,7 @@ public function findVarByName( $var_name ) {
*
* @param Less_Tree $var
* @return mixed
* @phan-return string|float|array<string|float>
*/
private function getVariableValue( Less_Tree $var ) {
switch ( get_class( $var ) ) {
Expand Down
2 changes: 1 addition & 1 deletion lib/Less/Tree/NameValue.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* A simple CSS name-value pair, e.g. `width: 100px;`
*
* In bootstrap, there are about 600-1000 simple name-value pairs (depending on
* how forgiving the match is) -vs- 6,020 dynamic rules (Less_Tree_Rule).
* how forgiving the match is) -vs- 6,020 dynamic rules (Less_Tree_Declaration).
*
* Using the name-value object can speed up bootstrap compilation slightly, but
* it breaks color keyword interpretation: `color: red` -> `color: #FF0000`.
Expand Down
15 changes: 14 additions & 1 deletion test/phpunit/ParserTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,9 @@ public function testGetVariables() {
// Rule > Dimension
@some_number: 123;
// Rule > Dimension
@some_number_dot: 123.1;
// Rule > Dimension
@some_unit: 12px;
Expand All @@ -97,6 +100,12 @@ public function testGetVariables() {
// Rule > Url > Quoted
@some_url: url("just/a/test.jpg");
@some_list_comma: 1, 2, foo, bar baz, 42;
@some_list_comma_dot: 1, 2.1, foo, bar baz, 42;
@some_list_space: foo bar baz;
';

$parser = new Less_Parser();
Expand All @@ -106,11 +115,15 @@ public function testGetVariables() {
$this->assertEquals(
[
'@some_string' => '"foo"',
'@some_number' => 123.0,
'@some_number' => '123',
'@some_number_dot' => 123.1,
'@some_unit' => '12px',
'@some_unit_op' => '5px',
'@some_color' => '#f9f9f9',
'@some_url' => 'url("just/a/test.jpg")',
'@some_list_comma' => [ '1', '2', 'foo', 'bar baz', '42' ],
'@some_list_comma_dot' => [ 1.0, 2.1, 'foo', 'bar baz', 42.0 ],
'@some_list_space' => 'foo bar baz',
],
$parser->getVariables()
);
Expand Down

0 comments on commit 9e047b4

Please sign in to comment.