Fix file_exists warning when resolving with long strings

[sbesselsen]

When resolving a promise with a long string, the following may happen:

• method_exists is called
• This triggers the class autoloader to check for a class by that name
• The class autoloader will use file_exists to check for a file by that name
• file_exists emits a warning: Warning: file_exists(): File name is longer than the maximum allowed path length on this platform ([...])

The method_exists() check only makes sense for objects, since further down the call $promiseOrValue->then(...) is made. Static then methods aren't supported anyway.

[clue]

@sbesselsen Good catch, that's a nasty one! 🥇

Does it make sense to add a test for this? 👍

[WyriHaximus]

@clue I think it makes sense to add a test. Not sure how we'd test this yet to be honest. But if @sbesselsen has an idea how to test this that would be great 👍

[jsor]

I'm also not sure how to test this. For me the patch is good as is. My only suggestion is to add a comment for the reasoning behind the is_object() check.

[sbesselsen]

Hi all, thanks for the feedback. I've added a comment as requested.

I've also thought about a good way to test it; the one thing I could think of was to create a test setup where we have a class with a method "then", and run resolve() with that class name. We would expect that promise to then resolve with the class name, whereas before we would get a PHP error "Call to a member function then() on string". Does that sound like a good test? I'm unsure because it would require us to define a class in the test script just for that purpose, which seems... strange?

[WyriHaximus]

LGTM 👍 . And while I really like to have a test for this I see no reason to block this, we can do the test in a follow up PR and put this out there in the mean time.