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

Incorrect tokenization for ternary operator with match inside of it #3789

Closed
schlndh opened this issue Apr 1, 2023 · 2 comments · Fixed by #3792
Closed

Incorrect tokenization for ternary operator with match inside of it #3789

schlndh opened this issue Apr 1, 2023 · 2 comments · Fixed by #3792
Milestone

Comments

@schlndh
Copy link

schlndh commented Apr 1, 2023

Describe the bug
It seems that normally the ? ... : ... should be tokenized as T_INLINE_THEN ... T_INLINE_ELSE, however if there is a match with default branch inside the "then" part then : remains T_COLON, which can break some sniffs (in my case it's this 3rd party sniff).

Code sample

<?php

$a = $b !== null
	? match ($c) {
		default => 5,
	}
	: null;

This is tokenized as:

    *** START PHP TOKENIZING ***
    Process token [0]: T_OPEN_TAG => <?php\n
    Process token [1]: T_WHITESPACE => \n
    Process token [2]: T_VARIABLE => $a
    Process token [3]: T_WHITESPACE => ·
    Process token  4 : T_EQUAL => =
    Process token [5]: T_WHITESPACE => ·
    Process token [6]: T_VARIABLE => $b
    Process token [7]: T_WHITESPACE => ·
    Process token [8]: T_IS_NOT_IDENTICAL => !==
    Process token [9]: T_WHITESPACE => ·
    Process token [10]: T_STRING => null
    Process token [11]: T_WHITESPACE => \n\t
    Process token  12 : T_NONE => ?
        * token 12 changed from ? to T_INLINE_THEN
    Process token [13]: T_WHITESPACE => ·
    Process token [14]: T_MATCH => match
    Process token [15]: T_WHITESPACE => ·
    Process token  16 : T_OPEN_PARENTHESIS => (
    Process token [17]: T_VARIABLE => $c
    Process token  18 : T_CLOSE_PARENTHESIS => )
    Process token [19]: T_WHITESPACE => ·
    Process token  20 : T_OPEN_CURLY_BRACKET => {
    Process token [21]: T_WHITESPACE => \n\t\t
    Process token [22]: T_DEFAULT => default
        * token 24 changed from T_DOUBLE_ARROW to T_MATCH_ARROW
        * token 22 changed from T_DEFAULT to T_MATCH_DEFAULT
    Process token [23]: T_WHITESPACE => ·
    Process token [24]: T_MATCH_ARROW => =>
    Process token [25]: T_WHITESPACE => ·
    Process token [26]: T_LNUMBER => 5
    Process token  27 : T_COMMA => ,
    Process token [28]: T_WHITESPACE => \n\t
    Process token  29 : T_CLOSE_CURLY_BRACKET => }
    Process token [30]: T_WHITESPACE => \n\t
    Process token  31 : T_COLON => :
        * token is T_CASE or T_DEFAULT opener, not T_INLINE_ELSE
    Process token [32]: T_WHITESPACE => ·
    Process token [33]: T_STRING => null
    Process token  34 : T_SEMICOLON => ;
    Process token [35]: T_WHITESPACE => \n
    *** END PHP TOKENIZING ***

To reproduce
Steps to reproduce the behavior:

  1. Create a file called test.php with the code sample above...
  2. Run phpcs -vvv test.php ...
  3. See the generated tokens

Expected behavior
Tokenize ? ... : ... as T_INLINE_THEN ... T_INLINE_ELSE even with match (...) {default => ...} inside then branch.

Versions (please complete the following information):

  • OS: Linux
  • PHP: 8.2
  • PHPCS: 3.7.2
  • Standard: Any
@jrfnl
Copy link
Contributor

jrfnl commented Apr 2, 2023

@schlndh Thanks for reporting this. I have confirmed the issue and pulled PR #3792 which should fix this. Testing appreciated.

@jrfnl jrfnl added the Type: bug label May 2, 2023
gsherwood added a commit that referenced this issue May 4, 2023
@jrfnl jrfnl added this to the 3.8.0 milestone May 4, 2023
@jrfnl
Copy link
Contributor

jrfnl commented Dec 8, 2023

FYI: the fix for this issue is included in today's PHP_CodeSniffer 3.8.0 release.

As per #3932, development on PHP_CodeSniffer will continue in the PHPCSStandards/PHP_CodeSniffer repository. If you want to stay informed, you may want to start "watching" that repo (or watching releases from that repo).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants