Skip to content

Commit

Permalink
Xlsx Reader Better Namespace Handling Phase 1
Browse files Browse the repository at this point in the history 
There have been a number of issues concerning the handling of legitimate but unexpected namespace prefixes in Xlsx spreadsheets created by software other than Excel and PhpSpreadsheet/PhpExcel.I have studied them, but, till now, have not had a good idea on how to act on them. A recent comment PHPOffice#860 (comment) in issue PHPOffice#860 by @IMSoP has triggered an idea about how to proceed.

Gnumeric Reader was recently changed to handle namespaces better. Using that as a model, this PR begins the process of doing the same for Xlsx. Xlsx is much larger and more complicated than Gnumeric, hence the need to tackle it in multiple phases. I believe that this PR handles all of:
- listWorkSheetNames
- listWorkSheetInfo. Note that there was a bug in this function which would cause it to count only used columns rather than all columns. That bug is corrected.
- active sheet
- selected cell and top left cell
- cell content (formulas, numbers, text)
- hyperlinks
- comments (partial - see below)

This PR does not address:
- styles
- images and charts
- VBA and ribbons
- many other items, I'm sure

The issue for non-standard namespacing till now has been the use of unexpected prefixes. While I was working on this change, @Lambik introduced issue PHPOffice#2067 PR PHPOffice#2068 which introduced a completely different problem - the use of unexpected URLs. That PR and the issue associated with it were quite well documented, including the supplying of a test file and tests for it. I asked if I could take a look to see if it could be integrated with my change, and the result seems to be yes, so those changes are also part of this PR.

While adding a comment to my test file, I discovered that Microsoft had added "threaded comments" as a new feature. I believe these are not yet supported by PhpSpreadsheet, and I am not going to add it, at least not now. I believe that, among other things, this will make identifying the author of a comment more difficult.

Although there are a number of Phpstan baseline changes as part of this PR, I did not attempt to resolve all Phpstan reports for Reader/Xlsx. Nor did I do anything to increase coverage. This change is already large and complex enough without those efforts.

I will add more detail as comments after I push this change.
  • Loading branch information
@oleibman
oleibman committed May 11, 2021
1 parent b05dc31 commit 5687a53
Show file tree
Hide file tree
Showing 26 changed files with 1,018 additions and 681 deletions.
215 changes: 0 additions & 215 deletions phpstan-baseline.neon
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3735,21 +3735,6 @@ parameters:
count: 1
path: src/PhpSpreadsheet/Reader/Xls/Style/FillPattern.php

-
message: "#^Cannot access property \\$Relationship on SimpleXMLElement\\|false\\.$#"
count: 12
path: src/PhpSpreadsheet/Reader/Xlsx.php

-
message: "#^Cannot access property \\$sheets on SimpleXMLElement\\|false\\.$#"
count: 6
path: src/PhpSpreadsheet/Reader/Xlsx.php

-
message: "#^Cannot call method registerXPathNamespace\\(\\) on SimpleXMLElement\\|false\\.$#"
count: 4
path: src/PhpSpreadsheet/Reader/Xlsx.php

-
message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Reader\\\\Xlsx\\:\\:castToBoolean\\(\\) has no return typehint specified\\.$#"
count: 1
Expand Down Expand Up @@ -3825,81 +3810,16 @@ parameters:
count: 2
path: src/PhpSpreadsheet/Reader/Xlsx.php

-
message: "#^Cannot call method xpath\\(\\) on SimpleXMLElement\\|false\\.$#"
count: 4
path: src/PhpSpreadsheet/Reader/Xlsx.php

-
message: "#^Parameter \\#1 \\$is of method PhpOffice\\\\PhpSpreadsheet\\\\Reader\\\\Xlsx\\:\\:parseRichText\\(\\) expects SimpleXMLElement\\|null, object given\\.$#"
count: 1
path: src/PhpSpreadsheet/Reader/Xlsx.php

-
message: "#^Parameter \\#1 \\$styleXml of class PhpOffice\\\\PhpSpreadsheet\\\\Reader\\\\Xlsx\\\\Styles constructor expects SimpleXMLElement, SimpleXMLElement\\|false given\\.$#"
count: 1
path: src/PhpSpreadsheet/Reader/Xlsx.php

-
message: "#^Cannot access property \\$workbookPr on SimpleXMLElement\\|false\\.$#"
count: 2
path: src/PhpSpreadsheet/Reader/Xlsx.php

-
message: "#^Parameter \\#2 \\$xmlWorkbook of method PhpOffice\\\\PhpSpreadsheet\\\\Reader\\\\Xlsx\\:\\:readProtection\\(\\) expects SimpleXMLElement, SimpleXMLElement\\|false given\\.$#"
count: 1
path: src/PhpSpreadsheet/Reader/Xlsx.php

-
message: "#^Parameter \\#1 \\$relsWorksheet of method PhpOffice\\\\PhpSpreadsheet\\\\Reader\\\\Xlsx\\\\Hyperlinks\\:\\:readHyperlinks\\(\\) expects SimpleXMLElement, SimpleXMLElement\\|false given\\.$#"
count: 1
path: src/PhpSpreadsheet/Reader/Xlsx.php

-
message: "#^Cannot access property \\$authors on SimpleXMLElement\\|false\\.$#"
count: 1
path: src/PhpSpreadsheet/Reader/Xlsx.php

-
message: "#^Cannot access property \\$commentList on SimpleXMLElement\\|false\\.$#"
count: 1
path: src/PhpSpreadsheet/Reader/Xlsx.php

-
message: "#^Argument of an invalid type array\\<SimpleXMLElement\\>\\|false supplied for foreach, only iterables are supported\\.$#"
count: 2
path: src/PhpSpreadsheet/Reader/Xlsx.php

-
message: "#^Negated boolean expression is always true\\.$#"
count: 1
path: src/PhpSpreadsheet/Reader/Xlsx.php

-
message: "#^Cannot access property \\$drawing on SimpleXMLElement\\|false\\.$#"
count: 2
path: src/PhpSpreadsheet/Reader/Xlsx.php

-
message: "#^Cannot call method children\\(\\) on SimpleXMLElement\\|false\\.$#"
count: 2
path: src/PhpSpreadsheet/Reader/Xlsx.php

-
message: "#^Cannot call method count\\(\\) on SimpleXMLElement\\|false\\.$#"
count: 1
path: src/PhpSpreadsheet/Reader/Xlsx.php

-
message: "#^Cannot call method asXML\\(\\) on SimpleXMLElement\\|false\\.$#"
count: 1
path: src/PhpSpreadsheet/Reader/Xlsx.php

-
message: "#^Cannot access property \\$definedNames on SimpleXMLElement\\|false\\.$#"
count: 4
path: src/PhpSpreadsheet/Reader/Xlsx.php

-
message: "#^Argument of an invalid type array\\<int, string\\>\\|false supplied for foreach, only iterables are supported\\.$#"
count: 1
Expand All @@ -3915,26 +3835,6 @@ parameters:
count: 1
path: src/PhpSpreadsheet/Reader/Xlsx.php

-
message: "#^Cannot access property \\$bookViews on SimpleXMLElement\\|false\\.$#"
count: 1
path: src/PhpSpreadsheet/Reader/Xlsx.php

-
message: "#^Cannot access property \\$Default on SimpleXMLElement\\|false\\.$#"
count: 1
path: src/PhpSpreadsheet/Reader/Xlsx.php

-
message: "#^Cannot access property \\$Override on SimpleXMLElement\\|false\\.$#"
count: 1
path: src/PhpSpreadsheet/Reader/Xlsx.php

-
message: "#^Parameter \\#1 \\$chartElements of static method PhpOffice\\\\PhpSpreadsheet\\\\Reader\\\\Xlsx\\\\Chart\\:\\:readChart\\(\\) expects SimpleXMLElement, SimpleXMLElement\\|false given\\.$#"
count: 1
path: src/PhpSpreadsheet/Reader/Xlsx.php

-
message: "#^Cannot call method addChart\\(\\) on PhpOffice\\\\PhpSpreadsheet\\\\Worksheet\\\\Worksheet\\|null\\.$#"
count: 1
Expand Down Expand Up @@ -4535,66 +4435,6 @@ parameters:
count: 1
path: src/PhpSpreadsheet/Reader/Xlsx/PageSetup.php

-
message: "#^Property PhpOffice\\\\PhpSpreadsheet\\\\Reader\\\\Xlsx\\\\Properties\\:\\:\\$securityScanner has no typehint specified\\.$#"
count: 1
path: src/PhpSpreadsheet/Reader/Xlsx/Properties.php

-
message: "#^Property PhpOffice\\\\PhpSpreadsheet\\\\Reader\\\\Xlsx\\\\Properties\\:\\:\\$docProps has no typehint specified\\.$#"
count: 1
path: src/PhpSpreadsheet/Reader/Xlsx/Properties.php

-
message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Reader\\\\Xlsx\\\\Properties\\:\\:extractPropertyData\\(\\) has no return typehint specified\\.$#"
count: 1
path: src/PhpSpreadsheet/Reader/Xlsx/Properties.php

-
message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Reader\\\\Xlsx\\\\Properties\\:\\:extractPropertyData\\(\\) has parameter \\$propertyData with no typehint specified\\.$#"
count: 1
path: src/PhpSpreadsheet/Reader/Xlsx/Properties.php

-
message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Reader\\\\Xlsx\\\\Properties\\:\\:readCoreProperties\\(\\) has parameter \\$propertyData with no typehint specified\\.$#"
count: 1
path: src/PhpSpreadsheet/Reader/Xlsx/Properties.php

-
message: "#^Call to an undefined method object\\:\\:registerXPathNamespace\\(\\)\\.$#"
count: 3
path: src/PhpSpreadsheet/Reader/Xlsx/Properties.php

-
message: "#^Call to an undefined method object\\:\\:xpath\\(\\)\\.$#"
count: 9
path: src/PhpSpreadsheet/Reader/Xlsx/Properties.php

-
message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Reader\\\\Xlsx\\\\Properties\\:\\:readExtendedProperties\\(\\) has parameter \\$propertyData with no typehint specified\\.$#"
count: 1
path: src/PhpSpreadsheet/Reader/Xlsx/Properties.php

-
message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Reader\\\\Xlsx\\\\Properties\\:\\:readCustomProperties\\(\\) has parameter \\$propertyData with no typehint specified\\.$#"
count: 1
path: src/PhpSpreadsheet/Reader/Xlsx/Properties.php

-
message: "#^Argument of an invalid type object supplied for foreach, only iterables are supported\\.$#"
count: 1
path: src/PhpSpreadsheet/Reader/Xlsx/Properties.php

-
message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Reader\\\\Xlsx\\\\Properties\\:\\:getArrayItem\\(\\) has no return typehint specified\\.$#"
count: 1
path: src/PhpSpreadsheet/Reader/Xlsx/Properties.php

-
message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Reader\\\\Xlsx\\\\Properties\\:\\:getArrayItem\\(\\) has parameter \\$key with no typehint specified\\.$#"
count: 1
path: src/PhpSpreadsheet/Reader/Xlsx/Properties.php

-
message: "#^Property PhpOffice\\\\PhpSpreadsheet\\\\Reader\\\\Xlsx\\\\SheetViewOptions\\:\\:\\$worksheet has no typehint specified\\.$#"
count: 1
Expand All @@ -4605,16 +4445,6 @@ parameters:
count: 1
path: src/PhpSpreadsheet/Reader/Xlsx/SheetViewOptions.php

-
message: "#^Property PhpOffice\\\\PhpSpreadsheet\\\\Reader\\\\Xlsx\\\\SheetViews\\:\\:\\$sheetViewXml has no typehint specified\\.$#"
count: 1
path: src/PhpSpreadsheet/Reader/Xlsx/SheetViews.php

-
message: "#^Property PhpOffice\\\\PhpSpreadsheet\\\\Reader\\\\Xlsx\\\\SheetViews\\:\\:\\$worksheet has no typehint specified\\.$#"
count: 1
path: src/PhpSpreadsheet/Reader/Xlsx/SheetViews.php

-
message: "#^Property PhpOffice\\\\PhpSpreadsheet\\\\Reader\\\\Xlsx\\\\Styles\\:\\:\\$styles has no typehint specified\\.$#"
count: 1
Expand Down Expand Up @@ -4820,11 +4650,6 @@ parameters:
count: 1
path: src/PhpSpreadsheet/ReferenceHelper.php

-
message: "#^Parameter \\#1 \\$pCellRange of method PhpOffice\\\\PhpSpreadsheet\\\\ReferenceHelper\\:\\:updateCellReference\\(\\) expects string, string\\|null given\\.$#"
count: 1
path: src/PhpSpreadsheet/ReferenceHelper.php

-
message: "#^Parameter \\#3 \\$subject of function str_replace expects array\\|string, string\\|null given\\.$#"
count: 1
Expand Down Expand Up @@ -6680,11 +6505,6 @@ parameters:
count: 1
path: src/PhpSpreadsheet/Worksheet/Worksheet.php

-
message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Worksheet\\\\Worksheet\\:\\:getFreezePane\\(\\) should return string but returns string\\|null\\.$#"
count: 1
path: src/PhpSpreadsheet/Worksheet/Worksheet.php

-
message: "#^Parameter \\#1 \\$row of method PhpOffice\\\\PhpSpreadsheet\\\\Collection\\\\Cells\\:\\:removeRow\\(\\) expects string, int given\\.$#"
count: 2
Expand Down Expand Up @@ -8395,16 +8215,6 @@ parameters:
count: 1
path: tests/PhpSpreadsheetTests/Reader/Xlsx/AutoFilterTest.php

-
message: "#^Function PhpOffice\\\\PhpSpreadsheetTests\\\\Reader\\\\getTitleText\\(\\) has no return typehint specified\\.$#"
count: 1
path: tests/PhpSpreadsheetTests/Reader/Xlsx/ChartsTitleTest.php

-
message: "#^Function PhpOffice\\\\PhpSpreadsheetTests\\\\Reader\\\\getTitleText\\(\\) has parameter \\$title with no typehint specified\\.$#"
count: 1
path: tests/PhpSpreadsheetTests/Reader/Xlsx/ChartsTitleTest.php

-
message: "#^Cannot call method setMinimumConditionalFormatValueObject\\(\\) on PhpOffice\\\\PhpSpreadsheet\\\\Style\\\\ConditionalFormatting\\\\ConditionalDataBar\\|null\\.$#"
count: 1
Expand Down Expand Up @@ -8435,31 +8245,6 @@ parameters:
count: 1
path: tests/PhpSpreadsheetTests/Reader/Xlsx/ConditionalFormattingDataBarXlsxTest.php

-
message: "#^Cannot call method getRowHeight\\(\\) on PhpOffice\\\\PhpSpreadsheet\\\\Worksheet\\\\RowDimension\\|null\\.$#"
count: 1
path: tests/PhpSpreadsheetTests/Reader/XlsxTest.php

-
message: "#^Cannot call method getVisible\\(\\) on PhpOffice\\\\PhpSpreadsheet\\\\Worksheet\\\\RowDimension\\|null\\.$#"
count: 1
path: tests/PhpSpreadsheetTests/Reader/XlsxTest.php

-
message: "#^Cannot call method getWidth\\(\\) on PhpOffice\\\\PhpSpreadsheet\\\\Worksheet\\\\ColumnDimension\\|null\\.$#"
count: 1
path: tests/PhpSpreadsheetTests/Reader/XlsxTest.php

-
message: "#^Cannot call method getVisible\\(\\) on PhpOffice\\\\PhpSpreadsheet\\\\Worksheet\\\\ColumnDimension\\|null\\.$#"
count: 1
path: tests/PhpSpreadsheetTests/Reader/XlsxTest.php

-
message: "#^Method PhpOffice\\\\PhpSpreadsheetTests\\\\Reader\\\\XlsxTest\\:\\:testStripsWhiteSpaceFromStyleString\\(\\) has parameter \\$string with no typehint specified\\.$#"
count: 1
path: tests/PhpSpreadsheetTests/Reader/XlsxTest.php

-
message: "#^Method PhpOffice\\\\PhpSpreadsheetTests\\\\Reader\\\\Xml\\\\XmlTest\\:\\:testInvalidSimpleXML\\(\\) has parameter \\$filename with no typehint specified\\.$#"
count: 1
Expand Down
Loading

0 comments on commit 5687a53

Please sign in to comment.