Add PHPStan static analysis at level 0, minimal fixes (#42)

Author: sanmai

.github/workflows/ci.yaml

@@ -59,7 +59,7 @@ jobs:
 
       - name: Install dependencies
         run: |
-          composer remove --no-update --dev friendsofphp/php-cs-fixer
+          composer remove --no-update --dev friendsofphp/php-cs-fixer phpstan/phpstan
           composer install --no-interaction --prefer-dist --no-progress
 
       - name: Lint code

.github/workflows/static-analysis.yaml

@@ -0,0 +1,48 @@
+name: Static Analysis
+
+on:
+  pull_request:
+  push:
+    branches:
+      - main
+      - master
+
+jobs:
+  phpstan:
+    name: PHPStan
+    runs-on: ubuntu-latest
+
+    strategy:
+      fail-fast: false
+      matrix:
+        php-version:
+          - '8.2'
+
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v4
+
+      - name: Setup PHP
+        uses: shivammathur/setup-php@v2
+        with:
+          php-version: ${{ matrix.php-version }}
+          coverage: none
+
+      - name: Get composer cache directory
+        id: composer-cache
+        run: echo "dir=$(composer config cache-files-dir)" >> $GITHUB_ENV
+
+      - name: Cache dependencies
+        uses: actions/cache@v4
+        with:
+          path: ${{ env.dir }}
+          key: composer-${{ runner.os }}-${{ matrix.php-version }}-${{ hashFiles('**/composer.json') }}
+          restore-keys: |
+            composer-${{ runner.os }}-${{ matrix.php-version }}-
+            composer-${{ runner.os }}-
+
+      - name: Install dependencies
+        run: composer install --prefer-dist --no-progress --no-suggest
+
+      - name: Run PHPStan
+        run: vendor/bin/phpstan analyse

Spreadsheet/Excel/Writer/Parser.php

@@ -249,12 +249,12 @@ protected function _initializeHashes()
             'ptgRefN'      => 0x2C,
             'ptgAreaN'     => 0x2D,
             'ptgMemAreaN'  => 0x2E,
-            'ptgMemNoMemN' => 0x2F,
+            // 'ptgMemNoMemN' => 0x2F, // Overwritten by 0x6F
             'ptgNameX'     => 0x39,
             'ptgRef3d'     => 0x3A,
             'ptgArea3d'    => 0x3B,
             'ptgRefErr3d'  => 0x3C,
-            'ptgAreaErr3d' => 0x3D,
+            // 'ptgAreaErr3d' => 0x3D, // Overwritten by 0x7D
             'ptgArrayV'    => 0x40,
             'ptgFuncV'     => 0x41,
             'ptgFuncVarV'  => 0x42,
@@ -263,20 +263,20 @@ protected function _initializeHashes()
             'ptgAreaV'     => 0x45,
             'ptgMemAreaV'  => 0x46,
             'ptgMemErrV'   => 0x47,
-            'ptgMemNoMemV' => 0x48,
+            // 'ptgMemNoMemV' => 0x48, // Duplicate of 0x28 (ptgMemNoMem) in Excel spec
             'ptgMemFuncV'  => 0x49,
             'ptgRefErrV'   => 0x4A,
             'ptgAreaErrV'  => 0x4B,
             'ptgRefNV'     => 0x4C,
             'ptgAreaNV'    => 0x4D,
             'ptgMemAreaNV' => 0x4E,
-            'ptgMemNoMemN' => 0x4F,
+            // 'ptgMemNoMemN' => 0x4F, // Overwritten by 0x6F
             'ptgFuncCEV'   => 0x58,
             'ptgNameXV'    => 0x59,
             'ptgRef3dV'    => 0x5A,
             'ptgArea3dV'   => 0x5B,
             'ptgRefErr3dV' => 0x5C,
-            'ptgAreaErr3d' => 0x5D,
+            // 'ptgAreaErr3d' => 0x5D, // Duplicate of 0x3D
             'ptgArrayA'    => 0x60,
             'ptgFuncA'     => 0x61,
             'ptgFuncVarA'  => 0x62,
@@ -285,7 +285,7 @@ protected function _initializeHashes()
             'ptgAreaA'     => 0x65,
             'ptgMemAreaA'  => 0x66,
             'ptgMemErrA'   => 0x67,
-            'ptgMemNoMemA' => 0x68,
+            // 'ptgMemNoMemA' => 0x68, // Duplicate of 0x28 (ptgMemNoMem) in Excel spec
             'ptgMemFuncA'  => 0x69,
             'ptgRefErrA'   => 0x6A,
             'ptgAreaErrA'  => 0x6B,
@@ -671,6 +671,8 @@ protected function _convertFunction($token, $num_args)
         if ($args == -1) {
             return pack("CCv", $this->ptg['ptgFuncVarV'], $num_args, $this->_functions[$token][0]);
         }
+
+        throw new \InvalidArgumentException("Invalid argument count $args for function $token");
     }
 
     /**

Spreadsheet/Excel/Writer/Worksheet.php

@@ -754,7 +754,7 @@ public function getData($cleanup = true)
         // Return data stored in memory
         if (isset($this->_data)) {
             $tmp   = $this->_data;
-            unset($this->_data);
+            $this->_data = null;
             if ($this->_using_tmpfile) {
                 fseek($this->_filehandle, 0);
             }
@@ -3679,4 +3679,4 @@ protected function _storeDataValidity()
             $this->_append($header . $dv);
         }
     }
-}
+}

composer.json

@@ -38,9 +38,10 @@
     "require-dev": {
         "friendsofphp/php-cs-fixer": "^3.64",
         "php-coveralls/php-coveralls": "^2.2",
+        "php-parallel-lint/php-parallel-lint": "^1.4",
+        "phpstan/phpstan": "^2.1",
         "phpunit/phpunit": ">=5 <10",
-        "sanmai/phpunit-legacy-adapter": "^6 || ^8",
-        "php-parallel-lint/php-parallel-lint": "^1.4"
+        "sanmai/phpunit-legacy-adapter": "^6 || ^8"
     },
     "autoload": {
         "psr-0": {
@@ -60,5 +61,8 @@
     "support": {
         "issues": "http://pear.php.net/bugs/search.php?cmd=display&package_name[]=Spreadsheet_Excel_Writer",
         "source": "https://github.com/pear/Spreadsheet_Excel_Writer"
+    },
+    "config": {
+        "sort-packages": true
     }
 }

phpstan.neon

@@ -0,0 +1,8 @@
+parameters:
+    level: 0
+    paths:
+        - Spreadsheet
+    ignoreErrors:
+        -
+            message: '#Method .*_substituteCellref.* should return array but return statement is missing#'
+            path: Spreadsheet/Excel/Writer/Worksheet.php

test/Test/Spreadsheet/Excel/Writer/ParserTest.php

@@ -15,20 +15,25 @@ public function testConvertFunctionReturnsValue()
     {
         $parser = new Spreadsheet_Excel_Writer_Parser(0, 0x0500);
 
-        // Access protected method via reflection
         $method = new \ReflectionMethod($parser, '_convertFunction');
         $method->setAccessible(true);
 
-        // Test with a function that has fixed args (should return early)
-        // TIME has 3 fixed arguments
+        // Fixed args (TIME=3) should return without issue
         $result = $method->invoke($parser, 'TIME', 3);
         $this->assertNotEmpty($result);
         $this->assertTrue(is_string($result));
 
-        // Test variable args path - SUM has variable args
+        // Variable args (SUM=-1) should return without issue
         $result = $method->invoke($parser, 'SUM', 2);
         $this->assertNotEmpty($result);
         $this->assertTrue(is_string($result));
+
+        // Array structure: [function_number, arg_count, unknown, volatile_flag]
+        $parser->_functions['INVALID'] = array(999, -2, 0, 0); // -2 is not valid
+
+        $this->expectException(\InvalidArgumentException::class);
+        $this->expectExceptionMessage('Invalid argument count -2 for function INVALID');
+        $method->invoke($parser, 'INVALID', 1);
     }
 
     /**
@@ -46,27 +51,19 @@ public function testDuplicatePtgValues()
     {
         $parser = new Spreadsheet_Excel_Writer_Parser(0, 0x0500);
 
-        // Access protected property via reflection
         $property = new \ReflectionProperty($parser, 'ptg');
         $property->setAccessible(true);
         $ptg = $property->getValue($parser);
 
-        // Test ptgMemNoMemN - should have the LAST duplicate value
-        // Original duplicates: 0x2F (commented), 0x4F (commented), 0x6F (active)
-        $this->assertArrayHasKey('ptgMemNoMemN', $ptg,
-            'ptgMemNoMemN key should exist in ptg array');
-        $this->assertSame(0x6F, $ptg['ptgMemNoMemN'],
-            'ptgMemNoMemN should be 0x6F (the last duplicate), not 0x2F or 0x4F');
+        // ptgMemNoMemN: last duplicate at 0x6F wins (0x2F, 0x4F were overwritten)
+        $this->assertArrayHasKey('ptgMemNoMemN', $ptg);
+        $this->assertSame(0x6F, $ptg['ptgMemNoMemN']);
 
-        // Test ptgAreaErr3d - should have the LAST duplicate value
-        // Original duplicates: 0x3D (commented), 0x5D (commented), 0x7D (active)
-        $this->assertArrayHasKey('ptgAreaErr3d', $ptg,
-            'ptgAreaErr3d key should exist in ptg array');
-        $this->assertSame(0x7D, $ptg['ptgAreaErr3d'],
-            'ptgAreaErr3d should be 0x7D (the last duplicate), not 0x3D or 0x5D');
+        // ptgAreaErr3d: last duplicate at 0x7D wins (0x3D, 0x5D were overwritten)
+        $this->assertArrayHasKey('ptgAreaErr3d', $ptg);
+        $this->assertSame(0x7D, $ptg['ptgAreaErr3d']);
 
-        // Verify that ptgMemNoMem exists with value 0x28
-        // (The duplicates at 0x48 and 0x68 are commented out per Excel spec)
-        $this->assertSame(0x28, $ptg['ptgMemNoMem'], 'ptgMemNoMem should be 0x28');
+        // ptgMemNoMem base variant at 0x28 (0x48, 0x68 duplicates removed per Excel spec)
+        $this->assertSame(0x28, $ptg['ptgMemNoMem']);
     }
 }

test/Test/Spreadsheet/Excel/Writer/WorksheetTest.php

@@ -97,5 +97,8 @@ public function testGetDataClearsDataProperty()
 
         // Check that data was returned
         $this->assertEquals($testData, $result);
+
+        // Check that _data is now null (not unset)
+        $this->assertNull($property->getValue($this->worksheet));
     }
 }