diff --git a/README.md b/README.md index 4cb8756..54e83ed 100644 --- a/README.md +++ b/README.md @@ -27,7 +27,9 @@ See individual rule documentation for detailed configuration examples. A [full c - [Classname Must Match Pattern Rule](docs/rules/Classname-Must-Match-Pattern-Rule.md) - [Dependency Constraints Rule](docs/rules/Dependency-Constraints-Rule.md) *(deprecated, use Forbidden Dependencies Rule)* - [Forbidden Accessors Rule](docs/rules/Forbidden-Accessors-Rule.md) +- [Forbidden Business Logic Rule](docs/rules/Forbidden-Business-Logic-Rule.md) - [Forbidden Dependencies Rule](docs/rules/Forbidden-Dependencies-Rule.md) +- [Forbidden Date Time Comparison Rule](docs/rules/Forbidden-Date-Time-Comparison-Rule.md) - [Forbidden Namespaces Rule](docs/rules/Forbidden-Namespaces-Rule.md) - [Forbidden Static Methods Rule](docs/rules/Forbidden-Static-Methods-Rule.md) - [Method Must Return Type Rule](docs/rules/Method-Must-Return-Type-Rule.md) @@ -39,6 +41,7 @@ See individual rule documentation for detailed configuration examples. A [full c ### Clean Code Rules - [Control Structure Nesting Rule](docs/rules/Control-Structure-Nesting-Rule.md) +- [Forbidden Else Statements Rule](docs/rules/Forbidden-Else-Statements-Rule.md) - [Too Many Arguments Rule](docs/rules/Too-Many-Arguments-Rule.md) - [Max Line Length Rule](docs/rules/Max-Line-Length-Rule.md) @@ -50,7 +53,7 @@ The rules in this package can be extended at project level to create self-docume A lot of the rules use regex patterns to match things. Many people are not good at writing them but thankfully there is AI today. -If you struggle to write the regex patterns you need, you can use AI tools like [ChatGPT](https://chat.openai.com/) to help you generate them. Just describe what you want to match, and it can provide you with a regex pattern that fits your needs. The regex can be tested using online tools like [regex101](https://regex101.com/). +If you struggle to write the regex patterns you need, you can use AI agents to help you generate them. Just describe what you want to match, and it can provide you with a regex pattern that fits your needs. The regex can be tested using online tools like [regex101](https://regex101.com/). ## Why PHPStan to enforce Architectural Rules? diff --git a/data/ForbiddenBusinessLogicRule/EmptyGlobalFixture.php b/data/ForbiddenBusinessLogicRule/EmptyGlobalFixture.php new file mode 100644 index 0000000..6a4ac10 --- /dev/null +++ b/data/ForbiddenBusinessLogicRule/EmptyGlobalFixture.php @@ -0,0 +1,20 @@ +`): Global list of forbidden construct names: `if`, `for`, `foreach`, `while`, `switch`. +- `patterns` (`list}> | list`): Regex entries against `Fqcn::methodName`, optionally with per-entry `forbiddenStatements` that replace the effective list when that pattern matches (last such match wins). + +## Class-wide patterns + +Match every method on a class with a regex on the `::` prefix, for example `/^App\\\\Module\\\\Foo::/` or `/^App\\\\Module\\\\Foo::.+$/`. + +## Ignoring violations + +Use PHPStan baseline or inline `@phpstan-ignore` with a short justification when a rare exception is required. diff --git a/docs/rules/Forbidden-Date-Time-Comparison-Rule.md b/docs/rules/Forbidden-Date-Time-Comparison-Rule.md new file mode 100644 index 0000000..67d1350 --- /dev/null +++ b/docs/rules/Forbidden-Date-Time-Comparison-Rule.md @@ -0,0 +1,49 @@ +# Forbidden Date Time Comparison Rule + +Forbids strict identity comparisons (`===` and `!==`) when **both** operands are known to implement `DateTimeInterface`. + +## Why + +In PHP, `===` and `!==` on objects compare **identity** (same object instance in memory), not whether two values represent the **same point in time**. Two distinct `DateTimeImmutable` instances with the same instant still compare as not identical with `===`. That makes `===` / `!==` a frequent source of subtle bugs when the intent was to compare calendar instants. + +Loose equality (`==` / `!=`) for `DateTimeInterface` compares the dates for equality in the sense most callers expect. Alternatively, compare normalized instants explicitly (for example `getTimestamp()`, a canonical `format()`, or `DateTimeImmutable::createFromInterface()` followed by a defined comparison strategy). + +## Semantics of `patterns` + +Patterns use the same convention as other method-targeting rules in this package: each entry is a PCRE regex matched against **`Full\Class\Name::methodName`** (FQCN of the class, `::`, then the method name). + +- **`patterns` omitted or empty (`[]`):** the rule is **global**. Any `===` / `!==` between two definitely-`DateTimeInterface` operands is reported in **any** analyzed scope (class methods, namespaced functions, file-level code, and so on), wherever PHPStan can prove both types. + +- **`patterns` non-empty:** the rule runs **only inside class methods** where both the class and the enclosing function are known. The current method must match **at least one** pattern. Code outside a class method (for example a namespaced function) is **not** checked, because there is no `Fqcn::methodName` string in that form. + +This differs from the [Forbidden Else Statements Rule](Forbidden-Else-Statements-Rule.md): there, an empty `patterns` list **disables** the rule. Here, an empty list means **no scope filter** (apply everywhere). + +## Configuration example + +Global (entire codebase): + +```neon + - + class: Phauthentic\PHPStanRules\Architecture\ForbiddenDateTimeComparisonRule + arguments: + patterns: [] + tags: + - phpstan.rules.rule +``` + +Scoped to specific methods (regex on `Fqcn::methodName`): + +```neon + - + class: Phauthentic\PHPStanRules\Architecture\ForbiddenDateTimeComparisonRule + arguments: + patterns: + - '/^App\\\\Presentation\\\\Http\\\\.*Controller::(handle|__invoke)$/' + - '/^App\\\\Module\\\\.*::execute$/' + tags: + - phpstan.rules.rule +``` + +## Parameters + +- `patterns`: List of PCRE regex strings. When empty, the rule applies globally. When non-empty, each pattern is matched against `Full\Class\Name::methodName` for the enclosing class method; the comparison is only checked if any pattern matches. diff --git a/docs/rules/Forbidden-Else-Statements-Rule.md b/docs/rules/Forbidden-Else-Statements-Rule.md new file mode 100644 index 0000000..4069929 --- /dev/null +++ b/docs/rules/Forbidden-Else-Statements-Rule.md @@ -0,0 +1,26 @@ +# Forbidden Else Statements Rule + +Forbids plain `else` branches in class methods whose `Full\Class\Name::methodName` matches any of the configured regex patterns. This matches the same targeting convention as rules such as **Method Must Return Type Rule** (`Fqcn::methodName` as a single string). + +`elseif` is not checked (it is a different AST node). Prefer early returns, guard clauses, or `elseif` where appropriate. + +If `patterns` is empty, the rule does nothing. If analysis is not inside a class method (no class or no enclosing function in scope), the rule does nothing—for example, `else` at file scope or in a global function is not matched. + +To apply the rule to every method on a class, use a regex prefix on the method side of `::`, for example `/^App\\Module\\Foo::/` or `/^App\\Module\\Foo::.+$/`. + +## Configuration Example + +```neon + - + class: Phauthentic\PHPStanRules\CleanCode\ForbiddenElseStatementsRule + arguments: + patterns: + - '/^App\\Presentation\\Http\\.*Controller::(handle|__invoke)$/' + - '/^App\\Module\\.*::execute$/' + tags: + - phpstan.rules.rule +``` + +## Parameters + +- `patterns`: List of PCRE regex strings. Each pattern is matched against `Full\Class\Name::methodName` for the enclosing method. If any pattern matches, an `else` in that method is reported. diff --git a/phpstan.neon b/phpstan.neon index 25b6441..9372ec0 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -2,5 +2,6 @@ parameters: level: 8 paths: - src + - tests parallel: maximumNumberOfProcesses: 8 diff --git a/rule-builder.html b/rule-builder.html index 4cf19bc..42a09d8 100644 --- a/rule-builder.html +++ b/rule-builder.html @@ -340,6 +340,7 @@

PHPStan Rule Builder

+ @@ -555,6 +556,20 @@

PHPStan Rule Builder

} ] }, + ForbiddenDateTimeComparisonRule: { + description: 'Forbids === and !== between DateTimeInterface values (identity vs same instant). Optional patterns on Fqcn::methodName; empty patterns = global.', + class: 'Phauthentic\\PHPStanRules\\Architecture\\ForbiddenDateTimeComparisonRule', + fields: [ + { + name: 'patterns', + label: 'Method patterns (Fqcn::methodName)', + type: 'array', + required: false, + help: 'PCRE regexes matched against Full\\Class\\Name::methodName. Leave empty for global (all code).', + default: [] + } + ] + }, ForbiddenAccessorsRule: { description: 'Forbids public and/or protected getters and setters on classes matching specified patterns.', class: 'Phauthentic\\PHPStanRules\\Architecture\\ForbiddenAccessorsRule', @@ -1719,10 +1734,12 @@

PHPStan Rule Builder

yaml += `# ${rule.description}\n`; yaml += `-\n`; yaml += ` class: ${rule.class}\n`; - yaml += ` arguments:\n`; - - for (const [key, value] of Object.entries(data)) { - yaml += renderYamlValue(key, value, 8); + const dataKeys = Object.keys(data); + if (dataKeys.length > 0) { + yaml += ` arguments:\n`; + for (const [key, value] of Object.entries(data)) { + yaml += renderYamlValue(key, value, 8); + } } yaml += ` tags:\n`; diff --git a/src/Architecture/ForbiddenBusinessLogicRule.php b/src/Architecture/ForbiddenBusinessLogicRule.php new file mode 100644 index 0000000..fe76f59 --- /dev/null +++ b/src/Architecture/ForbiddenBusinessLogicRule.php @@ -0,0 +1,223 @@ + + */ +class ForbiddenBusinessLogicRule implements Rule +{ + private const ERROR_MESSAGE = 'Statement "%s" is not allowed in this method.'; + + private const IDENTIFIER = 'phauthentic.architecture.forbiddenBusinessLogic'; + + /** @var list */ + private const DEFAULT_FORBIDDEN = ['if', 'for', 'foreach', 'while', 'switch']; + + /** + * @var array lowercase construct name => canonical name + */ + private const KNOWN_CONSTRUCTS = [ + 'if' => 'if', + 'for' => 'for', + 'foreach' => 'foreach', + 'while' => 'while', + 'switch' => 'switch', + ]; + + /** + * @var list + */ + private array $globalForbiddenStatements; + + /** + * @var list}> + */ + private array $patterns; + + /** + * @param list $forbiddenStatements + * @param array $patterns Legacy string entries or arrays with `pattern` and optional `forbiddenStatements`. + */ + public function __construct( + array $forbiddenStatements = self::DEFAULT_FORBIDDEN, + array $patterns = [], + ) { + $this->globalForbiddenStatements = $this->normalizeConstructNames($forbiddenStatements); + $this->patterns = $this->normalizePatterns($patterns); + } + + public function getNodeType(): string + { + return Node::class; + } + + /** + * @return list + */ + public function processNode(Node $node, Scope $scope): array + { + $construct = $this->nodeToConstructName($node); + if ($construct === null) { + return []; + } + + $classReflection = $scope->getClassReflection(); + $functionReflection = $scope->getFunction(); + if ($classReflection === null || $functionReflection === null) { + return []; + } + + $fullName = $classReflection->getName() . '::' . $functionReflection->getName(); + $effective = $this->resolveForbiddenStatementsFor($fullName); + if ($effective === []) { + return []; + } + + if (!in_array($construct, $effective, true)) { + return []; + } + + return [ + RuleErrorBuilder::message(sprintf(self::ERROR_MESSAGE, $construct)) + ->identifier(self::IDENTIFIER) + ->line($node->getLine()) + ->build(), + ]; + } + + /** + * @return list + */ + private function resolveForbiddenStatementsFor(string $fullName): array + { + $effective = $this->globalForbiddenStatements; + + foreach ($this->patterns as $entry) { + if (preg_match($entry['pattern'], $fullName) !== 1) { + continue; + } + if (array_key_exists('forbiddenStatements', $entry)) { + $effective = $this->normalizeConstructNames($entry['forbiddenStatements']); + } + } + + return $effective; + } + + /** + * @param list $names + * @return list + */ + private function normalizeConstructNames(array $names): array + { + $out = []; + foreach ($names as $name) { + $key = strtolower($name); + if (!isset(self::KNOWN_CONSTRUCTS[$key])) { + continue; + } + $canonical = self::KNOWN_CONSTRUCTS[$key]; + $out[$canonical] = $canonical; + } + + return array_values($out); + } + + /** + * @param array $patterns + * @return list}> + */ + private function normalizePatterns(array $patterns): array + { + $out = []; + foreach ($patterns as $entry) { + if (is_string($entry)) { + $out[] = ['pattern' => $entry]; + continue; + } + if (!is_array($entry)) { + continue; + } + $pattern = $entry['pattern'] ?? null; + if (!is_string($pattern)) { + continue; + } + /** @var array{pattern: string, forbiddenStatements?: list} $normalized */ + $normalized = ['pattern' => $pattern]; + if (array_key_exists('forbiddenStatements', $entry)) { + $stmts = $entry['forbiddenStatements']; + if (is_array($stmts)) { + /** @var list $list */ + $list = $stmts; + $normalized['forbiddenStatements'] = $list; + } + } + $out[] = $normalized; + } + + return $out; + } + + private function nodeToConstructName(Node $node): ?string + { + if ($node instanceof If_) { + return 'if'; + } + if ($node instanceof For_) { + return 'for'; + } + if ($node instanceof Foreach_) { + return 'foreach'; + } + if ($node instanceof While_) { + return 'while'; + } + if ($node instanceof Switch_) { + return 'switch'; + } + + return null; + } +} diff --git a/src/Architecture/ForbiddenDateTimeComparisonRule.php b/src/Architecture/ForbiddenDateTimeComparisonRule.php new file mode 100644 index 0000000..a4530af --- /dev/null +++ b/src/Architecture/ForbiddenDateTimeComparisonRule.php @@ -0,0 +1,116 @@ +`). + * + * Unlike {@see \Phauthentic\PHPStanRules\CleanCode\ForbiddenElseStatementsRule}, an empty `patterns` list does not disable this rule; + * it means “no scope filter” (global). + * + * @implements Rule + */ +class ForbiddenDateTimeComparisonRule implements Rule +{ + use ClassNameResolver; + + private const IDENTIFIER = 'phauthentic.architecture.forbiddenDateTimeComparison'; + + private const ERROR_MESSAGE = 'Cannot compare DateTimeInterface values with %s: this compares object identity (whether both sides are the same in-memory PHP instance), not whether the two datetimes represent the same point in time. Use == / != for value comparison, or compare instants explicitly (e.g. getTimestamp(), format(), DateTimeImmutable::createFromInterface()).'; + + /** + * @param list $patterns PCRE regexes matched against `Full\Class\Name::methodName`. Empty = global. + */ + public function __construct( + private array $patterns = [], + ) { + } + + public function getNodeType(): string + { + return BinaryOp::class; + } + + /** + * @param BinaryOp $node + */ + public function processNode(Node $node, Scope $scope): array + { + if ( + !$node instanceof BinaryOp\Identical + && !$node instanceof BinaryOp\NotIdentical + ) { + return []; + } + + if (!$this->shouldAnalyzeScope($scope)) { + return []; + } + + $leftType = $scope->getType($node->left); + $rightType = $scope->getType($node->right); + $dateTimeType = new ObjectType(\DateTimeInterface::class); + + if ( + !$dateTimeType->isSuperTypeOf($leftType)->yes() + || !$dateTimeType->isSuperTypeOf($rightType)->yes() + ) { + return []; + } + + return [ + RuleErrorBuilder::message(sprintf( + self::ERROR_MESSAGE, + $node->getOperatorSigil() + )) + ->identifier(self::IDENTIFIER) + ->line($node->getLine()) + ->build(), + ]; + } + + private function shouldAnalyzeScope(Scope $scope): bool + { + if ($this->patterns === []) { + return true; + } + + $classReflection = $scope->getClassReflection(); + $functionReflection = $scope->getFunction(); + if ($classReflection === null || $functionReflection === null) { + return false; + } + + $fullName = $classReflection->getName() . '::' . $functionReflection->getName(); + + return $this->matchesAnyPattern($fullName, $this->patterns); + } +} diff --git a/src/Architecture/MethodSignatureMustMatchRule.php b/src/Architecture/MethodSignatureMustMatchRule.php index 467bad1..06cc698 100644 --- a/src/Architecture/MethodSignatureMustMatchRule.php +++ b/src/Architecture/MethodSignatureMustMatchRule.php @@ -57,8 +57,8 @@ class MethodSignatureMustMatchRule implements Rule * minParameters: null|int, * maxParameters: null|int, * signature: array, * visibilityScope?: string|null, * required?: bool|null @@ -125,7 +125,7 @@ private function validateMethod(ClassMethod $method, string $className): array } /** - * @param array{pattern: string, minParameters: int|null, maxParameters: int|null, signature: array, visibilityScope?: string|null, required?: bool|null} $config + * @param array{pattern: string, minParameters: int|null, maxParameters: int|null, signature: array, visibilityScope?: string|null, required?: bool|null} $config * @return list */ private function validateMethodAgainstConfig(ClassMethod $method, array $config, string $fullName): array @@ -228,7 +228,7 @@ private function validateParameterName(array $expected, Param $param, string $fu } /** - * @param array{pattern: string, minParameters: int|null, maxParameters: int|null, signature: array, visibilityScope?: string|null, required?: bool|null} $config + * @param array{pattern: string, minParameters: int|null, maxParameters: int|null, signature: array, visibilityScope?: string|null, required?: bool|null} $config */ private function checkVisibility(array $config, ClassMethod $method, string $fullName): ?IdentifierRuleError { @@ -255,7 +255,7 @@ private function checkVisibility(array $config, ClassMethod $method, string $ful } /** - * @param array{pattern: string, minParameters: int|null, maxParameters: int|null, signature: array, visibilityScope?: string|null, required?: bool|null} $config + * @param array{pattern: string, minParameters: int|null, maxParameters: int|null, signature: array, visibilityScope?: string|null, required?: bool|null} $config */ private function checkMinParameters(array $config, int $paramCount, string $fullName, int $line): ?IdentifierRuleError { @@ -270,7 +270,7 @@ private function checkMinParameters(array $config, int $paramCount, string $full } /** - * @param array{pattern: string, minParameters: int|null, maxParameters: int|null, signature: array, visibilityScope?: string|null, required?: bool|null} $config + * @param array{pattern: string, minParameters: int|null, maxParameters: int|null, signature: array, visibilityScope?: string|null, required?: bool|null} $config */ private function checkMaxParameters(array $config, int $paramCount, string $fullName, int $line): ?IdentifierRuleError { @@ -339,7 +339,7 @@ private function classMatchesPattern(string $className, string $classPattern): b /** * Format the expected method signature for error messages. * - * @param array{pattern: string, minParameters: int|null, maxParameters: int|null, signature: array, visibilityScope?: string|null, required?: bool|null} $patternConfig + * @param array{pattern: string, minParameters: int|null, maxParameters: int|null, signature: array, visibilityScope?: string|null, required?: bool|null} $patternConfig */ private function formatSignatureForError(array $patternConfig): string { @@ -364,7 +364,7 @@ private function formatSignatureForError(array $patternConfig): string if (!empty($patternConfig['signature'])) { foreach ($patternConfig['signature'] as $i => $sig) { $paramParts = []; - if ($sig['type'] !== '') { + if (($sig['type'] ?? '') !== '') { $paramParts[] = $sig['type']; } $paramParts[] = '$param' . ($i + 1); @@ -397,7 +397,7 @@ private function checkRequiredMethods(Class_ $node, string $className): array } /** - * @param array{pattern: string, minParameters: int|null, maxParameters: int|null, signature: array, visibilityScope?: string|null, required?: bool|null} $config + * @param array{pattern: string, minParameters: int|null, maxParameters: int|null, signature: array, visibilityScope?: string|null, required?: bool|null} $config * @param array $implementedMethods */ private function checkRequiredMethod(array $config, string $className, array $implementedMethods, int $line): ?IdentifierRuleError diff --git a/src/CleanCode/ForbiddenElseStatementsRule.php b/src/CleanCode/ForbiddenElseStatementsRule.php new file mode 100644 index 0000000..42f0cfa --- /dev/null +++ b/src/CleanCode/ForbiddenElseStatementsRule.php @@ -0,0 +1,97 @@ + + */ +class ForbiddenElseStatementsRule implements Rule +{ + private const ERROR_MESSAGE = 'Else is not allowed in %s; prefer early returns or guard clauses.'; + + private const IDENTIFIER = 'phauthentic.cleancode.forbiddenElseStatements'; + + /** + * @param string[] $patterns Regex patterns matched against `Full\Class\Name::methodName` + */ + public function __construct( + private array $patterns = [], + ) { + } + + public function getNodeType(): string + { + return Else_::class; + } + + /** + * @param Else_ $node + * @return list + * @throws ShouldNotHappenException + */ + public function processNode(Node $node, Scope $scope): array + { + if ($this->patterns === []) { + return []; + } + + $classReflection = $scope->getClassReflection(); + $function = $scope->getFunction(); + if ($classReflection === null || $function === null) { + return []; + } + + $fullName = $classReflection->getName() . '::' . $function->getName(); + if (!$this->matchesAnyPattern($fullName)) { + return []; + } + + return [ + RuleErrorBuilder::message(sprintf(self::ERROR_MESSAGE, $fullName)) + ->identifier(self::IDENTIFIER) + ->line($node->getLine()) + ->build(), + ]; + } + + private function matchesAnyPattern(string $fullName): bool + { + foreach ($this->patterns as $pattern) { + if (preg_match($pattern, $fullName) === 1) { + return true; + } + } + + return false; + } +} diff --git a/tests/TestCases/Architecture/ClassMustBeReadonlyRuleTest.php b/tests/TestCases/Architecture/ClassMustBeReadonlyRuleTest.php index 24076f6..d3da4d6 100644 --- a/tests/TestCases/Architecture/ClassMustBeReadonlyRuleTest.php +++ b/tests/TestCases/Architecture/ClassMustBeReadonlyRuleTest.php @@ -8,11 +8,11 @@ use PHPStan\Testing\RuleTestCase; /** - * @extends RuleTestCase + * @extends RuleTestCase */ class ClassMustBeReadonlyRuleTest extends RuleTestCase { - protected function getRule(): \PHPStan\Rules\Rule + protected function getRule(): ClassMustBeReadonlyRule { return new ClassMustBeReadonlyRule([ '/Controller$/', // all classes that end with "Controller" diff --git a/tests/TestCases/Architecture/ForbiddenBusinessLogicRuleDefaultsTest.php b/tests/TestCases/Architecture/ForbiddenBusinessLogicRuleDefaultsTest.php new file mode 100644 index 0000000..d53b35b --- /dev/null +++ b/tests/TestCases/Architecture/ForbiddenBusinessLogicRuleDefaultsTest.php @@ -0,0 +1,30 @@ + + */ +class ForbiddenBusinessLogicRuleDefaultsTest extends RuleTestCase +{ + protected function getRule(): Rule + { + return new ForbiddenBusinessLogicRule(); + } + + public function testDefaultConstructorForbidsAllFiveWithEmptyPatterns(): void + { + $this->analyse([__DIR__ . '/../../../data/ForbiddenBusinessLogicRule/MinimalDefaults.php'], [ + [ + 'Statement "if" is not allowed in this method.', + 11, + ], + ]); + } +} diff --git a/tests/TestCases/Architecture/ForbiddenBusinessLogicRuleEmptyGlobalTest.php b/tests/TestCases/Architecture/ForbiddenBusinessLogicRuleEmptyGlobalTest.php new file mode 100644 index 0000000..750923c --- /dev/null +++ b/tests/TestCases/Architecture/ForbiddenBusinessLogicRuleEmptyGlobalTest.php @@ -0,0 +1,35 @@ + + */ +class ForbiddenBusinessLogicRuleEmptyGlobalTest extends RuleTestCase +{ + protected function getRule(): Rule + { + return new ForbiddenBusinessLogicRule( + [], + [ + ['pattern' => '/^App\\\\ForbiddenBusinessLogicRule\\\\EmptyGlobalFixture::onlyIf$/', 'forbiddenStatements' => ['if']], + ] + ); + } + + public function testEmptyGlobalUsesPatternOverrideOnly(): void + { + $this->analyse([__DIR__ . '/../../../data/ForbiddenBusinessLogicRule/EmptyGlobalFixture.php'], [ + [ + 'Statement "if" is not allowed in this method.', + 11, + ], + ]); + } +} diff --git a/tests/TestCases/Architecture/ForbiddenBusinessLogicRuleTest.php b/tests/TestCases/Architecture/ForbiddenBusinessLogicRuleTest.php new file mode 100644 index 0000000..c9d8536 --- /dev/null +++ b/tests/TestCases/Architecture/ForbiddenBusinessLogicRuleTest.php @@ -0,0 +1,55 @@ + + */ +class ForbiddenBusinessLogicRuleTest extends RuleTestCase +{ + protected function getRule(): Rule + { + return new ForbiddenBusinessLogicRule( + ['if', 'for', 'foreach', 'while', 'switch'], + [ + ['pattern' => '/^App\\\\ForbiddenBusinessLogicRule\\\\ScenarioFixture::onlyIf$/', 'forbiddenStatements' => ['if']], + '/::noOverrideX$/', + ['pattern' => '/::unknownNamesN$/', 'forbiddenStatements' => ['bogus', 'if']], + ['pattern' => '/^App\\\\ForbiddenBusinessLogicRule\\\\ScenarioFixture::lastWins/', 'forbiddenStatements' => ['if', 'for']], + ['pattern' => '/^App\\\\ForbiddenBusinessLogicRule\\\\ScenarioFixture::lastWinsM$/', 'forbiddenStatements' => ['switch']], + ] + ); + } + + public function testRule(): void + { + $this->analyse([__DIR__ . '/../../../data/ForbiddenBusinessLogicRule/ScenarioFixture.php'], [ + [ + 'Statement "if" is not allowed in this method.', + 14, + ], + [ + 'Statement "if" is not allowed in this method.', + 20, + ], + [ + 'Statement "if" is not allowed in this method.', + 28, + ], + [ + 'Statement "switch" is not allowed in this method.', + 38, + ], + [ + 'Statement "if" is not allowed in this method.', + 46, + ], + ]); + } +} diff --git a/tests/TestCases/Architecture/ForbiddenDateTimeComparisonRuleScopedTest.php b/tests/TestCases/Architecture/ForbiddenDateTimeComparisonRuleScopedTest.php new file mode 100644 index 0000000..8c08080 --- /dev/null +++ b/tests/TestCases/Architecture/ForbiddenDateTimeComparisonRuleScopedTest.php @@ -0,0 +1,42 @@ + + */ +class ForbiddenDateTimeComparisonRuleScopedTest extends RuleTestCase +{ + private const MSG_IDENTICAL = 'Cannot compare DateTimeInterface values with ===: this compares object identity (whether both sides are the same in-memory PHP instance), not whether the two datetimes represent the same point in time. Use == / != for value comparison, or compare instants explicitly (e.g. getTimestamp(), format(), DateTimeImmutable::createFromInterface()).'; + + protected function getRule(): Rule + { + return new ForbiddenDateTimeComparisonRule([ + '/^App\\\\ForbiddenDateTimeComparison\\\\ScopedViolations::matchedMethod$/', + ]); + } + + public function testOnlyMatchedMethodReports(): void + { + $this->analyse( + [__DIR__ . '/../../../data/ForbiddenDateTimeComparison/ScopedViolations.php'], + [ + [self::MSG_IDENTICAL, 13], + ] + ); + } + + public function testNamespacedFunctionSkippedWhenPatternsNonEmpty(): void + { + $this->analyse( + [__DIR__ . '/../../../data/ForbiddenDateTimeComparison/GlobalFunctionViolations.php'], + [] + ); + } +} diff --git a/tests/TestCases/Architecture/ForbiddenDateTimeComparisonRuleTest.php b/tests/TestCases/Architecture/ForbiddenDateTimeComparisonRuleTest.php new file mode 100644 index 0000000..ae6e968 --- /dev/null +++ b/tests/TestCases/Architecture/ForbiddenDateTimeComparisonRuleTest.php @@ -0,0 +1,47 @@ + + */ +class ForbiddenDateTimeComparisonRuleTest extends RuleTestCase +{ + /** Must match {@see ForbiddenDateTimeComparisonRule} output exactly for RuleTestCase. */ + private const MSG_IDENTICAL = 'Cannot compare DateTimeInterface values with ===: this compares object identity (whether both sides are the same in-memory PHP instance), not whether the two datetimes represent the same point in time. Use == / != for value comparison, or compare instants explicitly (e.g. getTimestamp(), format(), DateTimeImmutable::createFromInterface()).'; + + /** @see self::MSG_IDENTICAL */ + private const MSG_NOT_IDENTICAL = 'Cannot compare DateTimeInterface values with !==: this compares object identity (whether both sides are the same in-memory PHP instance), not whether the two datetimes represent the same point in time. Use == / != for value comparison, or compare instants explicitly (e.g. getTimestamp(), format(), DateTimeImmutable::createFromInterface()).'; + + protected function getRule(): Rule + { + return new ForbiddenDateTimeComparisonRule([]); + } + + public function testGlobalReportsInClassMethods(): void + { + $this->analyse( + [__DIR__ . '/../../../data/ForbiddenDateTimeComparison/GlobalViolations.php'], + [ + [self::MSG_IDENTICAL, 13], + [self::MSG_NOT_IDENTICAL, 18], + ] + ); + } + + public function testGlobalReportsOutsideClassMethods(): void + { + $this->analyse( + [__DIR__ . '/../../../data/ForbiddenDateTimeComparison/GlobalFunctionViolations.php'], + [ + [self::MSG_IDENTICAL, 11], + ] + ); + } +} diff --git a/tests/TestCases/CleanCode/ForbiddenElseStatementsRuleClassWideTest.php b/tests/TestCases/CleanCode/ForbiddenElseStatementsRuleClassWideTest.php new file mode 100644 index 0000000..380da69 --- /dev/null +++ b/tests/TestCases/CleanCode/ForbiddenElseStatementsRuleClassWideTest.php @@ -0,0 +1,38 @@ + + */ +class ForbiddenElseStatementsRuleClassWideTest extends RuleTestCase +{ + private const FIXTURE = __DIR__ . '/../../../data/ForbiddenElseStatementsRuleFixture.php'; + + protected function getRule(): Rule + { + return new ForbiddenElseStatementsRule([ + '/^App\\\\ElseRules\\\\Matched::/', + ]); + } + + public function testClassWidePatternMatchesAllMethodsOnClass(): void + { + $this->analyse([self::FIXTURE], [ + [ + 'Else is not allowed in App\ElseRules\Matched::matchedMethod; prefer early returns or guard clauses.', + 13, + ], + [ + 'Else is not allowed in App\ElseRules\Matched::anotherMatched; prefer early returns or guard clauses.', + 22, + ], + ]); + } +} diff --git a/tests/TestCases/CleanCode/ForbiddenElseStatementsRuleEmptyPatternsTest.php b/tests/TestCases/CleanCode/ForbiddenElseStatementsRuleEmptyPatternsTest.php new file mode 100644 index 0000000..5dcdac8 --- /dev/null +++ b/tests/TestCases/CleanCode/ForbiddenElseStatementsRuleEmptyPatternsTest.php @@ -0,0 +1,27 @@ + + */ +class ForbiddenElseStatementsRuleEmptyPatternsTest extends RuleTestCase +{ + private const FIXTURE = __DIR__ . '/../../../data/ForbiddenElseStatementsRuleFixture.php'; + + protected function getRule(): Rule + { + return new ForbiddenElseStatementsRule([]); + } + + public function testEmptyPatternsIsNoOp(): void + { + $this->analyse([self::FIXTURE], []); + } +} diff --git a/tests/TestCases/CleanCode/ForbiddenElseStatementsRuleNoMatchTest.php b/tests/TestCases/CleanCode/ForbiddenElseStatementsRuleNoMatchTest.php new file mode 100644 index 0000000..10ffcdd --- /dev/null +++ b/tests/TestCases/CleanCode/ForbiddenElseStatementsRuleNoMatchTest.php @@ -0,0 +1,29 @@ + + */ +class ForbiddenElseStatementsRuleNoMatchTest extends RuleTestCase +{ + private const FIXTURE = __DIR__ . '/../../../data/ForbiddenElseStatementsRuleFixture.php'; + + protected function getRule(): Rule + { + return new ForbiddenElseStatementsRule([ + '/^App\\\\ElseRules\\\\DoesNotExist::/', + ]); + } + + public function testNoReportWhenPatternDoesNotMatch(): void + { + $this->analyse([self::FIXTURE], []); + } +} diff --git a/tests/TestCases/CleanCode/ForbiddenElseStatementsRuleTest.php b/tests/TestCases/CleanCode/ForbiddenElseStatementsRuleTest.php new file mode 100644 index 0000000..c8bf90a --- /dev/null +++ b/tests/TestCases/CleanCode/ForbiddenElseStatementsRuleTest.php @@ -0,0 +1,34 @@ + + */ +class ForbiddenElseStatementsRuleTest extends RuleTestCase +{ + private const FIXTURE = __DIR__ . '/../../../data/ForbiddenElseStatementsRuleFixture.php'; + + protected function getRule(): Rule + { + return new ForbiddenElseStatementsRule([ + '/^App\\\\ElseRules\\\\Matched::matchedMethod$/', + ]); + } + + public function testReportsElseWhenFqcnMethodMatches(): void + { + $this->analyse([self::FIXTURE], [ + [ + 'Else is not allowed in App\ElseRules\Matched::matchedMethod; prefer early returns or guard clauses.', + 13, + ], + ]); + } +}