Add PHPStan static analysis, minimal fixes

- Install PHPStan at level 0
- Fix few issues that PHPStan found:
  - Add missing return statement in Parser::_convertFunction()
  - Fix regex patterns by properly escaping $ characters
  - Replace unset() with null assignment for PHP 8.4 compatibility
  - Fix duplicate array keys in ptg definitions
- Add unit tests to verify the fixes
- Add dedicated static analysis workflow using PHP 8.2

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,37 @@
+name: Static Analysis
+
+on:
+  pull_request:
+  push:
+    branches:
+      - main
+      - master
+
+jobs:
+  phpstan:
+    name: PHPStan
+    runs-on: ubuntu-latest
+
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v4
+
+      - name: Setup PHP
+        uses: shivammathur/setup-php@v2
+        with:
+          php-version: 8.2
+          coverage: none
+
+      - name: Cache Composer dependencies
+        uses: actions/cache@v4
+        with:
+          path: ~/.composer/cache
+          key: ${{ runner.os }}-composer-${{ hashFiles('**/composer.lock') }}
+          restore-keys: |
+            ${{ runner.os }}-composer-
+
+      - 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

@@ -270,13 +270,13 @@ protected function _initializeHashes()
             'ptgRefNV'     => 0x4C,
             'ptgAreaNV'    => 0x4D,
             'ptgMemAreaNV' => 0x4E,
-            'ptgMemNoMemN' => 0x4F,
+            'ptgMemNoMemNV' => 0x4F,
             'ptgFuncCEV'   => 0x58,
             'ptgNameXV'    => 0x59,
             'ptgRef3dV'    => 0x5A,
             'ptgArea3dV'   => 0x5B,
             'ptgRefErr3dV' => 0x5C,
-            'ptgAreaErr3d' => 0x5D,
+            'ptgAreaErr3dV' => 0x5D,
             'ptgArrayA'    => 0x60,
             'ptgFuncA'     => 0x61,
             'ptgFuncVarA'  => 0x62,
@@ -292,13 +292,13 @@ protected function _initializeHashes()
             'ptgRefNA'     => 0x6C,
             'ptgAreaNA'    => 0x6D,
             'ptgMemAreaNA' => 0x6E,
-            'ptgMemNoMemN' => 0x6F,
+            'ptgMemNoMemNA' => 0x6F,
             'ptgFuncCEA'   => 0x78,
             'ptgNameXA'    => 0x79,
             'ptgRef3dA'    => 0x7A,
             'ptgArea3dA'   => 0x7B,
             'ptgRefErr3dA' => 0x7C,
-            'ptgAreaErr3d' => 0x7D
+            'ptgAreaErr3dA' => 0x7D
             );
 
         // Thanks to Michael Meeks and Gnumeric for the initial arg values.
@@ -671,6 +671,9 @@ protected function _convertFunction($token, $num_args)
         if ($args == -1) {
             return pack("CCv", $this->ptg['ptgFuncVarV'], $num_args, $this->_functions[$token][0]);
         }
+        
+        // Default return for safety
+        return pack("Cv", $this->ptg['ptgFuncV'], $this->_functions[$token][0]);
     }
 
     /**

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; // Set to null instead of unset for PHP 8.4 compatibility
             if ($this->_using_tmpfile) {
                 fseek($this->_filehandle, 0);
             }
@@ -1455,20 +1455,21 @@ protected function _substituteCellref($cell)
         }
 
         // Convert a cell range: 'A1:B7'
-        if (preg_match("/\$?([A-I]?[A-Z]\$?\d+):\$?([A-I]?[A-Z]\$?\d+)/", $cell, $match)) {
+        if (preg_match("/\\$?([A-I]?[A-Z]\\$?\\d+):\\$?([A-I]?[A-Z]\\$?\\d+)/", $cell, $match)) {
             list($row1, $col1) =  $this->_cellToRowcol($match[1]);
             list($row2, $col2) =  $this->_cellToRowcol($match[2]);
             return (array($row1, $col1, $row2, $col2));
         }
 
         // Convert a cell reference: 'A1' or 'AD2000'
-        if (preg_match("/\$?([A-I]?[A-Z]\$?\d+)/", $cell)) {
+        if (preg_match("/\\$?([A-I]?[A-Z]\\$?\\d+)/", $cell, $match)) {
             list($row1, $col1) =  $this->_cellToRowcol($match[1]);
             return (array($row1, $col1));
         }
 
         // TODO use real error codes
         $this->raiseError("Unknown cell reference $cell", 0, PEAR_ERROR_DIE);
+        return array(0, 0); // Return default to satisfy static analysis
     }
 
     /**
@@ -1481,7 +1482,7 @@ protected function _substituteCellref($cell)
     */
     protected function _cellToRowcol($cell)
     {
-        preg_match("/\$?([A-I]?[A-Z])\$?(\d+)/",$cell,$match);
+        preg_match("/\\$?([A-I]?[A-Z])\\$?(\\d+)/",$cell,$match);
         $col     = $match[1];
         $row     = $match[2];
 

composer.json

@@ -40,7 +40,8 @@
         "php-coveralls/php-coveralls": "^2.2",
         "phpunit/phpunit": ">=5 <10",
         "sanmai/phpunit-legacy-adapter": "^6 || ^8",
-        "php-parallel-lint/php-parallel-lint": "^1.4"
+        "php-parallel-lint/php-parallel-lint": "^1.4",
+        "phpstan/phpstan": "^2.1"
     },
     "autoload": {
         "psr-0": {

phpstan.neon

@@ -0,0 +1,5 @@
+parameters:
+    level: 0
+    paths:
+        - Spreadsheet
+        - test

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

@@ -0,0 +1,34 @@
+<?php
+
+namespace Test\Spreadsheet\Excel\Writer;
+
+use Spreadsheet_Excel_Writer_Parser;
+
+class ParserTest extends \LegacyPHPUnit\TestCase
+{
+    /**
+     * Test that _convertFunction returns a value for all code paths
+     */
+    public function testConvertFunctionReturnsValue()
+    {
+        $parser = new Spreadsheet_Excel_Writer_Parser(0, 0x0500);
+        
+        // Initialize the parser properly
+        $parser->_initializeHashes();
+        
+        // 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
+        $result = $method->invoke($parser, 'TIME', 3);
+        $this->assertNotEmpty($result);
+        $this->assertInternalType('string', $result);
+        
+        // Test variable args path - SUM has variable args
+        $result = $method->invoke($parser, 'SUM', 2);
+        $this->assertNotEmpty($result);
+        $this->assertInternalType('string', $result);
+    }
+}

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

@@ -0,0 +1,95 @@
+<?php
+
+namespace Test\Spreadsheet\Excel\Writer;
+
+use Spreadsheet_Excel_Writer_Worksheet;
+use Spreadsheet_Excel_Writer_Workbook;
+
+class WorksheetTest extends \LegacyPHPUnit\TestCase
+{
+    private $workbook;
+    private $worksheet;
+    
+    public function setUp()
+    {
+        parent::setUp();
+        $this->workbook = new Spreadsheet_Excel_Writer_Workbook('php://memory');
+        $this->worksheet = new Spreadsheet_Excel_Writer_Worksheet(0x0500, 'Test', 0, 0, $this->workbook->_url_format);
+    }
+    
+    public function tearDown()
+    {
+        if ($this->workbook) {
+            $this->workbook->close();
+        }
+        parent::tearDown();
+    }
+    
+    /**
+     * Test that _substituteCellref handles regex with dollar signs correctly
+     */
+    public function testSubstituteCellrefWithDollarSigns()
+    {
+        $method = new \ReflectionMethod($this->worksheet, '_substituteCellref');
+        $method->setAccessible(true);
+        
+        // Test absolute cell reference
+        $result = $method->invoke($this->worksheet, '$A$1');
+        $this->assertEquals(array(0, 0), $result);
+        
+        // Test mixed references
+        $result = $method->invoke($this->worksheet, 'A$1');
+        $this->assertEquals(array(0, 0), $result);
+        
+        $result = $method->invoke($this->worksheet, '$A1');
+        $this->assertEquals(array(0, 0), $result);
+        
+        // Test cell range with dollar signs
+        $result = $method->invoke($this->worksheet, '$A$1:$B$2');
+        $this->assertEquals(array(0, 0, 1, 1), $result);
+    }
+    
+    /**
+     * Test that _cellToRowcol handles regex with dollar signs correctly
+     */
+    public function testCellToRowcolWithDollarSigns()
+    {
+        $method = new \ReflectionMethod($this->worksheet, '_cellToRowcol');
+        $method->setAccessible(true);
+        
+        // Test with absolute reference
+        $result = $method->invoke($this->worksheet, '$A$1');
+        $this->assertEquals(array(0, 0), $result);
+        
+        // Test with relative reference
+        $result = $method->invoke($this->worksheet, 'B2');
+        $this->assertEquals(array(1, 1), $result);
+        
+        // Test with mixed reference
+        $result = $method->invoke($this->worksheet, '$C3');
+        $this->assertEquals(array(2, 2), $result);
+    }
+    
+    /**
+     * Test that getData properly handles clearing _data property
+     */
+    public function testGetDataClearsDataProperty()
+    {
+        // Access protected property
+        $property = new \ReflectionProperty($this->worksheet, '_data');
+        $property->setAccessible(true);
+        
+        // Set some data
+        $testData = 'test data';
+        $property->setValue($this->worksheet, $testData);
+        
+        // Call getData
+        $result = $this->worksheet->getData();
+        
+        // Check that data was returned
+        $this->assertEquals($testData, $result);
+        
+        // Check that _data is now null (not unset)
+        $this->assertNull($property->getValue($this->worksheet));
+    }
+}