Support for delete, insert, update, replace (#483)

Co-authored-by: Jakub Vojacek <jakub@motv.eu> Co-authored-by: Markus Staab <maggus.staab@googlemail.com>
staabm · Jan 6, 2023 · af71a9e · af71a9e
1 parent c28af8a
commit af71a9e
Show file tree

Hide file tree

Showing 32 changed files with 6,928 additions and 6,493 deletions.
diff --git a/.phpstan-dba-mysqli.cache b/.phpstan-dba-mysqli.cache
diff --git a/.phpstan-dba-pdo-mysql.cache b/.phpstan-dba-pdo-mysql.cache
diff --git a/README.md b/README.md
@@ -12,6 +12,7 @@ This extension provides the following features, as long as you [stick to the rul
 * [query plan analysis](https://staabm.github.io/2022/08/16/phpstan-dba-query-plan-analysis.html) to detect performance issues
 * builtin support for `doctrine/dbal`, `mysqli`, and `PDO`
 * API to configure the same features for your custom sql based database access layer
+* Opt-In analysis of write queries (since version 0.2.55+)
 
 In case you are using Doctrine ORM, you might use `phpstan-dba` in tandem with [phpstan-doctrine](https://github.com/phpstan/phpstan-doctrine).
 
@@ -56,6 +57,7 @@ $config = new RuntimeConfiguration();
 // $config->debugMode(true);
 // $config->stringifyTypes(true);
 // $config->analyzeQueryPlans(true);
+// $config->analyzeWriteQueries(true); // requires transaction support in db schema and db driver
 
 // TODO: Put your database credentials here
 $mysqli = new mysqli('hostname', 'username', 'password', 'database');

diff --git a/docs/configuration.md b/docs/configuration.md
@@ -8,3 +8,4 @@ If not configured otherwise, the following defaults are used:
 - when analyzing a php8+ codebase, [`PDO::ERRMODE_EXCEPTION` error handling](https://www.php.net/manual/en/pdo.error-handling.php) is assumed.
 - when analyzing a php8.1+ codebase, [`mysqli_report(\MYSQLI_REPORT_ERROR | \MYSQLI_REPORT_STRICT);` error handling](https://www.php.net/mysqli_report) is assumed.
 - the fetch mode defaults to `QueryReflector::FETCH_TYPE_BOTH`, but can be configured using the [`defaultFetchMode`](https://github.com/staabm/phpstan-dba/tree/main/src/QueryReflection/RuntimeConfiguration.php) option.
+- only readable queries are analyzed per default. In case your database schema and database driver support transactions, you may consider enabled writable queries using the [`analyzeWriteQueries`](https://github.com/staabm/phpstan-dba/tree/main/src/QueryReflection/RuntimeConfiguration.php) option.
diff --git a/src/Analyzer/QueryPlanAnalyzerMysql.php b/src/Analyzer/QueryPlanAnalyzerMysql.php
@@ -51,14 +51,26 @@ public function analyze(string $query): QueryPlanResult
         $simulatedQuery = 'EXPLAIN '.$query;
 
         if ($this->connection instanceof PDO) {
-            $stmt = $this->connection->query($simulatedQuery);
+            $this->connection->beginTransaction();
 
-            // @phpstan-ignore-next-line
-            return $this->buildResult($simulatedQuery, $stmt);
+            try {
+                $stmt = $this->connection->query($simulatedQuery);
+
+                // @phpstan-ignore-next-line
+                return $this->buildResult($simulatedQuery, $stmt);
+            } finally {
+                $this->connection->rollBack();
+            }
         } else {
-            $result = $this->connection->query($simulatedQuery);
-            if ($result instanceof \mysqli_result) {
-                return $this->buildResult($simulatedQuery, $result);
+            $this->connection->begin_transaction();
+
+            try {
+                $result = $this->connection->query($simulatedQuery);
+                if ($result instanceof \mysqli_result) {
+                    return $this->buildResult($simulatedQuery, $result);
+                }
+            } finally {
+                $this->connection->rollback();
             }
         }
 

diff --git a/src/Ast/ExpressionFinder.php b/src/Ast/ExpressionFinder.php
@@ -109,8 +109,6 @@ public function findBindCalls(Expr $expr): array
     }
 
     /**
-     * XXX use astral simpleNameResolver instead.
-     *
      * @param Expr|Variable|MethodCall $node
      *
      * @return string|null

diff --git a/src/DbSchema/SchemaHasherMysql.php b/src/DbSchema/SchemaHasherMysql.php
@@ -61,15 +61,27 @@ public function hashDb(): string
 
         $hash = '';
         if ($this->connection instanceof PDO) {
-            $stmt = $this->connection->query($query);
-            foreach ($stmt as $row) {
-                $hash = $row['dbsignature'] ?? '';
+            $this->connection->beginTransaction();
+
+            try {
+                $stmt = $this->connection->query($query);
+                foreach ($stmt as $row) {
+                    $hash = $row['dbsignature'] ?? '';
+                }
+            } finally {
+                $this->connection->rollBack();
             }
         } else {
-            $result = $this->connection->query($query);
-            if ($result instanceof \mysqli_result) {
-                $row = $result->fetch_assoc();
-                $hash = $row['dbsignature'] ?? '';
+            $this->connection->begin_transaction();
+
+            try {
+                $result = $this->connection->query($query);
+                if ($result instanceof \mysqli_result) {
+                    $row = $result->fetch_assoc();
+                    $hash = $row['dbsignature'] ?? '';
+                }
+            } finally {
+                $this->connection->rollback();
             }
         }
 

diff --git a/src/QueryReflection/MysqliQueryReflector.php b/src/QueryReflection/MysqliQueryReflector.php
@@ -56,6 +56,7 @@ public function __construct(mysqli $mysqli)
         $this->db->set_charset('utf8');
         // enable exception throwing on php <8.1
         mysqli_report(\MYSQLI_REPORT_ERROR | \MYSQLI_REPORT_STRICT);
+        $this->db->autocommit(false);
     }
 
     public function validateQueryString(string $queryString): ?Error
@@ -145,7 +146,11 @@ private function simulateQuery(string $queryString)
             return $this->cache[$queryString] = null;
         }
 
-        $this->db->begin_transaction(\MYSQLI_TRANS_START_READ_ONLY);
+        if (QueryReflection::getRuntimeConfiguration()->isAnalyzingWriteQueries()) {
+            $this->db->begin_transaction();
+        } else {
+            $this->db->begin_transaction(\MYSQLI_TRANS_START_READ_ONLY);
+        }
 
         try {
             $result = $this->db->query($simulatedQuery);

diff --git a/src/QueryReflection/PdoMysqlQueryReflector.php b/src/QueryReflection/PdoMysqlQueryReflector.php
@@ -48,24 +48,14 @@ protected function simulateQuery(string $queryString)
             return $this->cache[$queryString] = null;
         }
 
-        try {
-            $this->pdo->beginTransaction();
-        } catch (PDOException $e) {
-            // not all drivers may support transactions
-            throw new \RuntimeException('Failed to start transaction', $e->getCode(), $e);
-        }
+        $this->pdo->beginTransaction();
 
         try {
             $stmt = $this->pdo->query($simulatedQuery);
         } catch (PDOException $e) {
             return $this->cache[$queryString] = $e;
         } finally {
-            try {
-                $this->pdo->rollBack();
-            } catch (PDOException $e) {
-                // not all drivers may support transactions
-                throw new \RuntimeException('Failed to rollback transaction', $e->getCode(), $e);
-            }
+            $this->pdo->rollBack();
         }
 
         $this->cache[$queryString] = [];

diff --git a/src/QueryReflection/QueryReflection.php b/src/QueryReflection/QueryReflection.php
@@ -59,8 +59,20 @@ public static function setupReflector(QueryReflector $reflector, RuntimeConfigur
 
     public function validateQueryString(string $queryString): ?Error
     {
-        if ('SELECT' !== self::getQueryType($queryString)) {
-            return null;
+        if (self::getRuntimeConfiguration()->isAnalyzingWriteQueries()) {
+            if (!\in_array(self::getQueryType($queryString), [
+                'SELECT',
+                'INSERT',
+                'DELETE',
+                'UPDATE',
+                'REPLACE',
+            ], true)) {
+                return null;
+            }
+        } else {
+            if ('SELECT' !== self::getQueryType($queryString)) {
+                return null;
+            }
         }
 
         // this method cannot validate queries which contain placeholders.

diff --git a/src/QueryReflection/QuerySimulation.php b/src/QueryReflection/QuerySimulation.php
@@ -129,7 +129,11 @@ public static function simulate(string $queryString): ?string
         if (null === $queryString) {
             return null;
         }
-        $queryString .= ' LIMIT 0';
+
+        // make sure we don't unnecessarily transfer data, as we are only interested in the statement is succeeding
+        if ('SELECT' === QueryReflection::getQueryType($queryString)) {
+            $queryString .= ' LIMIT 0';
+        }
 
         return $queryString;
     }

diff --git a/src/QueryReflection/RuntimeConfiguration.php b/src/QueryReflection/RuntimeConfiguration.php
@@ -43,6 +43,10 @@ final class RuntimeConfiguration
      * @var bool
      */
     private $stringifyTypes = false;
+    /**
+     * @var bool
+     */
+    private $writableQueries = false;
     /**
      * @var bool|0|positive-int
      */
@@ -107,6 +111,21 @@ public function stringifyTypes(bool $stringify): self
         return $this;
     }
 
+    /**
+     * Enables checking of writable queries (INSERT, UPDATE, DELETE,...).
+     *
+     * This feature requires a database and a database driver which supports transactions.
+     * Otherwise, the analysis might lead to data loss!
+     *
+     * Also make sure your mysql tables use the InnoDB engine.
+     */
+    public function analyzeWriteQueries(bool $enabled): self
+    {
+        $this->writableQueries = $enabled;
+
+        return $this;
+    }
+
     /**
      * Enables query plan analysis, which indicates performance problems.
      *
@@ -154,6 +173,11 @@ public function isStringifyTypes(): bool
         return $this->stringifyTypes;
     }
 
+    public function isAnalyzingWriteQueries(): bool
+    {
+        return $this->writableQueries;
+    }
+
     /**
      * @return QueryReflector::FETCH_TYPE*
      */