diff --git a/docs/rules.md b/docs/rules.md index 3602fd01..ab1ec5ad 100644 --- a/docs/rules.md +++ b/docs/rules.md @@ -551,7 +551,7 @@ custom-extensions.rst Name | Required | Allowed Types | Default --- | --- | --- | --- -`directives` | `false` | `string[]` | `[]` +`directives` | `false` | `array` | `[]` ## `indention` diff --git a/src/Rule/ForbiddenDirectives.php b/src/Rule/ForbiddenDirectives.php index 42e56279..b01882f6 100644 --- a/src/Rule/ForbiddenDirectives.php +++ b/src/Rule/ForbiddenDirectives.php @@ -21,6 +21,8 @@ use App\Value\RuleGroup; use App\Value\Violation; use App\Value\ViolationInterface; +use Symfony\Component\OptionsResolver\Exception\InvalidOptionsException; +use Symfony\Component\OptionsResolver\Options; use Symfony\Component\OptionsResolver\OptionsResolver; #[Description('Make sure forbidden directives are not used')] @@ -29,7 +31,7 @@ class ForbiddenDirectives extends AbstractRule implements Configurable, LineCont use DirectiveTrait; /** - * @var array + * @var array */ private array $forbiddenDirectives; @@ -37,7 +39,24 @@ public function configureOptions(OptionsResolver $resolver): OptionsResolver { $resolver ->setRequired('directives') - ->setAllowedTypes('directives', 'string[]') + ->setAllowedTypes('directives', 'array') + ->setNormalizer('directives', static function (Options $options, $directives): array { + return \array_map(static function (array|string $directive) { + if (!\is_array($directive)) { + $directive = ['directive' => $directive]; + } + + if (isset($directive['replacements']) && \is_string($directive['replacements'])) { + $directive['replacements'] = [$directive['replacements']]; + } + + if (!isset($directive['directive']) || !\is_string($directive['directive'])) { + throw new InvalidOptionsException('A directive in "directives" is invalid. It needs at least a "directive" key with a string value'); + } + + return $directive; + }, $directives); + }) ->setDefault('directives', []); return $resolver; @@ -63,9 +82,17 @@ public function check(Lines $lines, int $number, string $filename): ViolationInt $line = $lines->current(); foreach ($this->forbiddenDirectives as $forbiddenDirective) { - if (RstParser::directiveIs($line, $forbiddenDirective)) { + if (RstParser::directiveIs($line, $forbiddenDirective['directive'])) { $message = \sprintf('Please don\'t use directive "%s" anymore', $line->raw()->toString()); + if (isset($forbiddenDirective['replacements'])) { + $message = \sprintf( + '%s, use "%s" instead', + $message, + \implode('" or "', $forbiddenDirective['replacements']), + ); + } + return Violation::from( $message, $filename, diff --git a/tests/Rule/ForbiddenDirectivesTest.php b/tests/Rule/ForbiddenDirectivesTest.php index 04aedc40..1a88147f 100644 --- a/tests/Rule/ForbiddenDirectivesTest.php +++ b/tests/Rule/ForbiddenDirectivesTest.php @@ -28,14 +28,11 @@ final class ForbiddenDirectivesTest extends UnitTestCase * * @dataProvider checkProvider */ - public function check(ViolationInterface $expected, RstSample $sample): void + public function check(array $directiveOptions, ViolationInterface $expected, RstSample $sample): void { $rule = new ForbiddenDirectives(); $rule->setOptions([ - 'directives' => [ - '.. index::', - '.. caution::', - ], + 'directives' => $directiveOptions, ]); self::assertEquals( @@ -45,11 +42,14 @@ public function check(ViolationInterface $expected, RstSample $sample): void } /** - * @return \Generator + * @return \Generator */ public static function checkProvider(): iterable { yield [ + [ + '.. index::', + ], Violation::from( 'Please don\'t use directive ".. index::" anymore', 'filename', @@ -62,8 +62,31 @@ public static function checkProvider(): iterable ]; yield [ + [ + [ + 'directive' => '.. notice::', + ], + ], Violation::from( - 'Please don\'t use directive ".. caution::" anymore', + 'Please don\'t use directive ".. notice::" anymore', + 'filename', + 1, + '.. notice::', + ), + new RstSample([ + '.. notice::', + ]), + ]; + + yield [ + [ + [ + 'directive' => '.. caution::', + 'replacements' => '.. warning::', + ], + ], + Violation::from( + 'Please don\'t use directive ".. caution::" anymore, use ".. warning::" instead', 'filename', 1, '.. caution::', @@ -74,6 +97,27 @@ public static function checkProvider(): iterable ]; yield [ + [ + [ + 'directive' => '.. caution::', + 'replacements' => ['.. warning::', '.. danger::'], + ], + ], + Violation::from( + 'Please don\'t use directive ".. caution::" anymore, use ".. warning::" or ".. danger::" instead', + 'filename', + 1, + '.. caution::', + ), + new RstSample([ + '.. caution::', + ]), + ]; + + yield [ + [ + '.. index::', + ], NullViolation::create(), new RstSample([ '.. tip::', @@ -81,6 +125,9 @@ public static function checkProvider(): iterable ]; yield [ + [ + '.. index::', + ], NullViolation::create(), new RstSample('temp'), ]; @@ -92,7 +139,7 @@ public static function checkProvider(): iterable public function invalidOptionType(): void { $this->expectExceptionObject( - new InvalidOptionsException('The option "directives" with value ".. caution::" is expected to be of type "string[]", but is of type "string".'), + new InvalidOptionsException('The option "directives" with value ".. caution::" is expected to be of type "array", but is of type "string".'), ); $rule = new ForbiddenDirectives(); @@ -101,6 +148,45 @@ public function invalidOptionType(): void ]); } + /** + * @test + */ + public function invalidDirective(): void + { + $this->expectExceptionObject( + new InvalidOptionsException('A directive in "directives" is invalid. It needs at least a "directive" key with a string value'), + ); + + $rule = new ForbiddenDirectives(); + $rule->setOptions([ + 'directives' => [ + [ + 'directive' => 2, + 'replacements' => '.. caution::', + ], + ], + ]); + } + + /** + * @test + */ + public function missingDirective(): void + { + $this->expectExceptionObject( + new InvalidOptionsException('A directive in "directives" is invalid. It needs at least a "directive" key with a string value'), + ); + + $rule = new ForbiddenDirectives(); + $rule->setOptions([ + 'directives' => [ + [ + 'replacements' => '.. caution::', + ], + ], + ]); + } + /** * @test */