Fix callables that are expected to return bool but don't

I went with the helper function in Unicode, then realised we need the reverse of that so I implemented that too. Since we're only checking if the string is empty it doesn't matter if we were using strlen or mb_strlen before, they would either return 0 or something other than 0 and that's all that matters.

Hmm, one could choose to use the helper function. Unicode is a weird place though. And a String class cant be called String i think.

But personally since the logic in the 2 methods is so simple, why not use

$array = array_filter($a, fn($v) => $v !== '');<code>.

I'm also thinking though, we assume all are string in those arrays. But might not be. Seems that there is a slight change in output should for some reason a boolean FALSE be passed.

https://3v4l.org/O7KfE
<?php
array_filter($a, fn($v) => $v !== '')
array(7) {
  [1]=>
  string(1) " "
  [2]=>
  string(1) "0"
  [3]=>
  int(0)
  [4]=>
  int(12)
  [5]=>
  float(15.5)
  [6]=>
  bool(false)
  [7]=>
  bool(true)
}

array_filter($a, 'strlen');
array(6) {
  [1]=>
  string(1) " "
  [2]=>
  string(1) "0"
  [3]=>
  int(0)
  [4]=>
  int(12)
  [5]=>
  float(15.5)
  [7]=>
  bool(true)
}

?>

So I think we need to be carefull here. Not sure whats the smart way out, perhaps the other solution might be safer with <code>strlen($v) > 0

Thanks for looking at this @bbrala. I initially went down the simple arrow function route:

fn ($value) => strlen($value) > 0

But strlen is coercing values to a string. If we later add declare(strict_types=1) then we get a TypeError. So we'd need to cast to match the current logic:

fn ($value) => strlen((string) $value) > 0

But later we might decide arrow functions should be typed:

fn (mixed $value): bool => strlen((string) $value) > 0

We might also later decide to enable SlevomatCodingStandard.Functions.StaticClosure:

static fn (mixed $value): bool => strlen((string) $value) > 0

It's starting to get rather convoluted, and we need to reuse this in a number of different places. This is why I went with the helper function.

I guess we do need to allow mixed and do the casting, and find a better spot for it. Perhaps the helper function could do the array_filter part as well.

Hmmz yeah that will get out of hand. The better question might be then if we should just do your current setup and technically be more strict (and correct) on the output.

We do need a new place for that though. Unicode feels wrong.

I did check, didn't find a better place. So guessing an ArrayFilter helper makes sense.

Nice timing, I was just finishing up the tests. What do you think of these changes?

Re-ran unit tests twice but they fail each time so seems like could be related.

The array to string conversion is bogus, it doesn't work with the strlen we had before so it doesn't need to work now.

Feedback and conceirns addressed. All good.

I read the IS, comments and the MR. thanks for the easy to understand issue summary. All questions answered. Just needs another committer to look at this.

Leaving at RTBC

Committed/pushed to 11.x and cherry-picked to 10.4.x, thanks!

catch → closed merge request !9404

Automatically closed - issue fixed for 2 weeks with no activity.