Merge "Import,ImportVisitor: Simplify FileManager fallback"

wikimedia · Jul 1, 2024 · 1b8e1c8 · 1b8e1c8
2 parents 1ed8afc + 62abad9
commit 1b8e1c8
Show file tree

Hide file tree

Showing 4 changed files with 24 additions and 29 deletions.
diff --git a/lib/Less/Cache.php b/lib/Less/Cache.php
@@ -238,8 +238,8 @@ public static function CleanCache() {
 				continue;
 			}
 
-			$full_path = self::$cache_dir . $file;
-			$mtime = filemtime( $full_path );
+			$fullPath = self::$cache_dir . $file;
+			$mtime = filemtime( $fullPath );
 
 			// don't delete if it's a relatively new file
 			if ( $mtime > $check_time ) {
@@ -248,7 +248,7 @@ public static function CleanCache() {
 
 			// delete the list file and associated css file
 			if ( $type === 'list' ) {
-				self::ListFiles( $full_path, $list, $css_file_name );
+				self::ListFiles( $fullPath, $list, $css_file_name );
 				if ( $css_file_name ) {
 					$css_file = self::$cache_dir . $css_file_name;
 					if ( file_exists( $css_file ) ) {
@@ -257,7 +257,7 @@ public static function CleanCache() {
 				}
 			}
 
-			unlink( $full_path );
+			unlink( $fullPath );
 		}
 	}
 

diff --git a/lib/Less/FileManager.php b/lib/Less/FileManager.php
@@ -8,7 +8,7 @@ class Less_FileManager {
 	 * @see less-node/FileManager.getPath https://github.com/less/less.js/blob/v2.5.3/lib/less-node/file-manager.js#L70
 	 * @param string $filename
 	 * @param null|array $currentFileInfo
-	 * @return null|array
+	 * @return null|array{0:string,1:string}
 	 */
 	public static function getFilePath( $filename, $currentFileInfo ) {
 		if ( !$filename ) {

diff --git a/lib/Less/ImportVisitor.php b/lib/Less/ImportVisitor.php
@@ -101,13 +101,8 @@ public function processImportNode( $importNode, $env, &$importParent ) {
 					$path = preg_match( '/(\.[a-z]*$)|([\?;].*)$/', $path ) ? $path : $path . '.less';
 			}
 
-			$path_and_uri = Less_FileManager::getFilePath( $path, $importNode->currentFileInfo );
-			if ( $path_and_uri ) {
-				[ $full_path, $uri ] = $path_and_uri;
-			} else {
-				$full_path = $uri = $importNode->getPath();
-			}
-			'@phan-var string $full_path';
+			[ $fullPath, $uri ] =
+				Less_FileManager::getFilePath( $path, $importNode->currentFileInfo ) ?? [ $path, $path ];
 
 			// @see less-2.5.3.js#ImportManager.prototype.push/loadFileCallback
 
@@ -122,22 +117,22 @@ public function processImportNode( $importNode, $env, &$importParent ) {
 			$e = null;
 			try {
 				if ( $importNode->options['inline'] ) {
-					if ( !file_exists( $full_path ) ) {
+					if ( !file_exists( $fullPath ) ) {
 						throw new Less_Exception_Parser(
-							sprintf( 'File `%s` not found.', $full_path ),
+							sprintf( 'File `%s` not found.', $fullPath ),
 							null,
 							$importNode->index,
 							$importNode->currentFileInfo
 						);
 					}
-					$root = file_get_contents( $full_path );
+					$root = file_get_contents( $fullPath );
 				} else {
 					$parser = new Less_Parser( $env );
 					// NOTE: Upstream sets `env->processImports = false` here to avoid
 					// running ImportVisitor again (infinite loop). We instead separate
 					// Less_Parser->parseFile() from Less_Parser->getCss(),
 					// and only getCss() runs ImportVisitor.
-					$root = $parser->parseFile( $full_path, $uri, true );
+					$root = $parser->parseFile( $fullPath, $uri, true );
 				}
 			} catch ( Less_Exception_Parser $err ) {
 				$e = $err;
@@ -148,7 +143,7 @@ public function processImportNode( $importNode, $env, &$importParent ) {
 			if ( $importNode->options['optional'] && $e ) {
 				$e = null;
 				$root = new Less_Tree_Ruleset( null, [] );
-				$full_path = null;
+				$fullPath = null;
 			}
 
 			// @see less-2.5.3.js#ImportVisitor.prototype.onImported
@@ -162,7 +157,7 @@ public function processImportNode( $importNode, $env, &$importParent ) {
 				throw $e;
 			}
 
-			$duplicateImport = isset( $this->recursionDetector[$full_path] );
+			$duplicateImport = isset( $this->recursionDetector[$fullPath] );
 
 			if ( !$env->importMultiple ) {
 				if ( $duplicateImport ) {
@@ -175,16 +170,16 @@ public function processImportNode( $importNode, $env, &$importParent ) {
 				}
 			}
 
-			if ( !$full_path && $importNode->options['optional'] ) {
+			if ( !$fullPath && $importNode->options['optional'] ) {
 				$importNode->doSkip = true;
 			}
 
 			if ( $root ) {
 				$importNode->root = $root;
-				$importNode->importedFilename = $full_path;
+				$importNode->importedFilename = $fullPath;
 
-				if ( !$inlineCSS && ( $env->importMultiple || !$duplicateImport ) && $full_path ) {
-					$this->recursionDetector[$full_path] = true;
+				if ( !$inlineCSS && ( $env->importMultiple || !$duplicateImport ) && $fullPath ) {
+					$this->recursionDetector[$fullPath] = true;
 					$oldContext = $this->env;
 					$this->env = $env;
 					$this->visitObj( $root );

diff --git a/lib/Less/Tree/Import.php b/lib/Less/Tree/Import.php
@@ -195,22 +195,22 @@ public function compile( $env ) {
 	 * @return bool|null
 	 */
 	public function skip( $env ) {
-		[ $path ] = Less_FileManager::getFilePath( $this->getPath(), $this->currentFileInfo );
-
-		if ( !$path ) {
-			$path = $this->getPath();
-		}
+		$path = $this->getPath();
+		// TODO: Since our Import->getPath() varies from upstream Less.js (ours can return null).
+		// we therefore need an empty string fallback here. Remove this fallback once getPath()
+		// is in sync with upstream.
+		$fullPath = Less_FileManager::getFilePath( $path, $this->currentFileInfo )[0] ?? $path ?? '';
 
 		if ( $this->doSkip !== null ) {
 			return $this->doSkip;
 		}
 
 		// @see less-2.5.3.js#ImportVisitor.prototype.onImported
-		if ( isset( $env->importVisitorOnceMap[$path] ) ) {
+		if ( isset( $env->importVisitorOnceMap[$fullPath] ) ) {
 			return true;
 		}
 
-		$env->importVisitorOnceMap[$path] = true;
+		$env->importVisitorOnceMap[$fullPath] = true;
 		return false;
 	}
 }