From ff52e4674bc51e79677406af6f6c848270c39786 Mon Sep 17 00:00:00 2001 From: Marcelo Rocha Date: Sat, 24 Feb 2024 10:40:53 -0300 Subject: [PATCH 1/5] Fix phpstan check of hasMethod, use the yes method instead of no --- src/Rule/Model/OrmSelectQueryFindMatchOptionsTypesRule.php | 2 +- .../TestCase/Rule/Model/Fake/FailingOrmFindRuleItemsLogic.php | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/Rule/Model/OrmSelectQueryFindMatchOptionsTypesRule.php b/src/Rule/Model/OrmSelectQueryFindMatchOptionsTypesRule.php index d95e0cb..259b84a 100644 --- a/src/Rule/Model/OrmSelectQueryFindMatchOptionsTypesRule.php +++ b/src/Rule/Model/OrmSelectQueryFindMatchOptionsTypesRule.php @@ -337,7 +337,7 @@ protected function getSpecificFinderOptions(array $details, Scope $scope): array } $object = new ObjectType($tableClass); $finderMethod = 'find' . $finder; - if ($object->hasMethod($finderMethod)->no()) { + if (!$object->hasMethod($finderMethod)->yes()) { return []; } $method = $object->getMethod($finderMethod, $scope); diff --git a/tests/TestCase/Rule/Model/Fake/FailingOrmFindRuleItemsLogic.php b/tests/TestCase/Rule/Model/Fake/FailingOrmFindRuleItemsLogic.php index cdb1d9c..fa9e252 100644 --- a/tests/TestCase/Rule/Model/Fake/FailingOrmFindRuleItemsLogic.php +++ b/tests/TestCase/Rule/Model/Fake/FailingOrmFindRuleItemsLogic.php @@ -138,5 +138,8 @@ public function process() $Table->find( 'featured', //custom finder is known but required options are missing ); + $Table->find( + 'unkonwn', + ); } } From e0843a2418f66a81848ef4981798d9deeb2d40fd Mon Sep 17 00:00:00 2001 From: Marcelo Rocha Date: Mon, 26 Feb 2024 15:12:13 -0300 Subject: [PATCH 2/5] OrmSelectQueryFindMatchOptionsTypesRule added code for backward compatibility with CakePHP 4 finder structure, findSomething($query, array $options) --- ...rmSelectQueryFindMatchOptionsTypesRule.php | 18 +++++- .../Fake/FailingOrmFindRuleItemsLogic.php | 59 ++++++++++++++++++ ...lectQueryFindMatchOptionsTypesRuleTest.php | 3 + tests/test_app/Model/Table/NotesTable.php | 61 +++++++++++++++++++ 4 files changed, 140 insertions(+), 1 deletion(-) diff --git a/src/Rule/Model/OrmSelectQueryFindMatchOptionsTypesRule.php b/src/Rule/Model/OrmSelectQueryFindMatchOptionsTypesRule.php index 259b84a..ebe18b3 100644 --- a/src/Rule/Model/OrmSelectQueryFindMatchOptionsTypesRule.php +++ b/src/Rule/Model/OrmSelectQueryFindMatchOptionsTypesRule.php @@ -22,6 +22,8 @@ use PHPStan\Rules\RuleError; use PHPStan\Rules\RuleErrorBuilder; use PHPStan\Rules\RuleLevelHelper; +use PHPStan\Type\ArrayType; +use PHPStan\Type\MixedType; use PHPStan\Type\ObjectType; use PHPStan\Type\Type; use PHPStan\Type\VerbosityLevel; @@ -342,13 +344,27 @@ protected function getSpecificFinderOptions(array $details, Scope $scope): array } $method = $object->getMethod($finderMethod, $scope); $parameters = $method->getVariants()[0]->getParameters(); + if (!isset($parameters[1])) { return []; } + if (count($parameters) === 2) { + //Backward compatibility with CakePHP 4 finder structure, findSomething($query, array $options) + $secondParam = $parameters[1]; + $paramType = $secondParam->getType(); + if ( + $parameters[1]->getName() === 'options' + && !$secondParam->isVariadic() + && ($paramType instanceof MixedType || $paramType instanceof ArrayType) + ) { + return []; + } + } foreach ($parameters as $key => $param) { - if ($key === 0) { + if ($key === 0 || $param->isVariadic()) { continue; } + $specificFinderOptions[$param->getName()] = $param; } diff --git a/tests/TestCase/Rule/Model/Fake/FailingOrmFindRuleItemsLogic.php b/tests/TestCase/Rule/Model/Fake/FailingOrmFindRuleItemsLogic.php index fa9e252..5c19933 100644 --- a/tests/TestCase/Rule/Model/Fake/FailingOrmFindRuleItemsLogic.php +++ b/tests/TestCase/Rule/Model/Fake/FailingOrmFindRuleItemsLogic.php @@ -141,5 +141,64 @@ public function process() $Table->find( 'unkonwn', ); + $Table->find('legacy');//Legacy should ignore params check + $Table->find('legacy', [//Legacy should ignore params check + 'sort' => ['Notes.note' => 'ASC'], + ]); + $Table->find('legacy', [//Legacy should ignore params check + 'sort' => ['Notes.note' => 'ASC'], + 'type' => 'featured', + 'active' => false, + ]); + $Table->find('optionsPacked'); + $Table->find('optionsPacked', [//Legacy should ignore params check + 'sort' => ['Notes.note' => 'ASC'], + ]); + $Table->find('optionsPacked', [//Legacy should ignore params check + 'sort' => ['Notes.note' => 'ASC'], + 'labelField' => 'id', + ]); + $Table->find('optionsPacked', [//Legacy should ignore params check + 'sort' => ['Notes.note' => 'ASC'], + 'labelField' => 'id', + ]); + $Table->find( + 'optionsPacked', + sort: ['Notes.note' => 'ASC'], + labelField: 'id' + ); + $Table->find( + 'optionsPacked', + sort: ['Notes.note' => 'ASC'], + labelField: 'id' + ); + $Table->find('argsPacked'); + $Table->find( + 'argsPacked', + sort: ['Notes.note' => 'ASC'], + groupLabel: 'type' + ); + $Table->find('argsPacked', [ + 'sort' => ['Notes.note' => 'ASC'], + 'groupLabel' => 'id', + ]); + $Table->find('twoArgsButNotLegacy', [ + 'sort' => ['Notes.note' => 'ASC'], + 'myType' => 'featured', + ]); + $Table->find('twoArgsButNotLegacy', [ + 'sort' => ['Notes.note' => 'ASC'], + ]); + $Table->find( + 'twoArgsButNotLegacy', + sort: ['Notes.note' => 'ASC'], + myType: 'featured' + ); + $Table->find('twoArgsButNotLegacy'); + $Table->find( + 'twoArgsButNotLegacy', + sort: ['Notes.note' => 'ASC'], + myType: 19 + ); } } diff --git a/tests/TestCase/Rule/Model/OrmSelectQueryFindMatchOptionsTypesRuleTest.php b/tests/TestCase/Rule/Model/OrmSelectQueryFindMatchOptionsTypesRuleTest.php index 623f685..0ca6622 100644 --- a/tests/TestCase/Rule/Model/OrmSelectQueryFindMatchOptionsTypesRuleTest.php +++ b/tests/TestCase/Rule/Model/OrmSelectQueryFindMatchOptionsTypesRuleTest.php @@ -78,6 +78,9 @@ public function testRule(): void ['Call to App\Model\Table\NotesTable::find is missing required finder option "fun".', 134], ['Call to App\Model\Table\NotesTable::find is missing required finder option "year".', 138], ['Call to App\Model\Table\NotesTable::find is missing required finder option "fun".', 138], + ['Call to App\Model\Table\NotesTable::find is missing required finder option "myType".', 189], + ['Call to App\Model\Table\NotesTable::find is missing required finder option "myType".', 197], + ['Call to App\Model\Table\NotesTable::find with option "myType" (string) does not accept int.', 198], ]); } diff --git a/tests/test_app/Model/Table/NotesTable.php b/tests/test_app/Model/Table/NotesTable.php index 0709c74..872094f 100644 --- a/tests/test_app/Model/Table/NotesTable.php +++ b/tests/test_app/Model/Table/NotesTable.php @@ -60,6 +60,13 @@ public function warning(): array $this->Users->find('all', order: ['Users.id' => 'DESC'], limit: 12); $entity = $this->saveOrFail($entity); } + $this->find('optionsPacked'); + $this->find( + 'twoArgsButNotLegacy', + sort: ['Notes.note' => 'ASC'], + myType: 'featured' + ); + $this->find('argsPacked'); return [ 'type' => 'warning', @@ -88,4 +95,58 @@ public function findFeatured(SelectQuery $query, string|int $year, bool $fun): S return $query->where($where) ->orderBy(['Notes.created' => 'DESC']); } + + /** + * @param \Cake\ORM\Query\SelectQuery $query + * @param string $myType + * @return \Cake\ORM\Query\SelectQuery + */ + public function findTwoArgsButNotLegacy(SelectQuery $query, string $myType): SelectQuery + { + $where = [ + 'year <=' => 2010, + 'type' => $myType, + ]; + + return $query->where($where) + ->orderBy(['Notes.created' => 'DESC']); + } + + /** + * @param \Cake\ORM\Query\SelectQuery $query + * @param array $options + * @return \Cake\ORM\Query\SelectQuery + */ + public function findLegacy(SelectQuery $query, array $options): SelectQuery + { + return $query + ->where([ + 'type' => $options['type'] ?? 'normal', + 'active' => $options['active'] ?? false, + ]); + } + + /** + * @param \Cake\ORM\Query\SelectQuery $query + * @param mixed ...$options + * @return \Cake\ORM\Query\SelectQuery + */ + public function findOptionsPacked(SelectQuery $query, mixed ...$options): SelectQuery + { + return $query->select([ + $options['labelField'] ?? 'note', + ]); + } + + /** + * @param \Cake\ORM\Query\SelectQuery $query + * @param mixed ...$args + * @return \Cake\ORM\Query\SelectQuery + */ + public function findArgsPacked(SelectQuery $query, mixed ...$args): SelectQuery + { + return $query->select([ + $args['groupLabel'] ?? 'note', + ]); + } } From 99e3a4503153a5a87bfa44426056d15381ecb7f1 Mon Sep 17 00:00:00 2001 From: Marcelo Rocha Date: Mon, 26 Feb 2024 15:40:22 -0300 Subject: [PATCH 3/5] OrmSelectQueryFindMatchOptionsTypesRule should be ignored when not used string args --- .../Model/OrmSelectQueryFindMatchOptionsTypesRule.php | 9 ++++++++- .../Rule/Model/Fake/FailingOrmFindRuleItemsLogic.php | 10 ++++++++++ tests/test_app/Model/Table/NotesTable.php | 8 ++++++++ 3 files changed, 26 insertions(+), 1 deletion(-) diff --git a/src/Rule/Model/OrmSelectQueryFindMatchOptionsTypesRule.php b/src/Rule/Model/OrmSelectQueryFindMatchOptionsTypesRule.php index ebe18b3..c70f007 100644 --- a/src/Rule/Model/OrmSelectQueryFindMatchOptionsTypesRule.php +++ b/src/Rule/Model/OrmSelectQueryFindMatchOptionsTypesRule.php @@ -11,6 +11,7 @@ use Cake\ORM\Query\SelectQuery; use Cake\ORM\Table; use CakeDC\PHPStan\Rule\Traits\ParseClassNameFromArgTrait; +use InvalidArgumentException; use PhpParser\Node; use PhpParser\Node\Expr\Array_; use PhpParser\Node\Expr\MethodCall; @@ -104,7 +105,11 @@ public function processNode(Node $node, Scope $scope): array if (empty($referenceClasses)) { return []; } - $details = $this->getDetails($referenceClasses, $node->name->name, $args); + try { + $details = $this->getDetails($referenceClasses, $node->name->name, $args); + } catch (InvalidArgumentException) { + return []; + } if ($details === null) { return []; } @@ -275,6 +280,8 @@ protected function getOptionsFromArray(Array_ $source, array $options): array foreach ($source->items as $item) { if (isset($item->key) && $item->key instanceof String_) { $options[$item->key->value] = $item->value; + } else { + throw new InvalidArgumentException('Rule is ignored because one option key is not string'); } } diff --git a/tests/TestCase/Rule/Model/Fake/FailingOrmFindRuleItemsLogic.php b/tests/TestCase/Rule/Model/Fake/FailingOrmFindRuleItemsLogic.php index 5c19933..b8b9a01 100644 --- a/tests/TestCase/Rule/Model/Fake/FailingOrmFindRuleItemsLogic.php +++ b/tests/TestCase/Rule/Model/Fake/FailingOrmFindRuleItemsLogic.php @@ -200,5 +200,15 @@ public function process() sort: ['Notes.note' => 'ASC'], myType: 19 ); + $field = $Table->getTypeTestTwoArgsButNotLegacy(); + $value = 'featured'; + $Table->find('twoArgsButNotLegacy', [$field => $value]); + $Table->find('twoArgsButNotLegacy', [$field => 'test']); + $Table->find('twoArgsButNotLegacy', [$Table->getTypeTestTwoArgsButNotLegacy() => $value]); + $Table->find('twoArgsButNotLegacy', [$Table->getTypeTestTwoArgsButNotLegacy() => 'sample']); + $Table->find('twoArgsButNotLegacy', ...[$field => $value]); + $Table->find('twoArgsButNotLegacy', ...[$field => 'test']); + $Table->find('twoArgsButNotLegacy', ...[$Table->getTypeTestTwoArgsButNotLegacy() => $value]); + $Table->find('twoArgsButNotLegacy', ...[$Table->getTypeTestTwoArgsButNotLegacy() => 'sample']); } } diff --git a/tests/test_app/Model/Table/NotesTable.php b/tests/test_app/Model/Table/NotesTable.php index 872094f..e05df87 100644 --- a/tests/test_app/Model/Table/NotesTable.php +++ b/tests/test_app/Model/Table/NotesTable.php @@ -149,4 +149,12 @@ public function findArgsPacked(SelectQuery $query, mixed ...$args): SelectQuery $args['groupLabel'] ?? 'note', ]); } + + /** + * @return string + */ + public function getTypeTestTwoArgsButNotLegacy(): string + { + return 'myType'; + } } From 76a3e02cf155a9931cc7ba603ff50e9106a679bf Mon Sep 17 00:00:00 2001 From: Marcelo Rocha Date: Tue, 27 Feb 2024 12:24:14 -0300 Subject: [PATCH 4/5] refactor --- src/Rule/Model/OrmSelectQueryFindMatchOptionsTypesRule.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Rule/Model/OrmSelectQueryFindMatchOptionsTypesRule.php b/src/Rule/Model/OrmSelectQueryFindMatchOptionsTypesRule.php index c70f007..73ee26f 100644 --- a/src/Rule/Model/OrmSelectQueryFindMatchOptionsTypesRule.php +++ b/src/Rule/Model/OrmSelectQueryFindMatchOptionsTypesRule.php @@ -360,7 +360,7 @@ protected function getSpecificFinderOptions(array $details, Scope $scope): array $secondParam = $parameters[1]; $paramType = $secondParam->getType(); if ( - $parameters[1]->getName() === 'options' + $secondParam->getName() === 'options' && !$secondParam->isVariadic() && ($paramType instanceof MixedType || $paramType instanceof ArrayType) ) { From fa559f5521f46b4548fa0ae96e260c9e87a4fe93 Mon Sep 17 00:00:00 2001 From: Marcelo Rocha Date: Tue, 27 Feb 2024 12:26:56 -0300 Subject: [PATCH 5/5] Set version 3.1.2 --- .semver | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.semver b/.semver index 9624a57..3b53786 100644 --- a/.semver +++ b/.semver @@ -1,5 +1,5 @@ --- :major: 3 :minor: 1 -:patch: 1 +:patch: 2 :special: ''