A continuation of PR #3859. Change code that would be flagged if we were to run Phpstan at level 9 (we currently run level 8). I may or may not follow up with source code (454 level-9 problems remain for src), but there is no reason to avoid the effort for samples.
No changes are made to src.
* Let Phpstan Run on Samples
Phpstan currently analyzes all source and test members. We already run phpcs and php-cs-fixer on samples as well. I would expect that samples are often used as templates for code in userland; it behooves us to be at least as careful with those members as for the others which are already being analyzed. Aside from 1300+ messages `Variable $helper might not be defined.`, which will be suppressed in phpstan.neon.dist, there are really only a few changes needed for sample members, so that part of the code base was already in good shape, and is now even better. No annotations were needed.
* Scrutinizer 2 out of 3
1 false positive, now suppressed; fix other 2.
* Remove Dead Code
* Very Minor Changes
* Add infra
* Check For and Eliminate Octal Literals
Php8.1 introduced a new form of octal literal (0o123); the old form (0123) continues to be supported, but can certainly be misleading. There are exactly 6 uses of the old form in the code base (5 in tests, 1 in infra). 4 of these are clearly unintended (02 or 03), and the leading 0 should just be dropped for those. The other 2 are for file permissions and are easily replaced.
* Minor Changes to LocaleGenerator
Testing the permissions change exposed some problems when the locale files don't exist. This never shows up as an issue when the files do exist, which is pretty much all the time, but correct the theoretical exposures anyhow.
* Populate bg and en_uk Locale Files from Spreadsheet
Also rename locale generator so that it is executed first. This is when it should happen because other tests depend on its results.
* Some Bulgarian Function Omissions
Figured out a reasonable way to compare old Bulgarian function file to new; this identified a handful of now-corrected omissions and errors. Note that JIS function, in old list, is not on spreadsheet, but the Bulgarian translation is the same as English (so the translation isn't needed), and the function has been replaced by DBCS, which is also not on the spreadsheet.
* R1C1 Format and Internationalization, plus Relative Offsets
Fix#1704, albeit imperfectly. Excel's implementation of this feature makes it impossible to fix perfectly. I don't know why it was necessary to internationalize R1C1 in the first place - the benefits are so minimal,and the result is worksheets that break when opened in different locales. Ugh. I can't even find complete documentation about the format in different languages; I am using https://answers.microsoft.com/en-us/officeinsider/forum/all/indirect-function-is-broken-at-least-for-excel-in/1fcbcf20-a103-4172-abf1-2c0dfe848e60 as my definitive reference.
This fix concentrates on the original report, using the INDIRECT function; there may be other areas similarly affected. As with ambiguous date formats, PhpSpreadsheet will do a little better than Excel itself when reading spreadsheets with internationalized R1C1 by trying all possibilities before giving up. When it does give up, it will now return `#REF!`, as Excel does, rather than throwing an exception, which is certainly friendlier. Although read now works better, when writing it will use whatever the user specified, so spreadsheets breaking in the wrong locale will still happen.
There were some bugs that turned up as I added test cases, all of them concerning relative addressing in R1C1 format, e.g. `R[+1]C[-1]`. The regexp for validating the format allowed for minus signs, but not plus signs. Also, the relevant functions did not allow for passing the current cell address, which made relative addressing impossible. The code now allows these, and suitable test cases are added.
* Use Locale for Formats, but Not for XML
Implementing a suggestion from @MarkBaker to use the system locale for determining R1C1 format rather than looping through a set of regexes and accepting any that work. This is closer to how Excel itself operates. The assumption we are making is to use the first character of the translated ROW and COLUMN functions. This will not work for Russian or Bulgarian, where each starts with the same letter, but it appears that Russian, at least, still uses R1C1. So our algorithm will not use non-ASCII characters, nor characters where ROW and COLUMN start with the same letter, falling back to R/C in those cases. Turkish falls into that category. Czech uses an accented character for one of the functions, and I'm guessing to use the unaccented character in that case. Polish COLUMN function is NR.KOLUMNY, and I'm guessing to use K in that case.
The function that converts R1C1 references is also used by the XML reader *where the format is always R1C1*, not locale-based (confirmed by successfully opening in Excel an XML spreadsheet when my language is set to French). The conversion code now handles that distinction through the use of an extra parameter. Xml Reader Load Test is duplicated to confirm that spreadsheet is loaded properly whether the locale is English or French. (No, I did not add an INDIRECT function to the Xml spreadsheet.)
Tests CsvIssue2232Test and TranslationTest both changed locale without resetting it when done. That omission was exposed by the new code, and both are now corrected.
* OpenOffice and Gnumeric
OpenOffice and Gnumeric make it much easier to test with other languages - they can be handled with an environment variable. Sensibly, they require R and C as the characters for R1C1 notation regardless of the language. Change code to recognize this difference from Excel.
* Handle Output of ADDRESS Function
One other function has to deal with R1C1 format as a string. Unlike INDIRECT, which receives the string on input, ADDRESS generates the string on output. Ensure that the ADDRESS output is consistent with the INDIRECT input.
ADDRESS expects its 4th arg to be bool, but it can also accept int, and many examples on the net supply it as an int. This had not been handled properly, but is now corrected.
* More Structured Test
I earlier introduced a new test for relative R1C1 addressing. Rewrite it to be clearer.
* Add Row for This to Locale Spreadsheet
It took a while for me to figure out how it all works. I have added a new row (with English value `*RC`) to Translations.xlsx, in the "Lookup and Reference" section of sheet "Excel Functions". By starting the "function name" with an asterisk, it will not be confused with a "real" function (confirmed by a new test). This approach also gives us the flexibility to do something similar if another surprise case occurs in future; in particular, I think this is more flexible than adding this as another option on the "Excel Localisation" sheet. It also means that any errors or omissions in the list below will be handled as with any other translation problem, by updating the spreadsheet without needing to touch any code.
The spreadsheet has the following entries in the *RC row:
- first letter of ROW/COLUMN functions for da, de, es, fi, fr, hu, nl, nb, pt, pt_br, sv
- no value for locales where ROW/COLUMN functions start with same letter - bg, ru, tr
- no value for locales with a multi-part name for ROW and/or COLUMN - it, pl (I had not previously noted Italian as an exception)
- no value for locales where ROW and/or COLUMN starts with a non-ASCII character - cs (this would also apply to bg and ru which are already included under "same letter")
- it does nothing for locales which are defined on the "Excel Localisation" sheet but have no entries yet on the "Excel Functions" sheet (e.g. eu)
Note that all but the first bullet item will continue to use R/C, which leaves them no worse off than they were before this change.
See issue #2172. The locale files are regenerated whenever the test suite is run.
The use of PHP_EOL in LocaleGenerator.php is awkward on Windows systems, since it causes git to think the file has changed. Change to use `"\n"` instead.
oleibman
Adrien Crivelli
oleibman
MarkBaker