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

Better cover the regex related rules. #608

Merged
merged 2 commits into from
Jul 27, 2016
Merged
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
3 changes: 2 additions & 1 deletion WordPress-Core/ruleset.xml
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,8 @@
<rule ref="WordPress.Functions.DontExtract"/>
<rule ref="WordPress.NamingConventions.ValidHookName"/>
<rule ref="WordPress.DB.RestrictedFunctions"/>

<rule ref="WordPress.DB.RestrictedClasses"/>

<rule ref="WordPress.PHP.POSIXFunctions" />

</ruleset>
7 changes: 0 additions & 7 deletions WordPress/Sniffs/PHP/DiscouragedFunctionsSniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,6 @@ class WordPress_Sniffs_PHP_DiscouragedFunctionsSniff extends Generic_Sniffs_PHP_
* @var array(string => string|null)
*/
public $forbiddenFunctions = array(
// Deprecated.
'ereg_replace' => 'preg_replace',
'ereg' => null,
'eregi_replace' => 'preg_replace',
'split' => null,
'spliti' => null,

// Development.
Copy link
Member

Choose a reason for hiding this comment

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

Maybe leave a comment that this is handled with the new sniff?

Copy link
Member Author

@jrfnl jrfnl Jul 17, 2016

Choose a reason for hiding this comment

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

Leaving a comment like that in the code will become very untidy in combination with the upcoming #582 work. I'd rather suggest at some point also re-ordering the vip ruleset according to their documentation and making a note in the ruleset about rules like this which are already covered in core. Ok ?

'print_r' => null,
'debug_print_backtrace' => null,
Expand Down
69 changes: 69 additions & 0 deletions WordPress/Sniffs/PHP/POSIXFunctionsSniff.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
<?php
/**
* WordPress Coding Standard.
*
* @category PHP
* @package PHP_CodeSniffer
* @link https://make.wordpress.org/core/handbook/best-practices/coding-standards/
*/

/**
* Perl compatible regular expressions (PCRE, preg_ functions) should be used in preference
* to their POSIX counterparts.
*
* @link https://make.wordpress.org/core/handbook/best-practices/coding-standards/php/#regular-expressions
* @link http://php.net/manual/en/ref.regex.php
*
* @category PHP
* @package PHP_CodeSniffer
* @author Juliette Reinders Folmer <[email protected]>
*/
class WordPress_Sniffs_PHP_POSIXFunctionsSniff extends WordPress_AbstractFunctionRestrictionsSniff {

/**
* Groups of functions to restrict.
*
* Example: groups => array(
* 'lambda' => array(
* 'type' => 'error' | 'warning',
* 'message' => 'Use anonymous functions instead please!',
* 'functions' => array( 'eval', 'create_function' ),
* )
* )
*
* @return array
*/
public function getGroups() {
return array(
'ereg' => array(
'type' => 'error',
'message' => '%s has been deprecated since PHP 5.3 and removed in PHP 7.0, please use preg_match() instead.',
'functions' => array(
'ereg',
'eregi',
'sql_regcase',
),
),

'ereg_replace' => array(
'type' => 'error',
'message' => '%s has been deprecated since PHP 5.3 and removed in PHP 7.0, please use preg_replace() instead.',
'functions' => array(
'ereg_replace',
'eregi_replace',
),
),

'split' => array(
'type' => 'error',
'message' => '%s has been deprecated since PHP 5.3 and removed in PHP 7.0, please use explode(), str_split() or preg_split() instead.',
'functions' => array(
'split',
'spliti',
),
),

);
} // end getGroups()

} // end class
32 changes: 0 additions & 32 deletions WordPress/Sniffs/VIP/RestrictedFunctionsSniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -285,38 +285,6 @@ public function getGroups() {
),
),

'ereg' => array(
'type' => 'error',
'message' => '%s is prohibited, please use preg_match() instead. See http://php.net/manual/en/function.ereg.php',
'functions' => array(
'ereg',
),
),

'eregi' => array(
'type' => 'error',
'message' => '%s is prohibited, please use preg_match() with i modifier instead. See http://php.net/manual/en/function.eregi.php',
'functions' => array(
'eregi',
),
),

'ereg_replace' => array(
'type' => 'error',
'message' => '%s is prohibited, please use preg_replace() instead. See http://php.net/manual/en/function.ereg-replace.php',
'functions' => array(
'ereg_replace',
),
),

'split' => array(
'type' => 'error',
'message' => '%s is prohibited, please use explode() or preg_split() instead. See http://php.net/manual/en/function.split.php',
'functions' => array(
'split',
),
),

'runtime_configuration' => array(
Copy link
Member

Choose a reason for hiding this comment

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

Maybe leave a comment that this is handled with the new sniff?

'type' => 'error',
'message' => '%s is prohibited, changing configuration at runtime is not allowed on VIP Production.',
Expand Down
18 changes: 0 additions & 18 deletions WordPress/Tests/PHP/DiscouragedFunctionsUnitTest.inc
Original file line number Diff line number Diff line change
Expand Up @@ -9,24 +9,6 @@ var_dump( $post_id ); // Bad, forbidden use
print_r( $post_ID ); // Bad, forbidden use


// DEPRECATED PHP FUNCTIONS
// ------------------------

$title = get_the_title();

$title = ereg_replace( 'cool', 'not cool', get_the_title() ); // Bad, ereg_replace has been deprecated. Use preg_replace instead.
$title = preg_replace( 'cool', 'not cool', get_the_title() ); // Good

if ( ereg( '[A-Za-z]+', $title, $regs ) ) // Bad, ereg also deprecated. Use preg_match instead.
die( $regs );

$title = eregi_replace( 'cool', 'not cool', get_the_title() ); // Bad, eregi_replace also deprecated. Use preg_replace instead.

list( $year, $month, $day ) = split( ':', $date ); // Bad, split has been deprecated. Use preg_split or explode instead.

$title_parts = spliti( ' ', get_the_title(), 4 ); // Bad, spliti also deprecated. Use preg_split instead.


// DEPRECATED WORDPRESS FUNCTIONS
// ------------------------------

Expand Down
19 changes: 7 additions & 12 deletions WordPress/Tests/PHP/DiscouragedFunctionsUnitTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -49,28 +49,23 @@ public function getWarningList() {
return array(
8 => 1,
9 => 1,
15 => 1,
17 => 1,
20 => 1,
19 => 1,
21 => 1,
23 => 1,
25 => 1,
27 => 1,
29 => 1,
31 => 1,
33 => 1,
35 => 1,
37 => 1,
39 => 1,
41 => 1,
43 => 1,
45 => 1,
47 => 1,
49 => 1,
51 => 1,
53 => 1,
55 => 1,
57 => 1,
63 => 1,
65 => 1,
70 => 1,
72 => 1,
52 => 1,
54 => 1,
);

} // end getWarningList()
Expand Down
26 changes: 26 additions & 0 deletions WordPress/Tests/PHP/POSIXFunctionsUnitTest.inc
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
<?php

// These are ok.
$regex = preg_quote( 'cool', '#' ); // Good
$title = preg_match( 'cool', get_the_title() ); // Good
$title = preg_match_all( 'cool', get_the_title(), $matches ); // Good
$title = preg_replace( 'cool', 'not cool', get_the_title() ); // Good
$title = preg_replace_callback( 'cool', 'callback_function', get_the_title() ); // Good
$title = preg_split( 'cool', get_the_title() ); // Good


// These should all trigger an error.
if ( ereg( '[A-Za-z]+', $title, $regs ) ) // Bad, ereg deprecated. Use preg_match instead.
die( $regs );

if ( eregi( '[a-z]+', $title, $regs ) ) {} // Bad, eregi deprecated. Use preg_match instead.

$title = ereg_replace( 'cool', 'not cool', get_the_title() ); // Bad, ereg_replace has been deprecated. Use preg_replace instead.

$title = eregi_replace( 'cool', 'not cool', get_the_title() ); // Bad, eregi_replace also deprecated. Use preg_replace instead.

list( $year, $month, $day ) = split( ':', $date ); // Bad, split has been deprecated. Use preg_split or explode instead.

$title_parts = spliti( ' ', get_the_title(), 4 ); // Bad, spliti also deprecated. Use preg_split instead.

sql_regcase( 'Foo - bar.'); // Bad. Deprecated.
Copy link
Member

Choose a reason for hiding this comment

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

I don't see any reference to this above?

Copy link
Member Author

Choose a reason for hiding this comment

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

grin yeah, that's because the previous sniffs weren't complete.
The handbook talks about the whole POSIX extension and that's another - albeit rarely used - function from that group: http://php.net/manual/en/ref.regex.php
As the POSIX extension was deprecated in 5.3 and removed in 7.0, being complete seemed like the right thing to do.

Or do you mean in the sniff itself ? It's there - in the first group of functions. I grouped them based on the advised replacement function.

61 changes: 61 additions & 0 deletions WordPress/Tests/PHP/POSIXFunctionsUnitTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
<?php
/**
* Unit test class for WordPress Coding Standard.
*
* @category PHP
* @package PHP_CodeSniffer
* @link https://make.wordpress.org/core/handbook/best-practices/coding-standards/
*/

/**
* WordPress_Tests_PHP_POSIXFunctionsUnitTest
*
* A sniff unit test checks a .inc file for expected violations of a single
* coding standard. Expected errors and warnings are stored in this class.
*
* @category PHP
* @package PHP_CodeSniffer
* @author Akeda Bagus <[email protected]>
* @author Greg Sherwood <[email protected]>
* @author Marc McIntyre <[email protected]>
* @license https://github.com/squizlabs/PHP_CodeSniffer/blob/master/licence.txt BSD Licence
* @version Release: @package_version@
* @link http://pear.php.net/package/PHP_CodeSniffer
*/
class WordPress_Tests_PHP_POSIXFunctionsUnitTest extends AbstractSniffUnitTest {

/**
* Returns the lines where errors should occur.
*
* The key of the array should represent the line number and the value
* should represent the number of errors that should occur on that line.
*
* @return array(int => int)
*/
public function getErrorList() {
return array(
13 => 1,
16 => 1,
18 => 1,
20 => 1,
22 => 1,
24 => 1,
26 => 1,
);

} // end getErrorList()

/**
* Returns the lines where warnings should occur.
*
* The key of the array should represent the line number and the value
* should represent the number of warnings that should occur on that line.
*
* @return array(int => int)
*/
public function getWarningList() {
return array();

} // end getWarningList()

} // end class