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

Disabling Else / Elseif #68

Closed
JBx0 opened this issue May 10, 2021 · 5 comments · Fixed by #257
Closed

Disabling Else / Elseif #68

JBx0 opened this issue May 10, 2021 · 5 comments · Fixed by #257
Labels
enhancement New feature or request

Comments

@JBx0
Copy link

JBx0 commented May 10, 2021

Hey hey,
I would love to enable else/elseif in my codebase.

How would one go and do that, I tried all kinds of wildcard strings whitespace etc etc with your package but it didn't pick up any else & elseifs

I managed to detect elseifs with the banned code extension like this:

            -
                type: Stmt_ElseIf
                functions: null

Tried it with:

 disallowedMethodCalls:
            -
                method: 'PhpParser\Node\Stm\Else::__construct'
                message: 'use earlier return types and/or smaller methods'
            -
                method: 'PhpParser\Node\Stm\ElseIf::___construct'
                message: 'use earlier return types and/or smaller methods'
                
    disallowedFunctionCalls:
        -
            function: 'else*(*)*'
            message: 'use logger instead'
        -
            function: 'ElseIf*'
            message: 'use logger instead'
        -
            function: 'elseif*'
            message: 'use logger instead'
        -
            function: 'elseIf '
            message: 'use logger instead'
        -
            function: 'ElseIf()'
            message: 'use logger instead'
        -
            function: 'ElseIf ()'
            message: 'use logger instead'                
@spaze
Copy link
Owner

spaze commented May 11, 2021

Hi, language constructs like if, else, elseif and also foreach, return etc. are currently not supported.

I have no plan to add those, at least for now.

@JBx0
Copy link
Author

JBx0 commented May 11, 2021

Would you be open to a pull request regarding the functionality? @spaze

@spaze
Copy link
Owner

spaze commented May 11, 2021

I'm generally open for PRs and welcome them but in this case I think the PR would be too big for a feature that would be rarely used. Disallowing only elses and elseifs alone wouldn't make much sense so probably all the control structures would need to be supported. And there's quite a few of them https://www.php.net/manual/en/language.control-structures.php 😜

I'd say you'd be better off with a custom PHPStan extension, if you're not willing to use the banned code extension. Building a custom extension might be easier than it seems. For example look at the following class
https://github.com/spaze/phpstan-disallowed-calls/blob/master/src/FunctionCalls.php
getNodeType() would need to return ElseIf_ and processNode would return ['nope'].

Then you need to register the extension, similar (a bit easier) to

https://github.com/spaze/phpstan-disallowed-calls/blob/b99318e15a968c58148cc69940f78412fa421efe/extension.neon#L81-84

Let me know if you need help.

@spaze
Copy link
Owner

spaze commented Mar 2, 2024

Hey @JBx0, now as the extension has matured a bit, I was wondering whether adding support for if and else and more language constructs would be easier than before and there's only one way to find out 😀 But I was wondering whether it would still be useful for you if I add the support?

@spaze spaze reopened this May 4, 2024
spaze added a commit that referenced this issue May 4, 2024
Checking params inside ( ... ) doesn't work at the moment, so you can disallow all `declare()`s but can't re-allow e.g. `declare(strict-types = 1)`.

Close #68
spaze added a commit that referenced this issue May 4, 2024
Checking params inside ( ... ) doesn't work at the moment, so you can disallow all `declare()`s but can't re-allow e.g. `declare(strict-types = 1)`.

Close #68
@spaze spaze closed this as completed in #257 May 4, 2024
spaze added a commit that referenced this issue May 4, 2024
Checking params inside ( ... ) doesn't work at the moment, so you can disallow all `declare()`s but can't re-allow e.g. `declare(strict-types = 1)`.

If you try to disallow `else if` with the space, an exception will be thrown, because `else if` is parsed as `else` followed by `if`, so disallowing `else if` with the space wouldn't have the desired effect and the result would be unexpected.

Close #68
@spaze
Copy link
Owner

spaze commented May 4, 2024

Added basic support to disallow control structures in #257.

@spaze spaze added the enhancement New feature or request label May 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants