Skip to content

Conversation

@zigzagdev
Copy link

Summary

This PR adds explicit mixed type hints to the getArgument() method in order to align the actual method signature with the existing PHPDoc annotations.

Motivation

  • Clarifies function signature in accordance with PHP 8+
  • Improves IDE support and static analysis
  • Does not introduce any backward compatibility breaks

@zigzagdev zigzagdev force-pushed the refactor/add-mixed-typehint-to-getArgument branch from 3cce4a9 to 44e69f9 Compare July 14, 2025 01:01
@zigzagdev
Copy link
Author

Note: The CI failure seems unrelated to this PR.
The errors (e.g., dynamic ::class usage in Enqueue or deprecated test traits) are coming from unrelated packages or deprecated framework components that this PR does not touch.

Let me know if I should rebase or help isolate the failures in a separate PR.

* @return mixed
*/
public function getArgument($key = null, $default = null)
public function getArgument(mixed $key = null, mixed $default = null)
Copy link
Member

Choose a reason for hiding this comment

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

In order to use mixed we would have to drop PHP 7.x support. I'd rather not do that in the 1.x branch as this branch supports CakePHP 4.x. However, this change could be made in the 2.x branch which has a min PHP version of 8.1.

Copy link
Author

Choose a reason for hiding this comment

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

@markstory

Thank you for your reply!

After switching the base branch from 1.x to 2.x, I noticed that the changes I proposed have already been implemented there.
So I'll go ahead and close this PR. Thanks again for your time and feedback!

@zigzagdev zigzagdev changed the base branch from 1.x to 2.x July 22, 2025 01:23
@zigzagdev zigzagdev closed this Jul 22, 2025
@zigzagdev zigzagdev deleted the refactor/add-mixed-typehint-to-getArgument branch July 25, 2025 00:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants