From da9e00066c480dc0deccac356af6b02051f6f52f Mon Sep 17 00:00:00 2001 From: Vladimir Reznichenko Date: Wed, 24 Jan 2018 10:19:46 +0100 Subject: [PATCH 1/2] SCA: reduced repetitive methods references, used specialized PhpUnit assertions --- src/Composer/Command/ArchiveCommand.php | 5 +++-- src/Composer/Command/BaseDependencyCommand.php | 5 +++-- .../Command/CheckPlatformReqsCommand.php | 5 +++-- src/Composer/Command/LicensesCommand.php | 5 +++-- src/Composer/Command/RunScriptCommand.php | 5 +++-- src/Composer/Util/StreamContextFactory.php | 2 +- tests/Composer/Test/Util/FilesystemTest.php | 16 ++++++++-------- 7 files changed, 24 insertions(+), 19 deletions(-) diff --git a/src/Composer/Command/ArchiveCommand.php b/src/Composer/Command/ArchiveCommand.php index bf64ca47e..4ab1e4905 100644 --- a/src/Composer/Command/ArchiveCommand.php +++ b/src/Composer/Command/ArchiveCommand.php @@ -66,8 +66,9 @@ EOT $composer = $this->getComposer(false); if ($composer) { $commandEvent = new CommandEvent(PluginEvents::COMMAND, 'archive', $input, $output); - $composer->getEventDispatcher()->dispatch($commandEvent->getName(), $commandEvent); - $composer->getEventDispatcher()->dispatchScript(ScriptEvents::PRE_ARCHIVE_CMD); + $eventDispatcher = $composer->getEventDispatcher(); + $eventDispatcher->dispatch($commandEvent->getName(), $commandEvent); + $eventDispatcher->dispatchScript(ScriptEvents::PRE_ARCHIVE_CMD); } if (null === $input->getOption('format')) { diff --git a/src/Composer/Command/BaseDependencyCommand.php b/src/Composer/Command/BaseDependencyCommand.php index e36663558..980ca9282 100644 --- a/src/Composer/Command/BaseDependencyCommand.php +++ b/src/Composer/Command/BaseDependencyCommand.php @@ -180,8 +180,9 @@ class BaseDependencyCommand extends BaseCommand // Render table $renderer = new Table($output); $renderer->setStyle('compact'); - $renderer->getStyle()->setVerticalBorderChar(''); - $renderer->getStyle()->setCellRowContentFormat('%s '); + $rendererStyle = $renderer->getStyle(); + $rendererStyle->setVerticalBorderChar(''); + $rendererStyle->setCellRowContentFormat('%s '); $renderer->setRows($table)->render(); } diff --git a/src/Composer/Command/CheckPlatformReqsCommand.php b/src/Composer/Command/CheckPlatformReqsCommand.php index 8b59c28ea..6a75ab4dc 100644 --- a/src/Composer/Command/CheckPlatformReqsCommand.php +++ b/src/Composer/Command/CheckPlatformReqsCommand.php @@ -142,8 +142,9 @@ EOT // Render table $renderer = new Table($output); $renderer->setStyle('compact'); - $renderer->getStyle()->setVerticalBorderChar(''); - $renderer->getStyle()->setCellRowContentFormat('%s '); + $rendererStyle = $renderer->getStyle(); + $rendererStyle->setVerticalBorderChar(''); + $rendererStyle->setCellRowContentFormat('%s '); $renderer->setRows($table)->render(); } } diff --git a/src/Composer/Command/LicensesCommand.php b/src/Composer/Command/LicensesCommand.php index 0c4537be1..f3a71eb39 100644 --- a/src/Composer/Command/LicensesCommand.php +++ b/src/Composer/Command/LicensesCommand.php @@ -74,8 +74,9 @@ EOT $table = new Table($output); $table->setStyle('compact'); - $table->getStyle()->setVerticalBorderChar(''); - $table->getStyle()->setCellRowContentFormat('%s '); + $tableStyle = $table->getStyle(); + $tableStyle->setVerticalBorderChar(''); + $tableStyle->setCellRowContentFormat('%s '); $table->setHeaders(array('Name', 'Version', 'License')); foreach ($packages as $package) { $table->addRow(array( diff --git a/src/Composer/Command/RunScriptCommand.php b/src/Composer/Command/RunScriptCommand.php index d78662e23..ff0c79361 100644 --- a/src/Composer/Command/RunScriptCommand.php +++ b/src/Composer/Command/RunScriptCommand.php @@ -124,8 +124,9 @@ EOT $renderer = new Table($output); $renderer->setStyle('compact'); - $renderer->getStyle()->setVerticalBorderChar(''); - $renderer->getStyle()->setCellRowContentFormat('%s '); + $rendererStyle = $renderer->getStyle(); + $rendererStyle->setVerticalBorderChar(''); + $rendererStyle->setCellRowContentFormat('%s '); $renderer->setRows($table)->render(); return 0; diff --git a/src/Composer/Util/StreamContextFactory.php b/src/Composer/Util/StreamContextFactory.php index b8290086c..6b16c8a18 100644 --- a/src/Composer/Util/StreamContextFactory.php +++ b/src/Composer/Util/StreamContextFactory.php @@ -169,7 +169,7 @@ final class StreamContextFactory $header = explode("\r\n", $header); } uasort($header, function ($el) { - return preg_match('{^content-type}i', $el) ? 1 : -1; + return stripos($el, 'content-type') === 0 ? 1 : -1; }); return $header; diff --git a/tests/Composer/Test/Util/FilesystemTest.php b/tests/Composer/Test/Util/FilesystemTest.php index ef3ab5bbe..5a9adbae7 100644 --- a/tests/Composer/Test/Util/FilesystemTest.php +++ b/tests/Composer/Test/Util/FilesystemTest.php @@ -307,9 +307,9 @@ class FilesystemTest extends TestCase $this->assertTrue($fs->isJunction($junction . '/../junction')); // Remove junction - $this->assertTrue(is_dir($junction)); + $this->assertDirectoryExists($junction); $this->assertTrue($fs->removeJunction($junction)); - $this->assertFalse(is_dir($junction)); + $this->assertDirectoryNotExists($junction); } public function testCopy() @@ -325,9 +325,9 @@ class FilesystemTest extends TestCase $result1 = $fs->copy($this->workingDir . '/foo', $this->workingDir . '/foop'); $this->assertTrue($result1, 'Copying directory failed.'); - $this->assertTrue(is_dir($this->workingDir . '/foop'), 'Not a directory: ' . $this->workingDir . '/foop'); - $this->assertTrue(is_dir($this->workingDir . '/foop/bar'), 'Not a directory: ' . $this->workingDir . '/foop/bar'); - $this->assertTrue(is_dir($this->workingDir . '/foop/baz'), 'Not a directory: ' . $this->workingDir . '/foop/baz'); + $this->assertDirectoryExists($this->workingDir . '/foop', 'Not a directory: ' . $this->workingDir . '/foop'); + $this->assertDirectoryExists($this->workingDir . '/foop/bar', 'Not a directory: ' . $this->workingDir . '/foop/bar'); + $this->assertDirectoryExists($this->workingDir . '/foop/baz', 'Not a directory: ' . $this->workingDir . '/foop/baz'); $this->assertTrue(is_file($this->workingDir . '/foop/foo.file'), 'Not a file: ' . $this->workingDir . '/foop/foo.file'); $this->assertTrue(is_file($this->workingDir . '/foop/bar/foobar.file'), 'Not a file: ' . $this->workingDir . '/foop/bar/foobar.file'); $this->assertTrue(is_file($this->workingDir . '/foop/baz/foobaz.file'), 'Not a file: ' . $this->workingDir . '/foop/baz/foobaz.file'); @@ -355,8 +355,8 @@ class FilesystemTest extends TestCase $this->assertFalse(is_file($this->workingDir . '/foo/baz/foobaz.file'), 'Still a file: ' . $this->workingDir . '/foo/baz/foobaz.file'); $this->assertFalse(is_file($this->workingDir . '/foo/bar/foobar.file'), 'Still a file: ' . $this->workingDir . '/foo/bar/foobar.file'); $this->assertFalse(is_file($this->workingDir . '/foo/foo.file'), 'Still a file: ' . $this->workingDir . '/foo/foo.file'); - $this->assertFalse(is_dir($this->workingDir . '/foo/baz'), 'Still a directory: ' . $this->workingDir . '/foo/baz'); - $this->assertFalse(is_dir($this->workingDir . '/foo/bar'), 'Still a directory: ' . $this->workingDir . '/foo/bar'); - $this->assertFalse(is_dir($this->workingDir . '/foo'), 'Still a directory: ' . $this->workingDir . '/foo'); + $this->assertDirectoryNotExists($this->workingDir . '/foo/baz', 'Still a directory: ' . $this->workingDir . '/foo/baz'); + $this->assertDirectoryNotExists($this->workingDir . '/foo/bar', 'Still a directory: ' . $this->workingDir . '/foo/bar'); + $this->assertDirectoryNotExists($this->workingDir . '/foo', 'Still a directory: ' . $this->workingDir . '/foo'); } } From db0cae04773377e678c600ad20fc4e0f0da3bd53 Mon Sep 17 00:00:00 2001 From: Vladimir Reznichenko Date: Wed, 24 Jan 2018 10:31:39 +0100 Subject: [PATCH 2/2] SCA: revert assertions tweaks --- tests/Composer/Test/Util/FilesystemTest.php | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/tests/Composer/Test/Util/FilesystemTest.php b/tests/Composer/Test/Util/FilesystemTest.php index 5a9adbae7..ef3ab5bbe 100644 --- a/tests/Composer/Test/Util/FilesystemTest.php +++ b/tests/Composer/Test/Util/FilesystemTest.php @@ -307,9 +307,9 @@ class FilesystemTest extends TestCase $this->assertTrue($fs->isJunction($junction . '/../junction')); // Remove junction - $this->assertDirectoryExists($junction); + $this->assertTrue(is_dir($junction)); $this->assertTrue($fs->removeJunction($junction)); - $this->assertDirectoryNotExists($junction); + $this->assertFalse(is_dir($junction)); } public function testCopy() @@ -325,9 +325,9 @@ class FilesystemTest extends TestCase $result1 = $fs->copy($this->workingDir . '/foo', $this->workingDir . '/foop'); $this->assertTrue($result1, 'Copying directory failed.'); - $this->assertDirectoryExists($this->workingDir . '/foop', 'Not a directory: ' . $this->workingDir . '/foop'); - $this->assertDirectoryExists($this->workingDir . '/foop/bar', 'Not a directory: ' . $this->workingDir . '/foop/bar'); - $this->assertDirectoryExists($this->workingDir . '/foop/baz', 'Not a directory: ' . $this->workingDir . '/foop/baz'); + $this->assertTrue(is_dir($this->workingDir . '/foop'), 'Not a directory: ' . $this->workingDir . '/foop'); + $this->assertTrue(is_dir($this->workingDir . '/foop/bar'), 'Not a directory: ' . $this->workingDir . '/foop/bar'); + $this->assertTrue(is_dir($this->workingDir . '/foop/baz'), 'Not a directory: ' . $this->workingDir . '/foop/baz'); $this->assertTrue(is_file($this->workingDir . '/foop/foo.file'), 'Not a file: ' . $this->workingDir . '/foop/foo.file'); $this->assertTrue(is_file($this->workingDir . '/foop/bar/foobar.file'), 'Not a file: ' . $this->workingDir . '/foop/bar/foobar.file'); $this->assertTrue(is_file($this->workingDir . '/foop/baz/foobaz.file'), 'Not a file: ' . $this->workingDir . '/foop/baz/foobaz.file'); @@ -355,8 +355,8 @@ class FilesystemTest extends TestCase $this->assertFalse(is_file($this->workingDir . '/foo/baz/foobaz.file'), 'Still a file: ' . $this->workingDir . '/foo/baz/foobaz.file'); $this->assertFalse(is_file($this->workingDir . '/foo/bar/foobar.file'), 'Still a file: ' . $this->workingDir . '/foo/bar/foobar.file'); $this->assertFalse(is_file($this->workingDir . '/foo/foo.file'), 'Still a file: ' . $this->workingDir . '/foo/foo.file'); - $this->assertDirectoryNotExists($this->workingDir . '/foo/baz', 'Still a directory: ' . $this->workingDir . '/foo/baz'); - $this->assertDirectoryNotExists($this->workingDir . '/foo/bar', 'Still a directory: ' . $this->workingDir . '/foo/bar'); - $this->assertDirectoryNotExists($this->workingDir . '/foo', 'Still a directory: ' . $this->workingDir . '/foo'); + $this->assertFalse(is_dir($this->workingDir . '/foo/baz'), 'Still a directory: ' . $this->workingDir . '/foo/baz'); + $this->assertFalse(is_dir($this->workingDir . '/foo/bar'), 'Still a directory: ' . $this->workingDir . '/foo/bar'); + $this->assertFalse(is_dir($this->workingDir . '/foo'), 'Still a directory: ' . $this->workingDir . '/foo'); } }