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

match() nested within if() statement breaks setting of "conditions" key #3750

Open
momala454 opened this issue Feb 3, 2023 · 4 comments
Open

Comments

@momala454
Copy link

momala454 commented Feb 3, 2023

Describe the bug
The rule SlevomatCodingStandard.ControlStructures.EarlyExit will cause the following error at line 1 (but there is nothing on line 1), and there shouldn't be any error, the code is valid and doesn't contains any if without a curly brace

FOUND 1 ERROR AFFECTING 1 LINE
------------------------------------------------------------------------------------------------------------------------------------------------
 1 | ERROR | An error occurred during processing; checking has been aborted. The error message was: "if" without curly braces is not supported.
------------------------------------------------------------------------------------------------------------------------------------------------

Code sample

function testBugCurly(): void
{
    foreach ([1,2] as $step) {
        if (1 !== 1) {
            continue;
        }

        if (1 !== null) {
            if (!match (1) {
                default => 1,
            }) {
            }
        }
    }
}

Custom ruleset

<?xml version="1.0"?>
<ruleset xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" name="PSR12" xsi:noNamespaceSchemaLocation="../../../phpcs.xsd">
    <description>The PSR-12 coding standard.</description>
    <arg name="tab-width" value="4"/>

    <!-- Exclude the Composer Vendor directory. -->
	<exclude-pattern>/vendor/*</exclude-pattern>

    
    <!-- CUSTOM -->
    <config name="installed_paths" value="vendor/slevomat/coding-standard"/>

    <rule ref="SlevomatCodingStandard.ControlStructures.EarlyExit">
    </rule>

</ruleset>

Strangely, adding anything (even a comment) on the last line inside foreach avoid the error. Like this :

function testBugCurly(): void
{
    foreach ([1,2] as $step) {
        if (1 !== 1) {
            continue;
        }

        if (1 !== null) {
            if (!match (1) {
                default => 1,
            }) {
            }
        }
        // hello
    }
}

To reproduce
Steps to reproduce the behavior:

  1. Create a file called test.php with the code sample above...
  2. Run phpcs test.php ...
  3. See error message displayed
PHPCS output here

Expected behavior
A clear and concise description of what you expected to happen.

Versions (please complete the following information):

  • OS:windows 10
  • PHP: 8.1
  • PHPCS: PHP_CodeSniffer version 3.7.1 (stable)
  • Standard: see xml above

Additional context
Add any other context about the problem here.

Initially reported there slevomat/coding-standard#1506 . They say :

         It looks like a bug in PHPCS. The `if (!match (1) {` is missing `scope_closer`.

Originally posted by @kukulich in slevomat/coding-standard#1506 (comment)

@jrfnl
Copy link
Contributor

jrfnl commented Feb 3, 2023

Thanks for reporting this @momala454 !

I've been able to confirm the bug. Smallest reproduction example:

if (!match (1) {
	default => 1,
}) {
}

The match correctly gets scope opener/closers and gets added to conditions. The if does not.

@jrfnl
Copy link
Contributor

jrfnl commented Feb 4, 2023

I haven't had a chance to properly debug this yet (and may be a while before I can). Relevant part of the verbose output:

        Start scope map at 2:T_IF => if
        => Begin scope map recursion at token 2 with depth 1
        Process token 3 on line 3 []: T_WHITESPACE =>
        Process token 4 on line 3 []: T_OPEN_PARENTHESIS => (
        Process token 5 on line 3 []: T_BOOLEAN_NOT => !
        Process token 6 on line 3 []: T_MATCH => match
        => Found new opening condition before scope opener for 2:T_IF, backtracking

... and then the backtrace doesn't appear to happen ?

I can see that for closures the debug output from the Tokenizer::recurseScopeMap() method says "handled manually", so maybe for nested match structures it should be too ?

Note: as I said, haven't done proper debugging yet, so I may well be wrong.

@jrfnl
Copy link
Contributor

jrfnl commented Feb 18, 2023

#3763 may be a duplicate of this (unconfirmed), but definitely looks related. Probably a good idea to look at both code samples when addressing either these.

@momala454
Copy link
Author

hello, any updates ?

@jrfnl jrfnl added this to the 3.8.0 milestone Sep 21, 2023
@jrfnl jrfnl changed the title SlevomatCodingStandard.ControlStructures.EarlyExit causes "if" without curly braces is not supported match() nested within if() statement breaks setting of "conditions" key Sep 21, 2023
@jrfnl jrfnl removed this from the 3.8.0 milestone Dec 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants