Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix crash due to unresolvable constant. #7490

Merged

Conversation

AndrolGenhald
Copy link
Collaborator

@AndrolGenhald AndrolGenhald commented Jan 25, 2022

#7123 introduced a crash whenever the new UnresolvableConstant issue is supposed to be added. It wasn't caught by tests because the tests end as soon as the first issue is raised, and the issue is raised before it crashes.

Not the prettiest fix, but at least it works. I'm not super happy with it, but I can't think of a better way at the moment.

@AndrolGenhald
Copy link
Collaborator Author

Oh, I forgot trailing commas are still disallowed in argument lists until PHP 8.0 🙁

@AndrolGenhald
Copy link
Collaborator Author

AndrolGenhald commented Jan 25, 2022

Actually maybe it'd be better to pass an optional CodeLocation into expandUnion and raise the issue there? The reason I added the exception is because expandUnion doesn't have access to the CodeLocation.
Edit: or maybe not, that would require passing in suppressed issues as well.

@orklah orklah added the release:fix The PR will be included in 'Fixes' section of the release notes label Jan 26, 2022
@orklah orklah merged commit e5a7e00 into vimeo:master Jan 26, 2022
@orklah
Copy link
Collaborator

orklah commented Jan 26, 2022

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:fix The PR will be included in 'Fixes' section of the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants