Skip to content

Commit c981e06

Browse files
jmolivasjmolivas
committed
Merge pull request #20 from MontealegreLuis/master
Fix validation of 'git' and 'files' options
2 parents d206afc + b348a7c commit c981e06

File tree

6 files changed

+251
-33
lines changed

6 files changed

+251
-33
lines changed

‎phpunit.xml.dist‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
3+
<phpunit backupGlobals="false"
4+
backupStaticAttributes="false"
5+
colors="true"
6+
convertErrorsToExceptions="true"
7+
convertNoticesToExceptions="true"
8+
convertWarningsToExceptions="true"
9+
processIsolation="false"
10+
stopOnFailure="false"
11+
syntaxCheck="false"
12+
bootstrap="vendor/autoload.php">
13+
14+
<testsuites>
15+
<testsuite name="Unit tests">
16+
<directory>tests/unit</directory>
17+
</testsuite>
18+
<testsuite name="Integration tests">
19+
<directory>tests/integration</directory>
20+
</testsuite>
21+
</testsuites>
22+
23+
</phpunit>

‎src/Command/AnalyzeCommand.php‎

Lines changed: 18 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22

33
namespace JMOlivas\Phpqa\Command;
44

5+
use Exception;
6+
use JMOlivas\Phpqa\Input\FilesOption;
57
use Symfony\Component\Console\Command\Command;
68
use Symfony\Component\Console\Input\InputInterface;
79
use Symfony\Component\Console\Input\InputOption;
@@ -46,7 +48,7 @@ protected function configure()
4648
'git',
4749
null,
4850
InputOption::VALUE_NONE,
49-
'All files added to git index will be analyze.'
51+
'All files added to git index will be analyzed.'
5052
);
5153
}
5254

@@ -65,7 +67,7 @@ protected function execute(InputInterface $input, OutputInterface $output)
6567
$config = $application->getConfig();
6668

6769
if (!$config->isCustom() && !$project) {
68-
throw new \Exception(
70+
throw new Exception(
6971
sprintf(
7072
'No local phpqa.yml or phpqa.yml.dist at current working directory ' .
7173
'you must provide a valid project value (%s)',
@@ -75,7 +77,7 @@ protected function execute(InputInterface $input, OutputInterface $output)
7577
}
7678

7779
if (!$config->isCustom() && !in_array($project, $this->projects)) {
78-
throw new \Exception(
80+
throw new Exception(
7981
sprintf(
8082
'You must provide a valid project value (%s)',
8183
implode(',', $this->projects)
@@ -89,31 +91,25 @@ protected function execute(InputInterface $input, OutputInterface $output)
8991

9092
$output->writeln(sprintf('<question>%s</question>', $application->getName()));
9193

92-
$files = $input->getOption('files');
94+
$filesOption = new FilesOption($input->getOption('files'));
95+
$git = $input->getOption('git');
9396

94-
$git = false;
95-
if ($input->hasOption('git')) {
96-
$git = $input->getOption('git');
97+
if (!$filesOption->isAbsent() && $git) {
98+
throw new Exception('Options `files` and `git` cannot be used in combination.');
9799
}
98100

99-
if ($files && $git) {
100-
throw new \Exception('Options `files` and `git` can not used in combination.');
101+
if ($filesOption->isAbsent() && !$git) {
102+
throw new Exception('You must set `files` or `git` options.');
101103
}
102104

103-
if ($files) {
104-
$files = explode(',', $files[0]);
105-
}
106-
107-
if (!$files[0]) {
108-
$files = [];
109-
}
110-
111-
if (!$files && !$git) {
112-
throw new \Exception('You must set `files` or `git` options.');
105+
if (!$filesOption->isAbsent() && $filesOption->isEmpty()) {
106+
throw new Exception('Options `files` needs at least one file.');
113107
}
114108

115109
if ($git) {
116110
$files = $this->extractCommitedFiles($output, $config);
111+
} else {
112+
$files = $filesOption->normalize();
117113
}
118114

119115
$output->writeln(
@@ -202,7 +198,7 @@ private function checkComposer($output, $files, $config)
202198

203199
if ($config->get('application.method.composer.exception')) {
204200
if ($composerJsonDetected && !$composerLockDetected) {
205-
throw new \Exception($config->get('application.messages.composer.error'));
201+
throw new Exception($config->get('application.messages.composer.error'));
206202
}
207203

208204
$output->writeln(
@@ -269,7 +265,7 @@ private function analyzer($output, $analyzer, $files, $config, $project)
269265
}
270266

271267
if ($exception && !$success) {
272-
throw new \Exception($config->get('application.messages.'.$analyzer.'.error'));
268+
throw new Exception($config->get('application.messages.'.$analyzer.'.error'));
273269
}
274270
}
275271

@@ -312,7 +308,7 @@ public function executeProcess($output, $processArguments, $file, $prefixes, $po
312308
private function validateBinary($binaryFile)
313309
{
314310
if (!file_exists($this->directory.$binaryFile)) {
315-
throw new \Exception(
311+
throw new Exception(
316312
sprintf('%s do not exist!', $binaryFile)
317313
);
318314
}

‎src/Console/Application.php‎

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -23,29 +23,25 @@ class Application extends BaseApplication
2323
*/
2424
private $config;
2525

26-
/**
27-
* @return \JMOlivas\Phpqa\Config
28-
*/
29-
public function getConfig()
26+
public function __construct($name = 'UNKNOWN', $version = 'UNKNOWN')
3027
{
31-
return $this->config;
28+
parent::__construct($name, $version);
29+
$this->config = new Config();
3230
}
3331

3432
/**
35-
* {@inheritdoc}
33+
* @return \JMOlivas\Phpqa\Config
3634
*/
37-
public function doRun(InputInterface $input, OutputInterface $output)
35+
public function getConfig()
3836
{
39-
$this->config = new Config();
40-
41-
parent::doRun($input, $output);
37+
return $this->config;
4238
}
4339

4440
/**
4541
* @return string
4642
*/
4743
public function getApplicationDirectory()
4844
{
49-
return __DIR__.'/../../';
45+
return __DIR__ . '/../../';
5046
}
5147
}

‎src/Input/FilesOption.php‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
<?php
2+
3+
namespace JMOlivas\Phpqa\Input;
4+
5+
class FilesOption
6+
{
7+
/** @var array */
8+
private $files;
9+
10+
/**
11+
* @param array $files
12+
*/
13+
public function __construct(array $files)
14+
{
15+
$this->files = $files;
16+
}
17+
18+
/**
19+
* Returns true if this option is provided but has no values
20+
*
21+
* @return bool
22+
*/
23+
public function isEmpty()
24+
{
25+
return count($this->files) === 1 && $this->files[0] === null;
26+
}
27+
28+
/**
29+
* Returns true if this option is not provided
30+
*
31+
* @return bool
32+
*/
33+
public function isAbsent()
34+
{
35+
return empty($this->files);
36+
}
37+
38+
/**
39+
* Normalize the provided values as an array
40+
*
41+
* - If it's either empty or absent, it returns an empty array
42+
* - If it's a single value separated by commas, it converts it to array
43+
* - Otherwise returns the value as is.
44+
*
45+
* @return array
46+
*/
47+
public function normalize()
48+
{
49+
if ($this->isAbsent() || $this->isEmpty()) {
50+
return [];
51+
}
52+
if (count($this->files) === 1 && strpos($this->files[0], ',') !== false) {
53+
return explode(',', $this->files[0]);
54+
}
55+
56+
return $this->files;
57+
}
58+
}

‎tests/integration/Command/AnalyzeCommandTest.php‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
<?php
2+
3+
namespace JMOlivas\Phpqa\Command;
4+
5+
use JMOlivas\Phpqa\Console\Application;
6+
use PHPUnit_Framework_TestCase as TestCase;
7+
use Symfony\Component\Console\Tester\CommandTester;
8+
9+
class AnalyzeCommandTest extends TestCase
10+
{
11+
/**
12+
* @test
13+
* @expectedException \Exception
14+
* @expectedExceptionMessage You must set `files` or `git` options.
15+
*/
16+
function it_should_throw_exception_if_neither_files_nor_git_options_are_provided()
17+
{
18+
$application = new Application();
19+
$command = new AnalyzeCommand();
20+
$command->setApplication($application);
21+
22+
$tester = new CommandTester($command);
23+
24+
$tester->execute([]);
25+
}
26+
27+
/**
28+
* @test
29+
* @expectedException \Exception
30+
* @expectedExceptionMessage Options `files` and `git` cannot be used in combination.
31+
*/
32+
function it_should_throw_exception_if_both_files_and_git_options_are_provided()
33+
{
34+
$application = new Application();
35+
$command = new AnalyzeCommand();
36+
$command->setApplication($application);
37+
38+
$tester = new CommandTester($command);
39+
40+
$tester->execute([
41+
'--files' => [null],
42+
'--git' => true
43+
]);
44+
}
45+
46+
/**
47+
* @test
48+
* @expectedException \Exception
49+
* @expectedExceptionMessage Options `files` needs at least one file.
50+
*/
51+
function it_should_throw_exception_if_files_is_provided_but_it_is_empty()
52+
{
53+
$application = new Application();
54+
$command = new AnalyzeCommand();
55+
$command->setApplication($application);
56+
57+
$tester = new CommandTester($command);
58+
59+
$tester->execute([
60+
'--files' => [null],
61+
]);
62+
}
63+
}

‎tests/unit/Input/FilesOptionTest.php‎

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
<?php
2+
3+
namespace JMOlivas\Phpqa\Input;
4+
5+
use PHPUnit_Framework_TestCase as TestCase;
6+
7+
class FilesOptionTest extends TestCase
8+
{
9+
/** @test */
10+
function it_should_recognize_if_option_is_absent()
11+
{
12+
$absentInput = [];
13+
$files = new FilesOption($absentInput);
14+
15+
$this->assertTrue($files->isAbsent());
16+
}
17+
18+
/** @test */
19+
function it_should_recognize_if_option_is_provided_but_is_empty()
20+
{
21+
$emptyInput = [null];
22+
$files = new FilesOption($emptyInput);
23+
24+
$this->assertTrue($files->isEmpty());
25+
}
26+
27+
/** @test */
28+
function it_should_recognize_if_option_is_provided_correctly()
29+
{
30+
$validInput = ['src/'];
31+
$files = new FilesOption($validInput);
32+
33+
$this->assertFalse($files->isAbsent());
34+
$this->assertFalse($files->isEmpty());
35+
}
36+
37+
/** @test */
38+
function it_should_normalize_input_separated_by_commas()
39+
{
40+
// bin/phpqa analyze --files=src/,test/
41+
$singleInputWithMultipleValues = ['src/,test/'];
42+
$files = new FilesOption($singleInputWithMultipleValues);
43+
44+
$values = $files->normalize();
45+
46+
$this->assertCount(2, $values);
47+
$this->assertEquals('src/', $values[0]);
48+
$this->assertEquals('test/', $values[1]);
49+
}
50+
51+
/** @test */
52+
function it_should_return_multiple_files_input_as_is()
53+
{
54+
// bin/phpqa analyze --files=src/ --files=test/
55+
$singleInputWithMultipleValues = ['src/','test/'];
56+
$files = new FilesOption($singleInputWithMultipleValues);
57+
58+
$values = $files->normalize();
59+
60+
$this->assertCount(2, $values);
61+
$this->assertEquals('src/', $values[0]);
62+
$this->assertEquals('test/', $values[1]);
63+
}
64+
65+
/** @test */
66+
function it_should_return_empty_array_if_input_is_absent()
67+
{
68+
$absentInput = [];
69+
$files = new FilesOption($absentInput);
70+
71+
$this->assertCount(0, $files->normalize());
72+
}
73+
74+
/** @test */
75+
function it_should_return_empty_array_if_input_is_empty()
76+
{
77+
$emptyInput = [null];
78+
$files = new FilesOption($emptyInput);
79+
80+
$this->assertCount(0, $files->normalize());
81+
}
82+
}

0 commit comments

Comments
 (0)