Skip to content
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.

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

Added Enqueued functions check for version and in footer #1233

Closed
wants to merge 14 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,12 @@ This projects adheres to [Semantic Versioning](http://semver.org/) and [Keep a C

_Nothing yet._


## [0.14.1] - 2018-02-15

### Fixed
- The `WordPress.NamingConventions.PrefixAllGlobals` sniff contained a bug which could inadvertently trigger class autoloading of the project being sniffed and by extension could cause fatal errors during the PHPCS run.

## [0.14.0] - 2017-11-01

### Added
Expand Down Expand Up @@ -524,6 +530,7 @@ See the comparison for full list.
Initial tagged release.

[Unreleased]: https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards/compare/master...HEAD
[0.14.1]: https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards/compare/0.14.0...0.14.1
[0.14.0]: https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards/compare/0.13.1...0.14.0
[0.13.1]: https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards/compare/0.13.0...0.13.1
[0.13.0]: https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards/compare/0.12.0...0.13.0
Expand Down
5 changes: 5 additions & 0 deletions WordPress-Extra/ruleset.xml
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,11 @@
https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards/issues/503 -->
<rule ref="WordPress.PHP.StrictInArray"/>

<!-- Check Enqueued functions that IF Source ($src) is specified,
make sure there is a Version ($ver) and In Footer ($in_footer) is true.
https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards/issues/1146 -->
<rule ref="WordPress.PHP.EnqueuedCheck"/>

<!-- Discourage use of the backtick operator (execution of shell commands).
https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards/pull/646 -->
<rule ref="Generic.PHP.BacktickOperator"/>
Expand Down
111 changes: 111 additions & 0 deletions WordPress/Sniffs/PHP/EnqueuedCheckSniff.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
<?php
/**
* WordPress Coding Standard.
*
* @package WPCS\WordPressCodingStandards
* @link https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards
* @license https://opensource.org/licenses/MIT MIT
*/

namespace WordPress\Sniffs\PHP;

use WordPress\AbstractFunctionParameterSniff;

/**
* Based off the StrictInArraySniff.php sniff, this checks the enqueued 4th parameter to make sure a version is available.
* The Enqueued functions are:
* wp_register_script()
* wp_enqueue_script()
* wp_register_style()
* wp_enqueue_style()
* If a source ($src) value is passed, then version ($ver) needs to have a value.
* Additionally, If a source ($src) value is passed a check for in footer ($in_footer)
* to alert the user if the value isnt True
*
* @link https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards/issues/1146
*
* @package WPCS\WordPressCodingStandards
*
* @since 1.0.0
*/
class EnqueuedCheckSniff extends AbstractFunctionParameterSniff {

/**
* The group name for this group of functions.
*
* @since 1.0.0
* @var string
*/
protected $group_name = 'Enqueued';

/**
* List of enqueued functions that need to be check to make sure
*
* @link https://developer.wordpress.org/reference/functions/wp_register_script/
* @link https://developer.wordpress.org/reference/functions/wp_enqueue_script/
* @link https://developer.wordpress.org/reference/functions/wp_register_style/
* @link https://developer.wordpress.org/reference/functions/wp_enqueue_style/
*
* @since 1.0.0
*
* @var array <string function_name> => <bool always needed ?>
*/
protected $target_functions = array(
'wp_register_script' => true,
'wp_enqueue_script' => true,
'wp_register_style' => true,
'wp_enqueue_style' => true,
);

/**
* Process the parameters of a matched function.
*
* @since 1.0.0
*
* @param int $stackPtr The position of the current token in the stack.
* @param array $group_name The name of the group which was matched.
* @param string $matched_content The token content (function name) which was matched.
* @param array $parameters Array with information about the parameters.
*
* @return void
*/
public function process_parameters( $stackPtr, $group_name, $matched_content, $parameters ) {

// Check to see if a source ($src) is specified.
if ( ! isset( $parameters[2] ) ) {
return;
}

/*
* Version Check
*
* Check to make sure a Version ($ver) is set
* Otherwise it will Show an error to add a src (url) to the enqueued function
*/
if ( false === isset( $parameters[4] ) || ! $parameters[4]['raw'] ) {
$this->phpcsFile->addError( 'No Version found for %s; Please supply a value for the fourth argument', $stackPtr, 'MissingVersion', array( $matched_content ) );
return;
}

/*
* In footer Check
*
* Check to make sure that $in_footer is set to true
* Otherwise it will warn the user to make sure if its correct
*/
if ( isset( $parameters[5] ) ) {
/*
* Only wp_register_script and wp_enqueue_script need this check
* As it is not available to wp_register_style and wp_enqueue_style
*/
switch ( $matched_content ) {
case 'wp_register_script':
case 'wp_enqueue_script':
if ( 'true' !== $parameters[5]['raw'] ) {
$this->phpcsFile->addWarning( 'If the Footer is not set to True for %s; Double check if correct or set to True', $stackPtr, 'MissingInFooter', array( $matched_content ) );
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If is not needed here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"The $in_footer is not set to true. ..."

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @GaryJones With the IF removed, all checks fail. THis check needs to be here in order to get the warning. Unless I missed something?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@darthvader666uk I think @GaryJones's remark was not about the conditional, but about the phrasing of the error message.

For that matter, mind capitalizing words in the error message when it's not needed.

return;
}
}
}
}
} // End class.
18 changes: 18 additions & 0 deletions WordPress/Tests/PHP/EnqueuedCheckUnitTest.inc
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
<?php

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about cases for a couple of the functions that have a version but no in_footer param?

wp_register_script( 'someScript-js', 'https://domain.com/someScript.js' , array( 'jquery' ), '1.1.1', true ); // Ok.
wp_register_script( 'someScript-js', 'https://domain.com/someScript.js' , array( 'jquery' ), '1.1.1', $in_footer ); // Warning - $in_footer is not a boolean
wp_register_script( 'someScript-js', 'https://domain.com/someScript.js' , array( 'jquery' ) ); // Error - missing $in_footer.

wp_register_script( 'someScript-js', 'https://domain.com/someScript.js' , array( 'jquery' ), true ); // Warning.
wp_register_script( 'someScript-js', 'https://domain.com/someScript.js' , array( 'jquery' ), $in_footer ); // Warning - $in_footer is not a boolean

wp_enqueue_script( 'script-name', 'https://domain.com/someScript.js', false, '1.0.0', true ); // Ok.
wp_enqueue_script( 'script-name', 'https://domain.com/someScript.js', false, '1.0.0', false ); // Warning.
wp_enqueue_script( 'script-name', 'https://domain.com/someScript.js' ); // Error.

wp_register_style( 'script-name', 'https://domain.com/someScript.js', false, '1.0.0' ); // Ok.
wp_register_style( 'script-name', 'https://domain.com/someScript.js' ); // Error.

wp_enqueue_style( 'script-name', 'https://domain.com/someScript.js', false, '1.0.0'); // Ok.
wp_enqueue_style( 'script-name', 'https://domain.com/someScript.js' ); // Error.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clear line space needed at the end.

49 changes: 49 additions & 0 deletions WordPress/Tests/PHP/EnqueuedCheckUnitTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
<?php
/**
* Unit test class for WordPress Coding Standard.
*
* @package WPCS\WordPressCodingStandards
* @link https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards
* @license https://opensource.org/licenses/MIT MIT
*/

namespace WordPress\Tests\PHP;

use PHP_CodeSniffer\Tests\Standards\AbstractSniffUnitTest;

/**
* Unit test class for the EnqueuedCheck sniff.
*
* @package WPCS\WordPressCodingStandards
*
* @since 1.0.0
*/
class EnqueuedCheckUnitTest extends AbstractSniffUnitTest {

/**
* Returns the lines where errors should occur.
*
* @return array <int line number> => <int number of errors>
*/
public function getErrorList() {
return array(
5 => 1,
9 => 1,
12 => 1,
15 => 1,
);
}

/**
* Returns the lines where warnings should occur.
*
* @return array <int line number> => <int number of warnings>
*/
public function getWarningList() {
return array(
4 => 1,
8 => 1,
);
}

} // End class.