Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,21 @@ Feature: User Authentication

<br>

### 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()
{
// ...
}
```

<br>

*Protip*: Add this command to your CI, to get instant feedback of any changes in every pull-request.

That's it!
Expand Down
2 changes: 1 addition & 1 deletion src/Analyzer/DuplicatedScenarioTitlesAnalyzer.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
2 changes: 2 additions & 0 deletions src/Enum/RuleIdentifier.php
Original file line number Diff line number Diff line change
Expand Up @@ -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';
}
57 changes: 57 additions & 0 deletions src/Rule/RedundantRegexDefinitionRule.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
<?php

declare(strict_types=1);

namespace Rector\Behastan\Rule;

use Rector\Behastan\Contract\RuleInterface;
use Rector\Behastan\Enum\RuleIdentifier;
use Rector\Behastan\ValueObject\Pattern\RegexPattern;
use Rector\Behastan\ValueObject\PatternCollection;
use Rector\Behastan\ValueObject\RuleError;
use Symfony\Component\Finder\SplFileInfo;

final readonly class RedundantRegexDefinitionRule implements RuleInterface
{
/**
* @param SplFileInfo[] $contextFiles
* @param SplFileInfo[] $featureFiles
* @return RuleError[]
*/
public function process(
array $contextFiles,
array $featureFiles,
PatternCollection $patternCollection,
string $projectDirectory
): array {
$ruleErrors = [];

/** @var RegexPattern[] $regexPatterns */
$regexPatterns = $patternCollection->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;
}
}
12 changes: 12 additions & 0 deletions src/ValueObject/Pattern/RegexPattern.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;

}
}
39 changes: 7 additions & 32 deletions src/ValueObject/PatternCollection.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
}
20 changes: 20 additions & 0 deletions tests/ValueObject/Pattern/RegexPatternTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
<?php

declare(strict_types=1);

namespace Rector\Behastan\Tests\ValueObject\Pattern;

use PHPUnit\Framework\TestCase;
use Rector\Behastan\ValueObject\Pattern\RegexPattern;

final class RegexPatternTest extends TestCase
{
public function test(): void
{
$regexPattern = new RegexPattern('/foo/', 'someFilePath', 123, 'SomeClass', 'someMethod');
$this->assertFalse($regexPattern->isRegexPatternNeccessary());

$regexPattern = new RegexPattern('/foo (.*) and that/', 'someFilePath', 123, 'SomeClass', 'someMethod');
$this->assertTrue($regexPattern->isRegexPatternNeccessary());
}
}
3 changes: 1 addition & 2 deletions tests/ValueObject/PatternCollectionTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
}