Skip to content

OptionalToRequiredFunctionParameters: add support for named parameters + bug fix#1430

Merged
wimg merged 3 commits intodevelopfrom
feature/optionaltorequiredparams-support-named-parameters
Dec 14, 2022
Merged

OptionalToRequiredFunctionParameters: add support for named parameters + bug fix#1430
wimg merged 3 commits intodevelopfrom
feature/optionaltorequiredparams-support-named-parameters

Conversation

@jrfnl
Copy link
Copy Markdown
Member

@jrfnl jrfnl commented Dec 6, 2022

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 :

mktime() and gmmktime() now require at least one argument. time() can be
used to get the current timestamp.

This commit fixes a bug for the offset being wrong for the gmmktime() function (introduced in
610e79c ).

Note: as the change for gmmktime() and mktime() 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 the hour param is missing, even when one of the other parameters would be passed.

OptionalToRequiredFunctionParameters: add support for named parameters

  1. Adjusted the way the correct parameter is retrieved to use the new PHPCSUtils 1.0.0-alpha4 PassedParameters::getParameterFromStack() method.
  2. Verified the parameter name used is in line with the name as per the PHP 8.0 release.
    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:

Includes 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

Loading
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants