Add sniffs for the theme checks

[grappler]

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:

• Add the open issues from the Theme Check repo for rules which haven't got a check to this list.
• [Extra] Add a check for up-to-date version of TGMPA if TGMPA is detected.
• Get the theme review board to look over the list and where necessary correct it.
• Move ownership of the GH Theme Check plugin repo to the WordPress organisation on GH.
• Get people on the theme review team to create sniff unit tests for every single item on the list.
• For each item for which there are unit tests, someone could start creating the actual sniff.
• For any finished sniff, the 'old' check can be removed from the TC plugin.
• Once the first batch of sniffs are created, - in a develop branch, set up TC to use PHPCS and WPCS for at least part of the checks and start working on a deploy script which pulls in the dependencies via composer.

We'd need two new rulesets:

• Parent-theme
• Child-theme, were child-theme would basically be a subset of parent-theme with only the 'negative' (you are not allowed to...) rules.

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:

• ERROR | Check that capabilities are used not roles. Functions to check: get_role(), current_user_can(), current_user_can_for_blog(), user_can(), add_..._page(). [New Sniff] Check that capabilities are used not roles WPTT/WPThemeReview#27 / PR: Use capabilities not roles WPTT/WPThemeReview#36
• ERROR | Check that only add_theme_page() is used. Other add_..._page() functions not allowed. [New Sniff] Check for adding admin page other than through add_theme_page() WPTT/WPThemeReview#11 / PR: Add sniff to check for add_..._page() functions + unit tests. WPTT/WPThemeReview#13
• ERROR | No hiding of the admin bar - check if show_admin_bar( false ) is called or if add_filter( 'show_admin_bar', '__return_false' ) is somewhere in the code. [New sniff] No disabling of the admin toolbar. WPTT/WPThemeReview#1
  • WARNING | Code similar to remove_action( 'wp_head', '_admin_bar_bump_cb' );
  • WARNING | CSS check for manipulating the styling of the admin bar with for instance display: none CSS identifiers: #wpadmin and .show-admin-bar
    These last two are suggested as warning as they will be less precise and need manual verification.
• ERROR | Forbidden functions which include: eval(), ini_set, popen(), proc_open, exec(), shell_exec(), system(), passthru(), base64_decode(), base64_encode, uudecode(), str_rot13(). List need proper review when creating the sniff (also look at the original regexes in the Theme Check plugin) [New Sniff] Forbidden functions WPTT/WPThemeReview#9 / PR: Add Theme forbidden PHP functions sniff + unit tests. WPTT/WPThemeReview#10
  This can probably be an extended class of one of the related checks like WordPress.PHP.DiscouragedFunctions or Generic.PHP.ForbiddenFunctions.
• ERROR | Javascript - check for similar functions like the ones listed above. [New sniff][JS] Forbidden functions WPTT/WPThemeReview#92
• ERROR | Advertising/tracking codes in both PHP as well as JS and CSS and any other potential files which contain this type of code. Things like cx=[0-9]{21}:[a-z0-9]{10} or pub-[0-9]{16} [Implement sniff] Disallow advertising/tracking codes WPTT/WPThemeReview#125
• WARNING | Using a CDN is discouraged. All JS and CSS should be bundled. For a list of typical source strings to look for, see Theme-Check plugin - /checks/cdn.php [New sniff] Using a CDN is discouraged WPTT/WPThemeReview#20 / PR: No CDN Allowed WPTT/WPThemeReview#64
• ERROR | Check for usage of deprecated WP constants. [New sniff] Check for disallowed theme related WP constants WPTT/WPThemeReview#23 / PR: Add sniff for deprecated Theme related WP Constants. WPTT/WPThemeReview#30
  For details, see https://github.com/Otto42/theme-check/pull/162/files
  We could probably extend an existing sniff of similar ilk for this.
• ERROR | Ensure that all calls the ->add_setting() for the Customizer include a sanitize_callback or a sanitize_js_callback parameter and that it's non-empty.
  An exception will need to be made for the (two) calls found in Kirki_Settings as they do correctly comply, but are wrappers for the real call. Check Customizer sanitize-callback is not empty WPTT/WPThemeReview#68 / PR: No Sanitize Callback WPTT/WPThemeReview#76
• ERROR | Check for usage of deprecated WP functions (required).
  For details, see Theme-Check plugin - /checks/deprecated.php Add sniffs for deprecated WP features #576 / PR: Modularizing the discouraged functions #633
  We could probably extend an existing sniff of similar ilk for this.
• WARNING | Check for usage of deprecated WP functions (recommended - only for recent changes, i.e. deprecation within the last three WP versions). Add sniffs for deprecated WP features #576 / PR: Modularizing the discouraged functions #633
  How many versions back will need to be checked with the theme review board and might actually need to be challenged as the current 2/3 versions is based on when WP was only updated approx once a year from what I gather.
  For details, see Theme-Check plugin - /checks/dep_recommended.php
  We could probably extend an existing sniff of similar ilk for this.
• WARNING | Verify that wp_deregister_script() isn't called. [New sniff] Verify that wp_deregister_script() isn't called WPTT/WPThemeReview#21 / PR: Add rule to prevent deregistering scripts. WPTT/WPThemeReview#26
  This is basically meant to only check that core scripts aren't being deregistered, however maintaining a list of core scripts for that purpose would be a maintenance nightmare, so returning a warning when any such call is encountered is the current solution.
• ERROR | Verify that the theme is not auto-generated, see Theme-Check plugin - /checks/generated.php for sample phrases to search for.
  Discussion needed in the Theme review board on whether this lists needs to be expanded with other generators and if so, which. Verify that the theme is not auto-generated WPTT/WPThemeReview#63 / PR: No Auto Generated Themes WPTT/WPThemeReview#65
• WARNING | Verify that no favicon / Apple icon / Windows tile / Android whatever they call it is being added from the theme. The current check is in Theme-Check plugin - /checks/favicon.php, but could definitely use some fine-tuning and improvement. Use core favicon WPTT/WPThemeReview#70 / PR: No Favicon WPTT/WPThemeReview#78
• ERROR | Verify that the i18n functions are used properly => use existing sniff from WPCS. [Implement sniff] Verify that the i18n functions are used properly WPTT/WPThemeReview#31 / PR: Check that the I18N functions are used correctly. WPTT/WPThemeReview#32
• WARNING | Check for the use of <iframes (often used for malicious code). [New sniff] Check for the use of iframes WPTT/WPThemeReview#22 / PR: Feature/discouraged iframe WPTT/WPThemeReview#109
• WARNING (manual check required) | Check if a theme uses include(_once) or require(_once) (where they should use get_template_part()). Warning for using include and require WPTT/WPThemeReview#66 / PR: Warning for using include or require WPTT/WPThemeReview#67
  Current implementation excluded the functions.php file from this check. We may want to continue doing so.
• ERROR | Verify that line endings are LF only. Using Generic.Files.LineEndings should be sufficient for this. [Implement sniff] Have either UNIX or DOS line endings for PHP, JS and CSS files WPTT/WPThemeReview#3 / PR: Add rule to only allow UNIX style line endings. WPTT/WPThemeReview#7 Mark mixed line endings as error WPTT/WPThemeReview#90
• WARNING | Verify that no hard-coded URLs are found. Manual verification necessary.
  An exception is made for the Author URI and (Theme) URI as set in the style.css header as well as links to wordpress.org. [New Sniff] Check for hardcoded URLs WPTT/WPThemeReview#14 / PR: Check for Hardcoded Urls WPTT/WPThemeReview#28
  Links in the text of PHP/JS comments should be excluded from this check.
• WARNING | Similar to deprecated/forbidden functions check, verify that file system calls use the WP_Filesystem method and not PHP native functions. No php file functions WPTT/WPThemeReview#71
  For a list of functions to trigger on, see Theme-Check plugin - /checks/malware.php
• ERROR | Check for usage of deprecated WP functions and provide alternative based on the parameter passed. Check for deprecated function args WPTT/WPThemeReview#72 / PR: Theme Restricted Parameter WPTT/WPThemeReview#80
  For details, see Theme-Check plugin - /checks/more_deprecated.php
  We could probably extend an existing sniff of similar ilk for this or even combine this with the other deprecated checks.
• ERROR | In the (register|wp)_nav_menu() function, the theme should only use a variable for the menu name. No hard-coded menu names allowed. [New sniff] No hard-coded menu names in nav_menu functions WPTT/WPThemeReview#93
• WARNING | Check for non-printable characters [\x00-\x08\x0B-\x0C\x0E-\x1F\x80-\xFF]. Quite possibly there will be an existing sniff somewhere we could re-use for this. [New sniff?] Check for non-printable characters WPTT/WPThemeReview#94
• ERROR | No short open tags allowed. = the existing Generic.PHP.DisallowShortOpenTag sniff. [Implement sniff] No PHP short tags WPTT/WPThemeReview#2 / PR: Add rule to disallow PHP short open tags. WPTT/WPThemeReview#6
• ERROR | No alternative opening tags allowed. [Implement sniff ?] No alternative PHP open tags ? WPTT/WPThemeReview#15 / PR: Add rule to disallow alternative PHP open tags. WPTT/WPThemeReview#16
  Check if there is an existing sniff for the alternative opening tags, if not, I'd suggest opening a PR upstream at PHPCS to cover this in a separate sniff in Generic or add to extend the above mentioned sniff to cover this.
• ERROR | No short open tags allowed for XML.
  Extend the existing Generic.PHP.DisallowShortOpenTag sniff to an XML specific one. [New sniff] No short open tags in XML WPTT/WPThemeReview#95
• ERROR | The following three functions are not allowed (plugin territory): register_post_type(), register_taxonomy(), add_shortcode(). Check for functions that are plugin territory WPTT/WPThemeReview#73 / PR: Plugin Territory Functions WPTT/WPThemeReview#81
  Review this list with the Theme Review board as there might be some more functions which could be added.
  The sniff could probably just extend the Forbidden Functions sniff - though it should be kept as a separate sniff for clarity.
• ERROR | check that no include calls to searchform.php are found, if they are, recommend using get_search_form() instead. Check for direct load of searchform.php WPTT/WPThemeReview#74 / PR: Direct including of template files WPTT/WPThemeReview#82
• ERROR | Check specifically against style.css whether the required headers are found. See Theme-Check plugin - /checks/style_needed.php for the list.
• WARNING | Check specifically against style.css whether the two recommended headers are found. See Theme-Check plugin - /checks/style_suggested.php for the list.
• ERROR | Check that any Tags in the style.css file header comply with the current guidelines (allowed tags, deprecated tags (=> WARNING) etc). See Theme-Check plugin - /checks/style_tags.php for the list.
• WARNING | Check if the accessibility tag is used in the style.css file header tag list and add a warning that an accessibility review is needed. [New sniff] Check for the accessibility-ready tag WPTT/WPThemeReview#24
• ERROR | Check for incorrect usage of certain parameters in WP functions (hard-coded) and provide valid alternative based on the parameter passed. [New sniff] Incorrect function parameter usage WPTT/WPThemeReview#96
  For details, see Theme-Check plugin - /checks/time_date.php
• ERROR | Check that <title.. isn't used except for in inline SVG code. [New sniff] Check for <title> WPTT/WPThemeReview#25 / PR: Add sniff to check that <title> tags are not used.  WPTT/WPThemeReview#41
• ERROR | Check that there are no calls to wp_title(). [New sniff?] No calls to wp_title() WPTT/WPThemeReview#97
• ERROR | Check in style.css for the Author URI and Theme URI and verify that these are not the same.
• ERROR | Verify - in style.css that the Theme URI does not point to wordpress.org (with predefined list of themes which are exempt and live under the wordpressdotorg user or have a check based on Author name - see the current check in checks/uri.php).
• ERROR | Verify a number of typical php snippets which are known as malware indicators, see Theme-Check plugin - /checks/worms.php [New sniff] Malware/worm sniff WPTT/WPThemeReview#98

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:

• ERROR | Verify that an add_theme_support() call is made for any feature the theme has been tagged with from the following list: custom-background, custom-header, custom-menu, featured-images/post-thumbnails, post-formats, custom-logo
• ERROR | Check that the comment reply script is being enqueued (comments should always be supported by themes).
• ERROR | Check that the comment_reply string or rather any HTML identifiers needed for the JS script to work are present (need more info) (comments should always be supported by themes, enqueuing the script alone is not enough)
• ERROR | Check that comment pagination is supported. At least one of the following functions would need to be found in at least one of the template files, fail if none are found at all. paginate_comments_links(), the_comments_navigation(), the_comments_pagination(), next_comments_link() or previous_comments_link()
• Similar to the above a check is needed that the 'threaded-comments' theme tag is only used if the theme actually supports threaded comments.
• ERROR | Check that - normally in functions.php, but could be in another file - the global variable $content_width is set, so either in the global namespace using $content_width or within a function using global $content_width; $content_width =... or $GLOBALS['content_width'].
  Note: currently the Theme Check plugin also checks for filters on embed_defaults and content_width and passes if those are found. Those checks are outdated and should not be ported.
• ERROR | Verify that an add_editor_style() call is made if the theme has been tagged with editor-style.
• ERROR | Verify that get_avatar() or wp_list_comments() is used at least once.
• WARNING | Verify that there are max one link each to the author's website and one link to wordpress.org in front-end visitor facing template, e.g. footer.php or similar.
• WARNING | Verify that (register|wp)_nav_menu() is used at least once.
  This should become an error if the theme is tagged with custom-menu.
• ERROR | Verify that (get_post_format()|has_format() or CSS rules covering .format are found, at least once if the theme has a add_theme_support( 'post-format' ) call.
  This should become an error if the theme is tagged with post-formats.
• ERROR | Check that post pagination is supported. At least one of the following functions would need to be found in at least one of the template files, fail if none are found at all. posts_nav_link(), paginate_links(), the_posts_navigation(), the_posts_pagination(), next_posts_link() or previous_posts_link()
• 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.
• ERROR | Check if a number of specific CSS identifiers have been given styles in any of the CSS files. See Theme-Check plugin - /checks/style_needed.php for the list.
• ERROR | Check that post tags are supported in the theme. At least one of the following functions would need to be found in at least one of the template files, fail if none are found at all. the_tags(), get_the_tag_list(), get_the_term_list()
• WARNING | Check on the number of unique text domains encountered. If one => ok, if two => warning, if three => error.
• ERROR | Check that add_theme_support( 'title-tag' ) is used in at least one file.
• WARNING | Check if at least one call to register_sidebar() or dynamic_sidebar() is made.
• ERROR | If a call to register_sidebar() is found, make sure there is at least one call to dynamic_sidebar() as well and visa versa.
• ERROR | Check that the register_sidebar() function is called with an add_action( 'widget_init', ... ) call.
• ERROR | Check for a number of function calls which each theme has to contain. See Theme-Check plugin - /checks/basic.php for the list.
• ERROR | Check that the theme contains a DOCTYPE headers somewhere.

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

• WARNING | Have a default (always on) INFO item which will warn people not to use their own functions for features which should be supported through add_theme_support().
• ERROR | Check for extraneous directories, such as .git, .svn, .hg, .bzr. These should not exist in the repo.
  Exact list of exclusions should be re-determined in conjunction with the theme review board. This should include both system as well as development related directories.
• ERROR | Check for extraneous files, like thumbs.db, .DS_Store, travis config, PHPCS config .zip files etc. For a list, see Theme-Check plugin - /checks/filenames.php. These should not exist in the repo. Zips are often plugin files and are not allowed to be shipped with a theme either.
  Exact list of exclusions should be re-determined in conjunction with the theme review board. Again, this should include both system as well as development related files.
• ERROR | Check that at the very least the following two files exist: index.php and style.css.
• WARNING | Check that a readme.txt file exists.
• WARNING | Verify that the text-domain used is the same as the theme slug. This can't use the custom setting in the phpcs.xml file as that file will not be allowed to be included in themes (to prevent people excluding rules).
  The text-domain should also be in the style.css file header and again be consistent.
• ERROR | Check that at least one screenshot is found.
• ERROR | Check that the screenshot is either a jpg or png.
• ERROR | Check that the screenshot is smaller than 1200x900, has a 4:3 size ratio.
• WARNING | Recommend a screenshot size of 1200 x 900 if the screenshot is smaller.

[justintadlock]

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:

• get_the_post_thumbnail()
• get_post_thumbnail_id()

[jrfnl]

FYI:

• Initial development of these sniffs will take place in https://github.com/WPTRT/WordPress-Coding-Standards/tree/feature/theme-review-sniffs
• Issues will be opened in the WPTRT fork for each item to keep track of them all and measure progress.
• If you want to contribute to this effort, for now, please send in pull requests against the above mentioned branch and fork.
• Once an initial set of sniffs has been created, the feature branch will be pulled against WPCS for review and merge.

[manishsaini1126]

I SOLVE YOUR PROBLEM I HAVE DONE SAME WORK IN PAST

[Rarst]

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

[manishsaini1126]

can you please briefly describe your problem

[Rarst]

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

[jrfnl]

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:

• FileInclude - discourage use of include(_once) and require(_once) as in themes, more often than not get_template_part() should be used instead.
• NoAddAdminPages - forbid adding admin pages anywhere but in the Theme submenu.
• NoAutogenerate - forbids auto generated themes.
• NoFavicon - checks for hardcoded favicons instead of using the WP core implementation.
• NoTitleTag - forbid the use of the <title> tag as add_theme_support( 'title-tag' ) should be used instead.
• PluginTerritory - forbid the use of functions which add functionality and should be in plugins instead, think add_shortcode() and register_post_type().

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]

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

[jrfnl]

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]

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

[GaryJones]

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]

Number 3 :)

Also, I would like to see some sniffs for:

• wp_remote_*() functions. I've seen them used to get (sometimes post) lists of themes, changelogs and other stuff.
• registering/displaying WordPress dashboard widgets.

[jrfnl]

@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]

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]

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

[jrfnl]

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!