dereuromark / cakephp-ide-helper

IDE Helper plugin for CakePHP
MIT License
185 stars 39 forks source link

Form->execute() annotation #257

Closed dereuromark closed 2 years ago

dereuromark commented 2 years ago

What I found useful to add - manually for now:

/** @uses \App\Form\ReleaseForm::_execute() */
$result = $releaseForm->execute($data);

As clicking on execute() directly brings you to

    public function execute(array $data, array $options = []): bool

of Cake\Form\Form, and not to the actual code behind it, which is inside

    protected function _execute(array $data): bool

of my App\Form\ReleaseForm class

We could add this to all such calls as inline annotation.

What do others think? or is there a better solution?

LordSimal commented 2 years ago

This reminds me of virtual fields where you don't get directly to the _getMyVirtualField() function if you click on a virtual field but rather the property. In that case one still get to the correct class though so its not that big of a deal.

In your ReleaseForm example one would have to click on the new ReleaseForm() to get to the correct class though.

dereuromark commented 2 years ago

Yes, but only class, not the method itself, and it can sometimes be in different files, if you pass the object around...

It is also bad for beginners, as they dont find the code to it, or dont understand how it continues here.

Erwane commented 2 years ago

Cake\Form\Form is sometime used directly without a project form class ? Maybe Form::execute() could be abstract.

Yes, extra code in each App Form class to set data and validate but maybe more clear.

edit : not a right plugin answer but framework one ... sorry

dereuromark commented 2 years ago

@ADmad @dakota What is your take on these things?

ADmad commented 2 years ago

"Linking" the execute() call to the _execute() hook method seems like good usability.

dereuromark commented 2 years ago

Similar problem with

/** @uses \App\Mailer\NotificationMailer::notify() */
$this->mailer->send('notify', [$releasesToNotify, $releaseData, $pr]);

otherwise linking to

public function send(?string $action = null, array $args = [], array $headers = []): array

of Mailer itself, which does easily confuse people probably, when looking for what it actually does.

dereuromark commented 2 years ago

I think both of those are only properly solvable by rector (as it needs to know the object type at runtime). maybe we should add those there instead?

dereuromark commented 2 years ago

The form part is tackled in https://github.com/dereuromark/cakephp-ide-helper/pull/259 Someone else can maybe replicate the same with the Mailer class? Should be straight forward