Note: the inspection has settings
Note: the inspection documentation is not complete - some chapters are missing
By literal operators we mean and, or keywords used "instead" of &&, ||. Instead is double quoted for the same
reason why we encourage you to use &&, || - because of the operators precedence which differs.
Please reference to the official documentation for more details. Following example should give you a common idea about side-effects:
$x = false || true; // is ($x = (false || true)) => true
$x = false or true; // is (($x = false) or true) => false
$x = true && false; // is ($x = (true && false)) => false
$x = true and false; // is (($x = true) and false) => trueThe inspection reports following cases:
/* case 1: PDO:query() can be used instead */
$statement = $pdo->prepare('...');
/* the prepared statement executed immediately and without parameters and result check */
$statement->execute(); The case doesn't have any parameters binding and the prepared statement is not really make any sense. Also, the result of the query execution is not even checked. Following changes can be applied to fix the case:
- Apply a Quick-Fix to use PDO:query() instead
- Bind arguments while executing the statement
- Validate the statement execution result
/* case 2: PDO:exec() can be used instead - result statement of PDO:query() is not used */
$pdo->query('...');The case will consume system resources while the constructed statement is not used at all. Following changes can be applied to fix the case:
- Apply a Quick-Fix to use PDO:exec() instead
- Use the returned statement
Since generic exceptions are not helpful when maintaining and debugging PHP-application, we would suggest using more specific SPL exceptions.
Informative and reasonably detailed exception message saves debugging time a lot, hence we spot places where exception messages are not provided at all.
In case of well-structured domains, exception classes naming can be speaking itself. A couple ideas about raising them Php Inspections (EA Extended) friendly way:
/* exception parameters injection (builder approach) */
throw (new UserNotFoundException())->withUserId($userId);
/* custom constructor */
throw new UserNotFoundException($userId);The inspection finds places where conditional return-statements can be simplified (by Quick-Fixing), reducing code complexity metrics and size of maintainable codebase.
/* before */
if ($variable === 'value') {
return true;
}
return false;
/* after */
return $variable === 'value';The inspection finds loop-constructs that are terminated with continue, break, throw or return in the first iteration. While this approach applicable to generators and iterable classes, in general it points to refactoring leftovers, bad merges and bugs.
Note: while terminating statements can be preceded by other statements, the inspection still reports the pattern
The inspection finds switch-constructs which can be refactored into more suitable constructs (e.g. if-constructs), reducing cognitive load and clearer expressing code intention.
/* before */
switch ($variable) {
case 'value':
/* operations: 1st batch */
break;
default:
/* operations: 2nd batch */
break;
}
/* after */
if ($variable === 'value') {
/* operations: 1st batch */
} else {
/* operations: 2nd batch */
}The inspection finds for-constructs which can be refactored (by Quick-Fixing) into foreach-constructs, reducing cognitive load, improving maintainability and enabling the analyzer to apply more checks.
Note: foreach-statements are also well optimized for working with iterators.
Reports if ternary operator can be refactored to simply use the conditional variable as the result (thus omitting the ternary branches). This reduces the both cyclomatic and cognitive code complexity.
/* before */
$variable = $number > 0 ? true : false;
$variable = $number & $flag ? true : false;
/* after */
$variable = $number > 0;
$variable = (bool) ($number & $flag);Analyzes foreach loops and reports expressions that are not connected with the loop's scope. In the most cases such expression can be executed once outside of corresponding loop.
Note: known false-positives are related to the input/output operations, executed in the loop - suppression recommended for this case
Reports 'gettype()' function usages, which can be replaced by 'is_array()' and similar functions. Such replacement is clearer expresses intention and more friendly to Static Code Analysis tools relying to types inference.
Analyzes if the 'unset(...)' function was called sequentially. This can be safely replaced with the 'unset(..., ...[, ...])' construction.
/* before */
unset($variable);
unset($argument);
/* after */
unset($variable, $argument);In some cases variables are used as temporary value containers and it's possible to omit such variables at all.
/* before */
$object = new Clazz();
return $object->method();
/* after */
return (new Clazz())->method();Since PHP 5.5 it's possible to use mass-assignments from array with 'list($variable, ...) = $array'. Additionally, to code compactness and performance, you'll notice if data array contains any un-used entries.
/* before */
$array = [ ... ];
$first = $array[0];
$second = $array[1];
/* after */
list($first, $second) = [ ... ];Reports if 'array_search(...)' is used in the context of 'in_array(...)', where overhead lies in storing and returning index, which is not used in case of 'in_array(...)'. Also reported cases are misleading and refactoring with 'in_array(...)' would clearer express intention of code constructs.
/* before */
if (array_search($what, $where, true) === false) {
/* some logic here */
}
/* after */
if (! in_array($what, $where, true)) {
/* some logic here */
}Reports if 'strtr(...)' is used in the context of 'str_replace(...)'. Reported cases are misleading and refactoring with 'str_replace(...)' would clearer express intention of code constructs.
/* before */
$normalizedPath = strtr($string, '\\', '/');
/* after */
$normalizedPath = str_replace('\\', '/', $string);'substr(...)' invokes additional memory allocation (more work for GC), which is not needed in the context. 'strpos(...)' will do the same job with CPU resources only (assuming you are not dealing with big text fragments).
/* before */
$containsString = substr($where, 0, strlen($what)) === $what;
/* after */
$containsString = strpos($where, $what) === 0;If you are eager for performance in your project this inspection in combination with "Fixed-time string starts with checks" (which is disabled by default) would be a good option.
The inspection checks order of string case and length manipulations and reports anomalies.
/* before */
$normalizedString = trim(strtolower($string));
/* after */
$normalizedString = strtolower(trim($string));Certain if-else conditions can have their else clause removed without changing the semantic.
In the example below, the 'else' statement can be safely removed because its 'if' clause returns a value. Thus, even without the 'else/elseif', there’s no way you’ll be able to proceed past the if clause body.
/* before */
if ($value !== 'forbidden-value') {
return $value;
} else {
return null;
}
/* after */
if ($value !== 'forbidden-value') {
return $value;
}
return null;Please reference to corresponding stackoverflow thread for more details.
Note: this inspection is disabled by default as it might conflict with "Redundant 'else' keyword" inspection and guard clauses code style
Certain if-else conditions can have their conditions unnecessary inverted, so removing inversion and exchanging branches content is not changing the semantic.
/* before */
if (! $rule->validate()) {
/* failed validation logic here */
} else {
/* passed validation logic here */
}
/* after */
if ($rule->validate()) {
/* passed validation logic here */
} else {
/* failed validation logic here */
}In some cases string case manipulation is not necessary.
/* before */
$matched = preg_match('/^prefix/i', strtolower($string));
/* after */
$matched = preg_match('/^prefix/i', $string);We were promoting usage of 'array_count_values(...)' instead of 'array_unique(...)' in some contexts previously, but the 'array_unique(...)' was optimized and we are now suggesting the opposite way.
See this thread for more details.
/* before */
$values = array_keys(array_count_values($array));
$count = count(array_count_values($array));
/* after */
$values = array_values(array_unique($array));
$count = count(array_unique($array));Note: this inspection is disabled by default as using 'compact(...)' is not a common practice
Suggests using 'compact(...)' when it makes sense, normally this happens in context of dispatching arguments into templates.
/* before */
$array = ['first' => $first, 'second' => $second];
/* after */
$array = compact('first', 'second');The inspection is advising when multiple 'isset(...)' statements can be merged into one.
/* before */
$value = isset($first) && isset($second);
/* after */
$value = isset($first, $second);Analyzes 'strlen(...)' usages and reports if the construction can be replaced with empty string comparison.
/* before */
if (strlen($string)) {}
if (! strlen($string)) {}
/* after */
if ($string !== '') {}
if ($string === '') {}Analyzes and reports code construct, which could use 'ob_get_clean()' function.
/* before */
$content = ob_get_contents();
ob_end_clean();
/* after */
$content = ob_get_clean();