From ac73f70759da3fbeb0b91bbb1eac687b768d6453 Mon Sep 17 00:00:00 2001 From: Fran Moreno Date: Wed, 28 Apr 2021 01:22:27 +0200 Subject: [PATCH 1/2] Use severity defined in config file if not provided The argument was required, so it was always "warning" by default even if in the config file was defined as something else. With this change, it delegates determinating the severity to the Config class. --- src/Console/LintCommand.php | 40 ++++++++++--------------------------- 1 file changed, 11 insertions(+), 29 deletions(-) diff --git a/src/Console/LintCommand.php b/src/Console/LintCommand.php index 4617c14..031b153 100644 --- a/src/Console/LintCommand.php +++ b/src/Console/LintCommand.php @@ -23,7 +23,7 @@ public function configure() ->addArgument('paths', InputArgument::IS_ARRAY | InputArgument::OPTIONAL, 'The path to scan for twig files.', null) ->addOption('twig-version', 't', InputOption::VALUE_REQUIRED, 'The major version of twig to use.', 3) ->addOption('exclude', null, InputOption::VALUE_IS_ARRAY | InputOption::VALUE_REQUIRED, 'Excluded folder of path.', []) - ->addOption('severity', 's', InputOption::VALUE_REQUIRED, 'The maximum allowed error level.', 'warning') + ->addOption('severity', 's', InputOption::VALUE_OPTIONAL, 'The maximum allowed error level.') ->addOption('reporter', 'r', InputOption::VALUE_REQUIRED, 'The reporter to use.') ->addOption('display', 'd', InputOption::VALUE_REQUIRED, 'The violations to display, "'.self::DISPLAY_ALL.'" or "'.self::DISPLAY_BLOCKING.'".', self::DISPLAY_ALL) ->addOption('throw-syntax-error', 'e', InputOption::VALUE_NONE, 'Throw syntax error when a template contains an invalid token.') @@ -73,19 +73,19 @@ public function execute(InputInterface $input, OutputInterface $output) } } - $violations = $this->filterDisplayViolations($input, $violations); + $severityLimit = $resolver->getSeverityLimit(); + + $violations = $this->filterDisplayViolations($input, $severityLimit, $violations); $resolver->getReporter()->report($output, $violations); - return $this->determineExitCode($input, $violations); + return $this->determineExitCode($severityLimit, $violations); } - private function determineExitCode(InputInterface $input, array $violations): int + private function determineExitCode(int $severityLevel, array $violations): int { - $limit = $this->getSeverityLimit($input); - foreach ($violations as $violation) { - if ($violation->getSeverity() > $limit) { + if ($violation->getSeverity() > $severityLevel) { return 1; } } @@ -93,32 +93,14 @@ private function determineExitCode(InputInterface $input, array $violations): in return 0; } - private function filterDisplayViolations(InputInterface $input, array $violations): array + private function filterDisplayViolations(InputArgument $input, int $severityLevel, array $violations): array { - if (self::DISPLAY_ALL === $input->getOption('display')) { + if (ConfigInterface::DISPLAY_ALL === $input->getOption('display')) { return $violations; } - $limit = $this->getSeverityLimit($input); - - return array_filter($violations, function (Violation $violation) use ($limit) { - return $violation->getSeverity() > $limit; + return array_filter($violations, function (Violation $violation) use ($severityLevel) { + return $violation->getSeverity() > $severityLevel; }); } - - private function getSeverityLimit(InputInterface $input): int - { - switch ($input->getOption('severity')) { - case 'ignore': - return Violation::SEVERITY_IGNORE - 1; - case 'info': - return Violation::SEVERITY_INFO - 1; - case 'warning': - return Violation::SEVERITY_WARNING - 1; - case 'error': - return Violation::SEVERITY_ERROR - 1; - default: - throw new \InvalidArgumentException('Invalid severity limit provided.'); - } - } } From f082471affdca5594736a8e56518685884efdc83 Mon Sep 17 00:00:00 2001 From: Fran Moreno Date: Wed, 28 Apr 2021 01:26:28 +0200 Subject: [PATCH 2/2] Allow to configure display option from config file It also deprecates constants defined in LintCommand for display and have been moved to ConfigInterface. The display argument from the cli is now optional and it is determined from the Config class. --- src/Config/Config.php | 13 +++++++++ src/Config/ConfigInterface.php | 7 +++-- src/Config/ConfigResolver.php | 14 ++++++++++ src/Console/LintCommand.php | 17 +++++++++--- tests/Console/LintCommandTest.php | 27 ++++++++++++++----- .../.twig_cs_with_display_blocking.dist | 11 ++++++++ 6 files changed, 77 insertions(+), 12 deletions(-) create mode 100644 tests/data/config/external/.twig_cs_with_display_blocking.dist diff --git a/src/Config/Config.php b/src/Config/Config.php index 8941eb0..fd1f36b 100644 --- a/src/Config/Config.php +++ b/src/Config/Config.php @@ -15,6 +15,7 @@ class Config implements ConfigInterface private $reporter = 'console'; private $ruleset = Official::class; private $specificRulesets = []; + private $display = ConfigInterface::DISPLAY_ALL; public function __construct($name = 'default') { @@ -146,4 +147,16 @@ public function setSpecificRulesets(array $ruleSet): self return $this; } + + public function getDisplay(): string + { + return $this->display; + } + + public function setDisplay(string $display): self + { + $this->display = $display; + + return $this; + } } diff --git a/src/Config/ConfigInterface.php b/src/Config/ConfigInterface.php index b618218..2121a3a 100644 --- a/src/Config/ConfigInterface.php +++ b/src/Config/ConfigInterface.php @@ -2,13 +2,16 @@ namespace FriendsOfTwig\Twigcs\Config; -use Symfony\Component\Finder\Finder; - /** * Special thanks to https://github.com/c33s/twigcs/ which this feature was inspired from. + * + * @method string getDisplay() */ interface ConfigInterface { + public const DISPLAY_BLOCKING = 'blocking'; + public const DISPLAY_ALL = 'all'; + /** * Returns the name of the configuration. * diff --git a/src/Config/ConfigResolver.php b/src/Config/ConfigResolver.php index 5c1080f..9054829 100644 --- a/src/Config/ConfigResolver.php +++ b/src/Config/ConfigResolver.php @@ -44,6 +44,7 @@ final class ConfigResolver 'exclude' => [], 'config' => null, 'twig-version' => null, + 'display' => null, ]; private $finders; @@ -171,6 +172,19 @@ public function getSeverityLimit() } } + public function getDisplay(): string + { + if (null !== $this->options['display']) { + return $this->options['display']; + } + + if (!method_exists($this->getConfig(), 'getDisplay')) { + return ConfigInterface::DISPLAY_ALL; + } + + return $this->getConfig()->getDisplay(); + } + public function getConfig(): ConfigInterface { if (null === $this->config) { diff --git a/src/Console/LintCommand.php b/src/Console/LintCommand.php index 031b153..38d2128 100644 --- a/src/Console/LintCommand.php +++ b/src/Console/LintCommand.php @@ -2,6 +2,7 @@ namespace FriendsOfTwig\Twigcs\Console; +use FriendsOfTwig\Twigcs\Config\ConfigInterface; use FriendsOfTwig\Twigcs\Config\ConfigResolver; use FriendsOfTwig\Twigcs\TwigPort\Source; use FriendsOfTwig\Twigcs\TwigPort\SyntaxError; @@ -13,7 +14,14 @@ class LintCommand extends ContainerAwareCommand { + /** + * @deprecated use ConfigInterface::DISPLAY_BLOCKING instead + */ const DISPLAY_BLOCKING = 'blocking'; + + /** + * @deprecated use ConfigInterface::DISPLAY_ALL instead + */ const DISPLAY_ALL = 'all'; public function configure() @@ -25,7 +33,7 @@ public function configure() ->addOption('exclude', null, InputOption::VALUE_IS_ARRAY | InputOption::VALUE_REQUIRED, 'Excluded folder of path.', []) ->addOption('severity', 's', InputOption::VALUE_OPTIONAL, 'The maximum allowed error level.') ->addOption('reporter', 'r', InputOption::VALUE_REQUIRED, 'The reporter to use.') - ->addOption('display', 'd', InputOption::VALUE_REQUIRED, 'The violations to display, "'.self::DISPLAY_ALL.'" or "'.self::DISPLAY_BLOCKING.'".', self::DISPLAY_ALL) + ->addOption('display', 'd', InputOption::VALUE_OPTIONAL, 'The violations to display, "'.ConfigInterface::DISPLAY_ALL.'" or "'.ConfigInterface::DISPLAY_BLOCKING.'".') ->addOption('throw-syntax-error', 'e', InputOption::VALUE_NONE, 'Throw syntax error when a template contains an invalid token.') ->addOption('ruleset', null, InputOption::VALUE_REQUIRED, 'Ruleset class to use') ->addOption('config', null, InputOption::VALUE_REQUIRED, 'Config file to use', null) @@ -44,6 +52,7 @@ public function execute(InputInterface $input, OutputInterface $output) 'ruleset-class-name' => $input->getOption('ruleset'), 'twig-version' => $input->getOption('twig-version'), 'config' => $input->getOption('config'), + 'display' => $input->getOption('display'), ]); $finders = $resolver->getFinders(); @@ -75,7 +84,7 @@ public function execute(InputInterface $input, OutputInterface $output) $severityLimit = $resolver->getSeverityLimit(); - $violations = $this->filterDisplayViolations($input, $severityLimit, $violations); + $violations = $this->filterDisplayViolations($resolver->getDisplay(), $severityLimit, $violations); $resolver->getReporter()->report($output, $violations); @@ -93,9 +102,9 @@ private function determineExitCode(int $severityLevel, array $violations): int return 0; } - private function filterDisplayViolations(InputArgument $input, int $severityLevel, array $violations): array + private function filterDisplayViolations(string $display, int $severityLevel, array $violations): array { - if (ConfigInterface::DISPLAY_ALL === $input->getOption('display')) { + if (ConfigInterface::DISPLAY_ALL === $display) { return $violations; } diff --git a/tests/Console/LintCommandTest.php b/tests/Console/LintCommandTest.php index 7e35d7c..576eb71 100644 --- a/tests/Console/LintCommandTest.php +++ b/tests/Console/LintCommandTest.php @@ -2,6 +2,7 @@ namespace FriendsOfTwig\Twigcs\Tests\Console; +use FriendsOfTwig\Twigcs\Config\ConfigInterface; use FriendsOfTwig\Twigcs\Console\LintCommand; use FriendsOfTwig\Twigcs\Container; use FriendsOfTwig\Twigcs\TwigPort\SyntaxError; @@ -13,7 +14,7 @@ class LintCommandTest extends TestCase /** @var CommandTester */ private $commandTester; - public function setUp(): void + protected function setUp(): void { $container = new Container(); $command = new LintCommand(); @@ -113,7 +114,7 @@ public function testErrorsOnlyDisplayBlocking() $this->commandTester->execute([ 'paths' => ['tests/data/exclusion/bad/mixed.html.twig'], '--severity' => 'error', - '--display' => LintCommand::DISPLAY_BLOCKING, + '--display' => ConfigInterface::DISPLAY_BLOCKING, ]); $output = $this->commandTester->getDisplay(); @@ -130,7 +131,7 @@ public function testErrorsDisplayAll() $this->commandTester->execute([ 'paths' => ['tests/data/exclusion/bad/mixed.html.twig'], '--severity' => 'error', - '--display' => LintCommand::DISPLAY_ALL, + '--display' => ConfigInterface::DISPLAY_ALL, ]); $output = $this->commandTester->getDisplay(); @@ -148,7 +149,7 @@ public function testSyntaxErrorThrow() $this->commandTester->execute([ 'paths' => ['tests/data/syntax_error/syntax_errors.html.twig'], '--severity' => 'error', - '--display' => LintCommand::DISPLAY_ALL, + '--display' => ConfigInterface::DISPLAY_ALL, '--throw-syntax-error' => true, ]); @@ -161,7 +162,7 @@ public function testSyntaxErrorNotThrow() $this->commandTester->execute([ 'paths' => ['tests/data/syntax_error/syntax_errors.html.twig'], '--severity' => 'error', - '--display' => LintCommand::DISPLAY_ALL, + '--display' => ConfigInterface::DISPLAY_ALL, '--throw-syntax-error' => false, ]); @@ -177,7 +178,7 @@ public function testSyntaxErrorNotThrowOmitArgument() $this->commandTester->execute([ 'paths' => ['tests/data/syntax_error/syntax_errors.html.twig'], '--severity' => 'error', - '--display' => LintCommand::DISPLAY_ALL, + '--display' => ConfigInterface::DISPLAY_ALL, ]); $output = $this->commandTester->getDisplay(); @@ -223,6 +224,20 @@ public function testConfigFileWithCliPath() 3 violation(s) found', $output); } + public function testConfigFileWithDisplayAndSeverity() + { + $this->commandTester->execute([ + '--config' => 'tests/data/config/external/.twig_cs_with_display_blocking.dist', + ]); + + $output = $this->commandTester->getDisplay(); + $statusCode = $this->commandTester->getStatusCode(); + $this->assertSame($statusCode, 1); + $this->assertStringContainsString('tests/data/syntax_error/syntax_errors.html.twig +l.1 c.17 : ERROR Unexpected "}". +1 violation(s) found', $output); + } + public function testConfigFileSamePathWithRulesetOverrides() { chdir(__DIR__.'/../data/config/local'); diff --git a/tests/data/config/external/.twig_cs_with_display_blocking.dist b/tests/data/config/external/.twig_cs_with_display_blocking.dist new file mode 100644 index 0000000..74f532c --- /dev/null +++ b/tests/data/config/external/.twig_cs_with_display_blocking.dist @@ -0,0 +1,11 @@ +in(__DIR__.'/../../syntax_error') +; + +return \FriendsOfTwig\Twigcs\Config\Config::create() + ->addFinder($finder) + ->setSeverity('error') + ->setDisplay('blocking') +;