Skip to content

PHP 8.1 | Tokenizer/PHP: readonly vs union types bug fix#3513

Merged
gsherwood merged 1 commit intosquizlabs:masterfrom
jrfnl:feature/tokenizer-php-bugfix-readonly-union-types
Dec 20, 2021
Merged

PHP 8.1 | Tokenizer/PHP: readonly vs union types bug fix#3513
gsherwood merged 1 commit intosquizlabs:masterfrom
jrfnl:feature/tokenizer-php-bugfix-readonly-union-types

Conversation

@jrfnl
Copy link
Copy Markdown
Contributor

@jrfnl jrfnl commented Dec 18, 2021

More follow up on #3480.

Done some more stress testing and found that the retokenization of | to T_TYPE_UNION was broken when combined with the readonly keyword.

Fixed now, includes test.

/cc @kukulich

Done some more stress testing and found that the retokenization of `|` to `T_TYPE_UNION` was broken when combined with the `readonly` keyword.

Fixed now, includes test.
@jrfnl jrfnl changed the title Tokenizer/PHP: readonly vs union types bug fix PHP 8.1 | Tokenizer/PHP: readonly vs union types bug fix Dec 18, 2021
@gsherwood gsherwood added this to the 3.7.0 milestone Dec 19, 2021
@gsherwood gsherwood merged commit a6bc7e4 into squizlabs:master Dec 20, 2021
@gsherwood
Copy link
Copy Markdown
Member

Thanks for fixing this

@jrfnl jrfnl deleted the feature/tokenizer-php-bugfix-readonly-union-types branch December 20, 2021 13:11
@jrfnl
Copy link
Copy Markdown
Contributor Author

jrfnl commented Dec 20, 2021

@gsherwood I'm fairly certain I found one more case where T_READONLY should be added to a condition in the PHP Tokenizer class, but I haven't been able to come up with a failing test case for that one yet. Will keep an eye on it:

https://github.com/jrfnl/PHP_CodeSniffer/blob/d5c1cf755070447305e6e847db0299bc4e4c0190/src/Tokenizers/PHP.php#L1577

@gsherwood
Copy link
Copy Markdown
Member

but I haven't been able to come up with a failing test case for that one yet

I tried a few code snippets but couldn't come up with any failed cases either.

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