Skip to content

Commit

Permalink
fix issue #187
Browse files Browse the repository at this point in the history
  • Loading branch information
llaville committed Feb 23, 2023
1 parent 1cbfb39 commit 4cb2bd2
Show file tree
Hide file tree
Showing 5 changed files with 94 additions and 21 deletions.
4 changes: 4 additions & 0 deletions .changes/unreleased/Fixed-20230223-153848.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
kind: Fixed
body: '[#187](https://github.com/overtrue/phplint/issues/187) : Improve log options
implementation'
time: 2023-02-23T15:38:48.338553903Z
19 changes: 17 additions & 2 deletions src/Configuration/AbstractOptionsResolver.php
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,8 @@ public function __construct(InputInterface $input, array $configuration = [])
OptionDefinition::NO_CACHE => false,
OptionDefinition::PROGRESS => OptionDefinition::DEFAULT_PROGRESS_WIDGET,
OptionDefinition::NO_PROGRESS => false,
OptionDefinition::LOG_JSON => null,
OptionDefinition::LOG_JUNIT => null,
OptionDefinition::LOG_JSON => false,
OptionDefinition::LOG_JUNIT => false,
OptionDefinition::WARNING => false,
OptionDefinition::OPTION_MEMORY_LIMIT => ini_get('memory_limit'),
OptionDefinition::IGNORE_EXIT_CODE => false,
Expand Down Expand Up @@ -76,6 +76,21 @@ public function __construct(InputInterface $input, array $configuration = [])
unset($options['warning']);
}

// log options that accept :
// - NULL or empty string are values considered to enable output format for standard output
// - a string to identify stream
$names = [
OptionDefinition::LOG_JSON,
OptionDefinition::LOG_JUNIT,
];
foreach ($names as $name) {
if ('' === $options[$name]) {
$options[$name] = true;
} elseif (null === $options[$name] && (true === $input->hasParameterOption(['--' . $name], true))) {
$options[$name] = true;
}
}

// options that cannot be overridden by YAML config file values
$names = [
OptionDefinition::CONFIGURATION,
Expand Down
62 changes: 52 additions & 10 deletions src/Configuration/OptionsFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,16 @@

namespace Overtrue\PHPLint\Configuration;

use Closure;
use Symfony\Component\OptionsResolver\Options as SymfonyOptions;
use Symfony\Component\OptionsResolver\OptionsResolver;

use function array_keys;
use function filter_var;
use function in_array;
use function is_bool;

use const FILTER_VALIDATE_BOOLEAN;

/**
* @author Laurent Laville
Expand Down Expand Up @@ -52,8 +58,8 @@ protected function configureOptions(OptionsResolver $resolver): void
OptionDefinition::NO_CACHE => 'bool',
OptionDefinition::PROGRESS => ['null', 'string'],
OptionDefinition::NO_PROGRESS => 'bool',
OptionDefinition::LOG_JSON => ['bool', 'null', 'string'],
OptionDefinition::LOG_JUNIT => ['bool', 'null', 'string'],
OptionDefinition::LOG_JSON => ['bool', 'string'],
OptionDefinition::LOG_JUNIT => ['bool', 'string'],
OptionDefinition::WARNING => 'bool',
OptionDefinition::OPTION_MEMORY_LIMIT => ['int', 'string'],
OptionDefinition::IGNORE_EXIT_CODE => 'bool',
Expand Down Expand Up @@ -81,13 +87,49 @@ protected function configureOptions(OptionsResolver $resolver): void
return (int) $value;
});

$outputFormat = function (SymfonyOptions $options, $value) {
if (true === $value) {
$value = OptionDefinition::DEFAULT_STANDARD_OUTPUT;
}
return $value;
};
$resolver->setNormalizer(OptionDefinition::LOG_JSON, $outputFormat);
$resolver->setNormalizer(OptionDefinition::LOG_JUNIT, $outputFormat);
$names = [
OptionDefinition::LOG_JSON,
OptionDefinition::LOG_JUNIT,
];
foreach ($names as $name) {
$resolver->setNormalizer($name, Closure::fromCallable([$this, 'logNormalizer']));
}
}

/**
* Reused by unit tests suite ConsoleConfigTest
*/
public static function logNormalizer(SymfonyOptions $options, $value)
{
$bool = static::toBool($value);
if (is_bool($bool)) {
$value = $bool ? OptionDefinition::DEFAULT_STANDARD_OUTPUT : false;
}
return $value;
}

/**
* Best strategy to convert string to boolean
*
*
* @link https://stackoverflow.com/questions/7336861/how-to-convert-string-to-boolean-php#answer-15075609
* @link https://www.php.net/manual/en/function.filter-var
*/
public static function toBool(mixed $value): bool|int
{
$booleanValueDomain = [
false, 'false', 0, '0', 'off', 'no',
true, 'true', 1, '1', 'on', 'yes',
];

if (in_array($value, ['', null], true)) {
// CAUTION: null or empty string must be considered as logging to standard output (like true boolean value)
$out = true;
} elseif (in_array($value, $booleanValueDomain, true)) {
$out = filter_var($value, FILTER_VALIDATE_BOOLEAN);
} else {
$out = -1;
}
return $out;
}
}
20 changes: 15 additions & 5 deletions tests/Configuration/ConsoleConfigTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,15 @@
use Overtrue\PHPLint\Command\LintCommand;
use Overtrue\PHPLint\Configuration\ConsoleOptionsResolver;
use Overtrue\PHPLint\Configuration\OptionDefinition;
use Overtrue\PHPLint\Configuration\OptionsFactory;
use Overtrue\PHPLint\Configuration\Resolver;
use Overtrue\PHPLint\Event\EventDispatcher;
use Overtrue\PHPLint\Tests\TestCase;
use Symfony\Component\Console\Input\ArrayInput;

use Symfony\Component\OptionsResolver\OptionsResolver;

use function dirname;
use function is_string;
use function realpath;

final class ConsoleConfigTest extends TestCase
Expand Down Expand Up @@ -66,9 +68,17 @@ public static function commandInputProvider(): array
'multiple path modified' => [['path' => [dirname(__DIR__) . '/Cache', __DIR__]], __CLASS__ . '::expectedPathModified'],
'without external configuration' => [['--no-configuration' => true], __CLASS__ . '::expectedExternalConfigNotFetched'],
'with external empty configuration' => [['--configuration' => 'tests/Configuration/empty.yaml'], __CLASS__ . '::expectedExternalEmptyConfig'],
'output to JSON format on Stdout' => [['--log-json' => null], __CLASS__ . '::expectedJsonOutputFormat'],
'output to JSON format on Stdout 1/3' => [['--log-json' => null], __CLASS__ . '::expectedJsonOutputFormat'],
'output to JSON format on Stdout 2/3' => [['--log-json' => ''], __CLASS__ . '::expectedJsonOutputFormat'],
'output to JSON format on Stdout 3/3' => [['--log-json' => true], __CLASS__ . '::expectedJsonOutputFormat'],
'disable output to JSON format 1/2' => [['--log-json' => false], __CLASS__ . '::expectedJsonOutputFormat'],
'disable output to JSON format 2/2' => [['--log-json' => 'off'], __CLASS__ . '::expectedJsonOutputFormat'],
'output to JSON format on File' => [['--log-json' => '/tmp/phplint.json'], __CLASS__ . '::expectedJsonOutputFormat'],
'output to XML format on Stdout' => [['--log-junit' => true], __CLASS__ . '::expectedXmlOutputFormat'],
'output to XML format on Stdout 1/3' => [['--log-junit' => null], __CLASS__ . '::expectedXmlOutputFormat'],
'output to XML format on Stdout 2/3' => [['--log-junit' => ''], __CLASS__ . '::expectedXmlOutputFormat'],
'output to XML format on Stdout 3/3' => [['--log-junit' => true], __CLASS__ . '::expectedXmlOutputFormat'],
'disable output to XML format 1/2' => [['--log-junit' => false], __CLASS__ . '::expectedXmlOutputFormat'],
'disable output to XML format 2/2' => [['--log-junit' => 'FALSE'], __CLASS__ . '::expectedXmlOutputFormat'],
'output to XML format on File' => [['--log-junit' => '/tmp/phplint.xml'], __CLASS__ . '::expectedXmlOutputFormat'],
];
}
Expand Down Expand Up @@ -99,15 +109,15 @@ protected static function expectedJsonOutputFormat(Resolver $resolver, array $ar
{
$expected = self::getExpectedValues($resolver);
$logJson = $arguments['--log-json'];
$expected['log-json'] = (is_string($logJson)) ? $logJson : (empty($logJson) ? null : 'php://stdout');
$expected['log-json'] = OptionsFactory::logNormalizer(new OptionsResolver(), $logJson);
return $expected;
}

protected static function expectedXmlOutputFormat(Resolver $resolver, array $arguments): array
{
$expected = self::getExpectedValues($resolver);
$logJunit = $arguments['--log-junit'];
$expected['log-junit'] = (is_string($logJunit)) ? $logJunit : (empty($logJunit) ? null : 'php://stdout');
$expected['log-junit'] = OptionsFactory::logNormalizer(new OptionsResolver(), $logJunit);
return $expected;
}

Expand Down
10 changes: 6 additions & 4 deletions tests/Configuration/YamlConfigTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@

use Overtrue\PHPLint\Command\LintCommand;
use Overtrue\PHPLint\Configuration\FileOptionsResolver;
use Overtrue\PHPLint\Configuration\OptionDefinition;
use Overtrue\PHPLint\Configuration\OptionsFactory;
use Overtrue\PHPLint\Configuration\Resolver;
use Overtrue\PHPLint\Event\EventDispatcher;
use Overtrue\PHPLint\Tests\TestCase;
Expand Down Expand Up @@ -88,17 +90,17 @@ protected static function expectedJobsModified(Resolver $resolver): array
return $expected;
}

protected static function expectedJsonOutputFormat(Resolver $resolver): array
protected static function expectedJsonOutputFormat(Resolver $resolver, array $arguments): array
{
$expected = self::getExpectedValues($resolver);
$expected['log-json'] = 'php://stdout'; // see 'log-json.yaml' contents
$expected['log-json'] = OptionDefinition::DEFAULT_STANDARD_OUTPUT; // see 'log-json.yaml' contents
return $expected;
}

protected static function expectedXmlOutputFormat(Resolver $resolver): array
protected static function expectedXmlOutputFormat(Resolver $resolver, array $arguments): array
{
$expected = self::getExpectedValues($resolver);
$expected['log-junit'] = '/tmp/phplint-results.xml'; // see 'log-junit.yaml' contents
$expected['log-junit'] = '/tmp/phplint-results.xml'; // see 'log-junit.yaml' contents
return $expected;
}

Expand Down

0 comments on commit 4cb2bd2

Please sign in to comment.