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/scope mapping: scope owner not correctly set with heredoc in condition #149

Open
3 tasks done
jrfnl opened this issue Dec 10, 2023 · 1 comment · May be fixed by #295
Open
3 tasks done

Tokenizer/scope mapping: scope owner not correctly set with heredoc in condition #149

jrfnl opened this issue Dec 10, 2023 · 1 comment · May be fixed by #295

Comments

@jrfnl
Copy link
Member

jrfnl commented Dec 10, 2023

Describe the bug

The 'scope_condition' key doesn't always get set correctly in the $tokens array.
Consequently, tokens within the curlies do not get the 'conditions' array set correctly either.

Both things can trip sniffs up.

Code sample

// Correct: the curlies get assigned the "if" as "scope condition".
if (true) {
    return;
}

// Bug: the curlies do NOT get a "scope condition" assigned, while it should be the "if".
if (foo(<<<EOD
foobar!
EOD
)) {
    return;
}

Token debug output

Ptr | Ln  | Col  | Cond | ( #) | Token Type                 | [len]: Content
--------------------------------------------------------------------------
  0 | L01 | C  1 | CC 0 | ( 0) | T_OPEN_TAG                 | [  5]: <?php

  1 | L02 | C  1 | CC 0 | ( 0) | T_WHITESPACE               | [  0]:

  2 | L03 | C  1 | CC 0 | ( 0) | T_IF                       | [  2]: if
  3 | L03 | C  3 | CC 0 | ( 0) | T_WHITESPACE               | [  1]: ⸱
  4 | L03 | C  4 | CC 0 | ( 0) | T_OPEN_PARENTHESIS         | [  1]: (
  5 | L03 | C  5 | CC 0 | ( 1) | T_TRUE                     | [  4]: true
  6 | L03 | C  9 | CC 0 | ( 0) | T_CLOSE_PARENTHESIS        | [  1]: )
  7 | L03 | C 10 | CC 0 | ( 0) | T_WHITESPACE               | [  1]: ⸱
  8 | L03 | C 11 | CC 0 | ( 0) | T_OPEN_CURLY_BRACKET       | [  1]: {
  9 | L03 | C 12 | CC 1 | ( 0) | T_WHITESPACE               | [  0]:

 10 | L04 | C  1 | CC 1 | ( 0) | T_WHITESPACE               | [  4]: ⸱⸱⸱⸱
 11 | L04 | C  5 | CC 1 | ( 0) | T_RETURN                   | [  6]: return
 12 | L04 | C 11 | CC 1 | ( 0) | T_SEMICOLON                | [  1]: ;
 13 | L04 | C 12 | CC 1 | ( 0) | T_WHITESPACE               | [  0]:

 14 | L05 | C  1 | CC 0 | ( 0) | T_CLOSE_CURLY_BRACKET      | [  1]: }
 15 | L05 | C  2 | CC 0 | ( 0) | T_WHITESPACE               | [  0]:

 16 | L06 | C  1 | CC 0 | ( 0) | T_WHITESPACE               | [  0]:

 17 | L07 | C  1 | CC 0 | ( 0) | T_IF                       | [  2]: if
 18 | L07 | C  3 | CC 0 | ( 0) | T_WHITESPACE               | [  1]: ⸱
 19 | L07 | C  4 | CC 0 | ( 0) | T_OPEN_PARENTHESIS         | [  1]: (
 20 | L07 | C  5 | CC 0 | ( 1) | T_STRING                   | [  3]: foo
 21 | L07 | C  8 | CC 0 | ( 1) | T_OPEN_PARENTHESIS         | [  1]: (
 22 | L07 | C  9 | CC 0 | ( 2) | T_START_HEREDOC            | [  6]: <<<EOD

 23 | L08 | C  1 | CC 1 | ( 2) | T_HEREDOC                  | [  7]: foobar!

 24 | L09 | C  1 | CC 0 | ( 2) | T_END_HEREDOC              | [  3]: EOD
 25 | L09 | C  4 | CC 0 | ( 2) | T_WHITESPACE               | [  0]:

 26 | L10 | C  1 | CC 0 | ( 1) | T_CLOSE_PARENTHESIS        | [  1]: )
 27 | L10 | C  2 | CC 0 | ( 0) | T_CLOSE_PARENTHESIS        | [  1]: )
 28 | L10 | C  3 | CC 0 | ( 0) | T_WHITESPACE               | [  1]: ⸱
 29 | L10 | C  4 | CC 0 | ( 0) | T_OPEN_CURLY_BRACKET       | [  1]: {
 30 | L10 | C  5 | CC 0 | ( 0) | T_WHITESPACE               | [  0]:

 31 | L11 | C  1 | CC 0 | ( 0) | T_WHITESPACE               | [  4]: ⸱⸱⸱⸱
 32 | L11 | C  5 | CC 0 | ( 0) | T_RETURN                   | [  6]: return
 33 | L11 | C 11 | CC 0 | ( 0) | T_SEMICOLON                | [  1]: ;
 34 | L11 | C 12 | CC 0 | ( 0) | T_WHITESPACE               | [  0]:

 35 | L12 | C  1 | CC 0 | ( 0) | T_CLOSE_CURLY_BRACKET      | [  1]: }
 36 | L12 | C  2 | CC 0 | ( 0) | T_WHITESPACE               | [  0]:

As can be seen here, token 9 - 13 correctly have the scope conditions set, while token 30 - 34 do not.

Expected behavior

That the conditions get set correctly in all cases ;-)

Versions (please complete the following information)

Operating System not relevant
PHP version 8.2.13, though shouldn't be relevant
PHP_CodeSniffer version master
Standard not relevant
Install type git clone

Please confirm:

  • I have searched the issue list and am not opening a duplicate issue.
  • I confirm that this bug is a bug in PHP_CodeSniffer and not in one of the external standards.
  • I have verified the issue still exists in the master branch of PHP_CodeSniffer.
@jrfnl
Copy link
Member Author

jrfnl commented Mar 20, 2024

@fredden There is a second similar report in the PHPCS repo - could you please check if your PR handles that code sample correctly too ?

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.

1 participant