Skip to content

Commit 73379cd

Browse files
authored
Improve Coverage for Gnumeric (#1517)
* Improve Coverage for Gnumeric I believe that both BaseReader and Gnumeric Reader are now 100% covered. My goal was to use PhpSpreadsheet to load the test file, save it as Xlsx, and visually compare the two, then add a test loaded with assertions. Results were generally pretty good, but there were no tests with assertions. I added a few cells to exercise some previously uncovered code. Code was extensively refactored; logic changes are noted below. Code allowed for specifying document properties in an old format. I considered removing that, but I found the original spec at http://www.jfree.org/jworkbook/download/gnumeric-xml.pdf This allowed me to create an old file, which was not handled correctly because of namespace differences. The code was corrected to allow for this difference. Added support for textRotation. Mapping of fill types was not correct. * PHP7.2 Error One assertion failed under PHP7.2. Apparently there was some change in the handling of SimpleXMLElement between 7.2 and 7.3. Casting to string before use eliminates the problem. * Scrutinizer Recommendations All minor, solved (hopefully) mostly by casts. * One Last Scrutinizer Fix ... I hope.
1 parent ce6ac1f commit 73379cd

File tree

9 files changed

+1050
-569
lines changed

9 files changed

+1050
-569
lines changed
241 Bytes
Binary file not shown.

samples/templates/old.gnumeric

1.25 KB
Binary file not shown.

src/PhpSpreadsheet/Reader/BaseReader.php

+13-10
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
namespace PhpOffice\PhpSpreadsheet\Reader;
44

5+
use PhpOffice\PhpSpreadsheet\Reader\Exception as ReaderException;
56
use PhpOffice\PhpSpreadsheet\Reader\Security\XmlScanner;
67
use PhpOffice\PhpSpreadsheet\Shared\File;
78

@@ -133,11 +134,7 @@ public function setReadFilter(IReadFilter $pValue)
133134

134135
public function getSecurityScanner()
135136
{
136-
if (property_exists($this, 'securityScanner')) {
137-
return $this->securityScanner;
138-
}
139-
140-
return null;
137+
return $this->securityScanner;
141138
}
142139

143140
/**
@@ -147,12 +144,18 @@ public function getSecurityScanner()
147144
*/
148145
protected function openFile($pFilename): void
149146
{
150-
File::assertFile($pFilename);
147+
if ($pFilename) {
148+
File::assertFile($pFilename);
151149

152-
// Open file
153-
$this->fileHandle = fopen($pFilename, 'rb');
154-
if ($this->fileHandle === false) {
155-
throw new Exception('Could not open file ' . $pFilename . ' for reading.');
150+
// Open file
151+
$fileHandle = fopen($pFilename, 'rb');
152+
} else {
153+
$fileHandle = false;
154+
}
155+
if ($fileHandle !== false) {
156+
$this->fileHandle = $fileHandle;
157+
} else {
158+
throw new ReaderException('Could not open file ' . $pFilename . ' for reading.');
156159
}
157160
}
158161
}

0 commit comments

Comments
 (0)