diff --git a/README.md b/README.md index 93cee658a..03b3b3f59 100644 --- a/README.md +++ b/README.md @@ -126,6 +126,21 @@ Feature: User Authentication
+### 5. Rerport redundant regex definitions (`redundant-regex-definitions`) + +When defining step definitions in Behat, it's common to use regular expressions to match patterns. However, sometimes these regex patterns can be overly complex or redundant, making them harder to read and maintain. This rule identifies such redundant regex definitions: + +```diff +-#[When('#I have apples#')] ++#[When('I have apples')] + public function iHaveApples() + { + // ... + } +``` + +
+ *Protip*: Add this command to your CI, to get instant feedback of any changes in every pull-request. That's it! diff --git a/src/Analyzer/DuplicatedScenarioTitlesAnalyzer.php b/src/Analyzer/DuplicatedScenarioTitlesAnalyzer.php index 7bfc74db7..378cfa516 100644 --- a/src/Analyzer/DuplicatedScenarioTitlesAnalyzer.php +++ b/src/Analyzer/DuplicatedScenarioTitlesAnalyzer.php @@ -33,6 +33,6 @@ public function analyze(array $featureFiles): array } } - return array_filter($scenarioNamesToFiles, fn(array $files): bool => count($files) > 1); + return array_filter($scenarioNamesToFiles, fn (array $files): bool => count($files) > 1); } } diff --git a/src/Enum/RuleIdentifier.php b/src/Enum/RuleIdentifier.php index 3f89a4dd8..af313a549 100644 --- a/src/Enum/RuleIdentifier.php +++ b/src/Enum/RuleIdentifier.php @@ -13,4 +13,6 @@ final class RuleIdentifier public const string DUPLICATED_PATTERNS = 'duplicated-patterns'; public const string UNUSED_DEFINITIONS = 'unused-definitions'; + + public const string REDUNDANT_REGEX_DEFINITION = 'redundant-regex-definition'; } diff --git a/src/Rule/RedundantRegexDefinitionRule.php b/src/Rule/RedundantRegexDefinitionRule.php new file mode 100644 index 000000000..6924362b1 --- /dev/null +++ b/src/Rule/RedundantRegexDefinitionRule.php @@ -0,0 +1,57 @@ +byType(RegexPattern::class); + + foreach ($regexPatterns as $regexPattern) { + // is regex pattern more then just exact start + end? + if ($regexPattern->isRegexPatternNeccessary()) { + continue; + } + + $errorMessage = sprintf( + 'The regex pattern "%s" is redundant. Remove start + end and use plain string exact string instead.', + $regexPattern->pattern, + ); + + $ruleErrors[] = new RuleError( + $errorMessage, + [$regexPattern->filePath . ':' . $regexPattern->line], + $this->getIdentifier() + ); + } + + return $ruleErrors; + } + + public function getIdentifier(): string + { + return RuleIdentifier::REDUNDANT_REGEX_DEFINITION; + } +} diff --git a/src/ValueObject/Pattern/RegexPattern.php b/src/ValueObject/Pattern/RegexPattern.php index 0c5de1983..3186a869e 100644 --- a/src/ValueObject/Pattern/RegexPattern.php +++ b/src/ValueObject/Pattern/RegexPattern.php @@ -4,6 +4,18 @@ namespace Rector\Behastan\ValueObject\Pattern; +use Entropy\Utils\Regex; + final class RegexPattern extends AbstractPattern { + public function isRegexPatternNeccessary(): bool + { + // simple exact match regexes are redundant + if (Regex::match($this->pattern, '/^\/[^\^\$\.\*\+\?\|\(\)\[\]\{\}\\\]+\/$/')) { + return false; + } + + return true; + + } } diff --git a/src/ValueObject/PatternCollection.php b/src/ValueObject/PatternCollection.php index c3affacb5..89e065698 100644 --- a/src/ValueObject/PatternCollection.php +++ b/src/ValueObject/PatternCollection.php @@ -4,7 +4,6 @@ namespace Rector\Behastan\ValueObject; -use InvalidArgumentException; use Rector\Behastan\ValueObject\Pattern\AbstractPattern; use Rector\Behastan\ValueObject\Pattern\ExactPattern; use Rector\Behastan\ValueObject\Pattern\RegexPattern; @@ -62,41 +61,17 @@ public function byType(string $type): array return array_filter($this->patterns, fn (AbstractPattern $pattern): bool => $pattern instanceof $type); } - public function regexPatternString(): string - { - $regexPatterns = $this->byType(RegexPattern::class); - - $regexPatternStrings = array_map( - fn (RegexPattern $regexPattern): string => $regexPattern->pattern, - $regexPatterns - ); - - return $this->combineRegexes($regexPatternStrings, '#'); - } - /** - * @param string[] $regexes Like ['/foo/i', '~bar\d+~', '#baz#u'] + * @return string[] */ - private function combineRegexes(array $regexes, string $delimiter = '#'): string + public function regexPatternsStrings(): array { - $parts = []; - - foreach ($regexes as $regex) { - // Very common case: regex is given like "/pattern/flags" - // Parse: delimiter + pattern + delimiter + flags - if (! preg_match('~^(.)(.*)\\1([a-zA-Z]*)$~s', $regex, $m)) { - throw new InvalidArgumentException('Invalid regex: ' . $regex); - } - - $pattern = $m[2]; - $flags = $m[3]; + $regexPatterns = $this->byType(RegexPattern::class); - // If you truly have mixed flags per-regex, you can't naively merge them. - // Best practice: normalize flags beforehand (same for all). - // We'll ignore per-regex flags here and let the caller decide final flags. - $parts[] = '(?:' . $pattern . ')'; - } + $regexPatternStrings = array_map(function (RegexPattern $regexPattern): string { + return $regexPattern->pattern; + }, $regexPatterns); - return $delimiter . '(?:' . implode('|', $parts) . ')' . $delimiter; + return array_values($regexPatternStrings); } } diff --git a/tests/ValueObject/Pattern/RegexPatternTest.php b/tests/ValueObject/Pattern/RegexPatternTest.php new file mode 100644 index 000000000..533cba338 --- /dev/null +++ b/tests/ValueObject/Pattern/RegexPatternTest.php @@ -0,0 +1,20 @@ +assertFalse($regexPattern->isRegexPatternNeccessary()); + + $regexPattern = new RegexPattern('/foo (.*) and that/', 'someFilePath', 123, 'SomeClass', 'someMethod'); + $this->assertTrue($regexPattern->isRegexPatternNeccessary()); + } +} diff --git a/tests/ValueObject/PatternCollectionTest.php b/tests/ValueObject/PatternCollectionTest.php index 1b3ab5d6b..e2e043784 100644 --- a/tests/ValueObject/PatternCollectionTest.php +++ b/tests/ValueObject/PatternCollectionTest.php @@ -31,7 +31,6 @@ public function testRegexPatterns(): void new RegexPattern('#here is more#', 'file1.php', 10, 'SomeClass', 'someMethod'), ]); - $this->assertSame('#(?:(?:this is it)|(?:here is more))#', $patternCollection->regexPatternString()); - + $this->assertSame(['#this is it#', '#here is more#'], $patternCollection->regexPatternsStrings()); } }