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 a sniff to force types in comments to use use statements. #201

Closed
herregroen opened this issue Apr 7, 2020 · 1 comment · Fixed by #363
Closed

Add a sniff to force types in comments to use use statements. #201

herregroen opened this issue Apr 7, 2020 · 1 comment · Fixed by #363
Milestone

Comments

@herregroen
Copy link
Contributor

Typehints in comments should, if they refer to a namespaced class, always use a use statement and use the short form in the comment.

The following docblock would break this rule:

/**
 * A function.
 * 
 * @param \Yoast\WP\SEO\Input $input The input.
 *
 * @returns \Yoast\WP\SEO\Output The output.
 */

And the following docblock would be a correct replacement:

use Yoast\WP\SEO\Input;
use Yoast\WP\SEO\Output;

/**
 * A function.
 * 
 * @param Input $input The input.
 *
 * @returns Output The output.
 */

Ideally an auto-fixer would insert new use statements in alphabetical order at the top of the file and fix the comments directly.

We are aware not all WordPress tools would be able to correctly parse this notation but believe that the better way forward here would be to support this notation. Especially given that in the wider PHP open source environment this is also the standard.

@jrfnl
Copy link
Collaborator

jrfnl commented Apr 8, 2020

Ideally an auto-fixer would insert new use statements at the top of the file and fix the comments directly.

I propose to address this in YoastCS 3.0 - also see #208

If my proposal in #208 would be accepted, the following rule can be added to the ruleset to address this (and auto-fix issues):

    <rule ref="SlevomatCodingStandard.Namespaces.ReferenceUsedNamesOnly">
        <properties>
            <property name="searchAnnotations" value="true"/>
        </properties>
    </rule>

Note: more configuration for this sniff will be needed depending on choices made for other things (like exception, global functions, constants), but that's outside the scope of this issue.

use statements in alphabetical order

This is a separate request as there currently is no rule about use statements needing to be in alphabetically order.

If a rule to that effect should be added, it needs its own issue.

@herregroen Didn't you also want to forbid use import statements for the same namespace ? If so, this would need its own issue as well to make sure we address this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants