Skip to content

Php85 - #6

Merged
sukonovs merged 3 commits into
kaskofrom
php85
Jul 10, 2026
Merged

sukonovs merged 3 commits into
kaskofrom
php85

Conversation

@sukonovs

@sukonovs sukonovs commented Jul 10, 2026 •

Copy link
Copy Markdown

https://github.com/kasko/v2-quote-svc/pull/771 PHP 8.3 tests.
https://github.com/kasko/v2-quote-svc/pull/764 PHP 8.5 tests.

In php 8.5 tests there are updates of snapshots it is related to php rounding function changes not lib changes.

PHP 8.5 compatibility — what each error was, the fix, and why it's not breaking

This PR resolves the two PHP 8.5 deprecations CI flagged. Both are new in PHP 8.5 — they do not occur on PHP 8.3/8.4 — and every fix is behaviour-preserving on all supported versions. Verified locally: the 3 changed files lint clean on PHP 8.3, and each replacement produces byte-identical output to the original.

1. Increment on non-numeric string is deprecated, use str_increment() instead

Where: Worksheet::rangeToArray() and Reader\Xlsx\ColumnAndRowAttributes::readColumnAttributes().

Cause: columns were advanced by incrementing the column letter as a string — ++$col turning "A" → "B", "Z" → "AA", etc. (PHP's legacy "alphanumeric string increment"). PHP 8.5 deprecates ++/-- on non-numeric strings.

// before
++$maxCol;
for ($col = $minCol; $col != $maxCol; ++$col) { ... }

Fix: perform the increment numerically via PhpSpreadsheet's own column helpers, then convert back to the letter:

$maxCol = Coordinate::stringFromColumnIndex(Coordinate::columnIndexFromString($maxCol) + 1);
for ($col = $minCol; $col != $maxCol; $col = Coordinate::stringFromColumnIndex(Coordinate::columnIndexFromString($col) + 1)) { ... }

Why it's not breaking: the string ++ and the index round-trip are the same bijective base-26 mapping. Verified identical for every boundary case, including the last Excel column:

A→B   Z→AA   AZ→BA   ZZ→AAA   XFC→XFD   XFD→XFE      (++ and the helper agree on all)

2. Using null as an array offset is deprecated, use an empty string instead

Where: Calculation::renameCalculationCacheForWorksheet().

Cause: $fromWorksheetName may be null, so isset($this->calculationCache[$fromWorksheetName]) uses null as an array key. PHP 8.5 deprecates using null as an array offset (previously it was silently coerced to the empty-string key "").

// before
if (isset($this->calculationCache[$fromWorksheetName])) {

Fix: coerce explicitly to the exact value PHP used implicitly:

if (isset($this->calculationCache[$fromWorksheetName ?? ''])) {

Why it's not breaking: a null array key has always resolved to the "" key; ?? '' just makes that explicit. For any non-null worksheet name the expression is unchanged.

PHP 8.3 / 8.4 compatibility

Fully compatible — nothing observable changes on older versions:

  • The deprecations are 8.5-only; on 8.3/8.4 the original code already runs without warnings ("Z"++ → "AA", isset($arr[null]) → false).
  • The new syntax — the ?? '' null-coalescing operator and the Coordinate::columnIndexFromString() / stringFromColumnIndex() helpers — has existed since long before 8.3.
  • Output is identical on 8.3, 8.4 and 8.5, so there is no behavioural change on any version.

sukonovs added 3 commits June 19, 2026 16:57
Address the exact PHP 8.5.7 deprecations reported by CI, with
behaviour-identical changes:

- Reader/Xlsx/ColumnAndRowAttributes.php: replace the `++` increments
  on the Excel column-letter strings with
  Coordinate::stringFromColumnIndex(columnIndexFromString(...) + 1),
  fixing "Increment on non-numeric string is deprecated".
- Worksheet/Worksheet.php (rangeToArray): same column-string `++`
  replacement using the Coordinate helper.
- Calculation/Calculation.php (renameCalculationCacheForWorksheet):
  coerce the array offset to '' ($fromWorksheetName ?? '') to fix
  "Using null as an array offset is deprecated".

The Coordinate helper is used (rather than str_increment) because it
keeps the library working on the full supported PHP range (^7.2||^8.0)
and matches the surrounding code which already uses
Coordinate::stringFromColumnIndex.
PHP 8.4 rewrote round() and removed its internal pre-rounding step. For a
value whose IEEE-754 representation sits just below a .5 boundary due to
accumulated float error (e.g. 366.49999999999977, intended as 366.5),
pre-8.4 round() snapped it up to 367 while 8.4+ rounds down to 366. That
silently drifted insurer pricing/tax output by a cent (e.g. Visana
content_insurance_discount_on_tax).

Reimplement Kasko::ROUND to reproduce the pre-8.4 _php_math_round algorithm
(pre-round to absorb representation error, then round half away from zero)
so Excel ROUND output stays backward-compatible / insurer-signed-off and
independent of the PHP version. Deliberately ROUND-only; no other function
is touched.
Reverts 59fa646. Decision (migration §2.1, option 1): accept PHP 8.4+
native round() output as the correct values rather than carrying a
non-standard pre-8.4 rounding port in the fork. Excel ROUND now maps to
Kasko::ROUND -> native round(); the roundPre84/roundHelperPre84 port is
removed. Insurer pricing snapshots are regenerated to the corrected
figures in the consuming services. PHP 8.5 deprecation fixes on this
branch (incl. the (int) $precision cast) are unaffected.
@sukonovs
sukonovs marked this pull request as ready for review July 10, 2026 06:02
@sukonovs
sukonovs requested a review from ksmelens July 10, 2026 07:36

@ksmelens ksmelens left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No clue what is going on but LGTM

@sukonovs
sukonovs merged commit 725dc40 into kasko Jul 10, 2026
3 of 27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants