Skip to content

Commit 313fdad

Browse files
authored
[rule] add useless regex rule (#23)
* col * add useless regex rule
1 parent 358ba09 commit 313fdad

File tree

8 files changed

+115
-35
lines changed

8 files changed

+115
-35
lines changed

README.md

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,21 @@ Feature: User Authentication
126126
127127
<br>
128128
129+
### 5. Rerport redundant regex definitions (`redundant-regex-definitions`)
130+
131+
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:
132+
133+
```diff
134+
-#[When('#I have apples#')]
135+
+#[When('I have apples')]
136+
public function iHaveApples()
137+
{
138+
// ...
139+
}
140+
```
141+
142+
<br>
143+
129144
*Protip*: Add this command to your CI, to get instant feedback of any changes in every pull-request.
130145

131146
That's it!

src/Analyzer/DuplicatedScenarioTitlesAnalyzer.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,6 @@ public function analyze(array $featureFiles): array
3333
}
3434
}
3535

36-
return array_filter($scenarioNamesToFiles, fn(array $files): bool => count($files) > 1);
36+
return array_filter($scenarioNamesToFiles, fn (array $files): bool => count($files) > 1);
3737
}
3838
}

src/Enum/RuleIdentifier.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,4 +13,6 @@ final class RuleIdentifier
1313
public const string DUPLICATED_PATTERNS = 'duplicated-patterns';
1414

1515
public const string UNUSED_DEFINITIONS = 'unused-definitions';
16+
17+
public const string REDUNDANT_REGEX_DEFINITION = 'redundant-regex-definition';
1618
}
Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Rector\Behastan\Rule;
6+
7+
use Rector\Behastan\Contract\RuleInterface;
8+
use Rector\Behastan\Enum\RuleIdentifier;
9+
use Rector\Behastan\ValueObject\Pattern\RegexPattern;
10+
use Rector\Behastan\ValueObject\PatternCollection;
11+
use Rector\Behastan\ValueObject\RuleError;
12+
use Symfony\Component\Finder\SplFileInfo;
13+
14+
final readonly class RedundantRegexDefinitionRule implements RuleInterface
15+
{
16+
/**
17+
* @param SplFileInfo[] $contextFiles
18+
* @param SplFileInfo[] $featureFiles
19+
* @return RuleError[]
20+
*/
21+
public function process(
22+
array $contextFiles,
23+
array $featureFiles,
24+
PatternCollection $patternCollection,
25+
string $projectDirectory
26+
): array {
27+
$ruleErrors = [];
28+
29+
/** @var RegexPattern[] $regexPatterns */
30+
$regexPatterns = $patternCollection->byType(RegexPattern::class);
31+
32+
foreach ($regexPatterns as $regexPattern) {
33+
// is regex pattern more then just exact start + end?
34+
if ($regexPattern->isRegexPatternNeccessary()) {
35+
continue;
36+
}
37+
38+
$errorMessage = sprintf(
39+
'The regex pattern "%s" is redundant. Remove start + end and use plain string exact string instead.',
40+
$regexPattern->pattern,
41+
);
42+
43+
$ruleErrors[] = new RuleError(
44+
$errorMessage,
45+
[$regexPattern->filePath . ':' . $regexPattern->line],
46+
$this->getIdentifier()
47+
);
48+
}
49+
50+
return $ruleErrors;
51+
}
52+
53+
public function getIdentifier(): string
54+
{
55+
return RuleIdentifier::REDUNDANT_REGEX_DEFINITION;
56+
}
57+
}

src/ValueObject/Pattern/RegexPattern.php

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,18 @@
44

55
namespace Rector\Behastan\ValueObject\Pattern;
66

7+
use Entropy\Utils\Regex;
8+
79
final class RegexPattern extends AbstractPattern
810
{
11+
public function isRegexPatternNeccessary(): bool
12+
{
13+
// simple exact match regexes are redundant
14+
if (Regex::match($this->pattern, '/^\/[^\^\$\.\*\+\?\|\(\)\[\]\{\}\\\]+\/$/')) {
15+
return false;
16+
}
17+
18+
return true;
19+
20+
}
921
}

src/ValueObject/PatternCollection.php

Lines changed: 7 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44

55
namespace Rector\Behastan\ValueObject;
66

7-
use InvalidArgumentException;
87
use Rector\Behastan\ValueObject\Pattern\AbstractPattern;
98
use Rector\Behastan\ValueObject\Pattern\ExactPattern;
109
use Rector\Behastan\ValueObject\Pattern\RegexPattern;
@@ -62,41 +61,17 @@ public function byType(string $type): array
6261
return array_filter($this->patterns, fn (AbstractPattern $pattern): bool => $pattern instanceof $type);
6362
}
6463

65-
public function regexPatternString(): string
66-
{
67-
$regexPatterns = $this->byType(RegexPattern::class);
68-
69-
$regexPatternStrings = array_map(
70-
fn (RegexPattern $regexPattern): string => $regexPattern->pattern,
71-
$regexPatterns
72-
);
73-
74-
return $this->combineRegexes($regexPatternStrings, '#');
75-
}
76-
7764
/**
78-
* @param string[] $regexes Like ['/foo/i', '~bar\d+~', '#baz#u']
65+
* @return string[]
7966
*/
80-
private function combineRegexes(array $regexes, string $delimiter = '#'): string
67+
public function regexPatternsStrings(): array
8168
{
82-
$parts = [];
83-
84-
foreach ($regexes as $regex) {
85-
// Very common case: regex is given like "/pattern/flags"
86-
// Parse: delimiter + pattern + delimiter + flags
87-
if (! preg_match('~^(.)(.*)\\1([a-zA-Z]*)$~s', $regex, $m)) {
88-
throw new InvalidArgumentException('Invalid regex: ' . $regex);
89-
}
90-
91-
$pattern = $m[2];
92-
$flags = $m[3];
69+
$regexPatterns = $this->byType(RegexPattern::class);
9370

94-
// If you truly have mixed flags per-regex, you can't naively merge them.
95-
// Best practice: normalize flags beforehand (same for all).
96-
// We'll ignore per-regex flags here and let the caller decide final flags.
97-
$parts[] = '(?:' . $pattern . ')';
98-
}
71+
$regexPatternStrings = array_map(function (RegexPattern $regexPattern): string {
72+
return $regexPattern->pattern;
73+
}, $regexPatterns);
9974

100-
return $delimiter . '(?:' . implode('|', $parts) . ')' . $delimiter;
75+
return array_values($regexPatternStrings);
10176
}
10277
}
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Rector\Behastan\Tests\ValueObject\Pattern;
6+
7+
use PHPUnit\Framework\TestCase;
8+
use Rector\Behastan\ValueObject\Pattern\RegexPattern;
9+
10+
final class RegexPatternTest extends TestCase
11+
{
12+
public function test(): void
13+
{
14+
$regexPattern = new RegexPattern('/foo/', 'someFilePath', 123, 'SomeClass', 'someMethod');
15+
$this->assertFalse($regexPattern->isRegexPatternNeccessary());
16+
17+
$regexPattern = new RegexPattern('/foo (.*) and that/', 'someFilePath', 123, 'SomeClass', 'someMethod');
18+
$this->assertTrue($regexPattern->isRegexPatternNeccessary());
19+
}
20+
}

tests/ValueObject/PatternCollectionTest.php

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@ public function testRegexPatterns(): void
3131
new RegexPattern('#here is more#', 'file1.php', 10, 'SomeClass', 'someMethod'),
3232
]);
3333

34-
$this->assertSame('#(?:(?:this is it)|(?:here is more))#', $patternCollection->regexPatternString());
35-
34+
$this->assertSame(['#this is it#', '#here is more#'], $patternCollection->regexPatternsStrings());
3635
}
3736
}

0 commit comments

Comments
 (0)