Skip to content

Commit 7dcdab2

Browse files
jrfnljrfnl
committed
Change exit codes for scan runs
This commit implements the change in the exit codes for scan runs, both `phpcs` as well as `phpcbf`, as previously outlined and extensively discussed in 184. It also introduces a new `ignore_non_auto_fixable_on_exit` config flag to influence the exit code. To allow for determining whether there are fixable and/or non-fixable issues left at the end of a run, while also supporting the previously already supported `ignore_errors_on_exit` and `ignore_warnings_on_exit` config flags, this required more extensive changes than originally anticipated. Originally, the `File` class and the `Reporter` class tracked (single file/all files) totals for: * Errors found * Warnings found * Fixable violations found (errors and warnings combined) * Violations fixed (errors and warnings combined) Additionally, the total for "fixed" could be higher than the "fixable" count as it totals the fixes applied in all loops, including fixes for issues _created by fixers_ and not in the original scan, which made the "fixed" total unusable for the new exit code determination. To allow for correctly determining the new exit codes, the "fixable" total had to be split up into "fixable errors" and "fixable warnings", both in the `File` class, as well as in the `Reporter` class. Secondly, the "fixed" total also had to be split up, as well as being based on the _effective_ number of fixes made instead of the _actual_ number of fixes made. These changes also affect the way scan results are cached, as the cache now also needs to store the split up numbers (for "fixable") and how parallel processing accumulates the totals of the child processes. And as the logic for the exit code determination for the new exit codes is largely the same for both `phpcs` as well as `phpcbf`, a new `ExitCode::calculate()` method has been introduced to transparently handle this for both. Notable API changes: * The signature of the `DummyFile::setErrorCounts()` method has changed: ```diff -public function setErrorCounts($errorCount, $warningCount, $fixableCount, $fixedCount) +public function setErrorCounts($errorCount, $warningCount, $fixableErrorCount, $fixableWarningCount, $fixedErrorCount, $fixedWarningCount) ``` * The `Reporter::$totalFixable` and `Reporter::$totalFixed` properties are now deprecated. They will still be supported as readonly properties for the 4.x branch (via magic methods), but support will be removed in PHPCS 5.0. * The return value of the `private` `Runner::run()` method has changed from `int` to ` void` as the return value was no longer used. Includes mentioning the new `ignore_non_auto_fixable_on_exit` config flag in the CLI help text. Includes extensive integration tests for the exit code calculations and the changes made in support of the new exit codes. Includes git-ignoring temporary files which will be created during the test run in case the test run would bork out before the tear down methods are run. Includes tests for the new magic methods in the `Reporter` class to safeguard that the deprecated properties are still accessible, but not writeable. Includes improving the documentation for `Fixer::$numFixes`/`Fixer::getFixCount()` to mention that these numbers are _actual_ number fixes, not _effective_ number of fixes. Fixes 184 Fixes squizlabs/PHP_CodeSniffer 2898
1 parent 7ba0a48 commit 7dcdab2

24 files changed

+1408
-126
lines changed

‎.gitignore

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,3 +11,5 @@ composer.lock
1111
phpstan.neon
1212
/node_modules/
1313
/tests/Standards/sniffStnd.xml
14+
/tests/Core/Util/ExitCode/Fixtures/ExitCodeTest/*.fixed
15+
/tests/Core/Util/ExitCode/Fixtures/ExitCodeTest/phpcs.cache

‎src/Files/DummyFile.php

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -62,19 +62,23 @@ public function __construct($content, Ruleset $ruleset, Config $config)
6262
/**
6363
* Set the error, warning, and fixable counts for the file.
6464
*
65-
* @param int $errorCount The number of errors found.
66-
* @param int $warningCount The number of warnings found.
67-
* @param int $fixableCount The number of fixable errors found.
68-
* @param int $fixedCount The number of errors that were fixed.
65+
* @param int $errorCount The number of errors found.
66+
* @param int $warningCount The number of warnings found.
67+
* @param int $fixableErrorCount The number of fixable errors found.
68+
* @param int $fixableWarningCount The number of fixable warning found.
69+
* @param int $fixedErrorCount The number of errors that were fixed.
70+
* @param int $fixedWarningCount The number of warning that were fixed.
6971
*
7072
* @return void
7173
*/
72-
public function setErrorCounts($errorCount, $warningCount, $fixableCount, $fixedCount)
74+
public function setErrorCounts($errorCount, $warningCount, $fixableErrorCount, $fixableWarningCount, $fixedErrorCount, $fixedWarningCount)
7375
{
74-
$this->errorCount = $errorCount;
75-
$this->warningCount = $warningCount;
76-
$this->fixableCount = $fixableCount;
77-
$this->fixedCount = $fixedCount;
76+
$this->errorCount = $errorCount;
77+
$this->warningCount = $warningCount;
78+
$this->fixableErrorCount = $fixableErrorCount;
79+
$this->fixableWarningCount = $fixableWarningCount;
80+
$this->fixedErrorCount = $fixedErrorCount;
81+
$this->fixedWarningCount = $fixedWarningCount;
7882

7983
}//end setErrorCounts()
8084

‎src/Files/File.php

Lines changed: 126 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -153,19 +153,67 @@ class File
153153
protected $warningCount = 0;
154154

155155
/**
156-
* The total number of errors and warnings that can be fixed.
156+
* The original total number of errors that can be fixed (first run on a file).
157+
*
158+
* {@internal This should be regarded as an immutable property.}
157159
*
158160
* @var integer
159161
*/
160-
protected $fixableCount = 0;
162+
private $fixableErrorCountFirstRun;
161163

162164
/**
163-
* The total number of errors and warnings that were fixed.
165+
* The original total number of warnings that can be fixed (first run on a file).
166+
*
167+
* {@internal This should be regarded as an immutable property.}
168+
*
169+
* @var integer
170+
*/
171+
private $fixableWarningCountFirstRun;
172+
173+
/**
174+
* The current total number of errors that can be fixed.
175+
*
176+
* @var integer
177+
*/
178+
protected $fixableErrorCount = 0;
179+
180+
/**
181+
* The current total number of warnings that can be fixed.
182+
*
183+
* @var integer
184+
*/
185+
protected $fixableWarningCount = 0;
186+
187+
/**
188+
* The actual number of errors and warnings that were fixed.
189+
*
190+
* I.e. how many fixes were applied. This may be higher than the originally found
191+
* issues if the fixer from one sniff causes other sniffs to come into play in follow-up loops.
192+
* Example: if a brace is moved to a new line, the `ScopeIndent` sniff may need to ensure
193+
* the brace is indented correctly in the next loop.
164194
*
165195
* @var integer
166196
*/
167197
protected $fixedCount = 0;
168198

199+
/**
200+
* The effective number of errors that were fixed.
201+
*
202+
* I.e. how many of the originally found errors were fixed.
203+
*
204+
* @var integer
205+
*/
206+
protected $fixedErrorCount = 0;
207+
208+
/**
209+
* The effective number of warnings that were fixed.
210+
*
211+
* I.e. how many of the originally found warnings were fixed.
212+
*
213+
* @var integer
214+
*/
215+
protected $fixedWarningCount = 0;
216+
169217
/**
170218
* TRUE if errors are being replayed from the cache.
171219
*
@@ -309,11 +357,12 @@ public function process()
309357
return;
310358
}
311359

312-
$this->errors = [];
313-
$this->warnings = [];
314-
$this->errorCount = 0;
315-
$this->warningCount = 0;
316-
$this->fixableCount = 0;
360+
$this->errors = [];
361+
$this->warnings = [];
362+
$this->errorCount = 0;
363+
$this->warningCount = 0;
364+
$this->fixableErrorCount = 0;
365+
$this->fixableWarningCount = 0;
317366

318367
$this->parse();
319368

@@ -351,11 +400,12 @@ public function process()
351400
|| substr($commentTextLower, 0, 17) === '@phpcs:ignorefile'
352401
) {
353402
// Ignoring the whole file, just a little late.
354-
$this->errors = [];
355-
$this->warnings = [];
356-
$this->errorCount = 0;
357-
$this->warningCount = 0;
358-
$this->fixableCount = 0;
403+
$this->errors = [];
404+
$this->warnings = [];
405+
$this->errorCount = 0;
406+
$this->warningCount = 0;
407+
$this->fixableErrorCount = 0;
408+
$this->fixableWarningCount = 0;
359409
return;
360410
} else if (substr($commentTextLower, 0, 9) === 'phpcs:set'
361411
|| substr($commentTextLower, 0, 10) === '@phpcs:set'
@@ -505,7 +555,14 @@ public function process()
505555
StatusWriter::write('*** END SNIFF PROCESSING REPORT ***', 1);
506556
}
507557

508-
$this->fixedCount += $this->fixer->getFixCount();
558+
if (isset($this->fixableErrorCountFirstRun, $this->fixableWarningCountFirstRun) === false) {
559+
$this->fixableErrorCountFirstRun = $this->fixableErrorCount;
560+
$this->fixableWarningCountFirstRun = $this->fixableWarningCount;
561+
}
562+
563+
$this->fixedCount += $this->fixer->getFixCount();
564+
$this->fixedErrorCount = ($this->fixableErrorCountFirstRun - $this->fixableErrorCount);
565+
$this->fixedWarningCount = ($this->fixableWarningCountFirstRun - $this->fixableWarningCount);
509566

510567
}//end process()
511568

@@ -1004,7 +1061,11 @@ protected function addMessage($error, $message, $line, $column, $code, $data, $s
10041061

10051062
$messageCount++;
10061063
if ($fixable === true) {
1007-
$this->fixableCount++;
1064+
if ($error === true) {
1065+
$this->fixableErrorCount++;
1066+
} else {
1067+
$this->fixableWarningCount++;
1068+
}
10081069
}
10091070

10101071
if ($this->configCache['recordErrors'] === false
@@ -1114,13 +1175,37 @@ public function getWarningCount()
11141175
*/
11151176
public function getFixableCount()
11161177
{
1117-
return $this->fixableCount;
1178+
return ($this->fixableErrorCount + $this->fixableWarningCount);
11181179

11191180
}//end getFixableCount()
11201181

11211182

11221183
/**
1123-
* Returns the number of fixed errors/warnings.
1184+
* Returns the number of fixable errors raised.
1185+
*
1186+
* @return int
1187+
*/
1188+
public function getFixableErrorCount()
1189+
{
1190+
return $this->fixableErrorCount;
1191+
1192+
}//end getFixableErrorCount()
1193+
1194+
1195+
/**
1196+
* Returns the number of fixable warnings raised.
1197+
*
1198+
* @return int
1199+
*/
1200+
public function getFixableWarningCount()
1201+
{
1202+
return $this->fixableWarningCount;
1203+
1204+
}//end getFixableWarningCount()
1205+
1206+
1207+
/**
1208+
* Returns the actual number of fixed errors/warnings.
11241209
*
11251210
* @return int
11261211
*/
@@ -1131,6 +1216,30 @@ public function getFixedCount()
11311216
}//end getFixedCount()
11321217

11331218

1219+
/**
1220+
* Returns the effective number of fixed errors.
1221+
*
1222+
* @return int
1223+
*/
1224+
public function getFixedErrorCount()
1225+
{
1226+
return $this->fixedErrorCount;
1227+
1228+
}//end getFixedErrorCount()
1229+
1230+
1231+
/**
1232+
* Returns the effective number of fixed warnings.
1233+
*
1234+
* @return int
1235+
*/
1236+
public function getFixedWarningCount()
1237+
{
1238+
return $this->fixedWarningCount;
1239+
1240+
}//end getFixedWarningCount()
1241+
1242+
11341243
/**
11351244
* Returns the list of ignored lines.
11361245
*

‎src/Files/LocalFile.php

Lines changed: 19 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -106,9 +106,10 @@ public function process()
106106
$this->replayErrors($cache['errors'], $cache['warnings']);
107107
$this->configCache['cache'] = true;
108108
} else {
109-
$this->errorCount = $cache['errorCount'];
110-
$this->warningCount = $cache['warningCount'];
111-
$this->fixableCount = $cache['fixableCount'];
109+
$this->errorCount = $cache['errorCount'];
110+
$this->warningCount = $cache['warningCount'];
111+
$this->fixableErrorCount = $cache['fixableErrorCount'];
112+
$this->fixableWarningCount = $cache['fixableWarningCount'];
112113
}
113114

114115
if (PHP_CODESNIFFER_VERBOSITY > 0
@@ -129,14 +130,15 @@ public function process()
129130
parent::process();
130131

131132
$cache = [
132-
'hash' => $hash,
133-
'errors' => $this->errors,
134-
'warnings' => $this->warnings,
135-
'metrics' => $this->metrics,
136-
'errorCount' => $this->errorCount,
137-
'warningCount' => $this->warningCount,
138-
'fixableCount' => $this->fixableCount,
139-
'numTokens' => $this->numTokens,
133+
'hash' => $hash,
134+
'errors' => $this->errors,
135+
'warnings' => $this->warnings,
136+
'metrics' => $this->metrics,
137+
'errorCount' => $this->errorCount,
138+
'warningCount' => $this->warningCount,
139+
'fixableErrorCount' => $this->fixableErrorCount,
140+
'fixableWarningCount' => $this->fixableWarningCount,
141+
'numTokens' => $this->numTokens,
140142
];
141143

142144
Cache::set($this->path, $cache);
@@ -166,11 +168,12 @@ public function process()
166168
*/
167169
private function replayErrors($errors, $warnings)
168170
{
169-
$this->errors = [];
170-
$this->warnings = [];
171-
$this->errorCount = 0;
172-
$this->warningCount = 0;
173-
$this->fixableCount = 0;
171+
$this->errors = [];
172+
$this->warnings = [];
173+
$this->errorCount = 0;
174+
$this->warningCount = 0;
175+
$this->fixableErrorCount = 0;
176+
$this->fixableWarningCount = 0;
174177

175178
$this->replayingErrors = true;
176179

‎src/Fixer.php

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,12 @@ class Fixer
102102
private $inConflict = false;
103103

104104
/**
105-
* The number of fixes that have been performed.
105+
* The actual number of fixes that have been performed.
106+
*
107+
* I.e. how many fixes were applied. This may be higher than the originally found
108+
* issues if the fixer from one sniff causes other sniffs to come into play in follow-up loops.
109+
* Example: if a brace is moved to a new line, the `ScopeIndent` sniff may need to ensure
110+
* the brace is indented correctly in the next loop.
106111
*
107112
* @var integer
108113
*/
@@ -336,7 +341,7 @@ public function generateDiff($filePath=null, $colors=true)
336341

337342

338343
/**
339-
* Get a count of fixes that have been performed on the file.
344+
* Get a count of the actual number of fixes that have been performed on the file.
340345
*
341346
* This value is reset every time a new file is started, or an existing
342347
* file is restarted.

0 commit comments

Comments
 (0)