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.

Sign up for GitHub

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

Jump to bottom

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
Labels
Core Component: Tokenizer Type: bug

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.
This was referenced Dec 13, 2023
Closed
Open
@fredden fredden linked a pull request Jan 20, 2024 that will close this issue
Draft
6 tasks
@jrfnl jrfnl mentioned this issue Mar 20, 2024
Open
3 tasks
@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
Assignees
No one assigned
Labels
Core Component: Tokenizer Type: bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix tokenizer bug - Heredoc in if condition broke scope mapping fredden/PHP_CodeSniffer
1 participant
@jrfnl