Fix deprecation notice when passing $length = null to Statement::bindParam() with DBAL 2.x (#613)

Co-authored-by: Stefano Arlandini <[email protected]>

Author: Cameron Junge

CHANGELOG.md

@@ -2,6 +2,8 @@
 
 ## Unreleased
 
+- Fix deprecation notice thrown when instrumenting the `PDOStatement::bindParam()` method and passing `$length = null` on DBAL `2.x` (#613)
+
 ## 4.2.8 (2022-03-31)
 
 - Fix compatibility issue with Doctrine Bundle `>= 2.6.0` (#608)

phpstan-baseline.neon

@@ -441,12 +441,12 @@ parameters:
 			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\\.$#"
+			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: "#^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\\.$#"
+			message: "#^Parameter \\#4 \\$length of method Sentry\\\\SentryBundle\\\\Tracing\\\\Doctrine\\\\DBAL\\\\TracingStatementForV2\\:\\:bindParam\\(\\) expects int\\|null, mixed given\\.$#"
 			count: 1
 			path: tests/Tracing/Doctrine/DBAL/TracingStatementForV2Test.php
 

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, ...\array_slice(\func_get_args(), 3));
+        return $this->decoratedStatement->bindParam($param, $variable, $type, $length ?? 0, ...\array_slice(\func_get_args(), 4));
     }
 
     /**

tests/Tracing/Doctrine/DBAL/TracingStatementForV2Test.php

@@ -117,27 +117,42 @@ public function testBindValue(): void
         $this->assertTrue($this->statement->bindValue('foo', 'bar', ParameterType::INTEGER));
     }
 
-    public function testBindParam(): void
+    /**
+     * @dataProvider bindParamDataProvider
+     *
+     * @param mixed[] $callArgs
+     * @param mixed[] $expectedCallArgs
+     */
+    public function testBindParam(array $callArgs, array $expectedCallArgs): void
     {
         $variable = 'bar';
+        $decoratedStatement = $this->createPartialMock(TracingStatementForV2Stub::class, array_diff(get_class_methods(TracingStatementForV2Stub::class), ['bindParam']));
 
-        $this->decoratedStatement->expects($this->once())
-            ->method('bindParam')
-            ->with('foo', $variable, ParameterType::INTEGER, 10)
-            ->willReturn(true);
+        $this->statement = new TracingStatementForV2($this->hub, $decoratedStatement, 'SELECT 1', ['db.system' => 'sqlite']);
 
-        $this->assertTrue($this->statement->bindParam('foo', $variable, ParameterType::INTEGER, 10));
+        $this->assertTrue($this->statement->bindParam('foo', $variable, ParameterType::INTEGER, ...$callArgs));
+        $this->assertSame($expectedCallArgs, $decoratedStatement->bindParamCallArgs);
     }
 
-    public function testBindParamForwardsLengthParamOnlyWhenExplicitlySet(): void
+    /**
+     * @return \Generator<mixed>
+     */
+    public function bindParamDataProvider(): \Generator
     {
-        $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);
+        yield '$length parameter not passed at all' => [
+            [],
+            ['foo', 'bar', 1, 0],
+        ];
+
+        yield '$length parameter passed as `null`' => [
+            [null],
+            ['foo', 'bar', 1, 0],
+        ];
+
+        yield 'additional parameters passed' => [
+            [null, 'baz'],
+            ['foo', 'bar', 1, 0, 'baz'],
+        ];
     }
 
     public function testErrorCode(): void
@@ -211,6 +226,10 @@ public function testRowCount(): void
 if (!interface_exists(Statement::class)) {
     abstract class TracingStatementForV2Stub
     {
+        /**
+         * @var mixed[]
+         */
+        public $bindParamCallArgs = [];
     }
 } else {
     /**
@@ -219,17 +238,17 @@ abstract class TracingStatementForV2Stub
     abstract class TracingStatementForV2Stub implements \IteratorAggregate, Statement
     {
         /**
-         * @var int
+         * @var mixed[]
          */
-        public $bindParamCallArgsCount = 0;
+        public $bindParamCallArgs = [];
 
         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();
+            $this->bindParamCallArgs = \func_get_args();
 
             return true;
         }