Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Parameter and return types for hook implementations #404

Open
mfb opened this issue May 8, 2022 · 4 comments
Open

Parameter and return types for hook implementations #404

mfb opened this issue May 8, 2022 · 4 comments

Comments

@mfb
Copy link

mfb commented May 8, 2022

For hook implementations in contrib projects, drupal-check v1.4.0 reports Function filemime_file_mimetype_mapping_alter() has parameter $mapping with no value type specified in iterable type array. 💡 See: https://phpstan.org/blog/solving-phpstan-no-value-type-specified-in-iterable-type

But if you do document the @param type in your contrib project, drupal/coder flags that "Hook implementations should not duplicate @param documentation" and "Hook implementations should not duplicate @return documentation" - see https://git.drupalcode.org/project/coder/-/blame/8.3.x/coder_sniffer/Drupal/Sniffs/Commenting/HookCommentSniff.php#L96

I guess ideally drupal-check could figure out the type hints from the hook definitions and apply them to hook implementations? Or if that's not possible, perhaps these warnings should be ignored for now?

See also https://www.drupal.org/project/coder/issues/3279146

@mglaman
Copy link
Owner

mglaman commented May 9, 2022

This isn't asking for phpDoc. It's asking for PHP native types on arguments and return type.

There's another issue. This is a "won't fix". Drupal core and all contrib hooks defined in .api.php should have these.

Again, "won't fix" because you don't need to define anything in phpDoc.

If you want to, use @phpstan-param instead

@mglaman mglaman closed this as completed May 9, 2022
@mfb
Copy link
Author

mfb commented May 9, 2022

Oops sorry, I pasted the wrong message in the initial issue report.

@mglaman mglaman reopened this May 9, 2022
@mfb
Copy link
Author

mfb commented May 9, 2022

What I wanted to file an issue about here is that even after I add the parameter and return type declarations, drupal-check still reports an issue - the array type needs to be further documented.

@mglaman
Copy link
Owner

mglaman commented May 9, 2022

Actually reopening once I reread.

This needs docs, so I want to keep it open so I remember to.

And I forgot that I wanted to see if we could map hooks to .api.php file definitions somehow.

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

No branches or pull requests

2 participants