From e322dc565615299b726e0090240dfa62833ef6a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9on=20Gersen?= Date: Wed, 16 Mar 2022 11:33:01 +0100 Subject: [PATCH 1/4] #6840 Report useless array filter --- conf/config.level5.neon | 1 + src/Rules/Functions/ArrayFilterEmptyRule.php | 87 +++++++++++++++++++ .../Command/data/file-without-errors.php | 2 +- .../Functions/ArrayFilterEmptyRuleTest.php | 71 +++++++++++++++ .../Functions/data/array_filter_empty.php | 28 ++++++ 5 files changed, 188 insertions(+), 1 deletion(-) create mode 100644 src/Rules/Functions/ArrayFilterEmptyRule.php create mode 100644 tests/PHPStan/Rules/Functions/ArrayFilterEmptyRuleTest.php create mode 100644 tests/PHPStan/Rules/Functions/data/array_filter_empty.php diff --git a/conf/config.level5.neon b/conf/config.level5.neon index c890be88ec..c6d7885f59 100644 --- a/conf/config.level5.neon +++ b/conf/config.level5.neon @@ -8,6 +8,7 @@ parameters: rules: - PHPStan\Rules\DateTimeInstantiationRule - PHPStan\Rules\Functions\ImplodeFunctionRule + - PHPStan\Rules\Functions\ArrayFilterEmptyRule services: - diff --git a/src/Rules/Functions/ArrayFilterEmptyRule.php b/src/Rules/Functions/ArrayFilterEmptyRule.php new file mode 100644 index 0000000000..c7460cd58c --- /dev/null +++ b/src/Rules/Functions/ArrayFilterEmptyRule.php @@ -0,0 +1,87 @@ + + */ +class ArrayFilterEmptyRule implements Rule +{ + + public function __construct(private ReflectionProvider $reflectionProvider) + { + } + + public function getNodeType(): string + { + return FuncCall::class; + } + + public function processNode(Node $node, Scope $scope): array + { + if (!($node->name instanceof Node\Name)) { + return []; + } + + $functionName = $this->reflectionProvider->resolveFunctionName($node->name, $scope); + + if ($functionName === null || strtolower($functionName) !== 'array_filter') { + return []; + } + + $args = $node->getArgs(); + if (count($args) !== 1) { + return []; + } + + $arrayType = $scope->getType($args[0]->value); + + if ($arrayType->isIterableAtLeastOnce()->no()) { + $message = 'Parameter #1 $array (%s) to function array_filter is empty, call has no effect.'; + return [ + RuleErrorBuilder::message(sprintf( + $message, + $arrayType->describe(VerbosityLevel::value()), + ))->build(), + ]; + } + + $falsyType = StaticTypeFactory::falsey(); + $isSuperType = $falsyType->isSuperTypeOf($arrayType->getIterableValueType()); + + if ($isSuperType->no()) { + $message = 'Parameter #1 $array (%s) to function array_filter cannot contain empty values, call has no effect.'; + return [ + RuleErrorBuilder::message(sprintf( + $message, + $arrayType->describe(VerbosityLevel::value()), + ))->build(), + ]; + } + + if ($isSuperType->yes()) { + $message = 'Parameter #1 $array (%s) to function array_filter can only contain empty values, call always results in [].'; + return [ + RuleErrorBuilder::message(sprintf( + $message, + $arrayType->describe(VerbosityLevel::value()), + ))->build(), + ]; + } + + return []; + } + +} diff --git a/tests/PHPStan/Command/data/file-without-errors.php b/tests/PHPStan/Command/data/file-without-errors.php index 4c4ad920ae..08929907d3 100644 --- a/tests/PHPStan/Command/data/file-without-errors.php +++ b/tests/PHPStan/Command/data/file-without-errors.php @@ -1,3 +1,3 @@ + */ +class ArrayFilterEmptyRuleTest extends RuleTestCase +{ + + protected function getRule(): Rule + { + return new ArrayFilterEmptyRule($this->createReflectionProvider()); + } + + public function testFile(): void + { + $expectedErrors = [ + [ + 'Parameter #1 $array (array{1, 3}) to function array_filter cannot contain empty values, call has no effect.', + 11, + ], + [ + 'Parameter #1 $array (array{\'test\'}) to function array_filter cannot contain empty values, call has no effect.', + 12, + ], + [ + 'Parameter #1 $array (array{true, true}) to function array_filter cannot contain empty values, call has no effect.', + 17, + ], + [ + 'Parameter #1 $array (array{stdClass}) to function array_filter cannot contain empty values, call has no effect.', + 18, + ], + [ + 'Parameter #1 $array (array) to function array_filter cannot contain empty values, call has no effect.', + 20, + ], + [ + 'Parameter #1 $array (array{0}) to function array_filter can only contain empty values, call always results in [].', + 23, + ], + [ + 'Parameter #1 $array (array{null}) to function array_filter can only contain empty values, call always results in [].', + 24, + ], + [ + 'Parameter #1 $array (array{null, null}) to function array_filter can only contain empty values, call always results in [].', + 25, + ], + [ + 'Parameter #1 $array (array{null, 0}) to function array_filter can only contain empty values, call always results in [].', + 26, + ], + [ + 'Parameter #1 $array (array) to function array_filter can only contain empty values, call always results in [].', + 27, + ], + [ + 'Parameter #1 $array (array{}) to function array_filter is empty, call has no effect.', + 28, + ], + ]; + + $this->analyse([__DIR__ . '/data/array_filter_empty.php'], $expectedErrors); + } + +} diff --git a/tests/PHPStan/Rules/Functions/data/array_filter_empty.php b/tests/PHPStan/Rules/Functions/data/array_filter_empty.php new file mode 100644 index 0000000000..3bcb4dd6ea --- /dev/null +++ b/tests/PHPStan/Rules/Functions/data/array_filter_empty.php @@ -0,0 +1,28 @@ + $objectsOrNull */ +$objectsOrNull = []; +/** @var array $falsey */ +$falsey = []; + +array_filter([0,1,3]); +array_filter([1,3]); +array_filter(['test']); +array_filter(['', 'test']); +array_filter([null, 'test']); +array_filter([false, 'test']); +array_filter([true, false]); +array_filter([true, true]); +array_filter([new \stdClass()]); +array_filter([new \stdClass(), null]); +array_filter($objects); +array_filter($objectsOrNull); + +array_filter([0]); +array_filter([null]); +array_filter([null, null]); +array_filter([null, 0]); +array_filter($falsey); +array_filter([]); From 204cc2b978cf1d59a0de00382ce453953fd36f15 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Fri, 18 Mar 2022 13:33:16 +0100 Subject: [PATCH 2/4] Rename rule --- conf/config.level5.neon | 2 +- .../{ArrayFilterEmptyRule.php => ArrayFilterRule.php} | 2 +- ...ArrayFilterEmptyRuleTest.php => ArrayFilterRuleTest.php} | 6 +++--- 3 files changed, 5 insertions(+), 5 deletions(-) rename src/Rules/Functions/{ArrayFilterEmptyRule.php => ArrayFilterRule.php} (97%) rename tests/PHPStan/Rules/Functions/{ArrayFilterEmptyRuleTest.php => ArrayFilterRuleTest.php} (91%) diff --git a/conf/config.level5.neon b/conf/config.level5.neon index c6d7885f59..a17616c915 100644 --- a/conf/config.level5.neon +++ b/conf/config.level5.neon @@ -8,7 +8,7 @@ parameters: rules: - PHPStan\Rules\DateTimeInstantiationRule - PHPStan\Rules\Functions\ImplodeFunctionRule - - PHPStan\Rules\Functions\ArrayFilterEmptyRule + - PHPStan\Rules\Functions\ArrayFilterRule services: - diff --git a/src/Rules/Functions/ArrayFilterEmptyRule.php b/src/Rules/Functions/ArrayFilterRule.php similarity index 97% rename from src/Rules/Functions/ArrayFilterEmptyRule.php rename to src/Rules/Functions/ArrayFilterRule.php index c7460cd58c..5e5acadafd 100644 --- a/src/Rules/Functions/ArrayFilterEmptyRule.php +++ b/src/Rules/Functions/ArrayFilterRule.php @@ -17,7 +17,7 @@ /** * @implements Rule */ -class ArrayFilterEmptyRule implements Rule +class ArrayFilterRule implements Rule { public function __construct(private ReflectionProvider $reflectionProvider) diff --git a/tests/PHPStan/Rules/Functions/ArrayFilterEmptyRuleTest.php b/tests/PHPStan/Rules/Functions/ArrayFilterRuleTest.php similarity index 91% rename from tests/PHPStan/Rules/Functions/ArrayFilterEmptyRuleTest.php rename to tests/PHPStan/Rules/Functions/ArrayFilterRuleTest.php index 890bcf2883..256f1e75cc 100644 --- a/tests/PHPStan/Rules/Functions/ArrayFilterEmptyRuleTest.php +++ b/tests/PHPStan/Rules/Functions/ArrayFilterRuleTest.php @@ -6,14 +6,14 @@ use PHPStan\Testing\RuleTestCase; /** - * @extends RuleTestCase + * @extends RuleTestCase */ -class ArrayFilterEmptyRuleTest extends RuleTestCase +class ArrayFilterRuleTest extends RuleTestCase { protected function getRule(): Rule { - return new ArrayFilterEmptyRule($this->createReflectionProvider()); + return new ArrayFilterRule($this->createReflectionProvider()); } public function testFile(): void From 7627b52b977480e71254a84f08b01e6251076619 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Fri, 18 Mar 2022 13:36:08 +0100 Subject: [PATCH 3/4] Enable ArrayFilterRule only with bleedingEdge --- conf/bleedingEdge.neon | 1 + conf/config.level5.neon | 8 +++++++- conf/config.neon | 2 ++ 3 files changed, 10 insertions(+), 1 deletion(-) diff --git a/conf/bleedingEdge.neon b/conf/bleedingEdge.neon index 5af3a763db..cde18d2a32 100644 --- a/conf/bleedingEdge.neon +++ b/conf/bleedingEdge.neon @@ -3,5 +3,6 @@ parameters: bleedingEdge: true skipCheckGenericClasses: [] explicitMixedInUnknownGenericNew: true + arrayFilter: true stubFiles: - ../stubs/bleedingEdge/Countable.stub diff --git a/conf/config.level5.neon b/conf/config.level5.neon index a17616c915..3339b0a836 100644 --- a/conf/config.level5.neon +++ b/conf/config.level5.neon @@ -5,10 +5,13 @@ parameters: checkFunctionArgumentTypes: true checkArgumentsPassedByReference: true +conditionalTags: + PHPStan\Rules\Functions\ArrayFilterRule: + phpstan.rules.rule: %featureToggles.arrayFilter% + rules: - PHPStan\Rules\DateTimeInstantiationRule - PHPStan\Rules\Functions\ImplodeFunctionRule - - PHPStan\Rules\Functions\ArrayFilterRule services: - @@ -17,3 +20,6 @@ services: reportMaybes: %reportMaybes% tags: - phpstan.rules.rule + + - + class: PHPStan\Rules\Functions\ArrayFilterRule diff --git a/conf/config.neon b/conf/config.neon index 5f3f094aef..6a0898629d 100644 --- a/conf/config.neon +++ b/conf/config.neon @@ -28,6 +28,7 @@ parameters: - FilterIterator - RecursiveCallbackFilterIterator explicitMixedInUnknownGenericNew: false + arrayFilter: false fileExtensions: - php checkAdvancedIsset: false @@ -205,6 +206,7 @@ parametersSchema: disableRuntimeReflectionProvider: bool(), skipCheckGenericClasses: listOf(string()), explicitMixedInUnknownGenericNew: bool(), + arrayFilter: bool(), ]) fileExtensions: listOf(string()) checkAdvancedIsset: bool() From b29a2673e9e45f56e5e460d67319723ae5a8b76a Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Fri, 18 Mar 2022 13:39:02 +0100 Subject: [PATCH 4/4] Change error messages to my taste --- src/Rules/Functions/ArrayFilterRule.php | 4 ++-- .../Rules/Functions/ArrayFilterRuleTest.php | 20 +++++++++---------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/Rules/Functions/ArrayFilterRule.php b/src/Rules/Functions/ArrayFilterRule.php index 5e5acadafd..2ff8dd0e90 100644 --- a/src/Rules/Functions/ArrayFilterRule.php +++ b/src/Rules/Functions/ArrayFilterRule.php @@ -62,7 +62,7 @@ public function processNode(Node $node, Scope $scope): array $isSuperType = $falsyType->isSuperTypeOf($arrayType->getIterableValueType()); if ($isSuperType->no()) { - $message = 'Parameter #1 $array (%s) to function array_filter cannot contain empty values, call has no effect.'; + $message = 'Parameter #1 $array (%s) to function array_filter does not contain falsy values, the array will always stay the same.'; return [ RuleErrorBuilder::message(sprintf( $message, @@ -72,7 +72,7 @@ public function processNode(Node $node, Scope $scope): array } if ($isSuperType->yes()) { - $message = 'Parameter #1 $array (%s) to function array_filter can only contain empty values, call always results in [].'; + $message = 'Parameter #1 $array (%s) to function array_filter contains falsy values only, the result will always be an empty array.'; return [ RuleErrorBuilder::message(sprintf( $message, diff --git a/tests/PHPStan/Rules/Functions/ArrayFilterRuleTest.php b/tests/PHPStan/Rules/Functions/ArrayFilterRuleTest.php index 256f1e75cc..bb60484ed6 100644 --- a/tests/PHPStan/Rules/Functions/ArrayFilterRuleTest.php +++ b/tests/PHPStan/Rules/Functions/ArrayFilterRuleTest.php @@ -20,43 +20,43 @@ public function testFile(): void { $expectedErrors = [ [ - 'Parameter #1 $array (array{1, 3}) to function array_filter cannot contain empty values, call has no effect.', + 'Parameter #1 $array (array{1, 3}) to function array_filter does not contain falsy values, the array will always stay the same.', 11, ], [ - 'Parameter #1 $array (array{\'test\'}) to function array_filter cannot contain empty values, call has no effect.', + 'Parameter #1 $array (array{\'test\'}) to function array_filter does not contain falsy values, the array will always stay the same.', 12, ], [ - 'Parameter #1 $array (array{true, true}) to function array_filter cannot contain empty values, call has no effect.', + 'Parameter #1 $array (array{true, true}) to function array_filter does not contain falsy values, the array will always stay the same.', 17, ], [ - 'Parameter #1 $array (array{stdClass}) to function array_filter cannot contain empty values, call has no effect.', + 'Parameter #1 $array (array{stdClass}) to function array_filter does not contain falsy values, the array will always stay the same.', 18, ], [ - 'Parameter #1 $array (array) to function array_filter cannot contain empty values, call has no effect.', + 'Parameter #1 $array (array) to function array_filter does not contain falsy values, the array will always stay the same.', 20, ], [ - 'Parameter #1 $array (array{0}) to function array_filter can only contain empty values, call always results in [].', + 'Parameter #1 $array (array{0}) to function array_filter contains falsy values only, the result will always be an empty array.', 23, ], [ - 'Parameter #1 $array (array{null}) to function array_filter can only contain empty values, call always results in [].', + 'Parameter #1 $array (array{null}) to function array_filter contains falsy values only, the result will always be an empty array.', 24, ], [ - 'Parameter #1 $array (array{null, null}) to function array_filter can only contain empty values, call always results in [].', + 'Parameter #1 $array (array{null, null}) to function array_filter contains falsy values only, the result will always be an empty array.', 25, ], [ - 'Parameter #1 $array (array{null, 0}) to function array_filter can only contain empty values, call always results in [].', + 'Parameter #1 $array (array{null, 0}) to function array_filter contains falsy values only, the result will always be an empty array.', 26, ], [ - 'Parameter #1 $array (array) to function array_filter can only contain empty values, call always results in [].', + 'Parameter #1 $array (array) to function array_filter contains falsy values only, the result will always be an empty array.', 27, ], [