From 4bafd07620d85091cfddc776550a5a5ff20ed1ce Mon Sep 17 00:00:00 2001 From: Simon Baese Date: Wed, 14 Aug 2024 23:53:22 +0900 Subject: [PATCH] feat: add rule to make sure that raw filter is justified by a comment --- src/RawFilterRule.php | 79 ++++++++++++++++++++++++++++ src/TwigCsRuleset.php | 3 +- tests/RulesetFunctionalTest.php | 63 ++++++++++++++++++++++ tests/WithOnlyRuleFunctionalTest.php | 52 ------------------ tests/assets/fail.twig | 3 ++ tests/assets/pass.twig | 9 ++++ 6 files changed, 156 insertions(+), 53 deletions(-) create mode 100644 src/RawFilterRule.php create mode 100644 tests/RulesetFunctionalTest.php delete mode 100644 tests/WithOnlyRuleFunctionalTest.php diff --git a/src/RawFilterRule.php b/src/RawFilterRule.php new file mode 100644 index 0000000..af1c466 --- /dev/null +++ b/src/RawFilterRule.php @@ -0,0 +1,79 @@ +isEOF()) { + if ($tokens->isEOF()) { + break; + } + $token = $tokens->getCurrent(); + if ($new_violations = $this->validateRawFilterComment($token, $tokens)) { + $violations[] = $new_violations; + } + $tokens->next(); + } + + return array_merge(...$violations); + } + + /** + * Validates that any raw filter is preceded by a comment. + */ + protected function validateRawFilterComment(Token $token, TokenStream &$tokens): array { + + $violations = []; + + // Consider "|raw" filter. + if ($token->getValue() === 'raw' && + Token::NAME_TYPE === $token->getType() && + Token::PUNCTUATION_TYPE === $tokens->look(Lexer::PREVIOUS_TOKEN)->getType() + ) { + + $line = $token->getLine(); + $column = $token->getColumn() + 1; + $reason = 'usage of the "raw" filter should be justified in a preceding comment.'; + + // If the "raw" filter appears on the first line there can not be a + // preceding comment. + if ($line === 1) { + $violations[] = $this->createViolation($tokens->getSourceContext()->getPath(), $line, $column, $reason); + return $violations; + } + + // Look behind until the previous line is reached. + $look_behind = Lexer::PREVIOUS_TOKEN; + while ($previous_token = $tokens->look($look_behind)) { + if ($previous_token->getLine() !== $line) { + break; + } + $look_behind--; + } + + // Check whether the previous line is a comment. + if ($previous_token->getType() !== Token::COMMENT_TYPE) { + $violations[] = $this->createViolation($tokens->getSourceContext()->getPath(), $line, $column, $reason); + } + } + + return $violations; + } + +} diff --git a/src/TwigCsRuleset.php b/src/TwigCsRuleset.php index 1c7078e..b44c2c8 100644 --- a/src/TwigCsRuleset.php +++ b/src/TwigCsRuleset.php @@ -13,10 +13,11 @@ class TwigCsRuleset extends Official { /** * @{inheritdoc} */ - public function getRules() { + public function getRules(): array { $rules = parent::getRules(); return array_merge($rules, [ new WithOnlyRule(Violation::SEVERITY_ERROR), + new RawFilterRule(Violation::SEVERITY_WARNING), ]); } diff --git a/tests/RulesetFunctionalTest.php b/tests/RulesetFunctionalTest.php new file mode 100644 index 0000000..3872323 --- /dev/null +++ b/tests/RulesetFunctionalTest.php @@ -0,0 +1,63 @@ + + */ +class RulesetFunctionalTest extends TestCase { + + /** + * @dataProvider getData + */ + public function testExpressions($expression, $violationCount): void { + $lexer = new Lexer(); + $validator = new Validator(); + + $violations = $validator->validate(new TwigCsRuleset(2), $lexer->tokenize(new Source($expression, 'src', 'src.html.twig'))); + $this->assertCount(0, $validator->getCollectedData()[RegEngineRule::class]['unrecognized_expressions'] ?? []); + + if ($violationCount) { + $this->assertCount($violationCount, $violations, sprintf("There should be %d violation in:\n %s", $violationCount, $expression)); + } + else { + $this->assertCount(0, $violations, sprintf("There should be no violations in:\n %s", $expression)); + } + } + + /** + * Provides test scenarios. + */ + public function getData(): array { + return [ + // NOTE: We expect exactly 17 violations in the fail scenario. + // In case we add more, don't forget to update violation count. + [$this->getFailScenarios(), 17], + [$this->getPassScenarios(), 0], + ]; + } + + /** + * Gets failure scenarios. + */ + public function getFailScenarios() { + return file_get_contents(__DIR__ . '/assets/fail.twig'); + } + + /** + * Gets pass scenarios. + */ + public function getPassScenarios() { + return file_get_contents(__DIR__ . '/assets/pass.twig'); + } + +} diff --git a/tests/WithOnlyRuleFunctionalTest.php b/tests/WithOnlyRuleFunctionalTest.php deleted file mode 100644 index 265a7c4..0000000 --- a/tests/WithOnlyRuleFunctionalTest.php +++ /dev/null @@ -1,52 +0,0 @@ - - */ -class WithOnlyRuleFunctionalTest extends TestCase -{ - /** - * @dataProvider getData - */ - public function testExpressions($expression, $violationCount) - { - $lexer = new Lexer(); - $validator = new Validator(); - - $violations = $validator->validate(new TwigCsRuleset(2), $lexer->tokenize(new Source($expression, 'src', 'src.html.twig'))); - $this->assertCount(0, $validator->getCollectedData()[RegEngineRule::class]['unrecognized_expressions'] ?? []); - - if ($violationCount) { - $this->assertCount($violationCount, $violations, sprintf("There should be %d violation in:\n %s", $violationCount, $expression)); - } else { - $this->assertCount(0, $violations, sprintf("There should be no violations in:\n %s", $expression)); - } - } - - public function getData() - { - return [ - // NOTE: We expect exactly 15 violations in the fail scenario. - // In case we add more, don't forget to update violation count. - [$this->getFailScenarios(), 15], - [$this->getPassScenarios(), 0], - ]; - } - - public function getFailScenarios() { - return file_get_contents(__DIR__ . '/assets/fail.twig'); - } - - public function getPassScenarios() { - return file_get_contents(__DIR__ . '/assets/pass.twig'); - } -} \ No newline at end of file diff --git a/tests/assets/fail.twig b/tests/assets/fail.twig index c08b280..3aaa180 100644 --- a/tests/assets/fail.twig +++ b/tests/assets/fail.twig @@ -26,4 +26,7 @@ FAIL: {% include "@elements/icon/icon.twig" with { only } %} + FAIL: {{ content|raw }} + FAIL: {# Lorem ipsum dolor sit amet. #} {{ content|raw }} + diff --git a/tests/assets/pass.twig b/tests/assets/pass.twig index 3135c9f..26eb460 100644 --- a/tests/assets/pass.twig +++ b/tests/assets/pass.twig @@ -28,4 +28,13 @@ PASS: {% include "@elements/icon/icon.twig" with { only } only %} + PASS: + {# Lorem ipsum dolor sit amet. #} + {{ content|raw }} + PASS: + {# + Lorem ipsum + dolor sit amet. + #} + {{ content|raw }}