Skip to content

Allow typed finder arguments after the $options array#16780

Closed
othercorey wants to merge 1 commit into4.nextfrom
finder-args
Closed

Allow typed finder arguments after the $options array#16780
othercorey wants to merge 1 commit into4.nextfrom
finder-args

Conversation

@othercorey
Copy link
Copy Markdown
Contributor

@othercorey othercorey commented Sep 30, 2022

refs #12787

This does not try to make $options optional.

@othercorey othercorey added this to the 4.5.0 milestone Sep 30, 2022
* @param mixed ...$args Finder arguments
*/
public function find(string $type = 'all', array $options = []): Query
public function find(string $type = 'all', array $options = [], ...$args): Query
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know if overriding find() is actually supported. If so, this has to be cake 5 only.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to maintain BC for the public method's signature. It's possible that someone has overridden this method.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, cake 5 it is.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Users will most certainly have overridden find.

@ADmad
Copy link
Copy Markdown
Member

ADmad commented Sep 30, 2022

There's ORM\Query::find() too, I don't see any change to that in the PR.

@othercorey
Copy link
Copy Markdown
Contributor Author

There's ORM\Query::find() too, I don't see any change to that in the PR.

I definitely forgot it exists.

@ndm2
Copy link
Copy Markdown
Contributor

ndm2 commented Sep 30, 2022

There's also Association::find().

* @throws \BadMethodCallException When the method is unknown.
*/
public function callFinder(string $type, array $args = []): Query
public function callFinder(string $type, ...$args): Query
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this also changing the same of args as now options are the 0th element instead of the only thing?

Copy link
Copy Markdown
Contributor Author

@othercorey othercorey Oct 1, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not really sure why BehaviorRegistry was using an array originally. Maybe it was trying to mimic __call().

* @param mixed ...$args Finder arguments
*/
public function find(string $type = 'all', array $options = []): Query
public function find(string $type = 'all', array $options = [], ...$args): Query
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Users will most certainly have overridden find.

@othercorey othercorey closed this Oct 1, 2022
@othercorey othercorey deleted the finder-args branch October 1, 2022 07:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants