Improve the tests for the TracingStatementForV*::bindParam() method

Author: ste93cry

CHANGELOG.md

@@ -2,7 +2,7 @@
 
 ## Unreleased
 
-- Fix PDOStatement deprecation notice on `bindParam()` (#586)
+- Fix deprecation notice thrown when instrumenting the `PDOStatement::bindParam()` method and passing `$length = null` (#586)
 
 ## 4.2.6 (2022-01-10)
 

phpstan-baseline.neon

@@ -245,6 +245,16 @@ parameters:
 			count: 1
 			path: tests/Tracing/Doctrine/DBAL/TracingServerInfoAwareDriverConnectionTest.php
 
+		-
+			message: "#^Access to an undefined property PHPUnit\\\\Framework\\\\MockObject\\\\MockObject&Sentry\\\\SentryBundle\\\\Tests\\\\Tracing\\\\Doctrine\\\\DBAL\\\\TracingStatementForV2Stub\\:\\:\\$bindParamCallArgsCount\\.$#"
+			count: 1
+			path: tests/Tracing/Doctrine/DBAL/TracingStatementForV2Test.php
+
+		-
+			message: "#^Parameter \\#2 \\$decoratedStatement of class Sentry\\\\SentryBundle\\\\Tracing\\\\Doctrine\\\\DBAL\\\\TracingStatementForV2 constructor expects Doctrine\\\\DBAL\\\\Driver\\\\Statement, PHPUnit\\\\Framework\\\\MockObject\\\\MockObject&Sentry\\\\SentryBundle\\\\Tests\\\\Tracing\\\\Doctrine\\\\DBAL\\\\TracingStatementForV2Stub given\\.$#"
+			count: 1
+			path: tests/Tracing/Doctrine/DBAL/TracingStatementForV2Test.php
+
 		-
 			message: "#^Trying to mock an undefined method closeCursor\\(\\) on class Doctrine\\\\DBAL\\\\Driver\\\\Statement\\.$#"
 			count: 1

src/Tracing/Doctrine/DBAL/TracingStatementForV2.php

@@ -108,7 +108,7 @@ public function bindValue($param, $value, $type = ParameterType::STRING): bool
      */
     public function bindParam($param, &$variable, $type = ParameterType::STRING, $length = null): bool
     {
-        return $this->decoratedStatement->bindParam($param, $variable, $type, $length);
+        return $this->decoratedStatement->bindParam($param, $variable, $type, ...\array_slice(\func_get_args(), 3));
     }
 
     /**

src/Tracing/Doctrine/DBAL/TracingStatementForV3.php

@@ -27,7 +27,7 @@ public function bindValue($param, $value, $type = ParameterType::STRING): bool
      */
     public function bindParam($param, &$variable, $type = ParameterType::STRING, $length = null): bool
     {
-        return $this->decoratedStatement->bindParam($param, $variable, $type, ...array_slice(func_get_args(), 3));
+        return $this->decoratedStatement->bindParam($param, $variable, $type, ...\array_slice(\func_get_args(), 3));
     }
 
     /**

tests/Tracing/Doctrine/DBAL/TracingStatementForV2Test.php

@@ -123,10 +123,21 @@ public function testBindParam(): void
 
         $this->decoratedStatement->expects($this->once())
             ->method('bindParam')
-            ->with('foo', $variable, ParameterType::INTEGER)
+            ->with('foo', $variable, ParameterType::INTEGER, 10)
             ->willReturn(true);
 
+        $this->assertTrue($this->statement->bindParam('foo', $variable, ParameterType::INTEGER, 10));
+    }
+
+    public function testBindParamForwardsLengthParamOnlyWhenExplicitlySet(): void
+    {
+        $variable = 'bar';
+        $decoratedStatement = $this->createPartialMock(TracingStatementForV2Stub::class, array_diff(get_class_methods(TracingStatementForV2Stub::class), ['bindParam']));
+
+        $this->statement = new TracingStatementForV2($this->hub, $decoratedStatement, 'SELECT 1', ['db.system' => 'sqlite']);
+
         $this->assertTrue($this->statement->bindParam('foo', $variable, ParameterType::INTEGER));
+        $this->assertSame(3, $decoratedStatement->bindParamCallArgsCount);
     }
 
     public function testErrorCode(): void
@@ -195,3 +206,31 @@ public function testRowCount(): void
         $this->assertSame(10, $this->statement->rowCount());
     }
 }
+
+if (!interface_exists(Statement::class)) {
+    abstract class TracingStatementForV2Stub
+    {
+    }
+} else {
+    /**
+     * @phpstan-implements \IteratorAggregate<mixed, mixed>
+     */
+    abstract class TracingStatementForV2Stub implements \IteratorAggregate, Statement
+    {
+        /**
+         * @var int
+         */
+        public $bindParamCallArgsCount = 0;
+
+        public function bindParam($param, &$variable, $type = ParameterType::STRING, $length = null): bool
+        {
+            // Since PHPUnit forcefully calls the mocked methods with all
+            // parameters, regardless of whether they were originally passed
+            // in an explicit manner, we can't use a mock to assert the number
+            // of args used in the call to the function
+            $this->bindParamCallArgsCount = \func_num_args();
+
+            return true;
+        }
+    }
+}

tests/Tracing/Doctrine/DBAL/TracingStatementForV3Test.php

@@ -61,22 +61,47 @@ public function testBindParam(): void
 
         $this->decoratedStatement->expects($this->once())
             ->method('bindParam')
-            ->with('foo', $variable, ParameterType::INTEGER)
+            ->with('foo', $variable, ParameterType::INTEGER, 10)
             ->willReturn(true);
 
-        $this->assertTrue($this->statement->bindParam('foo', $variable, ParameterType::INTEGER));
+        $this->assertTrue($this->statement->bindParam('foo', $variable, ParameterType::INTEGER, 10));
     }
 
-    public function testBindParamWithoutLength(): void
+    public function testBindParamForwardsLengthParamOnlyWhenExplicitlySet(): void
     {
         $variable = 'bar';
-
-        $this->decoratedStatement = $this->createPartialMock(TestStatement::class, ['bindValue', 'execute']);
-        $this->statement = new TracingStatementForV3($this->hub, $this->decoratedStatement, 'SELECT 1', ['db.system' => 'sqlite']);
+        $decoratedStatement = new class() implements Statement {
+            /**
+             * @var int
+             */
+            public $bindParamCallArgsCount = 0;
+
+            public function bindValue($param, $value, $type = ParameterType::STRING): bool
+            {
+                throw new \BadMethodCallException();
+            }
+
+            public function bindParam($param, &$variable, $type = ParameterType::STRING, $length = null): bool
+            {
+                // Since PHPUnit forcefully calls the mocked methods with all
+                // parameters, regardless of whether they were originally passed
+                // in an explicit manner, we can't use a mock to assert the number
+                // of args used in the call to the function
+                $this->bindParamCallArgsCount = \func_num_args();
+
+                return true;
+            }
+
+            public function execute($params = null): Result
+            {
+                throw new \BadMethodCallException();
+            }
+        };
+
+        $this->statement = new TracingStatementForV3($this->hub, $decoratedStatement, 'SELECT 1', ['db.system' => 'sqlite']);
 
         $this->assertTrue($this->statement->bindParam('foo', $variable, ParameterType::INTEGER));
-        $this->assertInstanceOf(TestStatement::class, $this->decoratedStatement);
-        $this->assertCount(3, $this->decoratedStatement->args);
+        $this->assertSame(3, $decoratedStatement->bindParamCallArgsCount);
     }
 
     public function testExecute(): void
@@ -121,18 +146,3 @@ public function testExecuteDoesNothingIfNoSpanIsSetOnHub(): void
         $this->assertSame($driverResult, $this->statement->execute(['foo' => 'bar']));
     }
 }
-
-abstract class TestStatement implements Statement {
-
-    /**
-     * @var mixed[]|null
-     */
-    public $args = null;
-
-    public function bindParam($param, &$variable, $type = ParameterType::STRING, $length = null)
-    {
-        $this->args = func_get_args();
-
-        return true;
-    }
-}