PhpSpreadsheet treats CODE and UNICODE as equivalent, likewise for CHAR and UNICHAR. They are, in fact, different. CODE and CHAR deal only with single-byte character sets (Windows-1252 or MacRoman), while UNICODE and UNICHAR deal with all of Unicode. This PR separates them. The existing unit test for CODE was, in many cases, applicable to UNICODE (for which there was no separate test). The tests are corrected for CODE, new tests are added, and a separate test for UNICODE is added. CHAR was mostly okay, new tests are added, and a separate test for UNICHAR is added.
They aren't quite interchangeable. Both are used in the test suite, with no indication of why one or the other. I think we'd be best off being consistent. Based on the names, I think `_calculateFormulaValue` was intended as a private, or at least internal, method, so favor `calculateFormula`. I do not intend to rename or re-categorize `_calculateFormulaValue`, just remove its usage when it isn't clearly warranted.
With PhpUnit 10 came the ability to use Php attributes rather than doc-block annotations for things like "data provider". PhpUnit 11 deprecates the use of annotations, and PhpUnit 12 will not not permit their use. Since PhpUnit 11 requires Php8.2+, we cannot adopt it as long as we support Php8.1, which will continue to be the case for some time. However, there is no penalty for early adoption.
Php-cs-fixer can use:
```
'php_unit_attributes' => ['keep_annotations' => false],
```
This allows us to run `composer fix` to automate all the needed changes. No manual changes were needed for any of the test members.
With this change, PhpUnit 9 can no longer be used with the test suite. File composer.json is updated to reflect that reality, and phpunit9.xml.dist, which has been supplied in case anyone needed to use PhpUnit 9, is no longer required, and is thus deleted. For now, PhpUnit 11 is not being added as a possibility.
No source code is changed in this PR.
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.
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+.
* PHP8.1 Deprecation Passing Null to String Function
For each of the files in this PR, one or more statements can pass a null to string functions like strlower. This is deprecated in PHP8.1, and, when deprecated messages are enabled, causes many tests to error out. In every case, use coercion to pass null string rather than null.
* TextData - Minor Changes, Test Coverage
Per agreement on a previous push, I looked into standardizing the initialization of the TextData functions (like Engineering and MathTrig), with particular regard for avoiding multiple later null coercions. This simplifies the code quite a bit. This PR also increases coverage to 100% for all TextData modules. All entries in Phpstan baseline for non-deprecated TEXTDATA functions are removed. There were some minor bugfixes.
Whereas Excel (and Gnumeric) treat booleans when supplied as strings as 'TRUE' or 'FALSE', ODS treats them as '1' or '0'. Unlike Excel, ODS generally does not allow bool for int arguments; it does, however, allow them for FIND and SEARCH. ODS allows boolean for into for SUBSTITUTE even though Excel doesn't. ODS allows bool for string for NUMBERVALUE and VALUE even though Excel doesn't. ODS accepts 0 as an argument for CHAR; Excel doesn't. Most of this seems like random decisions on the part of the developers; I've done my best to follow the products in each case. There is a new test member devoted to ODS tests.
Gnumeric has an anomaly vis-a-vis the others - if length is supplied to LEFT/MID/RIGHT as null, Gnumeric treats it as 0 rather than 1.
All tests now take place in the context of a spreadsheet ...
Except for RETURNSTRING, which is not the implementation of an Excel function, and is referred to in the rest of PhpSpreadsheet only in the unit tests for itself. It should probably be deprecated, but that is not part of this PR, just in case there is some reason for it that I couldn't discern.
I have tried to make the first line of each doc block identify the Excel function name rather than its name in PhpSpreadsheet. I think it makes things more comprehensible.
Some tests call Settings::setLocale, but there was no Settings::getLocale. At the end of the tests which do it, they invoke setLocale('EN-US'), which, in a practical sense, is sufficient. However, in theory it would be better for them to get the current locale before changing it, then changing it back to the original when the time came. I have added getLocale and made the appropriate testing change.
The CHAR function took an interesting turn. One can set the value of a cell to, say, CHAR(2), the ASCII/UTF-8 representation of a control character, which is not legal in certain contexts. The only Reader/Writer that could handle this without problems is Xls, which deals with binary data all the time. However, if you tried to write it to Xlsx, Excel would not be able to open the resulting file because of what it considers an illegal character. I changed the Xlsx writer to escape such characters when writing the value of a string function. I did not make any other changes to the Xlsx writer - it seems to me that setting a cell to CHAR(2) is legitimate, but setting it to say `"\x02"` seems less likely to be legitimate, so the latter will still fail (although `="\x02"` should work). The Xlsx reader already supports the escape mechanism that I added to the writer.
CHAR control character and Ods - not supported by either Reader or Writer. I did not attempt to add this now. There is lots still missing from ODS, and this item just can't be a high priority amongst all of those.
CHAR control character and Csv - it is supported by reader and writer if the file has a csv extension. However, trying to guess the mime type without an extension - the control character makes mime_get_type guess application/octet-stream, and PhpSpreadsheet therefore thinks that Csv can't read it.
CHAR control character and Html. Actual use of the control character in the file is subject to the same problems as Xml (i.e. Xlsx and Ods). It wasn't terribly difficult to get the Html Writer to change `"\x02"` to "``". I believe that this is technically legal; however, DOMDocument.loadHTML rejects it as an illegal entity, and I am not convinced that it is wrong to do so, so I haven't changed the Html writer.
* Scrutinizer
Correct 3 minor errors.
* Let's start with some appeasements to phpstan, just to reduce the baseline
* Appeasements to phpstan, taking the number of reported errors down to just 61
This request does not change any source code, only tests.
For a change on which I was working, a test passed when run on its own,
but failed when run as part of the full test suite. It turned out that
an existing test had changed a static value,
thousands separator in this case, and failed to restore it.
The test turned out to be AdvancedBinderTest.
The search for the offending test was more difficult than it should have
been because 26 test scripts which had nothing to do with thousands
separator nevertheless changed that value. They all changed
decimal separator, currency code, and compatibility mode as well,
again for no reason. I changed all of those to eliminate those operations.
I changed the following tests, which actually do change the static
properties identified above for a reason, to restore them as part of teardown.
- CalculationTest sets compatibilityMode and locale
- DayTest sets compatibilityMode, returnDateType, and excelCalendar
- CountTest sets compatibilityMode
- FunctionsTest sets compatibilityMode and returnDateType
- AdvancedValueBinderTest sets currencyCode, decimalSeparator, thousandsSeparator
- StringHelperTest sets currencyCode, decimalSeparator, thousandsSeparator
- NumberFormatTest sets currencyCode, decimalSeparator, thousandsSeparator
- HtmlNumberFormatTest sets currencyCode, decimalSeparator, thousandsSeparator
* Merge branch 'master' of C:\Projects\PHPOffice\PHPSpreadsheet\develop with conflicts.
* Argument fix
* Text Test functions refactored into individual test files
* Codestyle (line at eof)
* docblocks
oleibman
Adrien Crivelli
MarkBaker
oleibman