From 43219fa15e12e7e78772e78f5dbd9bbb2cb18f93 Mon Sep 17 00:00:00 2001
From: Mark Story <mark@mark-story.com>
Date: Mon, 8 Apr 2024 23:44:07 -0400
Subject: [PATCH] Fix finder compatibilty with 4.x

We have rector rules to update the callsites of finders to use named
parameters. What we lack is rector operations to upgrade finder
declarations to use named parameters. Because this is an impossible
task, we should have better compatibility for new style finder calls
with old style options arrays. This helps improve finder ergonomics
(less typing) for more folks.

This would have saved me a bunch of work when upgraded an application
recently and making the upgrade smoother is important to me.
---
 src/ORM/Table.php                             | 37 ++++++++++++++-----
 tests/TestCase/ORM/TableTest.php              | 17 +++++++++
 .../TestApp/Model/Table/ArticlesTable.php     | 12 ++++++
 3 files changed, 56 insertions(+), 10 deletions(-)

diff --git a/src/ORM/Table.php b/src/ORM/Table.php
index ffe06dcdc30..7ada22fee5a 100644
--- a/src/ORM/Table.php
+++ b/src/ORM/Table.php
@@ -2666,22 +2666,29 @@ public function callFinder(string $type, SelectQuery $query, mixed ...$args): Se
      */
     public function invokeFinder(Closure $callable, SelectQuery $query, array $args): SelectQuery
     {
+        // TODO this is where the missing shims need to be.
         $reflected = new ReflectionFunction($callable);
         $params = $reflected->getParameters();
         $secondParam = $params[1] ?? null;
-        $secondParamType = null;
+
+        $secondParamType = $secondParam?->getType();
+        $secondParamTypeName = $secondParamType instanceof ReflectionNamedType ? $secondParamType->getName() : null;
+
+        $secondParamIsOptions = (
+            count($params) === 2 &&
+            $secondParam?->name === 'options' &&
+            !$secondParam->isVariadic() &&
+            ($secondParamType === null || $secondParamTypeName === 'array')
+        );
 
         if ($args === [] || isset($args[0])) {
             $secondParamType = $secondParam?->getType();
             $secondParamTypeName = $secondParamType instanceof ReflectionNamedType ? $secondParamType->getName() : null;
-            // Backwards compatibility of 4.x style finders with signature `findFoo(SelectQuery $query, array $options)`
+
+            // Backwards compatibility of 4.x style finders
+            // with signature `findFoo(SelectQuery $query, array $options)`
             // called as `find('foo')` or `find('foo', [..])`
-            if (
-                count($params) === 2 &&
-                $secondParam?->name === 'options' &&
-                !$secondParam->isVariadic() &&
-                ($secondParamType === null || $secondParamTypeName === 'array')
-            ) {
+            if ($secondParamIsOptions) {
                 if (isset($args[0])) {
                     deprecationWarning(
                         '5.0.0',
@@ -2697,8 +2704,9 @@ public function invokeFinder(Closure $callable, SelectQuery $query, array $args)
                 return $callable($query, $query->getOptions());
             }
 
-            // Backwards compatibility for core finders like `findList()` called in 4.x style
-            // with an array `find('list', ['valueField' => 'foo'])` instead of `find('list', valueField: 'foo')`
+            // Backwards compatibility for core finders like `findList()` called in 4.x
+            // style with an array `find('list', ['valueField' => 'foo'])` instead of
+            // `find('list', valueField: 'foo')`
             if (isset($args[0]) && is_array($args[0]) && $secondParamTypeName !== 'array') {
                 deprecationWarning(
                     '5.0.0',
@@ -2710,6 +2718,15 @@ public function invokeFinder(Closure $callable, SelectQuery $query, array $args)
             }
         }
 
+        // Backwards compatibility for 4.x style finders with signatures like
+        // `findFoo(SelectQuery $query, array $options)` called as
+        // `find('foo', key: $value)`.
+        if (!isset($args[0]) && $secondParamIsOptions) {
+            $query->applyOptions($args);
+
+            return $callable($query, $query->getOptions());
+        }
+
         if ($args) {
             $query->applyOptions($args);
             // Fetch custom args without the query options.
diff --git a/tests/TestCase/ORM/TableTest.php b/tests/TestCase/ORM/TableTest.php
index 75bd036887e..d3e577dbf4b 100644
--- a/tests/TestCase/ORM/TableTest.php
+++ b/tests/TestCase/ORM/TableTest.php
@@ -1286,6 +1286,23 @@ public function testFindTypedParameters(): void
         $this->assertSame(2, $author->id);
     }
 
+    public function testFindTypedParameterCompatibility(): void
+    {
+        $articles = $this->fetchTable('Articles');
+        $article = $articles->find('titled')->first();
+        $this->assertNotEmpty($article);
+
+        // Options arrays should work
+        $article = $articles->find('titled', ['title' => 'Second Article'])->first();
+        $this->assertNotEmpty($article);
+        $this->assertEquals('Second Article', $article->title);
+
+        // Named parameters should be compatible with options finders
+        $article = $articles->find('titled', title: 'Second Article')->first();
+        $this->assertNotEmpty($article);
+        $this->assertEquals('Second Article', $article->title);
+    }
+
     public function testFindForFinderVariadic(): void
     {
         $testTable = $this->fetchTable('Test');
diff --git a/tests/test_app/TestApp/Model/Table/ArticlesTable.php b/tests/test_app/TestApp/Model/Table/ArticlesTable.php
index 5d9d045bc2c..542592aa549 100644
--- a/tests/test_app/TestApp/Model/Table/ArticlesTable.php
+++ b/tests/test_app/TestApp/Model/Table/ArticlesTable.php
@@ -57,6 +57,18 @@ public function findWithAuthors($query, array $options = []): SelectQuery
         return $query->contain('Authors');
     }
 
+    /**
+     * Finder for testing named parameter compatibility
+     */
+    public function findTitled(SelectQuery $query, array $options): SelectQuery
+    {
+        if (!empty($options['title'])) {
+            $query->where(['Articles.title' => $options['title']]);
+        }
+
+        return $query;
+    }
+
     /**
      * Example public method
      */