diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index ce89373..f606c67 100644 --- a/.github/workflows/ci.yaml +++ b/.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 diff --git a/.github/workflows/static-analysis.yaml b/.github/workflows/static-analysis.yaml new file mode 100644 index 0000000..ed0e3f4 --- /dev/null +++ b/.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 diff --git a/Spreadsheet/Excel/Writer/Parser.php b/Spreadsheet/Excel/Writer/Parser.php index 9cac064..d7b1833 100644 --- a/Spreadsheet/Excel/Writer/Parser.php +++ b/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"); } /** diff --git a/Spreadsheet/Excel/Writer/Worksheet.php b/Spreadsheet/Excel/Writer/Worksheet.php index 299e399..840d715 100644 --- a/Spreadsheet/Excel/Writer/Worksheet.php +++ b/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); } } -} \ No newline at end of file +} diff --git a/composer.json b/composer.json index 19a495b..1d7a77a 100644 --- a/composer.json +++ b/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 } } diff --git a/phpstan.neon b/phpstan.neon new file mode 100644 index 0000000..2aa9b95 --- /dev/null +++ b/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 diff --git a/test/Test/Spreadsheet/Excel/Writer/ParserTest.php b/test/Test/Spreadsheet/Excel/Writer/ParserTest.php index d9030c7..9bf390a 100644 --- a/test/Test/Spreadsheet/Excel/Writer/ParserTest.php +++ b/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']); } } diff --git a/test/Test/Spreadsheet/Excel/Writer/WorksheetTest.php b/test/Test/Spreadsheet/Excel/Writer/WorksheetTest.php index 4c738b0..d9276bd 100644 --- a/test/Test/Spreadsheet/Excel/Writer/WorksheetTest.php +++ b/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)); } }