Improve support for @import (reference) matching Less.js 2.x

https://lesscss.org/features/#import-atrules-feature-reference * v2.3.0: Fix missing `@support` blocks imported by reference. see less/less.js@8f1c35a * v2.5.2: Preserve inline comments imported by reference. see less/less.js@74ef1eb * v2.5.0: Fix unexpected output from indirect imports when imported by reference. see less/less.js@6c66aeb Bug: T362647 Change-Id: Ibe33cc358fea5b1f8fa6e6445b4315157a8ddfe2
wikimedia · Apr 29, 2024 · 6edad32 · 6edad32
1 parent 299a47f
commit 6edad32
Show file tree

Hide file tree

Showing 9 changed files with 154 additions and 20 deletions.
diff --git a/lib/Less/Tree.php b/lib/Less/Tree.php
@@ -131,7 +131,6 @@ public static function numericCompare( $a, $b ) {
 	public static function ReferencedArray( $rules ) {
 		foreach ( $rules as $rule ) {
 			if ( method_exists( $rule, 'markReferenced' ) ) {
-				// @phan-suppress-next-line PhanUndeclaredMethod
 				$rule->markReferenced();
 			}
 		}

diff --git a/lib/Less/Tree/Anonymous.php b/lib/Less/Tree/Anonymous.php
@@ -11,25 +11,27 @@ class Less_Tree_Anonymous extends Less_Tree implements Less_Tree_HasValuePropert
 	public $currentFileInfo;
 	/** @var bool */
 	public $rulesetLike;
-	// TODO: Do we need upstream's bool $referenced?
+	public $isReferenced;
 
 	/**
 	 * @param string $value
 	 * @param int|null $index
 	 * @param array|null $currentFileInfo
 	 * @param bool|null $mapLines
 	 * @param bool $rulesetLike
+	 * @param bool $referenced
 	 */
-	public function __construct( $value, $index = null, $currentFileInfo = null, $mapLines = null, $rulesetLike = false ) {
+	public function __construct( $value, $index = null, $currentFileInfo = null, $mapLines = null, $rulesetLike = false, $referenced = false ) {
 		$this->value = $value;
 		$this->index = $index;
 		$this->mapLines = $mapLines;
 		$this->currentFileInfo = $currentFileInfo;
 		$this->rulesetLike = $rulesetLike;
+		$this->isReferenced = $referenced;
 	}
 
 	public function compile( $env ) {
-		return new self( $this->value, $this->index, $this->currentFileInfo, $this->mapLines );
+		return new self( $this->value, $this->index, $this->currentFileInfo, $this->mapLines, $this->rulesetLike, $this->isReferenced );
 	}
 
 	/**
@@ -56,4 +58,11 @@ public function toCSS() {
 		return $this->value;
 	}
 
+	public function markReferenced() {
+		$this->isReferenced = true;
+	}
+
+	public function getIsReferenced() {
+		return !isset( $this->currentFileInfo['reference'] ) || !$this->currentFileInfo['reference'] || $this->isReferenced;
+	}
 }
diff --git a/lib/Less/Tree/Directive.php b/lib/Less/Tree/Directive.php
@@ -13,7 +13,7 @@ class Less_Tree_Directive extends Less_Tree implements Less_Tree_HasValuePropert
 	public $currentFileInfo;
 	public $debugInfo;
 
-	public function __construct( $name, $value = null, $rules = null, $index = null, $isRooted = false, $currentFileInfo = null, $debugInfo = null ) {
+	public function __construct( $name, $value = null, $rules = null, $index = null, $isRooted = false, $currentFileInfo = null, $debugInfo = null, $isReferenced = false ) {
 		$this->name = $name;
 		$this->value = $value;
 
@@ -34,6 +34,7 @@ public function __construct( $name, $value = null, $rules = null, $index = null,
 		$this->isRooted = $isRooted;
 		$this->currentFileInfo = $currentFileInfo;
 		$this->debugInfo = $debugInfo;
+		$this->isReferenced = $isReferenced;
 	}
 
 	public function accept( $visitor ) {
@@ -95,7 +96,7 @@ public function compile( $env ) {
 		$env->mediaPath = $mediaPathBackup;
 		$env->mediaBlocks = $mediaPBlocksBackup;
 
-		return new self( $this->name, $value, $rules, $this->index, $this->isRooted, $this->currentFileInfo, $this->debugInfo );
+		return new self( $this->name, $value, $rules, $this->index, $this->isRooted, $this->currentFileInfo, $this->debugInfo, $this->isReferenced );
 	}
 
 	public function variable( $name ) {
@@ -117,6 +118,10 @@ public function markReferenced() {
 		}
 	}
 
+	public function getIsReferenced() {
+		return !isset( $this->currentFileInfo['reference'] ) || !$this->currentFileInfo['reference'] || $this->isReferenced;
+	}
+
 	public function emptySelectors() {
 		$el = new Less_Tree_Element( '', '&', $this->index, $this->currentFileInfo );
 		$sels = [ new Less_Tree_Selector( [ $el ], [], null, $this->index, $this->currentFileInfo ) ];

diff --git a/lib/Less/Tree/Expression.php b/lib/Less/Tree/Expression.php
@@ -88,6 +88,16 @@ public function throwAwayComments() {
 		}
 	}
 
+	public function markReferenced() {
+		if ( is_array( $this->value ) ) {
+			foreach ( $this->value as $v ) {
+				if ( method_exists( $v, 'markReferenced' ) ) {
+					$v->markReferenced();
+				}
+			}
+		}
+	}
+
 	/**
 	 * Should be used only in Less_Tree_Call::functionCaller()
 	 * to retrieve expression without comments

diff --git a/lib/Less/Tree/Import.php b/lib/Less/Tree/Import.php
@@ -80,9 +80,7 @@ public function accept( $visitor ) {
 	}
 
 	public function genCSS( $output ) {
-		// TODO: this.path.currentFileInfo.reference === undefined
-		// Related: T352862 (test/less-2.5.3/less/import-reference.less)
-		if ( $this->css ) {
+		if ( $this->css && !isset( $this->path->currentFileInfo["reference"] ) ) {
 			$output->add( '@import ', $this->currentFileInfo, $this->index );
 			$this->path->genCSS( $output );
 			if ( $this->features ) {
@@ -184,8 +182,8 @@ public function compile( $env ) {
 					'reference' => $this->currentFileInfo['reference'] ?? null,
 				],
 				true,
-				true
-				// TODO: We might need upstream's bool $referenced param to Anonymous
+				true,
+				false
 			);
 			return $this->features
 				? new Less_Tree_Media( [ $contents ], $this->features->value )

diff --git a/lib/Less/Tree/Rule.php b/lib/Less/Tree/Rule.php
@@ -123,4 +123,24 @@ public function makeImportant() {
 		return new self( $this->name, $this->value, '!important', $this->merge, $this->index, $this->currentFileInfo, $this->inline );
 	}
 
+	public function mark( $value ) {
+		if ( !is_array( $this->value ) ) {
+
+			if ( method_exists( $value, 'markReferenced' ) ) {
+				// @phan-suppress-next-line PhanUndeclaredMethod
+				$value->markReferenced();
+			}
+		} else {
+			foreach ( $this->value as $v ) {
+				$this->mark( $v );
+			}
+		}
+	}
+
+	public function markReferenced() {
+		if ( $this->value ) {
+			$this->mark( $this->value );
+		}
+	}
+
 }
diff --git a/lib/Less/Tree/Ruleset.php b/lib/Less/Tree/Ruleset.php
@@ -486,12 +486,41 @@ public function genCSS( $output ) {
 	}
 
 	public function markReferenced() {
-		if ( !$this->selectors ) {
-			return;
+		if ( $this->selectors !== null ) {
+			foreach ( $this->selectors as $selector ) {
+				$selector->markReferenced();
+			}
+		}
+
+		if ( $this->rules ) {
+			foreach ( $this->rules as $rule ) {
+				if ( method_exists( $rule, 'markReferenced' ) ) {
+					$rule->markReferenced();
+				}
+			}
+		}
+	}
+
+	public function getIsReferenced() {
+		if ( $this->paths ) {
+			foreach ( $this->paths as $path ) {
+				foreach ( $path as $p ) {
+					if ( method_exists( $p, 'getIsReferenced' ) && $p->getIsReferenced() ) {
+						return true;
+					}
+				}
+			}
 		}
-		foreach ( $this->selectors as $selector ) {
-			$selector->markReferenced();
+
+		if ( $this->selectors ) {
+			foreach ( $this->selectors as $selector ) {
+				if ( method_exists( $selector, 'getIsReferenced' ) && $selector->getIsReferenced() ) {
+					return true;
+				}
+			}
 		}
+
+		return false;
 	}
 
 	/**

diff --git a/lib/Less/Visitor/toCSS.php b/lib/Less/Visitor/toCSS.php
@@ -52,11 +52,12 @@ public function visitMedia( $mediaNode, &$visitDeeper ) {
 		return $mediaNode;
 	}
 
-	public function visitDirective( $directiveNode ) {
-		if ( isset( $directiveNode->currentFileInfo['reference'] ) && ( !property_exists( $directiveNode, 'isReferenced' ) || !$directiveNode->isReferenced ) ) {
-			return [];
-		}
+	public function visitDirective( $directiveNode, &$visitDeeper ) {
 		if ( $directiveNode->name === '@charset' ) {
+
+			if ( !$directiveNode->getIsReferenced() ) {
+				return;
+			}
 			// Only output the debug info together with subsequent @charset definitions
 			// a comment (or @media statement) before the actual @charset directive would
 			// be considered illegal css as it has to be on the first line
@@ -72,6 +73,35 @@ public function visitDirective( $directiveNode ) {
 			}
 			$this->charset = true;
 		}
+
+		if ( $directiveNode->rules ) {
+			$this->_mergeRules( $directiveNode->rules[0]->rules );
+			// process childs
+			$directiveNode->accept( $this );
+			$visitDeeper = false;
+
+			// the directive was directly referenced and therefore needs to be shown in the output
+			if ( $directiveNode->getIsReferenced() ) {
+				return $directiveNode;
+			}
+
+			if ( !$directiveNode->rules ) {
+				return [];
+			}
+			if ( $this->hasVisibleChild( $directiveNode ) ) {
+				// marking as referenced in case the directive is stored inside another directive
+				$directiveNode->markReferenced();
+				return $directiveNode;
+			}
+				// The directive was not directly referenced and does not contain anything that
+				//was referenced. Therefore it must not be shown in output.
+				return;
+		} else {
+			if ( !$directiveNode->getIsReferenced() ) {
+				return;
+			}
+		}
+
 		return $directiveNode;
 	}
 
@@ -143,6 +173,22 @@ public function visitRuleset( $rulesetNode, &$visitDeeper ) {
 		return $rulesets;
 	}
 
+	public function visitAnonymous( $anonymousNode ) {
+		if ( !$anonymousNode->getIsReferenced() ) {
+			return;
+		}
+
+		$anonymousNode->accept( $this );
+		return $anonymousNode;
+	}
+
+	public function visitImport( $importNode ) {
+		if ( isset( $importNode->path->currentFileInfo["reference"] ) && $importNode->css ) {
+			return;
+		}
+		return $importNode;
+	}
+
 	/**
 	 * Helper function for visitiRuleset
 	 *
@@ -273,4 +319,23 @@ public static function toValue( $values ) {
 		}
 		return new Less_Tree_Value( $mapped );
 	}
+
+	public function hasVisibleChild( $directiveNode ) {
+		// prepare list of childs
+		$rule = $bodyRules = $directiveNode->rules;
+		// if there is only one nested ruleset and that one has no path, then it is
+		//just fake ruleset that got not replaced and we need to look inside it to
+		//get real childs
+		if ( count( $bodyRules ) === 1 && ( !$bodyRules[0]->paths || count( $bodyRules[0]->paths ) === 0 ) ) {
+			$bodyRules = $bodyRules[0]->rules;
+		}
+		foreach ( $bodyRules as $rule ) {
+			if ( method_exists( $rule, 'getIsReferenced' ) && $rule->getIsReferenced() ) {
+				// the directive contains something that was referenced (likely by extend)
+				//therefore it needs to be shown in output too
+				return true;
+			}
+		}
+		return false;
+	}
 }
diff --git a/test/phpunit/FixturesTest.php b/test/phpunit/FixturesTest.php
@@ -24,7 +24,6 @@ class phpunit_FixturesTest extends phpunit_bootstrap {
 
 			// Temporary disabled
 			'css' => true, // T352911 & T352866
-			'import-reference' => true, // T352862
 			'mixins-guards' => true, // T352867
 			'urls' => true, // T353147
 			'variables' => true, // T352830, T352866