Skip to content

Commit b4a8812

Browse files
authored
Merge pull request #72 from Automattic/override-get-children-message-in-ruleset
Override get children message in ruleset
2 parents 0de9045 + dc5a1aa commit b4a8812

File tree

6 files changed

+118
-72
lines changed

6 files changed

+118
-72
lines changed

WordPressVIPMinimum/Sniffs/VIP/RestrictedFunctionsSniff.php

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -30,13 +30,6 @@ public function getGroups() {
3030
'message' => '%s is prohibited on the WordPress.com VIP platform',
3131
'functions' => array( 'get_super_admins' ),
3232
),
33-
'get_children' => array(
34-
'type' => 'error',
35-
'message' => '%1$s() performs a no-LIMIT query by default, make sure to set a reasonable posts_per_page. %1$s() will do a -1 query by default, a maximum of 100 should be used.',
36-
'functions' => array(
37-
'get_children',
38-
),
39-
),
4033
'internal' => array(
4134
'type' => 'error',
4235
'message' => '%1$s() is for internal use only.',
@@ -53,13 +46,7 @@ public function getGroups() {
5346
),
5447
);
5548

56-
$original_groups['get_posts']['functions'] = array_filter( $original_groups['get_posts']['functions'], array( $this, 'filter_out_get_children' ) );
57-
5849
return array_merge( $original_groups, $new_groups );
5950

6051
} // end getGroups().
61-
62-
public function filter_out_get_children( $v ) {
63-
return ! in_array( $v, array( 'get_children' ), true );
64-
}
6552
}

WordPressVIPMinimum/Tests/VIP/RestrictedFunctionsUnitTest.inc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
wpcom_vip_irc(); // Bad.
44

5-
get_children(); // Bad.
5+
get_children(); // Bad. Warning.
66

77
wp_cache_get_multi(); // Bad.
88

WordPressVIPMinimum/Tests/VIP/RestrictedFunctionsUnitTest.php

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ class WordPressVIPMinimum_Tests_VIP_RestrictedFunctionsUnitTest extends Abstract
1616
public function getErrorList() {
1717
return array(
1818
3 => 1,
19-
5 => 1,
2019
7 => 1,
2120
9 => 1,
2221
11 => 1,
@@ -29,7 +28,9 @@ public function getErrorList() {
2928
* @return array <int line number> => <int number of warnings>
3029
*/
3130
public function getWarningList() {
32-
return array();
31+
return array(
32+
5 => 1,
33+
);
3334

3435
}
3536

WordPressVIPMinimum/ruleset.xml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,4 +101,9 @@
101101
<rule ref="WordPress.WP.AlternativeFunctions.file_get_contents">
102102
<message>%s() is highly discouraged, please use vip_safe_wp_remote_get() instead.</message>
103103
</rule>
104+
105+
<rule ref="WordPressVIPMinimum.VIP.RestrictedFunctions.get_posts_get_children">
106+
<type>error</type>
107+
<message>%1$s() performs a no-LIMIT query by default, make sure to set a reasonable posts_per_page. %1$s() will do a -1 query by default, a maximum of 100 should be used.</message>
108+
</rule>
104109
</ruleset>

ruleset_test.inc

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,4 +124,6 @@ wp_remote_post( $this->endpoint, array(
124124
eval(); // Bad. Error.
125125

126126
// WordPressVIPMinimum.VIP.RestrictedFunctions
127-
wpcom_vip_irc(); // Bad. Error.
127+
wpcom_vip_irc(); // Bad. Error.
128+
129+
get_children(); // Bad. Warning. Message.

ruleset_test.php

Lines changed: 106 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
* ```
1515
*/
1616

17+
// Expected values.
1718
$expected = array(
1819
'errors' => array(
1920
4 => 1,
@@ -40,6 +41,7 @@
4041
116 => 1,
4142
124 => 1,
4243
127 => 1,
44+
129 => 1,
4345
),
4446
'warnings' => array(
4547
9 => 1,
@@ -57,81 +59,130 @@
5759
108 => 1,
5860
109 => 1,
5961
),
62+
'messages' => array(
63+
129 => array(
64+
'get_children() performs a no-LIMIT query by default, make sure to set a reasonable posts_per_page. get_children() will do a -1 query by default, a maximum of 100 should be used.',
65+
)
66+
),
6067
);
6168

62-
$errors = $warnings = array();
63-
$total_issues = 0;
69+
class PHPCS_Ruleset_Test {
6470

65-
// Travis support.
66-
if ( false === getenv( 'PHPCS_BIN' ) ) {
67-
putenv( "PHPCS_BIN=phpcs" );
68-
}
71+
private $errors = array();
72+
73+
private $warnings = array();
74+
75+
private $messages = array();
6976

70-
// Collect the PHPCS result.
71-
$output = shell_exec( '$PHPCS_BIN --standard=WordPressVIPMinimum --report=json ./ruleset_test.inc' );
77+
private $total_issues = 0;
7278

73-
$output = json_decode( $output, true );
79+
public $expected = array();
7480

75-
foreach( $output['files'] as $file => $issues ) {
76-
foreach( $issues['messages'] as $issue ) {
77-
if ( 'ERROR' === $issue['type'] ) {
78-
$errors[$issue['line']] = ( isset( $errors[$issue['line']] ) ) ? $errors[$issue['line']]++ : 1;
79-
} else {
80-
$warnings[$issue['line']] = ( isset( $warnings[$issue['line']] ) ) ? $warnings[$issue['line']]++ : 1;
81+
public function __construct( $expected = array() ) {
82+
$this->expected = $expected;
83+
84+
// Travis support.
85+
if ( false === getenv( 'PHPCS_BIN' ) ) {
86+
putenv( "PHPCS_BIN=phpcs" );
87+
}
88+
89+
// Collect the PHPCS result.
90+
$output = shell_exec( '$PHPCS_BIN --standard=WordPressVIPMinimum --report=json ./ruleset_test.inc' );
91+
92+
$output = json_decode( $output, true );
93+
94+
if ( false === is_array( $output ) || true === empty( $output ) ) {
95+
printf( 'The PHPCS command checking the ruleset haven\'t returned any issues. Bailing ...' . PHP_EOL );
96+
exit( 1 ); // Die early, if we don't have any output.
97+
}
98+
99+
foreach( $output['files'] as $file => $issues ) {
100+
foreach( $issues['messages'] as $issue ) {
101+
if ( 'ERROR' === $issue['type'] ) {
102+
$this->errors[$issue['line']] = ( isset( $this->errors[$issue['line']] ) ) ? $this->errors[$issue['line']]++ : 1;
103+
} else {
104+
$this->warnings[$issue['line']] = ( isset( $this->warnings[$issue['line']] ) ) ? $this->warnings[$issue['line']]++ : 1;
105+
}
106+
$this->messages[$issue['line']] = ( false === isset( $this->messages[$issue['line']] ) || false === is_array( $this->messages[$issue['line']] ) ) ? array( $issue['message'] ) : array_merge( $this->messages[$issue['line']], array( $issue['message'] ) );
107+
}
81108
}
82109
}
83-
}
84110

85-
// Check for missing expected values
86-
foreach ( $expected as $type => $lines ) {
87-
if ( 'errors' === $type ) {
88-
foreach( $lines as $line => $number ) {
89-
if ( false === isset( $errors[$line] ) ) {
90-
printf( 'Expected %d errors, found %d on line %d' . PHP_EOL, $number, 0, $line );
91-
$total_issues++;
92-
} else if ( $errors[$line] !== $number ) {
93-
printf( 'Expected %d errors, found %d on line %d' . PHP_EOL, $number, $errors[$line], $line );
94-
$total_issues++;
111+
public function run() {
112+
// Check for missing expected values
113+
$this->check_missing_expected_values();
114+
// Check for extra values which were not expected
115+
$this->check_unexpected_values();
116+
// Check for expected messages
117+
$this->check_messages();
118+
119+
return $this->total_issues;
120+
}
121+
122+
private function check_missing_expected_values() {
123+
foreach( $this->expected as $type => $lines ) {
124+
if ( 'messages' === $type ) {
125+
continue;
126+
}
127+
foreach ( $lines as $line => $number ) {
128+
if ( false === isset( $this->$type[ $line ] ) ) {
129+
$this->error_warning_message( $number, $type, 0, $line );
130+
$this->total_issues ++;
131+
} else if ( $this->$type[ $line ] !== $number ) {
132+
$this->error_warning_message( $number, $type, $this->$type[ $line ], $line );
133+
$this->total_issues ++;
134+
}
135+
unset( $this->$type[ $line ] );
95136
}
96-
unset( $errors[$line] );
97137
}
98-
} else {
99-
foreach( $lines as $line => $number ) {
100-
if ( false === isset( $warnings[$line] ) ) {
101-
printf( 'Expected %d warnings, found %d on line %d' . PHP_EOL, $number, 0, $line );
102-
$total_issues++;
103-
} else if ( $warnings[$line] !== $number ) {
104-
printf( 'Expected %d warnings, found %d on line %d' . PHP_EOL, $number, $warnings[$line], $line );
105-
$total_issues++;
138+
}
139+
140+
private function check_unexpected_values() {
141+
foreach( array( 'errors', 'warnings' ) as $type ) {
142+
foreach ( $this->$type as $line => $number ) {
143+
if ( false === isset( $expected[ $type ][ $line ] ) ) {
144+
$this->error_warning_message( 0, $type, $number, $line );
145+
$this->total_issues ++;
146+
} else if ( $number !== $expected[ $type ][ $line ] ) {
147+
$this->error_warning_message( $expected[ $type ][ $line ], $type, $number, $line );
148+
$this->total_issues ++;
149+
}
106150
}
107-
unset( $warnings[$line] );
108-
109151
}
110152
}
111-
}
112153

113-
// Check for extra values which were not expected
114-
foreach( $errors as $line => $number ) {
115-
if ( false === isset( $expected['errors'][$line] ) ) {
116-
printf( 'Expected %d errors, found %d on line %d' . PHP_EOL, 0, $number, $line );
117-
$total_issues++;
118-
} else if ( $number !== $expected['errors'][$line] ) {
119-
printf( 'Expected %d errors, found %d on line %d' . PHP_EOL, $expected['errors'][$line], $number, $line );
120-
$total_issues++;
154+
private function check_messages() {
155+
foreach( $this->expected['messages'] as $line => $messages ) {
156+
foreach( $messages as $message ) {
157+
if ( false === isset( $this->messages[ $line ] ) ) {
158+
printf( 'Expected "%s" but found no message for line %d' . PHP_EOL, $message, $line );
159+
$this->total_issues ++;
160+
} else if ( false === in_array( $message, $this->messages[$line] ) ) {
161+
printf( 'Expected message "%s" was not found for line %d' . PHP_EOL, $message, $line );
162+
$this->total_issues ++;
163+
}
164+
}
165+
}
166+
foreach( $this->messages as $line => $messages ) {
167+
foreach( $messages as $message ) {
168+
if ( true === isset( $this->expected['messages'][$line] ) ) {
169+
if ( false === in_array( $message, $this->expected['messages'][$line] ) ) {
170+
printf( 'Unexpected message "%s" was found for line %d' . PHP_EOL, $message, $line );
171+
$this->total_issues ++;
172+
}
173+
}
174+
}
175+
}
121176
}
122-
}
123177

124-
foreach( $warnings as $line => $number ) {
125-
if ( false === isset( $expected['warnings'][$line] ) ) {
126-
printf( 'Expected %d warnings, found %d on line %d' . PHP_EOL, 0, $number, $line );
127-
$total_issues++;
128-
} else if ( $number !== $expected['warnings'][$line] ) {
129-
printf( 'Expected %d warnings, found %d on line %d' . PHP_EOL, $expected['warnings'][$line], $number, $line );
130-
$total_issues++;
178+
private function error_warning_message( $expected, $type, $number, $line ) {
179+
printf( 'Expected %d %s, found %d on line %d' . PHP_EOL, $expected, $type, $number, $line );
131180
}
132181
}
133182

134-
if ( 0 === $total_issues ) {
183+
// Run the tests!
184+
$test = new PHPCS_Ruleset_Test( $expected );
185+
if ( 0 === $test->run() ) {
135186
printf( 'No issues found. All tests passed!' . PHP_EOL );
136187
exit( 0 );
137188
} else {

0 commit comments

Comments
 (0)