This was suggested by the investigation of issue #3811. No fix is necessary for the issue. However, two possible code solutions (Php setlocale, which comes with certain design flaws, and StringHelper set(Decimal/Thousands)Separator were suggested, and neither is adequately tested. This PR adds such tests.
Unusually, getting StringHelper Decimal Separator, Thousands Separator, and Currency Code can result in a change to those properties. So, the existing design in several tests where those properties are captured in Setup and restored in Teardown do not work quite as designed. Instead, the ability to set those properties to their default value (null) is added, and the tests re-done to restore the default in Teardown.
The two methods yield the same results when parsing input. However, they diverge when examining output fields through `getFormattedValue`. Such output is currently correct (usually) when using setlocale, but not when using StringHelper. The former works through the 'trick' of using `sprintf(%f)`, which generates a locale-aware string. However, using non-locale-aware `sprintf(%F)` followed by `str_replace` will produce the correct result for both setlocale and StringHelper. One place in the code uses a cast to string, which is incorrect for both methods. Following that up with the same str_replace makes it correct for both. These changes permit, but do not require, the user to avoid setlocale altogether.
It remains an open question whether Settings/Calculation::setLocale should set DecimalSeparator, CurrencySeparator, and CurrencyCode. That makes logical sense, but it would be a breaking change, and having to explicitly set those values when using setLocale does not seem especially burdensome. For now, such a change will not be made.
While we might never be able to have 100% of our code strict, we can at
the very least do it for all of our tests. This ensures that our tests
are using our API with the types as intended by the test author, and not
silently be cast to what our API requires.
And quite a bit more manual changes. The idea is that typing of our
tests can be a bit more loose, so we assume PHPDoc is mostly correct. If
that happens to be wrong, it should be caught by the tests themselves.
* Php 8.3 Problem - RLM Added to NumberFormatter Currency - Minor Break
Fix#3571. This isn't truly a Php8.3 problem - it all depends on the version of ICU with which Php was linked. ICU 72.1 adds an RLM (right-to-left mark) character in some circumstances when creating a currency format. This broke some tests for the Currency and Accounting wizards, and can result in a difference in the appearance of some spreadsheet cells.
This PR changes code to strip out the RLM or not depending on a new property. The new property could default to true (so end-users will not see any change no matter what release of ICU is used), or false. For the latter, users might see a break, but my assumption is that the ICU developers have good reasons for their change, and it's probably best to go along with it. If users wish to retain the existing behavior, they can do so by adding the following code before setting the wizard's locale:
```php
$wizard->setStripLeadingRLM(true);
```
* Eliminate 2 Dead Statements
Correctly flagged by Scrutinizer.
Successor to PR #3523. There are 494 single-line changes (`public function provider` to `public static function provider`) in this PR. None of these were made manually; they were all created with the following script (adapted from
https://stackoverflow.com/questions/25909820/how-to-recursively-iterate-through-files-in-php):
```php
$dir = 'C:/git/unit10prep2/tests/PhpSpreadsheetTests';
$it = new RecursiveDirectoryIterator($dir);
// Loop through files
foreach(new RecursiveIteratorIterator($it) as $file) {
if ($file->getExtension() === 'php') {
$contents = file_get_contents($file);
$new = preg_replace('/public function (\\w*)([Pp])rovider/', 'public static function $1$2rovider', $contents);
if ($new !== $contents) {
echo "changing $file\n";
file_put_contents($file, $new);
}
}
}
```
After this PR, there will be one more, with a small number of test changes, and enabling PhpUnit 10 for Php 8.1+.
* Problems Formatting Very Small and Very Large Numbers
Fix#3128 (author found a workaround but the problem remains). For some complex masks, when a cast of the cell value from float to string results in the use of scientific notation, the result of the formatting is unusable. I believe this PR solves the problem for numbers close to zero (Php cast from float to string uses scientific notation starting with 1E-5), and for a range of large numbers which are not handled correctly now.
However, I have not found a way to ensure that the results match Excel for very large numbers (1E18 or larger); this change at least ensures that the resulting string is an accurate rendition of the number (which is not the case now) even if doesn't match Excel. As an example, if you use the mask reported in the original issue `0 000.0` and enter a value of 1E90 into the cell, Excel will show it as 1 followed by 87 zeros, a space, 3 more zeros, decimal point and zero. I have not figured out how to get PhpSpreadsheet to do that; for now, it will just return the formatted value as 1 followed by 90 zeroes instead (I might have chosen to go with scientific notation instead). I will continue to think about those, but do not feel it is worth delaying the improvements in this ticket while I do so.
The affected section of code also truncated to the appropriate precision. It now rounds, as Excel does.
This seemed to be an area of code where problems might arise on 32-bit systems, and, indeed, I found something in the formatting code which had to be changed for 32-bit to work correctly. As long as I was doing that anyhow, I ran the full test suite, and found that Php8.1 had introduced some new stringencies which caused problems in a handful of places. All were found in Xls Reader, and all are corrected now.
* Remove Dead Assignment
Scrutinizer will be happy now.
Fix#1929. This was already substantially fixed, but there was a lingering problem with an unexpected leading space. It turns out there was also a problem with leading zeros, also fixed. There are also problems involving commas; fixing those seems too complicated to delay these changes, but I will add it to my to-do list.
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 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.
* Fix Spreadsheet Copy, Disable Clone, Improve Coverage
This PR was supposed to be merely to increase coverage in Spreadsheet. However, in doing so, I discovered that neither clone nor copy worked correctly. Neither had been covered in the test suite. Copy not only did not work, it broke the source spreadsheet as well. I tried to debug and got nowhere; I even tried using myclabs/deep-copy which is already in use in the test suite, but it failed as well. However, write and reload ought to work just fine for copy. It can't be used for clone; however, since copy does what clone ought to do, there's no reason why clone needs to be used, so __clone is changed to throw an exception if attempted.
One other source change was needed, an obvious bug where an if condition uses 'or' when it should use 'and'. Also, one docblock declaration needed a change. Aside from that, the rest of this PR is test cases, and overall coverage passes 89% for the first time.
* Clone is Okay After All
But copy wasn't, changing it to just return clone. Perhaps save and reload will be needed instead at some point, but not yet.
* An Error I Cannot Reproduce
PHP8.1 unit test says error because GdImage can't be serialized. I can't reproduce this error on any of my test systems. I have no idea why GdImage is even involved. Using try/catch to see if it helps.
* Weird Failures in Github
I thought restoring clone was a good idea. That left me in a state where, after one change, copy/clone no longer worked on Github (unable to reproduce on any of my test systems). After a second change, copy worked but clone didn't, again unable to reproduce. So, reverting to original version - copy does save and reload, clone throws exception.
* Big Memory Leak in One Test
Many tests leak a little by not issuing disconnectWorksheets at the end. CellMatcherTest leaks a lot by opening a spreadsheet many times. Phpunit reported a high watermark of 390MB; fixing CellMatcherTest brings that figure down to 242MB. I have also changed it to use a formal assertion in many cases where markTestSkipped was (IMO inappropriately) used.
* Another Leak
Issue2301Test lacks a disconnect. Adding one reduces HWM from 242MB to 224.
Mostly new tests, some code annotations, some minor code changes:
- RichText clone logic is wrong
- TextElement doesn't have object properties, doesn't need clone
* Eliminate Most Scrutinizer Problems in Test Suite
Mostly minor code changes, with some annotations.
* Missed 2 php-cs-fixer Problems
They should be fixed now.
You can set up a conditional style to, say, apply to cells equal to boolean values. For such conditions, the Excel XML specifies `TRUE` or `FALSE`. It is noteworthy that false matches empty cells as well as FALSE, but not 0; similarly TRUE does not match 1. The Xlsx Writer just casts these values to string, which will not work properly. The Xlsx Reader treats the values as strings, so it won't work properly either. This PR corrects both. Also the doc blocks in Style/Conditional allow bool in some places, but not in others; these are corrected but no executable code is changed there.
Fix#2499, which see for details of an obscure problem affecting both PhpSpreadsheet and Excel. Add support for palette contained in workbook styles. This seems to be a very rare occurrence, so allow it only when the palette contains exactly 64 entries. If there are other possibilities, we'll presumably have a new workbook to guide us how to handle them. Also add some tests for specification of indexed color without palette, another rarity (no in-range examples amongst our current files). Also change one private static array, initialized once at run-time and never changed, to a constant.
* WIP Namespacing Phase 2 - Styles
This is part 2 of a several-phase process to permit PhpSpreadsheet to handle input Xlsx files which use unexpected namespacing. The first phase, introduced as part of release 1.19.0, essentially handled the reading of data. This phase handles the reading of styles. More phases are planned.
It is my intention to leave this in draft status for at least a month. This will give time for additional testing, by me and, I hope, others who might be interested.
This fixes the same problem addressed by PR #2458, if it reaches mergeable status before I am ready to take this out of draft status. I do not anticipate any difficult merge conflicts if the other change is merged first.
This change is more difficult than I'd hoped. I can't get xpath to work properly with the namespaced style file, even though I don't have difficulties with others. Normally we expect:
```xml
<stylesheet xmlns="http://whatever" ...
```
In the namespaced files, we typically see:
```xml
<x:stylesheet xmlns:x="http://whatever" ...
```
Simplexml_load_file specifying a namespace handles the two situations the same, as expected. But, for some reason that I cannot figure out, there are significant differences when xpath processes the result. However, I can manipulate the xml if necessary; I'm not proud of doing that, and will gladly accept any suggestions. In the meantime, it seems to work.
My major non-standard unit test file had disabled any style-related tests when phase 1 was installed. These are now all enabled.
* Scrutinizer
Its analysis is wrong, but the "errors" it pointed out are easy to fix.
* Eliminate XML Source Manipulation
Original solution required XML manipulation to overcome what appears to be an xpath problem. This version replaces xpath with iteration, eliminating the need to manipulate the XML.
* Handle Some Edge Cases
For example, Style file without a Fills section.
* Restore RGB/ARGB Interchangeability
Fix#2494. Apparently EPPlus outputs fill colors as `<fgColor rgb="BFBFBF">` while most output fill colors as `<fgColor rgb="FFBFBFBF">`. EPPlus actually makes more sense. Regardless, validating length of rgb/argb is a recent development for PhpSpreadsheet, under the assumption that an incorrect length is a user error. This development invalidates that assumption, so restore the previous behavior.
In addition, a comment in Colors.php says that the supplied color is "the ARGB value for the colour, or named colour". However, although named colors are accepted, nothing sensible is done with them - they are passed unchanged to the ARGB value, where Excel treats them as black. The routine should either reject the named color, or convert it to the appropriate ARGB value. This change implements the latter.
* Allow single-cell checks on conditional styles, even when the style is configured for a range of cells
* Work on the CellMatcher logic to evaluate Conditionals for a cell based on its value, and identify which conditional styles should be applied
* Refactor style merging and cell matching for conditional formatting into separate classes; this should make it easier to test, and easier to extend for other CF expressions subsequently
* Added support for containsErrors and notContainsErrors
* Initial work on a wizard to help simplify created Conditional Formatting rules, to ensure that the correct expressions are set
* Further work on extending the Conditional Formatting rules to cover more of the options that are available in MS Excel
* Prevent phpcs-fixer from removing class @method annotations, used to identify the signature for magic methods used in Wizard classes
* Implement `fromConditional()`` method to allow the creation of a CF Wizard from an existing Conditional
* Ensure that xlsx Reader picks up the timePeriod attribute for DatesOccurring CF Rules
* Allow Duplicates/Uniques CF Rules to be recognised in the Xlsx Reader
* Basic Xlsx reading of CF Rules/Styles from <extLst><ext><ConditinalFormattings> element, and not just the <ConditinalFormatting> element of the worksheet
* Add some validation for operands passed to the CF Wizards
- remove any leading ``=` from formulae, because they'll be embedded into other formulae
- unwrap any string literals from quotes, because that's also handled internally
Handle cross-worksheet cell references in cellReferences and Formulae/Expressions
* re-baseline phpstan
* Update Change Log with details of the CF Improvements
* Fill Pattern Start and End Colors
Fix#2441. The Fill constructor sets start color to white and end color to black and the Xlsx writer writes these values to the output file. This appears to be the wrong setting for all 7 LIGHT* pattern types, 2 of the 7 DARK* patterns (DARKGRAY and DARKTRELLIS), and 1 of the 3 GRAY patterns (GRAY0625). When the wrong colors are written at save time, those patterns are not as expected. Xls writer does not appear to have the same problem.
The XML does not require either a start or end color, and the omission of these colors in the file being read was responsible for the problem. The code is changed to mimic that behavior by omitting the color tags at write time if they have not changed from when they were created by the Fill constructor (they will be written for gradient or solid patterns regardless).
This is another change which is easier to confirm via samples rather than tests. There are separate samples for Xlsx and Xls; as Excel will be quick to warn you, Xls is not as fully functional as Xlsx with respect to fill patterns. The samples do include a cell where one of the cells (LightGrid in C11) explicitly specifies the "default" colors.
* Scrutinizer
It somehow ascribed to me a problem in code which was unchanged by this PR. Correct it anyhow, along with some Phpstan fixes (errors now ignored because of change).
* Added Tests
Also corrected some docBlock problems with Style/*/parent and getSharedComponent.
* Create 2 Abstract Methods
Scrutinizer complained that 2 methods found in all Supervisor sub-types were not defined in Supervisor. Add abstract methods to satisfy it.
* Scrutinizer Ignoring Typehints
Try this instead.
* Slight Improvement
Better handling of Style->getParent().
Fix#2385. NumberFormatter is using sprintf on a float, and is seeing inconsistent rounding as a result (it will also occasionally result in `-0`). Change to round the number before presenting it to sprintf.
Prevent calling clone and getHashCode when not needed
because these calls are very expensive.
When applying styles to a range of cells can we cache the
styles we encounter along the way so we don't need to look
them up with getHashCode later.
This change restored behavior from PHP7 in PHP8. In PHP7 calling
setSize(0) resulted in font size being set to 10. The fix addresses
change to equal comparisons in PHP8. Extra comparison is added to keep
result from PHP7 in PHP8 for the setSize(0) case.
- Move TREND() functions into the Statistical Trends class
- Unit tests for TREND()
- Create Confidence class for Statistical Confidence functions, and the CONFIDENCE() method
* Apply Column and Row Styles to Existing Cells
This is a fix for issue #1712.
When a style is applied to an entire row or column, it is currently
only effective for cells which don't already contain a value.
The code needs to iterate through existing cells in the row/column
in order to apply the style to them.
This could be considered a breaking change, however, I believe that
the change makes things operate as users would expect, and that the
existing implementation is incomplete.
The change also removes protected element conditionalStyles from
the Style class. That element is an unused remnant, and can no longer be
set or retrieved - methods getConditionalStyles and setConditionalStyles
actually act on an element in the Worksheet class.
Finally, additional tests are added so that Style, and in fact the
entire Style directory, now has 100% test coverage.
* Scrutinizer Changes
Scrutinizer flagged 6 statements. 5 can be easily corrected.
One is absolutely wrong (it thinks iterating through cells in column
can return null). Let's see if we can satisfy it.
* Remove Exception For CellIterator on Empty Row/Column
For my first attempt at this change, which corrects a bug by updating styles
for non-empty cells when a style is set on a row or column, I wished to make things
more efficient by using setIterateOnlyExistingCells, something which the
existing documentation recommends. This caused an exception to be generated
when the row or column is empty. So I removed that part of the change while I
researched what was going on.
I have completed that research. The existing code does throw an exception
when the row/column is empty and iterateOnlyExistingCells is true. However,
that does not seem like a reasonable action. This situation is analagous to
iterating over an empty array, and that action is legal and does not throw.
The same should apply here. There were no tests for this situation,
and now there are.
I have added additional tests, and coverage for all of RowCellIterator,
ColumnCellIterator, and CellIterator are all now 100%. Some of my new tests
were added in new members, because the existing tests all relied on mocking,
which was not the best choice for the new tests. One of the existing tests
for RowCellIteratorTest (testSeekOutOfRange) was wrong; it issued the expected
exception, but for the wrong reason. I have added an additional test to
ensure that it fails "correctly".
The existing documentation says that the default value for
IterateOnlyExistingCells is true. In fact, the default value is false.
I have corrected the documentation.
* More Scrutinizer
I believe its analysis is incorrect, but this should silence it.
* DocBlock Correction
ColumnCellIterator DocBlock for current indicated it could return null
or Cell, but it can really return only Cell. This had caused Scrutinizer
to complain earlier.
* PHP8 Environment Appears to be Fixed
Cosmetic change to Doc member. I suspect there is a way to rerun all
the tests without another push, but I have been unable to figure out how.
Issue #580 has gone stale since I started work on this.
Nevertheless, this implements an exportArray function as an
exact counterpart of applyFromArry.
I chose the name exportArray to avoid confusion with the existing
method getStyleArray, which does something completely different.
This change also increases coverage for all the Style classes to 100%,
with the exception of Style.php itself. There were several (unchanged)
places in Style.php where I did not have sufficient understanding of
what was supposed to be happening, so could not create tests.
All properties used by applyFromArray are exported by this method.
Note that conditional styles are not covered; this is consistent
with the fact that they are not covered by applyFromArray.
The method is implemented as a final public function in Style/Supervisor,
which calls abstract protected function exportArray1, which is implemented
in each of the subclasses, and which calls final protected
function exportArray2 in Style/Supervisor.
So exportArray is usable for any of the subclasses as well.
The new method is added to the documentation.
The existing documentation for applyFromArray was alphabetized to make
it easier to follow.
One property (Style quotePrefix) was added to the documentation.
Some Borders pseudo-properties (vertical, horizontal, and outline) were
documented as usable by applyFromArray,
but aren't actually supported - they were removed.
The documentation of the properties seemed to use setProperty and
getProperty fairly randomly - it now uses setProperty exclusively.
New constants were added for the textRotation "angles" used to create a
"stacked" cell. I felt that changing the readers and writers to use
these constants was beyond the scope of this change, but it is
on my to-do list.
If font style Superscript is set to true, Subscript is set to false.
Likewise, setting Subscript to true sets Superscript to false.
Both of these are working as they should. However,
setting Superscript to false causes Subscript to be set to true,
and setting Subscript to false causes Superscript to be set to true.
I believe that is an error in both cases. This change fixes it.
There seem to be no existing tests for Font styles.
I added the tests necessary to validate this change.
I will put adding more on my to-do list.
oleibman
Adrien Crivelli
Mark Baker
Mark Baker
oleibman
Einar
Matjaž Drolc