From 1d553a84c2b2b673688a0fd3fc74236fdd9da9cd Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Fri, 22 Nov 2024 17:04:29 +0100 Subject: [PATCH 1/6] Fix broken test --- tests/Rules/CallMethodRuleTest.php | 27 ++++++++++++++++++++++----- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/tests/Rules/CallMethodRuleTest.php b/tests/Rules/CallMethodRuleTest.php index 4b8e362..1409c07 100644 --- a/tests/Rules/CallMethodRuleTest.php +++ b/tests/Rules/CallMethodRuleTest.php @@ -8,6 +8,7 @@ use PHPStan\Rules\Methods\MethodCallCheck; use PHPStan\Rules\NullsafeCheck; use PHPStan\Rules\PhpDoc\UnresolvableTypeHelper; +use PHPStan\Rules\Properties\PropertyReflectionFinder; use PHPStan\Rules\Rule; use PHPStan\Rules\RuleLevelHelper; use PHPStan\Testing\RuleTestCase; @@ -17,15 +18,31 @@ class CallMethodRuleTest extends RuleTestCase { protected function getRule(): Rule { - $reflectionProvider = $this->createReflectionProvider(); - $ruleLevelHelper = new RuleLevelHelper($reflectionProvider, true, true, true, false); + $reflectionProvider = self::createReflectionProvider(); + $ruleLevelHelper = new RuleLevelHelper($reflectionProvider, true, true, true, true, false, true); + return new CallMethodsRule( - new MethodCallCheck($reflectionProvider, $ruleLevelHelper, true, true), - new FunctionCallParametersCheck($ruleLevelHelper, new NullsafeCheck(), new PhpVersion(PHP_VERSION_ID), new UnresolvableTypeHelper(), true, false, false, false) + new MethodCallCheck( + $reflectionProvider, + $ruleLevelHelper, + true, + true + ), + new FunctionCallParametersCheck( + $ruleLevelHelper, + new NullsafeCheck(), + new PhpVersion(PHP_VERSION_ID), + new UnresolvableTypeHelper(), + new PropertyReflectionFinder(), + true, + true, + true, + true + ) ); } - public function testSafePregReplace() + public function testSafePregReplace(): void { // FIXME: this rule actually runs code but will always return no error because the rule executed is not the correct one. // This provides code coverage but assert is not ok. From 593271e0dc943615153347cfef95d4ce73d0333d Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Sun, 24 Nov 2024 22:40:44 +0100 Subject: [PATCH 2/6] Make requirement of PhpParser 5 explicit --- composer.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/composer.json b/composer.json index 14771e8..4b4513b 100644 --- a/composer.json +++ b/composer.json @@ -12,7 +12,8 @@ "require": { "php": "^7.4 || ^8.0", "phpstan/phpstan": "^2.0", - "thecodingmachine/safe": "^1.0 || ^2.0" + "thecodingmachine/safe": "^1.0 || ^2.0", + "nikic/php-parser": "^5" }, "require-dev": { "phpunit/phpunit": "^9.6", From 1e68143b8018eab407331eac0d44645223a1ae21 Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Mon, 25 Nov 2024 09:02:55 +0100 Subject: [PATCH 3/6] Drop commented out test bootstrap --- phpunit.xml.dist | 2 +- tests/bootstrap.php | 33 --------------------------------- tests/phpstan-bootstrap.php | 4 ---- 3 files changed, 1 insertion(+), 38 deletions(-) delete mode 100644 tests/bootstrap.php delete mode 100644 tests/phpstan-bootstrap.php diff --git a/phpunit.xml.dist b/phpunit.xml.dist index ac73057..cea1406 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -1,7 +1,7 @@ defaultExtensions = []; -$configurator->setDebugMode(true); -$configurator->setTempDirectory($tmpDir); -$configurator->addConfig($confDir . '/config.neon'); -$configurator->addConfig($confDir . '/config.level5.neon'); -$configurator->addParameters([ - 'rootDir' => $rootDir, - 'tmpDir' => $tmpDir, - 'currentWorkingDirectory' => $rootDir, - 'cliArgumentsVariablesRegistered' => false, -]); -$container = $configurator->createContainer(); - -PHPStan\Testing\TestCase::setContainer($container); -PHPStan\Type\TypeCombinator::setUnionTypesEnabled(true); -require_once __DIR__ . '/phpstan-bootstrap.php'; -*/ diff --git a/tests/phpstan-bootstrap.php b/tests/phpstan-bootstrap.php deleted file mode 100644 index c39b13a..0000000 --- a/tests/phpstan-bootstrap.php +++ /dev/null @@ -1,4 +0,0 @@ - Date: Mon, 25 Nov 2024 09:03:09 +0100 Subject: [PATCH 4/6] Cover tests with static analysis --- phpstan.neon | 3 +++ 1 file changed, 3 insertions(+) diff --git a/phpstan.neon b/phpstan.neon index 46b5aa0..f4ecada 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -2,6 +2,9 @@ parameters: level: max paths: - src + - tests + excludePaths: + - tests/Rules/data ignoreErrors: - message: '#^Implementing PHPStan\\Rules\\IdentifierRuleError is not covered by backward compatibility promise\. The interface might change in a minor PHPStan version\.$#' From e8a661124a33950682c4d870d1d3c43dac774986 Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Mon, 25 Nov 2024 09:13:49 +0100 Subject: [PATCH 5/6] Fix tests static analysis --- tests/Rules/CallMethodRuleTest.php | 3 +++ tests/Rules/UseSafeClassesRuleTest.php | 9 +++++--- tests/Rules/UseSafeFunctionsRuleTest.php | 29 ++++++++++-------------- tests/Utils/FunctionListLoaderTest.php | 4 ++-- 4 files changed, 23 insertions(+), 22 deletions(-) diff --git a/tests/Rules/CallMethodRuleTest.php b/tests/Rules/CallMethodRuleTest.php index 1409c07..f12bd45 100644 --- a/tests/Rules/CallMethodRuleTest.php +++ b/tests/Rules/CallMethodRuleTest.php @@ -14,6 +14,9 @@ use PHPStan\Testing\RuleTestCase; use TheCodingMachine\Safe\PHPStan\Type\Php\ReplaceSafeFunctionsDynamicReturnTypeExtension; +/** + * @template-extends RuleTestCase + */ class CallMethodRuleTest extends RuleTestCase { protected function getRule(): Rule diff --git a/tests/Rules/UseSafeClassesRuleTest.php b/tests/Rules/UseSafeClassesRuleTest.php index b3258be..bfa8aaf 100644 --- a/tests/Rules/UseSafeClassesRuleTest.php +++ b/tests/Rules/UseSafeClassesRuleTest.php @@ -2,17 +2,20 @@ namespace TheCodingMachine\Safe\PHPStan\Rules; +use PHPStan\Rules\Rule; use PHPStan\Testing\RuleTestCase; -use TheCodingMachine\Safe\PHPStan\Type\Php\ReplaceSafeFunctionsDynamicReturnTypeExtension; +/** + * @template-extends RuleTestCase + */ class UseSafeClassesRuleTest extends RuleTestCase { - protected function getRule(): \PHPStan\Rules\Rule + protected function getRule(): Rule { return new UseSafeClassesRule(); } - public function testDateTime() + public function testDateTime(): void { $this->analyse([__DIR__ . '/data/datetime.php'], [ [ diff --git a/tests/Rules/UseSafeFunctionsRuleTest.php b/tests/Rules/UseSafeFunctionsRuleTest.php index 4014828..55ba182 100644 --- a/tests/Rules/UseSafeFunctionsRuleTest.php +++ b/tests/Rules/UseSafeFunctionsRuleTest.php @@ -2,17 +2,20 @@ namespace TheCodingMachine\Safe\PHPStan\Rules; +use PHPStan\Rules\Rule; use PHPStan\Testing\RuleTestCase; -use TheCodingMachine\Safe\PHPStan\Type\Php\ReplaceSafeFunctionsDynamicReturnTypeExtension; +/** + * @template-extends RuleTestCase + */ class UseSafeFunctionsRuleTest extends RuleTestCase { - protected function getRule(): \PHPStan\Rules\Rule + protected function getRule(): Rule { return new UseSafeFunctionsRule(); } - public function testCatch() + public function testCatch(): void { $this->analyse([__DIR__ . '/data/fopen.php'], [ [ @@ -22,31 +25,23 @@ public function testCatch() ]); } - public function testNoCatchSafe() + public function testNoCatchSafe(): void { $this->analyse([__DIR__ . '/data/safe_fopen.php'], []); } - public function testExprCall() + public function testExprCall(): void { $this->analyse([__DIR__ . '/data/undirect_call.php'], []); } - public function testJSONDecodeNoCatchSafe() + public function testJSONDecodeNoCatchSafe(): void { - if (version_compare(PHP_VERSION, '7.3.0', '>=')) { - $this->analyse([__DIR__ . '/data/safe_json_decode_for_7.3.0.php'], []); - } else { - $this->assertTrue(true); - } + $this->analyse([__DIR__ . '/data/safe_json_decode_for_7.3.0.php'], []); } - public function testJSONEncodeNoCatchSafe() + public function testJSONEncodeNoCatchSafe(): void { - if (version_compare(PHP_VERSION, '7.3.0', '>=')) { - $this->analyse([__DIR__ . '/data/safe_json_encode_for_7.3.0.php'], []); - } else { - $this->assertTrue(true); - } + $this->analyse([__DIR__ . '/data/safe_json_encode_for_7.3.0.php'], []); } } diff --git a/tests/Utils/FunctionListLoaderTest.php b/tests/Utils/FunctionListLoaderTest.php index 0234608..c5c7c50 100644 --- a/tests/Utils/FunctionListLoaderTest.php +++ b/tests/Utils/FunctionListLoaderTest.php @@ -6,10 +6,10 @@ class FunctionListLoaderTest extends TestCase { - - public function testGetFunctionList() + public function testGetFunctionList(): void { $functions = FunctionListLoader::getFunctionList(); $this->assertArrayHasKey('fopen', $functions); + $this->assertEquals('fopen', $functions['fopen']); } } From ca626ccfb5333087455e42be588b03b82bf48fb7 Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Mon, 25 Nov 2024 11:05:13 +0100 Subject: [PATCH 6/6] Change test for ReplaceSafeFunctionsDynamicReturnTypeExtension Coverage is ignored because it's tested at a static analysis level --- phpunit.xml.dist | 3 + ...afeFunctionsDynamicReturnTypeExtension.php | 2 +- tests/Rules/CallMethodRuleTest.php | 63 ------------------- ...unctionsDynamicReturnTypeExtensionTest.php | 22 +++++++ 4 files changed, 26 insertions(+), 64 deletions(-) delete mode 100644 tests/Rules/CallMethodRuleTest.php create mode 100644 tests/Type/Php/ReplaceSafeFunctionsDynamicReturnTypeExtensionTest.php diff --git a/phpunit.xml.dist b/phpunit.xml.dist index cea1406..7b1dd06 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -24,6 +24,9 @@ ./src + + src/Type/Php/ReplaceSafeFunctionsDynamicReturnTypeExtension.php + diff --git a/src/Type/Php/ReplaceSafeFunctionsDynamicReturnTypeExtension.php b/src/Type/Php/ReplaceSafeFunctionsDynamicReturnTypeExtension.php index 02711e1..ca7ba4f 100644 --- a/src/Type/Php/ReplaceSafeFunctionsDynamicReturnTypeExtension.php +++ b/src/Type/Php/ReplaceSafeFunctionsDynamicReturnTypeExtension.php @@ -20,7 +20,7 @@ class ReplaceSafeFunctionsDynamicReturnTypeExtension implements DynamicFunctionR { /** @var array */ - private $functions = [ + private array $functions = [ 'Safe\preg_replace' => 2, ]; diff --git a/tests/Rules/CallMethodRuleTest.php b/tests/Rules/CallMethodRuleTest.php deleted file mode 100644 index f12bd45..0000000 --- a/tests/Rules/CallMethodRuleTest.php +++ /dev/null @@ -1,63 +0,0 @@ - - */ -class CallMethodRuleTest extends RuleTestCase -{ - protected function getRule(): Rule - { - $reflectionProvider = self::createReflectionProvider(); - $ruleLevelHelper = new RuleLevelHelper($reflectionProvider, true, true, true, true, false, true); - - return new CallMethodsRule( - new MethodCallCheck( - $reflectionProvider, - $ruleLevelHelper, - true, - true - ), - new FunctionCallParametersCheck( - $ruleLevelHelper, - new NullsafeCheck(), - new PhpVersion(PHP_VERSION_ID), - new UnresolvableTypeHelper(), - new PropertyReflectionFinder(), - true, - true, - true, - true - ) - ); - } - - public function testSafePregReplace(): void - { - // FIXME: this rule actually runs code but will always return no error because the rule executed is not the correct one. - // This provides code coverage but assert is not ok. - $this->analyse([__DIR__ . '/data/safe_pregreplace.php'], []); - } - - - /** - * @return \PHPStan\Type\DynamicFunctionReturnTypeExtension[] - */ - public function getDynamicFunctionReturnTypeExtensions(): array - { - return [new ReplaceSafeFunctionsDynamicReturnTypeExtension()]; - } -} diff --git a/tests/Type/Php/ReplaceSafeFunctionsDynamicReturnTypeExtensionTest.php b/tests/Type/Php/ReplaceSafeFunctionsDynamicReturnTypeExtensionTest.php new file mode 100644 index 0000000..3ebce33 --- /dev/null +++ b/tests/Type/Php/ReplaceSafeFunctionsDynamicReturnTypeExtensionTest.php @@ -0,0 +1,22 @@ +assertStringNotContainsString('foo', $x); + } + + public function testWithArrays(): void + { + $x = \Safe\preg_replace(['/foo/'], ['bar'], ['baz']); + + $this->assertNotContains('foo', $x); + } +}