Merge branch 'development'

Author: Sam Parkinson

.travis.yml

@@ -13,7 +13,7 @@ cache:
 
 before_script:
   - composer self-update
-  - composer install --no-interaction --optimize-autoloader
+  - composer install --no-interaction --optimize-autoloader --prefer-source
 
 script:
   - vendor/bin/phpunit

CONTRIBUTING.md

@@ -4,35 +4,40 @@ The following guidelines for contribution should be followed if you wish to subm
 
 ## Getting Started
 
-* Submit an [issue ticket](https://github.com/sjparkinson/static-review/issues) for your issue if one **does not** exist already
+* If you have found a bug, submit an [ticket][issues] for your issue if one **does not** exist already
 
-    * Describe the issue and include steps to reproduce if it's a bug
+    * Describe the issue and include steps to reproduce the problem
 
 * If you are able and want to fix this, fork the repository on GitHub
 
-## Making Changes
+* When adding a feature, feel free to [get in contact][twitter] before starting any development
+
+[issues]:  https://github.com/sjparkinson/static-review/issues
+[twitter]: https://twitter.com/samparkinson_
 
-* In your forked repository, create a topic branch for your upcoming patch (e.g. `feature/autoplay` or `bugfix/ios-crash`)
+## Making Changes
 
-    * Usually this is based on the master branch
+* In your forked repository, create a topic branch for your upcoming patch (e.g. `feature/svn-support` or `bugfix/example-review-fix`)
 
-    * Create a branch based on master, but please **do not** work directly on the `master` branch
+    * Create a branch based on `master`, but please **do not** work directly on the `master` branch
 
 * Make sure you stick to the coding style that is used already
 
-* If possible, submit unit tests with your patch / new feature so it can be tested easily
+* If possible, submit unit tests with your patch / new feature
+
+* Ensure that nothing is broken by [running the test suite][test-suite]
 
-* Ensure that nothing is broken by [running all the unit tests](https://github.com/sjparkinson/static-review/blob/development/README.md#unit-tests)
+[test-suite]: https://github.com/sjparkinson/static-review/blob/development/README.md#unit-tests)
 
 ## Submitting Changes
 
 1. Push your changes to a topic branch in your fork of the repository
 
-2. Open a pull request to merge to the original repository
+2. Open a pull request to merge to the [original repository][origin]
 
-3. Lastly, please reference and update your original issue with the code changes
+[origin]: https://github.com/sjparkinson/static-review
 
 # Additional Resources
 
-* [General GitHub documentation](http://help.github.com/)
-* [GitHub pull request documentation](http://help.github.com/send-pull-requests/)
+* [General GitHub documentation](http://help.github.com)
+* [GitHub pull request documentation](http://help.github.com/send-pull-requests)

README.md

@@ -19,10 +19,9 @@ Within a [composer][composer] managed project you can simply do the following...
 
 ```bash
 $ composer require sjparkinson/static-review:~1.0
-$ vendor/bin/static-review.php list hook
 ```
 
-Then you can add one of the hooks to your project like so...
+You can then add one of the hooks to your project like so...
 
 ```bash
 $ vendor/bin/static-review.php hook:install example-pre-commit .git/hooks/pre-commit

Vagrantfile

@@ -1,8 +1,6 @@
 # -*- mode: ruby -*-
 # vi: set ft=ruby :
 
-# @see https://www.vagrantup.com
-
 Vagrant.configure("2") do |config|
 
   # Box
@@ -15,9 +13,10 @@ Vagrant.configure("2") do |config|
   # Setup
   config.vm.provision :shell, :inline => "apt-get update --fix-missing"
   config.vm.provision :shell, :inline => "apt-get install -q -y cowsay python-software-properties python g++ make git curl"
-  config.vm.provision :shell, :inline => "add-apt-repository ppa:ondrej/php5 && apt-get update"
+  config.vm.provision :shell, :inline => "add-apt-repository ppa:ondrej/php5"
+  config.vm.provision :shell, :inline => "apt-get update"
   config.vm.provision :shell, :inline => "apt-get install -q -y php5-cli php5-curl php5-xdebug"
-  config.vm.provision :shell, :inline => "curl -s https://getcomposer.org/installer | php"
+  config.vm.provision :shell, :inline => "curl -sS https://getcomposer.org/installer | php"
   config.vm.provision :shell, :inline => "mv ./composer.phar /usr/local/bin/composer"
 
   # Done

composer.json

@@ -26,7 +26,8 @@
 
     "autoload-dev": {
         "psr-4": {
-            "StaticReview\\Tests\\": "tests/"
+            "StaticReview\\Test\\Unit\\": "tests/unit/",
+            "StaticReview\\Test\\Functional\\": "tests/functional/"
         }
     },
 

phpunit.xml

@@ -8,7 +8,10 @@
     <!-- Test suites -->
     <testsuites>
         <testsuite name="unit">
-            <directory>tests/</directory>
+            <directory>tests/unit</directory>
+        </testsuite>
+        <testsuite name="functional">
+            <directory>tests/functional</directory>
         </testsuite>
     </testsuites>
 

src/Command/HookInstallCommand.php

@@ -21,36 +21,47 @@
 
 class HookInstallCommand extends Command
 {
+    const ARGUMENT_TARGET = 'target';
+    const ARGUMENT_LINK   = 'link';
+
     protected function configure()
     {
         $this->setName('hook:install');
 
         $this->setDescription('Symlink a hook to the given target.');
 
-        $this->addArgument('target', InputArgument::REQUIRED, 'The hook to link, either a path to a file or the filename of a hook in the hooks folder.')
-             ->addArgument('link', InputArgument::REQUIRED, 'The target location, including the filename (e.g. .git/hooks/pre-commit).');
+        $this->addArgument(self::ARGUMENT_TARGET, InputArgument::REQUIRED, 'The hook to link, either a path to a file or the filename of a hook in the hooks folder.')
+             ->addArgument(self::ARGUMENT_LINK, InputArgument::REQUIRED, 'The target location, including the filename (e.g. .git/hooks/pre-commit).');
 
         $this->addOption('force', 'f', InputOption::VALUE_NONE, 'Overrite any existing files at the symlink target.');
     }
 
     protected function execute(InputInterface $input, OutputInterface $output)
     {
+        $hookArgument = $input->getArgument(self::ARGUMENT_TARGET);
+
+        $target = $this->getTargetPath($hookArgument);
+        $link   = $input->getArgument(self::ARGUMENT_LINK);
         $force  = $input->getOption('force');
-        $target = $this->getTargetPath($input);
-        $link   = $input->getArgument('link');
 
         if ($output->isVeryVerbose()) {
-            $message = sprintf('<info>Using %s for the hook target.</info>', $target);
+            $message = sprintf('<info>Using %s for the install path.</info>', $link);
+            $output->writeln($message);
+
+            $message = sprintf('<info>Using %s as the hook.</info>', $target);
             $output->writeln($message);
+        }
 
-            $message = sprintf('<info>Using %s for the hook path.</info>', $link);
+        if (! is_dir(dirname($link))) {
+            $message = sprintf('<error>The directory at %s does not exist.</error>', $link);
             $output->writeln($message);
+            exit(1);
         }
 
         if (file_exists($link) && $force) {
             unlink($link);
 
-            $message = sprintf('<info>Removed existing file at %s.</info>', $link);
+            $message = sprintf('<comment>Removed existing file at %s.</comment>', $link);
             $output->writeln($message);
         }
 
@@ -66,16 +77,16 @@ protected function execute(InputInterface $input, OutputInterface $output)
     }
 
     /**
-     * @param $input InputInterface
+     * @param $hookArgument string
      * @return string
      */
-    protected function getTargetPath(InputInterface $input)
+    protected function getTargetPath($hookArgument)
     {
-        if (file_exists($input->getArgument('target'))) {
-            $target = realpath($input->getArgument('target'));
+        if (file_exists($hookArgument)) {
+            $target = realpath($hookArgument);
         } else {
             $path = '%s/%s.php';
-            $target = sprintf($path, realpath(__DIR__ . '/../../hooks/'), $input->getArgument('target'));
+            $target = sprintf($path, realpath(__DIR__ . '/../../hooks/'), $hookArgument);
         }
 
         if (! file_exists($target)) {

src/Command/HookListCommand.php

@@ -24,7 +24,7 @@ protected function configure()
     {
         $this->setName('hook:list');
 
-        $this->setDescription('Lists all the avaliable hooks.');
+        $this->setDescription('Lists all the included hooks.');
     }
 
     protected function execute(InputInterface $input, OutputInterface $output)

src/Command/HookRunCommand.php

@@ -21,23 +21,24 @@
 
 class HookRunCommand extends Command
 {
+    const ARGUMENT_HOOK = 'hook';
+
     protected function configure()
     {
         $this->setName('hook:run');
 
         $this->setDescription('Run the specified hook.');
 
-        $this->addArgument('hook', InputArgument::REQUIRED, 'The hook file to run.');
+        $this->addArgument(self::ARGUMENT_HOOK, InputArgument::REQUIRED, 'The hook file to run.');
     }
 
     protected function execute(InputInterface $input, OutputInterface $output)
     {
-        $hooksPath = realpath(__DIR__ . '/../../hooks/');
-
-        $source = $hooksPath . '/' . $input->getArgument('hook') . '.php';
+        $hookArg = $input->getArgument(self::ARGUMENT_HOOK);
+        $path = $this->getTargetPath($hookArg);
 
-        if (file_exists($source)) {
-            $cmd = 'php ' . $source;
+        if (file_exists($hook)) {
+            $cmd = 'php ' . $hook;
 
             $process = new Process($cmd);
 
@@ -46,4 +47,26 @@ protected function execute(InputInterface $input, OutputInterface $output)
             });
         }
     }
+
+    /**
+     * @param $hookArgument string
+     * @return string
+     */
+    protected function getTargetPath($hookArgument)
+    {
+        if (file_exists($hookArgument)) {
+            $target = realpath($hookArgument);
+        } else {
+            $path = '%s/%s.php';
+            $target = sprintf($path, realpath(__DIR__ . '/../../hooks/'), $hookArgument);
+        }
+
+        if (! file_exists($target)) {
+            $error = sprintf('<error>The hook %s does not exist!</error>', $target);
+            $output->writeln($error);
+            exit(1);
+        }
+
+        return $target;
+    }
 }

tests/Collection/CollectionTest.php tests/unit/Collection/CollectionTest.phptests/Collection/CollectionTest.php renamed to tests/unit/Collection/CollectionTest.php

@@ -10,7 +10,7 @@
  * @see http://github.com/sjparkinson/static-review/blob/master/LICENSE.md
  */
 
-namespace StaticReview\Tests\Collection;
+namespace StaticReview\Test\Unit\Collection;
 
 use StaticReview\Collection\Collection;
 
@@ -40,7 +40,7 @@ public function testConstructorWithArgument()
         $this->collection->__construct($items);
 
         for ($i = 0; $i > count($this->collection); $i++) {
-            $this->assertEquals($items[$i], $this->collection[$i]);
+            $this->assertSame($items[$i], $this->collection[$i]);
         }
     }
 
@@ -62,12 +62,12 @@ public function testAppendWithValidItem()
         $this->collection->append($item);
 
         $this->assertCount(1, $this->collection);
-        $this->assertEquals($item, $this->collection->current());
+        $this->assertSame($item, $this->collection->current());
 
         $this->collection->append($item);
 
         $this->assertCount(2, $this->collection);
-        $this->assertEquals($item, $this->collection->next());
+        $this->assertSame($item, $this->collection->next());
     }
 
     /**