Backfill Parameters

[MGatner]

Description
This PR addresses the now-deprecated(PHP 8) use of mandatory parameters following optional parameters. Split off from #3931.

Checklist:

• Securely signed commits
• Component(s) with PHPdocs
• Unit testing, with >80% coverage
• User guide updated
• Conforms to style guide

[MGatner]

Rebasing 4.1 and trying again.

[MGatner]

Looks good! @paulbalandan you still around by chance?

[MGatner]

Since this was approved in the other PR I am going to proceed with the merge.

[paulbalandan]

I was influenced by PHPStan's extra strict rules in that empty() implies weak comparison and we should strive to be more specific what 'emptiness' we are checking. We already have a rector rule that changes empty checks on arrays to === [] and I believe we should do the same for scalar types.

[paulbalandan]

Suggested change

	if (empty($alias))
	if ($alias === '')

[paulbalandan]

Suggested change

	throw new InvalidArgumentException('You must supply the parameter: alias.');
	throw new InvalidArgumentException('You must supply the parameter: alias.'); // @codeCoverageIgnore

[paulbalandan]

Suggested change

	if (empty($uri) || empty($userAgent))
	{
	throw new InvalidArgumentException('You must supply the parameters: uri, userAgent.');
	}
	if ($uri === null || $userAgent === null)
	{
	throw new InvalidArgumentException('You must supply the parameters: uri, userAgent.'); // @codeCoverageIgnore
	}

[paulbalandan]

Suggested change

	throw new InvalidArgumentException('You must supply the parameters: origWidth, origHeight.');
	throw new InvalidArgumentException('You must supply the parameters: origWidth, origHeight.'); // @codeCoverageIgnore

[paulbalandan]

Suggested change

	* @param string|null $fields List of fields that we should check if present
	* @param string $fields List of fields that we should check if present

[paulbalandan]

Suggested change

	public function required_with($str = null, string $fields = null, array $data = []): bool
	{
	if (is_null($fields) || empty($data))
	{
	throw new InvalidArgumentException('You must supply the parameters: fields, data.');
	}
	public function required_with($str = null, string $fields = '', array $data = []): bool
	{
	if ($fields === '' || $data === [])
	{
	throw new InvalidArgumentException('You must supply the parameters: fields, data.'); // @codeCoverageIgnore
	}

[MGatner]

Same as below, $fields is allowed to be an empty string so we need to add the null option. This could probably be addressed on calling classes but it is also possible that someone is calling this directly (though I really hope not).

[paulbalandan]

Same comment with required_with

[paulbalandan]

Suggested change

	protected function processRules(string $field, string $label = null, $value, $rules = null, array $data = null): bool
	{
	if (is_null($data))
	{
	throw new InvalidArgumentException('You must supply the parameter: data.');
	}
	protected function processRules(string $field, string $label = null, $value, $rules = null, array $data = []): bool
	{
	if ($data === [])
	{
	throw new InvalidArgumentException('You must supply the parameter: data.'); // @codeCoverageIgnore
	}

[MGatner]

This one has to add null as an input option because [] is actually valid input and we're trying to catch cases where the parameter was not supplied at all.

[paulbalandan]

Since this was approved in the other PR I am going to proceed with the merge.

Oh man! I was a little too late. 😂

[MGatner]

Ah sorry! Those are good changes. I am working on the class aliases but I'll get these done up later today or tomorrow.

[sfadschm]

Did this PR also cover functions with nullable optional arguments (or typed arguments that default to null)?
The link supplied in #3957 indicates that such definitions (while still being wrong) do not trigger the PHP8.0 deprecation message.

[MGatner]

What is wrong with nullable optional parameters?

This PR dealt with the deprecated use of mandatory parameters following optional ones.

[sfadschm]

Sorry for the confusion, I will try to elaborate:
Required arguments should never be put after optional arguments, or PHP8 will throw a deprecation message. This has been dealt with in this PR.

However, according to the link provided in the referenced issue, PHP8 will not show such message in the following case (typed optional argument defaulting to null):
function test(string $param1 = null, string $param2)

As far as I understand the deprecation, this type of argument list would still be incorrect, although it does not trigger the deprecation message.

Therefore I was asking whether the selection of functions targeted by this PR was based on the PHP8 deprecation messages or if all functions with optional parameters before required ones have been changed.
In the first case, this PR might have missed some of the functions with argument lists like shown in the example.

Hope this makes it clearer :-)

[MGatner]

While I would like to see that category changed as well, the focus of this PR was actual deprecation notices as they arose during PHP 8 testing. I am unhappy that these parameter inputs exist to begin with, but without breaking the methods for all users we can't really change that in a clean way.

If you identify any of the above and want to submit a non-breaking PR we could certainly resolve. I'll be honest I didn't know that was a separate category!