From c8615358cb83d695473d350dcf7f82ef35e287e9 Mon Sep 17 00:00:00 2001 From: Vladimir Reznichenko Date: Mon, 11 Sep 2017 19:40:43 +0200 Subject: [PATCH 1/2] SCA with PHP Inspections (EA Extended) --- src/Composer/Autoload/AutoloadGenerator.php | 4 ++-- src/Composer/Command/RunScriptCommand.php | 2 +- src/Composer/Command/SuggestsCommand.php | 2 +- src/Composer/Command/UpdateCommand.php | 3 ++- .../DependencyResolver/RuleSetGenerator.php | 2 +- .../DependencyResolver/RuleSetIterator.php | 6 +++--- .../DependencyResolver/Transaction.php | 6 +++--- .../Downloader/PerforceDownloader.php | 4 +--- src/Composer/Factory.php | 4 ++-- src/Composer/Json/JsonFile.php | 4 +--- src/Composer/Repository/BaseRepository.php | 2 +- .../Repository/Pear/ChannelRest10Reader.php | 3 +-- src/Composer/Repository/Vcs/GitDriver.php | 6 +----- .../Repository/Vcs/PerforceDriver.php | 8 ++----- src/Composer/Util/Perforce.php | 21 +++++++------------ src/Composer/Util/RemoteFilesystem.php | 6 +----- src/Composer/Util/Silencer.php | 2 +- src/Composer/Util/StreamContextFactory.php | 2 +- ...load_real_functions_with_include_paths.php | 2 +- .../Package/Archiver/ArchiveManagerTest.php | 4 +--- .../Test/Package/Loader/ArrayLoaderTest.php | 5 +---- tests/Composer/Test/Util/GitHubTest.php | 6 ++---- tests/Composer/Test/Util/GitLabTest.php | 4 +--- 23 files changed, 38 insertions(+), 70 deletions(-) diff --git a/src/Composer/Autoload/AutoloadGenerator.php b/src/Composer/Autoload/AutoloadGenerator.php index 519e5bea5..9b9710d9e 100644 --- a/src/Composer/Autoload/AutoloadGenerator.php +++ b/src/Composer/Autoload/AutoloadGenerator.php @@ -199,7 +199,7 @@ EOF; $targetDirLoader = null; $mainAutoload = $mainPackage->getAutoload(); if ($mainPackage->getTargetDir() && !empty($mainAutoload['psr-0'])) { - $levels = count(explode('/', $filesystem->normalizePath($mainPackage->getTargetDir()))); + $levels = substr_count($filesystem->normalizePath($mainPackage->getTargetDir()), '/') + 1; $prefixes = implode(', ', array_map(function ($prefix) { return var_export($prefix, true); }, array_keys($mainAutoload['psr-0']))); @@ -601,7 +601,7 @@ HEADER; if ($useIncludePath) { $file .= <<<'INCLUDE_PATH' $includePaths = require __DIR__ . '/include_paths.php'; - array_push($includePaths, get_include_path()); + $includePaths[] = get_include_path(); set_include_path(implode(PATH_SEPARATOR, $includePaths)); diff --git a/src/Composer/Command/RunScriptCommand.php b/src/Composer/Command/RunScriptCommand.php index 5696dd331..929056946 100644 --- a/src/Composer/Command/RunScriptCommand.php +++ b/src/Composer/Command/RunScriptCommand.php @@ -90,7 +90,7 @@ EOT $args = $input->getArgument('args'); - if (!is_null($timeout = $input->getOption('timeout'))) { + if (null !== $timeout = $input->getOption('timeout')) { if (!ctype_digit($timeout)) { throw new \RuntimeException('Timeout value must be numeric and positive if defined, or 0 for forever'); } diff --git a/src/Composer/Command/SuggestsCommand.php b/src/Composer/Command/SuggestsCommand.php index f3466c63b..8461fe7db 100644 --- a/src/Composer/Command/SuggestsCommand.php +++ b/src/Composer/Command/SuggestsCommand.php @@ -88,7 +88,7 @@ EOT continue; } foreach ($package['suggest'] as $suggestion => $reason) { - if (false === strpos('/', $suggestion) && !is_null($platform->findPackage($suggestion, '*'))) { + if (false === strpos('/', $suggestion) && null !== $platform->findPackage($suggestion, '*')) { continue; } if (!isset($installed[$suggestion])) { diff --git a/src/Composer/Command/UpdateCommand.php b/src/Composer/Command/UpdateCommand.php index 3c456ad59..a05c813ff 100644 --- a/src/Composer/Command/UpdateCommand.php +++ b/src/Composer/Command/UpdateCommand.php @@ -171,7 +171,8 @@ EOT ); $autocompleterValues = array(); foreach ($requires as $require) { - $autocompleterValues[strtolower($require->getTarget())] = $require->getTarget(); + $target = $require->getTarget(); + $autocompleterValues[strtolower($target)] = $target; } $installedPackages = $composer->getRepositoryManager()->getLocalRepository()->getPackages(); diff --git a/src/Composer/DependencyResolver/RuleSetGenerator.php b/src/Composer/DependencyResolver/RuleSetGenerator.php index 867aa812e..68d6f73bb 100644 --- a/src/Composer/DependencyResolver/RuleSetGenerator.php +++ b/src/Composer/DependencyResolver/RuleSetGenerator.php @@ -283,7 +283,7 @@ class RuleSetGenerator switch ($job['cmd']) { case 'install': if (!$job['fixed'] && $ignorePlatformReqs && preg_match(PlatformRepository::PLATFORM_PACKAGE_REGEX, $job['packageName'])) { - continue; + break; // or `continue 2` ? } $packages = $this->pool->whatProvides($job['packageName'], $job['constraint']); diff --git a/src/Composer/DependencyResolver/RuleSetIterator.php b/src/Composer/DependencyResolver/RuleSetIterator.php index 63563ae51..8c048624f 100644 --- a/src/Composer/DependencyResolver/RuleSetIterator.php +++ b/src/Composer/DependencyResolver/RuleSetIterator.php @@ -51,7 +51,7 @@ class RuleSetIterator implements \Iterator return; } - if ($this->currentOffset >= sizeof($this->rules[$this->currentType])) { + if ($this->currentOffset >= count($this->rules[$this->currentType])) { $this->currentOffset = 0; do { @@ -63,7 +63,7 @@ class RuleSetIterator implements \Iterator } $this->currentType = $this->types[$this->currentTypeOffset]; - } while (isset($this->types[$this->currentTypeOffset]) && !sizeof($this->rules[$this->currentType])); + } while (isset($this->types[$this->currentTypeOffset]) && !count($this->rules[$this->currentType])); } } @@ -83,7 +83,7 @@ class RuleSetIterator implements \Iterator } $this->currentType = $this->types[$this->currentTypeOffset]; - } while (isset($this->types[$this->currentTypeOffset]) && !sizeof($this->rules[$this->currentType])); + } while (isset($this->types[$this->currentTypeOffset]) && !count($this->rules[$this->currentType])); } public function valid() diff --git a/src/Composer/DependencyResolver/Transaction.php b/src/Composer/DependencyResolver/Transaction.php index bb60dccc2..3674a1989 100644 --- a/src/Composer/DependencyResolver/Transaction.php +++ b/src/Composer/DependencyResolver/Transaction.php @@ -111,16 +111,16 @@ class Transaction $packageId = $package->id; if (!isset($visited[$packageId])) { - array_push($queue, $package); + $queue[] = $package; if ($package instanceof AliasPackage) { - array_push($queue, $package->getAliasOf()); + $queue[] = $package->getAliasOf(); } else { foreach ($package->getRequires() as $link) { $possibleRequires = $this->pool->whatProvides($link->getTarget(), $link->getConstraint()); foreach ($possibleRequires as $require) { - array_push($queue, $require); + $queue[] = $require; } } } diff --git a/src/Composer/Downloader/PerforceDownloader.php b/src/Composer/Downloader/PerforceDownloader.php index 09980e8e9..a472b84c6 100644 --- a/src/Composer/Downloader/PerforceDownloader.php +++ b/src/Composer/Downloader/PerforceDownloader.php @@ -96,9 +96,7 @@ class PerforceDownloader extends VcsDownloader */ protected function getCommitLogs($fromReference, $toReference, $path) { - $commitLogs = $this->perforce->getCommitLogs($fromReference, $toReference); - - return $commitLogs; + return $this->perforce->getCommitLogs($fromReference, $toReference); } public function setPerforce($perforce) diff --git a/src/Composer/Factory.php b/src/Composer/Factory.php index 42be783c8..23eaaab09 100644 --- a/src/Composer/Factory.php +++ b/src/Composer/Factory.php @@ -204,7 +204,7 @@ class Factory if ($composerAuthEnv = getenv('COMPOSER_AUTH')) { $authData = json_decode($composerAuthEnv, true); - if (is_null($authData)) { + if (null === $authData) { throw new \UnexpectedValueException('COMPOSER_AUTH environment variable is malformed, should be a valid JSON object'); } @@ -437,7 +437,7 @@ class Factory { $composer = null; try { - $composer = self::createComposer($io, $config->get('home') . '/composer.json', $disablePlugins, $config->get('home'), $fullLoad); + $composer = $this->createComposer($io, $config->get('home') . '/composer.json', $disablePlugins, $config->get('home'), $fullLoad); } catch (\Exception $e) { $io->writeError('Failed to initialize global composer: '.$e->getMessage(), true, IOInterface::DEBUG); } diff --git a/src/Composer/Json/JsonFile.php b/src/Composer/Json/JsonFile.php index 1a33992f3..0557847d6 100644 --- a/src/Composer/Json/JsonFile.php +++ b/src/Composer/Json/JsonFile.php @@ -223,9 +223,7 @@ class JsonFile return $json; } - $result = JsonFormatter::format($json, $unescapeUnicode, $unescapeSlashes); - - return $result; + return JsonFormatter::format($json, $unescapeUnicode, $unescapeSlashes); } /** diff --git a/src/Composer/Repository/BaseRepository.php b/src/Composer/Repository/BaseRepository.php index 37321c713..f5233e197 100644 --- a/src/Composer/Repository/BaseRepository.php +++ b/src/Composer/Repository/BaseRepository.php @@ -78,7 +78,7 @@ abstract class BaseRepository implements RepositoryInterface foreach ($links as $link) { foreach ($needles as $needle) { if ($link->getTarget() === $needle) { - if (is_null($constraint) || (($link->getConstraint()->matches($constraint) === !$invert))) { + if ($constraint === null || ($link->getConstraint()->matches($constraint) === !$invert)) { // already displayed this node's dependencies, cutting short if (in_array($link->getSource(), $packagesInTree)) { $results[$link->getSource()] = array($package, $link, false); diff --git a/src/Composer/Repository/Pear/ChannelRest10Reader.php b/src/Composer/Repository/Pear/ChannelRest10Reader.php index 5b5fd7828..92498dae9 100644 --- a/src/Composer/Repository/Pear/ChannelRest10Reader.php +++ b/src/Composer/Repository/Pear/ChannelRest10Reader.php @@ -158,8 +158,7 @@ class ChannelRest10Reader extends BaseChannelReader $depthPath = '/r/' . strtolower($packageName) . '/deps.' . $version . '.txt'; $content = $this->requestContent($baseUrl, $depthPath); $dependencyArray = unserialize($content); - $result = $dependencyReader->buildDependencyInfo($dependencyArray); - return $result; + return $dependencyReader->buildDependencyInfo($dependencyArray); } } diff --git a/src/Composer/Repository/Vcs/GitDriver.php b/src/Composer/Repository/Vcs/GitDriver.php index 0f2c95714..6cb8be91a 100644 --- a/src/Composer/Repository/Vcs/GitDriver.php +++ b/src/Composer/Repository/Vcs/GitDriver.php @@ -216,10 +216,6 @@ class GitDriver extends VcsDriver } $process = new ProcessExecutor($io); - if ($process->execute('git ls-remote --heads ' . ProcessExecutor::escape($url), $output) === 0) { - return true; - } - - return false; + return $process->execute('git ls-remote --heads ' . ProcessExecutor::escape($url), $output) === 0; } } diff --git a/src/Composer/Repository/Vcs/PerforceDriver.php b/src/Composer/Repository/Vcs/PerforceDriver.php index f9c6937ec..667f914df 100644 --- a/src/Composer/Repository/Vcs/PerforceDriver.php +++ b/src/Composer/Repository/Vcs/PerforceDriver.php @@ -87,9 +87,7 @@ class PerforceDriver extends VcsDriver */ public function getBranches() { - $branches = $this->perforce->getBranches(); - - return $branches; + return $this->perforce->getBranches(); } /** @@ -97,9 +95,7 @@ class PerforceDriver extends VcsDriver */ public function getTags() { - $tags = $this->perforce->getTags(); - - return $tags; + return $this->perforce->getTags(); } /** diff --git a/src/Composer/Util/Perforce.php b/src/Composer/Util/Perforce.php index 364f2394f..1ae63d70f 100644 --- a/src/Composer/Util/Perforce.php +++ b/src/Composer/Util/Perforce.php @@ -116,16 +116,14 @@ class Perforce protected function executeCommand($command) { - $this->commandResult = ""; - $exit_code = $this->process->execute($command, $this->commandResult); - - return $exit_code; + $this->commandResult = ''; + return $this->process->execute($command, $this->commandResult); } public function getClient() { if (!isset($this->p4Client)) { - $cleanStreamName = str_replace('@', '', str_replace('/', '_', str_replace('//', '', $this->getStream()))); + $cleanStreamName = str_replace(array('//', '/', '@'), array('', '_', ''), $this->getStream()); $this->p4Client = 'composer_perforce_' . $this->uniquePerforceClientName . '_' . $cleanStreamName; } @@ -189,9 +187,7 @@ class Perforce public function getP4ClientSpec() { - $p4clientSpec = $this->path . '/' . $this->getClient() . '.p4.spec'; - - return $p4clientSpec; + return $this->path . '/' . $this->getClient() . '.p4.spec'; } public function getUser() @@ -276,8 +272,7 @@ class Perforce if ($useClient) { $p4Command = $p4Command . '-c ' . $this->getClient() . ' '; } - $p4Command = $p4Command . '-p ' . $this->getPort() . ' '; - $p4Command = $p4Command . $command; + $p4Command = $p4Command . '-p ' . $this->getPort() . ' ' . $command; return $p4Command; } @@ -538,9 +533,8 @@ class Perforce return null; } $fields = explode(' ', $changes); - $changeList = $fields[1]; - return $changeList; + return $fields[1]; } /** @@ -562,9 +556,8 @@ class Perforce $main = substr($fromReference, 0, $index) . '/...'; $command = $this->generateP4Command('filelog ' . $main . '@' . $fromChangeList. ',' . $toChangeList); $this->executeCommand($command); - $result = $this->commandResult; - return $result; + return $this->commandResult; } public function getFilesystem() diff --git a/src/Composer/Util/RemoteFilesystem.php b/src/Composer/Util/RemoteFilesystem.php index 7b0d650bd..aff781bc1 100644 --- a/src/Composer/Util/RemoteFilesystem.php +++ b/src/Composer/Util/RemoteFilesystem.php @@ -1031,10 +1031,6 @@ class RemoteFilesystem // Path for a public download follows this pattern /{user}/{repo}/downloads/{whatever} // {@link https://blog.bitbucket.org/2009/04/12/new-feature-downloads/} $pathParts = explode('/', $path); - if (count($pathParts) >= 4 && $pathParts[3] == 'downloads') { - return true; - } - - return false; + return count($pathParts) >= 4 && $pathParts[3] == 'downloads'; } } diff --git a/src/Composer/Util/Silencer.php b/src/Composer/Util/Silencer.php index 69a065dd6..dcb362b52 100644 --- a/src/Composer/Util/Silencer.php +++ b/src/Composer/Util/Silencer.php @@ -36,7 +36,7 @@ class Silencer $mask = E_WARNING | E_NOTICE | E_USER_WARNING | E_USER_NOTICE | E_DEPRECATED | E_USER_DEPRECATED | E_STRICT; } $old = error_reporting(); - array_push(self::$stack, $old); + self::$stack[] = $old; error_reporting($old & ~$mask); return $old; diff --git a/src/Composer/Util/StreamContextFactory.php b/src/Composer/Util/StreamContextFactory.php index 0a6764418..911bb36a1 100644 --- a/src/Composer/Util/StreamContextFactory.php +++ b/src/Composer/Util/StreamContextFactory.php @@ -139,7 +139,7 @@ final class StreamContextFactory $phpVersion = 'PHP ' . PHP_MAJOR_VERSION . '.' . PHP_MINOR_VERSION . '.' . PHP_RELEASE_VERSION; } - if (!isset($options['http']['header']) || false === strpos(strtolower(implode('', $options['http']['header'])), 'user-agent')) { + if (!isset($options['http']['header']) || false === stripos(implode('', $options['http']['header']), 'user-agent')) { $options['http']['header'][] = sprintf( 'User-Agent: Composer/%s (%s; %s; %s%s)', Composer::VERSION === '@package_version@' ? 'source' : Composer::VERSION, diff --git a/tests/Composer/Test/Autoload/Fixtures/autoload_real_functions_with_include_paths.php b/tests/Composer/Test/Autoload/Fixtures/autoload_real_functions_with_include_paths.php index f1ba4a011..6459cac05 100644 --- a/tests/Composer/Test/Autoload/Fixtures/autoload_real_functions_with_include_paths.php +++ b/tests/Composer/Test/Autoload/Fixtures/autoload_real_functions_with_include_paths.php @@ -24,7 +24,7 @@ class ComposerAutoloaderInitFilesAutoload spl_autoload_unregister(array('ComposerAutoloaderInitFilesAutoload', 'loadClassLoader')); $includePaths = require __DIR__ . '/include_paths.php'; - array_push($includePaths, get_include_path()); + $includePaths[] = get_include_path(); set_include_path(implode(PATH_SEPARATOR, $includePaths)); $useStaticLoader = PHP_VERSION_ID >= 50600 && !defined('HHVM_VERSION') && (!function_exists('zend_loader_file_encoded') || !zend_loader_file_encoded()); diff --git a/tests/Composer/Test/Package/Archiver/ArchiveManagerTest.php b/tests/Composer/Test/Package/Archiver/ArchiveManagerTest.php index fa17774ca..f9fe308fa 100644 --- a/tests/Composer/Test/Package/Archiver/ArchiveManagerTest.php +++ b/tests/Composer/Test/Package/Archiver/ArchiveManagerTest.php @@ -92,9 +92,7 @@ class ArchiveManagerTest extends ArchiverTest $packageName = $fileName; } - $target = $this->targetDir.'/'.$packageName.'.'.$format; - - return $target; + return $this->targetDir.'/'.$packageName.'.'.$format; } /** diff --git a/tests/Composer/Test/Package/Loader/ArrayLoaderTest.php b/tests/Composer/Test/Package/Loader/ArrayLoaderTest.php index 4093f4381..fe27caea4 100644 --- a/tests/Composer/Test/Package/Loader/ArrayLoaderTest.php +++ b/tests/Composer/Test/Package/Loader/ArrayLoaderTest.php @@ -126,10 +126,7 @@ class ArrayLoaderTest extends \PHPUnit_Framework_TestCase 'abandoned' => 'foo/bar', ); - $validTestArguments = array($validConfig); - $argumentsToProvide = array($validTestArguments); - - return $argumentsToProvide; + return array(array($validConfig)); } protected function fixConfigWhenLoadConfigIsFalse($config) diff --git a/tests/Composer/Test/Util/GitHubTest.php b/tests/Composer/Test/Util/GitHubTest.php index c7d4061e1..50e14690a 100644 --- a/tests/Composer/Test/Util/GitHubTest.php +++ b/tests/Composer/Test/Util/GitHubTest.php @@ -54,7 +54,7 @@ class GitHubTest extends \PHPUnit_Framework_TestCase $this->isFalse(), $this->anything() ) - ->willReturn(sprintf('{}', $this->token)) + ->willReturn('{}') ; $config = $this->getConfigMock(); @@ -116,9 +116,7 @@ class GitHubTest extends \PHPUnit_Framework_TestCase private function getConfigMock() { - $config = $this->getMock('Composer\Config'); - - return $config; + return $this->getMock('Composer\Config'); } private function getRemoteFilesystemMock() diff --git a/tests/Composer/Test/Util/GitLabTest.php b/tests/Composer/Test/Util/GitLabTest.php index e4d264055..1c359b653 100644 --- a/tests/Composer/Test/Util/GitLabTest.php +++ b/tests/Composer/Test/Util/GitLabTest.php @@ -125,9 +125,7 @@ class GitLabTest extends \PHPUnit_Framework_TestCase private function getConfigMock() { - $config = $this->getMock('Composer\Config'); - - return $config; + return $this->getMock('Composer\Config'); } private function getRemoteFilesystemMock() From c0eb32669a1ffa0c7ec4678e928f6de69d28c8c0 Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Tue, 12 Sep 2017 09:24:28 +0200 Subject: [PATCH 2/2] Remove comment --- src/Composer/DependencyResolver/RuleSetGenerator.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Composer/DependencyResolver/RuleSetGenerator.php b/src/Composer/DependencyResolver/RuleSetGenerator.php index 68d6f73bb..2cf150a33 100644 --- a/src/Composer/DependencyResolver/RuleSetGenerator.php +++ b/src/Composer/DependencyResolver/RuleSetGenerator.php @@ -283,7 +283,7 @@ class RuleSetGenerator switch ($job['cmd']) { case 'install': if (!$job['fixed'] && $ignorePlatformReqs && preg_match(PlatformRepository::PLATFORM_PACKAGE_REGEX, $job['packageName'])) { - break; // or `continue 2` ? + break; } $packages = $this->pool->whatProvides($job['packageName'], $job['constraint']);