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.
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+.
These changes have already been implemented twice, and been regressed twice. I'll try once more (with a different approach), then give up ...
As configured, Phpstan running under Php7 reports no errors. However, running under Php8, it reports 100 (!) errors. The vast majority of these are due to two reasons:
- renaming parameters in Php builtin functions in preparation for named parameters.
- using the new class GdImage rather than type resource as the argument type for many image-based functions.
Regardless of the cause, this will be a problem sooner or later. This PR is an attempt to get ahead of that problem. For source members, it mostly adds annotations or updates doc-blocks. Only 2 members have changes to executable code, and these are very minor - BitWise and Writer/Xlsx. For test members, all baseline errors are deleted and the code is fixed. Php7 and Php8 both report no errors with this configuration.
oleibman
Adrien Crivelli
MarkBaker