CS: Fix violations for admin-header.php

[marcomartins]

Fixes the coding standard violations for wp-admin/admin-header.php

Ignores the following sniffs:

• Generic.PHP.NoSilencedErrors.Discouraged
• WordPress.NamingConventions.ValidHookName.UseUnderscores
• WordPress.Variables.GlobalVariables.OverrideProhibited

[marcomartins]

Inline phpcs:ignore moved to its own line or replaced with phpcs:disable.

[marcomartins]

Fixes WordPress.WhiteSpace.PrecisionAlignment.Found and a faulty phpcs:ignore

[jrf]

@marcomartins Welcome as a new contributor and thank you for your efforts.

I've reviewed the latest patch (3).

1. Based on your patch I realize there is actually an issue with the ruleset as currently committed to core. The WordPress.Variables.GlobalVariables sniff is referenced there, but should not be as that sniff should not be part of the core ruleset. Core overwriting Core global variables should always be fine. So this should be fixed in the ruleset, not by using phpcs:ignore comments.
2. The hook names cannot be ignored for "allow for dash in dynamic hook". The handbook is quite clear that dashes are not allowed in hook names: ​https://make.wordpress.org/core/handbook/best-practices/coding-standards/php/#naming-conventions, including dynamic hook names (see the examples) ​https://make.wordpress.org/core/handbook/best-practices/coding-standards/php/#interpolation-for-naming-dynamic-hooks

So there are two options here:

• Either ignore it for the right reasons, i.e. not to break backwards-compatibility.
• Or deprecated the existing hook and introduce a new hook which complies with the naming conventions.

3. The disable comments can be combined within the hook documentation blocks, like so:

   <?php
   /**
    * Fires when scripts are printed for a specific admin page based on $hook_suffix.
    *
    * @since 2.1.0
    *
    * @phpcs:disable WordPress.NamingConventions.ValidHookName.UseUnderscores -- left as is not to break BC.
    */
   do_action( "admin_print_scripts-{$hook_suffix}" );
   //phpcs:enable
   

which is a "cleaner" solution than having a // comment above the docblock.

[jrf]

FYI: point 1 of my remarks is being addressed in ticket #43776

[antonvlasenko]

I wonder if the has-patch keyword has to be added to this issue.
There are 3 patches.

[SergeyBiryukov]

Hi there, thanks for the ticket!

This appears to be resolved as part of other coding standards fixes:

• [43013] / #43776 removed the WordPress.Variables.GlobalVariables sniff from the core ruleset.
• [45599] / #47632 addressed the hook names with hyphens.
• [45611] / #47632 removed the error suppression operator from header().
• [47219] / #49222 switched to Yoda conditions.

The wp-admin/admin-header.php file no longer displays any warnings when running WPCS checks.