Browse Source

Show deprecation warning for old style finder usage with options array.

ADmad 2 years ago
parent
commit
ec47339c34

+ 14 - 0
src/ORM/Table.php

@@ -57,6 +57,7 @@ use Exception;
 use InvalidArgumentException;
 use ReflectionFunction;
 use ReflectionNamedType;
+use function Cake\Core\deprecationWarning;
 use function Cake\Core\namespaceSplit;
 
 /**
@@ -2677,6 +2678,13 @@ class Table implements RepositoryInterface, EventListenerInterface, EventDispatc
                 $secondParam?->name === 'options' &&
                 ($secondParamType === null || $secondParamTypeName === 'array')
             ) {
+                if (!$secondParam->isDefaultValueAvailable()) {
+                    deprecationWarning(
+                        '5.0.0',
+                        '`$options` argument must have a default value'
+                    );
+                }
+
                 if (isset($options[0])) {
                     $options = $options[0];
                 }
@@ -2689,6 +2697,12 @@ class Table implements RepositoryInterface, EventListenerInterface, EventDispatc
             // 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',
+                    'Calling this finder with options array is deprecated.'
+                     . ' Use named arguments instead.'
+                );
+
                 $options = $args = $args[0];
             }
         }

+ 11 - 9
tests/TestCase/ORM/AssociationTest.php

@@ -496,15 +496,17 @@ class AssociationTest extends TestCase
     {
         $this->association->setFinder('withOptions');
 
-        $this->assertEquals(
-            ['this' => 'worked'],
-            $this->association->find(null)->getOptions()
-        );
-
-        $this->assertEquals(
-            ['that' => 'custom', 'this' => 'worked'],
-            $this->association->find(null, ['that' => 'custom'])->getOptions()
-        );
+        $this->deprecated(function () {
+            $this->assertEquals(
+                ['this' => 'worked'],
+                $this->association->find(null)->getOptions()
+            );
+
+            $this->assertEquals(
+                ['that' => 'custom', 'this' => 'worked'],
+                $this->association->find(null, ['that' => 'custom'])->getOptions()
+            );
+        });
     }
 
     /**

+ 2 - 2
tests/TestCase/ORM/Behavior/TreeBehaviorTest.php

@@ -635,7 +635,7 @@ class TreeBehaviorTest extends TestCase
         $table = $this->table;
 
         $expectedLevels = $table
-            ->find('list', ['valueField' => 'depth'])
+            ->find('list', valueField: 'depth')
             ->orderBy('lft')
             ->toArray();
         $table->updateAll(['lft' => null, 'rght' => null, 'depth' => null], []);
@@ -658,7 +658,7 @@ class TreeBehaviorTest extends TestCase
         $this->assertMpttValues($expected, $table);
 
         $result = $table
-            ->find('list', ['valueField' => 'depth'])
+            ->find('list', valueField: 'depth')
             ->orderBy('lft')
             ->toArray();
         $this->assertSame($expectedLevels, $result);

+ 1 - 1
tests/TestCase/ORM/BehaviorRegistryTest.php

@@ -304,7 +304,7 @@ class BehaviorRegistryTest extends TestCase
             ->method('findNoSlug')
             ->with($query)
             ->will($this->returnValue($query));
-        $return = $this->Behaviors->callFinder('noSlug', $query, []);
+        $return = $this->Behaviors->callFinder('noSlug', $query);
         $this->assertSame($query, $return);
     }
 

+ 20 - 21
tests/TestCase/ORM/TableTest.php

@@ -1592,26 +1592,29 @@ class TableTest extends TestCase
      * Test find('list') called with option array instead of named args for backwards compatility
      *
      * @return void
+     * @deprecated
      */
     public function testFindListWithArray(): void
     {
-        $articles = new Table([
-            'table' => 'articles',
-            'connection' => $this->connection,
-        ]);
+        $this->deprecated(function () {
+            $articles = new Table([
+                'table' => 'articles',
+                'connection' => $this->connection,
+            ]);
 
-        $articles->belongsTo('Authors');
-        $query = $articles->find('list', ['valueField' => 'author.name'])
-            ->contain(['Authors'])
-            ->orderBy('articles.id');
-        $this->assertEmpty($query->clause('select'));
+            $articles->belongsTo('Authors');
+            $query = $articles->find('list', ['valueField' => 'author.name'])
+                ->contain(['Authors'])
+                ->orderBy('articles.id');
+            $this->assertEmpty($query->clause('select'));
 
-        $expected = [
-            1 => 'mariano',
-            2 => 'larry',
-            3 => 'mariano',
-        ];
-        $this->assertSame($expected, $query->toArray());
+            $expected = [
+                1 => 'mariano',
+                2 => 'larry',
+                3 => 'mariano',
+            ];
+            $this->assertSame($expected, $query->toArray());
+        });
     }
 
     /**
@@ -3087,11 +3090,7 @@ class TableTest extends TestCase
         $result = $table->delete($entity);
         $this->assertFalse($result);
 
-        $query = $articles->find('all', [
-            'conditions' => [
-                'author_id' => $entity->id,
-            ],
-        ]);
+        $query = $articles->find('all', conditions: ['author_id' => $entity->id]);
         $this->assertFalse($query->all()->isEmpty(), 'Should find some rows.');
 
         $table->associations()->get('articles')->setCascadeCallbacks(false);
@@ -3989,7 +3988,7 @@ class TableTest extends TestCase
             ],
         ];
         $result = $this->getTableLocator()->get('PolymorphicTagged')
-            ->find('all', ['sort' => ['id' => 'DESC']])
+            ->find('all', sort: ['id' => 'DESC'])
             ->enableHydration(false)
             ->toArray();
         $this->assertEquals($expected, $result);

+ 1 - 1
tests/test_app/TestApp/Model/Table/GreedyCommentsTable.php

@@ -36,6 +36,6 @@ class GreedyCommentsTable extends Table
         }
         $options['conditions'] = array_merge($options['conditions'], ['Comments.published' => 'Y']);
 
-        return parent::find($type, $options);
+        return parent::find($type, ...$options);
     }
 }