Result of a method call on Eloquent\Builder depends on the method casing used

[luka-papez]

Laravel Version

10.29.0

PHP Version

8.1.2

Database Driver & Version

Not relevant but just in case: psql (PostgreSQL) 14.9 (Ubuntu 14.9-0ubuntu0.22.04.1)

Description

When calling a method on a instance of Eloquent\Builder, the result of the method will depend on the method casing used. I discovered this when I called $query->toSQL() instead of $query->toSql() and got back a Builder instead of a string which I expected. Given that PHP is case-insensitive about method names, I find this behavior surprising.

According to my understanding, the issue happens on this line:

• framework/src/Illuminate/Database/Eloquent/Builder.php

  Line 1967 in 98ffb75

  if (in_array($method, $this->passthru)) {

The Builder decides what to do with the result depending on whether the called method is found in the $passthru property. Since toSql is there, and toSQL is not, the result is different.

I believe the behavior could be fixed by lowercasing the method name and making the $passthru property contain lowercased method names, but I am not sure what kind of implications that might have down the road.

If needed, I would be happy to volunteer some time into fixing this issue, or reviewing the fix. Either way, thank you for all the hard work on making Laravel so great for the rest of us!

Steps To Reproduce

The behavior can be triggered by the following snippet:

        $query = User::query()->select('*');

        $camelCaseResult = $query->toSql();  // this returns a string
        $upperCaseResult = $query->toSQL();  // this returns $query

A small repository demonstrating this behavior can be found here:

• https://github.com/luka-papez/eloquent-bug-demonstrator/blob/main/tests/Feature/EloquentBuilderBugReportTest.php

It's based on the example Laravel app with a separate commit demonstrating the potential problem

[driesvints]

We'd welcome PRs for this 👍 thanks!

[github-actions]

Thank you for reporting this issue!

As Laravel is an open source project, we rely on the community to help us diagnose and fix issues as it is not possible to research and fix every issue reported to us via GitHub.

If possible, please make a pull request fixing the issue you have described, along with corresponding tests. All pull requests are promptly reviewed by the Laravel team.

Thank you!

[Guxinpei]

I've come across the same issue, and I appreciate you bringing this up and suggesting a fix by modifying the case. While this proposal may address the problem at hand, I believe employing a pipeline to manage the __call method's multiple priority processes could provide a more structured and robust solution, ensuring that casing discrepancies don’t affect functionality. This approach might also be more adaptable to other similar issues that could arise in the future. Just a thought!

[luka-papez]

@driesvints I went ahead and implemented a fix in: #48852
Any feedback is very welcome!

[luka-papez]

@Guxinpei While I agree that the current method dispatch could be made more robust, I am not sure if the added complexity would really be worth it.

Eloquent\Builder is such a widely used class, and yet it still took me many moons of using Laravel to stumble across this problem (in fact a student of mine pointed it out, classing "rookie mistake" 🙂 )

[bertoost]

Any existing $passthru lists with correct casing will be broken with this update. See my comment on the PR #48852