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

Allow print_r(..., true) #1

Closed
samuelszabo opened this issue Jan 30, 2020 · 4 comments · Fixed by #4
Closed

Allow print_r(..., true) #1

samuelszabo opened this issue Jan 30, 2020 · 4 comments · Fixed by #4

Comments

@samuelszabo
Copy link

Hello, would you mind to allow print_r() with $return = true argument?

We are using it quite often for a better format in logs.

I mean something like (print_r() specific):

services:
    -
        class: Spaze\PHPStan\Rules\Disallowed\FunctionCalls
        tags:
            - phpstan.rules.rule
        arguments:
            forbiddenCalls:
                -
                    function: 'print_r()'
                    allowReturn: true    #configurable
                    message: 'use logger instead'

https://github.com/spaze/phpstan-disallowed-calls/blob/master/src/FunctionCalls.php#L61 :

 if ($name === $forbiddenCall['function']) {
                $returnArg = $node->args[1] ? $node->args[1]->value->name->__toString() === 'true' : false;
                if ($returnArg === true && $forbiddenCall['allowReturn']) {
                    return [];
                }
}

Thanks.

@spaze
Copy link
Owner

spaze commented Feb 18, 2020

Hi, thanks. I'd love to allow some "forbidden calls" with specified parameters but I'd like it to be more generic and configurable.

For example allowReturn only makes sense with print_r() and var_export() but not with var_dump() for example.

What I do now, until support for params is added, I just simply ignore the error:

ignoreErrors:
	-
		message: '#^Calling print_r\(\) is forbidden, use logger instead#'  # Used with $return = true
		paths:
			- file.php
			- foo.php

To not have to ignore the error in many files, a logger that would accept any type and print_r(..., true)-it before actually logging it could also be used.

@spaze
Copy link
Owner

spaze commented Feb 26, 2020

Thinking out loud here (may server as a note to self or to anyone willing to implement it) but something this is what I'd like to see:

The allowedParams here, with type and/or value keys (both may be extraneous, so maybe type or value, or maybe neither, optionally, would need to think about it a bit more):

        arguments:
            forbiddenCalls:
                -
                    function: 'print_r()'
                    message: 'use logger instead'
                    allowedParams:
                      2:
                        type: boolean
                        value: true

@spaze
Copy link
Owner

spaze commented Aug 25, 2020

I've added allowParams support in #4, let me know if it works for you (it does for me), thanks!

@samuelszabo
Copy link
Author

Works perfect for me. Thanks.

spaze added a commit that referenced this issue Nov 28, 2023
Which throw:
```
 68     Parameter #1 $identifiers of method Spaze\PHPStan\Rules\Disallowed\Formatter\Formatter::formatIdentifier() expects list<string>, non-empty-array given.
```

And the `non-empty-list` is just because that's what it is.
spaze added a commit that referenced this issue Feb 20, 2024
This is a new bleeding edge rule added in PHPStan 1.10.59 which resulted in "Parameter #1 $array (non-empty-list<string>) of array_values is already a list, call has no effect."
spaze added a commit that referenced this issue Feb 20, 2024
This is a new bleeding edge rule added in PHPStan 1.10.59 which resulted in "Parameter #1 $array (non-empty-list<string>) of array_values is already a list, call has no effect."
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

Successfully merging a pull request may close this issue.

2 participants