Fix validation of 'git' and 'files' options

phpunit.xml.dist

@@ -0,0 +1,23 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<phpunit backupGlobals="false"
+         backupStaticAttributes="false"
+         colors="true"
+         convertErrorsToExceptions="true"
+         convertNoticesToExceptions="true"
+         convertWarningsToExceptions="true"
+         processIsolation="false"
+         stopOnFailure="false"
+         syntaxCheck="false"
+         bootstrap="vendor/autoload.php">
+
+    <testsuites>
+        <testsuite name="Unit tests">
+            <directory>tests/unit</directory>
+        </testsuite>
+        <testsuite name="Integration tests">
+            <directory>tests/integration</directory>
+        </testsuite>
+    </testsuites>
+
+</phpunit>

src/Command/AnalyzeCommand.php

@@ -2,6 +2,8 @@
 
 namespace JMOlivas\Phpqa\Command;
 
+use Exception;
+use JMOlivas\Phpqa\Input\FilesOption;
 use Symfony\Component\Console\Command\Command;
 use Symfony\Component\Console\Input\InputInterface;
 use Symfony\Component\Console\Input\InputOption;
@@ -46,7 +48,7 @@ protected function configure()
                 'git',
                 null,
                 InputOption::VALUE_NONE,
-                'All files added to git index will be analyze.'
+                'All files added to git index will be analyzed.'
             );
     }
 
@@ -65,7 +67,7 @@ protected function execute(InputInterface $input, OutputInterface $output)
         $config = $application->getConfig();
 
         if (!$config->isCustom() && !$project) {
-            throw new \Exception(
+            throw new Exception(
                 sprintf(
                     'No local phpqa.yml or phpqa.yml.dist at current working directory ' .
                     'you must provide a valid project value (%s)',
@@ -75,7 +77,7 @@ protected function execute(InputInterface $input, OutputInterface $output)
         }
 
         if (!$config->isCustom() && !in_array($project, $this->projects)) {
-            throw new \Exception(
+            throw new Exception(
                 sprintf(
                     'You must provide a valid project value (%s)',
                     implode(',', $this->projects)
@@ -89,31 +91,25 @@ protected function execute(InputInterface $input, OutputInterface $output)
 
         $output->writeln(sprintf('<question>%s</question>', $application->getName()));
 
-        $files = $input->getOption('files');
+        $filesOption = new FilesOption($input->getOption('files'));
+        $git = $input->getOption('git');
 
-        $git = false;
-        if ($input->hasOption('git')) {
-            $git = $input->getOption('git');
+        if (!$filesOption->isAbsent() && $git) {
+            throw new Exception('Options `files` and `git` cannot be used in combination.');
         }
 
-        if ($files && $git) {
-            throw new \Exception('Options `files` and `git` can not used in combination.');
+        if ($filesOption->isAbsent() && !$git) {
+            throw new Exception('You must set `files` or `git` options.');
         }
 
-        if ($files) {
-            $files = explode(',', $files[0]);
-        }
-
-        if (!$files[0]) {
-            $files = [];
-        }
-
-        if (!$files && !$git) {
-            throw new \Exception('You must set `files` or `git` options.');
+        if (!$filesOption->isAbsent() && $filesOption->isEmpty()) {
+            throw new Exception('Options `files` needs at least one file.');
         }
 
         if ($git) {
             $files = $this->extractCommitedFiles($output, $config);
+        } else {
+            $files = $filesOption->normalize();
         }
 
         $output->writeln(
@@ -202,7 +198,7 @@ private function checkComposer($output, $files, $config)
 
         if ($config->get('application.method.composer.exception')) {
             if ($composerJsonDetected && !$composerLockDetected) {
-                throw new \Exception($config->get('application.messages.composer.error'));
+                throw new Exception($config->get('application.messages.composer.error'));
             }
 
             $output->writeln(
@@ -269,7 +265,7 @@ private function analyzer($output, $analyzer, $files, $config, $project)
         }
 
         if ($exception && !$success) {
-            throw new \Exception($config->get('application.messages.'.$analyzer.'.error'));
+            throw new Exception($config->get('application.messages.'.$analyzer.'.error'));
         }
     }
 
@@ -312,7 +308,7 @@ public function executeProcess($output, $processArguments, $file, $prefixes, $po
     private function validateBinary($binaryFile)
     {
         if (!file_exists($this->directory.$binaryFile)) {
-            throw new \Exception(
+            throw new Exception(
                 sprintf('%s do not exist!', $binaryFile)
             );
         }

src/Console/Application.php

@@ -23,29 +23,25 @@ class Application extends BaseApplication
      */
     private $config;
 
-    /**
-     * @return \JMOlivas\Phpqa\Config
-     */
-    public function getConfig()
+    public function __construct($name = 'UNKNOWN', $version = 'UNKNOWN')
     {
-        return $this->config;
+        parent::__construct($name, $version);
+        $this->config = new Config();
     }
 
     /**
-     * {@inheritdoc}
+     * @return \JMOlivas\Phpqa\Config
      */
-    public function doRun(InputInterface $input, OutputInterface $output)
+    public function getConfig()
     {
-        $this->config = new Config();
-
-        parent::doRun($input, $output);
+        return $this->config;
     }
 
     /**
      * @return string
      */
     public function getApplicationDirectory()
     {
-        return __DIR__.'/../../';
+        return __DIR__ . '/../../';
     }
 }

src/Input/FilesOption.php

@@ -0,0 +1,58 @@
+<?php
+
+namespace JMOlivas\Phpqa\Input;
+
+class FilesOption
+{
+    /** @var array */
+    private $files;
+
+    /**
+     * @param array $files
+     */
+    public function __construct(array $files)
+    {
+        $this->files = $files;
+    }
+
+    /**
+     * Returns true if this option is provided but has no values
+     *
+     * @return bool
+     */
+    public function isEmpty()
+    {
+        return count($this->files) === 1 && $this->files[0] === null;
+    }
+
+    /**
+     * Returns true if this option is not provided
+     *
+     * @return bool
+     */
+    public function isAbsent()
+    {
+        return empty($this->files);
+    }
+
+    /**
+     * Normalize the provided values as an array
+     *
+     * - If it's either empty or absent, it returns an empty array
+     * - If it's a single value separated by commas, it converts it to array
+     * - Otherwise returns the value as is.
+     *
+     * @return array
+     */
+    public function normalize()
+    {
+        if ($this->isAbsent() || $this->isEmpty()) {
+            return [];
+        }
+        if (count($this->files) === 1 && strpos($this->files[0], ',') !== false) {
+            return explode(',', $this->files[0]);
+        }
+
+        return $this->files;
+    }
+}

tests/integration/Command/AnalyzeCommandTest.php

@@ -0,0 +1,63 @@
+<?php
+
+namespace JMOlivas\Phpqa\Command;
+
+use JMOlivas\Phpqa\Console\Application;
+use PHPUnit_Framework_TestCase as TestCase;
+use Symfony\Component\Console\Tester\CommandTester;
+
+class AnalyzeCommandTest extends TestCase
+{
+    /**
+     * @test
+     * @expectedException \Exception
+     * @expectedExceptionMessage You must set `files` or `git` options.
+     */
+    function it_should_throw_exception_if_neither_files_nor_git_options_are_provided()
+    {
+        $application = new Application();
+        $command = new AnalyzeCommand();
+        $command->setApplication($application);
+
+        $tester = new CommandTester($command);
+
+        $tester->execute([]);
+    }
+
+    /**
+     * @test
+     * @expectedException \Exception
+     * @expectedExceptionMessage Options `files` and `git` cannot be used in combination.
+     */
+    function it_should_throw_exception_if_both_files_and_git_options_are_provided()
+    {
+        $application = new Application();
+        $command = new AnalyzeCommand();
+        $command->setApplication($application);
+
+        $tester = new CommandTester($command);
+
+        $tester->execute([
+            '--files' => [null],
+            '--git' => true
+        ]);
+    }
+
+    /**
+     * @test
+     * @expectedException \Exception
+     * @expectedExceptionMessage Options `files` needs at least one file.
+     */
+    function it_should_throw_exception_if_files_is_provided_but_it_is_empty()
+    {
+        $application = new Application();
+        $command = new AnalyzeCommand();
+        $command->setApplication($application);
+
+        $tester = new CommandTester($command);
+
+        $tester->execute([
+            '--files' => [null],
+        ]);
+    }
+}

tests/unit/Input/FilesOptionTest.php

@@ -0,0 +1,82 @@
+<?php
+
+namespace JMOlivas\Phpqa\Input;
+
+use PHPUnit_Framework_TestCase as TestCase;
+
+class FilesOptionTest extends TestCase
+{
+    /** @test */
+    function it_should_recognize_if_option_is_absent()
+    {
+        $absentInput = [];
+        $files = new FilesOption($absentInput);
+
+        $this->assertTrue($files->isAbsent());
+    }
+
+    /** @test */
+    function it_should_recognize_if_option_is_provided_but_is_empty()
+    {
+        $emptyInput = [null];
+        $files = new FilesOption($emptyInput);
+
+        $this->assertTrue($files->isEmpty());
+    }
+
+    /** @test */
+    function it_should_recognize_if_option_is_provided_correctly()
+    {
+        $validInput = ['src/'];
+        $files = new FilesOption($validInput);
+
+        $this->assertFalse($files->isAbsent());
+        $this->assertFalse($files->isEmpty());
+    }
+
+    /** @test */
+    function it_should_normalize_input_separated_by_commas()
+    {
+        // bin/phpqa analyze --files=src/,test/
+        $singleInputWithMultipleValues = ['src/,test/'];
+        $files = new FilesOption($singleInputWithMultipleValues);
+
+        $values = $files->normalize();
+
+        $this->assertCount(2, $values);
+        $this->assertEquals('src/', $values[0]);
+        $this->assertEquals('test/', $values[1]);
+    }
+
+    /** @test */
+    function it_should_return_multiple_files_input_as_is()
+    {
+        // bin/phpqa analyze --files=src/ --files=test/
+        $singleInputWithMultipleValues = ['src/','test/'];
+        $files = new FilesOption($singleInputWithMultipleValues);
+
+        $values = $files->normalize();
+
+        $this->assertCount(2, $values);
+        $this->assertEquals('src/', $values[0]);
+        $this->assertEquals('test/', $values[1]);
+    }
+
+    /** @test */
+    function it_should_return_empty_array_if_input_is_absent()
+    {
+        $absentInput = [];
+        $files = new FilesOption($absentInput);
+
+        $this->assertCount(0, $files->normalize());
+    }
+
+    /** @test */
+    function it_should_return_empty_array_if_input_is_empty()
+    {
+        $emptyInput = [null];
+        $files = new FilesOption($emptyInput);
+
+        $this->assertCount(0, $files->normalize());
+    }
+}