RFC: 4.0+ Roadmap - Remove magic method

[davidyell]

This is a (multiple allowed):

• bug

• enhancement

• feature-discussion (RFC)

• CakePHP Version: 4.0 onwards

What you did

Tried to mock the \Cake\ORM\Query class, to create a fixed return. My IDE was suggesting that I could mock Cake\ORM\Query::extract() even though this method does not exist.

It's linked via a docblock, but doesn't exist in the class.

What happened

I was unable to achieve this and had to use a Fixture, and actually execute the query with data. Rather than just mocking the query and having a fixed return.

What you expected to happen

I expected to find the methods which I wanted to mock in the class. Then setup a mock of the Cake\ORM\Query object and configure a fixed return of either an array, or \Cake\ORM\ResultSet.

How did I solve the issue

I had to ask both my colleagues and in the Slack channel to find out how the \Cake\ORM\Query class was implementing Collection class methods, when there was no obvious extension, trait, or interface implementation shown in the code.

What was the solution

It turns out the \Cake\Datasource\QueryTrait has a __call() method. Shown here

As I am not overly familiar with magic methods. I was reading the source code, and 'chasing functions' in my IDE, I was unable to work out how all the methods were being made available. The code did not describe it's behaviour to me, making it hard to reason about.

RFC

I'd like to suggest an addition to the 4.x Roadmap to remove this magic method, and add concrete implementation in the ORM, so that the source code is easier to reason about, requires less docblock links and is more obvious when being read.

Side note

At PHP Conf UK one year I was talking to attendees about frameworks and they had a poor opinion of CakePHP because of its use of magic methods and it's "black box" approach. I feel that things like this are validating that opinion, rather than challenging it.

[markstory]

Fair points.

[dereuromark]

This will then also require less ide annotations maybe in the future for some areas.
More direct tooling and IDE - and developer - understanding of the code itself then sounds like a good idea.

But I am not sure if we should keep this ticket open here, if there are no more concrete things to discuss or create a plan of attack from this.

[davidyell]

Perhaps it can be broken down into actionable tickets which reference back to this one?

[dereuromark]

You could also update this one with a - [ ] checklist similar to what we have in other tickets.

[dereuromark]

@davidyell Did you add it anywhere yet? So it won't be forgotten?

[davidyell]

No @dereuromark I haven't

[dereuromark]

Is there anything concrete left to be done or tracked?
Otherwise, we can close this here now.

[dereuromark]

ping @davidyell

[davidyell]

I don't know how to add to the Roadmap. Seems I don't have permission to edit the roadmap wiki.

[dereuromark]

You can also just outline it here, in more concrete steps - and someone can copy it over.

[davidyell]

Remove the __call() magic method in \Cake\ORM\Query, and add concrete implementation in the ORM, so that the source code is easier to reason about, requires less docblock links and is more obvious when being read.

[markstory]

Added to the roadmap doc.

[dereuromark]

I added it to 4.1 yesterday :)