Merge "Less_Parser: Remove dead code in getVariables() and improve te…

…st coverage"
wikimedia · Aug 12, 2024 · 160bb6e · 160bb6e
2 parents 47b2459 + 9e047b4
commit 160bb6e
Show file tree

Hide file tree

Showing 3 changed files with 18 additions and 22 deletions.
diff --git a/lib/Less/Parser.php b/lib/Less/Parser.php
@@ -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;
@@ -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 ) ) {

diff --git a/lib/Less/Tree/NameValue.php b/lib/Less/Tree/NameValue.php
@@ -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`.

diff --git a/test/phpunit/ParserTest.php b/test/phpunit/ParserTest.php
@@ -86,6 +86,9 @@ public function testGetVariables() {
 			// Rule > Dimension
 			@some_number: 123;
 
+			// Rule > Dimension
+			@some_number_dot: 123.1;
+
 			// Rule > Dimension
 			@some_unit: 12px;
 
@@ -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();
@@ -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()
 		);