Framework: Illuminate\Database\Query\Builder violates Illuminate\Database\ConnectionInterface contract

[hasyimibhar]

• Laravel Version: master
• PHP Version: irrelevant
• Database Driver & Version: irrelevant

Description:

This isn't a bug, just a slight annoyance in code design.

If you look at Illuminate\Database\Query\Builder constructor, it specifies that Builder depends on the interface Illuminate\Database\ConnectionInterface. However, it actually assumes that $connection is Illuminate\Database\Connection, as it references several methods which exist on Connection but do not exist on ConnectionInterface, such as:

• getQueryGrammar
• getPostProcessor
• cursor
• getPdo

Either these methods should be added to ConnectionInterface, or Builder should just depend on Connection instead of ConnectionInterface.

[sisve]

There are other problems too; Builder::runSelect calls select() with three parameters, the interface only has two.

Sorry, but we do not include setters and getters on our interfaces.

Source: https://github.com/laravel/framework/issues/12250#issuecomment-182817704

[hasyimibhar]

I feel like writing a PR to fix this, but I see there are other similar PR that got rejected just because the changes make no difference (e.g. https://github.com/laravel/framework/pull/20513, https://github.com/laravel/framework/pull/18478).

I understand if the PR is rejected because it introduces breaking changes. But I don't understand why the PR is rejected because it "made no difference". Surely it's better for the implementation to respect the interface, no? If not, why bother having interface in the first place?

[hakuno]

@hasyimibhar

If not, why bother having interface in the first place?

Indeed :laughing:

[themsaid]

Please feel free to open a PR on the 5.6 branch with your suggested changes.