Skip to content

Commit 838d794

Browse files
johnstevensonjohnstevenson
committed
Updates for phpstan strict rules
1 parent b8672dc commit 838d794

19 files changed

+336
-177
lines changed

‎.github/workflows/continuous-integration.yml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ jobs:
4747
- name: Remove PHPStan if unsupported
4848
# Do this first to diffentiate cache keys
4949
if: ${{ matrix.php-version < '7.1' }}
50-
run: composer remove phpstan/phpstan --dev --no-update
50+
run: composer remove phpstan/phpstan phpstan/phpstan-strict-rules --dev --no-update
5151

5252
- name: Get composer cache directory
5353
id: composercache

‎composer.json‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,8 @@
2424
},
2525
"require-dev": {
2626
"symfony/phpunit-bridge": "^4.2 || ^5.0 || ^6.0",
27-
"phpstan/phpstan": "^1.0"
27+
"phpstan/phpstan": "^1.0",
28+
"phpstan/phpstan-strict-rules": "^1.1"
2829
},
2930
"autoload": {
3031
"psr-4": {

‎phpstan.neon.dist‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,6 @@
1+
includes:
2+
- vendor/phpstan/phpstan-strict-rules/rules.neon
3+
14
parameters:
25
level: 9
36
bootstrapFiles:
@@ -7,8 +10,12 @@ parameters:
710
- tests
811
excludePaths:
912
- tests/Helpers/LegacyLogger.php
10-
1113
ignoreErrors:
12-
# Ignore some irrelevant errors in test files
13-
- '~Method Composer\\XdebugHandler\\Tests\\[^:]+::test[^(]+\(\) has no return type specified.~'
14-
- '~Method Composer\\XdebugHandler\\Tests\\[^:]+::\w+?Provider\(\) has no return type specified.~'
14+
# strict-rules errors
15+
- message: "#^Call to function is_string\\(\\) with string will always evaluate to true.#"
16+
count: 1
17+
path: src/XdebugHandler.php
18+
19+
- message: "#^Call to function method_exists\\(\\) with .* and 'expectException' will always evaluate to true.#"
20+
count: 1
21+
path: tests/ClassTest.php

‎src/PhpConfig.php‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,8 @@ public function useOriginal()
3636
*/
3737
public function useStandard()
3838
{
39-
if ($data = $this->getDataAndReset()) {
39+
$data = $this->getDataAndReset();
40+
if ($data !== null) {
4041
return array('-n', '-c', $data['tmpIni']);
4142
}
4243

@@ -50,7 +51,8 @@ public function useStandard()
5051
*/
5152
public function usePersistent()
5253
{
53-
if ($data = $this->getDataAndReset()) {
54+
$data = $this->getDataAndReset();
55+
if ($data !== null) {
5456
$this->updateEnv('PHPRC', $data['tmpIni']);
5557
$this->updateEnv('PHP_INI_SCAN_DIR', '');
5658
}
@@ -66,7 +68,8 @@ public function usePersistent()
6668
*/
6769
private function getDataAndReset()
6870
{
69-
if ($data = XdebugHandler::getRestartSettings()) {
71+
$data = XdebugHandler::getRestartSettings();
72+
if ($data !== null) {
7073
$this->updateEnv('PHPRC', $data['phprc']);
7174
$this->updateEnv('PHP_INI_SCAN_DIR', $data['scanDir']);
7275
}

‎src/Process.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,8 +73,9 @@ public static function escape($arg, $meta = true, $module = false)
7373
public static function escapeShellCommand(array $args)
7474
{
7575
$command = '';
76+
$module = array_shift($args);
7677

77-
if ($module = array_shift($args)) {
78+
if ($module !== null) {
7879
$command = self::escape($module, true, true);
7980

8081
foreach ($args as $arg) {

‎src/Status.php‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ public function __construct($envAllowXdebug, $debug)
5757
{
5858
$start = getenv(self::ENV_RESTART);
5959
Process::setEnv(self::ENV_RESTART);
60-
$this->time = $start ? round((microtime(true) - $start) * 1000) : 0;
60+
$this->time = is_numeric($start) ? round((microtime(true) - $start) * 1000) : 0;
6161

6262
$this->envAllowXdebug = $envAllowXdebug;
6363
$this->debug = $debug && defined('STDERR');
@@ -85,14 +85,14 @@ public function setLogger(LoggerInterface $logger)
8585
*/
8686
public function report($op, $data)
8787
{
88-
if ($this->logger || $this->debug) {
88+
if ($this->logger !== null || $this->debug) {
8989
$callable = array($this, 'report'.$op);
9090

9191
if (!is_callable($callable)) {
9292
throw new \InvalidArgumentException('Unknown op handler: '.$op);
9393
}
9494

95-
$params = $data ?: array();
95+
$params = $data !== null ? $data : array();
9696
call_user_func_array($callable, array($params));
9797
}
9898
}
@@ -107,8 +107,8 @@ public function report($op, $data)
107107
*/
108108
private function output($text, $level = null)
109109
{
110-
if ($this->logger) {
111-
$this->logger->log($level ?: LogLevel::DEBUG, $text);
110+
if ($this->logger !== null) {
111+
$this->logger->log($level !== null ? $level: LogLevel::DEBUG, $text);
112112
}
113113

114114
if ($this->debug) {
@@ -125,8 +125,8 @@ private function reportCheck($loaded)
125125
{
126126
list($version, $mode) = explode('|', $loaded);
127127

128-
if ($version) {
129-
$this->loaded = '('.$version.')'.($mode ? ' mode='.$mode : '');
128+
if ($version !== '') {
129+
$this->loaded = '('.$version.')'.($mode !== '' ? ' mode='.$mode : '');
130130
}
131131
$this->modeOff = $mode === 'off';
132132
$this->output('Checking '.$this->envAllowXdebug);
@@ -159,9 +159,9 @@ private function reportNoRestart()
159159
{
160160
$this->output($this->getLoadedMessage());
161161

162-
if ($this->loaded) {
162+
if ($this->loaded !== null) {
163163
$text = sprintf('No restart (%s)', $this->getEnvAllow());
164-
if (!getenv($this->envAllowXdebug)) {
164+
if (!((bool) getenv($this->envAllowXdebug))) {
165165
$text .= ' Allowed by '.($this->modeOff ? 'mode' : 'application');
166166
}
167167
$this->output($text);
@@ -184,7 +184,7 @@ private function reportRestarted()
184184
{
185185
$loaded = $this->getLoadedMessage();
186186
$text = sprintf('Restarted (%d ms). %s', $this->time, $loaded);
187-
$level = $this->loaded ? LogLevel::WARNING : null;
187+
$level = $this->loaded !== null ? LogLevel::WARNING : null;
188188
$this->output($text, $level);
189189
}
190190

@@ -218,7 +218,7 @@ private function getEnvAllow()
218218
*/
219219
private function getLoadedMessage()
220220
{
221-
$loaded = $this->loaded ? sprintf('loaded %s', $this->loaded) : 'not loaded';
221+
$loaded = $this->loaded !== null ? sprintf('loaded %s', $this->loaded) : 'not loaded';
222222
return 'The Xdebug extension is '.$loaded;
223223
}
224224
}

‎src/XdebugHandler.php‎

Lines changed: 71 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ class XdebugHandler
3030
/** @var string|null */
3131
protected $tmpIni;
3232

33-
/** @var bool|null */
33+
/** @var bool */
3434
private static $inRestart;
3535

3636
/** @var string */
@@ -81,7 +81,7 @@ class XdebugHandler
8181
*/
8282
public function __construct($envPrefix)
8383
{
84-
if (!is_string($envPrefix) || empty($envPrefix)) {
84+
if (!is_string($envPrefix) || $envPrefix === '') {
8585
throw new \RuntimeException('Invalid constructor parameter');
8686
}
8787

@@ -90,20 +90,13 @@ public function __construct($envPrefix)
9090
$this->envOriginalInis = self::$name.self::SUFFIX_INIS;
9191

9292
if (extension_loaded('xdebug')) {
93-
$this->loaded = phpversion('xdebug') ?: 'unknown';
94-
95-
if (version_compare($this->loaded, '3.1', '>=')) {
96-
$modes = xdebug_info('mode');
97-
$this->mode = empty($modes) ? 'off' : implode(',', $modes);
98-
} elseif (false !== ($mode = ini_get('xdebug.mode'))) {
99-
$this->mode = getenv('XDEBUG_MODE') ?: ($mode ?: 'off');
100-
if (Preg::isMatch('/^,+$/', str_replace(' ', '', $this->mode))) {
101-
$this->mode = 'off';
102-
}
103-
}
93+
$version = phpversion('xdebug');
94+
$this->loaded = $version !== false ? $version : 'unknown';
95+
$this->mode = $this->getXdebugMode($this->loaded);
10496
}
10597

106-
self::$xdebugActive = $this->loaded && $this->mode !== 'off';
98+
self::$xdebugActive = $this->loaded !== null && $this->mode !== 'off';
99+
self::$inRestart = false;
107100

108101
if ($this->cli = PHP_SAPI === 'cli') {
109102
$this->debug = (string) getenv(self::DEBUG);
@@ -163,7 +156,7 @@ public function check()
163156
$this->notify(Status::CHECK, $this->loaded.'|'.$this->mode);
164157
$envArgs = explode('|', (string) getenv($this->envAllowXdebug));
165158

166-
if (empty($envArgs[0]) && $this->requiresRestart(self::$xdebugActive)) {
159+
if (!((bool) $envArgs[0]) && $this->requiresRestart(self::$xdebugActive)) {
167160
// Restart required
168161
$this->notify(Status::RESTART);
169162

@@ -181,7 +174,7 @@ public function check()
181174
Process::setEnv($this->envAllowXdebug);
182175
self::$inRestart = true;
183176

184-
if (!$this->loaded) {
177+
if ($this->loaded === null) {
185178
// Skipped version is only set if Xdebug is not loaded
186179
self::$skipped = $envArgs[1];
187180
}
@@ -194,8 +187,9 @@ public function check()
194187
}
195188

196189
$this->notify(Status::NORESTART);
190+
$settings = self::getRestartSettings();
197191

198-
if ($settings = self::getRestartSettings()) {
192+
if ($settings !== null) {
199193
// Called with existing settings, so sync our settings
200194
$this->syncSettings($settings);
201195
}
@@ -211,7 +205,7 @@ public function check()
211205
*/
212206
public static function getAllIniFiles()
213207
{
214-
if (!empty(self::$name)) {
208+
if (self::$name !== null) {
215209
$env = getenv(self::$name.self::SUFFIX_INIS);
216210

217211
if (false !== $env) {
@@ -220,8 +214,9 @@ public static function getAllIniFiles()
220214
}
221215

222216
$paths = array((string) php_ini_loaded_file());
217+
$scanned = php_ini_scanned_files();
223218

224-
if ($scanned = php_ini_scanned_files()) {
219+
if ($scanned !== false) {
225220
$paths = array_merge($paths, array_map('trim', explode(',', $scanned)));
226221
}
227222

@@ -365,7 +360,7 @@ private function doRestart(array $command)
365360
*/
366361
private function prepareRestart()
367362
{
368-
$error = '';
363+
$error = null;
369364
$iniFiles = self::getAllIniFiles();
370365
$scannedInis = count($iniFiles) > 1;
371366
$tmpDir = sys_get_temp_dir();
@@ -381,35 +376,37 @@ private function prepareRestart()
381376
} elseif (!$this->checkMainScript()) {
382377
$error = 'Unable to access main script: '.$this->script;
383378
} elseif (!$this->writeTmpIni($iniFiles, $tmpDir, $error)) {
384-
$error = $error ?: 'Unable to create temp ini file at: '.$tmpDir;
379+
$error = $error !== null ? $error : 'Unable to create temp ini file at: '.$tmpDir;
385380
} elseif (!$this->setEnvironment($scannedInis, $iniFiles)) {
386381
$error = 'Unable to set environment variables';
387382
}
388383

389-
if ($error) {
384+
if ($error !== null) {
390385
$this->notify(Status::ERROR, $error);
391386
}
392387

393-
return empty($error);
388+
return $error === null;
394389
}
395390

396391
/**
397392
* Returns true if the tmp ini file was written
398393
*
399394
* @param string[] $iniFiles All ini files used in the current process
400395
* @param string $tmpDir The system temporary directory
401-
* @param string $error Set by method if ini file cannot be read
396+
* @param null|string $error Set by method if ini file cannot be read
402397
*
403398
* @return bool
404399
*/
405400
private function writeTmpIni(array $iniFiles, $tmpDir, &$error)
406401
{
407-
if (!$this->tmpIni = (string) @tempnam($tmpDir, '')) {
402+
if (($tmpfile = @tempnam($tmpDir, '')) === false) {
408403
return false;
409404
}
410405

406+
$this->tmpIni = $tmpfile;
407+
411408
// $iniFiles has at least one item and it may be empty
412-
if (empty($iniFiles[0])) {
409+
if ($iniFiles[0] === '') {
413410
array_shift($iniFiles);
414411
}
415412

@@ -564,8 +561,9 @@ private function checkMainScript()
564561

565562
// Use a backtrace to resolve Phar and chdir issues.
566563
$trace = debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS);
564+
$main = end($trace);
567565

568-
if (($main = end($trace)) && isset($main['file'])) {
566+
if ($main !== false && isset($main['file'])) {
569567
return file_exists($this->script = $main['file']);
570568
}
571569

@@ -622,10 +620,12 @@ private function syncSettings(array $settings)
622620
*/
623621
private function checkScanDirConfig()
624622
{
625-
return !(getenv('PHP_INI_SCAN_DIR')
626-
&& !PHP_CONFIG_FILE_SCAN_DIR
627-
&& (PHP_VERSION_ID < 70113
628-
|| PHP_VERSION_ID === 70200));
623+
if (PHP_VERSION_ID >= 70113 && PHP_VERSION_ID !== 70200) {
624+
return true;
625+
}
626+
627+
return ((string) getenv('PHP_INI_SCAN_DIR') === '')
628+
|| PHP_CONFIG_FILE_SCAN_DIR !== '';
629629
}
630630

631631
/**
@@ -641,7 +641,7 @@ private function checkConfiguration(&$info)
641641
return false;
642642
}
643643

644-
if (extension_loaded('uopz') && !ini_get('uopz.disable')) {
644+
if (extension_loaded('uopz') && !((bool) ini_get('uopz.disable'))) {
645645
// uopz works at opcode level and disables exit calls
646646
if (function_exists('uopz_allow_exit')) {
647647
@uopz_allow_exit(true);
@@ -653,7 +653,9 @@ private function checkConfiguration(&$info)
653653

654654
// Check UNC paths when using cmd.exe
655655
if (defined('PHP_WINDOWS_VERSION_BUILD') && PHP_VERSION_ID < 70400) {
656-
if (!$workingDir = getcwd()) {
656+
$workingDir = getcwd();
657+
658+
if ($workingDir === false) {
657659
$info = 'unable to determine working directory';
658660
return false;
659661
}
@@ -696,4 +698,40 @@ private function tryEnableSignals()
696698
sapi_windows_set_ctrl_handler(function ($evt) {});
697699
}
698700
}
701+
702+
/**
703+
* Returns the Xdebug mode if available
704+
*
705+
* @param string $version
706+
*
707+
* @return string|null
708+
*/
709+
private function getXdebugMode($version)
710+
{
711+
if (version_compare($version, '3.1', '>=')) {
712+
$modes = xdebug_info('mode');
713+
return count($modes) === 0 ? 'off' : implode(',', $modes);
714+
}
715+
716+
// See if xdebug.mode is supported in this version
717+
$iniMode = ini_get('xdebug.mode');
718+
if ($iniMode === false) {
719+
return null;
720+
}
721+
722+
// Environment value wins but cannot be empty
723+
$envMode = (string) getenv('XDEBUG_MODE');
724+
if ($envMode !== '') {
725+
$mode = $envMode;
726+
} else {
727+
$mode = $iniMode !== '' ? $iniMode : 'off';
728+
}
729+
730+
// An empty comma-separated list is treated as mode 'off'
731+
if (Preg::isMatch('/^,+$/', str_replace(' ', '', $mode))) {
732+
$mode = 'off';
733+
}
734+
735+
return $mode;
736+
}
699737
}

0 commit comments

Comments
 (0)