fix: replace try...catch(BadMethodCallException) blocks with a check

[Yogarine]

Using BadMethodCallException in this way is not recommended because it may obfuscate underlying issues in the code.

And specifically for me, the call to the non-existent withTrashed() triggers a rare PHP bug which results in a Segmentation fault under very specific and rare circumstances.

This fix checks whether the withTrashed() method exists, or is provided through a macro, instead of relying on trying and catching an Exception if it fails.

[kohlerdominik]

@Yogarine does this properly cover laravels ForwardsCalls trait? Because I seem to recall that was my issue back then, and this is why I decided to do it explicit this way. I agree, its not exactly pretty, but at least having 100% coverage of all (laravel) magic.

For your segfault, if you have JIT enabled you may just have spotted the rabit whole. We had to disable it with Laravel/Symfony for now... https://github.com/php/php-src/issues?q=segmentation+fault

[Yogarine]

@kohlerdominik

@Yogarine does this properly cover laravels ForwardsCalls trait? Because I seem to recall that was my issue back then, and this is why I decided to do it explicit this way. I agree, its not exactly pretty, but at least having 100% coverage of all (laravel) magic.

Yeah. This is the same way Laravel / Eloquent code checks whether to call withTrashed(). (Well, besides the method_exists(), which I kept in there because I feel is the more appropriate way to check whether to call a method.)

I was initially about to replicate all the logic in Builder::__call() which would end up being something like this:

if (
    method_exists($builder, 'withTrashed')
    || $builder->hasMacro('withTrashed')
    || $builder->hasGlobalMacro('withTrashed')
    || $builder->hasNamedScope('withTrashed')
) {
    $builder->withTrashed();
}

...which is quite the mouthful. And I was contemplating hoisting that into a separate trait.

But then I found that in MorphTo::withTrashed() just checking with ->hasMacro() is considered enough:

        $callback = fn ($query) => $query->hasMacro('withTrashed') ? $query->withTrashed() : $query;

Welp, if it's good enough for uncle Taylor, it's good enough for me.

For your segfault, if you have JIT enabled you may just have spotted the rabit whole. We had to disable it with Laravel/Symfony for now... https://github.com/php/php-src/issues?q=segmentation+fault

Oooh, no no no no... We don't have JIT enabled for different compatibility reasons, but thanks for the heads up though.

[kohlerdominik]

But then I found that in MorphTo::withTrashed() just checking with ->hasMacro() is considered enough

Good work. There is a ton of laravel magic and it's always at risk for breaking something, but I can agree with the approach that if it works for internals, it works for packages.

@Yogarine @gcphost the current latest release 11.0.3 uses the source code from c279c66, which is the commit in broken state from PR #155.

Can you please fix the release?

[Yogarine]

@kohlerdominik

@Yogarine @gcphost the current latest release 11.0.3 uses the source code from c279c66, which is the commit in broken state from PR #155.

Can you please fix the release?

Thanks for the heads up! I didn't realise it had already been pushed in a release. I've pulled v11.0.3 and pushed a new release v11.0.4 with the correct fix.