Merge branch '4.next' into 5.0

psalm-baseline.xml

@@ -233,12 +233,13 @@
     </DeprecatedProperty>
   </file>
   <file src="src/TestSuite/TestCase.php">
-    <DeprecatedProperty occurrences="5">
+    <DeprecatedProperty occurrences="6">
       <code>$this-&gt;autoFixtures</code>
       <code>$this-&gt;autoFixtures</code>
       <code>$this-&gt;autoFixtures</code>
       <code>$this-&gt;autoFixtures</code>
       <code>$this-&gt;dropTables</code>
+      <code>$this-&gt;dropTables</code>
     </DeprecatedProperty>
   </file>
   <file src="src/TestSuite/TestSuite.php">

src/Database/Expression/ComparisonExpression.php

@@ -150,7 +150,10 @@ class ComparisonExpression implements ExpressionInterface, FieldInterface
             $field = $field->sql($binder);
         }
 
-        if ($this->_value instanceof ExpressionInterface) {
+        if ($this->_value instanceof IdentifierExpression) {
+            $template = '%s %s %s';
+            $value = $this->_value->sql($binder);
+        } elseif ($this->_value instanceof ExpressionInterface) {
             $template = '%s %s (%s)';
             $value = $this->_value->sql($binder);
         } else {
@@ -210,7 +213,7 @@ class ComparisonExpression implements ExpressionInterface, FieldInterface
     {
         $template = '%s ';
 
-        if ($this->_field instanceof ExpressionInterface) {
+        if ($this->_field instanceof ExpressionInterface && !$this->_field instanceof IdentifierExpression) {
             $template = '(%s) ';
         }
 

src/TestSuite/TestCase.php

@@ -232,6 +232,9 @@ abstract class TestCase extends BaseTestCase
         parent::setUp();
         $this->fixtureManager = FixtureLoader::getInstance();
         if ($this->fixtureManager) {
+            if ($this->dropTables) {
+                deprecationWarning('`$dropTables` is deprecated and will be removed in 5.0.');
+            }
             $this->fixtureManager->setupTest($this);
         }
 

src/View/ViewBuilder.php

@@ -555,6 +555,12 @@ class ViewBuilder implements JsonSerializable
             throw new MissingViewException(['class' => $this->_className]);
         }
 
+        if (!empty($vars)) {
+            deprecationWarning(
+                'The $vars argument is deprecated. Use the setVar()/setVars() methods instead.'
+            );
+        }
+
         $data = [
             'name' => $this->_name,
             'templatePath' => $this->_templatePath,

tests/TestCase/Database/Expression/AggregateExpressionTest.php

@@ -53,7 +53,7 @@ class AggregateExpressionTest extends FunctionExpressionTest
     {
         $f = (new AggregateExpression('MyFunction'))->filter(['this' => new IdentifierExpression('that')]);
         $this->assertEqualsSql(
-            'MyFunction() FILTER (WHERE this = (that))',
+            'MyFunction() FILTER (WHERE this = that)',
             $f->sql(new ValueBinder())
         );
 
@@ -61,13 +61,13 @@ class AggregateExpressionTest extends FunctionExpressionTest
             return $q->add(['this2' => new IdentifierExpression('that2')]);
         });
         $this->assertEqualsSql(
-            'MyFunction() FILTER (WHERE (this = (that) AND this2 = (that2)))',
+            'MyFunction() FILTER (WHERE (this = that AND this2 = that2))',
             $f->sql(new ValueBinder())
         );
 
         $f->over();
         $this->assertEqualsSql(
-            'MyFunction() FILTER (WHERE (this = (that) AND this2 = (that2))) OVER ()',
+            'MyFunction() FILTER (WHERE (this = that AND this2 = that2)) OVER ()',
             $f->sql(new ValueBinder())
         );
     }

tests/TestCase/Database/Expression/ComparisonExpressionTest.php

@@ -17,7 +17,9 @@ declare(strict_types=1);
 namespace Cake\Test\TestCase\Database\Expression;
 
 use Cake\Database\Expression\ComparisonExpression;
+use Cake\Database\Expression\IdentifierExpression;
 use Cake\Database\Expression\QueryExpression;
+use Cake\Database\ValueBinder;
 use Cake\TestSuite\TestCase;
 
 /**
@@ -26,6 +28,27 @@ use Cake\TestSuite\TestCase;
 class ComparisonExpressionTest extends TestCase
 {
     /**
+     * Test sql generation using IdentifierExpression
+     */
+    public function testIdentifiers(): void
+    {
+        $expr = new ComparisonExpression('field', new IdentifierExpression('other_field'));
+        $this->assertEqualsSql('field = other_field', $expr->sql(new ValueBinder()));
+
+        $expr = new ComparisonExpression(new IdentifierExpression('field'), new IdentifierExpression('other_field'));
+        $this->assertEqualsSql('field = other_field', $expr->sql(new ValueBinder()));
+
+        $expr = new ComparisonExpression(new IdentifierExpression('field'), new QueryExpression(['other_field']));
+        $this->assertEqualsSql('field = (other_field)', $expr->sql(new ValueBinder()));
+
+        $expr = new ComparisonExpression(new IdentifierExpression('field'), 'value');
+        $this->assertEqualsSql('field = :c0', $expr->sql(new ValueBinder()));
+
+        $expr = new ComparisonExpression(new QueryExpression(['field']), new IdentifierExpression('other_field'));
+        $this->assertEqualsSql('field = other_field', $expr->sql(new ValueBinder()));
+    }
+
+    /**
      * Tests that cloning Comparion instance clones it's value and field expressions.
      */
     public function testClone(): void

tests/TestCase/Database/QueryTest.php

@@ -2850,7 +2850,7 @@ class QueryTest extends TestCase
             'DELETE FROM <authors> WHERE \(' .
                 '<id> = :c0 OR \(<name>\) IS NULL OR \(<email>\) IS NOT NULL OR \(' .
                     '<name> not in \(:c1,:c2\) AND \(' .
-                        '\(<name>\) = :c3 OR <name> = :c4 OR \(SELECT <e>\.<name> WHERE <e>\.<name> = :c5\)' .
+                        '<name> = :c3 OR <name> = :c4 OR \(SELECT <e>\.<name> WHERE <e>\.<name> = :c5\)' .
                     '\)' .
                 '\)' .
             '\)',
@@ -2961,7 +2961,7 @@ class QueryTest extends TestCase
         $result = $query->sql();
 
         $this->assertQuotedQuery(
-            'UPDATE <comments> SET <article_id> = \(<user_id>\) WHERE <id> = :',
+            'UPDATE <comments> SET <article_id> = <user_id> WHERE <id> = :',
             $result,
             !$this->autoQuote
         );
@@ -2972,6 +2972,41 @@ class QueryTest extends TestCase
     }
 
     /**
+     * Tests update with subquery that references itself
+     */
+    public function testUpdateSubquery(): void
+    {
+        $this->skipIf($this->connection->getDriver() instanceof Mysql);
+
+        $this->loadFixtures('Comments');
+
+        $subquery = new Query($this->connection);
+        $subquery
+            ->select('created')
+            ->from(['c' => 'comments'])
+            ->where(['c.id' => new IdentifierExpression('comments.id')]);
+
+        $query = new Query($this->connection);
+        $query->update('comments')
+            ->set('updated', $subquery);
+
+        $this->assertEqualsSql(
+            'UPDATE comments SET updated = (SELECT created FROM comments c WHERE c.id = comments.id)',
+            $query->sql(new ValueBinder())
+        );
+
+        $result = $query->execute();
+        $this->assertCount(6, $result);
+        $result->closeCursor();
+
+        $result = (new Query($this->connection))->select(['created', 'updated'])->from('comments')->execute();
+        foreach ($result->fetchAll('assoc') as $row) {
+            $this->assertSame($row['created'], $row['updated']);
+        }
+        $result->closeCursor();
+    }
+
+    /**
      * Test update with array fields and types.
      */
     public function testUpdateArrayFields(): void
@@ -3059,7 +3094,7 @@ class QueryTest extends TestCase
             'UPDATE <authors> SET <name> = :c0 WHERE \(' .
                 '<id> = :c1 OR \(<name>\) IS NULL OR \(<email>\) IS NOT NULL OR \(' .
                     '<name> not in \(:c2,:c3\) AND \(' .
-                        '\(<name>\) = :c4 OR <name> = :c5 OR \(SELECT <e>\.<name> WHERE <e>\.<name> = :c6\)' .
+                        '<name> = :c4 OR <name> = :c5 OR \(SELECT <e>\.<name> WHERE <e>\.<name> = :c6\)' .
                     '\)' .
                 '\)' .
             '\)',

tests/TestCase/View/ViewBuilderTest.php

@@ -189,42 +189,44 @@ class ViewBuilderTest extends TestCase
      */
     public function testBuildComplete(): void
     {
-        $request = new ServerRequest();
-        $response = new Response();
-        $events = new EventManager();
-
-        $builder = new ViewBuilder();
-        $builder->setName('Articles')
-            ->setClassName('Ajax')
-            ->setTemplate('edit')
-            ->setLayout('default')
-            ->setTemplatePath('Articles/')
-            ->setHelpers(['Form', 'Html'])
-            ->setLayoutPath('Admin/')
-            ->setTheme('TestTheme')
-            ->setPlugin('TestPlugin')
-            ->setVars(['foo' => 'bar', 'x' => 'old']);
-        $view = $builder->build(
-            ['one' => 'value', 'x' => 'new'],
-            $request,
-            $response,
-            $events
-        );
-        $this->assertInstanceOf('Cake\View\AjaxView', $view);
-        $this->assertSame('edit', $view->getTemplate());
-        $this->assertSame('default', $view->getLayout());
-        $this->assertSame('Articles/', $view->getTemplatePath());
-        $this->assertSame('Admin/', $view->getLayoutPath());
-        $this->assertSame('TestPlugin', $view->getPlugin());
-        $this->assertSame('TestTheme', $view->getTheme());
-        $this->assertSame($request, $view->getRequest());
-        $this->assertInstanceOf(Response::class, $view->getResponse());
-        $this->assertSame($events, $view->getEventManager());
-        $this->assertSame(['one', 'x', 'foo'], $view->getVars());
-        $this->assertSame('value', $view->get('one'));
-        $this->assertSame('bar', $view->get('foo'));
-        $this->assertInstanceOf('Cake\View\Helper\HtmlHelper', $view->Html);
-        $this->assertInstanceOf('Cake\View\Helper\FormHelper', $view->Form);
+        $this->deprecated(function () {
+            $request = new ServerRequest();
+            $response = new Response();
+            $events = new EventManager();
+
+            $builder = new ViewBuilder();
+            $builder->setName('Articles')
+                ->setClassName('Ajax')
+                ->setTemplate('edit')
+                ->setLayout('default')
+                ->setTemplatePath('Articles/')
+                ->setHelpers(['Form', 'Html'])
+                ->setLayoutPath('Admin/')
+                ->setTheme('TestTheme')
+                ->setPlugin('TestPlugin')
+                ->setVars(['foo' => 'bar', 'x' => 'old']);
+            $view = $builder->build(
+                ['one' => 'value', 'x' => 'new'],
+                $request,
+                $response,
+                $events
+            );
+            $this->assertInstanceOf('Cake\View\AjaxView', $view);
+            $this->assertSame('edit', $view->getTemplate());
+            $this->assertSame('default', $view->getLayout());
+            $this->assertSame('Articles/', $view->getTemplatePath());
+            $this->assertSame('Admin/', $view->getLayoutPath());
+            $this->assertSame('TestPlugin', $view->getPlugin());
+            $this->assertSame('TestTheme', $view->getTheme());
+            $this->assertSame($request, $view->getRequest());
+            $this->assertInstanceOf(Response::class, $view->getResponse());
+            $this->assertSame($events, $view->getEventManager());
+            $this->assertSame(['one', 'x', 'foo'], $view->getVars());
+            $this->assertSame('value', $view->get('one'));
+            $this->assertSame('bar', $view->get('foo'));
+            $this->assertInstanceOf('Cake\View\Helper\HtmlHelper', $view->Html);
+            $this->assertInstanceOf('Cake\View\Helper\FormHelper', $view->Form);
+        });
     }
 
     /**