Skip to content
Merged
2 changes: 1 addition & 1 deletion .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
48 changes: 48 additions & 0 deletions .github/workflows/static-analysis.yaml
Original file line number Diff line number Diff line change
@@ -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
14 changes: 8 additions & 6 deletions Spreadsheet/Excel/Writer/Parser.php
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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");
}

/**
Expand Down
4 changes: 2 additions & 2 deletions Spreadsheet/Excel/Writer/Worksheet.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -3679,4 +3679,4 @@ protected function _storeDataValidity()
$this->_append($header . $dv);
}
}
}
}
8 changes: 6 additions & 2 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand All @@ -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
}
}
8 changes: 8 additions & 0 deletions phpstan.neon
Original file line number Diff line number Diff line change
@@ -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
37 changes: 17 additions & 20 deletions test/Test/Spreadsheet/Excel/Writer/ParserTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link

Copilot AI Jun 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Directly accessing the protected property _functions will cause a visibility error. Use ReflectionProperty to set accessibility before modifying this value.

Suggested change
$parser->_functions['INVALID'] = array(999, -2, 0, 0); // -2 is not valid
$functionsProperty = new \ReflectionProperty($parser, '_functions');
$functionsProperty->setAccessible(true);
$functions = $functionsProperty->getValue($parser);
$functions['INVALID'] = array(999, -2, 0, 0); // -2 is not valid
$functionsProperty->setValue($parser, $functions);

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Jun 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Directly assigning to _functions may fail if the property is protected. Use ReflectionProperty to make _functions accessible or provide a setter method in Parser for testing.

Copilot uses AI. Check for mistakes.

$this->expectException(\InvalidArgumentException::class);
$this->expectExceptionMessage('Invalid argument count -2 for function INVALID');
$method->invoke($parser, 'INVALID', 1);
}

/**
Expand All @@ -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']);
}
}
3 changes: 3 additions & 0 deletions test/Test/Spreadsheet/Excel/Writer/WorksheetTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
}