* More Scrutinizer Catch Up
Continue the work of PR #3043 by attending to the 12 remaining 'new' issues.
* Php 8.1 Problem
One new null-instead-of-string problem.
This PR expands on PR #2195 from @nkjackzhang. That PR has been stalled for some time awaiting requested fixes. Those fixes are part of this PR, and additional tests and samples are added. The original request was to handle `vertical-align:middle` in Css (Excel uses `center`). This PR does its best to also handle vertical alignment Excel values not found in Css - `justify` (as `middle`) and `distributed` (as `middle`). It likewises handles valid Css values not found in Excel (`baseline`, `sub`, and `text-bottom` as `bottom`; `super` and `text-top` as `top`; `middle` as `center`).
It also handles horizontal alignment Excel values not found in Css - `center-continuous` as `center` and `distributed` as `justify`; I couldn't think of a reasonable equivalent for `fill`, so it is ignored.
The values assigned for vertical and horizontal alignment are now lower-cased (special handling required for `centerContinuous`).
The last of these changes for now. No remaining Phpstan complaints in any Writer/Xlsx. Number of lines remaining in Phpstan baseline is now below 3500.
A lot of easily fixed problems throughout Writer/Xlsx/*, mostly supplying int rather than string as input to WriteAttribute/WriteElement. There are, in fact, so many opportunities, that I will split it over 2 or 3 PRs. But this first one will get Phpstan baseline down to the goal on its own.
Some of the other problems are also easily fixed. In particular, the docBlocks in Style/ConditionalFormatting/ConditionalDataBar do not allow for null values, and should.
For one of the Phpstan upgrades, some message text had changed so drastically that the only practical solution at the time was to move the messages from phpstan-baseline.neon to phpstan.neon.dist. This was not ideal, but it allowed us time to move on and study the errors, which I have now done. At one point, Parser is expecting a variable to be an array, and that was not clear from the code. If not an array, the code will error out (which was Phpstan's concern); I have changed it to throw an exception instead. This satisfies Phpstan, and I can get the message out of neon.dist (without needing to restore it to baseline). Unsurprisingly, the exception was never thrown in the existing test suite, although I added a couple of tests to exercise that code.
In Helper/Dimension, Phpstan flagged a statement inappropriately. I suppressed the message using an annotation and filed a bug report https://github.com/phpstan/phpstan/issues/7563. A fix for the problem was merged yesterday, which is good, but it puts us in a tenuous position. The annotation is needed now, but, when the fix is inevitably pushed to the version we use, the no-longer-needed annotation will trigger a different message. Recode so that neither the current nor the future versions will issue a message, eliminating the annotation in the process.
Fix#3011. Some properties for Trendlines were omitted in the original request for this feature. Also, the trendlines sample spreadsheet included two charts. The rendering script 35_Chart_render handles this, but overlays the first output file with the second. It is changed to produce files with different names.
When Reader/Xlsx/WorkbookView was split off from Reader/Xlsx.php, one statement accidentally brought `!empty($this->loadSheetsOnly)` with it. That property does not exist in WorkbookView, so the test is useless (it is always empty); and, in fact, the caller passes its own version of loadSheetsOnly as a parameter, so it isn't needed even it did exist. In Php8.2, this might be a deprecation, although it hasn't shown up in the GitHub 8.2 tests. Fix it anyhow.
* Charts - Add Support for Date Axis
Fix#2967. Fix#2969 (which had already been fixed prior to opening the issue, but had added urgency for Date Axes). Add ability to set axis type to date axis, in addition to original possiblities of value axis and category axis.
* Update 33_Chart_create_line_dateaxis.php
No idea why php-cs-fixer is complaining. It didn't do so when I first uploaded. I can't duplicate problem on my own system. Not enough detail in error message for me to act. Grasping at straws, I have moved the function definition (which is the only use of braces in the entire script) from the end of the script to the beginning.
* Update 33_Chart_create_line_dateaxis.php
Some comments were mis-aligned. This may be related to the reasons behind PR #3025, which didn't take care of this because this script had not yet been merged.
* Html Writer - Do Not Generate background-color When Fill is None
For PR #3002, I noted that there was a problem with Dompdf truncating images. I raised an issue with them (https://github.com/dompdf/dompdf/issues/2980), and they agree that there is a bug; however, they also suggested a workaround, namely omitting background-color from any cells which the image overlays. That did not at first appear to be a solution which could be generalized for PhpSpreasheet. However, investigating further, I saw that Html Writer is generating background-color for all cells, even though most of them use the default Fill type None (which suggests that background-color should not be specified after all). So this PR changes HTML Writer to generate background-color only when the user has actually set Fill type to something other than None. This is not a complete workaround for the Dompdf problem - we will still see truncation if the image overlays a cell which does specify a Fill type - however, it is almost certainly good enough for most use cases.
In addition to that change, I made the generated Html a little smaller and the code a little more efficient by combining the TD and TH styles for each cell into a single declaration and calling createCssStyle only once.
* Revamp One Test
Look for both td.style and th.style instead of just td.style in test.
The setup for unit testing in Github in the "Install dependencies" log reports 11 members as "does not comply with with psr-4 autoloading standard." In each case, it is because the test namespace does not match the directory; in most cases, it was caused by the member being moved from one directory to another without changing the namespace declaration. No harm results from these problems, but there's also no reason to not correct them.
* Upgrade Dev TCPDF to 6.5
Implementation of https://github.com/tecnickcom/TCPDF/pull/467, which is available in just-released Tcpdf 6.5, will improve look of Tcpdf rendering for PhpSpreadsheet. Fix#1164.
One test had been suppressed for Tcpdf, ostensibly because it was not compatible with Php8. As it turns out, the PhpSpreadsheet code which invokes Tcpdf was (harmlessly) incorrect, so the Php8 issue was actually with PhpSpreadsheet, not Tcpdf. That code is corrected, and the test is no longer suppressed.
* Update Change Log
Pick up some earlier changes as well as this one, and deprecations which had been omitted from the 1.24 change log.
See discussion in #2999. Mpdf is not acknowledging the styling that we're using to hide table rows. I have opened an issue with them, but enclosing the cells in the hidden row inside a div with appropriate css does seem to be a workaround, and that can be incorporated into PhpSpreadsheet. It's kludgey, and it isn't even valid HTML, but ...
Mpdf also doesn't like the addition of the ```file:///``` prefix when using local images from Windows (sample 21). Results are better when that prefix is not added.
Dompdf seemed to have problems with sample 21 images on both Windows and Unix, with or without the file prefix. It does, however, support data urls for both, so is changed to embed images. It's still not perfect - the image seems truncated to the row height - but the results are better. I will continue to research, but may proceed as-is if I don't find anything better to do.
Html Writer was producing a file with mixed line endings on Windows. This didn't cause any harm, but it seems a bit sloppy. It is changed to always use PHP_EOL as a line ending.
* Replace Dev jpgraph/jpgraph with mitoteam/jpgraph
PR #2979 added support for mitoteam/jpgraph as an alternative to jpgraph/jpgraph. The package jpgraph/jpgraph is abandoned in composer, and the version loaded with composer has been unusable for some time. This PR removes the dev requirement for jpgraph/jpgraph, and adds a dev requirement for mitoteam/jpgraph in its place.
With a usable graph library, a number of tests and samples that had been disabled are now re-enabled. A lot of new functionality has been added to Charts recently. Some of that new code has exposed bugs in JpgraphRendererBase. I have fixed those where I could. A handful of exceptions remain; I will investigate, and hopefully fix, those over time, but I don't feel it is necessary to fix them all before installing this PR - we are already way ahead of the game with the graphs that are working.
Three members had been ignoring code coverage in whole or in part because of the unavailability of a usable graph libray. Code coverage is restored in them. I am relieved to report that, although they aren't completely covered, adding them did not reduce code coverage by much - it is still over 90.4%.
I took a look at JpgraphRendererBase and Phpstan. Phpstan reports 128 problems. When I added some docblocks to correct some of those, the number increased to 284. Sigh. I will investigate over time, but, for now, we will still suppress Phpstan for JpgraphRendererBase.
I do not find a License file for mitoteam. However, there also wasn't one for jpgraph in the first place. Based on that and the discussion in #2996 (mitoteam will be used in exactly the same manner as mpdf), I don't think this is a problem. IANAL.
* PHP 8.2 Problems
Tons of "cannot create dynamic property" deprecations in jpgraph. Disable the test with most of those for now; leave the two with only a handful of messages enabled.
* Correct Failures in 2 Stock Charts
Down to 6 templates on which Render fails.
Addresses a problem identified in issue #2991. Chart name is set in constructor, but there is no method to subsequently change it. This PR adds a method to do so.
* Test if UConverter Exists Without Autoload
Fix#2982. That issue is actually closed, but it did expose a problem. Our test environments all enable php-intl, but that extension isn't a formal requirement for PhpSpreadsheet. Perhaps it ought to be. Nevertheless ...
Using UConverter for string translation solved some problems for us. However, it is only available when php-intl is enabled. The code tests if it exists before using it, so no big deal ... except it seems likely that the people reporting the issue not only did not have php-intl, but they do have their own autoloader which issues an exception when the class isn't found. The test for existence of UConverter defaulted to attempting to autoload it if not found. So, on a system without php-intl but with a custom autoloader, there is a problem. Code is changed to suppress autoload when testing UConverter existence.
Pending this fix, the workaround for this issue is to enable php-intl.
* Minor Improvement
Make mb_convert_encoding use same substitution character as UConverter, ensuring consistent results whatever the user's environment.
* And Now That I Figured That Out
Since mb_convert_encoding can now return the same output as UConverter, we don't need UConverter (or iconv) after all in sanitizeUTF8.
No source code changes, just additional tests. FormulaParser appears unused, replaced by newer code in Calculation. However, it's a public interface, so probably shouldn't be deleted without first deprecating it. I have no strong feelings about whether that should happen. However, as long as it's part of the package, we may as well have some formal unit tests for it.
oleibman
Mark Baker
Alexis Lefebvre
Mikhail Oleynik