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

Add rules to check for DocBlock comments #1

Merged
merged 4 commits into from
May 11, 2023
Merged

Conversation

fmfernandes
Copy link
Contributor

@fmfernandes fmfernandes commented May 10, 2023

This PR adds rules to check for DocBlock comments in functions and classes.

Squiz.Commenting.FunctionComment checks for the presence of @param, @return and @throws tags and will also make sure function parameter names and descriptions are aligned like so:

/**
 * Returns true if all the plugin's dependencies are met.
 *
 * @since 1.0.0
 * @version 1.0.0
 *
 * @param string|null $minimum_wc_version The minimum WooCommerce version required.
 * @param integer     $integer_param      An integer parameter.
 *
 * @return boolean
 */
public function is_active( string &$minimum_wc_version = null, int $integer_param ): bool {

Currently there's no sniff to check for @since or @version tags, so it also can not align them with @param tags. Though, I believe it's still an advantage to check for the presence of a DocBlock comment.

Generic.Commenting.DocComment is added so we ensure each function has a description in the DocBlock. The rule above only checks tags, not the description itself. This rule also ensures a sentence starts with a capital letter and ends with a period.

Squiz.Commenting.ClassComment handles checks for a class but we need to disable checks for invalid tags, as we may want to have @since and @version, but that does mean a class with a @var tag is valid.

@ahegyes
Copy link
Contributor

ahegyes commented May 11, 2023

Thanks for working on this @fmfernandes! I hope you don't mind, but I added some comments to the new sniffs so it's easier to parse the rules in the future after we all forget why there were added.

Otherwise, no notes 😄 I ran this set over my updates to the Team51 Donations plugin and it caught quite a few problems, so this is a very nice addition in my opinion!

@ahegyes
Copy link
Contributor

ahegyes commented May 11, 2023

as we may want to have @SInCE and @Version, but that does mean a class with a @var tag is valid.

If it makes sense to have a tag in there, I don't see why we should forbid it. Sometimes I even add a @link tag if I want to point to a StackOverflow answer or some other source that justifies the class' design.

@fmfernandes
Copy link
Contributor Author

Thanks for working on this @fmfernandes! I hope you don't mind, but I added some comments to the new sniffs so it's easier to parse the rules in the future after we all forget why there were added.

Nice! Thank you!

If it makes sense to have a tag in there, I don't see why we should forbid it. Sometimes I even add a @link tag if I want to point to a StackOverflow answer or some other source that justifies the class' design.

Agreed!

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