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

Allow @inheritDoc for implementations of interfaces and abstract methods #168

Closed
IreneStr opened this issue Nov 14, 2019 · 6 comments
Closed

Comments

@IreneStr
Copy link
Contributor

IreneStr commented Nov 14, 2019

We decided that the use of @inheritDoc should only be allowed for implementations of interfaces and abstract methods. If @inheritDoc is used to document any other method/function/class, Yoast CS should throw an error.

Slightly related: I've also +1ed this issue on WPCS to ensure no warnings about missing short descriptions will be thrown when we use @inheritDoc: WordPress/WordPress-Coding-Standards#1429

@jrfnl
Copy link
Collaborator

jrfnl commented Nov 14, 2019

Question: what about double/mock classes which extend a functional class ?

@IreneStr IreneStr changed the title Allow @inheritDoc for interfaces and abstract methods Allow @inheritDoc for implementations of interfaces and abstract methods Nov 14, 2019
@jrfnl
Copy link
Collaborator

jrfnl commented Nov 14, 2019

We decided that the use of @inheritdoc should only be allowed for implementations of interfaces and abstract methods.

Unfortunately that's not reliably sniffable.

PHPCS scans files in an undetermined order and for all plugins using YoastCS, with parallel scanning enabled.

To be able to sniff this we'd need to know in advance which abstract methods are in abstract classes and which methods are defined in interfaces, as @inheritdoc should only be allowed for those specific methods.

Unfortunately we don't have access to that information at the start of the scan.
We could keep a cache of information building this up during the scan, but at any point in time, this cache will be incomplete and possibly unreliable with parallel scanning.

And allowing just any method in a class which extends or implements to use @inheritdoc will result in a lot of missing documentation, so is something I would strongly discourage.

@herregroen
Copy link
Contributor

herregroen commented Nov 14, 2019

@jrfnl From what I can see PHPCS does load the composer autoloader which means all classes it is scanning should be available for reflection.

Using a combination of:

It should be possible to reliably compile a list of methods that may use @inheritDoc regardless of the order files are processed.

@jrfnl
Copy link
Collaborator

jrfnl commented Nov 14, 2019

@herregroen Hmm.. that's an interesting idea. I hadn't though of that as in most cases when working with PHPCS you can not presume a Composer install, but with YoastCS we can, of course. Nice.

I do wonder how much extra memory that will cost to use and whether autoloading those classes via Composer/Reflection will cause issues with "class already declared", but that's something to figure out when working on this.

@jrfnl
Copy link
Collaborator

jrfnl commented May 27, 2021

Since PHPCS 3.6.0, the Squiz.Commenting.FunctionComment sniff has a property to skip @inheritDoc, however this is not limited to specific circumstances.

Squiz.Commenting.FunctionComment is now able to ignore function comments that are only inheritdoc statements

  • Set the skipIfInheritdoc sniff property to true to skip checking function comments if the content is only {@inheritdoc}
  • The default remains at false, so these comments will continue to report errors

If we'd want to use this, we would need to extend the upstream sniff and toggle the property depending on whether the class in the file which is being scanned complies with the above mentioned conditions.

@jrfnl
Copy link
Collaborator

jrfnl commented Sep 22, 2023

Closing as superseded by the decision to forbid @inheritDoc completely - see #303.

@jrfnl jrfnl closed this as not planned Won't fix, can't repro, duplicate, stale Sep 22, 2023
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

3 participants