search

WordPress / WordPress-Coding-Standards

PHP_CodeSniffer rules (sniffs) to enforce WordPress coding conventions
MIT License
2.56k stars 487 forks source link

Add sniffs for the theme checks #578

Closed grappler closed 6 years ago

grappler commented 8 years ago

I am opening this issue after discussing this with @GaryJones, @westonruter, @fklein-lu, @jrf & @Rarst at the WCEU contributors day and showing this to the theme review admins and @samuelsidler before publishing. This is one part of the project to improve the theme review process.

Action plan:

We'd need two new rulesets:

A number of these sniffs are not - like a typical sniff - valid for each file, but need to sniff if something is sone at least once in any of the files. So we might need to add a wrapper for those particular sniff in the Theme Check bootstrap to run those against each file and only report if a positive/negative results was returned for all.

Rules which can probably be turned into a sniff:

Rules which can probably be turned into a sniff but would need to be run against every file before a positive/negative result can be determined:

Rules which would need another solution (like in the bootstrap file which would run PHPCS from the Theme-check plugin within an install):

justintadlock commented 8 years ago

ERROR | Verify that the_post_thumbnail() is found at least once if the theme has a add_theme_support( 'post-thumbnails' ) call. This should become an error if the theme is tagged with featured-image.

We'd also want to check against either of these two functions:

jrfnl commented 8 years ago

FYI:

manishsaini1126 commented 7 years ago

I SOLVE YOUR PROBLEM I HAVE DONE SAME WORK IN PAST

Rarst commented 7 years ago

@manishsaini1126 that reads a bit spammy, could you please clarify if you genuinely want to get involved with this. :)

manishsaini1126 commented 7 years ago

can you please briefly describe your problem

Rarst commented 7 years ago

The topic is described in great detail in the opening post.

jrfnl commented 6 years ago

Hi all,

I've had a discussion with @dingo-d today about the future of this ticket and the WPTRT-CS repo.

Progress has been very slow over the past year (and a half), but should speed up significantly over the next few weeks as @dingo-d intends to have a working prototype of the new ThemeCheck plugin which would use the WPTRT-CS ruleset ready for WCEU.

Looking back over how development has flowed so far, generally speaking, any sniffs which were created for WPTRT-CS which would benefit the wider WP community - not just the TRT - have been pulled here in WPCS and included in WPTRT-CS via the ruleset. Only the few sniffs which are really theme specific have been pulled to the WPTRT-CS fork, these currently are:

Some more will be added in the near future, but they will be along the same lines.

All these sniffs - with NoFavicon possibly being the only exception - are very theme specific and would not be applicable to Core, VIP or plugin development.

So....

The original idea was that the WPTRT-CS would be a fork in the development phase, but would merge back into WPCS once the majority of development was done. This is how things are still currently set up.

But merging WPTRT-CS back into WPCS would introduce these theme specific sniffs into the WordPress ruleset which for most users of WPCS will be undesirable.

As the new theme sniffer would use Composer anyway, the WPTRT-CS repo could be transformed to be a new standard in its own right (like the VIP repo), which would pull PHPCS and WPCS in as dependencies and can use the WPCS (and PHPCS) sniffs by referencing them in the ruleset.

This will also create more clarity for maintenance of the WPTRT-CS repo as it can be unclear now for contributors when something needs to be pulled here in WPCS and when something needs to be pulled to WPTRT-CS. It would also allow the WPTRT-CS more freedom in creating their own categories for sniffs and dropping PHPCS 2.x support already in favour of PHPCS 3.x.

So, @dingo-d and me would like to hear some opinions, please vote for either of these options:

  1. Stay the original course - WPTRT-CS should at some point be merged into WPCS.
  2. Keep WPTRT-CS separate but as a fork of WPCS.
  3. Split off WPTRT-CS from WPCS to become an external PHPCS standard in its own right with WPCS as a dependency.

Your input is appreciated!

/cc @grappler @WordPress-Coding-Standards/wpcs-admins

manishsaini1126 commented 6 years ago

@Rarst can we discs now and sorry me not reply your msg

jrfnl commented 6 years ago

FYI: I've re-ordered the list of sniffable rules into "Done" and "To do" to get a better overview of where this project is at.

JDGrimes commented 6 years ago

I would be OK with option 1, but number 3 may make the most sense at this point.

GaryJones commented 6 years ago

I'm 100% behind number 3.

Just as WPCS builds on top of (has a dependency on) PHPCS by using some sniffs from it and including our own, for the benefit of the WP subsection of the PHP community, so should WPTRT-CS build on top of (have a dependency on) WPCS by using some of our sniffs and including their own, for the benefit of the TRT (and theme builders) subsection of the WP community.

acosmin commented 6 years ago

Number 3 :)

Also, I would like to see some sniffs for:

jrfnl commented 6 years ago

@acosmin Would you mind checking if there are issues open for the two sniffs you mention in the TRT fork repo ? If not, could you open issues for this there ?

justintadlock commented 6 years ago

wp_remote_*() functions. I've seen them used to get (sometimes post) lists of themes, changelogs and other stuff.

What sort of sniffs do we need for wp_remote_*()? We've traditionally allowed wp_remote_get() in themes for displaying on an admin page. A good example is having a "child themes" admin page for displaying all of the current theme's children. Even I use it in at least one theme.

jrfnl commented 6 years ago

@justintadlock @acosmin Please have the discussion about wp_remote_get() in the TRT repo.

jrfnl commented 6 years ago

FYI: the split off of the Theme Review repo from WPCS has been finalized.

The Theme Review CS repo can now be found here: https://github.com/WPTRT/WPThemeReview

Any work which was originally contained in WPCS and remains in TRTCS (most notably three WPCS deprecated sniffs, but also some dev files) has been cherrypicked to the new WPThemeReview develop to maintain credits and copyright.

A number of the open items mentioned in the original issue + in the comments have not (yet) been turned into issues in the TRTCS repo. If anyone from the TRT fancies doing so, it would be very helpful and very welcome indeed!