Skip to content

Commit

Permalink
Caching now generates a more accurate file hash and error filtering i…
Browse files Browse the repository at this point in the history 
…s done at the end of a file run so the cache can be used even with filter (ref #530)

The cache key is now generated by looking for loaded external and internal sniff files, then looking for all important core files, and hashing the result. Use -vv to see what files are being used for the hash, and other cache key values.

The cache key no longer includes severity or sniffs (removed previously) as files will record all errors and warnings during a cache run no matter what the filter settings are. Local files then replay the errors and warnings found with the filters on, removing the ones that should not be shown. Subsequent runs can then use the cache and reply the errors manually instead of tokenizing and checking the file. The result is that a filtered run will now cache everything needed for future unfiltered runs. It makes the first run a bit slower, but the cache much more useful.

To help with all this, error messages must now have codes. You can no longer leave them blank, which makes them impossible to override or filter in rulesets anyway.
  • Loading branch information
@gsherwood
gsherwood committed May 18, 2015
1 parent 24c7a7f commit c587b27
Show file tree
Hide file tree
Showing 9 changed files with 230 additions and 67 deletions.
1 change: 1 addition & 0 deletions package.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ http://pear.php.net/dtd/package-2.0.xsd">
- Automatic discovery of executable paths (request #571)
-- Thanks to Sergey Morozov for the patch
- All sniff errors are now exceptions and caught to stop infinite loops
- Sniff codes are no longer optional - use them with addError/Warning/Fixable
- installed_paths config setting can now point directly to a standard instead of the dir above
- --report=full,summary,info instead of --report=full --report=summary --report=info
- Improved the performance of the CSS tokenizer, especially on very large CSS files (thousands of lines)
Expand Down
94 changes: 50 additions & 44 deletions src/Files/File.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -566,7 +566,7 @@ public function cleanUp()
public function addError(
$error,
$stackPtr,
$code='',
$code,
$data=array(),
$severity=0,
$fixable=false
Expand Down Expand Up @@ -600,7 +600,7 @@ public function addError(
public function addWarning(
$warning,
$stackPtr,
$code='',
$code,
$data=array(),
$severity=0,
$fixable=false
Expand Down Expand Up @@ -633,7 +633,7 @@ public function addWarning(
public function addErrorOnLine(
$error,
$line,
$code='',
$code,
$data=array(),
$severity=0
) {
Expand All @@ -657,7 +657,7 @@ public function addErrorOnLine(
public function addWarningOnLine(
$warning,
$line,
$code='',
$code,
$data=array(),
$severity=0
) {
Expand All @@ -683,7 +683,7 @@ public function addWarningOnLine(
public function addFixableError(
$error,
$stackPtr,
$code='',
$code,
$data=array(),
$severity=0
) {
Expand Down Expand Up @@ -714,7 +714,7 @@ public function addFixableError(
public function addFixableWarning(
$warning,
$stackPtr,
$code='',
$code,
$data=array(),
$severity=0
) {
Expand Down Expand Up @@ -751,24 +751,24 @@ protected function _addError($error, $line, $column, $code, $data, $severity, $f
// Work out which sniff generated the error.
if (substr($code, 0, 9) === 'Internal.') {
// Any internal message.
$sniff = $code;
$sniffCode = $code;
$listenerCode = Util\Common::getSniffCode($this->activeListener);
$sniffCode = $code;
} else if (strpos($code, '.') !== false) {
// The full message code has been passed in.
$sniffCode = $code;
$listenerCode = substr($sniffCode, 0, strrpos($sniffCode, '.'));
} else {
$parts = explode('\\', $this->activeListener);
if (isset($parts[5]) === true) {
$sniff = $parts[2].'.'.$parts[4].'.'.$parts[5];

// Remove "Sniff" from the end.
$sniff = substr($sniff, 0, -5);
} else {
$sniff = 'unknownSniff';
}
$listenerCode = Util\Common::getSniffCode($this->activeListener);
$sniffCode = $listenerCode.'.'.$code;
}

$sniffCode = $sniff;
if ($code !== '') {
$sniffCode .= '.'.$code;
}
}//end if
// Filter out any messages for sniffs that shouldn't have run.
if ($this->config->cache === false
&& empty($this->config->sniffs) === false
&& in_array($listenerCode, $this->config->sniffs) === false
) {
return false;
}

// If we know this sniff code is being ignored for this file, return early.
if (isset($this->ignoredCodes[$sniffCode]) === true) {
Expand All @@ -781,7 +781,7 @@ protected function _addError($error, $line, $column, $code, $data, $severity, $f
) {
// Pass this off to the warning handler.
return $this->_addWarning($error, $line, $column, $code, $data, $severity, $fixable);
} else if ($this->config->errorSeverity === 0) {
} else if ($this->config->cache === false && $this->config->errorSeverity === 0) {
// Don't bother doing any processing as errors are just going to
// be hidden in the reports anyway.
return false;
Expand All @@ -794,7 +794,7 @@ protected function _addError($error, $line, $column, $code, $data, $severity, $f
$severity = 5;
}

if ($this->config->errorSeverity > $severity) {
if ($this->config->cache === false && $this->config->errorSeverity > $severity) {
return false;
}

Expand Down Expand Up @@ -827,7 +827,7 @@ protected function _addError($error, $line, $column, $code, $data, $severity, $f
$this->fixableCount++;
}

if ($this->recordErrors === false) {
if ($this->config->cache === false && $this->recordErrors === false) {
if (isset($this->errors[$line]) === false) {
$this->errors[$line] = 0;
}
Expand Down Expand Up @@ -858,6 +858,7 @@ protected function _addError($error, $line, $column, $code, $data, $severity, $f
$this->errors[$line][$column][] = array(
'message' => $message,
'source' => $sniffCode,
'listener' => $this->activeListener,
'severity' => $severity,
'fixable' => $fixable,
);
Expand Down Expand Up @@ -896,27 +897,31 @@ protected function _addWarning($warning, $line, $column, $code, $data, $severity
return false;
}

if (isset($this->ruleset->sniffs[$this->activeListener]) === false) {
return false;
}

// Work out which sniff generated the warning.
if (substr($code, 0, 9) === 'Internal.') {
// Any internal message.
$sniff = $code;
$sniffCode = $code;
$listenerCode = Util\Common::getSniffCode($this->activeListener);
$sniffCode = $code;
} else if (strpos($code, '.') !== false) {
// The full message code has been passed in.
$sniffCode = $code;
$listenerCode = substr($sniffCode, 0, strrpos($sniffCode, '.'));
} else {
$parts = explode('\\', $this->activeListener);
if (isset($parts[5]) === true) {
$sniff = $parts[2].'.'.$parts[4].'.'.$parts[5];

// Remove "Sniff" from the end.
$sniff = substr($sniff, 0, -5);
} else {
$sniff = 'unknownSniff';
}
$listenerCode = Util\Common::getSniffCode($this->activeListener);
$sniffCode = $listenerCode.'.'.$code;
}

$sniffCode = $sniff;
if ($code !== '') {
$sniffCode .= '.'.$code;
}
}//end if
// Filter out any messages for sniffs that shouldn't have run.
if ($this->config->cache === false
&& empty($this->config->sniffs) === false
&& in_array($listenerCode, $this->config->sniffs) === false
) {
return false;
}

// If we know this sniff code is being ignored for this file, return early.
if (isset($this->ignoredCodes[$sniffCode]) === true) {
Expand All @@ -929,7 +934,7 @@ protected function _addWarning($warning, $line, $column, $code, $data, $severity
) {
// Pass this off to the error handler.
return $this->_addError($warning, $line, $column, $code, $data, $severity, $fixable);
} else if ($this->config->warningSeverity === 0) {
} else if ($this->config->cache === false && $this->config->warningSeverity === 0) {
// Don't bother doing any processing as warnings are just going to
// be hidden in the reports anyway.
return false;
Expand All @@ -942,7 +947,7 @@ protected function _addWarning($warning, $line, $column, $code, $data, $severity
$severity = 5;
}

if ($this->config->warningSeverity > $severity) {
if ($this->config->cache === false && $this->config->warningSeverity > $severity) {
return false;
}

Expand Down Expand Up @@ -975,7 +980,7 @@ protected function _addWarning($warning, $line, $column, $code, $data, $severity
$this->fixableCount++;
}

if ($this->recordErrors === false) {
if ($this->config->cache === false && $this->recordErrors === false) {
if (isset($this->warnings[$line]) === false) {
$this->warnings[$line] = 0;
}
Expand Down Expand Up @@ -1006,6 +1011,7 @@ protected function _addWarning($warning, $line, $column, $code, $data, $severity
$this->warnings[$line][$column][] = array(
'message' => $message,
'source' => $sniffCode,
'listener' => $this->activeListener,
'severity' => $severity,
'fixable' => $fixable,
);
Expand Down
73 changes: 67 additions & 6 deletions src/Files/LocalFile.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,18 +85,24 @@ function reloadContent()
public function process()
{
if ($this->config->cache === false) {
if (PHP_CODESNIFFER_VERBOSITY > 1) {
echo PHP_EOL;
}

return parent::process();
}

$hash = $this->path.'.'.md5_file($this->path);
$cache = Cache::get($hash);
if ($cache !== false) {
$this->errors = $cache['errors'];
$this->warnings = $cache['warnings'];
$this->metrics = $cache['metrics'];
$this->errorCount = $cache['errorCount'];
$this->warningCount = $cache['warningCount'];
$this->fixableCount = $cache['fixableCount'];
// We can't filter metrics, so just load all of them.
$this->metrics = $cache['metrics'];

// Replay the cached errors and warnings to filter out the ones
// we don't need for this specific run.
$this->config->cache = false;
$this->replayErrors($cache['errors'], $cache['warnings']);
$this->config->cache = true;

if (PHP_CODESNIFFER_VERBOSITY > 0
|| (PHP_CODESNIFFER_CBF === true && empty($this->config->files) === false)
Expand All @@ -107,6 +113,10 @@ public function process()
return;
}

if (PHP_CODESNIFFER_VERBOSITY > 1) {
echo PHP_EOL;
}

parent::process();

$cache = array(
Expand All @@ -120,7 +130,58 @@ public function process()

Cache::set($hash, $cache);

// During caching, we don't filter out errors in any way, so
// we need to do that manually now by replaying them.
$this->config->cache = false;
$this->replayErrors($this->errors, $this->warnings);
$this->config->cache = true;

}//end process()


private function replayErrors($errors, $warnings)
{
$this->errors = array();
$this->warnigns = array();
$this->errorCount = 0;
$this->warningCount = 0;
$this->fixableCount = 0;

foreach ($errors as $line => $lineErrors) {
foreach ($lineErrors as $column => $colErrors) {
foreach ($colErrors as $error) {
$this->activeListener = $error['listener'];
$this->_addError(
$error['message'],
$line,
$column,
$error['source'],
array(),
$error['severity'],
$error['fixable']
);
}
}
}

foreach ($warnings as $line => $lineErrors) {
foreach ($lineErrors as $column => $colErrors) {
foreach ($colErrors as $error) {
$this->activeListener = $error['listener'];
$this->_addWarning(
$error['message'],
$line,
$column,
$error['source'],
array(),
$error['severity'],
$error['fixable']
);
}
}
}

}//end replayErrors()


}//end class
7 changes: 6 additions & 1 deletion src/Ruleset.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,12 @@ class Ruleset
*/
public function __construct(Config $config)
{
$restrictions = $config->sniffs;
// Ignore sniff restrictions if caching is on.
$restrictions = array();
if ($config->cache === false) {
$restrictions = $config->sniffs;
}

$this->config = $config;

$sniffs = array();
Expand Down
20 changes: 13 additions & 7 deletions src/Runner.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -322,12 +322,21 @@ private function run()
$todo = new FileList($this->config, $ruleset);
$numFiles = count($todo);

if (PHP_CODESNIFFER_VERBOSITY > 0) {
echo "DONE ($numFiles files in queue)".PHP_EOL;
}

if ($this->config->cache === true) {
if (PHP_CODESNIFFER_VERBOSITY > 0) {
echo 'Loading cache... ';
}

Cache::load($this->config, $ruleset);
}

if (PHP_CODESNIFFER_VERBOSITY > 0) {
echo "DONE ($numFiles files in queue)".PHP_EOL;
if (PHP_CODESNIFFER_VERBOSITY > 0) {
$size = Cache::getSize();
echo "DONE ($size files in cache)".PHP_EOL;
}
}
}//end if

Expand All @@ -352,9 +361,6 @@ private function run()
if (PHP_CODESNIFFER_VERBOSITY > 0 || (PHP_CODESNIFFER_CBF === true && $this->config->stdin === false)) {
$startTime = microtime(true);
echo 'Processing '.basename($path).' ';
if (PHP_CODESNIFFER_VERBOSITY > 1) {
echo PHP_EOL;
}
}

try {
Expand All @@ -381,7 +387,7 @@ private function run()
}
} catch (\Exception $e) {
$error = 'An error occurred during processing; checking has been aborted. The error message was: '.$e->getMessage();
$file->addErrorOnLine($error, 1);
$file->addErrorOnLine($error, 1, 'Internal.Exception');
}//end try

$this->reporter->cacheFileReport($file, $this->config);
Expand Down
2 changes: 1 addition & 1 deletion src/Sniffs/AbstractPatternSniff.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,7 @@ final public function process(File $phpcsFile, $stackPtr)
}

foreach ($allErrors as $stackPtr => $error) {
$phpcsFile->addError($error, $stackPtr);
$phpcsFile->addError($error, $stackPtr, 'Found');
}

}//end process()
Expand Down
2 changes: 1 addition & 1 deletion src/Standards/Squiz/Sniffs/Commenting/BlockCommentSniff.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,7 @@ public function process(File $phpcsFile, $stackPtr)
$content = trim($tokens[$commentLines[$lastIndex]]['content']);
if ($content !== '*/' && $content !== '**/') {
$error = 'Comment closer must be on a new line';
$phpcsFile->addError($error, $commentLines[$lastIndex]);
$phpcsFile->addError($error, $commentLines[$lastIndex], 'CloserSameLine');
} else {
$content = $tokens[$commentLines[$lastIndex]]['content'];
$commentText = ltrim($content);
Expand Down
Loading

0 comments on commit c587b27

Please sign in to comment.