Skip to content

Silence deprecations / PHP 8.4 compatibility (non-breaking)#33

Open
redcuillin wants to merge 1 commit into
scriptotek:mainfrom
redcuillin:fix/php-deprecations-8x
Open

Silence deprecations / PHP 8.4 compatibility (non-breaking)#33
redcuillin wants to merge 1 commit into
scriptotek:mainfrom
redcuillin:fix/php-deprecations-8x

Conversation

@redcuillin
Copy link
Copy Markdown

Add type hinting and return types for PHP 8.4 compatibility. Update .gitignore to include .phpunit.result.cache.
Silences deprecation warnings if using PHP8.4 without breaking compatibility with earlier versions.

…roved PHP 8.4 compatibility. Update .gitignore to include .phpunit.result.cache.
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented May 1, 2026

❌ The last analysis has failed.

See analysis details on SonarQube Cloud

@redcuillin
Copy link
Copy Markdown
Author

Further to request in #32

@redcuillin redcuillin mentioned this pull request May 2, 2026
Copy link
Copy Markdown
Contributor

@rudolfbyker rudolfbyker left a comment

Choose a reason for hiding this comment

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

LGTM overall, but made one suggestion for improvement.

trait SerializableField
{
public function jsonSerialize(): array|string
public function jsonSerialize(): mixed
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is technically correct, but loses useful information. Suggestion: Add the missing information via PHPDoc, like this:

/**
 * @returns array|string
 */
public function jsonSerialize(): mixed

The same applies to many other places in this PR.

Comment on lines +42 to +46
$source = simplexml_load_string($data, 'SimpleXMLElement', 0, $ns, $isPrefix);
if (false === $source) {
throw new XmlException(libxml_get_errors());
}
$this->source = $source;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Good catch!

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