Fix importing of nested files with url()

Bug: T353146
Change-Id: Ie3f7a08a9a2880cac95022fb2770b59e7b763086

lib/Less/ImportVisitor.php

@@ -96,6 +96,7 @@ public function processImportNode( $importNode, $env, &$importParent ) {
 			// import dirs resolution logic.
 			// TODO: We might need upstream's `tryAppendLessExtension` logic here.
 			//       We currenlty do do this in getPath instead.
+
 			$path_and_uri = $env->callImportCallback( $importNode );
 			if ( !$path_and_uri ) {
 				$path_and_uri = $importNode->PathAndUri();
@@ -195,7 +196,7 @@ public function processImportNode( $importNode, $env, &$importParent ) {
 	}
 
 	public function addVariableImport( $callback ) {
-		array_push( $this->variableImports, $callback );
+		$this->variableImports[] = $callback;
 	}
 
 	public function tryRun() {

lib/Less/Tree/Import.php

@@ -97,25 +97,19 @@ public function genCSS( $output ) {
 	 * @return string|null
 	 */
 	public function getPath() {
-		if ( $this->path instanceof Less_Tree_Quoted ) {
-			$path = $this->path->value;
-			// TODO: This should be moved to ImportVisitor using $tryAppendLessExtension
-			// to match upstream. However, to remove this from here we have to first
-			// fix differences with how/when 'import_callback' is executed.
-			$path = ( isset( $this->css ) || preg_match( '/(\.[a-z]*$)|([\?;].*)$/', $path ) ) ? $path : $path . '.less';
-
 		// During the first pass, Less_Tree_Url may contain a Less_Tree_Variable (not yet expanded),
 		// and thus has no value property defined yet. Return null until we reach the next phase.
 		// https://github.com/wikimedia/less.php/issues/29
-		// TODO: Do we still need this now that we have ImportVisitor?
-		} elseif ( $this->path instanceof Less_Tree_Url && !( $this->path->value instanceof Less_Tree_Variable ) ) {
-			$path = $this->path->value->value;
-		} else {
-			return null;
+		// TODO: Upstream doesn't need a check against Less_Tree_Variable. Why do we?
+		$path = ( $this->path instanceof Less_Tree_Url && !( $this->path->value instanceof Less_Tree_Variable ) )
+			? $this->path->value->value
+			// e.g. Less_Tree_Quoted
+			: $this->path->value;
+
+		if ( is_string( $path ) ) {
+			// remove query string and fragment
+			return preg_replace( '/[\?#][^\?]*$/', '', $path );
 		}
-
-		// remove query string and fragment
-		return preg_replace( '/[\?#][^\?]*$/', '', $path );
 	}
 
 	public function isVariableImport() {
@@ -130,8 +124,11 @@ public function isVariableImport() {
 	}
 
 	public function compileForImport( $env ) {
-		// TODO: We might need upstream `if (path instanceof URL) { path = path.value; }`
-		return new self( $this->path->compile( $env ), $this->features, $this->options, $this->index, $this->currentFileInfo );
+		$path = $this->path;
+		if ( $path instanceof Less_Tree_Url ) {
+			 $path = $path->value;
+		}
+		return new self( $path->compile( $env ), $this->features, $this->options, $this->index, $this->currentFileInfo );
 	}
 
 	public function compilePath( $env ) {
@@ -216,6 +213,13 @@ public function compile( $env ) {
 	 */
 	public function PathAndUri() {
 		$evald_path = $this->getPath();
+
+		$tryAppendLessExtension = $this->css === null;
+
+		if ( $tryAppendLessExtension ) {
+			$evald_path = ( isset( $this->css ) || preg_match( '/(\.[a-z]*$)|([\?;].*)$/', $evald_path ) ) ? $evald_path : $evald_path . '.less';
+		}
+
 		// TODO: Move callImportCallback() and getPath() fallback logic from callers
 		//       to here so that PathAndUri() is equivalent to upstream fileManager.getPath()
 

test/phpunit/FixturesTest.php

@@ -25,7 +25,6 @@ class phpunit_FixturesTest extends phpunit_bootstrap {
 			// Temporary disabled
 			'comments2' => true, // T353132
 			'css' => true, // T352911 & T352866
-			'import' => true, // T353146
 			'import-reference' => true, // T352862
 			'mixin-args' => true, // T352897
 			'mixins-args' => true, // T352897

test/phpunit/ParserTest.php

@@ -142,7 +142,7 @@ public function testImportCallback() {
 		$parser = new Less_Parser( [
 			'compress' => true,
 			'import_callback' => static function ( $importNode ) {
-				if ( $importNode->getPath() === '@wikimedia/example.less' ) {
+				if ( $importNode->getPath() === '@wikimedia/example' ) {
 					return [ __DIR__ . '/data/importdir-somevars/callme.less', '/site' ];
 				}
 			}