`PEAR.Functions.ValidDefaultValue`: doesn't catch PHP 8.1 deprecation

[davidrans]

Describe the bug This sniff ignores optional parameters with a default value of null if they have a typehint, so it doesn't catch an issue that throws an exception in PHP 8.1

If I comment out the following lines from the sniff it works: https://github.com/squizlabs/PHP_CodeSniffer/blob/ed8e00df0a83aa96acf703f8c2979ff33341f879/src/Standards/PEAR/Sniffs/Functions/ValidDefaultValueSniff.php#L58-L63

Code sample This function should throw a PHPCS error, but doesn't:

public function apply(callable $errorCallback = null,  bool $showProgress): bool {                                 
   ...
}

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 NO error message displayed
4. But PHP throws the following deprecation warning:

   Deprecated: Optional parameter $errorCallback declared before required parameter $showProgress is implicitly treated as a required parameter

Expected behavior I would expect the ValidDefaultValue.NotAtEnd sniff to catch this error.

Versions (please complete the following information):

• OS: [e.g., Windows 10, MacOS 10.15]
• PHP: [8.0, 8.1]
• PHPCS: 3.7.2
• Standard: Custom

[jrfnl]

Eh... might just be me, but that code does NOT throw a deprecation in PHP 8.x.

It only does so when you remove the type declaration: https://3v4l.org/cQjUj

[davidrans]

Hmm, I'm not able to reproduce it in that environment either, though I was able to confirm that only passing the second param is an error in PHP 8.1 (and not in PHP 8.0): https://3v4l.org/dLoTl#v8.1.17

[jrfnl]

@davidrans I'm not sure what action is expected here. Can this issue be closed ?

[davidrans]

I believe the code I referenced in my issue is incorrect: https://github.com/squizlabs/PHP_CodeSniffer/blob/ed8e00df0a83aa96acf703f8c2979ff33341f879/src/Standards/PEAR/Sniffs/Functions/ValidDefaultValueSniff.php#L58-L63

A type-hinted parameter with a default value of NULL should be treated as an optional parameter.

[jrfnl]

A type-hinted parameter with a default value of NULL should be treated as an optional parameter.

Only when the type is not a Class/Interface etc name.

Parameters with Class/Interface name type declarations are made nullable by the null default. It doesn't necessarily make them optional.

As of PHP 8.0.0, declaring mandatory arguments after optional arguments is deprecated. This can generally be resolved by dropping the default value, since it will never be used. One exception to this rule are arguments of the form Type $param = null, where the null default makes the type implicitly nullable. This usage remains allowed, though it is recommended to use an explicit nullable type instead.

Ref: https://www.php.net/manual/en/functions.arguments.php (above example #10)

[davidrans]

Ah, so maybe this check just needs to get more sophisticated about checking the kind of typehint?

And maybe augmented with a related check that enforces the recommended nullable type syntax?