Skip to content

Config: only determine screen width if/when needed (performance improvement) #61

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Dec 4, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ The file documents changes to the PHP_CodeSniffer project.
- Thanks to Dan Wallis (@fredden) for the patch
- Squiz.PHP.InnerFunctions sniff no longer reports on OO methods for OO structures declared within a function or closure
- Thanks to @Daimona for the patch
- Runtime performance improvement for PHPCS CLI users. The improvement should be most noticeable for users on Windows.

### Removed
- Removed support for installing via PEAR
Expand Down
27 changes: 20 additions & 7 deletions src/Config.php
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,22 @@ public function __get($name)
throw new RuntimeException("ERROR: unable to get value of property \"$name\"");
}

// Figure out what the terminal width needs to be for "auto".
if ($name === 'reportWidth' && $this->settings[$name] === 'auto') {
if (function_exists('shell_exec') === true) {
$dimensions = shell_exec('stty size 2>&1');
if (is_string($dimensions) === true && preg_match('|\d+ (\d+)|', $dimensions, $matches) === 1) {
$this->settings[$name] = (int) $matches[1];
}
}

if ($this->settings[$name] === 'auto') {
// If shell_exec wasn't available or didn't yield a usable value, set to the default.
// This will prevent subsequent retrievals of the reportWidth from making another call to stty.
$this->settings[$name] = self::DEFAULT_REPORT_WIDTH;
}
}

return $this->settings[$name];

}//end __get()
Expand All @@ -229,13 +245,9 @@ public function __set($name, $value)

switch ($name) {
case 'reportWidth' :
// Support auto terminal width.
if ($value === 'auto' && function_exists('shell_exec') === true) {
$dimensions = shell_exec('stty size 2>&1');
if (is_string($dimensions) === true && preg_match('|\d+ (\d+)|', $dimensions, $matches) === 1) {
$value = (int) $matches[1];
break;
}
if (is_string($value) === true && $value === 'auto') {
// Nothing to do. Leave at 'auto'.
break;
}

if (is_int($value) === true) {
Expand All @@ -246,6 +258,7 @@ public function __set($name, $value)
$value = self::DEFAULT_REPORT_WIDTH;
}
break;

case 'standards' :
$cleaned = [];

Expand Down
42 changes: 40 additions & 2 deletions tests/Core/AbstractMethodUnitTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
use PHP_CodeSniffer\Ruleset;
use PHP_CodeSniffer\Files\DummyFile;
use PHPUnit\Framework\TestCase;
use ReflectionProperty;

abstract class AbstractMethodUnitTest extends TestCase
{
Expand Down Expand Up @@ -47,9 +48,22 @@ abstract class AbstractMethodUnitTest extends TestCase
*/
public static function initializeFile()
{
$config = new Config();
$config->standards = ['PSR1'];
/*
* Set the static properties in the Config class to specific values for performance
* and to clear out values from other tests.
*/

self::setStaticConfigProperty('executablePaths', []);

// Set to a usable value to circumvent Config trying to find a phpcs.xml config file.
self::setStaticConfigProperty('overriddenDefaults', ['standards' => ['PSR1']]);

// Set to values which prevent the test-runner user's `CodeSniffer.conf` file
// from being read and influencing the tests. Also prevent an `exec()` call to stty.
self::setStaticConfigProperty('configData', ['report_width' => 80]);
self::setStaticConfigProperty('configDataFile', '');

$config = new Config();
$ruleset = new Ruleset($config);

// Default to a file with the same name as the test class. Extension is property based.
Expand Down Expand Up @@ -78,9 +92,33 @@ public static function resetFile()
{
self::$phpcsFile = null;

// Reset the static properties in the Config class to their defaults to prevent tests influencing each other.
self::setStaticConfigProperty('overriddenDefaults', []);
self::setStaticConfigProperty('executablePaths', []);
self::setStaticConfigProperty('configData', null);
self::setStaticConfigProperty('configDataFile', null);

}//end resetFile()


/**
* Helper function to set the value of a private static property on the Config class.
*
* @param string $name The name of the property to set.
* @param mixed $value The value to set the property to.
*
* @return void
*/
public static function setStaticConfigProperty($name, $value)
{
$property = new ReflectionProperty('PHP_CodeSniffer\Config', $name);
$property->setAccessible(true);
$property->setValue(null, $value);
$property->setAccessible(false);

}//end setStaticConfigProperty()


/**
* Get the token pointer for a target token based on a specific comment found on the line before.
*
Expand Down