diff --git a/PhpcsChanged/CacheManager.php b/PhpcsChanged/CacheManager.php index 09b050f..139862f 100644 --- a/PhpcsChanged/CacheManager.php +++ b/PhpcsChanged/CacheManager.php @@ -162,13 +162,13 @@ public function setCacheForFile(string $filePath, string $type, string $hash, st public function addCacheEntry(CacheEntry $entry): void { $this->hasBeenModified = true; $this->pruneOldEntriesForFile($entry); - if (! isset($this->fileDataByPath[$entry->path])) { + if (! array_key_exists($entry->path, $this->fileDataByPath)) { $this->fileDataByPath[$entry->path] = []; } - if (! isset($this->fileDataByPath[$entry->path][$entry->type])) { + if (! array_key_exists($entry->type, $this->fileDataByPath[$entry->path])) { $this->fileDataByPath[$entry->path][$entry->type] = []; } - if (! isset($this->fileDataByPath[$entry->path][$entry->type][$entry->hash])) { + if (! array_key_exists($entry->hash, $this->fileDataByPath[$entry->path][$entry->type])) { $this->fileDataByPath[$entry->path][$entry->type][$entry->hash] = []; } $this->fileDataByPath[$entry->path][$entry->type][$entry->hash][$entry->phpcsStandard] = $entry; diff --git a/PhpcsChanged/Cli.php b/PhpcsChanged/Cli.php index dfe5b63..0c2a646 100644 --- a/PhpcsChanged/Cli.php +++ b/PhpcsChanged/Cli.php @@ -260,7 +260,7 @@ function runSvnWorkflowForFile(string $svnFile, CliOptions $options, ShellOperat } $modifiedFilePhpcsMessages = PhpcsMessages::fromPhpcsJson($modifiedFilePhpcsOutput, $fileName); - $hasNewPhpcsMessages = !empty($modifiedFilePhpcsMessages->getMessages()); + $hasNewPhpcsMessages = count($modifiedFilePhpcsMessages->getMessages()) > 0; if (! $hasNewPhpcsMessages) { throw new NoChangesException("Modified file '{$svnFile}' has no PHPCS messages; skipping"); @@ -357,7 +357,7 @@ function runGitWorkflowForFile(string $gitFile, CliOptions $options, ShellOperat } $modifiedFilePhpcsMessages = PhpcsMessages::fromPhpcsJson($modifiedFilePhpcsOutput, $fileName); - $hasNewPhpcsMessages = !empty($modifiedFilePhpcsMessages->getMessages()); + $hasNewPhpcsMessages = count($modifiedFilePhpcsMessages->getMessages()) > 0; $unifiedDiff = ''; $unmodifiedFilePhpcsOutput = ''; @@ -414,7 +414,7 @@ function fileHasValidExtension(\SplFileInfo $file, string $phpcsExtensions = '') // The following logic is copied from PHPCS itself. See https://github.com/squizlabs/PHP_CodeSniffer/blob/2ecd8dc15364cdd6e5089e82ffef2b205c98c412/src/Filters/Filter.php#L161 // phpcs:disable - if (empty($phpcsExtensions)) { + if (! boolval($phpcsExtensions)) { $AllowedExtensions = [ 'php', 'inc', @@ -443,7 +443,7 @@ function fileHasValidExtension(\SplFileInfo $file, string $phpcsExtensions = '') array_shift($fileParts); } $matches = array_intersect($extensions, $AllowedExtensions); - if (empty($matches) === true) { + if (count($matches) === 0) { return false; } @@ -529,10 +529,10 @@ function shouldIgnorePath(string $path, string $patternOption = null): bool { } function isCachingEnabled(array $options): bool { - if (isset($options['no-cache'])) { + if (array_key_exists('no-cache', $options)) { return false; } - if (isset($options['cache'])) { + if (array_key_exists('cache', $options)) { return true; } return false; @@ -553,7 +553,7 @@ function loadCache(CacheManager $cache, ShellOperator $shell, array $options): v } } - if (isset($options['clear-cache'])) { + if (array_key_exists('clear-cache', $options)) { $cache->clearCache(); try { $cache->save(); diff --git a/PhpcsChanged/CliOptions.php b/PhpcsChanged/CliOptions.php index 1517ca2..fe74a33 100644 --- a/PhpcsChanged/CliOptions.php +++ b/PhpcsChanged/CliOptions.php @@ -169,94 +169,94 @@ class CliOptions { public static function fromArray(array $options): self { $cliOptions = new self(); // Note that this array is likely created by `getopt()` which sets any - // boolean option to `false`, meaning that we must use `isset()` to - // determine if these options are set. - if (isset($options['files'])) { + // boolean option to `false` (it's so confusing), meaning that we cannot + // check the truthiness of the option. + if (array_key_exists('files', $options)) { $cliOptions->files = $options['files']; } - if (isset($options['no-vendor-phpcs'])) { + if (array_key_exists('no-vendor-phpcs', $options)) { $cliOptions->noVendorPhpcs = true; } - if (isset($options['phpcs-path'])) { + if (array_key_exists('phpcs-path', $options)) { $cliOptions->phpcsPath = $options['phpcs-path']; } - if (isset($options['git-path'])) { + if (array_key_exists('git-path', $options)) { $cliOptions->gitPath = $options['git-path']; } - if (isset($options['cat-path'])) { + if (array_key_exists('cat-path', $options)) { $cliOptions->catPath = $options['cat-path']; } - if (isset($options['svn-path'])) { + if (array_key_exists('svn-path', $options)) { $cliOptions->svnPath = $options['svn-path']; } - if (isset($options['svn'])) { + if (array_key_exists('svn', $options)) { $cliOptions->mode = Modes::SVN; } - if (isset($options['git'])) { + if (array_key_exists('git', $options)) { $cliOptions->mode = Modes::GIT_STAGED; } - if (isset($options['git-unstaged'])) { + if (array_key_exists('git-unstaged', $options)) { $cliOptions->mode = Modes::GIT_UNSTAGED; } - if (isset($options['git-staged'])) { + if (array_key_exists('git-staged', $options)) { $cliOptions->mode = Modes::GIT_STAGED; } - if (isset($options['git-base'])) { + if (array_key_exists('git-base', $options)) { $cliOptions->mode = Modes::GIT_BASE; $cliOptions->gitBase = $options['git-base']; } - if (isset($options['report'])) { + if (array_key_exists('report', $options)) { $cliOptions->reporter = $options['report']; } - if (isset($options['debug'])) { + if (array_key_exists('debug', $options)) { $cliOptions->debug = true; } - if (isset($options['clear-cache'])) { + if (array_key_exists('clear-cache', $options)) { $cliOptions->clearCache = true; } - if (isset($options['cache'])) { + if (array_key_exists('cache', $options)) { $cliOptions->useCache = true; } - if (isset($options['no-cache'])) { + if (array_key_exists('no-cache', $options)) { $cliOptions->useCache = false; } - if (isset($options['diff'])) { + if (array_key_exists('diff', $options)) { $cliOptions->mode = Modes::MANUAL; $cliOptions->diffFile = $options['diff']; } - if (isset($options['phpcs-unmodified'])) { + if (array_key_exists('phpcs-unmodified', $options)) { $cliOptions->mode = Modes::MANUAL; $cliOptions->phpcsUnmodified = $options['phpcs-unmodified']; } - if (isset($options['phpcs-modified'])) { + if (array_key_exists('phpcs-modified', $options)) { $cliOptions->mode = Modes::MANUAL; $cliOptions->phpcsModified = $options['phpcs-modified']; } - if (isset($options['s'])) { + if (array_key_exists('s', $options)) { $cliOptions->showMessageCodes = true; } - if (isset($options['standard'])) { + if (array_key_exists('standard', $options)) { $cliOptions->phpcsStandard = $options['standard']; } - if (isset($options['extensions'])) { + if (array_key_exists('extensions', $options)) { $cliOptions->phpcsExtensions = $options['extensions']; } - if (isset($options['always-exit-zero'])) { + if (array_key_exists('always-exit-zero', $options)) { $cliOptions->alwaysExitZero = true; } - if (isset($options['no-cache-git-root'])) { + if (array_key_exists('no-cache-git-root', $options)) { $cliOptions->noCacheGitRoot = true; } - if (isset($options['no-verify-git-file'])) { + if (array_key_exists('no-verify-git-file', $options)) { $cliOptions->noVerifyGitFile = true; } - if (isset($options['warning-severity'])) { + if (array_key_exists('warning-severity', $options)) { $cliOptions->warningSeverity = $options['warning-severity']; } - if (isset($options['error-severity'])) { + if (array_key_exists('error-severity', $options)) { $cliOptions->errorSeverity = $options['error-severity']; } - if (isset($options['i'])) { + if (array_key_exists('i', $options)) { $cliOptions->mode = Modes::INFO_ONLY; } $cliOptions->validate(); @@ -267,31 +267,31 @@ public function toArray(): array { $options = []; $options['report'] = $this->reporter; $options['files'] = $this->files; - if ($this->phpcsStandard) { + if (boolval($this->phpcsStandard)) { $options['standard'] = $this->phpcsStandard; } - if ($this->phpcsExtensions) { + if (boolval($this->phpcsExtensions)) { $options['extensions'] = $this->phpcsExtensions; } - if ($this->noVendorPhpcs) { + if (boolval($this->noVendorPhpcs)) { $options['no-vendor-phpcs'] = true; } - if ($this->phpcsPath) { + if (boolval($this->phpcsPath)) { $options['phpcs-path'] = $this->phpcsPath; } - if ($this->gitPath) { + if (boolval($this->gitPath)) { $options['git-path'] = $this->gitPath; } - if ($this->catPath) { + if (boolval($this->catPath)) { $options['cat-path'] = $this->catPath; } - if ($this->svnPath) { + if (boolval($this->svnPath)) { $options['svn-path'] = $this->svnPath; } - if ($this->debug) { + if (boolval($this->debug)) { $options['debug'] = true; } - if ($this->showMessageCodes) { + if (boolval($this->showMessageCodes)) { $options['s'] = true; } if ($this->mode === Modes::SVN) { @@ -309,13 +309,13 @@ public function toArray(): array { $options['git'] = true; $options['git-base'] = $this->gitBase; } - if ($this->useCache) { + if (boolval($this->useCache)) { $options['cache'] = true; } - if (! $this->useCache) { + if (! boolval($this->useCache)) { $options['no-cache'] = true; } - if ($this->clearCache) { + if (boolval($this->clearCache)) { $options['clear-cache'] = true; } if ($this->mode === Modes::MANUAL) { @@ -323,19 +323,22 @@ public function toArray(): array { $options['phpcs-unmodified'] = $this->phpcsUnmodified; $options['phpcs-modified'] = $this->phpcsModified; } - if ($this->alwaysExitZero) { + if (boolval($this->alwaysExitZero)) { $options['always-exit-zero'] = true; } - if ($this->noCacheGitRoot) { + if (boolval($this->noCacheGitRoot)) { $options['no-cache-git-root'] = true; } - if ($this->noVerifyGitFile) { + if (boolval($this->noVerifyGitFile)) { $options['no-verify-git-file'] = true; } - if (isset($this->warningSeverity)) { + // Note that both warningSeverity and errorSeverity can be the string '0' + // which is falsy in PHP but is a valid value here so we must be careful + // when testing for it. + if (is_string($this->warningSeverity) && strlen($this->warningSeverity) > 0) { $options['warning-severity'] = $this->warningSeverity; } - if (isset($this->errorSeverity)) { + if (is_string($this->errorSeverity) && strlen($this->errorSeverity) > 0) { $options['error-severity'] = $this->errorSeverity; } return $options; @@ -349,34 +352,62 @@ public function isGitMode(): bool { public function getExecutablePath(string $executableName): string { switch ($executableName) { case 'phpcs': - return $this->phpcsPath ?: getenv('PHPCS') ?: 'phpcs'; + if (is_string($this->phpcsPath) && strlen($this->phpcsPath) > 0) { + return $this->phpcsPath; + } + $env = getenv('PHPCS'); + if (is_string($env) && strlen($env) > 0) { + return $env; + } + return 'phpcs'; case 'git': - return $this->gitPath ?: getenv('GIT') ?: 'git'; + if (is_string($this->gitPath) && strlen($this->gitPath) > 0) { + return $this->gitPath; + } + $env = getenv('GIT'); + if (is_string($env) && strlen($env) > 0) { + return $env; + } + return 'git'; case 'cat': - return $this->catPath ?: getenv('CAT') ?: 'cat'; + if (is_string($this->catPath) && strlen($this->catPath) > 0) { + return $this->catPath; + } + $env = getenv('CAT'); + if (is_string($env) && strlen($env) > 0) { + return $env; + } + return 'cat'; case 'svn': - return $this->svnPath ?: getenv('SVN') ?: 'svn'; + if (is_string($this->svnPath) && strlen($this->svnPath) > 0) { + return $this->svnPath; + } + $env = getenv('SVN'); + if (is_string($env) && strlen($env) > 0) { + return $env; + } + return 'svn'; default: throw new \Exception("No executable found called '{$executableName}'."); } } public function validate(): void { - if (empty($this->mode)) { + if (! boolval($this->mode)) { throw new InvalidOptionException('You must use either automatic or manual mode.'); } if ($this->mode === Modes::MANUAL) { - if (empty($this->diffFile) || empty($this->phpcsUnmodified) || empty($this->phpcsModified)) { + if ( ! boolval($this->diffFile) || ! boolval($this->phpcsUnmodified) || ! boolval($this->phpcsModified)) { throw new InvalidOptionException('Manual mode requires a diff, the unmodified file phpcs output, and the modified file phpcs output.'); } } - if ($this->mode === Modes::GIT_BASE && empty($this->gitBase)) { + if ($this->mode === Modes::GIT_BASE && ! boolval($this->gitBase)) { throw new InvalidOptionException('git-base mode requires a git object.'); } - if ($this->isGitMode() && empty($this->files)) { + if ($this->isGitMode() && ! boolval($this->files)) { throw new InvalidOptionException('You must supply at least one file or directory to run in git mode.'); } - if ($this->mode === Modes::SVN && empty($this->files)) { + if ($this->mode === Modes::SVN && ! boolval($this->files)) { throw new InvalidOptionException('You must supply at least one file or directory to run in svn mode.'); } } diff --git a/PhpcsChanged/FullReporter.php b/PhpcsChanged/FullReporter.php index 60e8fc0..dd33991 100644 --- a/PhpcsChanged/FullReporter.php +++ b/PhpcsChanged/FullReporter.php @@ -13,7 +13,7 @@ public function getFormattedMessages(PhpcsMessages $messages, array $options): s $files = array_unique(array_map(function(LintMessage $message): string { return $message->getFile() ?? 'STDIN'; }, $messages->getMessages())); - if (empty($files)) { + if (count($files) === 0) { $files = ['STDIN']; } @@ -52,7 +52,7 @@ private function getFormattedMessagesForFile(array $messages, string $file, arra $formattedLines = implode("\n", array_map(function(LintMessage $message) use ($longestNumber, $options): string { $source = $message->getSource() ?: 'Unknown'; - $sourceString = isset($options['s']) ? " ({$source})" : ''; + $sourceString = array_key_exists('s', $options) ? " ({$source})" : ''; return sprintf(" %{$longestNumber}d | %s | %s%s", $message->getLineNumber(), $message->getType(), $message->getMessage(), $sourceString); }, $messages)); diff --git a/PhpcsChanged/JsonReporter.php b/PhpcsChanged/JsonReporter.php index 891f76c..56d8e06 100644 --- a/PhpcsChanged/JsonReporter.php +++ b/PhpcsChanged/JsonReporter.php @@ -13,7 +13,7 @@ public function getFormattedMessages(PhpcsMessages $messages, array $options): s $files = array_unique(array_map(function(LintMessage $message): string { return $message->getFile() ?? 'STDIN'; }, $messages->getMessages())); - if (empty($files)) { + if (count($files) === 0) { $files = ['STDIN']; } @@ -42,7 +42,7 @@ public function getFormattedMessages(PhpcsMessages $messages, array $options): s 'files' => array_merge([], ...$outputByFile), ]; $output = json_encode($dataForJson, JSON_UNESCAPED_SLASHES); - if (! $output) { + if (! boolval($output)) { throw new \Exception('Failed to JSON-encode result messages'); } return $output; diff --git a/PhpcsChanged/LintMessages.php b/PhpcsChanged/LintMessages.php index 1bf6184..6d6af64 100644 --- a/PhpcsChanged/LintMessages.php +++ b/PhpcsChanged/LintMessages.php @@ -35,7 +35,7 @@ public static function merge(array $messages) { */ public static function fromLintMessages(array $messages, string $fileName = null) { return new static(array_map(function(LintMessage $message) use ($fileName) { - if ($fileName) { + if (is_string($fileName) && strlen($fileName) > 0) { $message->setFile($fileName); } return $message; diff --git a/PhpcsChanged/PhpcsMessagesHelpers.php b/PhpcsChanged/PhpcsMessagesHelpers.php index 27a8755..befc9b4 100644 --- a/PhpcsChanged/PhpcsMessagesHelpers.php +++ b/PhpcsChanged/PhpcsMessagesHelpers.php @@ -9,7 +9,7 @@ class PhpcsMessagesHelpers { public static function fromPhpcsJson(string $messages, string $forcedFileName = null): PhpcsMessages { - if (empty($messages)) { + if (! boolval($messages)) { return self::fromArrays([], $forcedFileName ?? 'STDIN'); } /** diff --git a/PhpcsChanged/UnixShell.php b/PhpcsChanged/UnixShell.php index a402fe8..55688b8 100644 --- a/PhpcsChanged/UnixShell.php +++ b/PhpcsChanged/UnixShell.php @@ -72,7 +72,7 @@ protected function validateExecutableExists(string $name, string $command): void } private function getPhpcsExecutable(): string { - if (! empty($this->options->phpcsPath) || ! empty(getenv('PHPCS'))) { + if (boolval($this->options->phpcsPath) || boolval(getenv('PHPCS'))) { return $this->options->getExecutablePath('phpcs'); } if (! $this->options->noVendorPhpcs && $this->doesPhpcsExistInVendor()) { @@ -237,18 +237,18 @@ public function getGitHashOfUnmodifiedFile(string $fileName): string { } private function getPhpcsStandardOption(): string { - $phpcsStandard = $this->options->phpcsStandard; - $phpcsStandardOption = $phpcsStandard ? ' --standard=' . escapeshellarg($phpcsStandard) : ''; - $warningSeverity = $this->options->warningSeverity; - $phpcsStandardOption .= isset($warningSeverity) ? ' --warning-severity=' . escapeshellarg($warningSeverity) : ''; - $errorSeverity = $this->options->errorSeverity; - $phpcsStandardOption .= isset($errorSeverity) ? ' --error-severity=' . escapeshellarg($errorSeverity) : ''; + $phpcsStandard = $this->options->phpcsStandard ?? ''; + $phpcsStandardOption = strlen($phpcsStandard) > 0 ? ' --standard=' . escapeshellarg($phpcsStandard) : ''; + $warningSeverity = $this->options->warningSeverity ?? ''; + $phpcsStandardOption .= strlen($warningSeverity) > 0 ? ' --warning-severity=' . escapeshellarg($warningSeverity) : ''; + $errorSeverity = $this->options->errorSeverity ?? ''; + $phpcsStandardOption .= strlen($errorSeverity) > 0 ? ' --error-severity=' . escapeshellarg($errorSeverity) : ''; return $phpcsStandardOption; } private function getPhpcsExtensionsOption(): string { - $phpcsExtensions = $this->options->phpcsExtensions; - $phpcsExtensionsOption = $phpcsExtensions ? ' --extensions=' . escapeshellarg($phpcsExtensions) : ''; + $phpcsExtensions = $this->options->phpcsExtensions ?? ''; + $phpcsExtensionsOption = strlen($phpcsExtensions) > 0 ? ' --extensions=' . escapeshellarg($phpcsExtensions) : ''; return $phpcsExtensionsOption; } @@ -274,7 +274,7 @@ public function getGitUnifiedDiff(string $fileName): string { $debug = getDebug($this->options->debug); $git = $this->options->getExecutablePath('git'); $objectOption = $this->options->mode === Modes::GIT_BASE ? ' ' . escapeshellarg($this->options->gitBase) . '...' : ''; - $stagedOption = empty($objectOption) && $this->options->mode !== Modes::GIT_UNSTAGED ? ' --staged' : ''; + $stagedOption = ! boolval($objectOption) && $this->options->mode !== Modes::GIT_UNSTAGED ? ' --staged' : ''; $unifiedDiffCommand = "{$git} diff{$stagedOption}{$objectOption} --no-prefix " . escapeshellarg($fileName); $debug('running diff command:', $unifiedDiffCommand); $unifiedDiff = $this->executeCommand($unifiedDiffCommand); @@ -346,12 +346,8 @@ public function doesUnmodifiedFileExistInSvn(string $fileName): bool { public function getSvnRevisionId(string $fileName): string { $svnFileInfo = $this->getSvnFileInfo($fileName); preg_match('/\bLast Changed Rev:\s([^\n]+)/', $svnFileInfo, $matches); - $version = $matches[1] ?? null; - if (! $version) { - // New files will not have a revision - return ''; - } - return $version; + // New files will not have a revision + return $matches[1] ?? ''; } private function getSvnFileInfo(string $fileName): string { @@ -421,7 +417,11 @@ public function getPhpcsVersion(): string { } $matched = preg_match('/version\\s([0-9.]+)/uim', $versionPhpcsOutput, $matches); - if (empty($matched) || empty($matches[1])) { + if ( + $matched === false + || count($matches) < 2 + || strlen($matches[1]) < 1 + ) { throw new ShellException("Cannot parse phpcs version output"); } diff --git a/PhpcsChanged/XmlReporter.php b/PhpcsChanged/XmlReporter.php index 6bf15c2..2c9a92a 100644 --- a/PhpcsChanged/XmlReporter.php +++ b/PhpcsChanged/XmlReporter.php @@ -31,7 +31,7 @@ public function getFormattedMessages(PhpcsMessages $messages, array $options): s $files = array_unique(array_map(function(LintMessage $message): string { return $message->getFile() ?? 'STDIN'; }, $messages->getMessages())); - if (empty($files)) { + if (count($files) === 0) { $files = ['STDIN']; }