Skip to content

Only run orig file PHPCS linting when the new version has lint issues #51

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.

Sign up for GitHub

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 6 commits into from
Feb 4, 2022
Merged

Only run orig file PHPCS linting when the new version has lint issues #51

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f74ce83
Only run orig file PHPCS linting when the new version has lint issues
david-binda Feb 2, 2022
e2d74f7
Do not check orig verion of the file even in SVN workflow
david-binda Feb 2, 2022
2fe1631
Properly set the fallback value for $newFilePhpcsMessages
david-binda Feb 2, 2022
96d2445
Optimize variables defined in the catch statement.
david-binda Feb 3, 2022
02600eb
Add a test for git workflow ensuring the orig file is not linted if n…
david-binda Feb 4, 2022
3c6ae7d
Adjust the testFullSvnWorkflowForOneFileWithNoMessages test according…
david-binda Feb 4, 2022
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
94 changes: 57 additions & 37 deletions PhpcsChanged/Cli.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -238,8 +238,24 @@ function runSvnWorkflowForFile(string $svnFile, array $options, ShellOperator $s
$revisionId = getSvnRevisionId($svnFileInfo);
$isNewFile = isNewSvnFile($svnFileInfo);

$newFileHash = $shell->getFileHash($svnFile);
$newFilePhpcsOutput = isCachingEnabled($options) ? $cache->getCacheForFile($svnFile, 'new', $newFileHash, $phpcsStandard ?? '') : null;
if ($newFilePhpcsOutput) {
$debug("Using cache for new file '{$svnFile}' at revision '{$revisionId}', hash '{$newFileHash}', and standard '{$phpcsStandard}'");
}
if (! $newFilePhpcsOutput) {
$debug("Not using cache for new file '{$svnFile}' at revision '{$revisionId}', hash '{$newFileHash}', and standard '{$phpcsStandard}'");
$newFilePhpcsOutput = getSvnNewPhpcsOutput($svnFile, $phpcs, $cat, $phpcsStandardOption, [$shell, 'executeCommand'], $debug);
if (isCachingEnabled($options)) {
$cache->setCacheForFile($svnFile, 'new', $newFileHash, $phpcsStandard ?? '', $newFilePhpcsOutput);
}
}
$fileName = DiffLineMap::getFileNameFromDiff($unifiedDiff);
$newFilePhpcsMessages = PhpcsMessages::fromPhpcsJson($newFilePhpcsOutput, $fileName);
$hasNewFilePhpcsIssues = !empty($newFilePhpcsMessages->getMessages());

$oldFilePhpcsOutput = '';
if ( ! $isNewFile ) {
if ( ! $isNewFile && $hasNewFilePhpcsIssues) {
$oldFilePhpcsOutput = isCachingEnabled($options) ? $cache->getCacheForFile($svnFile, 'old', $revisionId, $phpcsStandard ?? '') : null;
if ($oldFilePhpcsOutput) {
$debug("Using cache for old file '{$svnFile}' at revision '{$revisionId}' and standard '{$phpcsStandard}'");
Expand All @@ -251,37 +267,30 @@ function runSvnWorkflowForFile(string $svnFile, array $options, ShellOperator $s
$cache->setCacheForFile($svnFile, 'old', $revisionId, $phpcsStandard ?? '', $oldFilePhpcsOutput);
}
}
}

$newFileHash = $shell->getFileHash($svnFile);
$newFilePhpcsOutput = isCachingEnabled($options) ? $cache->getCacheForFile($svnFile, 'new', $newFileHash, $phpcsStandard ?? '') : null;
if ($newFilePhpcsOutput) {
$debug("Using cache for new file '{$svnFile}' at revision '{$revisionId}', hash '{$newFileHash}', and standard '{$phpcsStandard}'");
}
if (! $newFilePhpcsOutput) {
$debug("Not using cache for new file '{$svnFile}' at revision '{$revisionId}', hash '{$newFileHash}', and standard '{$phpcsStandard}'");
$newFilePhpcsOutput = getSvnNewPhpcsOutput($svnFile, $phpcs, $cat, $phpcsStandardOption, [$shell, 'executeCommand'], $debug);
if (isCachingEnabled($options)) {
$cache->setCacheForFile($svnFile, 'new', $newFileHash, $phpcsStandard ?? '', $newFilePhpcsOutput);
} else {
if ($isNewFile) {
$debug('Skipping the linting of the orig file version as it is a new file.');
} else {
$debug('Skipping the linting of the orig file version as the new version of the file contains no lint errors.');
}
}
} catch( NoChangesException $err ) {
$debug($err->getMessage());
$unifiedDiff = '';
$oldFilePhpcsOutput = '';
$newFilePhpcsOutput = '';
$newFilePhpcsMessages = PhpcsMessages::fromPhpcsJson('');
} catch( \Exception $err ) {
$shell->printError($err->getMessage());
$shell->exitWithCode(1);
throw $err; // Just in case we do not actually exit, like in tests
}

$debug('processing data...');
$fileName = DiffLineMap::getFileNameFromDiff($unifiedDiff);
$fileName = $fileName ?? DiffLineMap::getFileNameFromDiff($unifiedDiff);
return getNewPhpcsMessages(
$unifiedDiff,
PhpcsMessages::fromPhpcsJson($oldFilePhpcsOutput, $fileName),
PhpcsMessages::fromPhpcsJson($newFilePhpcsOutput, $fileName)
$newFilePhpcsMessages
);
}

Expand Down Expand Up @@ -328,8 +337,31 @@ function runGitWorkflowForFile(string $gitFile, array $options, ShellOperator $s
validateGitFileExists($gitFile, $git, [$shell, 'isReadable'], [$shell, 'executeCommand'], $debug, $options);
$unifiedDiff = getGitUnifiedDiff($gitFile, $git, [$shell, 'executeCommand'], $options, $debug);
$isNewFile = isNewGitFile($gitFile, $git, [$shell, 'executeCommand'], $options, $debug);

$newFilePhpcsOutput = null;
$newFileHash = '';
if (isCachingEnabled($options)) {
$newFileHash = getNewGitFileHash($gitFile, $git, $cat, [$shell, 'executeCommand'], $options, $debug);
$newFilePhpcsOutput = $cache->getCacheForFile($gitFile, 'new', $newFileHash, $phpcsStandard ?? '');
}
if ($newFilePhpcsOutput) {
$debug("Using cache for new file '{$gitFile}' at hash '{$newFileHash}', and standard '{$phpcsStandard}'");
}
if (! $newFilePhpcsOutput) {
$debugMessage = (!empty($newFileHash)) ? "Not using cache for new file '{$gitFile}' at hash '{$newFileHash}', and standard '{$phpcsStandard}'" : "Not using cache for new file '{$gitFile}' with standard '{$phpcsStandard}'. No hash was calculated.";
$debug($debugMessage);
$newFilePhpcsOutput = getGitNewPhpcsOutput($gitFile, $git, $phpcs, $cat, $phpcsStandardOption, [$shell, 'executeCommand'], $options, $debug);
if (isCachingEnabled($options)) {
$cache->setCacheForFile($gitFile, 'new', $newFileHash, $phpcsStandard ?? '', $newFilePhpcsOutput);
}
}
$fileName = DiffLineMap::getFileNameFromDiff($unifiedDiff);
$newFilePhpcsMessages = PhpcsMessages::fromPhpcsJson($newFilePhpcsOutput, $fileName);
$hasNewFilePhpcsIssues = !empty($newFilePhpcsMessages->getMessages());

$oldFilePhpcsOutput = '';
if (! $isNewFile) {
if (! $isNewFile && $hasNewFilePhpcsIssues) {
$debug('Checking the orig file version for PHPCS issues since the file is not new and contains some linting issues.');
$oldFilePhpcsOutput = null;
$oldFileHash = '';
if (isCachingEnabled($options)) {
Expand All @@ -347,39 +379,27 @@ function runGitWorkflowForFile(string $gitFile, array $options, ShellOperator $s
$cache->setCacheForFile($gitFile, 'old', $oldFileHash, $phpcsStandard ?? '', $oldFilePhpcsOutput);
}
}
}

$newFilePhpcsOutput = null;
$newFileHash = '';
if (isCachingEnabled($options)) {
$newFileHash = getNewGitFileHash($gitFile, $git, $cat, [$shell, 'executeCommand'], $options, $debug);
$newFilePhpcsOutput = $cache->getCacheForFile($gitFile, 'new', $newFileHash, $phpcsStandard ?? '');
}
if ($newFilePhpcsOutput) {
$debug("Using cache for new file '{$gitFile}' at hash '{$newFileHash}', and standard '{$phpcsStandard}'");
}
if (! $newFilePhpcsOutput) {
$debugMessage = (!empty($newFileHash)) ? "Not using cache for new file '{$gitFile}' at hash '{$newFileHash}', and standard '{$phpcsStandard}'" : "Not using cache for new file '{$gitFile}' with standard '{$phpcsStandard}'. No hash was calculated.";
$debug($debugMessage);
$newFilePhpcsOutput = getGitNewPhpcsOutput($gitFile, $git, $phpcs, $cat, $phpcsStandardOption, [$shell, 'executeCommand'], $options, $debug);
if (isCachingEnabled($options)) {
$cache->setCacheForFile($gitFile, 'new', $newFileHash, $phpcsStandard ?? '', $newFilePhpcsOutput);
} else {
if ($isNewFile) {
$debug('Skipping the linting of the orig file version as it is a new file.');
} else {
$debug('Skipping the linting of the orig file version as the new version of the file contains no lint errors.');
}
}
} catch( NoChangesException $err ) {
$debug($err->getMessage());
$unifiedDiff = '';
$oldFilePhpcsOutput = '';
$newFilePhpcsOutput = '';
$newFilePhpcsMessages = PhpcsMessages::fromPhpcsJson('');
} catch(\Exception $err) {
$shell->printError($err->getMessage());
$shell->exitWithCode(1);
throw $err; // Just in case we do not actually exit
}

$debug('processing data...');
$fileName = DiffLineMap::getFileNameFromDiff($unifiedDiff);
return getNewPhpcsMessages($unifiedDiff, PhpcsMessages::fromPhpcsJson($oldFilePhpcsOutput, $fileName), PhpcsMessages::fromPhpcsJson($newFilePhpcsOutput, $fileName));
$fileName = $fileName ?? DiffLineMap::getFileNameFromDiff($unifiedDiff);
return getNewPhpcsMessages($unifiedDiff, PhpcsMessages::fromPhpcsJson($oldFilePhpcsOutput, $fileName), $newFilePhpcsMessages);
}

function reportMessagesAndExit(PhpcsMessages $messages, string $reportType, array $options): void {
Expand Down
20 changes: 20 additions & 0 deletions tests/GitWorkflowTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,26 @@ public function testFullGitWorkflowForOneFileUnstaged() {
$this->assertEquals($expected->getMessages(), $messages->getMessages());
}

public function testFullGitWorkflowForOneFileUnstagedLintOnlyPhpcsNewFile() {
$gitFile = 'foobar.php';
$shell = new TestShell([$gitFile]);
$fixture = $this->fixture->getAddedLineDiff('foobar.php', 'use Foobar;');
$shell->registerCommand("git diff --no-prefix 'foobar.php'", $fixture);
$shell->registerCommand("git status --porcelain 'foobar.php'", $this->fixture->getModifiedFileInfo('foobar.php'));
$shell->registerCommand("cat 'foobar.php' | phpcs", $this->phpcs->getEmptyResults()->toPhpcsJson());

$options = [
'git-unstaged' => '1',
];
$cache = new CacheManager( new TestCache(), '\PhpcsChangedTests\Debug' );
$expected = $this->phpcs->getEmptyResults();

$messages = runGitWorkflow([$gitFile], $options, $shell, $cache, '\PhpcsChangedTests\Debug');

$this->assertEquals($expected->getMessages(), $messages->getMessages());
$this->assertFalse($shell->wasCommandCalled("git show :0:$(git ls-files --full-name 'foobar.php') | phpcs"));
}

public function testFullGitWorkflowForOneFileUnstagedCachesDataThenUsesCache() {
$gitFile = 'foobar.php';
$shell = new TestShell([$gitFile]);
Expand Down
2 changes: 1 addition & 1 deletion tests/SvnWorkflowTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,12 +79,12 @@ public function testFullSvnWorkflowForOneFileWithNoMessages() {
$shell = new TestShell([$svnFile]);
$shell->registerCommand("svn diff 'foobar.php'", $this->fixture->getAddedLineDiff('foobar.php', 'use Foobar;'));
$shell->registerCommand("svn info 'foobar.php'", $this->fixture->getSvnInfo('foobar.php'));
$shell->registerCommand("svn cat 'foobar.php'", $this->phpcs->getResults('STDIN', [20, 99])->toPhpcsJson());
$shell->registerCommand("cat 'foobar.php'", $this->phpcs->getEmptyResults()->toPhpcsJson());
$options = [];
$expected = $this->phpcs->getEmptyResults();
$messages = runSvnWorkflow([$svnFile], $options, $shell, new CacheManager(new TestCache()), '\PhpcsChangedTests\debug');
$this->assertEquals($expected->getMessages(), $messages->getMessages());
$this->assertFalse($shell->wasCommandCalled("svn cat 'foobar.php'"));
}

public function testFullSvnWorkflowForOneFileWithCachingEnabledButNoCache() {
Expand Down