Skip to content

Conversation

veewee
Copy link
Owner

@veewee veewee commented Oct 15, 2025

Q A
Type improvement
BC Break no
Fixed issues

Summary

Upgrade PHP project to latest versions

  • Remove .phive and tools folder (use vendor/bin instead)
  • Update composer.json:
    • Change PHP dependency: Support 8.4, 8.5
    • Update azjezz/psl to support ~4.0 while maintaining 3.x compatibility
    • Add phpunit/phpunit ~12.3, php-cs-fixer/shim ~3.88, vimeo/psalm ~6.13
    • Add infection/infection ^0.31
    • Update scripts to use ./vendor/bin/ instead of ./tools/
  • Update GitHub workflows to use PHP 8.4 and 8.5 matrix
  • Update phpunit.xml to use vendor/phpunit/phpunit/phpunit.xsd and add failOnWarning attributes
  • Update psalm.xml with findUnusedCode=false and ensureOverrideAttribute=false
  • Update infection.json.dist to remove phpunit.customPath (use default)

Code changes by GitHub Copilot CLI agent.

- Remove .phive and tools folder (use vendor/bin instead)
- Update composer.json:
  - Change PHP dependency: Support 8.3, 8.4, 8.5 (removed 8.2)
  - Update azjezz/psl to support ~4.0 while maintaining 3.x compatibility
  - Add phpunit/phpunit ~12.3, php-cs-fixer/shim ~3.88, vimeo/psalm ~6.13
  - Add infection/infection ^0.31
  - Update scripts to use ./vendor/bin/ instead of ./tools/
- Update GitHub workflows to use PHP 8.3 and 8.4 matrix
- Update phpunit.xml to use vendor/phpunit/phpunit/phpunit.xsd and add failOnWarning attributes
- Update psalm.xml with findUnusedCode=false and ensureOverrideAttribute=false
- Update infection.json.dist to remove phpunit.customPath (use default)

Code changes by GitHub Copilot CLI agent.

Note: DataProvider annotations need to be converted to attributes for PHPUnit 12 compatibility - this will be done in a follow-up.
@veewee veewee requested a review from Copilot October 15, 2025 09:02
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR upgrades the PHP project to support the latest PHP versions (8.4, 8.5) and modernizes the development toolchain by moving from .phive tools to vendor/bin dependencies.

  • Updates PHP compatibility to support 8.4 and 8.5
  • Migrates from PHPUnit docblock annotations to PHP 8+ attributes
  • Replaces .phive tool management with composer dependencies

Reviewed Changes

Copilot reviewed 44 out of 48 changed files in this pull request and generated no comments.

Show a summary per file
File Description
composer.json Updates PHP version constraints, adds dev dependencies, updates scripts to use vendor/bin
.phive/phars.xml Removes .phive configuration (deleted file)
phpunit.xml Updates schema location and adds fail-on-warning attributes
psalm.xml Adds new configuration options and moves tests to ignoreFiles
infection.json.dist Removes custom phpunit path and adds mutation ignores
.github/workflows/*.yaml Updates CI matrix to include PHP 8.5
tests/**/*.php Converts @dataProvider annotations to #[DataProvider] attributes
src/Xml/Xslt/Configurator/security_preferences.php Adds explicit use statements for XSL constants

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@veewee veewee force-pushed the php-85-upgrade-4.x branch from 1454a7e to d7edc91 Compare October 15, 2025 09:05
@veewee
Copy link
Owner Author

veewee commented Oct 15, 2025

@nielsdos, Care to take a look at the failing test? I think this is broken / regression in PHP85.

<?php 

use Dom\XMLDocument;

$xml = XMLDocument::createFromString(<<<EOXML
<?xml version="1.0" encoding="UTF-8"?>
<note
      xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
      xsi:noNamespaceSchemaLocation="note-nonamespace1.xsd http://localhost/note-nonamespace2.xsd"
      xsi:schemaLocation="http://www.happy-helpers1.com note-namespace1.xsd http://www.happy-helpers2.com http://localhost/note-namespace2.xsd">
</note>
EOXML
);
$documentElement = $xml->documentElement;
$attributes = $documentElement->attributes;
$schemaLocation = $attributes->getNamedItemNS('http://www.w3.org/2001/XMLSchema-instance', 'schemaLocation');
var_dump(explode(' ', $schemaLocation->textContent ?? ''));

PHP 8.4 (https://3v4l.org/jjbDc#v8.4.13)

docker run -it --rm --name php84 -v "$PWD":/app -w /app php:8.4 php test.php                                                                                                                                                  

array(4) {
  [0]=>
  string(29) "http://www.happy-helpers1.com"
  [1]=>
  string(19) "note-namespace1.xsd"
  [2]=>
  string(29) "http://www.happy-helpers2.com"
  [3]=>
  string(36) "http://localhost/note-namespace2.xsd"
}

Php 8.5 RC2 (https://3v4l.org/jjbDc/rfc#vgit.master)

docker run -it --rm --name php85 -v "$PWD":/app -w /app php:8.5.0RC2-cli php test.php      
                                                                                                                                   
array(1) {
  [0]=>
  string(0) ""
}

@nielsdos
Copy link

@veewee Yeah, sorry about that, that is a regression. For context: the iterator code I inherited of ext/dom was very messy so I refactored the implementation in 8.5. Looks like this had a regression which you fortunately found (thanks!). I'll try to get this fixed soon.

@nielsdos
Copy link

@veewee I'm getting a bunch of Undefined constant Psl\File\WriteMode::Truncate errors when fixing that bug in php-src. Looks like it's TRUNCATE in caps instead of CamelCase. When making that change locally to your package, all tests pass with my local php-src fix.

@nielsdos
Copy link

Fix up at php/php-src#20185

@veewee
Copy link
Owner Author

veewee commented Oct 16, 2025

Thanks for looking into it!
Looking forward for that fix :)

@nielsdos
Copy link

Thanks for looking into it! Looking forward for that fix :)

Fix got merged to php-src, will be available in RC3 of PHP 8.5, which should release on the 23rd of october

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