Parser,Environment: Reduce various internal use of static state

This is not a port of any specific upstream Less.js changes. For many
years, whenever we ported upstream changes in Parser or Envirnment,
we mapped the logic onto static equivalents, to keep the changes
simpler. However, upstream is now for the most part based on instance
state and by-reference objects. This hasn't yet caused any obvious
bugs, but it does decrease confidence in how things work, and it makes
some big changes part of T353133 difficult to port.

This patchs should help prevent future bugs as result from upcoming
porting commits by re-synchronising the code to match upstream not
just in how the code looks, but also in the behaviour (instance vs
static).

Sync with Less.js:

* Manage math state in Less_Environment object, instead of
  modifying Less_Parser::$options or Less_Environment::$mathOn.

* Manage list of imported files in Less_Environment object.
  In Less.js, this is kept in the ImportVisitor, but we haven't
  implemented that yet. I'm leaving that for a future change by Hannah,
  and now only moving it away from static state as first step.

Also:

* Move SetOptions() in Less_Parser::__construct to Reset().
  This was causing a problem because $this->env is not yet defined
  during that call. Without this, SetOption() can't pass strictMath
  to Less_Environment.

Bug: T353133
Change-Id: If4a0a7ce052fb378f078e767fdb7449bd7e78de2

API.md

@@ -59,7 +59,7 @@ file(s) and any direct and indirect imports.
 $parser = new Less_Parser();
 $parser->parseFile( '/var/www/mysite/bootstrap.less', '/mysite/' );
 $css = $parser->getCss();
-$files = $parser->AllParsedFiles();
+$files = $parser->getParsedFiles();
 ```
 
 #### Compress output

lessc.inc.php

@@ -129,8 +129,7 @@ public function compile( $string, $name = null ) {
 		$parser->parse( $string );
 		$out = $parser->getCss();
 
-		$parsed = Less_Parser::AllParsedFiles();
-		foreach ( $parsed as $file ) {
+		foreach ( $parser->getParsedFiles() as $file ) {
 			$this->addParsedFile( $file );
 		}
 
@@ -165,8 +164,7 @@ public function compileFile( $fname, $outFname = null ) {
 		$parser->parseFile( $fname );
 		$out = $parser->getCss();
 
-		$parsed = Less_Parser::AllParsedFiles();
-		foreach ( $parsed as $file ) {
+		foreach ( $parser->getParsedFiles() as $file ) {
 			$this->addParsedFile( $file );
 		}
 

lib/Less/Cache.php

@@ -149,7 +149,7 @@ public static function Cache( &$less_files, $parser_options = [] ) {
 
 		$compiled = $parser->getCss();
 
-		$less_files = $parser->AllParsedFiles();
+		$less_files = $parser->getParsedFiles();
 
 		return $compiled;
 	}

lib/Less/Environment.php

@@ -26,6 +26,9 @@ class Less_Environment {
 	/** @var Less_Tree_Media[] */
 	public $mediaPath = [];
 
+	/** @var string[] */
+	public $imports = [];
+
 	public static $parensStack = 0;
 
 	public static $tabLevel = 0;
@@ -36,7 +39,7 @@ class Less_Environment {
 
 	public static $mixin_stack = 0;
 
-	public static $mathOn = true;
+	public $strictMath = false;
 
 	/**
 	 * @var array
@@ -56,6 +59,22 @@ public function Init() {
 		];
 	}
 
+	/**
+	 * @param string $file
+	 * @return void
+	 */
+	public function addParsedFile( $file ) {
+		$this->imports[] = $file;
+	}
+
+	/**
+	 * @param string $file
+	 * @return bool
+	 */
+	public function isFileParsed( $file ) {
+		return in_array( $file, $this->imports );
+	}
+
 	public function copyEvalEnv( $frames = [] ) {
 		$new_env = new self();
 		$new_env->frames = $frames;
@@ -66,11 +85,8 @@ public function copyEvalEnv( $frames = [] ) {
 	 * @return bool
 	 * @see Eval.prototype.isMathOn in less.js 3.0.0 https://github.com/less/less.js/blob/v3.0.0/dist/less.js#L1007
 	 */
-	public static function isMathOn() {
-		if ( !self::$mathOn ) {
-			return false;
-		}
-		return !Less_Parser::$options['strictMath'] || self::$parensStack;
+	public function isMathOn() {
+		return $this->strictMath ? (bool)self::$parensStack : true;
 	}
 
 	/**

lib/Less/Parser.php

@@ -36,9 +36,12 @@ class Less_Parser {
 
 	];
 
-	/** @var array{compress:bool,strictUnits:bool,strictMath:bool,numPrecision:int,import_dirs:array,import_callback:null|callable,indentation:string} */
+	/** @var array{compress:bool,strictUnits:bool,strictMath:bool,relativeUrls:bool,urlArgs:string,numPrecision:int,import_dirs:array,import_callback:null|callable,indentation:string} */
 	public static $options = [];
 
+	/** @var Less_Environment */
+	private static $envCompat;
+
 	private $input;					// Less input string
 	private $input_len;				// input string length
 	private $pos;					// current index in `input`
@@ -53,8 +56,6 @@ class Less_Parser {
 
 	protected $rules = [];
 
-	private static $imports = [];
-
 	public static $has_extends = false;
 
 	public static $next_id = 0;
@@ -74,8 +75,8 @@ public function __construct( $env = null ) {
 		// which will then be passed around by reference.
 		if ( $env instanceof Less_Environment ) {
 			$this->env = $env;
+			self::$envCompat = $this->env;
 		} else {
-			$this->SetOptions( self::$default_options );
 			$this->Reset( $env );
 		}
 
@@ -93,16 +94,15 @@ public function __construct( $env = null ) {
 	 */
 	public function Reset( $options = null ) {
 		$this->rules = [];
-		self::$imports = [];
 		self::$has_extends = false;
-		self::$imports = [];
 		self::$contentsMap = [];
 
 		$this->env = new Less_Environment();
+		self::$envCompat = $this->env;
 
 		// set new options
+		$this->SetOptions( self::$default_options );
 		if ( is_array( $options ) ) {
-			$this->SetOptions( self::$default_options );
 			$this->SetOptions( $options );
 		}
 
@@ -124,6 +124,10 @@ public function SetOptions( $options ) {
 	 */
 	public function SetOption( $option, $value ) {
 		switch ( $option ) {
+			case 'strictMath':
+				$this->env->strictMath = (bool)$value;
+				self::$options[$option] = $value;
+				return;
 
 			case 'import_dirs':
 				$this->SetImportDirs( $value );
@@ -444,7 +448,7 @@ public function parseFile( $filename, $uri_root = '', $returnRoot = false ) {
 
 		$this->SetFileInfo( $filename, $uri_root );
 
-		self::AddParsedFile( $filename );
+		$this->env->addParsedFile( $filename );
 
 		if ( $returnRoot ) {
 			$rules = $this->GetRules( $filename );
@@ -706,22 +710,19 @@ public function CacheFile( $file_path ) {
 	}
 
 	/**
-	 * @internal For use by Less_Tree_Import
+	 * @deprecated since 4.3.0 Use $parser->getParsedFiles() instead.
+	 * @return string[]
 	 */
-	public static function AddParsedFile( $file ) {
-		self::$imports[] = $file;
-	}
-
 	public static function AllParsedFiles() {
-		return self::$imports;
+		return self::$envCompat->imports;
 	}
 
 	/**
-	 * @internal For use by Less_Tree_Import
-	 * @param string $file
+	 * @since 4.3.0
+	 * @return string[]
 	 */
-	public static function FileParsed( $file ) {
-		return in_array( $file, self::$imports );
+	public function getParsedFiles() {
+		return $this->env->imports;
 	}
 
 	/**

lib/Less/Tree/Call.php

@@ -36,10 +36,10 @@ public function accept( $visitor ) {
 	// like: `saturate(@mycolor)`.
 	// The function should receive the value, not the variable.
 	//
-	public function compile( $env = null ) {
+	public function compile( $env ) {
 		// Turn off math for calc(). https://phabricator.wikimedia.org/T331688
-		$currentMathContext = Less_Environment::$mathOn;
-		Less_Environment::$mathOn = $this->mathOn;
+		$currentMathContext = $env->strictMath;
+		$env->strictMath = !$this->mathOn;
 
 		$args = [];
 		foreach ( $this->args as $a ) {
@@ -57,7 +57,7 @@ public function compile( $env = null ) {
 			}
 		}
 
-		Less_Environment::$mathOn = $currentMathContext;
+		$env->strictMath = $currentMathContext;
 
 		$nameLC = strtolower( $this->name );
 		switch ( $nameLC ) {
@@ -128,8 +128,4 @@ public function genCSS( $output ) {
 		$output->add( ')' );
 	}
 
-	// public function toCSS(){
-	//    return $this->compile()->toCSS();
-	//}
-
 }

lib/Less/Tree/Expression.php

@@ -54,7 +54,7 @@ public function compile( $env ) {
 			if ( !$this->parensInOp ) {
 				Less_Environment::$parensStack--;
 
-			} elseif ( !Less_Environment::isMathOn() && !$doubleParen ) {
+			} elseif ( !$env->isMathOn() && !$doubleParen ) {
 				$returnValue = new Less_Tree_Paren( $returnValue );
 
 			}

lib/Less/Tree/Import.php

@@ -170,7 +170,7 @@ public function compile( $env ) {
 		'@phan-var string $full_path';
 
 		if ( $this->options['inline'] ) {
-			Less_Parser::AddParsedFile( $full_path );
+			$env->addParsedFile( $full_path );
 			$contents = new Less_Tree_Anonymous( file_get_contents( $full_path ), 0, [], true, true );
 
 			if ( $this->features ) {
@@ -260,7 +260,11 @@ public function PathAndUri() {
 	 * @return Less_Tree_Media|array
 	 */
 	public function ParseImport( $full_path, $uri, $env ) {
+		// Most of the env changes during importing temporary, so make a copy.
+		// Except the imports list, which needs to be merged into the main env.
+		// Set it by-ref, to match JavaScript behaviour.
 		$import_env = clone $env;
+		$import_env->imports =& $env->imports;
 		if ( ( isset( $this->options['reference'] ) && $this->options['reference'] ) || isset( $this->currentFileInfo['reference'] ) ) {
 			$import_env->currentFileInfo['reference'] = true;
 		}
@@ -288,7 +292,7 @@ public function ParseImport( $full_path, $uri, $env ) {
 	private function skip( $path, $env ) {
 		$path = Less_Parser::AbsPath( $path, true );
 
-		if ( $path && Less_Parser::FileParsed( $path ) ) {
+		if ( $path && $env->isFileParsed( $path ) ) {
 
 			if ( isset( $this->currentFileInfo['reference'] ) ) {
 				return true;

lib/Less/Tree/Media.php

@@ -45,15 +45,15 @@ public function compile( $env ) {
 		$media = new self( [], [], $this->index, $this->currentFileInfo );
 
 		$strictMathBypass = false;
-		if ( Less_Parser::$options['strictMath'] === false ) {
+		if ( !$env->strictMath ) {
 			$strictMathBypass = true;
-			Less_Parser::$options['strictMath'] = true;
+			$env->strictMath = true;
 		}
 
 		$media->features = $this->features->compile( $env );
 
 		if ( $strictMathBypass ) {
-			Less_Parser::$options['strictMath'] = false;
+			$env->strictMath = false;
 		}
 
 		$env->mediaPath[] = $media;

lib/Less/Tree/Negative.php

@@ -10,10 +10,6 @@ public function __construct( $node ) {
 		$this->value = $node;
 	}
 
-	// function accept($visitor) {
-	//	$this->value = $visitor->visit($this->value);
-	//}
-
 	/**
 	 * @see Less_Tree::genCSS
 	 */
@@ -23,7 +19,7 @@ public function genCSS( $output ) {
 	}
 
 	public function compile( $env ) {
-		if ( Less_Environment::isMathOn() ) {
+		if ( $env->isMathOn() ) {
 			$ret = new Less_Tree_Operation( '*', [ new Less_Tree_Dimension( -1 ), $this->value ] );
 			return $ret->compile( $env );
 		}

lib/Less/Tree/Operation.php

@@ -29,7 +29,7 @@ public function compile( $env ) {
 		// For example, if one argument is a Less_Tree_Call like 'var(--foo)' then we
 		// preserve it as literal for native CSS.
 		// https://phabricator.wikimedia.org/T331688
-		if ( Less_Environment::isMathOn() ) {
+		if ( $env->isMathOn() ) {
 
 			if ( $a instanceof Less_Tree_Dimension && $b instanceof Less_Tree_Color ) {
 				$a = $a->toColor();

lib/Less/Tree/Rule.php

@@ -58,6 +58,7 @@ public function genCSS( $output ) {
 	}
 
 	/**
+	 * @see less-2.5.3.js#Rule.prototype.eval
 	 * @param Less_Environment $env
 	 * @return self
 	 */
@@ -73,9 +74,10 @@ public function compile( $env ) {
 			}
 		}
 
-		$strictMathBypass = Less_Parser::$options['strictMath'];
-		if ( $name === "font" && !Less_Parser::$options['strictMath'] ) {
-			Less_Parser::$options['strictMath'] = true;
+		$strictMathBypass = false;
+		if ( $name === "font" && !$env->strictMath ) {
+			$strictMathBypass = true;
+			$env->strictMath = true;
 		}
 
 		try {
@@ -102,7 +104,9 @@ public function compile( $env ) {
 			throw $e;
 		}
 
-		Less_Parser::$options['strictMath'] = $strictMathBypass;
+		if ( $strictMathBypass ) {
+			$env->strictMath = false;
+		}
 
 		return $return;
 	}

lib/Less/Visitor/import.php

@@ -65,7 +65,7 @@ function visitImport($importNode, &$visitDeeper ){
 					//todo needs to reference css file not import
 					//$contents = new Less_Tree_Anonymous($importNode->root, 0, array('filename'=>$importNode->importedFilename), true );
 
-					Less_Parser::AddParsedFile($full_path);
+					$this->env->addParsedFile($full_path);
 					$contents = new Less_Tree_Anonymous( file_get_contents($full_path), 0, array(), true );
 
 					if( $importNode->features ){

test/phpunit/ParserTest.php

@@ -39,22 +39,28 @@ public function testGetVariables() {
 		);
 	}
 
-	public function testAllParsedFiles() {
+	public function testGetParsedFiles() {
 		$parser = new Less_Parser();
 		$baseDir = Less_Parser::WinPath( realpath( $this->fixtures_dir . '/less.php/less' ) );
 		$parser->parseFile( $baseDir . '/imports.less' );
 		$parser->getCss();
 
-		$files = $parser->AllParsedFiles();
-		$files = array_map( fn ( $file ) => str_replace( $baseDir, '', $file ), $files );
+		$files = $parser->getParsedFiles();
 
+		$normalFiles = array_map( fn ( $file ) => str_replace( $baseDir, '', $file ), $files );
 		$this->assertEqualsCanonicalizing(
 			[
 				'/imports.less',
 				'/imports/b.less',
 				'/imports/a.less'
 			],
-			$files
+			$normalFiles
+		);
+
+		$this->assertEquals(
+			$files,
+			Less_Parser::AllParsedFiles(),
+			'AllParsedFiles compatibility'
 		);
 	}