Skip to content

Commit 0994996

Browse files
committed
Fix PHP Warning: Undefined property $value in extract()
== What == When a Less input file passes a list node to extract(), we access the internal array value and use that to access the given index. When a Less input file passes something else to extract(), the value should be treated as an array containing that 1 node (which means index 1 is the given node, and any other index returns null). This last part was not working correctly in our implementation. == Before == ``` $ bin/lessc test/Fixtures/less.php/less/T391735-extract-non-array.less PHP Warning: Undefined property: Less_Tree_Call::$value in lib/Less/Functions.php PHP Warning: Undefined property: Less_Tree_DetachedRuleset::$value in lib/Less/Functions.php ``` A bare slash can also cause the error (as reported at T391735), although only with `--math=parens-division`, because otherwise the slash operation is already compiled away before the function call is compiled. Note that "test/Fixtures/less.php" is registered in test/fixtures.php with math=always. ``` @x: 20px; .foo-one { .foo(@x / 1px); } .foo(@value: false) when not (@value = false) and not (extract(@value, 1) = false) and not (extract(@value, 2) = false) and not (extract(@value, 3) = false) and not (extract(@value, 4) = false) and not (extract(@value, 5) = false) { foo: @value; } ``` ``` $ bin/less --math=parens-division tmp.less PHP Warning: Undefined property: Less_Tree_Operation::$value in lib/Less/Functions.php ``` == How == We already did the right thing in Functions::length(), where we list the two Less_Tree classes that represent list nodes and have internal array values. Update our implementation to match Less.js 3.13, which moved this logic to getItemsFromNode() and re-uses it in Functions::extract(). Bug: T391735 Change-Id: Id64ff9f13039571caf475d2e4085d2ef06654508
1 parent d22e08f commit 0994996

File tree

3 files changed

+62
-18
lines changed

3 files changed

+62
-18
lines changed

lib/Less/Functions.php

Lines changed: 26 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -941,33 +941,41 @@ public function shade( $color, $amount = null ) {
941941
}
942942

943943
/**
944-
* @see less-3.13.1.js#functions _SELF
944+
* @see less-3.13.1.js#getItemsFromNode
945+
*/
946+
private function getItemsFromNode( Less_Tree $node ) {
947+
// handle non-array values as an array of length 1
948+
// return 'undefined' if index is invalid
949+
//
950+
// NOTE: Less.js uses duck-typing `isArray(node.value)`, which would cause warnings in PHP,
951+
// and potentially bugs for Less_Tree classes with a $value that is only sometimes an array.
952+
// Instead, check for Less_Tree classes that always implement an array $value.
953+
return ( $node instanceof Less_Tree_Expression || $node instanceof Less_Tree_Value )
954+
? $node->value
955+
: [ $node ];
956+
}
957+
958+
/**
959+
* @see less-3.13.1.js#_SELF
945960
*/
946961
public function _self( $args ) {
947962
return $args;
948963
}
949964

965+
/**
966+
* @see less-3.13.1.js#extract
967+
*/
950968
public function extract( $values, $index ) {
951-
$index = (int)$index->value - 1; // (1-based index)
952-
// handle non-array values as an array of length 1
953-
// return 'undefined' if index is invalid
954-
if ( !( $values instanceof Less_Tree_Color ) && is_array( $values->value ) ) {
955-
if ( isset( $values->value[$index] ) ) {
956-
return $values->value[$index];
957-
}
958-
return null;
959-
960-
} elseif ( (int)$index === 0 ) {
961-
return $values;
962-
}
963-
964-
return null;
969+
// (1-based index)
970+
$index = (int)$index->value - 1;
971+
return $this->getItemsFromNode( $values )[ $index ] ?? null;
965972
}
966973

974+
/**
975+
* @see less-3.13.1.js#length
976+
*/
967977
public function length( $values ) {
968-
$n = ( $values instanceof Less_Tree_Expression || $values instanceof Less_Tree_Value ) ?
969-
count( $values->value ) : 1;
970-
return new Less_Tree_Dimension( $n );
978+
return new Less_Tree_Dimension( count( $this->getItemsFromNode( $values ) ) );
971979
}
972980

973981
/**
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
.foo-one-div {
2+
foo: 1;
3+
}
4+
.foo-one-detached {
5+
foo: 1;
6+
}
7+
.foo-two {
8+
foo: 2;
9+
}
10+
.foo-includes-false {
11+
color: purple;
12+
}
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
@x: 20px;
2+
3+
.foo-one-div {
4+
.foo(calc(@x / 1px));
5+
}
6+
.foo-one-detached {
7+
.foo( { color: blue } );
8+
}
9+
.foo-two {
10+
.foo(calc(@x / 1px) 2px);
11+
}
12+
.foo-includes-false {
13+
color: purple;
14+
.foo(@x 2px false 4px);
15+
}
16+
17+
.foo(@value: false) when not (@value = false)
18+
and not (extract(@value, 1) = false)
19+
and not (extract(@value, 2) = false)
20+
and not (extract(@value, 3) = false)
21+
and not (extract(@value, 4) = false)
22+
and not (extract(@value, 5) = false) {
23+
foo: length(@value);
24+
}

0 commit comments

Comments
 (0)