Skip to content

[Php85] Handle crash on no arg on OrdSingleByteRector#7682

Merged
samsonasik merged 2 commits into
mainfrom
crash-no-arg
Nov 29, 2025
Merged

[Php85] Handle crash on no arg on OrdSingleByteRector#7682
samsonasik merged 2 commits into
mainfrom
crash-no-arg

Conversation

@samsonasik
Copy link
Copy Markdown
Member

@samsonasik
Copy link
Copy Markdown
Member Author

All checks have passed 🎉 @TomasVotruba I am merging it ;)

@samsonasik samsonasik enabled auto-merge (squash) November 29, 2025 01:55
@samsonasik samsonasik merged commit 9f4509d into main Nov 29, 2025
55 checks passed
@samsonasik samsonasik deleted the crash-no-arg branch November 29, 2025 01:55
$args = $node->getArgs();
if (! isset($args[0])) {
return null;
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is not possible, as one arg is required: https://www.php.net/manual/en/function.ord.php

Copy link
Copy Markdown
Member Author

@samsonasik samsonasik Nov 29, 2025

Choose a reason for hiding this comment

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

it is not error on older php version, eg: php 7.4, which only warning, but not error, the crash will on error because try to get ->value property on null.

Upgrade to php 8+ require runtime check on app level instead, not rector.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I've read the other comment 👌

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 3, 2026

This pull request has been automatically locked because it has been closed for 150 days. Please open a new PR if you want to continue the work.

@github-actions github-actions Bot locked as resolved and limited conversation to collaborators May 3, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants