Simplify SecurityAdvisory, do not expect or use passed-in instance of checker as it accomplishes nothing, fix CS.

Author: Thinkscape

src/ZendDiagnostics/Check/SecurityAdvisory.php

@@ -12,7 +12,7 @@
 use ZendDiagnostics\Result\Warning;
 
 /**
- * Checks installed dependencies against the SensioLabs Security Advisory database.
+ * Checks installed composer dependencies against the SensioLabs Security Advisory database.
  */
 class SecurityAdvisory extends AbstractCheck
 {
@@ -27,26 +27,21 @@ class SecurityAdvisory extends AbstractCheck
     protected $securityChecker;
 
     /**
-     * @param  SecurityChecker|null $securityChecker An instance of SecurityChecker
-     * @param  string               $lockFilePath    Path to composer.lock
-     * @throws \InvalidArgumentException
+     * @param  string $lockFilePath Path to composer.lock
+     * @throws InvalidArgumentException
      */
-    public function __construct(SecurityChecker $securityChecker = null, $lockFilePath = null)
+    public function __construct($lockFilePath = null)
     {
-        if(!$securityChecker) {
-            if(!class_exists('SensioLabs\Security\SecurityChecker')) {
-                throw new InvalidArgumentException(sprintf(
-                    'Unable to find "%s" class. Please install "%s" library to use this Check.',
-                    'SensioLabs\Security\SecurityChecker',
-                    'sensiolabs/security-checker'
-                ));
-            }
-
-            $securityChecker = new SecurityChecker();
+        if (!class_exists('SensioLabs\Security\SecurityChecker')) {
+            throw new InvalidArgumentException(sprintf(
+                'Unable to find "%s" class. Please install "%s" library to use this Check.',
+                'SensioLabs\Security\SecurityChecker',
+                'sensiolabs/security-checker'
+            ));
         }
 
-        if(!$lockFilePath) {
-            if(!file_exists('composer.lock')) {
+        if (!$lockFilePath) {
+            if (!file_exists('composer.lock')) {
                 throw new InvalidArgumentException(
                     'You have not provided lock file path and there is no "composer.lock" file in current directory.'
                 );
@@ -60,8 +55,8 @@ public function __construct(SecurityChecker $securityChecker = null, $lockFilePa
             ));
         }
 
-        $this->lockFilePath = $lockFilePath;
-        $this->securityChecker = $securityChecker;
+        $this->lockFilePath    = $lockFilePath;
+        $this->securityChecker = new SecurityChecker();
     }
 
     public function check()

tests/ZendDiagnosticsTest/ChecksTest.php

@@ -11,9 +11,9 @@
 use ZendDiagnostics\Check\ExtensionLoaded;
 use ZendDiagnostics\Check\PhpVersion;
 use ZendDiagnostics\Check\ProcessRunning;
-use ZendDiagnostics\Check\SecurityAdvisory;
 use ZendDiagnostics\Check\StreamWrapperExists;
 use ZendDiagnostics\Result\Success;
+use ZendDiagnosticsTest\TestAsset\Check\SecurityAdvisory;
 use ZendDiagnosticsTest\TestAsset\Check\AlwaysSuccess;
 
 class ChecksTest extends \PHPUnit_Framework_TestCase
@@ -416,33 +416,28 @@ public function testSecurityAdvisory()
         }
 
         $secureComposerLock = __DIR__ . '/TestAsset/secure-composer.lock';
-        $checker = new SecurityChecker();
-        $check = new SecurityAdvisory($checker, $secureComposerLock);
+        $check = new SecurityAdvisory($secureComposerLock);
         $result = $check->check();
         $this->assertNotInstanceOf('ZendDiagnostics\Result\Failure', $result);
 
         // check against non-existent lock file
-        $checker = new SecurityChecker();
-        $check = new SecurityAdvisory($checker, __DIR__ . '/improbable-lock-file-99999999999.lock');
+        $check = new SecurityAdvisory(__DIR__ . '/improbable-lock-file-99999999999.lock');
         $result = $check->check();
         $this->assertInstanceOf('ZendDiagnostics\Result\Failure', $result);
 
         // check against unreadable lock file
         $tmpDir = sys_get_temp_dir();
         if (!is_dir($tmpDir) || !is_writable($tmpDir)) {
             $this->markTestSkipped('Cannot access writable system temp dir to perform the test... ');
-
             return;
         }
         $unreadableFile = $tmpDir . '/composer.' . uniqid('', true) . '.lock';
         if (!file_put_contents($unreadableFile, 'foo') || !chmod($unreadableFile, 0000)) {
             $this->markTestSkipped('Cannot create temporary file in system temp dir to perform the test... ');
-
             return;
         }
 
-        $checker = new SecurityChecker();
-        $check = new SecurityAdvisory($checker, $unreadableFile);
+        $check = new SecurityAdvisory($unreadableFile);
         $result = $check->check();
         $this->assertInstanceOf('ZendDiagnostics\Result\Failure', $result);
 
@@ -463,7 +458,8 @@ public function testSecurityAdvisoryFailure()
             ->with($this->equalTo($secureComposerLock))
             ->will($this->returnValue('[{"a":1},{"b":2},{"c":3}]'));
 
-        $check = new SecurityAdvisory($checker, $secureComposerLock);
+        $check = new SecurityAdvisory($secureComposerLock);
+        $check->setSecurityChecker($checker);
         $result = $check->check();
         $this->assertInstanceOf('ZendDiagnostics\Result\Failure', $result);
     }
@@ -479,7 +475,8 @@ public function testSecurityAdvisoryInvalidServerResponse()
             ->method('check')
             ->with($this->equalTo($secureComposerLock))
             ->will($this->returnValue('404 error'));
-        $check = new SecurityAdvisory($checker, $secureComposerLock);
+        $check = new SecurityAdvisory($secureComposerLock);
+        $check->setSecurityChecker($checker);
         $result = $check->check();
         $this->assertInstanceOf('ZendDiagnostics\Result\Warning', $result);
 
@@ -495,7 +492,8 @@ public function testSecurityAdvisoryCheckerException()
             ->method('check')
             ->with($this->equalTo($secureComposerLock))
             ->will($this->throwException(new Exception));
-        $check = new SecurityAdvisory($checker, $secureComposerLock);
+        $check = new SecurityAdvisory($secureComposerLock);
+        $check->setSecurityChecker($checker);
         $result = $check->check();
         $this->assertInstanceOf('ZendDiagnostics\Result\Warning', $result);
     }

tests/ZendDiagnosticsTest/TestAsset/Check/SecurityAdvisory.php

@@ -0,0 +1,17 @@
+<?php
+
+namespace ZendDiagnosticsTest\TestAsset\Check;
+
+use SensioLabs\Security\SecurityChecker;
+use ZendDiagnostics\Check\SecurityAdvisory as BaseCheck;
+
+class SecurityAdvisory extends BaseCheck
+{
+    /**
+     * @param SecurityChecker $securityChecker
+     */
+    public function setSecurityChecker(SecurityChecker $securityChecker)
+    {
+        $this->securityChecker = $securityChecker;
+    }
+}