Conversation
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
OptionalToRequiredFunctionParameters: change the offsets from 0-based to 1-based
... to be in line with other sniffs and with the PassedParameters class.
OptionalToRequiredFunctionParameters: bug fix for gmmktime()
From https://github.com/php/php-src/blob/69888c3ff1f2301ead8e37b23ff8481d475e29d2/UPGRADING#L192-L194 :
This commit fixes a bug for the offset being wrong for the
gmmktime()function (introduced in610e79c ).
Note: as the change for
gmmktime()andmktime()is actually "require at least one argument", if we combine that with named parameters, we may need to change how we sniff this as currently, as we'd report when thehourparam is missing, even when one of the other parameters would be passed.OptionalToRequiredFunctionParameters: add support for named parameters
PassedParameters::getParameterFromStack()method.PHP itself renamed a lot of parameters in PHP 8.0. As named parameters did not exist before PHP 8.0, the parameter name as per PHP 8.0 (or above) is the only relevant name.
Name verification reference:
openssl_seal: https://github.com/php/php-src/blob/9dc42b411433f25d37bb4c03ae1270fef8c0d062/ext/openssl/openssl.stub.php#L182openssl_open: https://github.com/php/php-src/blob/9dc42b411433f25d37bb4c03ae1270fef8c0d062/ext/openssl/openssl.stub.php#L188Includes adding/adjusting the unit tests to include tests using named parameters.
Side-note: The sniff is based on the premise of positional parameters and does not report on missing required parameters which were always required. This has not changed, even though with named parameters, the chance of that happening has increased.
Related to #1239