Fix deprecation: PDOStatement::bindParam(): Passing null to $maxLength (

#586) * Fix deprecation: Deprecated: PDOStatement::bindParam(): Passing null to parameter #4 ($maxLength) of type int is deprecated in ./vendor/doctrine/dbal/src/Driver/PDO/Statement.php on line 83 * add testBindParamWithoutLength add changelog * Update tests/Tracing/Doctrine/DBAL/TracingStatementForV3Test.php Co-authored-by: Alessandro Lai <alessandro.lai85@gmail.com> * Update tests/Tracing/Doctrine/DBAL/TracingStatementForV3Test.php Co-authored-by: Alessandro Lai <alessandro.lai85@gmail.com> * Improve the tests for the `TracingStatementForV*::bindParam()` method Co-authored-by: Alessandro Lai <alessandro.lai85@gmail.com> Co-authored-by: Stefano Arlandini <sarlandini@alice.it>
getsentry · Feb 14, 2022 · 3a16fdc · 3a16fdc
1 parent 0c0b33c
commit 3a16fdc
Show file tree

Hide file tree

Showing 6 changed files with 92 additions and 4 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,8 @@
 
 ## Unreleased
 
+- Fix deprecation notice thrown when instrumenting the `PDOStatement::bindParam()` method and passing `$length = null` (#586)
+
 ## 4.2.6 (2022-01-10)
 
 - Add support for `symfony/cache-contracts` package version `3.x` (#588)

diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon
@@ -440,6 +440,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

diff --git a/src/Tracing/Doctrine/DBAL/TracingStatementForV2.php b/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));
     }
 
     /**

diff --git a/src/Tracing/Doctrine/DBAL/TracingStatementForV3.php b/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, $length);
+        return $this->decoratedStatement->bindParam($param, $variable, $type, ...\array_slice(\func_get_args(), 3));
     }
 
     /**

diff --git a/tests/Tracing/Doctrine/DBAL/TracingStatementForV2Test.php b/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
@@ -196,3 +207,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;
+        }
+    }
+}
diff --git a/tests/Tracing/Doctrine/DBAL/TracingStatementForV3Test.php b/tests/Tracing/Doctrine/DBAL/TracingStatementForV3Test.php
@@ -61,10 +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, 10));
+    }
+
+    public function testBindParamForwardsLengthParamOnlyWhenExplicitlySet(): void
+    {
+        $variable = 'bar';
+        $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->assertSame(3, $decoratedStatement->bindParamCallArgsCount);
     }
 
     public function testExecute(): void