THRIFT-5957: Add phpstan static analysis with CI guardrail

Client: php

The PHP runtime library at lib/php/lib/ had no static-analysis tooling
in CI. Refactors and type-modernization PRs went in without an
automated safety net.

This change:

  * Adds phpstan/phpstan ^1.12 as a require-dev dependency, plus a
    "scripts.phpstan" composer alias for local use.
  * Adds lib/php/phpstan.neon (level 1) targeting lib/php/lib/ with
    test/bootstrap.php for autoload, and a generated baseline that
    pins the 5 remaining issues (all "should return X but return
    statement is missing" in TJSONProtocol/TSimpleJSONProtocol's
    writeStructBegin/writeStructEnd and ThriftClassLoader::findFile —
    left for a follow-up ticket).
  * Adds a "Run phpstan" step alongside the existing per-language
    steps (cppcheck, flake8, phpcs, rubocop) in the sca.yml workflow,
    and wires its outcome into the aggregate "Fail if any SCA check
    failed" gate. Runs with --error-format=github so findings show up
    as inline annotations on PRs.
  * Bumps the sca job's setup-php from 7.1 to 8.1 to match the new
    project floor (THRIFT-5956) and to satisfy phpstan 1.x's PHP
    >= 7.2 requirement.

Also fixes the trivial subset of phpstan findings already discovered
during baseline generation:

  * TSocketPool::open: replace extract($this->servers_[$i]) with
    explicit $host / $port assignments. Removes 10 "might not be
    defined" findings and avoids extract()'s well-known footgun of
    silently rebinding existing locals.
  * TBufferedTransport::readAll: change the final "elseif" to "else"
    so phpstan can prove $data is always assigned.

Level 1 catches undefined variables and obvious type mismatches
without requiring widespread code changes. Higher levels will be
raised in follow-up tickets as the runtime library gains native
types and PHPDoc cleanup.

Out of scope (separate tickets):
  * PSR-12 migration / php-cs-fixer.
  * declare(strict_types=1) and native types in lib.
  * Static analysis of generated PHP under test/Resources/packages/.
  * Fixing the remaining 5 missing-return findings.
  * CHANGES.md is auto-generated on release; no manual entry here.

Generated-by: Claude Opus 4.7 (1M context)

Author: sveneld

.github/workflows/sca.yml

@@ -177,6 +177,13 @@ jobs:
           composer install --quiet
           ./vendor/bin/phpcs
 
+      - name: Run phpstan
+        id: phpstan
+        continue-on-error: true
+        run: |
+          # PHP static analysis
+          ./vendor/bin/phpstan analyse -c lib/php/phpstan.neon --no-progress --error-format=github
+
       - name: Run rubocop
         id: rubocop
         continue-on-error: true
@@ -209,6 +216,7 @@ jobs:
           CPPCHECK_OUTCOME: ${{ steps.cppcheck.outcome }}
           FLAKE8_OUTCOME: ${{ steps.flake8.outcome }}
           PHPCS_OUTCOME: ${{ steps.phpcs.outcome }}
+          PHPSTAN_OUTCOME: ${{ steps.phpstan.outcome }}
           RUBOCOP_OUTCOME: ${{ steps.rubocop.outcome }}
         run: |
           failed=0
@@ -225,6 +233,10 @@ jobs:
             echo "::error::Step 'phpcs' failed (outcome: $PHPCS_OUTCOME)"
             failed=1
           fi
+          if [ "$PHPSTAN_OUTCOME" != "success" ]; then
+            echo "::error::Step 'phpstan' failed (outcome: $PHPSTAN_OUTCOME)"
+            failed=1
+          fi
           if [ "$RUBOCOP_OUTCOME" != "success" ]; then
             echo "::error::Step 'rubocop' failed (outcome: $RUBOCOP_OUTCOME)"
             failed=1

composer.json

@@ -24,11 +24,15 @@
         "phpunit/phpunit": "^9.5",
         "squizlabs/php_codesniffer": "^3.10",
         "php-mock/php-mock-phpunit": "^2.10",
+        "phpstan/phpstan": "^1.12",
         "ext-json": "*",
         "ext-xml": "*",
         "ext-curl": "*",
         "ext-pcntl": "*"
     },
+    "scripts": {
+        "phpstan": "phpstan analyse -c lib/php/phpstan.neon"
+    },
     "autoload": {
         "psr-4": {"Thrift\\": "lib/php/lib/"}
     },

lib/php/lib/Transport/TBufferedTransport.php

@@ -132,7 +132,7 @@ public function readAll($len)
         } elseif ($have == $len) {
             $data = $this->rBuf_;
             $this->rBuf_ = '';
-        } elseif ($have > $len) {
+        } else {
             $data = TStringFuncFactory::create()->substr($this->rBuf_, 0, $len);
             $this->rBuf_ = TStringFuncFactory::create()->substr($this->rBuf_, $len);
         }

lib/php/lib/Transport/TSocketPool.php

@@ -192,8 +192,8 @@ public function open()
         $numServers = count($this->servers_);
 
         for ($i = 0; $i < $numServers; ++$i) {
-            // This extracts the $host and $port variables
-            extract($this->servers_[$i]);
+            $host = $this->servers_[$i]['host'];
+            $port = $this->servers_[$i]['port'];
 
             // Check APCu cache for a record of this server being down
             $failtimeKey = 'thrift_failtime:' . $host . ':' . $port . '~';

lib/php/phpstan-baseline.neon

@@ -0,0 +1,26 @@
+parameters:
+	ignoreErrors:
+		-
+			message: "#^Method Thrift\\\\ClassLoader\\\\ThriftClassLoader\\:\\:findFile\\(\\) should return string but return statement is missing\\.$#"
+			count: 1
+			path: lib/ClassLoader/ThriftClassLoader.php
+
+		-
+			message: "#^Method Thrift\\\\Protocol\\\\TJSONProtocol\\:\\:writeStructBegin\\(\\) should return int but return statement is missing\\.$#"
+			count: 1
+			path: lib/Protocol/TJSONProtocol.php
+
+		-
+			message: "#^Method Thrift\\\\Protocol\\\\TJSONProtocol\\:\\:writeStructEnd\\(\\) should return int but return statement is missing\\.$#"
+			count: 1
+			path: lib/Protocol/TJSONProtocol.php
+
+		-
+			message: "#^Method Thrift\\\\Protocol\\\\TSimpleJSONProtocol\\:\\:writeStructBegin\\(\\) should return int but return statement is missing\\.$#"
+			count: 1
+			path: lib/Protocol/TSimpleJSONProtocol.php
+
+		-
+			message: "#^Method Thrift\\\\Protocol\\\\TSimpleJSONProtocol\\:\\:writeStructEnd\\(\\) should return int but return statement is missing\\.$#"
+			count: 1
+			path: lib/Protocol/TSimpleJSONProtocol.php

lib/php/phpstan.neon

@@ -0,0 +1,11 @@
+parameters:
+    level: 1
+    paths:
+        - lib
+    bootstrapFiles:
+        - test/bootstrap.php
+    treatPhpDocTypesAsCertain: false
+    excludePaths:
+        - test/Resources/packages/*
+includes:
+    - phpstan-baseline.neon