fix: required_without rule logic in Validation class.

[ping-yee]

Description
Fixed the logic to support #5922 and #5942.

Checklist:

• Securely signed commits
• Component(s) with PHPDoc blocks, only if necessary or adds value
• Unit testing, with >80% coverage
• User guide updated
• Conforms to style guide

[ping-yee]

I have been implement the logic to support the #5922 case logic but I forget to change the PC to commit with GPG sign.
I will fix these commit that without GPG sign.

[kenjis]

Thank you!
See https://github.com/codeigniter4/CodeIgniter4/blob/develop/contributing/workflow.md#gpg-signing-old-commits

[MGatner]

This is much easier to comprehend! Thank you. Awaiting GPG

[paulbalandan]

Isn't this a breaking change? If I am using the rules programmatically, like (new Rules())->required_without(/* .. */), then the addition of an optional parameter that is required inside the body will break my app.

[ping-yee]

Yes, it seems a breaking change.

 $this->validation->setRules([
    'a.*.c' => 'required_without[a.*.b]',
])->run($data);

But I think this param is needed because this rule need to know what the key field is being passed in now (like: a.0.c, a.1.c...etc), otherwise it will only be passed the asterisk field (like: a.*.c).

[paulbalandan]

What is the implication of the $now? We have a $keyField then $nowKeyField. Then we have a $nowField and $nowFieldValue

[ping-yee]

 $this->validation->setRules([
    'a.*.c' => 'required_without[a.*.b]',
])->run($data);

Like this scenario, the implication of these name I set show as below .

a.*.c => key

$keyField => the key field is being passed in now (like: a.0.c, a.1.c).
$nowKeyField => the index of key field is being passed in now (like: 1 of a.1.c, 2 of a.2.c).

required_without[a.*.b] => the param field of required_without[]

$nowField => the field of required_without is changed from asterisk field to current field (like: a.*.b => a.0.b, a.1.b...etc).
$nowFieldValue => the value of current field in data set.

I think the name of these variables aren't the best, or are there more suitable names?

[kenjis]

I honestly don't understand what the variable names mean. Variable names need to be changed.

[ping-yee]

Variable names need to be changed.

I agree with you, do you guys have any suggestion for these variable names?

[kenjis]

How about this?

    /**
     * The field is required when any of other fields do not pass `required` checks.
     *
     * Example (field is required when the id or email field is missing):
     *     required_without[id,email]
     *
     * @param string|null $value       The value to check
     * @param string|null $otherFields The rule parameter fields. List of other fields to check if present
     * @param array       $data        All data to validate
     * @param string|null $field       The field name to check. If the rule parameter fields aren't present, this field is required.
     */
    public function required_without(
        $value = null,
        ?string $otherFields = null,
        array $data = [],
        ?string $error = null,
        ?string $field = null
    ): bool {
        if ($otherFields === null || empty($data)) {
            throw new InvalidArgumentException('You must supply the parameters: otherFields, data.');
        }

        // If the field is present we can safely assume that
        // the field is here, no matter whether the corresponding
        // search field is present or not.
        $otherFields = explode(',', $otherFields);
        $present     = $this->required($value ?? '');

        if ($present) {
            return true;
        }

        $otherFieldKey = 0;

        // Still here? Then we fail this test if
        // any of the fields are not present in $data
        foreach ($otherFields as $otherField) {
            if ((strpos($otherField, '.') === false) && (! array_key_exists($otherField, $data) || empty($data[$otherField]))) {
                return false;
            }
            if (strpos($otherField, '.') !== false) {
                if ($field === null) {
                    throw new InvalidArgumentException('You must supply the parameters: field.');
                }

                $otherFieldValue = dot_array_search($otherField, $data);
                $fieldArray      = explode('.', $field);
                $otherFieldKey   = $fieldArray[1];

                if (is_array($otherFieldValue)) {
                    return ! empty(dot_array_search($otherField, $data)[$otherFieldKey]);
                }

                $currentField      = str_replace('*', $otherFieldKey, $otherField);
                $currentFieldValue = dot_array_search($currentField, $data);

                return null !== $currentFieldValue;
            }
        }

        return true;
    }

[ping-yee]

I think this naming is ok, more readability.
I change it immediately.

[ping-yee]

I think the name of $fieldIndex is more appropriate to show the index of the field.
More readability?

[ping-yee]

I have been modified, review please.

[kenjis]

index implies 0, 1, 2, ...
But it is an array key and is a string.
$fieldKey is better?

[ping-yee]

$fieldKey is better?

I agree with you.
I will change that.

[kenjis]

Please fix errors and failures in PHPUnit testing.

ERRORS!
Tests: 5329, Assertions: 8867, Errors: 25, Failures: 3, Skipped: 14.
https://github.com/codeigniter4/CodeIgniter4/actions/runs/3126902870/jobs/5072927529

[kenjis]

Tests: 5329, Assertions: 8869, Errors: 3, Skipped: 15.

[ping-yee]

All of Errors are fixed!

[kenjis]

Please add the test case:

    public function testRequireWithoutWithWildCard()
    {
        $data = [
            'a' => [
                ['b' => 1, 'c' => 2],
                ['c' => ''],
            ],
        ];

        $this->validation->setRules([
            'a.*.c' => 'required_without[a.*.b]',
        ])->run($data);

        $this->assertSame(
            'The a.*.c field is required when a.*.b is not present.',
            $this->validation->getError('a.1.c')
        );
    }

[ping-yee]

Already made up the comment out, the dot of throw and test case!
Appreciated that! @kenjis

[ping-yee]

@kenjis All required changes have been corrected.
The changelog and upgrade-guide are also added.

[kenjis]

@ping-yee Thank you for updating!
I added suggestions for the docs.
Other things seem to be good.

[ping-yee]

I have been modified the docs.
I really appreciate your help and support during these days!

[kenjis]

LGTM!

[MGatner]

Thank you both! This one ended up being a lot of work, with many improvements along the way. Happy to see it closed up! 🥳