Framework: mergeWhen(false, [is trying to execute the enclosed statement])

[poing]

• Laravel Framework 5.7.20
• PHP 7.2.12-1+ubuntu18.04.1+deb.sury.org+1 (cli) (built: Nov 12 2018 09:55:44) ( NTS )
• mysql Ver 14.14 Distrib 5.7.24, for Linux (x86_64) using EditLine wrapper

Description:

When working with Eloquent: API Resources, mergeWhen() is trying to execute the enclosed statement. Which defeats the ability to merge conditional attributes, when an 'attribute' is the condition.

• This add difficulty to programmatically skipping unset variables.
• It may also be a security concern:

$this->mergeWhen(Auth::user()->isAdmin(), ['this is executed for nonAdmin users too']);

Steps To Reproduce:

Below are two (2) examples.

Call to a member function toDateTimeString() on null

return [
    'href' => $this->href,
    'validFor' => [
        'startDateTime' => $this->created_at->toDateTimeString(),
        $this->mergeWhen(false, [
            'endDateTime' => $this->deleted_at->toDateTimeString(),
        ]),
    ],
];

Undefined variable: foo

unset($foo);  // isset($foo) = false
return [
    'a' => 'b',
    'c' => 'e',
    $this->mergeWhen(isset($foo), [
        'f' => $foo,
    ]),
];

[driesvints]

I don't really get where you're going with this. Obviously the code in your examples is executed. That's just how PHP works.

Underneath it uses the value helper so you can use a closure instead to delay the execution:

return [
    'href' => $this->href,
    'validFor' => [
        'startDateTime' => $this->created_at->toDateTimeString(),
        $this->mergeWhen(false, function () {
             return [
                'endDateTime' => $this->deleted_at->toDateTimeString(),
            ];
        }),
    ],
];

[poing]

I would expect that a enclosed statement (technically it's the second parameter) is ONLY executed when the condition is met. That's how PHP is normally works. That's how conditional statements are supposed to work. mergeWhen() appears to be a conditional function.

unset($foo);
if (isset($foo)) {
  echo "PHP does NOT execute this line";
}
if (false) {
  echo "PHP would NOT execute this line either";
}

---

Using an enclosure does solve the issue. Thanks! It resolves the issue for me.

---

But... Since mergeWhen() appears to be a conditional function:

• Shouldn't it operated like a regular conditional?
• Shouldn't the closure be the default?
• Should a developer be required to apply a work-around because a false condition does not prevent evaluation of the second parameter?

// forgive the incorrect semantics, trying to make a point here
if (false, function() {
  echo "You're telling me to prevent execution of these lines..";
  echo "that I should change how the conditional function is written.";    
});  // If I found that if() did not work as expected, I would have to add a closure?

There's also the security concern. While I doubt the risk is high due to applicable uses ofmergeWhen() But as a conditional, the expected default behavior of mergeWhen() is it would not execute $this->is->dangerous() in the code below.

$admin = false;
   //
   $this->mergeWhen($admin, [
      'dangerous' => $this->is->dangerous(),
   ]),
   //

[driesvints]

@poing I think you're simply misunderstanding how code execution in PHP works. The way it's compiled is that any code that's being passed as a parameter is executed before it's passed through the argument. A closure based approach like I've shown above prevents this.

[poing]

@driesvints Nah, I get it. It's the parameter that's being evaluated. The enclosure let's the parameter pass, where it's not evaluated unless true in the function.

[bradley-varol]

I want to merge an array that uses many variables. Do I really have to pass them all into the closure with a use statement?