Skip to content

Fixed DocParser lookahead to work in PHP 7.4#284

Closed
davidbyoung wants to merge 1 commit intodoctrine:masterfrom
davidbyoung:parser-fix
Closed

Fixed DocParser lookahead to work in PHP 7.4#284
davidbyoung wants to merge 1 commit intodoctrine:masterfrom
davidbyoung:parser-fix

Conversation

@davidbyoung
Copy link
Copy Markdown

@davidbyoung davidbyoung commented Sep 28, 2019

Fixes #283 by explicitly checking if the lookahead position is null before using it (started becoming a problem in PHP 7.4). Also fixed spl autoload function to have a void return type rather than bool. Removed an error message from the PHPStan config because it is no longer thrown by the fixed DocParser

Comment thread phpstan.neon.dist
Fixed spl autoload function to return void per PHP documentation

Removed unnecessary ignored error message from phpstan config that is no longer thrown because of the fixes to DocParser
@malarzm
Copy link
Copy Markdown
Member

malarzm commented Sep 29, 2019

We should rather merge #276 up as it's doing mostly the same thing and is already in 1.7 branch

@greg0ire
Copy link
Copy Markdown
Member

So we should just merge up? Is this repo using a merge up policy?

@malarzm
Copy link
Copy Markdown
Member

malarzm commented Sep 29, 2019

@greg0ire I assumed it does although I don't know how much master diverged from 1.7

@alcaeus
Copy link
Copy Markdown
Member

alcaeus commented Sep 30, 2019

I'll merge up soon. I wanted to fix the 1.7 vs. 1.8 issue first (some PRs were marked for 1.7 but only merged to master), then solve the issue by merging up to master.

For what it's worth, we use merge-up strategy here as it requires less work (merging up is generally easier and less error-prone than backporting). As such I'm closing this PR, but I'll leave the related issue open until I've done the merge.

@alcaeus alcaeus closed this Sep 30, 2019
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.

4 participants