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

Tokenizer: improve tokenization of T_NULLABLE vs T_INLINE_THEN #2794

Conversation

jrfnl
Copy link
Contributor

@jrfnl jrfnl commented Jan 2, 2020

This is an attempt to make the differentiation between T_NULLABLE and T_INLINE_THEN more stable.

There is only a limited set of tokens which can be used for type declarations. If a ? is not followed by one of these, we can be certain that it is a ternary operator T_INLINE_THEN.

The more resource intensive "walking back" check will now only be done when it could be either.
The "walking back" token logic could probably still do with an additional check for T_INSTANCEOF, but the changes now made, should prevent the majority of issues where T_INLINE_THEN is misidentified as T_NULLABLE.

Includes unit tests via the PSR12.Functions.NullableTypeDeclaration sniff.

Fixes #2791

Note: the build failure is unrelated to this PR and will be fixed via #2793

This is an attempt to make the differentiation between T_NULLABLE and T_INLINE_THEN more stable.

There is only a limited set of tokens which can be used for type declarations. If a `?` is not followed by one of these, we can be certain that it is a ternary operator `T_INLINE_THEN`.

The more resource intensive "walking back" check will now only be done when it could be either.
The "walking back" token logic could probably still do with an additional check for `T_INSTANCEOF`, but the changes now made, should prevent the majority of issues where `T_INLINE_THEN` is misidentified as `T_NULLABLE`.

Includes unit tests via the PSR12.Functions.NullableTypeDeclaration sniff.
@jrfnl jrfnl force-pushed the feature/2791-tokenizer-issue-static-vs-typed-properties branch from d70407d to 603d9c6 Compare January 5, 2020 20:59
@jrfnl
Copy link
Contributor Author

jrfnl commented Jan 5, 2020

Rebased to show the build passes.

@gsherwood gsherwood added this to the 3.5.4 milestone Jan 5, 2020
gsherwood added a commit that referenced this pull request Jan 5, 2020
@gsherwood gsherwood merged commit 603d9c6 into squizlabs:master Jan 5, 2020
@gsherwood
Copy link
Member

There is only a limited set of tokens which can be used for type declarations. If a ? is not followed by one of these, we can be certain that it is a ternary operator T_INLINE_THEN.

Thanks a lot. This is a much better idea than how I originally implemented it.

@jrfnl jrfnl deleted the feature/2791-tokenizer-issue-static-vs-typed-properties branch January 5, 2020 22:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PSR12.Functions.NullableTypeDeclaration false positive when ternary operator used with instanceof
2 participants