Extra. Also annoying, because totally context senseless - assigning to the variable doesn't quite make sense in places like middle of array declaration.

All functions should include a PHPDoc comment block which indicate the parameters and return value, along with a description of what the function does.

Example. Note: curly braces should not be used around type statement eg: @param {array} $some_array These can not be parsed by PHPDocumenter.

//Correct
/**
 * Sets the post thumbnail on the list table
 *
 * @param int $post_id
 * @param string $size, The thumbnail size to use (optional)
 * @return bool|string, post thumbnail if post has thumbnail, false if not
 */
function wpized_list_thumbnail( $post_id, $size='medium' ) {

Reported by @c3mdigital

We should be using PHP template tags and not concatenating strings into variables for printing.

Introduce additional opt-in rulesets for extra sniffs which guide development toward best-practices.

Following on from Eric's comment, we should look through the other sniffs to check for other possibly confusing docstrings.

It is easy to sneak in logic into templates. I think a handy way to remind yourself not to do this is to always use PHP's alternative syntax for control structures in WP templates: http://www.php.net/manual/en/control-structures.alternative-syntax.php

The use of this syntax lends itself much better to meshing with markup since the tags are much more apparent, and it makes it much easier to indent with markup. Lastly, it is much more readable to see <?php endif; ?> then just <?php } ?>

The original sniffs were written for phpcs 1.3. Now version 1.4 is out, and we need to test to make sure that the sniffs continue to work as expected.

Currently the ! in the following snippet fails, complaining that there is not one space before:

$is_complicated_conditional = (
    false !== some_function()
    ||
    ! some_other_function()
);

Functions, classes, variables, and constants should all have unique prefixes to prevent collisions.

Reported by @mwic: I make frequent use of comments on else control structures , so that I don't have to scroll around to see what condition is being else'd. e.g.
if ( $parrotIsAlive ) {
[snip]
} else { //parrot has ceased to be
[snip]
}

With your standard I get a misleading message on this attempt.
"Blank line found at start of control structure"
Whether or not the comment is allowed, the message needs to be fixed

Full writeup:
http://mwic.org/wp/index.php/blank-line-found-at-start-of-control-structure-when-there-isnt-a-blank-line-there

https://gist.github.com/mwic/5248309

On 21 March 2013, Tom McFarlin posted that he contributed the first ever WordPress JavaScript Coding Standards:

Anyway, one of the things about the WordPress Coding Standards that’s always seemed incomplete to me is how little it focused on JavaScript. It provides guides for PHP, for HTML, and it even has a stub for CSS, but there’s nothing in the Codex about JavaScript.

For the past few months, I’ve been working on exactly that. Today, I contributed to the WordPress Coding Standards by introducing the WordPress JavaScript Coding Standards.

The JavaScript Coding Standards have been moved over to the Core Contributor Handbook: http://make.wordpress.org/core/handbook/coding-standards/javascript/

We should look at creating sniffs specifically for enforcing the JavaScript rules.

The current installation instructions depend on pear and git. Suggestion to use pear is taken by some people a little worse than suggestion to stab themselves in the face. Git is better, but might not be immediately available in beginner circles.

PHP CS currently has three ways to install documented: pear, composer and git (however only pear seems to produce correct bat files on Windows :(. It would make sense to extend installation instructions here to mention those or simply drop them completely and link off to PHPCS original instructions.

As possibility we could potentially put together Composer package that will bundle PHPCS and WP standard together for one-command (or one phar file) install.

Thoughts?

When creating new WP_Query instances and looping over them, it is important that the global post variables are reset to be reflect $wp_the_query. A sniff can check for instantiation of WP_Query followed by use of have_posts() and the_post() and if after this loop there does not exist a wp_reset_postdata(), there should be a warning flagged.

print_r() and var_dump() forbidden, really? :)

I understand it might make sense to sniff for them in production code, but just gets in the way in development and is for sure extra territory.

Also while sniff is called "discouraged" functions it seems to output messages with "forbidden" version of wording.

Sniff FunctionCallSignatureSniff does not allow to override type in ruleset.xml because $type variable is set to private

Scripts and stylesheets should not be hard-coded in templates. On the contrary, they should be registered using wp_register_script() and wp_register_style() during the wp_enqueue_scripts action. If a stylesheet <link> is hard-coded in the <head> then they are doing it wrong, or if a <script src="..."> appears in the footer, they are doing it wrong.

In order to be really useful, it should be possible to only report errors and warnings on lines that have been touched, and ignore the lines that have already been there. This is key for use with Core development.

A wrapper script could be provided which invokes phpcs, but which filters out lines from the report that do not correspond with uncommitted changes.

Is there a way to ignore parts of Sniffs? For example, I would like to ignore this rule: 'Blank line found at start of control structure' found here:

$firstContent = $phpcsFile->findNext(T_WHITESPACE, ($scopeOpener + 1), null, true);
if ($tokens[$firstContent]['line'] !== ($tokens[$scopeOpener]['line'] + 1) && $tokens[$firstContent]['code'] != T_CLOSE_TAG) {
    $error = 'Blank line found at start of control structure';
    $phpcsFile->addError($error, $scopeOpener);
}

$lastContent = $phpcsFile->findPrevious(T_WHITESPACE, ($scopeCloser - 1), null, true);
if ($tokens[$lastContent]['line'] !== ($tokens[$scopeCloser]['line'] - 1)) {
    $errorToken = $scopeCloser;
    for ($i = ($scopeCloser - 1); $i > $lastContent; $i--) {
        if ($tokens[$i]['line'] < $tokens[$scopeCloser]['line'] && $tokens[$firstContent]['code'] != T_OPEN_TAG) {

            $error = 'Blank line found at end of control structure';
            $phpcsFile->addError($error, $i);
            break;
        }
    }

}

You can also run the sniffs against our repos to identify areas where the rules are incorrect or lacking, or where we consistently violate the rules, and also where the sniffs miss.

Create new issues here for the needed improvements discovered.

Originally reported at http://pear.php.net/bugs/bug.php?id=19162 but as https://github.com/pear/PHP_CodeSniffer/tree/master/CodeSniffer/Standards/PEAR/Sniffs doesn't appear to have a Arrays directory (as I assumed the WP were adapted from the PEAR CS) then it's worth reporting here too.

Description:

Running PHP_CodeSniffer via a phing build on Jenkins, I get that one of the arrays has no trailing comma, yet I believe it has.

It's only a single item in the array, and the value of it is multiple nested function calls which itself includes an array.

I've got a similar bit of code lower down in my file which also produces the same error.

Test script:

$actions = array(
    'install' => sprintf( // Line 1179
        '<a href="%1$s" title="Install %2$s">Install</a>',
        wp_nonce_url(
            add_query_arg(
                array(
                    'page'          => TGM_Plugin_Activation::$instance->menu,
                    'plugin'        => $item['slug'],
                    'plugin_name'   => $item['sanitized_plugin'],
                    'plugin_source' => $item['url'],
                    'tgmpa-install' => 'install-plugin',
                ),
                admin_url( TGM_Plugin_Activation::$instance->parent_url_slug )
            ),
            'tgmpa-install'
        ),
        $item['sanitized_plugin']
    ), // <-- This comma not seen?
);

Expected result:

I'd expect no array trailing comma warning to appear.

Actual result:

class-tgm-plugin-activation.php:1179, ArrayDeclaration, Priority: High
Each line in an array declaration must end in a comma

Hello,

Being relatively new with this I hope it would be OK to ask in perhaps not that many technical terms.

I was wondering on how to write this snippet best:

add_action( 'init', 'create_secondary_taxonomies' );
function create_secondary_taxonomies() {
  $labels = array(
    'name' => _x( 'Mogelijkheden', 'taxonomy general name' ),
    'singular_name' => _x( 'Mogelijkheid', 'taxonomy singular name' ),
  );
  register_taxonomy(
    'mogelijkheid',
    array( 'product' ),
    array(
      'labels' => $labels,
      'hierarchical' => true,
      'query_var' => true,
      'rewrite' => array(
        'hierarchical' => true,
        'slug' => 'mogelijkheid',
        'with_front' => false,
      ),
      'public' => true,
      'show_ui' => true,
    )
  );
}

as I'm currently receiving this error:

FOUND 3 ERROR(S) AFFECTING 3 LINE(S)
--------------------------------------------------------------------------------
  9 | ERROR | Multi-line function definition not indented correctly; expected 6
    |       | spaces but found 4 (WordPress.Functions.FunctionCallSignature)
 10 | ERROR | Multi-line function definition not indented correctly; expected 6
    |       | spaces but found 4 (WordPress.Functions.FunctionCallSignature)
 11 | ERROR | Multi-line function definition not indented correctly; expected 6
    |       | spaces but found 4 (WordPress.Functions.FunctionCallSignature)
--------------------------------------------------------------------------------

I can understand why 4 spaces, but I'm somewhat lost on why 6.

Now that we have this new organization for WordPress-Coding-Standards, we should consolidate issues from the forks here. Specifically, there are lots of issues have have been created on the x-team fork: https://github.com/x-team/WordPress-Coding-Standards/issues

A tool have been developed to do this: https://github.com/mkorenkov/tools/blob/master/gh-issues-import/gh-issues-import.py

The use of spaces around if and foreach and other constructs is not consistent. It would be good if we could force the same style with a sniff.

We could introduce a new WP-CLI subcommand called lint or phpcs which would apply the sniffs in this project to the WordPress install in the current scope.

I'm not sure of the best route to go as for integrating the sniffs into WP-CLI. Perhaps it would mean forking WP-CLI https://github.com/wp-cli/wp-cli and then adding this repo as a submodule dependency.

http://www.jetbrains.com/phpstorm/webhelp/using-php-code-sniffer-tool.html

I have downloaded and installed the latest version of phpcs and these WordPress standards. I am running this with Netbeans and XAMPP on a Windows 7 machine. In Netbeans, phpcs works fine with the PEAR standard, but with the WordPress standard it gives this error: [Fatal Error] :1:1: Content is not allowed in prolog.

I ran phpcs.bat from the Windows command line, and the WordPress standard works fine, except that I see 2-3 bytes of garbage characters at the very beginning of the report. Those bytes are not there with the PEAR standard. So I suspect that this is the problem. A few bytes of something are getting output before the XML

Relates to XSS checking script, but we should restrict it further.

Code should not be directly accessing the CRUD methods of $wpdb since there should be WordPress API functions already available to do almost everything normally done.

We could allow for a PHP comment to be added which would direct PHPCS to not flag usage of $wpdb as an error. This is what was done with the EscapeOutputSniff and // xss ok.

In terms of the caching requirement, after an instance of a CRUD method of $wpdb is used, the sniff could then walk over the remainder of the function to check if wp_cache_set is used, and if wp_cache_get appears before. If not, then this would be a caching warning.

• Flag any use of $wpdb methods
• Add facility to whitelist usages
• Check for presence of wp_cache_set and wp_cache_get in the scope of any function using $wpdb

Direct Database Queries #

Thanks to WordPress’ extensive API, you should almost never need to query database tables directly. Using WordPress APIs rather than rolling your own functions saves you time and assures compatibility with past and future versions of WordPress and PHP. It also makes code reviews go more smoothly because we know we can trust the APIs. More information.

Additionally, direct database queries bypass our internal caching. If absolutely necessary, you should evaluate the potential performance of these queries and add caching if needed.

I needed to print output using wp's wp_kses_post but I was getting an error:

Expected a sanitizing function (see Codex for 'Data Validation'), but instead saw 'wp_kses_post'

Seems the wp_kses_post function was missing from the $sanitizing_functions array

The following are also defined as sanitizing functions in wordpress codex: data validation:

• wp_kses_post
• wp_kses_data
• wp_kses_allowed_html
• esc_sql
• like_escape
• validate_file
• wp_redirect
• wp_safe_redirect
• sanitize_title
• sanitize_user
• balanceTags
• sanitize_html_class
• is_email

not sure if this is a duplicate of:
#33

Also, not sure of the contributor rules but need this for work so going to fork and create a feature branch.

Hi!

Theres is a typo on the README.md file.

Where it says:

git clone git://github.com/mrchrisadams/Wordpress-Coding-Standards.git $(pear config-get php_dir)/PHP/CodeSniffer/Standards/WordPress

It should read:

git clone git://github.com/mrchrisadams/WordPress-Coding-Standards.git $(pear config-get php_dir)/PHP/CodeSniffer/Standards/WordPress

(note the uppercase P on WordPress-Coding-Standards.git)

If an if, while, for, foreach statement lacks a body that is enclosed in a block, in other words a single-line body, then we should flag this as a warning.

The following does not validate:

$foo = 'abc';
echo $foo;

I have to use this workaround:

$foo = 'abc';
echo '' . $foo;

Is this by design?

On a side note, this also does not validate, which is the translation gettext function:

$foo = 'abc';
echo _($foo);

Likewise show how to restrict reporting to just files modified or staged.

It doesn't work so well if you have comments or function calls appearing in the array.

<?php
$foo = array(
    'bar' => array_merge(
        array(
            'x' => 1,
        ),
        array(
            'y' => 2,
        )
    ),
    'baz' => function () {
        echo 'Jello World';
    },
);

--------------------------------------------------------------------------------
FOUND 2 ERROR(S) AFFECTING 2 LINE(S)
--------------------------------------------------------------------------------
  3 | ERROR | Each line in an array declaration must end in a comma
 11 | ERROR | Each line in an array declaration must end in a comma
--------------------------------------------------------------------------------

A lot of the sniffs are opinionated and may not be reflective of an actual problem. We should have a ruleset XML file that just contains the core sniffs for checking code formatting, and then another ruleset XML that has the extra sniffs (e.g. those labeled with extra in GitHub), perhaps including the other ruleset XML as a base or copying it somehow.

The following snippet fails:

   public function custom_column_display( $column, $post_id )
    {
        global $post;
        switch ( $column ) {
            case 'some_number' : 
                echo (int) get_post_meta( $post_id, SOME_NUMBER, true );
            break;
        }
    }

This is allowed by http://codex.wordpress.org/Data_Validation

intval( $int ) or (int) $int
If it's supposed to be an integer, cast it as one.

Automattic has a VIP Scanner tool which would greatly benefit from integrating PHP_CodeSniffer. From looking at the tool, the checks are implemented with regular expressions and so they'll be less powerful than PHPCS's sniffs which operate on tokenized streams.

The checks in the VIP Scanner should be ported over to PHPCS sniffs, then a fine-tuned ruleset (to opt-in/opt-out of the relevant rules) could be employed by the VIP Scanner. In effect, the scanner would become just be a wrapper for PHPCS.

http://codex.wordpress.org/WordPress_Coding_Standards

The goal would be that the sniffs can be applied directly to the WordPress codebase itself to ensure quality in core.

We will need to make sure that non-core conventions can be separated out and selectively included among the sniffs to apply.

This code:

$args = wp_parse_args( $args, array(
    '1' => 1,
    '2' => 2,
) );

produces paired set of warnings about parentheses around arguments of multi-line function call needing to be respectively last and first on their lines.

While I have no issue with such rule, it is not in official spec and existing core code much favors more compact style for such call, as in example snippet.

I had read that variables were discouraged from being used in the i18n functions. I assumed this was just to discourage them from being changed. Well, it's actually because some automated i18n tools will tokenize the PHP files to extract the textdomain from PHP without actually evaluating the file. See @markjaquith's blog post Translating WordPress Plugins and Themes: Don’t Get Clever.

So a sniff should be written which makes sure that translation functions have a text domain which is a literal string. The sniff should also ensure that variables are not being used for the text to be translated, or inside of the text to be translated.

Here's the fix:

c10b10@1c5e90c

It would be good to revise that there are no blank spaces at the end of lines:+1:.

This is a nuisance when working in different operating system.

Logic should be placed in functional includes, encapsulated with other logic for the component (e.g. in a class). Templates should consist of template tags, some conditionals, and building queries.

The sniff needs more logic to determine safe output

echo 'Some Raw String';  //Causes no escaping function warning
_e( $some_nasty_var );  //No warning
echo apply_filters( 'the_content', $post->post_content );  //Causing no escaping function warning

Reported by @c3mdigital

There are 3 other forks (by @bigfishdesign, @rarescosma, and @Yapapaya) from the original @mrchrisadams repo and we should integrate their relevant changes: https://github.com/mrchrisadams/WordPress-Coding-Standards/network

https://github.com/bigfishdesign/bigfish-coding-standards
https://github.com/rarescosma/WordPress-Coding-Standards
https://github.com/yapapaya/WordPress-Coding-Standards

Such code:

if ( true ) {

    // code
}

Triggers warning about blank line at start of control structure. While official spec doesn't have blank lines in examples:

if ( condition ) {
    action1();
    action2();
} elseif ( condition2 && condition3 ) {
    action3();
    action4();
} else {
    defaultaction();
}

...it doesn't forbid them explicitly or even comment on them. This is hugely annoying for code bases where such blank line is common.

I would suggest to either seek clarification in official spec or dump the rule from the main standard.

We can gather a list of all global variables in WordPress, and then we can have a sniff add a warning if any of them are overridden in the global scope.