WPTT / WPThemeReview

PHP_CodeSniffer rules (sniffs) to enforce WordPress theme review coding conventions
MIT License
209 stars 37 forks source link

Check for whitespace before a PHP open tag at start of file #148

Closed jrfnl closed 6 years ago

jrfnl commented 6 years ago

_Issue Title [New sniff] Check for whitespace before a PHP open tag at start of file

_Issue Content

Rule type:

Error

Rule:

Covered via:

Required: No PHP notices

Ref: https://make.wordpress.org/themes/handbook/review/required/explanations-and-examples/#code

There should be nothing (except possibly a hashbang) before the PHP open tag if it's the first real content in the file.

This can prevent "headers already send" PHP notices.

This is loosely related to the check for BOM-marks at the start of files. (#5)

Theme check file covering this rule:

Not checked.

Notes for implementation:

This was discussed in the video chat earlier today with an eye on adding the Generic.PHP.CharacterBeforePHPOpenTag sniff to the ruleset.

On closer review of that sniff, it demands that all PHP files start with a PHP open tag. This would cause issues with themes using views without a file docblock at the top, so the upstream sniff can not be used.

The sniff needed here would need to check that there is no inline HTML before the first PHP open tag to prevent these issues.

As this means that a custom sniff is needed, I'd like to point out that this sniff could be useful for more than just themes, so it should be pulled to WPCS and once it has been merged there, the sniff should be included into TRTCS via the ruleset.

To do:

jrfnl commented 6 years ago

PR https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards/pull/1365 has been opened to address this.

jrfnl commented 6 years ago

Update: I've closed the PR to WPCS as it turns out that this is already checked for by another sniff included in WPCS. This is an upstream PHPCS native sniff which can be included here too.

Note: the upstream sniff does not handle unicode whitespace preceeding the PHP open tag, but that should be fixed upstream and is being discussed there.