Skip to content

Run the full WordPress standard in phpcs #760

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.

Sign up for GitHub

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

Merged
obenland merged 1 commit into from
Aug 18, 2026
Merged

Run the full WordPress standard in phpcs #760

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
118 changes: 67 additions & 51 deletions phpcs.xml.dist
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,57 @@

<arg name="extensions" value="php" />

<rule ref="WordPress-Core">
<!--
... Keep this block ABOVE the `WordPress` rule. `WordPress` includes `WordPress-Docs` itself, and whichever
... reference registers a sniff first is the one whose `<exclude-pattern>` sticks. Declared afterwards, the
... parser-file exemption below is silently ignored and the block pattern parsers pick up ~36 docblock errors.
-->
<rule ref="WordPress-Docs">
<!-- If files/variables are given descriptive names like they should be, then an explicit description is usually unnecessary, so leave this as a judgement call. -->
<exclude name="Squiz.Commenting.FunctionComment.MissingParamComment" />
<exclude name="Squiz.Commenting.FileComment.Missing" />
<exclude name="Squiz.Commenting.ClassComment.Missing" />
<exclude name="Squiz.Commenting.FunctionComment.MissingParamTag" />
<exclude name="Generic.Commenting.DocComment.MissingShort" />
<exclude name="Squiz.Commenting.VariableComment.Missing" />
<exclude name="Squiz.Commenting.VariableComment.MissingVar" />

<!-- I don't see how these are useful. -->
<exclude name="Squiz.Commenting.FileComment.MissingPackageTag" />

<!-- We really only use basic exceptions, so this is kind of overkill and tedious. -->
<exclude name="Squiz.Commenting.FunctionComment.EmptyThrows" />

<!-- Whitespace makes things more readable. -->
<exclude name="Squiz.Commenting.FileComment.SpacingAfterOpen" />

<!-- There are some valid cases of this, like in identifying a closing tag from another file; e.g., in `themes/campsite-2017/footer.php`. -->
<exclude name="Squiz.Commenting.InlineComment.SpacingAfter" />

<!-- It's not wrong for WordPress plugin file headers. -->
<exclude name="Squiz.Commenting.FileComment.WrongStyle" />

<!-- Class comments are generally not useful, so they're left out, but then PHPCS confuses the plugin headers for a class comment -->
<exclude name="Squiz.Commenting.ClassComment.WrongStyle" />
<exclude name="Squiz.Commenting.ClassComment.SpacingAfter" />

<!-- WordPress have translators comment which requires no space after `//` -->
<exclude name="Squiz.Commenting.InlineComment.NoSpaceBefore" />

<!-- Ignore punctuation at the end of comments. -->
<exclude name="Squiz.Commenting.InlineComment.InvalidEndChar" />

<!-- Ignore docs issues in block pattern parser files. -->
<exclude-pattern>*/pattern-translations/includes/*.php$</exclude-pattern>
</rule>

<!--
... Reference the full `WordPress` standard rather than composing Core/Docs/Extra by hand. Only the meta
... standard carries the `namespace="WordPressCS\WordPress"` attribute that auto-includes every WordPressCS
... sniff; the sub-rulesets ref sniffs individually, which left WordPress.Security.ValidatedSanitizedInput
... out of the run entirely.
-->
<rule ref="WordPress">
<!-- I don't see anything wrong with this :) -->
<exclude name="Squiz.PHP.EmbeddedPhp.ContentAfterOpen" />
<exclude name="Squiz.PHP.EmbeddedPhp.ContentBeforeEnd" />
Expand Down Expand Up @@ -79,11 +129,8 @@
<!-- print_r() is perfectly accepted in some circumstances, like WP_CLI commands. -->
<exclude name="WordPress.PHP.DevelopmentFunctions.error_log_print_r" />

<!-- Allow short ternary pattern. -->
<exclude name="WordPress.PHP.DisallowShortTernary.Found" />

<!-- Allow short array syntax. -->
<exclude name="Generic.Arrays.DisallowShortArraySyntax.Found" />
<!-- I think it's better to have all the `use` statements come right after the namespace line. -->
<exclude name="PSR2.Namespaces.NamespaceDeclaration.BlankLineAfter" />
</rule>

<rule ref="WordPress.Files.FileName.NotHyphenatedLowercase">
Expand All @@ -100,55 +147,24 @@
</properties>
</rule>

<rule ref="WordPress-Docs">
<!-- If files/variables are given descriptive names like they should be, then an explicit description is usually unnecessary, so leave this as a judgement call. -->
<exclude name="Squiz.Commenting.FunctionComment.MissingParamComment" />
<exclude name="Squiz.Commenting.FileComment.Missing" />
<exclude name="Squiz.Commenting.ClassComment.Missing" />
<exclude name="Squiz.Commenting.FunctionComment.MissingParamTag" />
<exclude name="Generic.Commenting.DocComment.MissingShort" />
<exclude name="Squiz.Commenting.VariableComment.Missing" />
<exclude name="Squiz.Commenting.VariableComment.MissingVar" />

<!-- I don't see how these are useful. -->
<exclude name="Squiz.Commenting.FileComment.MissingPackageTag" />

<!-- We really only use basic exceptions, so this is kind of overkill and tedious. -->
<exclude name="Squiz.Commenting.FunctionComment.EmptyThrows" />

<!-- Whitespace makes things more readable. -->
<exclude name="Squiz.Commenting.FileComment.SpacingAfterOpen" />

<!-- There are some valid cases of this, like in identifying a closing tag from another file; e.g., in `themes/campsite-2017/footer.php`. -->
<exclude name="Squiz.Commenting.InlineComment.SpacingAfter" />

<!-- It's not wrong for WordPress plugin file headers. -->
<exclude name="Squiz.Commenting.FileComment.WrongStyle" />

<!-- Class comments are generally not useful, so they're left out, but then PHPCS confuses the plugin headers for a class comment -->
<exclude name="Squiz.Commenting.ClassComment.WrongStyle" />
<exclude name="Squiz.Commenting.ClassComment.SpacingAfter" />

<!-- WordPress have translators comment which requires no space after `//` -->
<exclude name="Squiz.Commenting.InlineComment.NoSpaceBefore" />

<!-- Ignore punctuation at the end of comments. -->
<exclude name="Squiz.Commenting.InlineComment.InvalidEndChar" />

<!-- Ignore docs issues in block pattern parser files. -->
<exclude-pattern>*/pattern-translations/includes/*.php$</exclude-pattern>
</rule>

<rule ref="WordPress-Extra">
<!-- I think it's better to have all the `use` statements come right after the namespace line. -->
<exclude name="PSR2.Namespaces.NamespaceDeclaration.BlankLineAfter" />
<!--
... `<arg value="psn"/>` above reports errors only, so a security sniff that emits warnings is invisible
... in CI. Promote the whole category to errors rather than dropping the `n`, which would also surface
... ~380 array-alignment warnings that have nothing to do with security.
...
... Must stay after the `WordPress` rule above to take effect.
-->
<rule ref="WordPress.Security">
<type>error</type>
</rule>

<!-- Verify that the text_domain is set to the desired text-domain. Multiple valid text domains can be
provided as a comma-delimited list. -->
<!-- Verify that the text_domain is set to the desired text-domain. Additional valid text domains can be
added as further `<element>` nodes. -->
<rule ref="WordPress.WP.I18n">
<properties>
<property name="text_domain" type="array" value="wporg-patterns" />
<property name="text_domain" type="array">
<element value="wporg-patterns" />
</property>
</properties>
</rule>
<!-- Short ternaries are used deliberately throughout; expanding them is a style call for maintainers. -->
Expand Down
4 changes: 2 additions & 2 deletions public_html/wp-content/plugins/pattern-creator/pattern-creator.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -337,8 +337,8 @@ function allow_reading_global_styles( $caps, $cap, $user_id, $args ) {
) {
return $caps;
}
$request_method = isset( $_SERVER['REQUEST_METHOD'] ) ? strtoupper( $_SERVER['REQUEST_METHOD'] ) : '';
$request_uri = isset( $_SERVER['REQUEST_URI'] ) ? $_SERVER['REQUEST_URI'] : '';
$request_method = isset( $_SERVER['REQUEST_METHOD'] ) ? strtoupper( sanitize_text_field( wp_unslash( $_SERVER['REQUEST_METHOD'] ) ) ) : '';
$request_uri = isset( $_SERVER['REQUEST_URI'] ) ? esc_url_raw( wp_unslash( $_SERVER['REQUEST_URI'] ) ) : '';
if (
( 'GET' !== $request_method && 'HEAD' !== $request_method ) ||
false === strpos( $request_uri, '/wp/v2/global-styles' )
Expand Down
7 changes: 2 additions & 5 deletions public_html/wp-content/plugins/pattern-directory/includes/admin-patterns.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -391,11 +391,8 @@ function handle_pattern_list_table_views( WP_Query $query ) {
* @return array
*/
function display_post_states( $post_states, $post ) {
if ( isset( $_REQUEST['post_status'] ) ) {
$post_status = $_REQUEST['post_status'];
} else {
$post_status = '';
}
// phpcs:ignore WordPress.Security.NonceVerification.Recommended -- read-only list table filter, compared against registered statuses.
$post_status = isset( $_REQUEST['post_status'] ) ? sanitize_key( wp_unslash( $_REQUEST['post_status'] ) ) : '';

if (
$post->post_status !== $post_status &&
Expand Down
188 changes: 188 additions & 0 deletions public_html/wp-content/plugins/pattern-directory/tests/phpunit/admin-patterns-test.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,188 @@
<?php
/**
* Test the Patterns list table admin helpers.
*/

use function WordPressdotorg\Pattern_Directory\Admin\Patterns\display_post_states;
use const WordPressdotorg\Pattern_Directory\Pattern_Post_Type\{ POST_TYPE, UNLISTED_STATUS, SPAM_STATUS };

/*
* These tests set the request superglobal directly to drive the list table filter, which is the
* point of them. The sniff guarding production input handling does not apply.
*/
// phpcs:disable WordPress.Security.NonceVerification.Recommended

/**
* Test the extra post states shown on the Patterns list table.
*/
class Pattern_Admin_Post_States_Test extends WP_UnitTestCase {
/**
* An unlisted pattern.
*
* @var int
*/
protected static $unlisted_pattern_id;

/**
* A pattern flagged as spam.
*
* @var int
*/
protected static $spam_pattern_id;

/**
* A published pattern, which never gets an extra state.
*
* @var int
*/
protected static $published_pattern_id;

/**
* The $_REQUEST superglobal as it was before the current test ran.
*
* @var array
*/
protected $original_request;

/**
* Set up shared fixtures.
*/
public static function wpSetUpBeforeClass( $factory ) {
self::$unlisted_pattern_id = $factory->post->create(
array(
'post_type' => POST_TYPE,
'post_status' => UNLISTED_STATUS,
)
);
self::$spam_pattern_id = $factory->post->create(
array(
'post_type' => POST_TYPE,
'post_status' => SPAM_STATUS,
)
);
self::$published_pattern_id = $factory->post->create(
array(
'post_type' => POST_TYPE,
'post_status' => 'publish',
)
);
}

/**
* Isolate each test from the real request.
*/
public function set_up() {
parent::set_up();

$this->original_request = $_REQUEST;
$_REQUEST = array();
}

/**
* Restore the real request.
*/
public function tear_down() {
$_REQUEST = $this->original_request;

parent::tear_down();
}

/**
* Without a status filter, unlisted and spam patterns are labelled so they stand out in the
* "All" view.
*
* @dataProvider data_flagged_statuses
*
* @param string $property Name of the fixture property holding the pattern ID.
* @param string $status The post status that should be labelled.
*/
public function test_flagged_status_is_labelled_in_unfiltered_view( $property, $status ) {
$post = get_post( self::${$property} );

$states = display_post_states( array(), $post );

$this->assertArrayHasKey( $status, $states );
$this->assertSame( get_post_status_object( $status )->label, $states[ $status ] );
}

/**
* Statuses that earn an extra label.
*
* @return array[]
*/
public function data_flagged_statuses() {
return array(
'unlisted' => array( 'unlisted_pattern_id', UNLISTED_STATUS ),
'spam' => array( 'spam_pattern_id', SPAM_STATUS ),
);
}

/**
* When the list table is already filtered to that status, the label is redundant and omitted.
*/
public function test_label_is_omitted_when_already_filtering_by_that_status() {
$_REQUEST['post_status'] = UNLISTED_STATUS;
$post = get_post( self::$unlisted_pattern_id );

$this->assertSame( array(), display_post_states( array(), $post ) );
}

/**
* An ordinary published pattern never gets an extra state.
*/
public function test_published_pattern_gets_no_extra_state() {
$post = get_post( self::$published_pattern_id );

$this->assertSame( array(), display_post_states( array(), $post ) );
}

/**
* Existing states are preserved rather than replaced.
*/
public function test_existing_states_are_preserved() {
$post = get_post( self::$unlisted_pattern_id );

$states = display_post_states( array( 'sticky' => 'Sticky' ), $post );

$this->assertArrayHasKey( 'sticky', $states );
$this->assertArrayHasKey( UNLISTED_STATUS, $states );
}

/**
* The `post_status` request variable runs through `sanitize_key()`, which lowercases it. A
* differently-cased filter therefore matches the post status and suppresses the label, where
* an unsanitized comparison would not.
*/
public function test_post_status_request_variable_is_sanitized() {
$_REQUEST['post_status'] = strtoupper( UNLISTED_STATUS );
$post = get_post( self::$unlisted_pattern_id );

$this->assertSame( array(), display_post_states( array(), $post ) );
}

/**
* Sanitizing strips characters a real status can never contain, so a value carrying markup
* cannot match and the label stays.
*/
public function test_post_status_carrying_markup_does_not_match() {
$_REQUEST['post_status'] = '<script>alert(1)</script>' . UNLISTED_STATUS;
$post = get_post( self::$unlisted_pattern_id );

$states = display_post_states( array(), $post );

$this->assertArrayHasKey( UNLISTED_STATUS, $states );
$this->assertStringNotContainsString( '<script>', wp_json_encode( $states ) );
}

/**
* A non-string `post_status` must not raise a type error.
*/
public function test_array_post_status_is_handled() {
$_REQUEST['post_status'] = array( UNLISTED_STATUS );
$post = get_post( self::$unlisted_pattern_id );

$states = display_post_states( array(), $post );

$this->assertArrayHasKey( UNLISTED_STATUS, $states );
}
}
Loading
Loading