WPThemeReview input/decision needed issues

One of the thing that helps theme reviewers and theme authors check and write better code is having automatic checks in place that will warn them if the code they wrote is compliant with Theme Review Team rules.

This is why we are working on WPThemeReview coding standards. But this ruleset is not finished and has some issues that we need help with. That is why we have a list of issues in the WPThemeReview standards with input/decision needed flag.

I will be pasting these issues with some comments, so that we can get some more insights about theme before making some decisions in the meetings.
Feel free to leave a comment on any of those, what you think needs to be done to get some clear resolution so that we can start writing sniffssniff A module for PHP Code Sniffer that analyzes code for a specific problem. Multiple stiffs are combined to create a PHPCS standard. The term is named because it detects code smells, similar to how a dog would "sniff" out food. 🙂

Issues list

Variables in template files are flagged as global

Part of this issue has been resolved with the modified PrefixAllGlobals sniffsniff A module for PHP Code Sniffer that analyzes code for a specific problem. Multiple stiffs are combined to create a PHPCS standard. The term is named because it detects code smells, similar to how a dog would "sniff" out food., but a decision needs to be made about GlobalVariablesOverride sniff which will warn about overwriting WordPress native global variables – and in templates (loaded using coreCore Core is the set of software required to run WordPress. The Core Development Team builds WordPress. functions) the variables such as $title, $post, $page etc.

Maybe we could extend the GlobalVariablesOverride sniff in the similar fashion as PrefixAllGlobals sniff and exclude that check if the file name matches the in the allowed folder or are coming from the template files.


Add EnqueuedResourceParameters sniff to the WPThemeReview ruleset

WPCSWordPress Community Support A public benefit corporation and a subsidiary of the WordPress Foundation, established in 2016. 1.0.0 added a new WordPress.WP.EnqueuedResourceParameters sniff which checks that when registering/enqueuing scripts and styles, a $version is passed correctly so new versions will break out of the browser cache and for scripts, that the $in_footer parameter is passed to prevent unnecessary layout-rendering blocking scripts.
Should this be added in WPThemeReview ruleset at all, and if yes, should it be a warning or an error?

While having $version parameter can be beneficial (cache busting), with modern development workflows (gulp, webpack), it’s not needed. But we don’t know how many theme authors are using these tools, so adding this as a warning may not be a bad idea.


Proposal to add additional rulesets to WPThemeReview

Separate ruleset in different rulesets like in WPCS (WordPress-Core, WordPress-Extra etc.). Similarly the WPThemeReview would have

WPThemeReview-Required with everything which is in the handbook.
WPThemeReview-Extra with sniffs covering additional best practices and sniffs which cover rules which have been proposed to the TRT, but have not (yet) been approved.

In practice this would result in the following:

WPThemeReview – the Theme native sniffs would live in this directory and the ruleset would include the Required and Extra rulesets
WPThemeReview-Required – directory with only a ruleset
WPThemeReview-Extra – directory with only a ruleset. Ruleset would include WPThemeReview-Required and references to additional sniffs.

Naming of these need to be decided and a list of extra rules we’d recommend would need to be added.


Discourage CSS rules for images setting width:auto

Flag CSSCSS CSS is an acronym for cascading style sheets. This is what controls the design or look and feel of a site. rules for images that have the property width: auto.
There is no rule in the handbook for this, but it falls under the general guideline to honor the user’s choices.

The default value for CSS width is auto, so there is no need to specify it. When it is specified, it will override the HTMLHTML HTML is an acronym for Hyper Text Markup Language. It is a markup language that is used in the development of web pages and websites. width attribute. So an img tag with a different width attribute than the source image is would be affected, and it will display as the size of the source image instead of the size the user requested.

Additional points to consider about this are

  • The CSS tokenizer is proposed to be removed in PHPCS 4.0, so while we can still sniff CSS files at this time, there’s no guarantee this can still be done in the future.
  • CSS within inline HTML in PHPPHP PHP (recursive acronym for PHP: Hypertext Preprocessor) is a widely-used open source general-purpose scripting language that is especially suited for web development and can be embedded into HTML. http://php.net/manual/en/intro-whatis.php. files can be checked, though if the value for width is set in a variable or constant, it will not be possible to sniff this in a reliable manner.

Adding metadata sniffs

We need sniffs that check metadata. Currently, the theme review team allows themes to have metadata if it is presentational in nature. Some common examples:

  • SidebarSidebar A sidebar in WordPress is referred to a widget-ready area used by WordPress themes to display information that is not a part of the main content. It is not always a vertical column on the side. It can be a horizontal rectangle below or above the content area, footer, header, or any where in the theme. position
  • Background color
  • Layout

However, we do not allow metadata that is non-presentational in nature. Some common examples:

Video URLURL A specific web address of a website or web page on the Internet, such as a website’s URL www.wordpress.org
Gallery shortcodeShortcode A shortcode is a placeholder used within a WordPress post, page, or widget to insert a form or function generated by a plugin in a specific location on your site.
User-specific social URLs

This would probably need to be an warning-level notice because we wouldn’t be able to know whether the metaMeta Meta is a term that refers to the inside workings of a group. For us, this is the team that works on internal WordPress sites like WordCamp Central and Make WordPress. is presentational or not via code.


Disallow hiding or removing the themes section of the customizer

The theme section in the customizerCustomizer Tool built into WordPress core that hooks into most modern themes. You can use it to preview and modify many of your site’s appearance settings. shows the name of the active theme and the Change button where the user can preview and select another theme.

By hiding or removing the section authors can lock users in.

A decision needs to be made for this, if this doesn’t exist in the handbook, a rule needs to be added, then the examples can be added and tested against when creating a sniff.


Passing dynamic information to sniffs

Adding an optional way to check for things like text domain when running PHPCSPHP Code Sniffer PHP Code Sniffer, a popular tool for analyzing code quality. The WordPress Coding Standards rely on PHPCS. using CLICLI Command Line Interface. Terminal (Bash) in Mac, Command Prompt in Windows, or WP-CLI for WordPress., or in the Theme SnifferTheme Sniffer Theme Sniffer is a plugin utilizing custom sniffs for PHP_CodeSniffer that statically analyzes your theme and ensures that it adheres to WordPress coding conventions, as well as checking your code against PHP version compatibility. The plugin is available from GitHub. Themes are not required to pass the Theme Sniffer scan without warnings or errors to be included in the theme directory. pluginPlugin A plugin is a piece of software containing a group of functions that can be added to a WordPress website. They can extend functionality or add new features to your WordPress websites. WordPress plugins are written in the PHP programming language and integrate seamlessly with WordPress. These can be free in the WordPress.org Plugin Directory https://wordpress.org/plugins/ or can be cost-based plugin from a third-party. Another thing to add would be the theme tags parameters, and then if a theme uses add_theme_support() but the tag is not, the sniffer would throw an error.

Input is needed about what additional dynamic variables could be checked in this way, what sniffs need to be added and decide on names of the sniffs.


Add malware/worm sniff

Verify a number of typical php snippets which are known as malware indicators

The list of snippets might need to be expanded.

The rule should be checked if exists in the handbook, if not add it. Then we need examples, and a sniff name can be made (based on the handbook rule) and created.


Incorrect function parameter usage

Check for incorrect usage of certain parameters in WP functions (hard-coded) and provide valid alternatives based on the parameter passed.

To create this sniff, a list of function should be compiled where theme authors are typically doing this wrong, i.e. passing hardcoded information instead of variables/function calls.

Which function and parameters should be covered by this sniff? Input needed.


Check for direct load of searchform.php

Check to ensure searchform.php is not loaded directly.

This was a WIP but since we’ve branched off to a separate ruleset was not worked on.

We need to see if there are other violations like this and maybe include them in a more general sniff that would cover the required rule in the handbook.


Check that all required headers in style.css are there

This sniff proposal needs an input based on the fact that CSS sniffs may be deprecated/removed in PHPCS 4.0. So we would be doing a double work. Maybe we should focus on moving CSS/JS issues to Theme Sniffer plugin, so that they can be checked using eslint and stylelint or some similar tool.


Make sure widget_title filter has all parameters

This sniff would check if the correct number of parameters is passed to widget_title.

This issue raises a number of questions:

  • Is this the only filterFilter Filters are one of the two types of Hooks https://codex.wordpress.org/Plugin_API/Hooks. They provide a way for functions to modify data of other functions. They are the counterpart to Actions. Unlike Actions, filters are meant to work in an isolated manner, and should never have side effects such as affecting global variables and output. where a check like this would need to take place or are there / will there be more actions/filters where the parameters will need to be checked ?
  • Does the check need to be for the amount of parameters passed or also need to go into the content ? (If the answer would be ‘content’, that would be neigh impossible as variables can be named differently across themes.)

Questions from an implementation perspective:

  • Do we need to create a re-usable abstract class which handles a large part of the logic and can be extended for individual actions/filters or small groups thereof?
  • If this will be the only one, we can just use a hard-coded check, if not, we may need to have an extensibleExtensible This is the ability to add additional functionality to the code. Plugins extend the WordPress core software. array property listing the hooksHooks In WordPress theme and development, hooks are functions that can be applied to an action or a Filter in WordPress. Actions are functions performed when a certain event occurs in WordPress. Filters allow you to modify certain functions. Arguments used to hook both filters and actions look the same. or in the case of the abstract, possibly a function instead.
    Also: is there a rule in the handbook covering this or should it be added ?

Check that only one – or at most two – text-domains are used in the theme

We need to see how to implement this into a sniff – what direction we want to go with this?


Check that all text strings are translated and in the same language

The “check that all translatable strings are in the same language” bit may be sniffable if we could do some sort of dictionary check, possibly using the PHP ICU extension, but the “check that all text strings are translatable” bit is neigh impossible to sniff for.

We need a decision about this, do we want to pursue this or not.


Using a CDN is discouraged

Using a CDN is discouraged. All JS and CSS should be bundled.

It would be really helpful for finishing this off to have code samples of both things which should be flagged as well as things which shouldn’t be. The more variety in the code samples, the better!


Check for removal of admin pages

Themes can remove admin pages from, for instance, a parent theme, but they are not allowed to remove WP Core admin pages, so there is a lot more involved in that.

Rule should be added in the handbook about this, and a sniff needs to be worked on.

Conclusion

These are issues where we need some decision on. Either updating/adding rule in the handbook, naming decision and most important of all: code examples.

The more code examples we have, the quicker and easier it is to write sniffs.

Additionally, if you have time and can go through the open issues and think that some of those could be a good first issue to solve that would be great, as we could work on them in the upcoming WCEU contributor dayContributor Day Contributor Days are standalone days, frequently held before or after WordCamps but they can also happen at any time. They are events where people get together to work on various areas of https://make.wordpress.org/ There are many teams that people can participate in, each with a different focus. https://2017.us.wordcamp.org/contributor-day/ https://make.wordpress.org/support/handbook/getting-started/getting-started-at-a-contributor-day/..

#automation, #rules, #sniffs, #wpthemereview