diff --git a/README.md b/README.md index a92ab91e..517f99a1 100644 --- a/README.md +++ b/README.md @@ -98,6 +98,36 @@ $dateTime = new \DateTime();
+### NoWithOnStubRule + +Disallow `with()` on stubs (mocks without `expects()`). PHPUnit deprecates `with*()` on test stubs because they silently swallow argument mismatches. + +```yaml +rules: + - Rector\Mockstan\Rules\NoWithOnStubRule +``` + +```php +$someMock = $this->createMock(Service::class); +$someMock->method('calculate') + ->with(10) + ->willReturn(20); +``` + +:x: + +```php +$someMock = $this->createMock(Service::class); +$someMock->expects($this->once()) + ->method('calculate') + ->with(10) + ->willReturn(20); +``` + +:+1: + +
+ ### NoDocumentMockingRule Prevent mocking of Doctrine ODM document classes. Use real instances instead. diff --git a/config/phpunit-mocks-rules.neon b/config/phpunit-mocks-rules.neon index 64464029..c1375b8c 100644 --- a/config/phpunit-mocks-rules.neon +++ b/config/phpunit-mocks-rules.neon @@ -9,6 +9,7 @@ rules: # explicit expects() - Rector\Mockstan\Rules\ExplicitExpectsMockMethodRule - Rector\Mockstan\Rules\AvoidAnyExpectsRule + - Rector\Mockstan\Rules\NoWithOnStubRule - Rector\Mockstan\Rules\RequireAtLeastOneRule # better alternative than mocks diff --git a/phpunit.xml b/phpunit.xml index 7ab9f4f9..0040c118 100644 --- a/phpunit.xml +++ b/phpunit.xml @@ -16,5 +16,6 @@ tests/Rules/ClassNameRespectsParentSuffixRule/Fixture/ tests/Rules/ExplicitExpectsMockMethodRule/Fixture tests/Rules/NoMockOnlyTestRule/Fixture + tests/Rules/NoWithOnStubRule/Fixture diff --git a/src/Enum/RuleIdentifier.php b/src/Enum/RuleIdentifier.php index 2ac152a2..b9bf660d 100644 --- a/src/Enum/RuleIdentifier.php +++ b/src/Enum/RuleIdentifier.php @@ -25,5 +25,7 @@ final class RuleIdentifier public const string AVOID_ANY_EXPECTS = 'mockstan.avoidAnyExpects'; + public const string NO_WITH_ON_STUB = 'mockstan.noWithOnStub'; + public const string REQUIRE_AT_LEAST_ONE = 'mockstan.requireAtLeastOne'; } diff --git a/src/Rules/NoWithOnStubRule.php b/src/Rules/NoWithOnStubRule.php new file mode 100644 index 00000000..889fbfff --- /dev/null +++ b/src/Rules/NoWithOnStubRule.php @@ -0,0 +1,75 @@ + + * + * @see \Rector\Mockstan\Tests\Rules\NoWithOnStubRule\NoWithOnStubRuleTest + */ +final class NoWithOnStubRule implements Rule +{ + public const string ERROR_MESSAGE = 'Using with() on a stub is misleading and deprecated by PHPUnit. Use explicit expects() to turn it into a mock, or drop with()'; + + public function getNodeType(): string + { + return MethodCall::class; + } + + /** + * @param MethodCall $node + * @return IdentifierRuleError[] + */ + public function processNode(Node $node, Scope $scope): array + { + if (! NamingHelper::isName($node->name, 'with')) { + return []; + } + + if (! TestClassDetector::isTestClass($scope)) { + return []; + } + + if (! $node->var instanceof MethodCall) { + return []; + } + + $methodCall = $node->var; + if (! NamingHelper::isName($methodCall->name, 'method')) { + return []; + } + + if ($methodCall->var instanceof MethodCall && NamingHelper::isName($methodCall->var->name, 'expects')) { + return []; + } + + if (! $methodCall->var instanceof Variable && ! $methodCall->var instanceof PropertyFetch) { + return []; + } + + $callerType = $scope->getType($methodCall->var); + if (! $callerType->hasMethod('expects')->yes()) { + return []; + } + + $identifierRuleError = RuleErrorBuilder::message(self::ERROR_MESSAGE) + ->identifier(RuleIdentifier::NO_WITH_ON_STUB) + ->build(); + + return [$identifierRuleError]; + } +} diff --git a/tests/Rules/NoWithOnStubRule/Fixture/SkipMockWithExpectsAndWith.php b/tests/Rules/NoWithOnStubRule/Fixture/SkipMockWithExpectsAndWith.php new file mode 100644 index 00000000..bb349a9f --- /dev/null +++ b/tests/Rules/NoWithOnStubRule/Fixture/SkipMockWithExpectsAndWith.php @@ -0,0 +1,18 @@ +createMock(\stdClass::class); + + $mock->expects($this->once()) + ->method('someMethod') + ->with('arg') + ->willReturn('value'); + } +} diff --git a/tests/Rules/NoWithOnStubRule/Fixture/StubWithWith.php b/tests/Rules/NoWithOnStubRule/Fixture/StubWithWith.php new file mode 100644 index 00000000..83bf1ca4 --- /dev/null +++ b/tests/Rules/NoWithOnStubRule/Fixture/StubWithWith.php @@ -0,0 +1,17 @@ +createMock(\stdClass::class); + + $mock->method('someMethod') + ->with('arg') + ->willReturn('value'); + } +} diff --git a/tests/Rules/NoWithOnStubRule/NoWithOnStubRuleTest.php b/tests/Rules/NoWithOnStubRule/NoWithOnStubRuleTest.php new file mode 100644 index 00000000..d2dafb69 --- /dev/null +++ b/tests/Rules/NoWithOnStubRule/NoWithOnStubRuleTest.php @@ -0,0 +1,38 @@ +> $expectedErrorsWithLines + */ + #[DataProvider('provideData')] + public function testRule(string $filePath, array $expectedErrorsWithLines): void + { + $this->analyse([$filePath], $expectedErrorsWithLines); + } + + /** + * @return Iterator, mixed>> + */ + public static function provideData(): Iterator + { + yield [__DIR__ . '/Fixture/StubWithWith.php', [[NoWithOnStubRule::ERROR_MESSAGE, 13]]]; + + yield [__DIR__ . '/Fixture/SkipMockWithExpectsAndWith.php', []]; + } + + protected function getRule(): Rule + { + return new NoWithOnStubRule(); + } +}