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/NestingLevel: should nested structures be skipped? #762

Open
4 tasks done
rodrigoprimo opened this issue Dec 9, 2024 · 1 comment
Open
4 tasks done

Generic/NestingLevel: should nested structures be skipped? #762

rodrigoprimo opened this issue Dec 9, 2024 · 1 comment

Comments

@rodrigoprimo
Copy link
Contributor

Describe the bug

Similar to #726, I wonder if the Generic.Metrics.NestingLevel should skip nested structures. Currently, it counts the nesting level of inner functions to the "outer" function.

Code sample

function hasNestedFunction() {
    function nestingSix() {
        if ( $condition ) {
            echo 'hi';
            switch ( $condition ) {
                case '1':
                    if ( $condition === '1' ) {
                        if ( $cond ) {
                            foreach ( $conds as $cond ) {
                                echo 'hi';
                            }
                        }
                    }
                    break;
            }
        }
    }
}

To reproduce

Steps to reproduce the behavior:

  1. Create a file called test.php with the code sample above.
  2. Run phpcs --standard=Generic --sniffs=Generic.Metrics.NestingLevel test.php
  3. See error message displayed
--------------------------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 2 WARNINGS AFFECTING 2 LINES
--------------------------------------------------------------------------------------------------------------------------------
 3 | WARNING | Function's nesting level (7) exceeds 5; consider refactoring the function (Generic.Metrics.NestingLevel.TooHigh)
 4 | WARNING | Function's nesting level (6) exceeds 5; consider refactoring the function (Generic.Metrics.NestingLevel.TooHigh)
--------------------------------------------------------------------------------------------------------------------------------

Expected behavior

Potentially the sniff should not display an error on line 3.

Versions (please complete the following information)

Operating System not relevant
PHP version not relevant
PHP_CodeSniffer version master
Standard Generic
Install type not relevant

Please confirm

  • I have searched the issue list and am not opening a duplicate issue.
  • I have read the Contribution Guidelines and this is not a support question.
  • 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

jrfnl commented Dec 10, 2024

I think that makes sense, but only for "closed" scope structures, like nested named functions, not for arrow functions, which are "open".

As for closures and anonymous classes, I'd like to hear some more opinions on whether those should be skipped over or not.

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

No branches or pull requests

2 participants