Duplicate code is one of the most pungent code smells. A rule that is often used is to re-structure code once it is duplicated in three or more places.
Common duplication problems, and corresponding solutions are:
Complex classes like Jetpack_Cxn_Test_Base often do a lot of different things. To break such a class down, we need to identify a cohesive component within that class. A common approach to find such a component is to look for fields/methods that share the same prefixes, or suffixes. You can also have a look at the cohesion graph to spot any un-connected, or weakly-connected components.
Once you have determined the fields that belong together, you can apply the Extract Class refactoring. If the component makes sense as a sub-class, Extract Subclass is also a candidate, and is often faster.
While breaking up the class, it is a good idea to analyze how other classes use Jetpack_Cxn_Test_Base, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
26 | class Jetpack_Cxn_Test_Base { |
||
27 | |||
28 | /** |
||
29 | * Tests to run on the Jetpack connection. |
||
30 | * |
||
31 | * @var array $tests |
||
32 | */ |
||
33 | protected $tests = array(); |
||
34 | |||
35 | /** |
||
36 | * Results of the Jetpack connection tests. |
||
37 | * |
||
38 | * @var array $results |
||
39 | */ |
||
40 | protected $results = array(); |
||
41 | |||
42 | /** |
||
43 | * Status of the testing suite. |
||
44 | * |
||
45 | * Used internally to determine if a test should be skipped since the tests are already failing. Assume passing. |
||
46 | * |
||
47 | * @var bool $pass |
||
48 | */ |
||
49 | protected $pass = true; |
||
50 | |||
51 | /** |
||
52 | * Jetpack_Cxn_Test constructor. |
||
53 | */ |
||
54 | public function __construct() { |
||
58 | |||
59 | /** |
||
60 | * Adds a new test to the Jetpack Connection Testing suite. |
||
61 | * |
||
62 | * @since 7.1.0 |
||
63 | * @since 7.3.0 Adds name parameter and returns WP_Error on failure. |
||
64 | * |
||
65 | * @param callable $callable Test to add to queue. |
||
66 | * @param string $name Unique name for the test. |
||
67 | * @param string $type Optional. Core Site Health type: 'direct' if test can be run during initial load or 'async' if test should run async. |
||
68 | * @param array $groups Optional. Testing groups to add test to. |
||
69 | * |
||
70 | * @return mixed True if successfully added. WP_Error on failure. |
||
71 | */ |
||
72 | public function add_test( $callable, $name, $type = 'direct', $groups = array( 'default' ) ) { |
||
92 | |||
93 | /** |
||
94 | * Lists all tests to run. |
||
95 | * |
||
96 | * @since 7.3.0 |
||
97 | * |
||
98 | * @param string $type Optional. Core Site Health type: 'direct' or 'async'. All by default. |
||
99 | * @param string $group Optional. A specific testing group. All by default. |
||
100 | * |
||
101 | * @return array $tests Array of tests with test information. |
||
102 | */ |
||
103 | public function list_tests( $type = 'all', $group = 'all' ) { |
||
123 | |||
124 | /** |
||
125 | * Run a specific test. |
||
126 | * |
||
127 | * @since 7.3.0 |
||
128 | * |
||
129 | * @param string $name Name of test. |
||
130 | * |
||
131 | * @return mixed $result Test result array or WP_Error if invalid name. { |
||
132 | * @type string $name Test name |
||
133 | * @type mixed $pass True if passed, false if failed, 'skipped' if skipped. |
||
134 | * @type string $message Human-readable test result message. |
||
135 | * @type string $resolution Human-readable resolution steps. |
||
136 | * } |
||
137 | */ |
||
138 | public function run_test( $name ) { |
||
144 | |||
145 | /** |
||
146 | * Runs the Jetpack connection suite. |
||
147 | */ |
||
148 | public function run_tests() { |
||
159 | |||
160 | /** |
||
161 | * Returns the full results array. |
||
162 | * |
||
163 | * @since 7.1.0 |
||
164 | * @since 7.3.0 Add 'type' |
||
165 | * |
||
166 | * @param string $type Test type, async or direct. |
||
167 | * @param string $group Testing group whose results we want. Defaults to all tests. |
||
168 | * @return array Array of test results. |
||
169 | */ |
||
170 | public function raw_results( $type = 'all', $group = 'all' ) { |
||
195 | |||
196 | /** |
||
197 | * Returns the status of the connection suite. |
||
198 | * |
||
199 | * @since 7.1.0 |
||
200 | * @since 7.3.0 Add 'type' |
||
201 | * |
||
202 | * @param string $type Test type, async or direct. Optional, direct all tests. |
||
203 | * @param string $group Testing group to check status of. Optional, default all tests. |
||
204 | * |
||
205 | * @return true|array True if all tests pass. Array of failed tests. |
||
206 | */ |
||
207 | public function pass( $type = 'all', $group = 'all' ) { |
||
220 | |||
221 | /** |
||
222 | * Return array of failed test messages. |
||
223 | * |
||
224 | * @since 7.1.0 |
||
225 | * @since 7.3.0 Add 'type' |
||
226 | * |
||
227 | * @param string $type Test type, direct or async. |
||
228 | * @param string $group Testing group whose failures we want. Defaults to "all". |
||
229 | * |
||
230 | * @return false|array False if no failed tests. Otherwise, array of failed tests. |
||
231 | */ |
||
232 | public function list_fails( $type = 'all', $group = 'all' ) { |
||
244 | |||
245 | /** |
||
246 | * Helper function to return consistent responses for a passing test. |
||
247 | * Possible Args: |
||
248 | * - name: string The raw method name that runs the test. Default 'unnamed_test'. |
||
249 | * - label: bool|string If false, tests will be labeled with their `name`. You can pass a string to override this behavior. Default false. |
||
250 | * - pass: bool|string True if the test passed. Default true. |
||
251 | * - short_description: bool|string A brief, non-html description that will appear in CLI results. Default 'Test passed!'. |
||
252 | * - long_description: bool|string An html description that will appear in the site health page. Default false. |
||
253 | * - severity: bool|string 'critical', 'recommended', or 'good'. Default: false. |
||
254 | * - action: bool|string A URL for the recommended action. Default: false |
||
255 | * - action_label: bool|string The label for the recommended action. Default: false |
||
256 | * - show_in_site_health: bool True if the test should be shown on the Site Health page. Default: true |
||
257 | * |
||
258 | * @param array $args Arguments to override defaults. |
||
259 | * |
||
260 | * @return array Test results. |
||
261 | */ |
||
262 | View Code Duplication | public static function passing_test( $args ) { |
|
278 | |||
279 | /** |
||
280 | * Helper function to return consistent responses for a skipped test. |
||
281 | * Possible Args: |
||
282 | * - name: string The raw method name that runs the test. Default unnamed_test. |
||
283 | * - label: bool|string If false, tests will be labeled with their `name`. You can pass a string to override this behavior. Default false. |
||
284 | * - pass: bool|string True if the test passed. Default 'skipped'. |
||
285 | * - short_description: bool|string A brief, non-html description that will appear in CLI results, and as headings in admin UIs. Default false. |
||
286 | * - long_description: bool|string An html description that will appear in the site health page. Default false. |
||
287 | * - severity: bool|string 'critical', 'recommended', or 'good'. Default: false. |
||
288 | * - action: bool|string A URL for the recommended action. Default: false |
||
289 | * - action_label: bool|string The label for the recommended action. Default: false |
||
290 | * - show_in_site_health: bool True if the test should be shown on the Site Health page. Default: true |
||
291 | * |
||
292 | * @param array $args Arguments to override defaults. |
||
293 | * |
||
294 | * @return array Test results. |
||
295 | */ |
||
296 | View Code Duplication | public static function skipped_test( $args = array() ) { |
|
312 | |||
313 | /** |
||
314 | * Helper function to return consistent responses for a failing test. |
||
315 | * Possible Args: |
||
316 | * - name: string The raw method name that runs the test. Default unnamed_test. |
||
317 | * - label: bool|string If false, tests will be labeled with their `name`. You can pass a string to override this behavior. Default false. |
||
318 | * - pass: bool|string True if the test passed. Default false. |
||
319 | * - short_description: bool|string A brief, non-html description that will appear in CLI results, and as headings in admin UIs. Default 'Test failed!'. |
||
320 | * - long_description: bool|string An html description that will appear in the site health page. Default false. |
||
321 | * - severity: bool|string 'critical', 'recommended', or 'good'. Default: 'critical'. |
||
322 | * - action: bool|string A URL for the recommended action. Default: false. |
||
323 | * - action_label: bool|string The label for the recommended action. Default: false. |
||
324 | * - show_in_site_health: bool True if the test should be shown on the Site Health page. Default: true |
||
325 | * |
||
326 | * @since 7.1.0 |
||
327 | * |
||
328 | * @param array $args Arguments to override defaults. |
||
329 | * |
||
330 | * @return array Test results. |
||
331 | */ |
||
332 | View Code Duplication | public static function failing_test( $args ) { |
|
348 | |||
349 | /** |
||
350 | * Provide WP_CLI friendly testing results. |
||
351 | * |
||
352 | * @since 7.1.0 |
||
353 | * @since 7.3.0 Add 'type' |
||
354 | * |
||
355 | * @param string $type Test type, direct or async. |
||
356 | * @param string $group Testing group whose results we are outputting. Default all tests. |
||
357 | */ |
||
358 | public function output_results_for_cli( $type = 'all', $group = 'all' ) { |
||
380 | |||
381 | /** |
||
382 | * Output results of failures in format expected by Core's Site Health tool for async tests. |
||
383 | * |
||
384 | * Specifically not asking for a testing group since we're opinionated that Site Heath should see all. |
||
385 | * |
||
386 | * @since 7.3.0 |
||
387 | * |
||
388 | * @return array Array of test results |
||
389 | */ |
||
390 | public function output_results_for_core_async_site_health() { |
||
444 | |||
445 | /** |
||
446 | * Provide single WP Error instance of all failures. |
||
447 | * |
||
448 | * @since 7.1.0 |
||
449 | * @since 7.3.0 Add 'type' |
||
450 | * |
||
451 | * @param string $type Test type, direct or async. |
||
452 | * @param string $group Testing group whose failures we want converted. Default all tests. |
||
453 | * |
||
454 | * @return WP_Error|false WP_Error with all failed tests or false if there were no failures. |
||
455 | */ |
||
456 | public function output_fails_as_wp_error( $type = 'all', $group = 'all' ) { |
||
480 | |||
481 | /** |
||
482 | * Encrypt data for sending to WordPress.com. |
||
483 | * |
||
484 | * @todo When PHP minimum is 5.3+, add cipher detection to use an agreed better cipher than RC4. RC4 should be the last resort. |
||
485 | * |
||
486 | * @param string $data Data to encrypt with the WP.com Public Key. |
||
487 | * |
||
488 | * @return false|array False if functionality not available. Array of encrypted data, encryption key. |
||
489 | */ |
||
490 | public function encrypt_string_for_wpcom( $data ) { |
||
511 | } |
||
512 |
This check compares calls to functions or methods with their respective definitions. If the call has more arguments than are defined, it raises an issue.
If a function is defined several times with a different number of parameters, the check may pick up the wrong definition and report false positives. One codebase where this has been known to happen is Wordpress.
In this case you can add the
@ignore
PhpDoc annotation to the duplicate definition and it will be ignored.