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

Generic/InlineControlStructure: stop listening for T_SWITCH #595

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Commits on Oct 25, 2024

  1. Generic/InlineControlStructure: stop listening for T_SWITCH tokens

    There is no inline version of a `switch` in PHP and JS so there is no
    reason for this sniff to listen to `T_SWITCH` tokens. Before this
    change, the sniff would bail early on the line below as a switch always
    has a scope opener, so this change should not impact in any way the
    behavior of the sniff.
    
    https://github.com/PHPCSStandards/PHP_CodeSniffer/blob/9a0c2546ea2fa7aac19881da7b655cc5f022bc10/src/Standards/Generic/Sniffs/ControlStructures/InlineControlStructureSniff.php#L71
    
    The InlineControlStructure sniff started listening for `T_SWITCH` tokens
    since the commit that added it to PHPCS:
    
    PHPCSStandards@ad96eb#diff-e1ea4eabd79d6324057bbf726a27074250478f87d92c11a3725a97f0afbd5513R50
    
    Some of the sniff tests using the `T_SWITCH` token were added to
    protect against regressions for changes in the tokenizer. For those,
    dedicated tokenizer tests will be created in subsequent commits. Those
    commits will also update the related sniff tests to use different
    control structures other than `T_SWITCH` when appropriate.
    
    The modified JS test lost part of its value over time as now the sniff
    bails early if there is no opening parenthesis after the control
    structure. Ideally, it should be moved to a tokenizer test, but since
    there are no tests for the JS tokenizer and support will be dropped in
    PHPCS 4.0, I opted to simply use a control structure other than
    T_SWITCH. This test was originally added in b3f4f83.
    
    References:
    
    - PHP: https://www.php.net/manual/en/control-structures.switch.php
    - JS: https://tc39.es/ecma262/multipage/ecmascript-language-statements-and-declarations.html
    rodrigoprimo committed Oct 25, 2024
    Configuration menu
    Copy the full SHA
    0bfa404 View commit details
    Browse the repository at this point in the history

Commits on Oct 28, 2024

  1. Tests/Tokenizer: tests to ensure scope is set correctly for T_SWITCH

    This commit copies, with some non-structural modifications, a sniff
    test to the tokenizer tests. The original test was added in b24b96b,
    part of squizlabs/PHP_CodeSniffer#497. b24b96b
    introduced a test to InlineControlStructureUnitTest.php as there were no
    Tokenizer tests back then. It was added to ensure that
    Tokenizer::recurseScopeMap() would correctly add scope information to
    the `T_SWITCH` token when handling a `switch` that uses the alternative
    syntax.
    
    This commit also adds a test to ensure the scope is correctly set when
    using the normal switch syntax. This was not part of b24b96b, but it is
    relevant anyway, as no other tokenizer test covers this part.
    
    Since the InlineControlStructure sniff doesn't listen for the `T_SWITCH`
    token anymore (see baa4f65), the original test added in b24b96b became
    useless and thus was refactored to use a different control structure.
    The sniff test was kept as testing code that uses a control structure,
    and opening and closing PHP tags is an interesting case not covered in
    other tests.
    rodrigoprimo committed Oct 28, 2024
    Configuration menu
    Copy the full SHA
    3a19186 View commit details
    Browse the repository at this point in the history
  2. Tests/Tokenizer: tests for T_CASE and T_IF scope condition

    This commit copies, with some non-structural modifications, a sniff test
    to the tokenizer tests. Two new tokenizer tests are created as the
    result of this copy. One to ensure that the scope of `T_CASE` is
    correctly set when the case shares the scope closer with `T_SWITCH`
    (no `T_BREAK`). And another to ensure that the scope of `T_IF` is
    correctly set when there is a nested `T_SWITCH` and `T_CASE` without a
    `T_BREAK`.
    
    The original sniff test was added in fddc61a, which is part of
    squizlabs/PHP_CodeSniffer#497. fddc61a
    introduced a test to InlineControlStructureUnitTest.php as there were no
    Tokenizer tests back then. The test was added to ensure that
    Tokenizer::recurseScopeMap() correctly adds scope information to the
    `T_CASE` token when it shares the closer with `T_SWITCH` and to `T_IF`
    when a nested `T_CASE` shares the scope closer with T_SWITCH`.
    
    Since the InlineControlStructure sniff doesn't listen for the `T_SWITCH`
    token anymore (see baa4f65), the original test added in fddc61a became
    useless and thus was refactored to use a different control structure.
    This new version of the sniff test was kept as testing code that uses
    nested alternative syntax control structures is still valid and is not
    covered in other tests.
    rodrigoprimo committed Oct 28, 2024
    Configuration menu
    Copy the full SHA
    332c959 View commit details
    Browse the repository at this point in the history
  3. Tests/Tokenizer: scope opener is set correctly for T_SWITCH

    This commit copies, with some modifications, a sniff test to the
    tokenizer tests. It ensures that the scope opener is set correctly for
    T_SWITCH when there is a closure in the condition. This safeguards
    against regressions related to 30c618e, which fixed https://github
    .com/squizlabs/PHP_CodeSniffer/issues/543. 30c618e originally added a
    test to InlineControlStructureUnitTest.php as there were no Tokenizer
    tests back then.
    
    Since the InlineControlStructure sniff doesn't listen for the `T_SWITCH`
    token anymore (see baa4f65), the original test added in 30c618e became
    useless and thus was refactored to use a different control structure.
    This new version of the sniff test was kept as testing code that uses
    a closure in the condition is still an interesting case and is not
    covered in other tests.
    rodrigoprimo committed Oct 28, 2024
    Configuration menu
    Copy the full SHA
    7574c4a View commit details
    Browse the repository at this point in the history
  4. Tests/Tokenizer: expand recurseScopeMap() tests for the case keyword

    This commit expands the existing `Tokenizer::recurseScopeMap()` tests
    for the `case` keyword when used in a switch statement to check the
    value of the `scope_condition`, `scope_opener` and `scope_closer`
    indexes besides checking if they are set. This is needed for a new
    test case that will be added in a subsequent commit.
    rodrigoprimo committed Oct 28, 2024
    Configuration menu
    Copy the full SHA
    b566b1c View commit details
    Browse the repository at this point in the history
  5. Tests/Tokenizer: test to ensure scope closer is set correctly for T_CASE

    This commit copies a sniff test from InlineControlStructureUnitTest.php
    to the `Tokenizer::recurseScopeMap()` tests for the case keyword. This
    test was added in b498dbe before the Tokenizer tests were created. It
    ensures that the scope for the `T_CASE` token is correctly set when
    there is an if/elseif/else with and without braces inside it.
    
    Note that this test currently does not fail even when the changes to the
    tokenizer applied in b498dbe are reverted. I'm assuming that a
    subsequent commit further improved the tokenizer and made the changes
    from b498dbe redundant, but I was not able to find the specific commit
    using `git bissect`. That being said, I was able to confirm that before
    b498dbe, the scope closer for the `T_CASE` token was incorrectly set to
    `T_RETURN` instead of `T_BREAK`.
    
    The original sniff test was kept without any modification as testing
    if/elseif/else with and without curly braces inside another control
    structure is still valid.
    rodrigoprimo committed Oct 28, 2024
    Configuration menu
    Copy the full SHA
    a7cd997 View commit details
    Browse the repository at this point in the history
  6. Tests/Tokenizer: test related to setting the scope for T_CASE

    This commit copies a sniff test from InlineControlStructureUnitTest.php
    to the Tokenizer::recurseScopeMap() tests for the `case` keyword. The
    copied test was added in 65ef053 before the Tokenizer tests were
    created. It ensures that the scope for the `T_CASE` token is correctly
    set when there is an inline control structure inside it with more than
    three lines.
    
    The original sniff test was modified to use `T_FOR` instead of
    `T_SWITCH`, but its purpose was not altered. The test is about adding
    braces to an `if` that returns a nested function call.
    rodrigoprimo committed Oct 28, 2024
    Configuration menu
    Copy the full SHA
    cbfa258 View commit details
    Browse the repository at this point in the history