9. Rules list

9.1. Introduction

9.2. $HTTP_RAW_POST_DATA Usage

$HTTP_RAW_POST_DATA is deprecated, and should be replaced by php://input.

$HTTP_RAW_POST_DATA is deprecated since PHP 5.6.

It is possible to ready by setting always_populate_raw_post_data to -1.

<?php

// PHP 5.5 and older
$postdata = $HTTP_RAW_POST_DATA;

// PHP 5.6 and more recent
$postdata = file_get_contents(php://input);

?>

See also $HTTP_RAW_POST_DATA variable.

Short name Php/RawPostDataUsage
Themes CompatibilityPHP56
Severity Major
Time To Fix Slow (1 hour)

9.3. $this Belongs To Classes Or Traits

$this variable represents only the current object.

It is a pseudo-variable, and should be used within class’s or trait’s methods (except for static) and not outside.

PHP 7.1 is stricter and check for $this at several positions. Some are found by static analysis, some are dynamic analysis.

<?php

// as an argument
function foo($this) {
    // Using global
    global $this;
    // Using static (not a property)
    static $this;

    // Can't unset it
    unset($this);

    try {
        // inside a foreach
        foreach($a as $this) {  }
        foreach($a as $this => $b) {  }
        foreach($a as $b => $this) {  }
    } catch (Exception $this) {
        // inside a catch
    }

    // with Variable Variable
    $a = this;
    $$a = 42;
}

class foo {
    function bar() {
        // Using references
        $a =& $this;
        $a = 42;

        // Using extract(), parse_str() or similar functions
        extract([this => 42]);  // throw new Error(Cannot re-assign $this)
        var_dump($this);
    }

    static function __call($name, $args) {
        // Using __call
        var_dump($this); // prints object(C)#1 (0) {}, php-7.0 printed NULL
        $this->test();   // prints ops
    }

}
?>

9.3.1. Suggestions

  • Do not use $this as a variable name, except for the current object, in a class, trait or closure.
Short name Classes/ThisIsForClasses
Themes Analyze
Severity Major
Time To Fix Quick (30 mins)
Examples OpenEMR

9.4. $this Is Not An Array

$this variable represents the current object and it is not an array.

This is unless the class (or its parents) has the ArrayAccess interface, or extends ArrayObject or SimpleXMLElement.

<?php

// $this is an array
class Foo extends ArrayAccess {
    function bar() {
        ++$this[3];
    }
}

// $this is not an array
class Foo2 {
    function bar() {
        ++$this[3];
    }
}

?>

See also ArrayAccess, ArrayObject and The Basics.

9.4.1. Suggestions

  • Extends ArrayObject, or a class that extends it, to use $this as an array too.
  • Implements ArrayAccess to use $this as an array too.
  • Use a property in the current class to store the data, instead of $this directly.
Short name Classes/ThisIsNotAnArray
Themes Analyze
Severity Major
Time To Fix Quick (30 mins)

9.5. $this Is Not For Static Methods

Static methods shouldn’t use $this variable.

$this variable represents an object, the current object. It is not compatible with a static method, which may operate without any object.

While executing a static method, $this is actually set to NULL.

<?php

class foo {
    static $staticProperty = 1;

    // Static methods should use static properties
    static public function count() {
        return self::$staticProperty++;
    }

    // Static methods can't use $this
    static public function bar() {
        return $this->a;   // No $this usage in a static method
    }
}

?>

See also Static Keyword.

9.5.1. Suggestions

  • Remove the static keyword on the method, and update all calls to this method to use $this
  • Remove the usage of $this in the method, replacing it with static properties
  • Make $this an argument (and change its name) : then, make the method a function
Short name Classes/ThisIsNotForStatic
Themes Analyze
Severity Major
Time To Fix Quick (30 mins)
ClearPHP no-static-this

9.6. ** For Exponent

PHP has the operator ** to provide exponents, instead of the slower function pow(). This operator was introduced in PHP 5.6.

<?php
    $cube = pow(2, 3); // 8

    $cubeInPHP56 = 2 ** 3; // 8
?>

If the code needs to be backward compatible to 5.5 or less, don’t use the new operator.

Be aware the the ‘-‘ operator has lower priority than the ** operator : this leads to the following confusing result.

<?php
    echo -3 ** 2;
    // displays -9, instead of 9
?>

This is due to the parser that process separately - and the following number. Since ** has priority, the power operation happens first.

Being an operator, ** is faster than pow(). This is a microoptimisation.

See also Arithmetic Operators.

9.6.1. Suggestions

  • Use the ** operator
  • For powers of 2, use the bitshift operators
  • For literal powers of 2, consider using the 0xFFFFFFFFF syntax.
Short name Php/NewExponent
Themes Suggestions
Php Version With PHP 5.6 and more recent
Severity Minor
Time To Fix Quick (30 mins)
Examples Traq, TeamPass

9.7. ::class

PHP has a special class constant to hold the name of the class : class keyword. It represents the class name that is used in the left part of the operator.

Using \:\:class is safer than relying on a string. It does adapt if the class’s name or its namespace is changed’. It is also faster, though it is a micro-optimisation.

It is introduced in PHP 5.5.

<?php

use A\B\C as UsedName;

class foo {
    public function bar( ) {
        echo ClassName::class;
        echo UsedName::class;
    }
}

$f = new Foo( );
$f->bar( );
// displays ClassName
// displays A\B\C

?>

Be aware that \:\:class is a replacement for __CLASS__ magic constant.

See also Class Constant.

9.7.1. Suggestions

  • Use ::class whenever possible. That exclude any dynamic call.
Short name Php/StaticclassUsage
Themes CompatibilityPHP53, CompatibilityPHP54
Php Version With PHP 5.5 and more recent
Severity Major
Time To Fix Slow (1 hour)

9.8. @ Operator

@ is the ‘no scream’ operator : it suppresses error output.

<?php

// Set x with incoming value, or else null.
$x = @$_GET['x'];

?>

This operator is actually very slow : it will process the error all the way up, and finally decide not to display it. It is often faster to check the conditions first, then run the method without @.

You may also set display_error to 0 in the php.ini : this will avoid user’s error display, but will keep the error in the PHP logs, for later processing.

The only situation where @ is useful is when a native PHP function displays errors messages when error happens and there is no way to check it from the code.

This is the case with fopen(), stream_socket_server(), token_get_all().

See also Error Control Operators and Five reasons why the shut-op operator should be avoided.

9.8.1. Suggestions

  • Remove the @ operator by default
Short name Structures/Noscream
Themes Analyze, Performances
Severity Minor
Time To Fix Quick (30 mins)
ClearPHP no-noscream
Examples Phinx, PhpIPAM

9.9. Abstract Or Implements

A class must implements all abstract methods of it parent, or be abstract too.

While PHP lints this code, it won’t execute it and stop with a Fatal Error : Class BA contains 1 abstract method and must therefore be declared abstract or implement the remaining methods (A\:\:aFoo).

<?php

abstract class Foo {
    abstract function FooBar();
}

// This is in another file : php -l would detect it right away

class FooFoo extends Foo {
    // The method is not defined.
    // The class must be abstract, just like Foo
}

?>

See also Class Abstraction.

9.9.1. Suggestions

  • Implements all the abstract methods of the class
  • Make the class abstract
Short name Classes/AbstractOrImplements
Themes Analyze, LintButWontExec
Severity Major
Time To Fix Quick (30 mins)
Examples Zurmo

9.10. Abstract Static Methods

Methods cannot be both abstract and static. Static methods belong to a class, and will not be overridden by the child class. For normal methods, PHP will start at the object level, then go up the hierarchy to find the method. With static, it is necessary to mention the name, or use Late Static Binding, with self or static. Hence, it is useless to have an abstract static method : it should be a static method.

A child class is able to declare a method with the same name than a static method in the parent, but those two methods will stay independent.

This is not the case anymore in PHP 7.0+.

<?php

abstract class foo {
    // This is not possible
    static abstract function bar() ;
}

?>

See also Why does PHP 5.2+ disallow abstract static class methods?.

Short name Classes/AbstractStatic
Themes Analyze
Php Version With PHP 7.0 and older
Severity Minor
Time To Fix Quick (30 mins)

9.11. Access Protected Structures

It is not allowed to access protected properties or methods from outside the class or its relatives.

<?php

class foo {
    protected $bar = 1;
}

$foo = new Foo();
$foo->bar = 2;

?>
Short name Classes/AccessProtected
Themes Analyze
Severity Major
Time To Fix Quick (30 mins)

9.12. Accessing Private

List of calls to private properties/methods that will compile but yield some fatal error upon execution.

<?php

class a {
    private $a;
}

class b extends a {
    function c() {
        $this->a;
    }
}

?>
Short name Classes/AccessPrivate
Themes Analyze
Severity Major
Time To Fix Quick (30 mins)

9.13. Add Default Value

Parameter in methods definition may receive a default value. This allows the called method to set a value when the parameter is omitted.

<?php

function foo($i) {
    if (!is_integer($i)) {
        $i = 0;
    }
}

?>

See also Function arguments.

9.13.1. Suggestions

  • Add a default value for parameters
Short name Functions/AddDefaultValue
Themes Suggestions
Severity Minor
Time To Fix Quick (30 mins)
Examples Zurmo, Typo3

9.14. Adding Zero

Adding 0 is useless, as 0 is the neutral element for addition. In PHP, it triggers a cast to integer.

It is recommended to make the cast explicit with (int)

<?php

// Explicit cast
$a = (int) foo();

// Useless addition
$a = foo() + 0;
$a = 0 + foo();

// Also works with minus
$b = 0 - $c; // drop the 0, but keep the minus
$b = $c - 0; // drop the 0 and the minus

$a += 0;
$a -= 0;

?>

If it is used to type cast a value to integer, then casting (integer) is clearer.

9.14.1. Suggestions

  • Remove the + 0
  • Use an explicit type casting operator (int)
Short name Structures/AddZero
Themes Analyze
Severity Minor
Time To Fix Instant (5 mins)
ClearPHP no-useless-math
Examples Thelia, OpenEMR

9.15. Aliases Usage

PHP manual recommends to avoid function aliases.

Some functions have several names, and both may be used the same way. However, one of the names is the main name, and the others are aliases. Aliases may be removed or change or dropped in the future. Even if this is not forecast, it is good practice to use the main name, instead of the aliases.

<?php

// official way to count an array
$n = count($array);

// official way to count an array
$n = sizeof($array);

?>

Aliases are compiled in PHP, and do not provide any performances over the normal function.

Aliases are more likely to be removed later, but they have been around for a long time.

See documentation : List of function aliases.

9.15.1. Suggestions

  • Always use PHP recommended functions
Short name Functions/AliasesUsage
Themes Analyze
Severity Minor
Time To Fix Quick (30 mins)
ClearPHP no-aliases
Examples Cleverstyle, phpMyAdmin

9.16. All Uppercase Variables

Usually, global variables are all in uppercase, so as to differentiate them easily. Though, this is not always the case, with examples like $argc, $argv or $http_response_header.

When using custom variables, try to use lowercase $variables, $camelCase, $sturdyCase or $snake_case.

<?php

// PHP super global, also identified by the initial _
$localVariable = $_POST;

// PHP globals
$localVariable = $GLOBALS['HTTPS'];

?>

See also Predefined Variables.

Short name Variables/VariableUppercase
Themes Coding Conventions
Severity Minor
Time To Fix Slow (1 hour)

9.17. Already Parents Interface

The same interface is implemented by a class and one of its children.

That way, the child doesn’t need to implement the interface, nor define its methods to be an instance of the interface.

<?php

interface i {
    function i();
}

class A implements i {
    function i() {
        return __METHOD__;
    }
}

// This implements is useless.
class AB extends A implements i {
    // No definition for function i()
}

// Implements i is understated
class AB extends A {
    // redefinition of the i method
    function i() {
        return __METHOD__.' ';
    }
}

$x = new AB;
var_dump($x instanceof i);
// true

$x = new AC;
var_dump($x instanceof i);
// true

?>

9.17.1. Suggestions

  • Keep the implements call in the class that do implements the methods. Remove it from the children classes.
Short name Interfaces/AlreadyParentsInterface
Themes Analyze, Suggestions
Severity Minor
Time To Fix Instant (5 mins)
Examples WordPress, Thelia

9.18. Already Parents Trait

Trait is already used a parent’s class or trait. There is no use to include it a second time.

<?php

trait ta {
    use tb;
}

trait t1 {
    use ta;
    use tb; // also used by ta
}

class b {
    use t1; // also required by class c
    use ta; // also required by trait t1
}

class c extends b {
    use t1;
}

?>

See also Traits.

9.18.1. Suggestions

  • Eliminate one of the trait request
Short name Traits/AlreadyParentsTrait
Themes Analyze
Severity Minor
Time To Fix Quick (30 mins)

9.19. Altering Foreach Without Reference

Foreach() loop that should use a reference.

When using a foreach loop that modifies the original source, it is recommended to use referenced variables, rather than access the original value with $source[$index].

Using references is then must faster, and easier to read.

<?php

// Using references in foreach
foreach($source as $key => &$value) {
    $value = newValue($value, $key);
}

// Avoid foreach : use array_map
$source = array_walk($source, 'newValue');
    // Here, $key MUST be the second argument or newValue

// Slow version to update the array
foreach($source as $key => &$value) {
    $source[$key] = newValue($value, $key);
}
?>

You may also use array_walk() or array_map() (when $key is not used) to avoid the use of foreach.

See also foreach.

9.19.1. Suggestions

  • Add the reference on the modified blind variable, and avoid accessing the source array
Short name Structures/AlteringForeachWithoutReference
Themes Analyze
Severity Major
Time To Fix Quick (30 mins)
ClearPHP use-reference-to-alter-in-foreach
Examples Contao, WordPress

9.20. Alternative Syntax Consistence

PHP allows for two syntax : the alternative syntax, and the classic syntax.

The classic syntax is almost always used. When used, the alternative syntax is used in templates.

This analysis reports files that are using both syntax at the same time. This is confusing.

<?php

// Mixing both syntax is confusing.
foreach($array as $item) :
    if ($item > 1) {
        print $item elements\n;
    } else {
        print $item element\n;
    }
endforeach;

?>
Short name Structures/AlternativeConsistenceByFile
Themes Analyze
Severity Major
Time To Fix Quick (30 mins)

9.21. Always Anchor Regex

Unanchored regex finds the requested pattern, and leaves room for malicious content.

Without ^ and $, the regex searches for any pattern that satisfies the criteria, leaving any unused part of the string available for arbitrary content. It is recommended to use both anchor

<?php

$birthday = getSomeDate($_GET);

// Permissive version : $birthday = '1970-01-01<script>xss();</script>';
if (!preg_match('/\d{4}-\d{2}-\d{2}/', $birthday) {
    error('Wrong data format for your birthday!');
}

// Restrictive version : $birthday = '1970-01-01';
if (!preg_match('/^\d{4}-\d{2}-\d{2}$/', $birthday) {
    error('Wrong data format for your birthday!');
}

echo 'Your birthday is on '.$birthday;

?>

Note that $ may be a line ending, still leaving room after it for injection.

<?php

$birthday = '1970-01-01'.PHP_EOL.'<script>xss();</script>';

?>

This analysis reports false positive when the regex is used to search a pattern in a much larger string. Check if this rule doesn’t apply, though.

See also CWE-625: Permissive Regular Expression.

Short name Security/AnchorRegex
Themes Security
Severity Major
Time To Fix Instant (5 mins)

9.22. Always Positive Comparison

Some PHP native functions, such as count(), strlen(), or abs() only returns positive or null values.

When comparing them to 0, the following expressions are always true and should be avoided.

<?php

$a = [1, 2, 3];

var_dump(count($a) >= 0);
var_dump(count($a) < 0);

?>

9.22.1. Suggestions

  • Compare count() to non-zero values
  • Use empty()
Short name Structures/NeverNegative
Themes Analyze
Severity Major
Time To Fix Instant (5 mins)
Examples Magento

9.23. Always Use Function With array_key_exists()

array_key_exists() has been granted a special VM opcode, and is much faster. This applies to PHP 7.4 and more recent.

It requires that array_key_exists() is statically resolved, either with an initial \, or a use function expression. This doesn’t affect the global namespace.

<?php

namespace my/name/space;

// do not forget the 'function' keyword, or it will apply to classes.
use function array_key_exists as foo; // the alias is not necessary, and may be omitted.

// array_key_exists is aliased to foo :
$c = foo($a, $b);

// This call requires a fallback to global, and will be slow.
$c = array_key_exists($a, $b);

?>

This analysis is related to Php/ShouldUseFunction, and is a special case, that only concerns array_key_exists().

See also Add array_key_exists to the list of specialy compiled functions.

9.23.1. Suggestions

  • Use the use command for arrray_key_exists(), at the beginning of the script
  • Use an initial before array_key_exists()
  • Remove the namespace
Short name Performances/Php74ArrayKeyExists
Themes Performances
Severity Minor
Time To Fix Quick (30 mins)

9.24. Ambiguous Array Index

Indexes should not be defined with different types than int or string.

Array indices only accept integers and strings, so any other type of literal is reported. In fact, null is turned into an empty string, booleans are turned into an integer, and real numbers are truncated (not rounded).

<?php

$x = [ 1  => 1,
      '1' => 2,
      1.0 => 3,
      true => 4];
// $x only contains one element : 1 => 4

// Still wrong, immediate typecast to 1
$x[1.0]  = 5;
$x[true] = 6;

?>

They are indeed distinct, but may lead to confusion.

See also array.

9.24.1. Suggestions

  • Only use string or integer as key for an array.
  • Use transtyping operator (string) and (int) to make sure of the type
Short name Arrays/AmbiguousKeys
Themes Analyze
Severity Minor
Time To Fix Quick (30 mins)
Examples PrestaShop, Mautic

9.25. Ambiguous Static

Methods or properties with the same name, are defined static in one class, and not static in another. This is error prone, as it requires a good knowledge of the code to make it static or not.

Try to keep the methods simple and unique. Consider renaming the methods and properties to distinguish them easily. A method and a static method have probably different responsibilities.

<?php

class a {
    function mixedStaticMethod() {}
}

class b {
    static function mixedStaticMethod() {}
}

/... a lot more code later .../

$c->mixedStaticMethod();
// or
$c::mixedStaticMethod();

?>
Short name Classes/AmbiguousStatic
Themes Analyze
Severity Minor
Time To Fix Slow (1 hour)

9.26. Ambiguous Visibilities

The properties have the same name, but have different visibilities, across different classes.

While it is legit to have a property with the same name in different classes, it may easily lead to confusion. As soon as the context is need to understand if the property is accessible or not, the readability suffers.

It is recommended to handle the same properties in the same way across classes, even when the classes are not related.

<?php

class person {
    public $name;
    private $address;
}

class gangster {
    private $name;
    public $nickname;
    private $address;
}

$someone = Human::load(123);
echo 'Hello, '.$someone->name;

?>

9.26.1. Suggestions

  • Sync visibilities for both properties, in the different classes
  • Use different names for properties with different usages
Short name Classes/AmbiguousVisibilities
Themes Analyze
Severity Minor
Time To Fix Slow (1 hour)
Examples Typo3

9.27. Anonymous Classes

Anonymous classes.

<?php

// Anonymous class, available since PHP 7.0
$object = new class { function __construct() { echo __METHOD__; } };

?>
Short name Classes/Anonymous
Themes CompatibilityPHP53, CompatibilityPHP54, CompatibilityPHP55, CompatibilityPHP56
Php Version With PHP 7.0 and more recent
Severity Major
Time To Fix Slow (1 hour)

9.28. Argument Should Be Typehinted

When a method expects objects as argument, those arguments should be typehinted. This way, it provides early warning that a wrong object is being sent to the method.

The analyzer will detect situations where a class, or the keywords ‘array’ or ‘callable’.

<?php

// What are the possible classes that have a 'foo' method?
function foo($bar) {
    return $bar->foo();
}

?>

Closure arguments are omitted.

See also Type declarations.

9.28.1. Suggestions

  • Add the typehint to the function arguments
Short name Functions/ShouldBeTypehinted
Themes Suggestions
Severity Minor
Time To Fix Slow (1 hour)
ClearPHP always-typehint
Examples Dolphin, Mautic

9.29. Assert Function Is Reserved

Avoid defining an assert function in namespaces.

While they work fine when the assertions are active (zend.assertions=1), calls to unqualified assert are optimized away when assertions are not active.

Since PHP 7.3, a fatal error is emitted : Defining a custom `assert() <http://www.php.net/assert>`_ function is deprecated, as the function has special semantics.

<?php
//      Run this with zend.assertions=1 and
// Then run this with zend.assertions=0

namespace Test {
    function assert() {
        global $foo;

        $foo = true;
    }
}

namespace Test {
    assert();

    var_dump(isset($foo));
}

?>

See also assert and User-defined assert function is optimized away with zend.assertions=-1.

9.29.1. Suggestions

  • Rename the custom function with another name
Short name Php/AssertFunctionIsReserved
Themes Analyze, CompatibilityPHP73
Severity Critical
Time To Fix Slow (1 hour)

9.30. Assign And Compare

Assignation has a lower precedence than comparison. As such, the assignation always happens after the comparison. This leads to the comparison being stored in the variable, and not the value being compared.

<?php

if ($id = strpos($string, $needle) !== false) {
    // $id now contains a boolean (true or false), but not the position of the $needle.
}

// probably valid comparison, as $found will end up being a boolean
if ($found = strpos($string, $needle) === false) {
    doSomething();
}

// always valid comparison, with parenthesis
if (($id = strpos($string, $needle)) !== false) {
    // $id now contains a boolean (true or false), but not the position of the $needle.
}

// Being a lone instruction, this is always valid : there is no double usage with if condition
$isFound = strpos($string, $needle) !== false;


?>

See also Operator Precedence.

9.30.1. Suggestions

  • Use parenthesis
  • Separate assignation and comparison
  • Drop assignation or comparison
Short name Structures/AssigneAndCompare
Themes Analyze
Severity Minor
Time To Fix Quick (30 mins)

9.31. Assign Default To Properties

Properties may be assigned default values at declaration time. Such values may be later modified, if needed.

<?php

class foo {
    private $propertyWithDefault = 1;
    private $propertyWithoutDefault;
    private $propertyThatCantHaveDefault;

    public function __construct() {
        // Skip this extra line, and give the default value above
        $this->propertyWithoutDefault = 1;

        // Static expressions are available to set up simple computation at definition time.
        $this->propertyWithoutDefault = OtherClass::CONSTANT + 1;

        // Arrays, just like scalars, may be set at definition time
        $this->propertyWithoutDefault = [1,2,3];

        // Objects or resources can't be made default. That is OK.
        $this->propertyThatCantHaveDefault = fopen('/path/to/file.txt');
        $this->propertyThatCantHaveDefault = new Fileinfo();
    }
}

?>

Default values will save some instructions in the constructor, and makes the value obvious in the code.

9.31.1. Suggestions

  • Add a default value whenever possible. This is easy for scalars, and array()
Short name Classes/MakeDefault
Themes Analyze
Severity Minor
Time To Fix Instant (5 mins)
ClearPHP use-properties-default-values
Examples LiveZilla, phpMyAdmin

9.32. Assign With And

The lettered logical operators yield to assignation. It may collect less information than expected.

It is recommended to use the &&, ^ and || operators, instead of and, or and xor, to prevent confusion.

<?php

// The expected behavior is
// The following are equivalent
 $a =  $b  && $c;
 $a = ($b && $c);

// The unexpected behavior is
// The following are equivalent
 $a = $b  and $c;
($a = $b) and $c;

?>

See also Operator Precedence.

9.32.1. Suggestions

  • Always use symbol && rather than letter and
  • To be safe, add parenthesis to enforce priorities
Short name Php/AssignAnd
Themes Analyze
Severity Critical
Time To Fix Quick (30 mins)
Examples xataface

9.33. Assigned Twice

The same variable is assigned twice in the same function.

While this is possible and quite common, it is also a good practice to avoid changing a value from one literal to another. It is far better to assign the new value to

Incremental changes to a variables are not reported here.

<?php

function foo() {
    // incremental changes of $a;
    $a = 'a';
    $a++;
    $a = uppercase($a);

    $b = 1;
    $c = bar($b);
    // B changed its purpose. Why not call it $d?
    $b = array(1,2,3);

    // This is some forgotten debug
    $e = $config->getSomeList();
    $e = array('OneElement');
}

?>
Short name Variables/AssignedTwiceOrMore
Themes Analyze
Severity Minor
Time To Fix Quick (30 mins)

9.34. Autoappend

Appending a variable to itself leads to enormous usage of memory.

<?php

// Always append a value to a distinct variable
foreach($a as $b) {
    $c[] = $d;
}

// This copies the array to itself, and double the size each loop
foreach($a as $b) {
    $c[] = $c;
}
?>

9.34.1. Suggestions

  • Change the variable in the append, on the left
  • Change the variable in the append, on the right
Short name Performances/Autoappend
Themes Performances
Severity Minor
Time To Fix Quick (30 mins)

9.35. Avoid Concat In Loop

Concatenations inside a loop generate a lot of temporary variables. They are accumulated and tend to raise the memory usage, leading to slower performances.

It is recommended to store the values in an array, and then use implode() on that array to make the concatenation at once. The effect is positive when the source array has at least 50 elements.

<?php

// Concatenation in one operation
$tmp = array();
foreach(data_source() as $data) {
    $tmp[] = $data;
}
$final = implode('', $tmp);

// Concatenation in many operations
foreach(data_source() as $data) {
    $final .= $data;
}

?>

The same doesn’t apply to addition and multiplication, with array_sum() and array_multiply(), as those operations work on the current memory allocation, and don’t need to allocate new memory at each step.

See also PHP 7 performance improvements (3/5): Encapsed strings optimization.

9.35.1. Suggestions

  • Collect all pieces in an array, then implode() the array in one call.
Short name Performances/NoConcatInLoop
Themes Performances, Top10
Severity Major
Time To Fix Slow (1 hour)
Examples SuiteCrm, ThinkPHP

9.36. Avoid Large Array Assignation

Avoid setting large arrays to local variables. This is done every time the function is called.

There are different ways to avoid this : inject the array, build the array once. Using an constant or even a global variable is faster.

The effect on small arrays (less than 10 elements) is not significant. Arrays with 10 elements or more are reported here. The effect is also more important on functions that are called often, or within loops.

<?php

// with constants, for functions
const ARRAY = array(1,2,3,4,5,6,7,8,9,10,11);
function foo() {
    $array = ARRAY;
    //more code
}

// with class constants, for methods
class x {
    const ARRAY = array(1,2,3,4,5,6,7,8,9,10,11);
    function foo() {
        $array = self::ARRAY;
        //more code
    }
}

// with properties, for methods
class x {
    private $array = array(1,2,3,4,5,6,7,8,9,10,11);

    function foo() {
        $array = $this->array;
        //more code
    }
}

// injection, leveraging default values
function foo($array = array(1,2,3,4,5,6,7,8,9,10,11)) {
    //more code
}

// local cache with static
function foo() {
    static $array;
    if ($array === null) {
        $array = array(1,2,3,4,5,6,7,8,9,10,11);
    }

    //more code
}

// Avoid creating the same array all the time in a function
class x {
    function foo() {
        // assign to non local variable is OK.
        // Here, to a property, though it may be better in a __construct or as default values
        $this->s = array(1,2,3,4,5,6,7,8,9,10,11);

        // This is wasting resources, as it is done each time.
        $array = array(1,2,3,4,5,6,7,8,9,10,11);
    }
}

?>
Short name Structures/NoAssignationInFunction
Themes Performances
Severity Minor
Time To Fix Slow (1 hour)

9.37. Avoid Optional Properties

Avoid optional properties, to prevent littering the code with existence checks.

When a property has to be checked once for existence, it is safer to check it each time. This leads to a decrease in readability.

Either make sure the property is set with an actual object rather than with null, or use a void object. A void object offers the same interface than the expected object, but does nothing. It allows calling its methods, without running into a Fatal error, nor testing it.

<?php

// Example is courtesy 'The Coding Machine' : it has been adapted from its original form. See link below.

class MyMailer {
    private $logger;

    public function __construct(LoggerInterface $logger = null) {
        $this->logger = $logger;
    }

    private function sendMail(Mail $mail) {
        // Since $this->logger may be null, it must be tested anytime it is used.
        if ($this->logger) {
            $this->logger->info('Mail successfully sent.');
        }
    }
}

?>

See also Avoid optional services as much as possible, The Null Object Pattern – Polymorphism in Domain Models, and Practical PHP Refactoring: Introduce Null Object.

Short name Classes/AvoidOptionalProperties
Themes Analyze
Severity Major
Time To Fix Slow (1 hour)

9.38. Avoid Parenthesis

Avoid Parenthesis for language construct. Languages constructs are a few PHP native elements, that looks like functions but are not.

Among other distinction, those elements cannot be directly used as variable function call, and they may be used with or without parenthesis.

<?php

// normal usage of include
include 'file.php';

// This looks like a function and is not
include('file2.php');

?>

The usage of parenthesis actually give some feeling of comfort, it won’t prevent PHP from combining those argument with any later operators, leading to unexpected results.

Even if most of the time, usage of parenthesis is legit, it is recommended to avoid them.

Short name Structures/PrintWithoutParenthesis
Themes Analyze
Severity Minor
Time To Fix Quick (30 mins)

9.39. Avoid Real

PHP has two float data type : real and double. real is rarely used, and might be deprecated in PHP 7.4.

To prepare code, avoid using is_real() and the (real) typecast.

<?php

// safe way to check for float
if (!is_float($a)) {
    $a = (float) $a;
}

// Avoid doing that
if (!is_real($a)) {
    $a = (real) $a;
}

?>

See also PHP RFC: Deprecations for PHP 7.4.

9.39.1. Suggestions

  • Replace is_real() by is_float()
  • Replace (real) by (float)
Short name Php/AvoidReal
Themes Suggestions, Top10
Severity Minor
Time To Fix Quick (30 mins)

9.40. Avoid Self In Interface

Self and Parent are tricky when used in an interface.

self refers to the current interface or its extended parents : as long as the constant is defined in the interface family, this is valid. On the other hand, when self refers to the current class, the resolution of names will happen at execution time, leading to confusing results.

parent has the same behavior than self, except that it doesn’t accept to be used inside an interface, as it will yield an error. This is one of those error that lint but won’t execute in certain conditions.

Static can’t be used in an interface, as it needs to be resolved at call time anyway.

<?php

interface i extends ii {
    // This 'self' is valid : it refers to the interface i
    public const I = self::I2 + 2;

    // This 'self' is also valid, as it refers to interface ii, which is a part of interface i
    public const I2 = self::IP + 4;

    // This makes interface i dependant on the host class
    public const I3 = parent::A;
}

?>

See also Scope Resolution Operator (::).

9.40.1. Suggestions

  • Use a fully qualified namespace instead of self
  • Use a locally defined constant, so self is a valid reference
Short name Interfaces/AvoidSelfInInterface
Themes ClassReview
Severity Critical
Time To Fix Slow (1 hour)

9.41. Avoid Those Hash Functions

The following cryptographic algorithms are considered unsecure, and should be replaced with new and more performant algorithms.

MD2, MD4, MD5, SHA0, SHA1, CRC, DES, 3DES, RC2, RC4.

When possible, avoid using them, may it be as PHP functions, or hashing function configurations (mcrypt, hash…).

<?php

// Weak cryptographic algorithm
echo md5('The quick brown fox jumped over the lazy dog.');

// Weak crypotgraphic algorthim, used with a modern PHP extension (easier to update)
echo hash('md5', 'The quick brown fox jumped over the lazy dog.');

// Strong crypotgraphic algorthim, used with a modern PHP extension
echo hash('sha156', 'The quick brown fox jumped over the lazy dog.');

?>

Weak cryptography is commonly used for hashing values when caching them. In such cases, security is not a primary concern. However, it may later become such, when hackers get access to the cache folders, or if the cached identifier is published. As a preventive protection, it is recommended to always use a secure hashing function.

See also Secure Hash Algorithms.

Short name Security/AvoidThoseCrypto
Themes Security
Severity Major
Time To Fix Quick (30 mins)

9.42. Avoid Using stdClass

stdClass is the default class for PHP. It is instantiated when PHP needs to return a object, but no class is specifically available.

It is recommended to avoid instantiating this class, nor use it is any way.

<?php

$json = '{a:1,b:2,c:3}';
$object = json_decode($json);
// $object is a stdClass, as returned by json_decode

// Fast building of $o
$a = [];
$a['a'] = 1;
$a['b'] = 2;
$a['c'] = 3;
json_encode( (object) $a);

// Slow building of $o
$o = new stdClass();
$o->a = 1;
$o->b = 2;
$o->c = 3;
json_encode($o);

?>

If you need a stdClass object, it is faster to build it as an array, then cast it, than instantiate stdClass. This is a micro-optimisation.

Short name Php/UseStdclass
Themes Analyze
Severity Minor
Time To Fix Slow (1 hour)

9.43. Avoid array_push()

array_push() is slower than the [] operator.

This is also true if the [] operator is called several times, while array_push() may be called only once. And using count after the push is also faster than collecting array_push() return value.

<?php

$a = [1,2,3];
// Fast version
$a[] = 4;

$a[] = 5;
$a[] = 6;
$a[] = 7;
$count = count($a);

// Slow version
array_push($a, 4);
$count = array_push($a, 5,6,7);

// Multiple version :
$a[] = 1;
$a[] = 2;
$a[] = 3;
array_push($a, 1, 2, 3);


?>

This is a micro-optimisation.

Short name Performances/AvoidArrayPush
Themes Performances
Severity Minor
Time To Fix Instant (5 mins)

9.44. Avoid array_unique()

The native function array_unique() is much slower than using other alternatives, such as array_count_values(), array_flip()/array_keys(), or even a foreach() loops.

<?php

// using array_unique()
$uniques = array_unique($someValues);

// When values are strings or integers
$uniques = array_keys(array_count_values($someValues));
$uniques = array_flip(array_flip($someValues))

//even some loops are faster.
$uniques = [];
foreach($someValues as $s) {
    if (!in_array($uniques, $s)) {
        $uniques[] $s;
    }
}

?>

See also array_unique.

9.44.1. Suggestions

  • Upgrade to PHP 7.2
  • Use an alternative way to make values unique in an array, using array_count_values(), for example.
Short name Structures/NoArrayUnique
Themes Performances
Severity Minor
Time To Fix Quick (30 mins)

9.45. Avoid get_class()

get_class() should be replaced with the instanceof operator to check the class of an object.

get_class() only compares the full namespace name of the object’s class, while instanceof actually resolves the name, using the local namespace and aliases.

<?php

    use Stdclass as baseClass;

    function foo($arg) {
        // Slow and prone to namespace errors
        if (get_class($arg) === 'Stdclass') {
            // doSomething()
        }
    }

    function bar($arg) {
        // Faster, and uses aliases.
        if ($arg instanceof baseClass) {
            // doSomething()
        }
    }
?>

See also get_class and Instanceof.

Short name Structures/UseInstanceof
Themes Analyze, Analyze
Severity Minor
Time To Fix Quick (30 mins)

9.46. Avoid glob() Usage

glob() and scandir() sorts results by default. When that kind of sorting is not needed, save some time by requesting NOSORT with those functions.

Besides, whenever possible, use scandir() instead of glob().

<?php

// Scandir without sorting is the fastest.
scandir('docs/', SCANDIR_SORT_NONE);

// Scandir sorts files by default. Same as above, but with sorting
scandir('docs/');

// glob sorts files by default. Same as below, but no sorting
glob('docs/*', GLOB_NOSORT);

// glob sorts files by default. This is the slowest version
glob('docs/*');

?>

Using opendir() and a while loop may be even faster.

This analysis skips scandir() and glob() if they are expliciely configured with flags (aka, sorting is explicitly needed).

glob() accepts wildchar, such as *, that may not easily replaced with scandir() or opendir().

See also Putting glob to the test.

9.46.1. Suggestions

  • Use FilesystemIterator, DirectoryIterator classes.
  • Use RegexIterator to filter any unwanted results from FilesystemIterator.
Short name Performances/NoGlob
Themes Performances
Severity Major
Time To Fix Quick (30 mins)
Examples Phinx, NextCloud

9.47. Avoid mb_dectect_encoding()

mb_dectect_encoding() is bad at guessing encoding.

For example, UTF-8 and ISO-8859-1 share some common characters : when a string is build with them it is impossible to differentiate the actual encoding.

<?php

$encoding = mb_encoding_detect($_GET['name']);

?>

See also mb_encoding_detect, PHP vs. The Developer: Encoding Character Sets, DPC2019: Of representation and interpretation: A unified theory - Arnout Boks.

9.47.1. Suggestions

  • Store and transmit the data format
Short name Php/AvoidMbDectectEncoding
Themes Analyze
Severity Minor
Time To Fix Quick (30 mins)

9.48. Avoid option arrays in constructors

Avoid option arrays in constructors. Use one parameter per injected element.

<?php

class Foo {
    // Distinct arguments, all typehinted if possible
    function __constructor(A $a, B $b, C $c, D $d) {
        $this->a = $a;
        $this->b = $b;
        $this->c = $c;
        $this->d = $d;
    }
}

class Bar {
    // One argument, spread over several properties
    function __constructor(array $options) {
        $this->a = $options['a'];
        $this->b = $options['b'];
        $this->c = $options['c'];
        $this->d = $options['d'];
    }
}

?>

See also Avoid option arrays in constructors.

9.48.1. Suggestions

  • Spread the options in the argument list, one argument each
  • Use a configuration class, that hold all the elements with clear names, instead of an array
Short name Classes/AvoidOptionArrays
Themes Analyze, ClassReview
Severity Minor
Time To Fix Quick (30 mins)

9.49. Avoid set_error_handler $context Argument

Avoid configuring set_error_handler() with a method that accepts 5 arguments. The last argument, $errcontext, is deprecated since PHP 7.2, and will be removed later.

<?php

// setting error_handler with an incorrect closure
set_error_handler(function($errno, $errstr, $errfile, $errline) {});

// setting error_handler with an incorrect closure
set_error_handler(function($errno, $errstr, $errfile, $errline, $errcontext) {});

?>

See also set_error_handler();

9.49.1. Suggestions

  • Remove the 6th argument of registered handlers.
Short name Php/AvoidSetErrorHandlerContextArg
Themes CompatibilityPHP72
Severity Major
Time To Fix Slow (1 hour)
Examples shopware, Vanilla

9.50. Avoid sleep()/usleep()

sleep() and usleep() help saturate the web server.

Pausing the script for a specific amount of time means that the Web server is also making all related resources sleep, such as database, sockets, session, etc. This may used to set up a DOS on the server.

<?php

$begin = microtime(true);
checkLogin($user, $password);
$end   = microtime(true);

// Making all login checks looks the same
usleep(1000000 - ($end - $begin) * 1000000);

// Any hit on this page now uses 1 second, no matter if load is high or not
// Is it now possible to saturate the webserver in 1 s ?

?>

As much as possible, avoid delaying the end of the script.

sleep() and usleep() have less impact in commandline (CLI).

Short name Security/NoSleep
Themes Security
Severity Minor
Time To Fix Quick (30 mins)

9.51. Bad Constants Names

PHP’s manual recommends that developer do not use constants with the convention __NAME__. Those are reserved for PHP future use.

For example, __TRAIT__ recently appeared in PHP, as a magic constant. In the future, other may appear.

<?php

const __MY_APP_CONST__ = 1;

const __MY_APP_CONST__ = 1;

define('__MY_OTHER_APP_CONST__', 2);

?>

The analyzer will report any constant which name is __.*.__, or even _.*_ (only one underscore).

See also Constants.

9.51.1. Suggestions

  • Avoid using names that doesn’t comply with PHP’s convention
Short name Constants/BadConstantnames
Themes Analyze
Severity Minor
Time To Fix Slow (1 hour)
Examples PrestaShop, Zencart

9.52. Bail Out Early

When using conditions, it is recommended to quit in the current context, and avoid else clause altogether.

The main benefit is to make clear the method applies a condition, and stop immediately when it is not satisfied. The main sequence is then focused on the actual code.

This works with the break, continue, throw and goto keywords too, depending on situations.

<?php

// Bailing out early, low level of indentation
function foo1($a) {
    if ($a > 0) {
        return false;
    }

    $a++;
    return $a;
}

// Works with continue too
foreach($array as $a => $b) {
    if ($a > 0) {
        continue false;
    }

    $a++;
    return $a;
}

// No need for else
function foo2($a) {
    if ($a > 0) {
        return false;
    } else {
        $a++;
    }

    return $a;
}

// No need for else : return goes into then.
function foo3($a) {
    if ($a < 0) {
        $a++;
    } else {
        return false;
    }

    return $a;
}

// Make a return early, and make the condition visible.
function foo3($a) {
    if ($a < 0) {
        $a++;
        methodcall();
        functioncall();
    }
}

?>

See also Avoid nesting too deeply and return early (part 1) and Avoid nesting too deeply and return early (part 2).

9.52.1. Suggestions

  • Detect errors, and then, return as soon as possible.
  • When a if…then branches are unbalanced, test for the small branch, finish it with return. Then keep the other branch as the main code.
Short name Structures/BailOutEarly
Themes Analyze
Severity Minor
Time To Fix Quick (30 mins)
Examples OpenEMR, opencfp

9.53. Binary Glossary

List of all the integer values using the binary format.

<?php

$a = 0b10;
$b = 0B0101;

?>
Short name Type/Binary
Themes CompatibilityPHP53
Php Version With PHP 5.4 and more recent
Severity Major
Time To Fix Quick (30 mins)

9.54. Bracketless Blocks

PHP allows one liners as for(), foreach(), while(), do/while() loops, or as then/else expressions.

It is generally considered a bad practice, as readability is lower and there are non-negligible risk of excluding from the loop the next instruction.

<?php

// Legit one liner
foreach(range('a', 'z') as $letter) ++$letterCount;

// More readable version, even for a one liner.
foreach(range('a', 'z') as $letter) {
    ++$letterCount;
}

?>

switch() cannot be without bracket.

Short name Structures/Bracketless
Themes Coding Conventions
Severity Minor
Time To Fix Instant (5 mins)

9.55. Break Outside Loop

Starting with PHP 7, break or continue that are outside a loop (for, foreach(), do…`while() <http://php.net/manual/en/control-structures.while.php>`_, while()) or a switch() statement won’t compile anymore.

It is not possible anymore to include a piece of code inside a loop that will then break.

<?php

    // outside a loop : This won't compile
    break 1;

    foreach($array as $a) {
        break 1; // Compile OK

        break 2; // This won't compile, as this break is in one loop, and not 2
    }

    foreach($array as $a) {
        foreach($array2 as $a2) {
            break 2; // OK in PHP 5 and 7
        }
    }
?>
Short name Structures/BreakOutsideLoop
Themes Analyze, CompatibilityPHP70
Php Version With PHP 7.0 and older
Severity Major
Time To Fix Slow (1 hour)

9.56. Break With 0

Cannot break 0, as this makes no sense. Break 1 is the minimum, and is the default value.

<?php
    // Can't break 0. Must be 1 or more, depending on the level of nesting.
    for($i = 0; $i < 10; $i++) {
        break 0;
    }

    for($i = 0; $i < 10; $i++) {
        for($j = 0; $j < 10; $j++) {
            break 2;
        }
    }

?>
Short name Structures/Break0
Themes CompatibilityPHP53
Php Version With PHP 5.4 and older
Severity Minor
Time To Fix Quick (30 mins)

9.57. Break With Non Integer

When using a break, the argument of the operator must be a positive non-null integer literal or be omitted.

Other values were acceptable in PHP 5.3 and previous version, but this is now reported as an error.

<?php
    // Can't break $a, even if it contains an integer.
    $a = 1;
    for($i = 0; $i < 10; $i++) {
        break $a;
    }

    // can't break on float
    for($i = 0; $i < 10; $i++) {
        for($j = 0; $j < 10; $j++) {
            break 2.2;
        }
    }

?>
Short name Structures/BreakNonInteger
Themes CompatibilityPHP54
Php Version With PHP 5.4 and older
Severity Minor
Time To Fix Quick (30 mins)

9.58. Buried Assignation

Those assignations are buried in the code, and placed in unexpected situations.

They are difficult to spot, and may be confusing. It is advised to place them in a more visible place.

<?php

// $b may be assigned before processing $a
$a = $c && ($b = 2);

// legit syntax, but the double assignation is not obvious.
for($i = 2, $j = 3; $j < 10; $j++) {

}
?>

9.58.1. Suggestions

  • Extract the assignation and set it on its own line, prior to the current expression.
  • Check if the local variable is necessary
Short name Structures/BuriedAssignation
Themes Analyze
Severity Minor
Time To Fix Slow (1 hour)
Examples XOOPS, Mautic

9.59. Cache Variable Outside Loop

Avoid recalculating constant values inside the loop.

Do the calculation once, outside the loops, and then reuse the value each time.

One of the classic example if doing count($array) in a for loop : since the source is constant during the loop, the result of count() is always the same.

<?php

$path = '/some/path';
$fullpath = realpath("$path/more/dirs/");
foreach($files as $file) {
    // Only moving parts are used in the loop
    copy($file, $fullpath.$file);
}

$path = '/some/path';
foreach($files as $file) {
    // $fullpath is calculated each loop
    $fullpath = realpath("$path/more/dirs/");
    copy($file, $fullpath.$file);
}

?>

Depending on the load of the called method, this may increase the speed of the loop from little to enormously.

Short name Performances/CacheVariableOutsideLoop
Themes Performances

9.60. Callback Needs Return

When used with array_map() functions, the callback must return something. This return may be in the form of a return statement, a global variable or a parameter with a reference. All those solutions extract information from the callback.

<?php

// This filters each element
$filtered = array_filter($array, function ($x) {return $x == 2; });

// This return void for every element
$filtered = array_filter($array, function ($x) {return ; });

// costly array_sum()
$sum = 0;
$filtered = array_filter($array, function ($x) use (&$sum) {$sum += $x; });

// costly array_sum()
global $sum = 0;
$filtered = array_filter($array, function () {global $sum; $sum += $x; });

?>

See also array_map.

9.60.1. Suggestions

  • Add an explicit return to the callback
  • Use null to unset elements in an array without destroying the index
Short name Functions/CallbackNeedsReturn
Themes Analyze
Severity Major
Time To Fix Instant (5 mins)
Examples Contao, Phpdocumentor

9.61. Calltime Pass By Reference

PHP doesn’t allow when a value is turned into a reference at functioncall, since PHP 5.4.

Either the function use a reference in its signature, either the reference won’t pass.

<?php

function foo($name) {
    $arg = ucfirst(strtolower($name));
    echo 'Hello '.$arg;
}

$a = 'name';
foo(&$a);

?>
Short name Structures/CalltimePassByReference
Themes CompatibilityPHP54
Php Version With PHP 5.4 and older
Severity Minor
Time To Fix Quick (30 mins)

9.62. Can’t Count Non-Countable

Count() emits an error when it tries to count scalars or objects what don’t implement Countable interface.

<?php

// Normal usage
$a = array(1,2,3,4);
echo count($a).items\n;

// Error emiting usage
$a = '1234';
echo count($a).chars\n;

// Error emiting usage
echo count($unsetVar).elements\n;

?>

See also Warn when counting non-countable types.

Short name Structures/CanCountNonCountable
Themes CompatibilityPHP72
Severity Major
Time To Fix Quick (30 mins)

9.63. Can’t Extend Final

It is not possible to extend final classes.

Since PHP fails with a fatal error, this means that the extending class is probably not used in the rest of the code. Check for dead code.

<?php
    // File Foo
    final class foo {
        public final function bar() {
            // doSomething
        }
    }
?>

In a separate file :

<?php
    // File Bar
    class bar extends foo {

    }
?>

See also Final Keyword.

9.63.1. Suggestions

  • Remove the final keyword
  • Remove the extending class
Short name Classes/CantExtendFinal
Themes Analyze, Dead code
Severity Critical
Time To Fix Instant (5 mins)

9.64. Can’t Throw Throwable

Classes extending Throwable can’t be thrown. The same applies to interfaces.

Although this code lints, PHP throws a Fatal error when executing or including it : Class fooThrowable cannot implement interface `Throwable <http://www.php.net/manual/en/class.throwable.php>`_, extend Exception or Error instead.

<?php

// This is the way to go
class fooException extends \Exception { }

// This is not possible and a lot of work
class fooThrowable implements \throwable { }

?>

See also Throwable, Exception and Error.

Short name Exceptions/CantThrow
Themes Analyze, LintButWontExec
Severity Minor
Time To Fix Slow (1 hour)

9.65. Cant Inherit Abstract Method

Inheriting abstract methods was made available in PHP 7.2. In previous versions, it emitted a fatal error.

<?php

abstract class A           { abstract function bar(stdClass $x);  }
abstract class B extends A { abstract function bar($x): stdClass; }

//   Fatal error: Can't inherit abstract function A::bar()
?>

See also PHP RFC: Allow abstract function override.

Short name Classes/CantInheritAbstractMethod
Themes CompatibilityPHP53, CompatibilityPHP70, CompatibilityPHP71, CompatibilityPHP54, CompatibilityPHP55, CompatibilityPHP56
Php Version With PHP 7.2 and more recent
Severity Critical
Time To Fix Quick (30 mins)

9.66. Cant Instantiate Class

When constructor is not public, it is not possible to instantiate such a class. Either this is a conception choice, or there are factories to handle that. Either way, it is not possible to call new on such class.

PHP reports an error similar to this one : ‘Call to private Y::__construct() from invalid context’.

<?php

//This is the way to go
$x = X::factory();

//This is not possible
$x = new X();

class X {
    //This is also the case with proctected __construct
    private function __construct() {}

    static public function factory() {
        return new X();
    }
}

?>

See also In a PHP5 class, when does a private constructor get called?, Named Constructors in PHP and PHP Constructor Best Practices And The Prototype Pattern.

Short name Classes/CantInstantiateClass
Themes Analyze
Severity Critical
Time To Fix Quick (30 mins)
Examples WordPress

9.67. Cant Use Return Value In Write Context

empty() used to work only on data containers, such as variables. Until PHP 5.5, it was not possible to use directly expressions, such as functioncalls, inside an empty() function call : they were met with a ‘Can’t use function return value in write context’ fatal error.

<?php

function foo($boolean) {
    return $boolean;
}

// Valid since PHP 5.5
echo empty(foo(true)) : 'true' : 'false';

?>

This also applies to methodcalls, static or not.

See also Cant Use Return Value In Write Context.

Short name Php/CantUseReturnValueInWriteContext
Themes CompatibilityPHP53, CompatibilityPHP54
Php Version With PHP 5.5 and more recent
Severity Major
Time To Fix Quick (30 mins)

9.68. Case Insensitive Constants

PHP constants may be case insensitive, when defined with define() and the third argument.

This feature is deprecated since PHP 7.3 and will be removed in PHP 8.0.

<?php

// case sensitive
define('A', 1);

// case insensitive
define('B', 1, true);

echo A;
// This is not possible
//echo a;

// both possible
echo B;
echo b;

?>

See also define.

Short name Constants/CaseInsensitiveConstants
Themes CompatibilityPHP73
Severity Critical
Time To Fix Slow (1 hour)

9.69. Cast To Boolean

This expression may be reduced by casting to boolean type.

<?php

$variable = $condition == 'met' ? 1 : 0;
// Same as
$variable = (bool) $condition == 'met';

$variable = $condition == 'met' ? 0 : 1;
// Same as (Note the condition inversion)
$variable = (bool) $condition != 'met';
// also, with an indentical condition
$variable = !(bool) $condition == 'met';

// This also works with straight booleans expressions
$variable = $condition == 'met' ? true : false;
// Same as
$variable = $condition == 'met';

?>

9.69.1. Suggestions

  • Remove the old expression and use (bool) operator instead
  • Change the target values from true/false, or 0/1 to non-binary values, like strings or integers beyond 0 and 1.
  • Complete the current branches with other commands
Short name Structures/CastToBoolean
Themes Analyze
Severity Minor
Time To Fix Instant (5 mins)
Examples MediaWiki, Dolibarr

9.70. Casting Ternary

Type casting has a precedence over ternary operator, and is applied first. When this happens, the condition is cast, although it is often useless as PHP will do it if needed.

This applies to the ternary operator, the coalesce operator ?: and the null-coalesce operator ??.

<?php
    $a = (string) $b ? 3 : 4;
    $a = (string) $b ?: 4;
    $a = (string) $b ?? 4;
?>

The last example generates first an error Undefined variable: b, since $b is first cast to a string. The result is then an empty string, which leads to an empty string to be stored into $a. Multiple errors cascade.

See also Operators Precedence.

9.70.1. Suggestions

  • Add parenthesis around the ternary operator
  • Skip the casting
  • Cast in another expression
Short name Structures/CastingTernary
Themes Analyze
Severity Major
Time To Fix Quick (30 mins)

9.71. Catch Overwrite Variable

The try/catch structure uses some variables that are also in use in this scope. In case of a caught exception, the exception will be put in the catch variable, and overwrite the current value, loosing some data.

<?php

// variables and caught exceptions are distinct
$argument = 1;
try {
    methodThatMayRaiseException($argument);
} (Exception $e) {
    // here, $e has been changed to an exception.
}

// variables and caught exceptions are overlapping
$e = 1;
try {
    methodThatMayRaiseException();
} (Exception $e) {
    // here, $e has been changed to an exception.
}

?>

It is recommended to use another name for these catch variables.

9.71.1. Suggestions

  • Use a standard : only use $e (or else) to catch exceptions. Avoid using them for anything else, parameter, property or local variable.
  • Change the variable, and keep the caught exception
Short name Structures/CatchShadowsVariable
Themes Analyze
Severity Minor
Time To Fix Instant (5 mins)
ClearPHP no-catch-overwrite
Examples PhpIPAM, SuiteCrm

9.72. Check All Types

When checking for time, avoid using else. Mention explicitly all tested type, and raise an exception when reaching else.

PHP has a short list of scalar types : null, boolean, integer, real, strings, object, resource and array. When a variable is not holding one the the type, then it may be of any other type.

Most of the time, when using a simple is_string() / else test, this is relying on the conception of the code. By construction, the arguments may be one of two types : array or string.

What happens often is that in case of failure in the code (database not working, another class not checking its results), a third type is pushed to the structure, and it ends up breaking the execution.

The safe way is to check the various types all the time, and use the default case (here, the else) to throw exception() or test an assertion and handle the special case.

<?php

// hasty version
if (is_array($argument)) {
    $out = $argument;
} else {
    // Here, $argument is NOT an array. What if it is an object ? or a NULL ?
    $out = array($argument);
}

// Safe type checking : do not assume that 'not an array' means that it is the other expected type.
if (is_array($argument)) {
    $out = $argument;
} elseif (is_string($argument)) {
    $out = array($argument);
} else {
    assert(false, '$argument is not an array nor a string, as expected!');
}

?>

Using is_callable(), is_iterable() with this structure is fine : when variable is callable or not, while a variable is an integer or else.

Using a type test without else is also accepted here. This is a special treatment for this test, and all others are ignored. This aspect may vary depending on situations and projects.

9.72.1. Suggestions

  • Include a default case to handle all unknown situations
  • Include and process explicit types as much as possible
Short name Structures/CheckAllTypes
Themes Analyze
Severity Major
Time To Fix Quick (30 mins)
Examples Zend-Config, Vanilla

9.73. Check JSON

Check errors whenever JSON is encoded or decoded.

In particular, NULL is a valid decoded JSON response. If you want to avoid mistaking NULL for an error, it is recommended to call json_last_error.

<?php

$encoded = json_encode($incoming);
// Unless JSON must contains some non-null data, this mistakes NULL and error
if(json_last_error() != JSON_ERROR_NONE) {
    die('Error when encoding JSON');
}

$decoded = json_decode($incoming);
// Unless JSON must contains some non-null data, this mistakes NULL and error
if($decoded === null) {
    die('ERROR');
}

?>

See also Option to make json_encode and json_decode throw exceptions on errors, json_last_error.

Short name Structures/CheckJson
Themes Analyze
Severity Major
Time To Fix Quick (30 mins)

9.74. Check On __Call Usage

When using the magic methods __call() and __staticcall(), make sure the method exists before calling it.

If the method doesn’t exists, then the same method will be called again, leading to the same failure. Finally, it will crash PHP.

<?php

class safeCall {
    function __class($name, $args) {
        // unsafe call, no checks
        if (method_exists($this, $name)) {
            $this->$name(...$args);
        }
    }
}

class unsafeCall {
    function __class($name, $args) {
        // unsafe call, no checks
        $this->$name(...$args);
    }
}

?>

See also Method overloading and ``Magical PHP: __call <https://www.garfieldtech.com/index.php/blog/magical-php-call>`_.

9.74.1. Suggestions

  • Add a call to method_exists() before using any method name
  • Relay the call to another object that doesn’t handle __call() or __callStatic()
Short name Classes/CheckOnCallUsage
Themes Analyze
Severity Minor
Time To Fix Quick (30 mins)

9.75. Child Class Removes Typehint

PHP 7.2 introduced the ability to remove a typehint when overloading a method. This is not valid code for older versions.

<?php

class foo {
    function foobar(foo $a) {}
}

class bar extends foo {
    function foobar($a) {}
}

?>
Short name Classes/ChildRemoveTypehint
Themes CompatibilityPHP53, CompatibilityPHP70, CompatibilityPHP71, CompatibilityPHP54, CompatibilityPHP55, CompatibilityPHP56
Php Version With PHP 7.2 and more recent
Severity Major
Time To Fix Quick (30 mins)

9.76. Class Const With Array

Constant defined with const keyword may be arrays but only stating with PHP 5.6. Define never accept arrays : it only accepts scalar values.

Short name Php/ClassConstWithArray
Themes CompatibilityPHP53, CompatibilityPHP54, CompatibilityPHP55
Php Version With PHP 5.5 and more recent
Severity Critical
Time To Fix Slow (1 hour)

9.77. Class Could Be Final

Any class that has no extension should be final by default.

As stated by Matthias Noback : If a class is not marked final, it has at least one subclass.

Prevent your classes from being subclassed by making them final. Sometimes, classes are not meant or thought to be derivable.

<?php

class x {}            // This class is extended
class y extends x {}  // This class is extended
class z extends y {}  // This class is not extended

final class z2 extends y {}  // This class is not extended

?>

See also Negative architecture, and assumptions about code.

9.77.1. Suggestions

  • Make the class final
  • Extends the class
Short name Classes/CouldBeFinal
Themes Analyze, ClassReview
Severity Minor
Time To Fix Quick (30 mins)

9.78. Class Function Confusion

Avoid classes and functions bearing the same name.

When functions and classes bear the same name, calling them may be confusing. This may also lead to forgotten ‘new’ keyword.

<?php

class foo {}

function foo() {}

// Forgetting the 'new' operator is easy
$object = new foo();
$object = foo();

?>
Short name Php/ClassFunctionConfusion
Themes Analyze
Severity Minor
Time To Fix Slow (1 hour)

9.79. Class Should Be Final By Ocramius

‘Make your classes always final, if they implement an interface, and no other public methods are defined’.

When a class should be final, as explained by Ocramius (Marco Pivetta).

<?php

interface i1 {
    function i1() ;
}

// Class should final, as its public methods are in an interface
class finalClass implements i1 {
    // public interface
    function i1 () {}

    // private method
    private function a1 () {}
}

?>

See also When to declare classes final.

Short name Classes/FinalByOcramius
Themes Analyze
Severity Minor
Time To Fix Slow (1 hour)

9.80. Class, Interface Or Trait With Identical Names

The following names are used at the same time for classes, interfaces or traits. For example,

<?php
    class a     { /* some definitions */ }
    interface a { /* some definitions */ }
    trait a     { /* some definitions */ }
?>

Even if they are in different namespaces, identical names makes classes easy to confuse. This is often solved by using alias at import time : this leads to more confusion, as a class suddenly changes its name.

Internally, PHP use the same list for all classes, interfaces and traits. As such, it is not allowed to have both a trait and a class with the same name.

In PHP 4, and PHP 5 before namespaces, it was not possible to have classes with the same name. They were simply included after a check.

9.80.1. Suggestions

  • Use distinct names for every class, trait and interface.
  • Keep eponymous classes, traits and interfaces in distinct files, for definition but also for usage. When this happens, rename one of them.
Short name Classes/CitSameName
Themes Analyze
Severity Minor
Time To Fix Quick (30 mins)
Examples shopware, NextCloud

9.81. Classes Mutually Extending Each Other

Those classes are extending each other, creating an extension loop. PHP will yield a fatal error at running time, even if it is compiling the code.

<?php

// This code is lintable but won't run
class Foo extends Bar { }
class Bar extends Foo { }

// The loop may be quite large
class Foo extends Bar { }
class Bar extends Bar2 { }
class Bar2 extends Foo { }

?>
Short name Classes/MutualExtension
Themes Analyze, LintButWontExec
Severity Major
Time To Fix Quick (30 mins)

9.82. Clone With Non-Object

The clone keyword must be used on variables, properties or results from a function or method call.

clone cannot be used with constants or literals.

<?php

class x { }
$x = new x();

// Valid clone
$y = clone $x;

// Invalid clone
$y = clone x;

?>

Cloning a non-object lint but won’t execute.

See also Object cloning.

9.82.1. Suggestions

  • Only clone containers (like variables, properties…)
Short name Classes/CloneWithNonObject
Themes Analyze, LintButWontExec
Severity Minor
Time To Fix Quick (30 mins)

9.83. Close Tags

PHP manual recommends that script should be left open, without the final closing ?>. This way, one will avoid the infamous bug ‘Header already sent’, associated with left-over spaces, that are lying after this closing tag.

Short name Php/CloseTags
Themes Coding Conventions
Severity Minor
Time To Fix Instant (5 mins)
ClearPHP leave-last-closing-out

9.84. Closure Could Be A Callback

Closure could be simplified to a callback. Callbacks are strings or arrays.

A simple closure that only returns arguments relayed to another function or method, could be reduced to a simpler expression. They

Closure may be simplified with a string, for functioncall, with an array for methodcalls and static methodcalls.

Performances : simplifying a closure tends to reduce the call time by 50%.

<?php

// Simple and faster call to strtoupper
$filtered = array_map('strtoupper', $array);

// Here the closure doesn't add any feature over strtoupper
$filtered = array_map(function ($x) { return strtoupper($x);}, $array);

// Methodcall example : no fix
$filtered = array_map(function ($x) { return $x->strtoupper() ;}, $array);

// Methodcall example  : replace with array($y, 'strtoupper')
$filtered = array_map(function ($x) use ($y) { return $y->strtoupper($x) ;}, $array);

// Static methodcall example
$filtered = array_map(function ($x) { return $x::strtoupper() ;}, $array);

// Static methodcall example   : replace with array('A', 'strtoupper')
$filtered = array_map(function ($x) { return A::strtoupper($x) ;}, $array);

?>

See also Closure class and Callbacks / Callables.

9.84.1. Suggestions

  • Replace the closure by a string, with the name of the called function
Short name Functions/Closure2String
Themes Suggestions, Performances
Severity Minor
Time To Fix Quick (30 mins)
Examples Tine20, NextCloud

9.85. Closure May Use $this

$this is automatically accessible to closures.

When closures were introduced in PHP, they couldn’t use the $this variable, making is cumbersome to access local properties when the closure was created within an object.

<?php

// Invalid code in PHP 5.4 and less
class Test
{
    public function testing()
    {
        return function() {
            var_dump($this);
        };
    }
}

$object = new Test;
$function = $object->testing();
$function();

?>

This is not the case anymore since PHP 5.4.

See also Anonymous functions.

Short name Php/ClosureThisSupport
Themes CompatibilityPHP53
Php Version With PHP 5.4 and older
Severity Minor
Time To Fix Quick (30 mins)

9.86. Common Alternatives

In the following conditional structures, expressions were found that are common to both ‘then’ and ‘else’. It may be interesting, though not always possible, to put them both out of the conditional, and reduce line count.

<?php
if ($c == 5) {
    $b = strtolower($b[2]);
    $a++;
} else {
    $b = strtolower($b[2]);
    $b++;
}
?>

may be rewritten in :

<?php

$b = strtolower($b[2]);
if ($c == 5) {
    $a++;
} else {
    $b++;
}

?>

9.86.1. Suggestions

  • Collect common expressions, and move them before of after the if/then expression.
  • Move a prefix and suffixes to a third-party method
Short name Structures/CommonAlternatives
Themes Analyze
Severity Major
Time To Fix Instant (5 mins)
Examples Dolibarr, NextCloud

9.87. Compact Inexistant Variable

Compact() doesn’t warn when it tries to work on an inexistant variable. It just ignores the variable.

This behavior changed in PHP 7.3, and compact() now emits a warning when the compacted variable doesn’t exist.

<?php

function foo($b = 2) {
    $a = 1;
    // $c doesn't exists, and is not compacted.
    return compact('a', 'b', 'c');
}
?>

For performances reasons, this analysis only works inside methods and functions.

See also compact and PHP RFC: Make compact function reports undefined passed variables.

Short name Php/CompactInexistant
Themes Suggestions, CompatibilityPHP73
Severity Major
Time To Fix Quick (30 mins)

9.88. Compare Hash

When comparing hash values, it is important to use the strict comparison : hash_equals(), === or !==.

In a number of situations, the hash value will start with 0e, and PHP will understand that the comparison involves integers : it will then convert the strings into numbers, and it may end up converting them to 0.

Here is an example

<?php

// The two following passwords hashes matches, while they are not the same.
$hashed_password = 0e462097431906509000000000000;
if (hash('md5','240610708',false) == $hashed_password) {
  print 'Matched.'.PHP_EOL;
}

// hash returns a string, that is mistaken with 0 by PHP
// The strength of the hashing algorithm is not a problem
if (hash('ripemd160','20583002034',false) == '0') {
  print 'Matched.'.PHP_EOL;
}

if (hash('md5','240610708',false) !== $hashed_password) {
  print 'NOT Matched.'.PHP_EOL;
}

// Display true
var_dump(md5('240610708') == md5('QNKCDZO') );

?>

You may also use password_hash() and password_verify() : they work together without integer conversion problems, and they can’t be confused with a number.

See also Magic Hashes What is the best way to compare hashed strings? (PHP) and md5(‘240610708’) == md5(‘QNKCDZO’).

Short name Security/CompareHash
Themes Security
Severity Major
Time To Fix Quick (30 mins)
ClearPHP strict-comparisons
Examples Traq, LiveZilla

9.89. Compared Comparison

Usually, comparison are sufficient, and it is rare to have to compare the result of comparison. Check if this two-stage comparison is really needed.

<?php

if ($a === strpos($string, $needle) > 2) {}

// the expression above apply precedence :
// it is equivalent to :
if (($a === strpos($string, $needle)) > 2) {}

?>

See also Operators Precedence.

Short name Structures/ComparedComparison
Themes Analyze
Severity Major
Time To Fix Quick (30 mins)

9.90. Complex Dynamic Names

Avoid using expressions as names for variables or methods.

There are no place for checks or flow control, leading to any rogue value to be used as is. Besides, the expression is often overlooke, and not expected there : this makes the code less readable.

It is recommended to buildd the name in a separate variable, apply the usual checks for existence and validity, and then use the name.

<?php

$a = new foo();

// Code is more readable
$name = strolower($string);
if (!property_exists($a, $name)) {
    throw new missingPropertyexception($name);
}
echo $a->$name;

// This is not check
echo $a->{strtolower($string)};

?>

This analysis only accept simple containers, such as variables, properties.

See also Dynamically Access PHP Object Properties with `$this <https://drupalize.me/blog/201508/dynamically-access-php-object-properties>`_.

9.90.1. Suggestions

  • Extract the expression from the variable syntax, and make it a separate variable.
Short name Variables/ComplexDynamicNames
Themes Suggestions
Severity Minor
Time To Fix Quick (30 mins)

9.91. Concat And Addition

Precedence between addition and concatenation has changed. In PHP 7.4, addition has precedence, and before, addition and concatenation had the same precedence.

From the RFC : Currently the precedence of '.', '+' and '-' operators are equal. Any combination of these operators are simply evaluated left-to-right.

This is counter-intuitive though: you rarely want to add or subtract concatenated strings which in general are not numbers. However, given PHPs capability of seamlessly converting an integer to a string, concatenation of these values is desired.``

<?php
// Extracted from the RFC
echo sum: . $a + $b;

// current behavior: evaluated left-to-right
echo (sum: . $a) + $b;

// desired behavior: addition and subtraction have a higher precendence
echo sum : . ($a + $b);

?>

This analysis reports any addition and concatenation that are mixed, without parenthesis. Addition also means substraction here, aka using + or -.

See also Change the precedence of the concatenation operator.

9.91.1. Suggestions

  • Add parenthesis around the addition to ensure its expected priority
  • Move the addition outside the concatenation
Short name Php/ConcatAndAddition
Themes Analyze, CompatibilityPHP53, CompatibilityPHP70, CompatibilityPHP71, CompatibilityPHP72, CompatibilityPHP73, CompatibilityPHP54, CompatibilityPHP74, CompatibilityPHP80, CompatibilityPHP55, CompatibilityPHP56, Top10
Severity Minor
Time To Fix Quick (30 mins)

9.92. Concat Empty String

Using a concatenation to make a value a string should be replaced with a type cast.

Type cast to a string is done with (string) operator. There is also the function strval(), although it is less recommended.

<?php

$a = 3;

// explicite way to cast a value
$b = (string) $a; // $b is a string with the content 3

// Wrong way to cast a value
$c = $a . ''; // $c is a string with the content 3
$c = '' . $a; // $c is a string with the content 3
$a .= '';     // $a is a string with the content 3

// Wrong way to cast a value
$c = $a . '' . $b; // This is not reported. The empty string is useless, but not meant to type cast

?>

See also Type Casting and PHP Type Casting.

9.92.1. Suggestions

  • Avoir concatenating with empty strings
  • Use (string) operator to cast to string
  • Remove any concatenated empty string
Short name Structures/ConcatEmpty
Themes Analyze
Severity Minor
Time To Fix Quick (30 mins)

9.93. Concrete Visibility

Methods that implements an interface in a class must be public.

PHP does lint this, unless the interface and the class are in the same file. At execution, it stops immediately with a Fatal error : ‘Access level to c::iPrivate() must be public (as in class i) ‘;

<?php

interface i {
    function iPrivate() ;
    function iProtected() ;
    function iPublic() ;
}

class c implements i {
    // Methods that implements an interface in a class must be public.
    private function iPrivate() {}
    protected function iProtected() {}
    public function iPublic() {}
}

?>

See also Interfaces.

9.93.1. Suggestions

  • Always set interface methods to public.
Short name Interfaces/ConcreteVisibility
Themes Analyze, LintButWontExec
Severity Major
Time To Fix Instant (5 mins)

9.94. Configure Extract

The extract() function overwrites local variables when left unconfigured.

Extract imports variables from an array into the local scope. In case of a conflict, that is when a local variable already exists, it overwrites the previous variable.

In fact, extract() may be configured to handle the situation differently : it may skip the conflicting variable, prefix it, prefix it only if it exists, only import overwriting variables… It may also import them as references to the original values.

This analysis reports extract() when it is not configured explicitly. If overwriting is the intended objective, it is not reported.

<?php

// ignore overwriting variables
extract($array, EXTR_SKIP);

// prefix all variables explicitly variables with 'php_'
extract($array, EXTR_PREFIX_ALL, 'php_');

// overwrites explicitly variables
extract($array, EXTR_OVERWRITE);

// overwrites implicitely variables : do we really want that?
extract($array, EXTR_OVERWRITE);

?>

Always avoid using extract() on untrusted sources, such as $_GET, $_POST, $_FILES, or even databases records.

See also extract.

9.94.1. Suggestions

  • Always use the second argument of extract(), and avoid using EXTR_OVERWRITE
Short name Security/ConfigureExtract
Themes Security
Severity Minor
Time To Fix Instant (5 mins)
Examples Zurmo, Dolibarr

9.95. Const Visibility Usage

Visibility for class constant controls the accessibility to class constant.

A public constant may be used anywhere in the code; a protected constant usage is restricted to the class and its relatives; a private constant is restricted to itself.

This feature was introduced in PHP 7.1. It is recommended to use explicit visibility, and, whenever possible, make the visibility private.

<?php

class x {
    public const a = 1;
    protected const b = 2;
    private const c = 3;
    const d = 4;
}

interface i {
    public const a = 1;
      const d = 4;
}

?>

See also Class Constants and PHP RFC: Support Class Constant Visibility.

Short name Classes/ConstVisibilityUsage
Themes CompatibilityPHP53, CompatibilityPHP70, CompatibilityPHP54, CompatibilityPHP55, CompatibilityPHP56
Php Version With PHP 7.1 and more recent
Severity Minor
Time To Fix Slow (1 hour)

9.96. Const With Array

The const keyword supports array. This feature was added in PHP 5.6.

The array must be filled with other constants. It may also be build using the ‘+’ operator.

<?php

const PRIMES = [2, 3, 5, 7];

class X {
    const TWENTY_THREE = 23;
    const MORE_PRIMES = PRIMES + [11, 13, 17, 19];
    const EVEN_MORE_PRIMES = self::MORE_PRIMES + [self::TWENTY_THREE];
}

?>

See also Class Constants and Constants Syntax.

Short name Php/ConstWithArray
Themes CompatibilityPHP53, CompatibilityPHP54, CompatibilityPHP55
Php Version With PHP 5.5 and more recent
Severity Major
Time To Fix Slow (1 hour)

9.97. Constant Class

A class or an interface only made up of constants. Constants usually have to be used in conjunction of some behavior (methods, class…) and never alone.

<?php

class ConstantClass {
    const KBIT = 1000;
    const MBIT = self::KBIT * 1000;
    const GBIT = self::MBIT * 1000;
    const PBIT = self::GBIT * 1000;
}

?>

As such, they should be PHP constants (build with define or const), or included in a class with other methods and properties.

See also PHP Classes containing only constants.

9.97.1. Suggestions

  • Make the class an interface
  • Make the class an abstract class, to avoid its instantiation
Short name Classes/ConstantClass
Themes Analyze
Severity Minor
Time To Fix Slow (1 hour)

9.98. Constant Comparison

Constant to the left or right is a favorite.

Comparisons are commutative : they may be $a == B or B == $a. The analyzed code show less than 10% of one of the two : for consistency reasons, it is recommended to make them all the same.

Putting the constant on the left is also called ‘Yoda Comparison’, as it mimics the famous characters style of speech. It prevents errors like ‘B = $a’ where the comparison is turned into an assignation.

The natural way is to put the constant on the right. It is often less surprising.

Every comparison operator is used when finding the favorite.

<?php

//
if ($a === B) { doSomething(); }
if ($c > D) { doSomething(); }
if ($e !== G) { doSomething(); }
do { doSomething(); } while ($f === B);
while ($a === B) { doSomething(); }

// be consistent
if (B === $a) {}

// Compari
if (B <= $a) {}

?>
Short name Structures/ConstantComparisonConsistance
Themes Coding Conventions
Severity Minor
Time To Fix Quick (30 mins)

9.99. Constant Scalar Expressions

Define constant with the result of static expressions. This means that constants may be defined with the const keyword, with the help of various operators but without any functioncalls.

This feature was introduced in PHP 5.6. It also supports array(), and expressions in arrays.

Those expressions (using simple operators) may only manipulate other constants, and all values must be known at compile time.

<?php

// simple definition
const A = 1;

// constant scalar expression
const B = A * 3;

// constant scalar expression
const C = [A ** 3, '3' => B];

?>

See also Constant Scalar Expressions.

Short name Structures/ConstantScalarExpression
Themes CompatibilityPHP53, CompatibilityPHP54, CompatibilityPHP55
Php Version With PHP 5.6 and more recent
Severity Major
Time To Fix Quick (30 mins)

9.100. Constants Created Outside Its Namespace

Constants Created Outside Its Namespace.

Using the define() function, it is possible to create constant outside their namespace, but using the fully qualified namespace.

<?php

namespace A\B {
    // define A\B\C as 1
    define('C', 1);
}

namespace D\E {
    // define A\B\C as 1, while outside the A\B namespace
    define('A\B\C', 1);
}

?>

However, this makes the code confusing and difficult to debug. It is recommended to move the constant definition to its namespace.

Short name Constants/CreatedOutsideItsNamespace
Themes Analyze
Severity Minor
Time To Fix Slow (1 hour)

9.101. Constants With Strange Names

List of constants being defined with names that are incompatible with PHP standards.

<?php

// Define a valid PHP constant
define('ABC', 1);
const ABCD = 2;

// Define an invalid PHP constant
define('ABC!', 1);
echo defined('ABC!') ? constant('ABC!') : 'Undefined';

// Const doesn't allow illegal names

?>

See also PHP Constants.

Short name Constants/ConstantStrangeNames
Themes Analyze
Severity Minor
Time To Fix Slow (1 hour)

9.102. Continue Is For Loop

break and continue are very similar in PHP : they both break out of loop or switch. Yet, continue should be reserved for loops.

Since PHP 7.3, the execution will emit a warning when finding a continue inside a switch inside a loop : ‘”continue” targeting switch is equivalent to “break”. Did you mean to use “continue 2”?’

<?php

while ($foo) {
    switch ($bar) {
        case 'baz':
            continue; // In PHP: Behaves like 'break;'
                      // In C:   Behaves like 'continue 2;'
    }
}

?>

See also Deprecate and remove `continue targeting switch <https://wiki.php.net/rfc/continue_on_switch_deprecation>`_.

9.102.1. Suggestions

  • Replace break by continue
Short name Structures/ContinueIsForLoop
Themes Analyze, CompatibilityPHP53, CompatibilityPHP70, CompatibilityPHP71, CompatibilityPHP72, CompatibilityPHP54, CompatibilityPHP55, CompatibilityPHP56, CompatibilityPHP73
Severity Minor
Time To Fix Quick (30 mins)
Examples Xoops

9.103. Could Be Abstract Class

An abstract class is never instantiated, and has children class that are. As such, a ‘parent’ class that is never instantiated by itself, but has its own children instantiated could be marked as abstract.

That will prevent new code to try to instantiate it.

<?php

// Example code would actually be split over multiple files.


// That class could be abstract
class motherClass {}

// Those classes shouldn't be abstract
class firstChildren extends motherClass {}
class secondChildren extends motherClass {}
class thirdChildren extends motherClass {}

new firstChildren();
new secondChildren();
new thirdChildren();

//Not a single : new motherClass()

?>

See also Class Abstraction Abstract classes and methods.

9.103.1. Suggestions

  • Make this class an abstract class
Short name Classes/CouldBeAbstractClass
Themes Analyze, ClassReview
Severity Minor
Time To Fix Quick (30 mins)
Examples Edusoho, shopware

9.104. Could Be Class Constant

When a property is defined and read, but never modified, it may be a constant.

<?php

class foo {
    // $this->bar is never modified.
    private $bar = 1;

    // $this->foofoo is modified, at least once
    private $foofoo = 2;

    function method($a) {
        $this->foofoo = $this->bar + $a + $this->foofoo;

        return $this->foofoo;
    }

}

?>

Starting with PHP 5.6, even array() may be defined as constants.

Short name Classes/CouldBeClassConstant
Themes ClassReview
Severity Minor
Time To Fix Quick (30 mins)

9.105. Could Be Constant

Literals may be replaced by an existing constant.

Constants makes the code easier to read, as they may bear a meaningful name. They also hide implementation values, with a readable name, such as const READABLE= true;. Later, upgrading constant values is easier than scouring the code with a new literal.

Not all literal can be replaced by a constant values : sometimes, literal may have the same literal value, but different meanings. Check with your application semantics before changing any literal with a constant.

<?php

const A = 'abc';
define('B', 'ab');

class foo {
    const X = 'abcd';
}

// Could be replaced by B;
$a = 'ab';

// Could be replaced by A;
$a = 'abc';

// Could be replaced by foo::X;
$a = 'abcd';

?>

This analysis currently doesn’t support arrays.

This analysis also skips very common values, such as boolean, 0 and 1. This prevents too many false positive.

9.105.1. Suggestions

  • Turn the literal into an existing constant
Short name Constants/CouldBeConstant
Themes Suggestions
Severity Minor
Time To Fix Quick (30 mins)

9.106. Could Be Else

Merge opposition conditions into one if/then structure.

When two if/then structures follow each other, using a condition and its opposite, they may be merged into one.

<?php

// Short version
if ($a == 1) {
    $b = 2;
} else {
    $b = 1;
}

// Long version
if ($a == 1) {
    $b = 2;
}

if ($a != 1) {
    $b = 3;
}

?>

9.106.1. Suggestions

  • Merge the two conditions into one structure
  • Check if the second condition is still applicable
Short name Structures/CouldBeElse
Themes Analyze
Severity Minor
Time To Fix Instant (5 mins)
Examples SugarCrm, OpenEMR

9.107. Could Be Private Class Constant

Class constant may use private visibility.

Since PHP 7.1, constants may also have a public/protected/private visibility. This restrict their usage to anywhere, class and children or class.

As a general rule, it is recommended to make constant private by default, and to relax this restriction as needed. PHP makes them public by default.

<?php

class foo {
    // pre-7.1 style
    const PRE_71_CONSTANT = 1;

    // post-7.1 style
    private const PRIVATE_CONSTANT = 2;
    public const PUBLIC_CONSTANT = 3;

    function bar() {
        // PRIVATE CONSTANT may only be used in its class
        echo self::PRIVATE_CONSTANT;
    }
}

// Other constants may be used anywhere
function x($a = foo::PUBLIC_CONSTANT) {
    echo $a.' '.foo:PRE_71_CONSTANT;
}

?>

Constant shall stay public when the code has to be compatible with PHP 7.0 and older.

They also have to be public in the case of component : some of those constants have to be used by external actors, in order to configure the component.

See also Class Constants.

Short name Classes/CouldBePrivateConstante
Themes ClassReview
Severity Minor
Time To Fix Quick (30 mins)
Examples Phinx

9.108. Could Be Protected Class Constant

Class constant may use ‘protected’ visibility.

Since PHP 7.1, constants may also have a public/protected/private visibility. This restrict their usage to anywhere, class and children or class.

As a general rule, it is recommended to make constant ‘private’ by default, and to relax this restriction as needed. PHP makes them public by default.

<?php

class foo {
    // pre-7.1 style
    const PRE_71_CONSTANT = 1;

    // post-7.1 style
    protected const PROTECTED_CONSTANT = 2;
    public const PUBLIC_CONSTANT = 3;
}

class foo2 extends foo {
    function bar() {
        // PROTECTED_CONSTANT may only be used in its class or its children
        echo self::PROTECTED_CONSTANT;
    }
}

class foo3 extends foo {
    function bar() {
        // PROTECTED_CONSTANT may only be used in its class or any of its children
        echo self::PROTECTED_CONSTANT;
    }
}

// Other constants may be used anywhere
function x($a = foo::PUBLIC_CONSTANT) {
    echo $a.' '.foo:PRE_71_CONSTANT;
}

?>
Short name Classes/CouldBeProtectedConstant
Themes ClassReview
Severity Minor
Time To Fix Quick (30 mins)

9.109. Could Be Protected Method

Those methods are declared public, but are never used publicly. They may be made protected.

<?php

class foo {
    // Public, and used publicly
    public publicMethod() {}

    // Public, but never used outside the class or its children
    public protectedMethod() {}

    private function bar() {
        $this->protectedMethod();
    }
}

$foo = new Foo();
$foo->publicMethod();

?>

These properties may even be made private.

Short name Classes/CouldBeProtectedMethod
Themes ClassReview
Severity Minor
Time To Fix Quick (30 mins)

9.110. Could Be Protected Property

Those properties are declared public, but are never used publicly. They may be made protected.

<?php

class foo {
    // Public, and used publicly
    public $publicProperty;
    // Public, but never used outside the class or its children
    public $protectedProperty;

    function bar() {
        $this->protectedProperty = 1;
    }
}

$foo = new Foo();
$foo->publicProperty = 3;

?>

This property may even be made private.

Short name Classes/CouldBeProtectedProperty
Themes ClassReview
Severity Minor
Time To Fix Slow (1 hour)

9.111. Could Be Static

This global is only used in one function or method. It may be called ‘static’, instead of global. This allows you to keep the value between call to the function, but will not be accessible outside this function.

<?php
function foo( ) {
    static $variableIsReservedForX; // only accessible within foo( ), even between calls.
    global $variableIsGlobal;       //      accessible everywhere in the application
}
?>
Short name Structures/CouldBeStatic
Themes Analyze, ClassReview, Analyze, ClassReview
Severity Major
Time To Fix Quick (30 mins)
Examples Dolphin, Contao

9.112. Could Be Static Closure

Closure may be static, and prevent the import of $this.

By preventing the useless import of $this, you avoid useless work.

This also has the added value to prevent the usage of $this from the closure. This is a good security practice.

<?php

class Foo
{
    function __construct()
    {

        // Not possible to use $this
        $func = static function() {
            var_dump($this);
        };
        $func();

        // Normal import of $this
        $closure = function() {
            var_dump($this);
        };
    }
};
new Foo();

?>

This is a micro-optimisation. Apply it in case of intensive usage.

See also Anonymous functions, GeneratedHydrator and Static anonymous functions.

9.112.1. Suggestions

  • Add the static keyword to the closure.
  • Make actual usage of $this in the closure.
Short name Functions/CouldBeStaticClosure
Themes Suggestions
Severity Minor
Time To Fix Quick (30 mins)
Examples Piwigo

9.113. Could Be Typehinted Callable

Those arguments may use the callable Typehint.

‘callable’ is a PHP keyword that represents callback functions. Those may be used in dynamic function call, like $function(); or as callback functions, like with array_map();

callable may be a string representing a function name or a static call (including ::), an array with two elements, (a class or object, and a method), or a closure.

When arguments are used to call a function, but are not marked with ‘callable’, they are reported by this analysis.

<?php

function foo(callable $callable) {
    // very simple callback
    return $callable();
}

function foo2($array, $callable) {
    // very simple callback
    return array_map($array, $callable);
}

?>

See also Callback / callable.

9.113.1. Suggestions

  • Add the typehint callable
  • Use the function is_callable() inside the method if ‘callable’ is too strong.
Short name Functions/CouldBeCallable
Themes Suggestions
Severity Minor
Time To Fix Quick (30 mins)
Examples Magento, PrestaShop

9.114. Could Make A Function

When a function is called across the code with the same arguments often enough, it should be turned into a local API.

This approach is similar to turning literals into constants : it centralize the value, it helps refactoring by updating it. It also makes the code more readable. Moreover, it often highlight common grounds between remote code locations.

The analysis looks for functions calls, and checks the arguments. When the calls occurs more than 4 times, it is reported.

<?php

// str_replace is used to clean '&' from strings.
// It should be upgraded to a central function
function foo($arg ) {
    $arg = str_replace('&', '', $arg);
    // do something with $arg
}

class y {
    function bar($database ) {
        $value = $database->queryName();
        $value = str_replace('&', '', $value);
        // $value = removeAmpersand($value);
        // do something with $arg2
    }
}

// helper function
function removeAmpersand($string) {
    return str_replace('&', '', $string);
}

?>

See also Don’t repeat yourself (DRY).

9.114.1. Suggestions

  • Create a constant for common pieces of data
  • Create a function based on context-free repeated elements
  • Create a class based on repeated elements with dependent values
Short name Functions/CouldCentralize
Themes Analyze, Suggestions
Severity Minor
Time To Fix Slow (1 hour)

9.115. Could Return Void

The following functions may bear the void return typehint.

<?php

// This can be Void
function foo(&$a) {
    ++$a;
    return;
}

// This can't be Void
function bar($a) {
    ++$a;
    return $a;
}

?>

See also Returning values and Void Return Type.

9.115.1. Suggestions

  • Add the return type void to the method or function
Short name Functions/CouldReturnVoid
Themes Suggestions
Severity Minor
Time To Fix Quick (30 mins)
Examples WordPress

9.116. Could Typehint

Arguments that are tested with instanceof gain from making it a Typehint.

<?php

function foo($a, $b) {
    // $a is tested for B with instanceof.
    if (!$a instanceof B) {
        return;
    }

    // More code
}

function foo(B $a, $b) {
    // May omit the initial test

    // More code
}

?>
Short name Functions/CouldTypehint
Themes Suggestions
Severity Minor
Time To Fix Quick (30 mins)

9.117. Could Use Alias

This long name may be reduced by using an available alias.

This applies to classes (as full name or prefix), and to constants and functions.

<?php

use a\b\c;
use function a\b\c\foo;
use const a\b\c\D;

// This may be reduced with the above alias to c\d()
new a\b\c\d();

// This may be reduced to c\d\e\f
new a\b\c\d\e\f();

// This may be reduced to c()
new a\b\c();

// This may be reduced to D
echo a\b\c\D;

// This may be reduced to D
a\b\c\foo();

// This can't be reduced : it is an absolute name
\a\b\c\foo();

// This can't be reduced : it is no an alias nor a prefix
a\b\d\foo();

?>

9.117.1. Suggestions

  • Use all your aliases so as to make the code shorter and more readable
  • Add new aliases for missing path
  • Make class names absolute and drop the aliases
Short name Namespaces/CouldUseAlias
Themes Suggestions
Severity Minor
Time To Fix Quick (30 mins)

9.118. Could Use Compact

Compact() turns a group of variables into an array. It may be used to simplify expressions.

<?php

$a = 1;
$b = 2;

// Compact call
$array = compact('a', 'b');

$array === [1, 2];

// Detailing all the keys and their value
$array = ['a' => $a, 'b' => $b];

?>

Note that compact accepts any string, and any undefined variable is not set, without a warning.

See also compact.

9.118.1. Suggestions

  • Replace the array() call with a compact() call.
Short name Structures/CouldUseCompact
Themes Suggestions
Severity Minor
Time To Fix Quick (30 mins)
Examples WordPress

9.119. Could Use Short Assignation

Use short assignment operator, to speed up code, and keep syntax clear.

Some operators, like * or +, have a compact and fast ‘do-and-assign’ version. They looks like a compacted version for = and the operator. This syntax is good for readability, and saves some memory in the process.

Depending on the operator, not all permutations of arguments are possible.

Addition and short assignation of addition have a different set of features when applied to arrays. Do not exchange one another in that case.

<?php

$a = 10 + $a;
$a += 10;

$b = $b - 1;
$b -= 1;

$c = $c * 2;
$c *= 2;

$d = $d / 3;
$d /= 3;

$e = $e % 4;
$e %= 4;

$f = $f | 5;
$f |= 5;

$g = $g & 6;
$g &= 6;

$h = $h ^ 7;
$h ^= 7;

$i = $i >> 8;
$i >>= 8;

$j = $j << 9;
$j <<= 9;

// PHP 7.4 and more recent
$l = $l ?? 'value';
$l ??= 'value';

?>

Short operators are faster than the extended version, though it is a micro-optimization.

See also Assignation Operators.

9.119.1. Suggestions

  • Change the expression to use the short assignation
Short name Structures/CouldUseShortAssignation
Themes Analyze, Performances
Severity Minor
Time To Fix Instant (5 mins)
ClearPHP use-short-assignations
Examples ChurchCRM, Thelia

9.120. Could Use Trait

The following classes have been found implementing all of a trait’s methods : it could use this trait, and remove duplicated code.

<?php

trait t {
    function t1() {}
    function t2() {}
    function t3() {}
}

// t1, t2, t3 method could be dropped, and replaced with 'use t'
class foo1 {
    function t1() {}
    function t2() {}
    function t3() {}

    function j() {}
}

// foo2 is just the same as foo1
class foo2 {
    use t;

    function j() {}
}

?>

The comparison between the class methods’ and the trait’s methods are based on token. They may yieldd some false-positives.

See also Forgotten Interface.

9.120.1. Suggestions

  • Use trait, and remove duplicated code
Short name Traits/CouldUseTrait
Themes Analyze
Severity Minor
Time To Fix Quick (30 mins)

9.121. Could Use Try

Some commands may raise exceptions. It is recommended to use the try/catch block to intercept those exceptions, and process them.

  • / : DivisionByZeroError
  • % : DivisionByZeroError
  • intdiv() : DivisionByZeroError
  • << : ArithmeticError
  • >> : ArithmeticError
  • Phar::mungserver : PharException
  • Phar::webphar : PharException

See also Predefined Exceptions, PharException.

9.121.1. Suggestions

  • Add a try/catch clause around those commands
  • Add a check on the values used with those operator : for example, check a dividend is not 0, or a bitshift is not negative
Short name Exceptions/CouldUseTry
Themes Suggestions
Severity Minor
Time To Fix Quick (30 mins)
Examples Mautic

9.122. Could Use __DIR__

Use __DIR__ constant to access the current file’s parent directory.

Avoid using dirname() on __FILE__.

<?php

// Better way
$fp = fopen(__DIR__.'/myfile.txt', 'r');

// compatible, but slow way
$fp = fopen(dirname(__FILE__).'/myfile.txt', 'r');

// Since PHP 5.3
assert(dirname(__FILE__) == __DIR__);

?>

__DIR__ has been introduced in PHP 5.3.0.

See also Magic Constants.

9.122.1. Suggestions

  • Use __DIR__ instead of dirname(__FILE__);
Short name Structures/CouldUseDir
Themes Analyze, Suggestions
Severity Major
Time To Fix Quick (30 mins)
Examples Woocommerce, Piwigo

9.123. Could Use array_fill_keys

array_fill_keys() is a native PHP function that creates an array from keys. It gets the list of keys, and a constant value to assign to each keys.

This is twice faster than doing the same with a loop.

Note that is possible to use an object as initializing value : every element of the final array will be pointing to the same value. And, also, using an object as initializing value means that the same object will be used for each key : the object will not be cloned for each value.

<?php

$array = range('a', 'z');

// Fast way to build the array
$b = array_fill_keys($a, 0);

// Fast way to build the array, but every element will be the same object
$b = array_fill_keys($a, new Stdclass());

// Slow way to build the array
foreach($array as $a) {
    $b[$a] = 0;
}

// Setting everything to null, slowly
$array = array_map(function() {}, $array);

?>

See also array_fill_keys.

9.123.1. Suggestions

  • Use array_fill_keys()
Short name Structures/CouldUseArrayFillKeys
Themes Suggestions
Severity Minor
Time To Fix Slow (1 hour)
Examples ChurchCRM, PhpIPAM

9.124. Could Use array_unique

Use array_unique() to collect unique elements from an array.

Always try to use native PHP functions, instead of rebuilding them with custom PHP code.

<?php

    $unique = array();
    foreach ($array as $b) {
        if (!in_array($b, $unique)) {
            /*  May be more code */
            $unique[] = $b;
        }
    }
?>

See also array_unique.

9.124.1. Suggestions

  • Turn the foreach() and its condition into a call to array_unique()
  • Extract the condition from the foreach() and add a separate call to array_unique()
Short name Structures/CouldUseArrayUnique
Themes Suggestions
Severity Minor
Time To Fix Quick (30 mins)
Examples Dolibarr, OpenEMR

9.125. Could Use self

self keyword refers to the current class, or any of its parents. Using it is just as fast as the full class name, it is as readable and it is will not be changed upon class or namespace change.

It is also routinely used in traits : there, self represents the class in which the trait is used, or the trait itself.

<?php

class x {
    const FOO = 1;

    public function bar() {
        return self::FOO;
// same as return x::FOO;
    }
}

?>

See also Scope Resolution Operator (::).

9.125.1. Suggestions

  • replace the explicit name with self
Short name Classes/ShouldUseSelf
Themes Analyze, Suggestions, ClassReview
Severity Minor
Time To Fix Instant (5 mins)
Examples WordPress, LiveZilla

9.126. Could Use str_repeat()

Use str_repeat() or str_pad() instead of making a loop.

Making a loop to repeat the same concatenation is actually much longer than using str_repeat(). As soon as the loop repeats more than twice, str_repeat() is much faster. With arrays of 30, the difference is significant, though the whole operation is short by itself.

<?php

// This adds 7 'e' to $x
$x .= str_repeat('e', 7);

// This is the same as above,
for($a = 3; $a < 10; ++$a) {
    $x .= 'e';
}

// here, $default must contains 7 elements to be equivalent to the previous code
foreach($default as $c) {
    $x .= 'e';
}

?>

9.126.1. Suggestions

  • Use strrepeat whenever possible
Short name Structures/CouldUseStrrepeat
Themes Analyze, Top10
Severity Minor
Time To Fix Slow (1 hour)
Examples Zencart

9.127. Crc32() Might Be Negative

crc32() may return a negative number, on 32 bits platforms.

According to the manual : Because PHP’s integer type is signed many CRC32 checksums will result in negative integers on 32 bits platforms. On 64 bits installations, all crc32() results will be positive integers though.

<?php

// display the checksum with %u, to make it unsigned
echo sprintf('%u', crc32($str));

// turn the checksum into an unsigned hexadecimal
echo dechex(crc32($str));

// avoid concatenating crc32 to a string, as it may be negative on 32bits platforms
echo 'prefix'.crc32($str);

?>

See also crc32().

Short name Php/Crc32MightBeNegative
Themes Analyze
Severity Major
Time To Fix Slow (1 hour)

9.128. Curly Arrays

PHP supports the curly brackets for arrays.

It is possible to access individual elements in an array by using its offset between square brackets [] or curly brackets {}.

<?php

$array = ['a', 'b', 'c', 'd', 'e'];

print $array[2]; // displays 'b';
print $array{3}; // displays 'c';


?>

Curly brackets are seldom used, and will probably confuse or surprise the reader. It is recommended not to used them.

See also Array.

Short name Arrays/CurlyArrays
Themes Coding Conventions
Severity Minor
Time To Fix Instant (5 mins)

9.129. Dangling Array References

Always unset a referenced-variable used in a loop.

It is highly recommended to unset blind variables when they are set up as references after a loop.

<?php

$array = array(1,2,3,4);

foreach($array as &$a) {
    $a += 1;
}
// This only unset the reference, not the value
unset($a);




// Dangling array problem
foreach($array as &$a) {
    $a += 1;
}
//$array === array(3,4,5,6);

// This does nothing (apparently)
// $a is already a reference, even if it doesn't show here.
foreach($array as $a) {}
//$array === array(3,4,5,5);

?>

When omitting this step, the next loop that will also require this variable will deal with garbage values, and produce unexpected results.

See also : No Dangling Reference, PHP foreach pass-by-reference: Do it right, or better not at all, How does PHP ‘foreach’ actually work?, References and foreach.

9.129.1. Suggestions

  • Avoid using the reference altogether : sometimes, the reference is not needed.
  • Add unset() right after the loop, to avoid reusing the reference.
Short name Structures/DanglingArrayReferences
Themes Analyze, Top10
Severity Major
Time To Fix Instant (5 mins)
ClearPHP no-dangling-reference
Examples Typo3, SugarCrm

9.130. Deep Definitions

Structures, such as functions, classes, interfaces, traits, etc. may be defined anywhere in the code, including inside functions. This is legit code for PHP.

Since the availability of __autoload, there is no need for that kind of code. Structures should be defined, and accessible to the autoloading. Inclusion and deep definitions should be avoided, as they compel code to load some definitions, while autoloading will only load them if needed.

<?php

class X {
    function init() {
        // myFunction is defined when and only if X::init() is called.
        if (!function_exists('myFunction'){
            function myFunction($a) {
                return $a + 1;
            }
        })
    }
}

?>

Functions are excluded from autoload, but shall be gathered in libraries, and not hidden inside other code.

Constants definitions are tolerated inside functions : they may be used for avoiding repeat, or noting the usage of such function.

See also Autoloading Classe.

Short name Functions/DeepDefinitions
Themes Analyze
Severity Major
Time To Fix Slow (1 hour)
Examples Dolphin

9.131. Define With Array

PHP 7.0 has the ability to define an array as a constant, using the define() native call. This was not possible until that version, only with the const keyword.

<?php

//Defining an array as a constant
define('MY_PRIMES', [2, 3, 5, 7, 11]);

?>
Short name Php/DefineWithArray
Themes CompatibilityPHP53, CompatibilityPHP54, CompatibilityPHP55, CompatibilityPHP56
Php Version With PHP 7.0 and more recent
Severity Critical
Time To Fix Slow (1 hour)

9.132. Dependant Abstract Classes

Abstract classes should be autonomous. It is recommended to avoid depending on methods, constant or properties that should be made available in inheriting classes.

The following abstract classes make usage of constant, methods and properties, static or not, that are not defined in the class. This means the inheriting classes must provide those constants, methods and properties, but there is no way to enforce this.

This may also lead to dead code : when the abstract class is removed, the host class have unused properties and methods.

<?php

// autonomous abstract class : all it needs is within the class
abstract class c {
    private $p = 0;

    function foo() {
        return ++$this->p;
    }
}

// dependant abstract class : the inheriting classes needs to provide some properties or methods
abstract class c2 {
    function foo() {
        // $p must be provided by the extending class
        return ++$this->p;
    }
}

class c3 extends c2 {
    private $p = 0;
}
?>

See also Dependant Trait.

9.132.1. Suggestions

  • Make the class only use its own resources
  • Split the class in autonomous classes
  • Add local property definitions to make the class independant
Short name Classes/DependantAbstractClass
Themes Analyze, ClassReview
Severity Minor
Time To Fix Quick (30 mins)

9.133. Dependant Trait

Traits should be autonomous. It is recommended to avoid depending on methods or properties that should be in the using class.

The following traits make usage of methods and properties, static or not, that are not defined in the trait. This means the host class must provide those methods and properties, but there is no way to enforce this.

This may also lead to dead code : when the trait is removed, the host class have unused properties and methods.

<?php

// autonomous trait : all it needs is within the trait
trait t {
    private $p = 0;

    function foo() {
        return ++$this->p;
    }
}

// dependant trait : the host class needs to provide some properties or methods
trait t2 {
    function foo() {
        return ++$this->p;
    }
}

class x {
    use t2;

    private $p = 0;
}
?>

See also Dependant Abstract Classes.

9.133.1. Suggestions

  • Add local property definitions to make the trait independant
  • Make the trait only use its own resources
  • Split the trait in autonomous traits
Short name Traits/DependantTrait
Themes Analyze
Severity Minor
Time To Fix Slow (1 hour)
Examples Zencart

9.134. Deprecated Functions

The following functions are deprecated. It is recommended to stop using them now and replace them with a durable equivalent.

Note that these functions may be still usable : they generate warning that help tracking their usage in the log. To eradicate their usage, watch the logs, and update any deprecated warning. This way, the code won’t be stuck when the function is actually removed from PHP.

<?php

// This is the current function
list($day, $month, $year) = explode('/', '08/06/1995');

// This is deprecated
list($day, $month, $year) = split('/', '08/06/1995');

?>

9.134.1. Suggestions

  • Replace those deprecated with modern syntax
  • Stop using deprecated syntax
Short name Php/Deprecated
Themes Analyze
Severity Major
Time To Fix Quick (30 mins)
ClearPHP no-deprecated
Examples Dolphin

9.135. Dereferencing String And Arrays

PHP allows the direct dereferencing of strings and arrays.

This was added in PHP 5.5. There is no need anymore for an intermediate variable between a string and array (or any expression generating such value) and accessing an index.

<?php
$x = array(4,5,6);
$y = $x[2] ; // is 6

May be replaced by
$y = array(4,5,6)[2];
$y = [4,5,6][2];
?>
Short name Structures/DereferencingAS
Themes CompatibilityPHP53, CompatibilityPHP54
Php Version With PHP 5.3 and older
Severity Major
Time To Fix Quick (30 mins)

9.136. Detect Current Class

Detecting the current class should be done with ::class operator.

__CLASS__ may be replaced by self\:\:class. get_called_class() may be replaced by static\:\:class.

__CLASS__ and get_called_class() are set to be deprecated in PHP 7.4.

<?php

class X {
    function foo() {
        echo __CLASS__.\n;          // X
        echo self::class.\n;        // X

        echo get_called_class().\n;  // Y
        echo static::class.\n;       // Y
    }
}

class Y extends X {}

$y = new Y();
$y->foo();

?>

See also PHP RFC: Deprecations for PHP 7.4.

Short name Php/DetectCurrentClass
Themes Suggestions, CompatibilityPHP74
Php Version With PHP 8.0 and older

9.137. Direct Call To __clone()

Direct call to magic method __clone() was forbidden. It is allowed since PHP 7.0.

From the RFC : Doing calls like $obj->`__clone( <http://www.php.net/manual/en/language.oop5.magic.php>`_ ) is now allowed. This was the only magic method that had a compile-time check preventing some calls to it, which doesn't make sense. If we allow all other magic methods to be called, there's no reason to forbid this one.

<?php

    class Foo {
        function __clone() {}
    }

    $a = new Foo;
    $a->__clone();
?>

See also Directly calling `__clone is allowed <https://wiki.php.net/rfc/abstract_syntax_tree#directly_calling_clone_is_allowed>`_.

Short name Php/DirectCallToClone
Themes CompatibilityPHP53, CompatibilityPHP54, CompatibilityPHP55, CompatibilityPHP56
Php Version With PHP 7.0 and more recent
Severity Critical
Time To Fix Slow (1 hour)

9.138. Direct Injection

The following code act directly upon PHP incoming variables like $_GET and $_POST. This makes those snippets very unsafe.

<?php

// Direct injection
echo Hello.$_GET['user']., welcome.;

// less direct injection
foo($_GET['user']);
function foo($user) {
    echo Hello.$user., welcome.;
}

?>

See also Cross-Site Scripting (XSS)

9.138.1. Suggestions

  • Validate input : make sure the incoming data are what you expect from them.
  • Escape output : prepare outgoing data for the next system to use.
Short name Security/DirectInjection
Themes Security
Severity Major
Time To Fix Quick (30 mins)

9.139. Directly Use File

Some PHP functions have a close cousin that work directly on files : use them. This is faster and less code to write.

<?php

// Good way
$file_hash = hash_file('sha512', 'example.txt');

// Slow way
$file_hash = hash('sha512', file_get_contents('example.txt'));

?>

See also hash_file.

9.139.1. Suggestions

  • Use the _file() version of those functions
Short name Structures/DirectlyUseFile
Themes Suggestions
Severity Minor
Time To Fix Instant (5 mins)

9.140. Disconnected Classes

One class is extending the other, but they do not use any features from one another. Basically, those two classes are using extends, but they are completely independant and may be separated.

When using the ‘extends’ keyword, the newly created classes are now acting together and making one. This should be visibile in calls from one class to the other, or simply by property usage : they can’t live without each other.

On the other hand, two completely independant classes that are merged should be kept separated.

<?php


?>

9.140.1. Suggestions

  • Remove the extension
  • Make actual usage of the classes, at least from one of them
Short name Classes/DisconnectedClasses
Themes ClassReview
Severity Minor
Time To Fix Slow (1 hour)

9.141. Do In Base

Use SQL expression to compute aggregates.

<?php

// Efficient way
$res = $db->query('SELECT sum(e) AS sumE FROM table WHERE condition');

// The sum is already done
$row = $res->fetchArray();
$c += $row['sumE'];

// Slow way
$res = $db->query('SELECT e FROM table WHERE condition');

// This aggregates the column e in a slow way
while($row = $res->fetchArray()) {
    $c += $row['e'];
}

?>
Short name Performances/DoInBase
Themes Performances
Severity Major
Time To Fix Quick (30 mins)

9.142. Don’t Be Too Manual

Adapt the examples from the PHP manual to your code. Don’t reuse directly the same names in your code : be more specific about what to expect in those variables.

<?php

// Search for phone numbers in a text
preg_match_all('/((\d{3})-(\d{3})-(\d{4}))/', $string, $phoneNumber);

// Search for phone numbers in a text
preg_match_all('/(\d{3})-(\d{3})-(\d{4})/', $string, $matches);

?>

9.142.1. Suggestions

  • Use precise name with your variables
Short name Structures/DontBeTooManual
Themes Coding Conventions
Severity Minor
Time To Fix Quick (30 mins)

9.143. Don’t Change Incomings

PHP hands over a lot of information using special variables like $_GET, $_POST, etc… Modifying those variables and those values inside variables means that the original content is lost, while it will still look like raw data, and, as such, will be untrustworthy.

<?php

// filtering and keeping the incoming value.
$_DATA'id'] = (int) $_GET['id'];

// filtering and changing the incoming value.
$_GET['id'] = strtolower($_GET['id']);

?>

It is recommended to put the modified values in another variable, and keep the original one intact.

Short name Structures/NoChangeIncomingVariables
Themes Analyze
Severity Minor
Time To Fix Slow (1 hour)

9.144. Don’t Echo Error

It is recommended to avoid displaying error messages directly to the browser.

PHP’s uses the display_errors directive to control display of errors to the browser. This must be kept to off when in production.

<?php

// Inside a 'or' test
mysql_connect('localhost', $user, $pass) or die(mysql_error());

// Inside a if test
$result = pg_query( $db, $query );
if( !$result )
{
     echo Erreur SQL: . pg_error();
     exit;
}

// Changing PHP configuration
ini_set('display_errors', 1);
// This is also a security error : 'false' means actually true.
ini_set('display_errors', 'false');

?>

Error messages should be logged, but not displayed.

See also Error reporting and List of php.ini directives.

9.144.1. Suggestions

  • Remove any echo, print, printf() call built with error messages from an exception, or external source.
Short name Security/DontEchoError
Themes Analyze, Security
Severity Critical
Time To Fix Instant (5 mins)
Examples ChurchCRM, Phpdocumentor

9.145. Don’t Loop On Yield

Use yield from, instead of looping on a generator with yield.

yield from delegate the yielding to another generator, and keep calling that generator until it is finished. It also works with implicit generator datastructure, like arrays.

<?php

function generator() {
    for($i = 0; $i < 10; ++$i) {
        yield $i;
    }
}

function delegatingGenerator() {
    yield from generator();
}

// Too much code here
function generator2() {
    foreach(generator() as $g) {
        yield $g;
    }
}

?>

There is a performance gain when delegating, over looping manually on the generator. You may even consider writing the loop to store all values in an array, then yield from the array.

See also Generator delegation via yield from.

9.145.1. Suggestions

  • Use yield from instead of the whole foreach() loop
Short name Structures/DontLoopOnYield
Themes Suggestions
Severity Minor
Time To Fix Quick (30 mins)
Examples Dolibarr, Tikiwiki

9.146. Don’t Read And Write In One Expression

Avoid giving value and using it at the same time, in one expression. This is an undefined behavior of PHP, and may change without warning.

One of those changes happens between PHP 7.2 and 7.3 :

<?php

$arr = [1];
$ref =& $arr[0];
var_dump($arr[0] + ($arr[0] = 2));
// PHP 7.2: int(4)
// PHP 7.3: int(3)

?>

See also UPGRADING 7.3.

9.146.1. Suggestions

  • Split the expression in two separate expressions
Short name Structures/DontReadAndWriteInOneExpression
Themes Analyze, CompatibilityPHP73, CompatibilityPHP74
Severity Critical
Time To Fix Quick (30 mins)

9.147. Don’t Send $this In Constructor

Don’t use $this as an argument while in the __construct(). Until the constructor is finished, the object is not finished, and may be in an unstable state. Providing it to another code may lead to error.

This is true when the receiving structure puts the incoming object immediately to work, and don’t store it for later use.

<?php

// $this is only provided when Foo is constructed
class Foo {
    private $bar = null;
    private $data = array();

    static public function build($data) {
        $foo = new Foo($data);
        // Can't build in one call. Must make it separate.
        $foo->finalize();
    }

    private function __construct($data) {
        // $this is provided too early
        $this->data = $data;
    }

    function finalize() {
        $this->bar = new Bar($this);
    }
}

// $this is provided too early, leading to error in Bar
class Foo2 extends Foo {
    private $bar = null;
    private $data = array();

    function __construct($data) {
        // $this is provided too early
        $this->bar = new Bar($this);
        $this->data = $data;
    }
}

class Bar {
    function __construct(Foo $foo) {
        // the cache is now initialized with a wrong
        $this->cache = $foo->getIt();
    }
}

?>

See also Don’t pass this out of a constructor.

9.147.1. Suggestions

  • Finish the constructor first, then call an external object.
  • Sending $this should be made accessible in a separate method, so external objects may call it.
  • Sending the current may be the responsibility of the method creating the object.
Short name Classes/DontSendThisInConstructor
Themes Analyze
Severity Minor
Time To Fix Slow (1 hour)
Examples Woocommerce, Contao

9.148. Don’t Unset Properties

Avoid unsetting properties. They would go undefined, and raise more warnings.

When getting rid of a property, assign it to null. This keeps the property in the object, yet allows existence check without errors.

<?php

class Foo {
    public $a = 1;
}

$a = new Foo();

var_dump((array) $a) ;
// la propriété est reportée, et null
// ['a' => null]

unset($a->a);

var_dump((array) $a) ;
//Empty []

// Check if a property exists
var_dump($a->b === null);

// Same result as above, but with a warning
var_dump($a->c === null);

?>

This analysis works on properties and static properties. It also reports magic properties being unset.

Thanks for Benoit Burnichon for the original idea.

9.148.1. Suggestions

  • Never unset properties : set it to null or its default value instead
  • Make the property an array, and set/unset its index
Short name Classes/DontUnsetProperties
Themes Analyze, Top10
Severity Major
Time To Fix Slow (1 hour)
Examples Vanilla, Typo3

9.149. Dont Change The Blind Var

When using a foreach(), the blind variables hold a copy of the original value. It is confusing to modify them, as it seems that the original value may be changed.

When actually changing the original value, use the reference in the foreach definition to make it obvious, and save the final reassignation.

When the value has to be prepared before usage, then save the filtered value in a separate variable. This makes the clean value obvious, and preserve the original value for a future usage.

<?php

// $bar is duplicated and kept
$foo = [1, 2, 3];
foreach($foo as $bar) {
    // $bar is updated but its original value is kept
    $nextBar = $bar + 1;
    print $bar . ' => ' . ($nextBar) . PHP_EOL;
    foobar($nextBar);
}

// $bar is updated and lost
$foo = [1, 2, 3];
foreach($foo as $bar) {
    // $bar is updated but its final value is lost
    print $bar . ' => ' . (++$bar) . PHP_EOL;
    // Now that $bar is reused, it is easy to confuse its value
    foobar($bar);
}

// $bar is updated and kept
$foo = [1, 2, 3];
foreach($foo as &$bar) {
    // $bar is updated and keept
    print $bar . ' => ' . (++$bar) . PHP_EOL;
    foobar($bar);
}

?>
Short name Structures/DontChangeBlindKey
Themes Analyze
Severity Minor
Time To Fix Quick (30 mins)

9.150. Dont Mix ++

++ operators have two distinct behaviors, and should be used in isolation.

When mixed with a larger expression, it is difficult to read, and may lead to unwanted behaviors.

<?php

    // Clear and defined behavior
    $i++;
    $a[$i] = $i;

    // $i is modified twice
    $i = --$i + 1;
?>

See also EXP30-C. Do not depend on the order of evaluation for side effects.

Short name Structures/DontMixPlusPlus
Themes Analyze
Severity Minor
Time To Fix Instant (5 mins)

9.151. Double Assignation

This happens when a container (variable, property, array index) is assigned with values twice in a row. One of them is probably a debug instruction, that was forgotten.

<?php

// Normal assignation
$a = 1;

// Double assignation
$b = 2;
$b = 3;

?>
Short name Structures/DoubleAssignation
Themes Analyze
Severity Major
Time To Fix Quick (30 mins)

9.152. Double Instructions

Twice the same call in a row. This is worth a check.

<?php

?>
Short name Structures/DoubleInstruction
Themes Analyze
Severity Minor
Time To Fix Instant (5 mins)

9.153. Double array_flip()

Avoid double array_flip() to gain speed. While array_flip() alone is usually useful, a double array_flip() usually is made to handle values and keys.

<?php

// without array_flip
function foo($array, $value) {
    $key = array_search($array, $value);

    if ($key !== false) {
        unset($array[$key]);
    }

    return $array;
}

// double array_flip
// array_flip() usage means that $array's values are all unique
function foo($array, $value) {
    $flipped = array_flip($value);
    unset($flipped[$value]);
    return array_flip($flipped);
}

?>
Short name Performances/DoubleArrayFlip
Themes Performances
Severity Major
Time To Fix Quick (30 mins)
Examples NextCloud

9.154. Drop Else After Return

Avoid else clause when the then clause returns, but not the else. And vice-versa.

This way, the else block disappears, and is now the main sequence of the function.

This is also true if else has a return, and then not. When doing so, don’t forget to reverse the condition.

<?php

// drop the else
if ($a) {
    return $a;
} else {
    doSomething();
}

// drop the then
if ($b) {
    doSomething();
} else {
    return $a;
}

// return in else and then
if ($a3) {
    return $a;
} else {
    $b = doSomething();
    return $b;
}

?>
Short name Structures/DropElseAfterReturn
Themes Analyze, Suggestions
Severity Minor
Time To Fix Quick (30 mins)

9.155. Drop Substr Last Arg

Substr() works till the end of the string when the last argument is omitted. There is no need to calculate string size to make this work.

<?php

$string = 'abcdef';

// Extract the end of the string
$cde = substr($string, 2);

// Too much work
$cde = substr($string, 2, strlen($string));

?>

See also substr.

9.155.1. Suggestions

  • Use negative length
  • Omit the last argument to get the string till its end
Short name Structures/SubstrLastArg
Themes Suggestions
Severity Minor
Time To Fix Quick (30 mins)
Examples SuiteCrm, Tine20

9.156. Dynamic Library Loading

Loading a variable dynamically requires a lot of care in the preparation of the library name.

In case of injection in the variable, the dynamic loading of a library gives a lot of power to an intruder.

<?php

    // dynamically loading a library
     dl($library. PHP_SHLIB_SUFFIX);

    // dynamically loading ext/vips
     dl('vips.' . PHP_SHLIB_SUFFIX);

    // static loading ext/vips (unix only)
     dl('vips.so');

?>

See also dl.

Short name Security/DynamicDl
Themes Security
Severity Major
Time To Fix Slow (1 hour)

9.157. Echo Or Print

Echo and print have the same functional use. <?= and printf() are also considered in this analysis.

There seems to be a choice that is not enforced : one form is dominant, (> 90%) while the others are rare.

The analyzed code has less than 10% of one of the three : for consistency reasons, it is recommended to make them all the same.

It happens that print, echo or <?= are used depending on coding style and files. One file may be consistently using print, while the others are all using echo.

<?php

echo 'a';
echo 'b';
echo 'c';
echo 'd';
echo 'e';
echo 'f';
echo 'g';
echo 'h';
echo 'i';
echo 'j';
echo 'k';

// This should probably be written 'echo';
print 'l';

?>
Short name Structures/EchoPrintConsistance
Themes Coding Conventions
Severity Minor
Time To Fix Quick (30 mins)

9.158. Echo With Concat

Optimize your echo’s by not concatenating at echo time, but serving all argument separated. This will save PHP a memory copy.

If values, literals and variables, are small enough, this won’t have visible impact. Otherwise, this is less work and less memory waste.

<?php
  echo $a, ' b ', $c;
?>

instead of

<?php
  echo  $a . ' b ' . $c;
  echo $a b $c;
?>

9.158.1. Suggestions

  • Turn the concatenation into a list of argument, by replacing the dots by commas.
Short name Structures/EchoWithConcat
Themes Performances, Analyze, Suggestions
Severity Minor
Time To Fix Quick (30 mins)
ClearPHP no-unnecessary-string-concatenation
Examples Phpdocumentor, TeamPass

9.159. Ellipsis Usage

Usage of the ellipsis keyword. The keyword is three dots : . It is also named variadic or splat operator.

It may be in function definitions, either in functioncalls.

allows for packing or unpacking arguments into an array.

<?php

$args = [1, 2, 3];
foo(...$args);
// Identical to foo(1,2,3);

function bar(...$a) {
    // Identical to : $a = func_get_args();
}
?>

See also PHP RFC: Syntax for variadic functions, PHP 5.6 and the Splat Operator, and Variable-length argument lists.

Short name Php/EllipsisUsage
Themes CompatibilityPHP53, CompatibilityPHP54, CompatibilityPHP55
Php Version With PHP 5.6 and more recent
Severity Major
Time To Fix Slow (1 hour)

9.160. Else If Versus Elseif

Always use elseif instead of else and if.

“The keyword elseif SHOULD be used instead of else if so that all control keywords look like single words”. Quoted from the PHP-FIG documentation

<?php

// Using elseif
if ($a == 1) { doSomething(); }
elseif ($a == 2) { doSomethingElseIf(); }
else { doSomethingElse(); }

// Using else if
if ($a == 1) { doSomething(); }
else if ($a == 2) { doSomethingElseIf(); }
else { doSomethingElse(); }

// Using else if, no {}
if ($a == 1)  doSomething();
else if ($a == 2) doSomethingElseIf();
else  doSomethingElse();

?>

See also elseif/else if.

9.160.1. Suggestions

  • Merge else and if into elseif
  • Turn the else expression into a block, and have more than the second if in this block
  • Turn the if / else if / else into a switch structure
Short name Structures/ElseIfElseif
Themes Analyze
Severity Minor
Time To Fix Quick (30 mins)
Examples TeamPass, Phpdocumentor

9.161. Empty Blocks

Full empty block, part of a control structures.

It is recommended to remove those blocks, so as to reduce confusion in the code.

<?php

foreach($foo as $bar) ; // This block seems erroneous
    $foobar++;

if ($a === $b) {
    doSomething();
} else {
    // Empty block. Remove this
}

// Blocks containing only empty expressions are also detected
for($i = 0; $i < 10; $i++) {
    ;
}

// Although namespaces are not control structures, they are reported here
namespace A;
namespace B;

?>

9.161.1. Suggestions

  • Fill the block with a command
  • Fill the block with a comment that explain the situation
  • Remove the block and its commanding operator
Short name Structures/EmptyBlocks
Themes Analyze
Severity Minor
Time To Fix Instant (5 mins)
Examples Cleverstyle, PhpIPAM

9.162. Empty Classes

Classes that do no define anything at all. This is probably dead code.

Classes that are directly derived from an exception are omitted.

<?php

//Empty class
class foo extends bar {}

//Not an empty class
class foo2 extends bar {
    const FOO = 2;
}

//Not an empty class, as derived from Exception
class barException extends \Exception {}

?>

9.162.1. Suggestions

  • Remove an empty class :it is probably dead code.
  • Add some code to the class to make it concrete.
Short name Classes/EmptyClass
Themes Analyze
Severity Minor
Time To Fix Quick (30 mins)
Examples WordPress

9.163. Empty Function

Function or method whose body is empty.

Such functions or methods are rarely useful. As a bare minimum, the function should return some useful value, even if constant.

A method is considered empty when it contains nothing, or contains expressions without impact.

<?php

// classic empty function
function emptyFunction() {}

class bar {
    // classic empty method
    function emptyMethod() {}

    // classic empty function
    function emptyMethodWithParent() {}
}

class barbar extends bar {
    // NOT an empty method : it overwrites the parent method
    function emptyMethodWithParent() {}
}

?>

Methods which overwrite another methods are omitted. Methods which are the concrete version of an abstract method are considered.

9.163.1. Suggestions

  • Fill the function with actual code
  • Remove any usage of the function, then remove the function
Short name Functions/EmptyFunction
Themes Analyze
Severity Minor
Time To Fix Quick (30 mins)
Examples Contao

9.164. Empty Instructions

Empty instructions are part of the code that have no instructions.

This may be trailing semi-colon or empty blocks for if-then structures.

Comments that explains the reason of the situation are not taken into account.

<?php
    $condition = 3;;;;
    if ($condition) { }
?>

9.164.1. Suggestions

  • Remove the empty lines
  • Fill the empty lines
Short name Structures/EmptyLines
Themes Dead code, Analyze
Severity Minor
Time To Fix Instant (5 mins)
Examples Zurmo, ThinkPHP

9.165. Empty Interfaces

Empty interfaces are a code smell. Interfaces should contains at least a method or a constant, and not be totally empty.

<?php

// an empty interface
interface empty {}

// an normal interface
interface normal {
    public function i() ;
}

// a constants interface
interface constantsOnly {
    const FOO = 1;
}

?>

See also Empty interfaces are bad practice and Blog : Are empty interfaces code smell?.

Short name Interfaces/EmptyInterface
Themes Analyze
Severity Minor
Time To Fix Instant (5 mins)

9.166. Empty List

Empty list() are not allowed anymore in PHP 7. There must be at least one variable in the list call.

<?php

//Not accepted since PHP 7.0
list() = array(1,2,3);

//Still valid PHP code
list(,$x) = array(1,2,3);

?>
Short name Php/EmptyList
Themes Analyze, CompatibilityPHP70
Php Version With PHP 7.0 and older
Severity Major
Time To Fix Quick (30 mins)

9.167. Empty Namespace

Declaring a namespace in the code and not using it for structure declarations or global instructions is useless.

Using simple style :

<?php

namespace Y;

class foo {}


namespace X;
// This is useless

?>

Using bracket-style syntax :

<?php

namespace X {
    // This is useless
}

namespace Y {

    class foo {}

}

?>
Short name Namespaces/EmptyNamespace
Themes Analyze, Dead code
Severity Minor
Time To Fix Instant (5 mins)
ClearPHP no-empty-namespace

9.168. Empty Slots In Arrays

PHP tolerates the last element of an array to be empty.

<?php
    $a = array( 1, 2, 3, );
    $b =      [ 4, 5, ];
?>
Short name Arrays/EmptySlots
Themes Coding Conventions
Severity Minor
Time To Fix Instant (5 mins)

9.169. Empty Traits

List of all empty trait defined in the code.

<?php

// empty trait
trait t { }

// Another empty trait
trait t2 {
    use t;
}

?>

Such traits may be reserved for future use. They may also be forgotten, and dead code.

9.169.1. Suggestions

  • Add some code to the trait
  • Remove the trait
Short name Traits/EmptyTrait
Themes Analyze
Severity Minor
Time To Fix Instant (5 mins)

9.170. Empty Try Catch

The code does try, then catch errors but do no act upon the error.

<?php

try {
    doSomething();
} catch (Throwable $e) {
    // ignore this
}

?>

At worst, the error should be logged, so as to measure the actual usage of the catch expression.

catch( Exception $e) (PHP 5) or catch(`Throwable <http://www.php.net/manual/en/class.throwable.php>`_ $e) with empty catch block should be banned. They ignore any error and proceed as if nothing happened. At worst, the event should be logged for future analysis.

See also Empty Catch Clause.

9.170.1. Suggestions

  • Add some logging in the catch
  • Add a comment to mention why the catch is empty
  • Change the exception, chain it and throw again
Short name Structures/EmptyTryCatch
Themes Analyze
Severity Minor
Time To Fix Quick (30 mins)
Examples LiveZilla, Mautic

9.171. Empty With Expression

empty() doesn’t accept expressions until PHP 5.5. Until then, it is necessary to store the result of the expression in a variable and then, test it with empty().

<?php

// PHP 5.5+ empty() usage
if (empty(strtolower($b . $c))) {
    doSomethingWithoutA();
}

// Compatible empty() usage
$a = strtolower($b . $c);
if (empty($a)) {
    doSomethingWithoutA();
}

?>

See also empty.

9.171.1. Suggestions

  • Use the compatible syntax, and store the result in a local variable before testing it with empty
Short name Structures/EmptyWithExpression
Themes Suggestions
Php Version With PHP 5.5 and more recent
Severity Major
Time To Fix Quick (30 mins)
Examples HuMo-Gen

9.172. Encoded Simple Letters

Some simple letters are written in escape sequence.

Usually, escape sequences are made to encode unusual characters. Using escape sequences for simple characters, like letters or numbers is suspicious.

This analysis also detect unicode codepoint with superfluous leading zeros.

<?php

// This escape sequence makes eval hard to spot
$a = ev1l;
$a('php_info();');

// With a PHP 7.0 unicode code point sequence
$a = ev\u{000041}l;
$a('php_info();');

// With a PHP 5.0+ hexadecimal sequence
$a = ev\x41l;
$a('php_info();');

?>

9.172.1. Suggestions

  • Make all simple letter appear clearly
  • Add comments about why this code is encoded
Short name Security/EncodedLetters
Themes Security
Severity Minor
Time To Fix Quick (30 mins)
Examples Zurmo

9.173. Eval() Usage

Using eval() is evil.

Using eval() is bad for performances (compilation time), for caches (it won’t be compiled), and for security (if it includes external data).

<?php
    // Avoid using incoming data to build the eval() expression : any filtering error leads to PHP injection
    $mathExpression = $_GET['mathExpression'];
    $mathExpression = preg_replace('#[^0-9+\-*/\(/)]#is', '', $mathExpression); // expecting 1+2
    $literalCode = '$a = '.$mathExpression.';';
    eval($literalCode);
    echo $a;

    // If the code code given to eval() is known at compile time, it is best to put it inline
    $literalCode = 'phpinfo();';
    eval($literalCode);

?>

Most of the time, it is possible to replace the code by some standard PHP, like variable variable for accessing a variable for which you have the name. At worse, including a pregenerated file is faster and cacheable.

There are several situations where eval() is actually the only solution :

For PHP 7.0 and later, it is important to put eval() in a try..catch expression.

See also eval and The Land Where PHP Uses `eval() <https://www.exakat.io/land-where-php-uses-eval/>`_.

9.173.1. Suggestions

  • Use a dynamic feature of PHP to replace the dynamic code
  • Store the code on the disk, and use include
  • Replace create_function() with a closure!
Short name Structures/EvalUsage
Themes Analyze, Performances, Security
Severity Major
Time To Fix Quick (30 mins)
ClearPHP no-eval
Examples XOOPS, Mautic

9.174. Exception Order

When catching exception, the most specialized exceptions must be in the early catch, and the most general exceptions must be in the later catch. Otherwise, the general catches intercept the exception, and the more specialized will not be read.

<?php

class A extends \Exception {}
class B extends A {}

try {
    throw new A();
}
catch(A $a1) { }
catch(B $b2 ) {
    // Never reached, as previous Catch is catching the early worm
}

?>
Short name Exceptions/AlreadyCaught
Themes Dead code
Examples Woocommerce

9.175. Exit() Usage

Using exit or die() <http://www.php.net/`die>`_ in the code makes the code untestable (it will break unit tests). Moreover, if there is no reason or string to display, it may take a long time to spot where the application is stuck.

<?php

// Throw an exception, that may be caught somewhere
throw new \Exception('error');

// Dying with error message.
die('error');

function foo() {
    //exiting the function but not dying
    if (somethingWrong()) {
        return true;
    }
}
?>

Try exiting the function/class with return, or throw exception that may be caught later in the code.

9.175.1. Suggestions

  • Avoid exit and die. Let the script finish.
  • Throw an exception and let it be handled before finishing
Short name Structures/ExitUsage
Themes Analyze
Severity Major
Time To Fix Quick (30 mins)
ClearPHP no-exit
Examples Traq, ThinkPHP

9.176. Exponent Usage

Usage of the ** operator or **=, to make exponents.

<?php

$eight = 2 ** 3;

$sixteen = 4;
$sixteen \*\*\= 2;

?>

See also Arithmetic Operators.

Short name Php/ExponentUsage
Themes CompatibilityPHP53, CompatibilityPHP54, CompatibilityPHP55
Php Version With PHP 5.6 and more recent
Severity Major
Time To Fix Instant (5 mins)

9.177. Failed Substr Comparison

The extracted string must be of the size of the compared string.

This is also true for negative lengths.

<?php

// Possible comparison
if (substr($a, 0, 3) === 'abc') { }
if (substr($b, 4, 3) === 'abc') { }

// Always failing
if (substr($a, 0, 3) === 'ab') { }
if (substr($a, 3, -3) === 'ab') { }

// Omitted in this analysis
if (substr($a, 0, 3) !== 'ab') { }

?>

9.177.1. Suggestions

  • Fix the string
  • Fix the length of the string
  • Put the string in a constant, and use strlen() or mb_strlen()
Short name Structures/FailingSubstrComparison
Themes Analyze, Top10
Severity Major
Time To Fix Instant (5 mins)
Examples Zurmo, MediaWiki

9.178. Fetch One Row Format

When reading results with ext/Sqlite3, it is recommended to explicitly request SQLITE3_NUM or SQLITE3_ASSOC, while avoiding the default value and SQLITE3_BOTH.

<?php

$res = $database->query($query);

// Fastest version, but less readable
$row = $res->fetchArray(\SQLITE3_NUM);
// Almost the fastest version, and more readable
$row = $res->fetchArray(\SQLITE3_ASSOC);

// Default version. Quite slow
$row = $res->fetchArray();

// Worse case
$row = $res->fetchArray(\SQLITE3_BOTH);

?>

This is a micro-optimisation. The difference may be visible with 200k rows fetches, and measurable with 10k.

Short name Performances/FetchOneRowFormat
Themes Performances
Severity Minor
Time To Fix Instant (5 mins)

9.179. Final Class Usage

List of all final classes being used.

final may be applied to classes and methods.

<?php
class BaseClass {
   public function test() {
       echo 'BaseClass::test() called'.PHP_EOL;
   }

   final public function moreTesting() {
       echo 'BaseClass::moreTesting() called'.PHP_EOL;
   }
}

class ChildClass extends BaseClass {
   public function moreTesting() {
       echo 'ChildClass::moreTesting() called'.PHP_EOL;
   }
}
// Results in Fatal error: Cannot override final method BaseClass::moreTesting()
?>

See also Final Keyword.

Short name Classes/Finalclass
Themes ClassReview, LintButWontExec

9.180. Final Methods Usage

List of all final methods being used.

final may be applied to classes and methods.

<?php
class BaseClass {
   public function test() {
       echo 'BaseClass::test() called'.PHP_EOL;
   }

   final public function moreTesting() {
       echo 'BaseClass::moreTesting() called'.PHP_EOL;
   }
}

class ChildClass extends BaseClass {
   public function moreTesting() {
       echo 'ChildClass::moreTesting() called'.PHP_EOL;
   }
}
// Results in Fatal error: Cannot override final method BaseClass::moreTesting()
?>

See also Final Keyword.

Short name Classes/Finalmethod
Themes LintButWontExec, ClassReview

9.181. Find Key Directly

No need for a foreach() to search for a key.

PHP offers two solutions : array_search() and array_keys(). Array_search() finds the first key that fits a value, and array_keys returns all the keys.

<?php

$array = ['a', 'b', 'c', 'd', 'e'];

print array_search($array, 'c');
// print 2 => 'c';

print_r(array_keys($array, 'c'));
// print 2 => 'c';

?>

See also array_search and array_keys.

Short name Structures/GoToKeyDirectly
Themes Suggestions
Severity Major
Time To Fix Instant (5 mins)

9.182. Flexible Heredoc

Flexible syntax for Heredoc.

The new flexible syntax for heredoc and nowdoc enable the closing marker to be indented, and remove the new line requirement after the closing marker.

It was introduced in PHP 7.3.

<?php

// PHP 7.3 and newer
foo($a = <<<END

    flexible syntax
    with extra indentation

    END);

// All PHP versions
$a = <<<END

    Normal syntax

END;


?>

This syntax is backward incompatible : once adopted in the code, previous versions won’t compile it.

See also Heredoc and Flexible Heredoc and Nowdoc Syntaxes.

Short name Php/FlexibleHeredoc
Themes CompatibilityPHP53, CompatibilityPHP70, CompatibilityPHP71, CompatibilityPHP72, CompatibilityPHP54, CompatibilityPHP55, CompatibilityPHP56
Php Version With PHP 7.3 and more recent
Severity Critical
Time To Fix Instant (5 mins)

9.183. For Using Functioncall

It is recommended to avoid functioncall in the for() statement.

<?php

// Fastest way
$nb = count($array);
for($i = 0; $i < $nb; ++$i) {
    doSomething($i);
}

// Same as above, but slow
for($i = 0; $i < count($array); ++$i) {
    doSomething($i);
}

// Same as above, but slow
foreach($portions as &$portion) {
    // here, array_sum() doesn't depends on the $grade. It should be out of the loop
    $portion = $portion / array_sum($portions);
}

$total = array_sum($portion);
foreach($portion as &$portion) {
    $portion = $portion / $total;
}

?>

This is true with any kind of functioncall that returns the same value throughout the loop.

Short name Structures/ForWithFunctioncall
Themes Performances, Top10
Severity Minor
Time To Fix Slow (1 hour)
ClearPHP no-functioncall-in-loop

9.184. Foreach Don’t Change Pointer

foreach loops use their own internal cursor.

A foreach loop won’t change the internal pointer of the array, as it works on a copy of the source. Hence, applying array pointer’s functions such as current() or next() to the source array won’t have the same behavior in PHP 5 than PHP 7.

This only applies when a foreach() by reference is used.

<?php

$numbers = range(1, 10);
next($numbers);
foreach($numbers as &$number){
    print $number;
    print current($numbers).\n; // Always
}

?>

See also foreach no longer changes the internal array pointer and foreach.

Short name Php/ForeachDontChangePointer
Themes CompatibilityPHP70
Php Version With PHP 7.0 and older
Severity Major
Time To Fix Slow (1 hour)

9.185. Foreach On Object

Foreach on object looks like a typo. This is particularly true when both object and member are variables.

Foreach on an object member is a legit PHP syntax, though it is very rare : blind variables rarely have to be securing in an object to be processed.

<?php

// Looks suspicious
foreach($array as $o -> $b) {
    doSomething();
}

// This is the real thing
foreach($array as $o => $b) {
    doSomething();
}

?>
Short name Php/ForeachObject
Themes Analyze
Severity Major
Time To Fix Instant (5 mins)

9.186. Foreach Reference Is Not Modified

Foreach statement may loop using a reference, especially when the loop has to change values of the array it is looping on.

In the spotted loop, reference are used but never modified. They may be removed.

<?php

$letters = range('a', 'z');

// $letter is not used here
foreach($letters as &$letter) {
    $alphabet .= $letter;
}

// $letter is actually used here
foreach($letters as &$letter) {
    $letter = strtoupper($letter);
}

?>

9.186.1. Suggestions

  • Remove the reference from the foreach
  • Actually modify the content of the reference
Short name Structures/ForeachReferenceIsNotModified
Themes Analyze
Severity Minor
Time To Fix Quick (30 mins)
Examples Dolibarr

9.187. Foreach With list()

Foreach loops have the ability to use list as blind variables. This syntax assign directly array elements to various variables.

PHP 5.5 introduced the usage of list in foreach() loops. Until PHP 7.1, it was not possible to use non-numerical arrays as list() wouldn’t support string-indexed arrays.

<?php
    // PHP 5.5 and later, with numerically-indexed arrays
    foreach($array as list($a, $b)) {
        // do something
    }


    // PHP 7.1 and later, with arrays
    foreach($array as list('col1' => $a, 'col3' => $b)) { // 'col2 is ignored'
        // do something
    }
?>

Previously, it was compulsory to extract() the data from the blind array :

<?php
    foreach($array as $c) {
        list($a, $b) = $c;
        // do something
    }
?>

See also The list function & practical uses of array destructuring in PHP.

Short name Structures/ForeachWithList
Themes CompatibilityPHP53, CompatibilityPHP54
Php Version With PHP 5.5 and more recent
Severity Minor
Time To Fix Slow (1 hour)

9.188. Forgotten Interface

The following classes have been found implementing an interface’s methods, though it doesn’t explicitly implements this interface. This may have been forgotten.

<?php

interface i {
    function i();
}

// i is not implemented and declared
class foo {
    function i() {}
    function j() {}
}

// i is implemented and declared
class foo implements i {
    function i() {}
    function j() {}
}

?>

See also Could Use Trait.

9.188.1. Suggestions

  • Mention interfaces explicitly whenever possible
Short name Interfaces/CouldUseInterface
Themes Analyze
Severity Minor
Time To Fix Quick (30 mins)

9.189. Forgotten Thrown

An exception is instantiated, but not thrown.

<?php

class MyException extends \Exception { }

if ($error !== false) {
    // This looks like 'throw' was omitted
    new MyException();
}

?>

9.189.1. Suggestions

  • Remove the throw expression
  • Add the new to the throw expression
Short name Exceptions/ForgottenThrown
Themes Analyze
Severity Major
Time To Fix Instant (5 mins)

9.190. Forgotten Visibility

Some classes elements (property, method, constant) are missing their explicit visibility.

By default, it is public. It should at least be mentioned as public, or may be reviewed as protected or private.

Class constants support also visibility since PHP 7.1.

final, static and abstract are not counted as visibility. Only public, private and protected. The PHP 4 var keyword is counted as undefined.

Traits, classes and interfaces are checked.

<?php

// Explicit visibility
class X {
    protected sconst NO_VISIBILITY_CONST = 1; // For PHP 7.2 and later

    private $noVisibilityProperty = 2;

    public function Method() {}
}

// Missing visibility
class X {
    const NO_VISIBILITY_CONST = 1; // For PHP 7.2 and later

    var $noVisibilityProperty = 2; // Only with var

    function NoVisibilityForMethod() {}
}

?>

See also Visibility and Understanding The Concept Of Visibility In Object Oriented PHP.

9.190.1. Suggestions

  • Always add explicit visibility to methods and constants in a class
  • Always add explicit visibility to properties in a class, after PHP 7.4
Short name Classes/NonPpp
Themes Analyze
Severity Minor
Time To Fix Instant (5 mins)
ClearPHP always-have-visibility
Examples FuelCMS, LiveZilla

9.191. Forgotten Whitespace

Forgotten whitespaces only bring misery to the code.

White spaces have been left at either end of a file : before the PHP opening tag, or after the closing tag.

Usually, such whitespaces are forgotten, and may end up summoning the infamous ‘headers already sent’ error. It is better to remove them.

<?php
    // This script has no forgotten whitespace, not at the beginning
    function foo() {}

    // This script has no forgotten whitespace, not at the end
?>

See also How to fix Headers already sent error in PHP.

9.191.1. Suggestions

  • Remove all whitespaces before and after a script. This doesn’t apply to template, which may need to use those spaces.
  • Remove the final tag, to prevent any whitespace to be forgotten at the end of the file. This doesn’t apply to the opening PHP tag, which is always necessary.
Short name Structures/ForgottenWhiteSpace
Themes Analyze
Severity Minor
Time To Fix Instant (5 mins)

9.192. Fully Qualified Constants

Constants defined with their namespace.

When defining constants with define() function, it is possible to include the actual namespace :

<?php

define('a\b\c', 1);

?>

However, the name should be fully qualified without the initial . Here, abc constant will never be accessible as a namespace constant, though it will be accessible via the constant() function.

Also, the namespace will be absolute, and not a relative namespace of the current one.

9.192.1. Suggestions

  • Drop the initial when creating constants with define() : for example, use trim($x, ‘’), which removes anti-slashes before and after.
Short name Namespaces/ConstantFullyQualified
Themes Analyze
Severity Minor
Time To Fix Slow (1 hour)

9.193. Function Subscripting

It is possible to use the result of a methodcall directly as an array, without storing the result in a temporary variable.

This works, given that the method actually returns an array.

This syntax was not possible until PHP 5.4. Until then, it was compulsory to store the result in a variable first. Although this is now superfluous, it has been a standard syntax in PHP, and is still being used.

<?php

function foo() {
    return array(1 => 'a', 'b', 'c');
}

echo foo()[1]; // displays 'a';

// Function subscripting, the old way
function foo() {
    return array(1 => 'a', 'b', 'c');
}

$x = foo();
echo $x[1]; // displays 'a';

?>

Storing the result in a variable is still useful if the result is actually used more than once.

Short name Structures/FunctionSubscripting
Themes CompatibilityPHP53
Php Version With PHP 5.4 and more recent
Severity Minor
Time To Fix Instant (5 mins)

9.194. Function Subscripting, Old Style

Since PHP 5.4, it is now possible use function results as an array, and access directly its element :

<?php

function foo() {
    return array(1 => 'a', 'b', 'c');
}

echo foo()[1]; // displays 'a';

// Function subscripting, the old way
function foo() {
    return array(1 => 'a', 'b', 'c');
}

$x = foo();
echo $x[1]; // displays 'a';

?>

9.194.1. Suggestions

  • Skip the local variable and directly use the return value from the function
Short name Structures/FunctionPreSubscripting
Themes Suggestions
Php Version With PHP 5.4 and more recent
Severity Minor
Time To Fix Instant (5 mins)
Examples OpenConf

9.195. Functions Removed In PHP 5.4

Those functions were removed in PHP 5.4.

<?php

// Deprecated as of PHP 5.4.0
$link = mysql_connect('localhost', 'mysql_user', 'mysql_password');
$db_list = mysql_list_dbs($link);

while ($row = mysql_fetch_object($db_list)) {
     echo $row->Database . "\n";
}

?>

See also Deprecated features in PHP 5.4.x.

Short name Php/Php54RemovedFunctions
Themes CompatibilityPHP54
Php Version With PHP 5.4 and older
Severity Major
Time To Fix Quick (30 mins)

9.196. Functions Removed In PHP 5.5

Those functions were removed in PHP 5.5.

<?php

echo '<img src="' . $_SERVER['PHP_SELF'] .
     '?=' . php_logo_guid() . '" alt="PHP Logo !" />';

?>

See also Deprecated features in PHP 5.5.x.

Short name Php/Php55RemovedFunctions
Themes CompatibilityPHP55
Php Version With PHP 5.5 and older
Severity Major
Time To Fix Quick (30 mins)

9.197. Functions/BadTypehintRelay

9.197.1. Suggestions

Short name Functions/BadTypehintRelay
Themes Analyze
Severity Minor
Time To Fix Quick (30 mins)

9.198. Generator Cannot Return

Generators could not use return and yield at the same time. In PHP 7.0, generator can now use both of them.

<?php

// This is not allowed until PHP 7.0
function foo() {
    yield 1;
    return 'b';
}

?>

9.198.1. Suggestions

  • Remove the return
Short name Functions/GeneratorCannotReturn
Themes CompatibilityPHP53, CompatibilityPHP54, CompatibilityPHP55, CompatibilityPHP56
Severity Major
Time To Fix Quick (30 mins)

9.199. Getting Last Element

Getting the last element of an array relies on array_key_last().

array_key_last() was added in PHP 7.3. Before that,

<?php

$array = ['a' => 1, 'b' => 2, 'c' => 3];

// Best solutions, by far
$last = $array[array_key_last($array)];

// Best solutions, just as fast as each other
$last = $array[count($array) - 1];
$last = end($array);

// Bad solutions

// popping, but restoring the value.
$last = array_pop($array);
$array[] = $last;

// array_unshift would be even worse

// reversing array
$last = array_reverse($array)[0];

// slicing the array
$last = array_slice($array, -1)[0]',
$last = current(array_slice($array, -1));
);

?>
Short name Arrays/GettingLastElement
Themes Performances
Severity Minor
Time To Fix Instant (5 mins)

9.200. Global Inside Loop

The global keyword must be out of loops. It is evaluated each loop, slowing the whole process.

<?php

// Good idea, global is used once
global $total;
foreach($a as $b) {
    $total += $b;
}

// Bad idea, this is slow.
foreach($a as $b) {
    global $total;
    $total += $b;
}
?>
Short name Structures/GlobalOutsideLoop
Themes Performances

9.201. Global Usage

List usage of globals variables, with global keywords or direct access to $GLOBALS.

<?php
$a = 1; /* global scope */

function test()
{
    echo $a; /* reference to local scope variable */
}

test();

?>

It is recommended to avoid using global variables, at it makes it very difficult to track changes in values across the whole application.

See also Variable scope.

Short name Structures/GlobalUsage
Themes Analyze
Severity Minor
Time To Fix Slow (1 hour)
ClearPHP no-global

9.202. Group Use Declaration

The group use declaration is used in the code.

<?php

// Adapted from the RFC documentation
// Pre PHP 7 code
use some\name_space\ClassA;
use some\name_space\ClassB;
use some\name_space\ClassC as C;

use function some\name_space\fn_a;
use function some\name_space\fn_b;
use function some\name_space\fn_c;

use const some\name_space\ConstA;
use const some\name_space\ConstB;
use const some\name_space\ConstC;

// PHP 7+ code
use some\name_space\{ClassA, ClassB, ClassC as C};
use function some\name_space\{fn_a, fn_b, fn_c};
use const some\name_space\{ConstA, ConstB, ConstC};

?>

See also Group Use Declaration RFC and Using namespaces: Aliasing/Importing.

Short name Php/GroupUseDeclaration
Themes CompatibilityPHP53, CompatibilityPHP54, CompatibilityPHP55, CompatibilityPHP56
Severity Minor
Time To Fix Instant (5 mins)

9.203. Group Use Trailing Comma

The usage of a final empty slot in array() was allowed with use statements. This works in PHP 7.2 and more recent.

Although this empty instruction is ignored at execution, this allows for clean presentation of code, and short diff when committing in a VCS.

<?php

// Valid in PHP 7.2 and more recent.
use a\b\{c,
         d,
         e,
         f,
        };

// This won't compile in 7.1 and older.

?>

See also Trailing Commas In List Syntax and Revisit trailing commas in function arguments.

Short name Php/GroupUseTrailingComma
Themes CompatibilityPHP53, CompatibilityPHP70, CompatibilityPHP71, CompatibilityPHP54, CompatibilityPHP55, CompatibilityPHP56
Php Version With PHP 7.2 and more recent
Severity Major
Time To Fix Instant (5 mins)

9.204. Hardcoded Passwords

Hardcoded passwords in the code.

Hardcoding passwords is a bad idea. Not only it make the code difficult to change, but it is an information leak. It is better to hide this kind of information out of the code.

<?php

$ftp_server = '300.1.2.3';   // yes, this doesn't exists, it's an example
$conn_id = ftp_connect($ftp_server);

// login with username and password
$login_result = ftp_login($conn_id, 'login', 'password');

?>

See also 10 GitHub Security Best Practices and Git How-To: Remove Your Password from a Repository.

9.204.1. Suggestions

  • Remove all passwords from the code. Also, check for history if you are using a VCS.
Short name Functions/HardcodedPasswords
Themes Analyze, Security
Severity Major
Time To Fix Slow (1 hour)
ClearPHP no-hardcoded-credential

9.205. Hash Algorithms

There is a long but limited list of hashing algorithm available to PHP. The one found doesn’t seem to be existing.

<?php

// This hash has existed in PHP. Check with hash_algos() if it is available on your system.
echo hash('ripmed160', 'The quick brown fox jumped over the lazy dog.');

// This hash doesn't exist
echo hash('ripemd160', 'The quick brown fox jumped over the lazy dog.');

?>

See also hash_algos.

9.205.1. Suggestions

  • Use a hash algorithm that is available on several PHP versions
  • Fix the name of the hash algorithm
Short name Php/HashAlgos
Themes Analyze
Severity Major
Time To Fix Slow (1 hour)

9.206. Hash Algorithms Incompatible With PHP 5.3

List of hash algorithms incompatible with PHP 5.3.

<?php

// Compatible only with 5.3 and more recent
echo hash('md2', 'The quick brown fox jumped over the lazy dog.');

// Always compatible
echo hash('ripemd320', 'The quick brown fox jumped over the lazy dog.');

?>

See also hash_algos.

Short name Php/HashAlgos53
Themes CompatibilityPHP53, CompatibilityPHP70, CompatibilityPHP71, CompatibilityPHP54, CompatibilityPHP55, CompatibilityPHP56, CompatibilityPHP72
Severity Major
Time To Fix Slow (1 hour)

9.207. Hash Algorithms Incompatible With PHP 5.4/5.5

List of hash algorithms incompatible with PHP 5.4 and 5.5.

<?php

// Compatible only with 5.4 and more recent
echo hash('fnv132', 'The quick brown fox jumped over the lazy dog.');

// Always compatible
echo hash('ripemd320', 'The quick brown fox jumped over the lazy dog.');

?>

See also hash_algos.

Short name Php/HashAlgos54
Themes CompatibilityPHP54, CompatibilityPHP70, CompatibilityPHP71, CompatibilityPHP55, CompatibilityPHP56, CompatibilityPHP72
Php Version With PHP 5.4 and older
Severity Major
Time To Fix Slow (1 hour)

9.208. Hash Algorithms Incompatible With PHP 7.1-

List of hash algorithms incompatible with PHP 7.1 and more recent. At the moment of writing, this is compatible up to 7.3.

The hash algorithms were introduced in PHP 7.1.

<?php

// Compatible only with 7.1 and more recent
echo hash('sha512/224', 'The quick brown fox jumped over the lazy dog.');

// Always compatible
echo hash('ripemd320', 'The quick brown fox jumped over the lazy dog.');

?>

See also hash_algos.

Short name Php/HashAlgos71
Themes CompatibilityPHP53, CompatibilityPHP70, CompatibilityPHP54, CompatibilityPHP55, CompatibilityPHP56
Php Version With PHP 7.1 and older
Severity Major
Time To Fix Slow (1 hour)

9.209. Hash Will Use Objects

The ext/hash extension used resources, and is being upgraded to use resources.

<?php

// Post 7.2 code
    $hash = hash_init('sha256');
    if (!is_object($hash)) {
        trigger_error('error');
    }
    hash_update($hash, $message);

// Pre-7.2 code
    $hash = hash_init('md5');
    if (!is_resource($hash)) {
        trigger_error('error');
    }
    hash_update($hash, $message);

?>

See also Move ext/hash from resources to objects.

Short name Php/HashUsesObjects
Themes CompatibilityPHP72
Severity Major
Time To Fix Quick (30 mins)

9.210. Heredoc Delimiter

Heredoc and Nowdoc expressions may use a variety of delimiters.

There seems to be a standard delimiter in the code, and some exceptions : one or several forms are dominant (> 90%), while the others are rare.

The analyzed code has less than 10% of the rare delimiters. For consistency reasons, it is recommended to make them all the same.

Generally, one or two delimiters are used, with generic value. It is recommended to use a humanly readable delimiter : SQL, HTML, XML, GREMLIN, etc. This helps readability in the code.

<?php

echo <<<SQL
SELECT * FROM table1;
SQL;

echo <<<SQL
SELECT * FROM table2;
SQL;

echo <<<SQL
SELECT * FROM table3;
SQL;

echo <<<SQL
SELECT * FROM table4;
SQL;

echo <<<SQL
SELECT * FROM table5;
SQL;

echo <<<SQL
SELECT * FROM table11;
SQL;

echo <<<SQL
SELECT * FROM table12;
SQL;

echo <<<SQL
SELECT * FROM table13;
SQL;

// Nowdoc
echo <<<'SQL'
SELECT * FROM table14;
SQL;

echo <<<SQL
SELECT * FROM table15;
SQL;


echo <<<HEREDOC
SELECT * FROM table215;
HEREDOC;

?>
Short name Structures/HeredocDelimiterFavorite
Themes Coding Conventions

9.211. Hexadecimal In String

Mark strings that may be confused with hexadecimal.

Until PHP 7.0, PHP recognizes hexadecimal numbers inside strings, and converts them accordingly.

PHP 7.0 and until 7.1, converts the string to 0, silently.

PHP 7.1 and later, emits a ‘A non-numeric value encountered’ warning, and convert the string to 0.

<?php
    $a = '0x0030';
    print $a + 1;
    // Print 49

    $c = '0x0030zyc';
    print $c + 1;
    // Print 49

    $b = 'b0x0030';
    print $b + 1;
    // Print 0
?>
Short name Type/HexadecimalString
Themes CompatibilityPHP70, CompatibilityPHP71
Severity Major
Time To Fix Slow (1 hour)

9.212. Hidden Use Expression

The use expression for namespaces should always be at the beginning of the namespace block.

It is where everyone expect them, and it is less confusing than having them at various levels.

<?php

// This is visible
use A;

class B {}

// This is hidden
use C as D;

class E extends D {
    use traitT; // This is a use for a trait

    function foo() {
        // This is a use for a closure
        return function ($a) use ($b) {}
    }
}

?>

9.212.1. Suggestions

  • Group all uses together, at the beginning of the namespace or class
Short name Namespaces/HiddenUse
Themes Analyze
Severity Minor
Time To Fix Instant (5 mins)
Examples Tikiwiki, OpenEMR

9.213. Htmlentities Calls

htmlentities() and htmlspecialchars() are used to prevent injecting special characters in HTML code. As a bare minimum, they take a string and encode it for HTML.

The second argument of the functions is the type of protection. The protection may apply to quotes or not, to HTML 4 or 5, etc. It is highly recommended to set it explicitly.

The third argument of the functions is the encoding of the string. In PHP 5.3, it is ISO-8859-1, in 5.4, was UTF-8, and in 5.6, it is now default_charset, a php.ini configuration that has the default value of UTF-8. It is highly recommended to set this argument too, to avoid distortions from the configuration.

<?php
$str = 'A quote is <b>bold</b>';

// Outputs, without depending on the php.ini: A &#039;quote&#039; is &lt;b&gt;bold&lt;/b&gt;
echo htmlentities($str, ENT_QUOTES, 'UTF-8');

// Outputs, while depending on the php.ini: A quote is &lt;b&gt;bold&lt;/b&gt;
echo htmlentities($str);

?>

Also, note that arguments 2 and 3 are constants and string, respectively, and should be issued from the list of values available in the manual. Other values than those will make PHP use the default values.

See also htmlentities and htmlspecialchars.

9.213.1. Suggestions

  • Always use the third argument with htmlentities()
Short name Structures/Htmlentitiescall
Themes Analyze
Severity Major
Time To Fix Instant (5 mins)

9.214. Identical Conditions

These logical expressions contain members that are identical.

This means those expressions may be simplified.

<?php

// twice $a
if ($a || $b || $c || $a) {  }

// Hiding in parenthesis is bad
if (($a) ^ ($a)) {}

// expressions may be large
if ($a === 1 && 1 === $a) {}

?>

9.214.1. Suggestions

  • Merge the two structures into one unique test
  • Add extra expressions between the two structures
  • Nest the structures, to show that different attempts are made
Short name Structures/IdenticalConditions
Themes Analyze
Severity Critical
Time To Fix Quick (30 mins)
Examples WordPress, Dolibarr, Mautic

9.215. Identical Consecutive Expression

Identical consecutive expressions are worth being checked.

They may be a copy/paste with unmodified content. When the content has to be duplicated, it is recommended to avoid executing the expression again, and just access the cached result.

<?php

$current  = $array[$i];
$next     = $array[$i + 1];
$nextnext = $array[$i + 1]; // OOps, nextnext is wrong.

// Initialization
$previous = foo($array[1]); // previous is initialized with the first value on purpose
$next     = foo($array[1]); // the second call to foo() with the same arguments should be avoided
// the above can be rewritten as :
$next     = $previous; // save the processing.

for($i = 1; $i < 200; ++$i) {
    $next = doSomething();
}
?>
Short name Structures/IdenticalConsecutive
Themes Analyze
Severity Minor
Time To Fix Instant (5 mins)

9.216. Identical Methods

When the parent class and the child class have the same method, the child might drop it. This reduces code duplication.

Duplicate code in methods is often the results of code evolution, where a method was copied with the hierarchy, but the original wasn’t removed.

This doesn’t apply to private methods, which are reserved for one class.

<?php

class a {
    public function foo() {
        return rand(0, 100);
    }
}

class b extends a {
    public function foo() {
        return rand(0, 100);
    }
}

?>

9.216.1. Suggestions

  • Drop the method from the parent class, in particular if only one child uses the method.
  • Drop the method from the child class, in particular if there are several children class
  • Use an abstract method, and make sure every child has its own implementation
  • Modify one of the methods so they are different
Short name Classes/IdenticalMethods
Themes Analyze, ClassReview
Severity Minor
Time To Fix Quick (30 mins)

9.217. Identical On Both Sides

Operands should be different when comparing or making a logical combination. Of course, the value each operand holds may be identical. When the same operand appears on both sides of the expression, the result is know before execution.

<?php

// Trying to confirm consistency
if ($login == $login) {
    doSomething();
}

// Works with every operators
if ($object->login( ) !== $object->login()) {
    doSomething();
}

if ($sum >= $sum) {
    doSomething();
}

//
if ($mask && $mask) {
    doSomething();
}

if ($mask || $mask) {
    doSomething();
}

?>

9.217.1. Suggestions

  • Remove one of the alternative, and remove the logical link
  • Modify one of the alternative, and make it different from the other
Short name Structures/IdenticalOnBothSides
Themes Analyze
Severity Major
Time To Fix Quick (30 mins)
Examples phpMyAdmin, HuMo-Gen

9.218. If With Same Conditions

Successive If / then structures that have the same condition may be either merged or have one of the condition changed.

<?php

if ($a == 1) {
    doSomething();
}

if ($a == 1) {
    doSomethingElse();
}

// May be replaced by
if ($a == 1) {
    doSomething();
    doSomethingElse();
}

?>

Note that if the values used in the condition have been modified in the first if/then structure, the two distinct conditions may be needed.

<?php

// May not be merged
if ($a == 1) {
    // Check that this is really the situation
    $a = checkSomething();
}

if ($a == 1) {
    doSomethingElse();
}

?>

9.218.1. Suggestions

  • Merge the two conditions so the condition is used once.
  • Change one of the condition, so they are different
  • Make it obvious that the first condition is a try, preparing the normal conditions.
Short name Structures/IfWithSameConditions
Themes Analyze
Severity Major
Time To Fix Quick (30 mins)
Examples phpMyAdmin, Phpdocumentor

9.219. Iffectations

Affectations that appears in a condition.

Iffectations are a way to do both a test and an affectations. They may also be typos, such as if ($x = 3) { }, leading to a constant condition.

<?php

// an iffectation : assignation in a If condition
if($connexion = mysql_connect($host, $user, $pass)) {
    $res = mysql_query($connexion, $query);
}

// Iffectation may happen in while too.
while($row = mysql_fetch($res)) {
    $store[] = $row;
}

?>
Short name Structures/Iffectation
Themes Analyze
Severity Minor
Time To Fix Quick (30 mins)

9.220. Illegal Name For Method

PHP has reserved usage of methods starting with __ for magic methods. It is recommended to avoid using this prefix, to prevent confusions.

<?php

class foo{
    // Constructor
    function __construct() {}

    // Constructor's typo
    function __constructor() {}

    // Illegal function name, even as private
    private function __bar() {}
}

?>

See also Magic Methods.

9.220.1. Suggestions

  • Avoid method names starting with a double underscore : __
  • Use method visibilities to ensure that methods are only available to the current class or its children
Short name Classes/WrongName
Themes Analyze
Severity Major
Time To Fix Slow (1 hour)
Examples PrestaShop, Magento

9.221. Implement Is For Interface

With class heritage, implements should be used for interfaces, and extends with classes.

PHP defers the implements check until execution : the code in example does lint, but won,t run.

<?php

class x {
    function foo() {}
}

interface y {
    function foo();
}

// Use implements with an interface
class z implements y {}

// Implements is for an interface, not a class
class z implements x {}

?>

9.221.1. Suggestions

  • Create an interface from the class, and use it with the implements keyword
Short name Classes/ImplementIsForInterface
Themes Analyze
Severity Minor
Time To Fix Quick (30 mins)

9.222. Implemented Methods Are Public

Class methods that are defined in an interface must be public. They cannot be either private, nor protected.

This error is not reported by lint, but is reported at execution time.

<?php

interface i {
    function foo();
}

class X {
    // This method is defined in the interface : it must be public
    protected function foo() {}

    // other methods may be private
    private function bar() {}
}

?>

See also Interfaces and Interfaces - the next level of abstraction.

9.222.1. Suggestions

  • Make the implemented method public
Short name Classes/ImplementedMethodsArePublic
Themes Analyze
Severity Major
Time To Fix Instant (5 mins)

9.223. Implicit Global

Global variables, that are used in local scope with global keyword, but are not declared as global in the global scope. They may be mistaken with distinct values, while, in PHP, variables in the global scope are truly global.

<?php

// This is implicitely global
$implicitGlobal = 1;

global $explicitGlobal;
$explicitGlobal = 2;

foo();
echo $explicitFunctionGlobal;

function foo() {
    // This global is needed, but not the one in the global space
    global $implicitGlobal, $explicitGlobal, $explicitFunctionGlobal;

    // This won't be a global, as it must be 'global' in a function scope
    $notImplicitGlobal = 3;
    $explicitFunctionGlobal = 3;
}

?>
Short name Structures/ImplicitGlobal
Themes Analyze
Severity Minor
Time To Fix Quick (30 mins)

9.224. Implied If

It is confusing to emulate if/then with boolean operators.

It is possible to emulate a if/then structure by using the operators ‘and’ and ‘or’. Since optimizations will be applied to them : when the left operand of ‘and’ is false, the right one is not executed, as its result is useless; when the left operand of ‘or’ is true, the right one is not executed, as its result is useless;

However, such structures are confusing. It is easy to misread them as conditions, and ignore an important logic step.

<?php

// Either connect, or die
mysql_connect('localhost', $user, $pass) or die();

// Defines a constant if not found.
defined('SOME_CONSTANT') and define('SOME_CONSTANT', 1);

// Defines a default value if provided is empty-ish
// Warning : this is
$user = $_GET['user'] || 'anonymous';

?>

It is recommended to use a real ‘if then’ structures, to make the condition readable.

Short name Structures/ImpliedIf
Themes Analyze
Severity Major
Time To Fix Instant (5 mins)
ClearPHP no-implied-if

9.225. Implode One Arg

implode() may be called with one arg. It is recommended to avoid it.

Using two arguments makes it less surprising to new comers, and consistent with explode() syntax.

<?php

$array = range('a', 'c');

// empty string is the glue
print implode('', $array);

// only the array : PHP uses the empty string as glue.
// Avoid this
print implode($array);

?>

See also implode.

9.225.1. Suggestions

  • Add an empty string as first argument
Short name Php/ImplodeOneArg
Themes Suggestions
Severity Minor
Time To Fix Quick (30 mins)

9.226. Inclusion Wrong Case

Inclusion should follow exactly the case of included files and path. This prevents the infamous case-sensitive filesystem bug, where files are correctly included in a case-insensitive system, and failed to be when moved to production.

<?php

// There must exist a path called path/to and a file library.php with this case
include path/to/library.php;

// Error on the case, while the file does exist
include path/to/LIBRARY.php;

// Error on the case, on the PATH
include path/TO/library.php;

?>

See also include_once, about case sensitivity and inclusions.

9.226.1. Suggestions

  • Make the inclusion string identical to the file name.
  • Change the name of the file to reflect the actual inclusion. This is the best way when a naming convention has been set up for the project, and the file doesn’t adhere to it. Remember to change all other inclusion.
Short name Files/InclusionWrongCase
Themes Analyze
Severity Major
Time To Fix Slow (1 hour)

9.227. Incompatible Signature Methods

Methods should have the same signature when being overwritten.

The same signatures means the children class must have : + the same name + the same visibility or less restrictive + the same typehint or removed + the same default value or removed + a reference like its parent

This problem emits a fatal error, for abstract methods, or a warning error, for normal methods. Yet, it is difficult to lint, because classes are often stored in different files. As such, PHP do lint each file independently, as unknown parent classes are not checked if not present. Yet, when executing the code, PHP lint the actual code and may encounter a fatal error.

<?php

class a {
    public function foo($a = 1) {}
}

class ab extends a {
    // foo is overloaded and now includes a default value for $a
    public function foo($a) {}
}

?>

See also Object Inheritance.

9.227.1. Suggestions

  • Make signatures compatible again
Short name Classes/IncompatibleSignature
Themes Analyze, LintButWontExec
Severity Critical
Time To Fix Quick (30 mins)
Examples SuiteCrm

9.228. Incompilable Files

Files that cannot be compiled, and, as such, be run by PHP. Scripts are linted against various versions of PHP.

This is usually undesirable, as all code must compile before being executed. It may be that such files are not compilable because they are not yet ready for an upcoming PHP version.

<?php

// Can't compile this : Print only accepts one argument
print $a, $b, $c;

?>

Code that is incompilable with older PHP versions means that the code is breaking backward compatibility : good or bad is project decision.

When the code is used as a template for PHP code generation, for example at installation time, it is recommended to use a distinct file extension, so as to distinguish them from actual PHP code.

9.228.1. Suggestions

  • If this file is a template for PHP code, change the extension to something else than .php
  • Fix the syntax so it works with various versions of PHP
Short name Php/Incompilable
Themes Analyze
Severity Critical
Time To Fix Slow (1 hour)
ClearPHP no-incompilable
Examples xataface

9.229. Inconsistent Elseif

Chaining if/elseif requires a consistent string of conditions. The conditions are executed one after the other, and the conditions shouldn’t overlap.

This analysis reports chains of elseif that don’t share a common variable (or array, or property, etc.. ). As such, testing different conditions are consistent.

<?php

// $a is always common, so situations are mutually exclusive
if ($a === 1) {
    doSomething();
} else if ($a > 1) {
    doSomethingElse();
} else {
    doSomethingDefault();
}

// $a is always common, so situations are mutually exclusive
// although, it may be worth checking the consistency here
if ($a->b === 1) {
    doSomething();
} else if ($a->c > 1) {
    doSomethingElse();
} else {
    doSomethingDefault();
}

// if $a === 1, then $c doesn't matter?
// This happens, but then logic doesn't appear in the code.
if ($a === 1) {
    doSomething();
} else if ($c > 1) {
    doSomethingElse();
} else {
    doSomethingDefault();
}

?>
Short name Structures/InconsistentElseif
Themes Analyze
Severity Major
Time To Fix Slow (1 hour)

9.230. Indices Are Int Or String

Indices in an array notation such as $array['indice'] may only be integers or string.

Boolean, Null or float will be converted to their integer or string equivalent.

<?php
    $a = [true => 1,
          1.0  => 2,
          1.2  => 3,
          1    => 4,
          '1'  => 5,
          0.8  => 6,
          0x1  => 7,
          01   => 8,

          null  => 1,
          ''    => 2,

          false => 1,
          0     => 2,

          '0.8' => 3,
          '01'  => 4,
          '2a'  => 5
          ];

    print_r($a);

/*
The above displays
Array
(
    [1] => 8
    [0] => 2
    [] => 2
    [0.8] => 3
    [01] => 4
    [2a] => 5
)
*/
?>

Decimal numbers are rounded to the closest integer; Null is transtyped to ‘’ (empty string); true is 1 and false is 0; Integers in strings are transtyped, while partial numbers or decimals are not analyzed in strings.

As a general rule of thumb, only use integers or strings that don’t look like integers.

This analyzer may find constant definitions, when available.

Note also that PHP detects integer inside strings, and silently turn them into integers. Partial numbers and octals are not transformed.

<?php
    $a = [1      => 1,
          '2'    => 2,
          '011'  => 9, // octal number
          '11d'  => 11, // partial number
          ];

    var_dump($a);

/*
The above displays
array(4) {
  [1]=>
  int(1)
  [2]=>
  int(2)
  [011]=>
  int(9)
  [11d]=>
  int(11)
}*/
?>

See also Arrays syntax.

9.230.1. Suggestions

  • Do not use any type but string or integer
  • Force typecast the keys when building an array
Short name Structures/IndicesAreIntOrString
Themes Analyze
Severity Major
Time To Fix Quick (30 mins)
Examples Zencart, Mautic

9.231. Indirect Injection

Look for injections through indirect usage for GPRC values ($_GET, $_POST, $_REQUEST, $_COOKIE).

<?php

$a = $_GET['a'];
echo $a;

function foo($b) {
    echo $b;
}
foo($_POST['c']);

?>

9.231.1. Suggestions

  • Always validate incoming values before using them.
Short name Security/IndirectInjection
Themes Security
Severity Critical
Time To Fix Slow (1 hour)

9.232. Infinite Recursion

A method is calling itself, with unchanged arguments. This will probably repeat indefinitely.

This applies to recursive functions without any condition. This also applies to function which inject the incoming arguments, without modifications.

<?php

function foo($a, $b) {
    if ($a > 10) {
        return;
    }
    foo($a, $b);
}

function foo2($a, $b) {
    ++$a;   // $a is modified
    if ($a > 10) {
        return;
    }
    foo2($a, $b);
}

?>

9.232.1. Suggestions

  • Modify arguments before reinjecting them in the same method
  • Use different values when callling the same method
Short name Structures/InfiniteRecursion
Themes Analyze
Severity Major
Time To Fix Quick (30 mins)

9.233. Instantiating Abstract Class

PHP cannot instantiate an abstract class.

The classes are actually abstract classes, and should be derived into a concrete class to be instantiated.

<?php

abstract class Foo {
    protected $a;
}

class Bar extends Foo {
    protected $b;
}

// instantiating a concrete class.
new Bar();

// instantiating an abstract class.
// In real life, this is not possible also because the definition and the instantiation are in the same file
new Foo();

?>

See also Class Abstraction.

Short name Classes/InstantiatingAbstractClass
Themes Analyze
Severity Major
Time To Fix Quick (30 mins)

9.234. Insufficient Typehint

An argument is typehinted, but it actually calls methods that are not listed in the interface.

Classes may be implementing more methods than the one that are listed in the interface they also implements. This means that filtering objects with a typehint, but calling other methods will be solved at execution time : if the method is available, it will be used; if it is not, a fatal error is reported.

<?php

class x implements i {
    function methodI() {}
    function notInI() {}
}

interface i {
    function methodI();
}

function foo(i $x) {
    $x->methodI(); // this call is valid
    $x->notInI();  // this call is not garanteed
}
?>

Inspired by discussion with Brandon Savage.

9.234.1. Suggestions

  • Extend the interface with the missing called methods
  • Change the body of the function to use only the methods that are available in the interface
  • Change the used objects so they don’t depend on extra methods
Short name Functions/InsufficientTypehint
Themes Analyze
Severity Major
Time To Fix Quick (30 mins)

9.235. Integer As Property

It is backward incompatible to use integers are property names. This feature was introduced in PHP 7.2.

If the code must be compatible with previous versions, avoid casting arrays to object.

<?php

// array to object
$arr = [0 => 1];
$obj = (object) $arr;
var_dump(
    $obj,
    $obj->{'0'}, // PHP 7.2+ accessible
    $obj->{0} // PHP 7.2+ accessible

    $obj->{'b'}, // always been accessible
);
?>

See also PHP RFC: Convert numeric keys in object/array casts.

Short name Classes/IntegerAsProperty
Themes CompatibilityPHP53, CompatibilityPHP70, CompatibilityPHP71, CompatibilityPHP54, CompatibilityPHP55, CompatibilityPHP56
Php Version With PHP 7.2 and more recent
Severity Major
Time To Fix Slow (1 hour)

9.236. Integer Conversion

Comparing incoming variables to integer may lead to injection.

When comparing a variable to an integer, PHP applies type juggling, and transform the variable in an integer too. When the value converts smoothly to an integer, this means the validation may pass and yet, the value may carry an injection.

<?php

// This is safer
if ($_GET['x'] === 2) {
    echo $_GET['x'];
}

// Using (int) for validation and display
if ((int) $_GET['x'] === 2) {
    echo (int) $_GET['x'];
}

// This is an injection
if ($_GET['x'] == 2) {
    echo $_GET['x'];
}

// This is unsafe, as $_GET['x']  is tester as an integer, but echo'ed raw
if ((int) $_GET['x'] === 2) {
    echo $_GET['x'];
}

?>

This analysis spots situations where an incoming value is compared to an integer. The usage of the validated value is not considered.

See also Type Juggling Authentication Bypass Vulnerability in CMS Made Simple, PHP STRING COMPARISON VULNERABILITIES and PHP Magic Tricks: Type Juggling.

9.236.1. Suggestions

Short name Security/IntegerConversion
Themes Security
Severity Major
Time To Fix Quick (30 mins)

9.237. Interpolation

The following strings contain variables that are will be replaced. However, the following characters are ambiguous, and may lead to confusion.

<?php

class b {
    public $b = 'c';
    function __toString() { return __CLASS__; }
}
$x = array(1 => new B());

// -> after the $x[1] looks like a 2nd dereferencing, but it is not.
print $x[1]->b;
// displays : b->b

print {$x[1]->b};
// displays : c

?>

It is advised to add curly brackets around those structures to make them non-ambiguous.

See also Double quoted.

Short name Type/StringInterpolation
Themes Coding Conventions
Severity Minor
Time To Fix Quick (30 mins)

9.238. Invalid Class Name

The spotted classes are used with a different case than their definition. While PHP accepts this, it makes the code harder to read.

It may also be a violation of coding conventions.

<?php

// This use statement has wrong case for origin.
use Foo as X;

// Definition of the class
class foo {}

// Those instantiations have wrong case
new FOO();
new X();

?>

See also PHP class name constant case sensitivity and PSR-11.

9.238.1. Suggestions

  • Match the defined class name with the called name
Short name Classes/WrongCase
Themes Coding Conventions, Analyze
Severity Minor
Time To Fix Instant (5 mins)
Examples WordPress

9.239. Invalid Constant Name

There is a naming convention for PHP constants names.

According to PHP’s manual, constant names, ‘ A valid constant name starts with a letter or underscore, followed by any number of letters, numbers, or underscores.’.

Constant, must follow this regex : /[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*/.

In particular when defined using define() function, no error is produced. When using const, on the other hand, the

<?php

define('+3', 1); // wrong constant!

echo constant('+3'); // invalid constant access

?>

See also Constants.

9.239.1. Suggestions

  • Change constant name
Short name Constants/InvalidName
Themes Analyze
Severity Major
Time To Fix Quick (30 mins)
Examples OpenEMR

9.240. Invalid Octal In String

Any octal sequence inside a string can’t be beyond 7. Those will be a fatal error at parsing time.

This is true, starting with PHP 7.1. In PHP 7.0 and older, those sequences were silently adapted (divided by 0).

<?php

// Emit no error in PHP 7.1
echo 0; // @

// Emit an error in PHP 7.1
echo 0; // @

?>

See also Integers.

Short name Type/OctalInString
Themes CompatibilityPHP71
Php Version With PHP 7.1 and older
Severity Major
Time To Fix Quick (30 mins)

9.241. Invalid Pack Format

Some characters are invalid in a pack() format string.

pack() and unpack() accept the following format specifiers : aAhHcCsSnviIlLNVqQJPfgGdeExXZ.

unpack() also accepts a name after the format specifier and an optional quantifier.

All other situations is not a valid, and produces a warning : pack(): Type t: unknown format code

<?php
    $binarydata = pack(nvc*, 0x1234, 0x5678, 65, 66);

    // the first unsigned short is stored as 'first'. The next matches are names with numbers.
    $res = unpack('nfirst/vc*', $binarydata);
?>

Check pack() documentation for format specifiers that were introduced in various PHP version, namely 7.0, 7.1 and 7.2.

See also pack and unpack.

9.241.1. Suggestions

  • Fix the packing format with correct values
Short name Structures/InvalidPackFormat
Themes Analyze
Severity Major
Time To Fix Quick (30 mins)

9.242. Invalid Regex

The PCRE regex doesn’t compile. It isn’t a valid regex.

Several reasons may lead to this situation : syntax error, Unknown modifier, missing parenthesis or reference.

<?php

// valid regex
preg_match('/[abc]/', $string);

// invalid regex (missing terminating ] for character class
preg_match('/[abc/', $string);

?>

Regex are check with the Exakat version of PHP.

Dynamic regex are only checked for simple values. Dynamic values may eventually generate a compilation error.

9.242.1. Suggestions

  • Fix the regex before running it
Short name Structures/InvalidRegex
Themes Analyze
Severity Major
Time To Fix Quick (30 mins)
Examples SugarCrm

9.243. Is Actually Zero

This addition actually may be simplified because one term is actually negated by another.

This kind of error happens when the expression is very large : the more terms are included, the more chances are that some auto-annihilation happens.

This error may also be a simple typo : for example, calculating the difference between two consecutive terms.

<?php

// This is quite obvious
$a = 2 - 2;

// This is obvious too. This may be a typo-ed difference between two consecutive terms.
// Could have been $c = $fx[3][4] - $fx[3][3] or $c = $fx[3][5] - $fx[3][4];
$c = $fx[3][4] - $fx[3][4];

// This is less obvious
$a = $b[3] - $c + $d->foo(1,2,3) + $c + $b[3];

?>

9.243.1. Suggestions

  • Clean the code and remove the null sum
  • Fix one of the variable : this expression needs another variable here
  • When adding differences, calculate the difference in a temporary variable first.
Short name Structures/IsZero
Themes Analyze
Severity Minor
Time To Fix Instant (5 mins)
Examples Dolibarr, SuiteCrm

9.244. Isset Multiple Arguments

isset() may be used with multiple arguments and acts as a AND.

<?php

// isset without and
if (isset($a, $b, $c)) {
    // doSomething()
}

// isset with and
if (isset($a) && isset($b) && isset($c)) {
    // doSomething()
}

?>

See also Isset <http://www.php.net/`isset>`_.

9.244.1. Suggestions

  • Merge all isset() calls into one
Short name Php/IssetMultipleArgs
Themes Suggestions
Severity Minor
Time To Fix Instant (5 mins)
Examples ThinkPHP, LiveZilla

9.245. Isset() On The Whole Array

Isset() works quietly on a whole array. There is no need to test all previous index before testing for the target index.

<?php

// Straight to the point
if (isset($a[1]['source'])) {
    // Do something with $a[1]['source']
}

// Doing too much work
if (isset($a) && isset($a[1]) && isset($a[1]['source'])) {
    // Do something with $a[1]['source']
}

?>

There is a gain in readability, by avoiding long and hard to read logical expression, and reducing them in one simple isset call.

There is a gain in performances by using one call to isset, instead of several, but it is a micro-optimization.

See also Isset <http://www.php.net/`isset>`_.

9.245.1. Suggestions

  • Remove all unnecessary calls to isset()
Short name Performances/IssetWholeArray
Themes Suggestions, Performances
Severity Minor
Time To Fix Instant (5 mins)
Examples Tine20, ExpressionEngine

9.246. Joining file()

Use file() to read lines separately.

Applying join(‘’, ) or implode(‘’, ) to the result of file() provides the same results than using file_get_contents(), but at a higher cost of memory and processing.

If the delimiter is not ‘’, then implode() and file() are a better solution than file_get_contents() and str_replace() or nl2br().

<?php

// memory intensive
$content = file_get_contents('path/to/file.txt');

// memory and CPU intensive
$content = join('', file('path/to/file.txt'));

// Consider reading the data line by line and processing it along the way,
// to save memory
$fp = fopen('path/to/file.txt', 'r');
while($line = fget($fp)) {
    // process a line
}
fclose($fp);

?>

Always use file_get_contents() to get the content of a file as a string. Consider using readfile() to echo the content directly to the output.

See also file_get_contents and file.

9.246.1. Suggestions

  • Use file_get_contents() instead of implode(file()) to read the whole file at once.
  • Use readfile() to echo the content to stdout at once.
  • Use fopen() to read the lines one by one, generator style.
Short name Performances/JoinFile
Themes Performances
Severity Minor
Time To Fix Quick (30 mins)
Examples WordPress, SPIP, ExpressionEngine, PrestaShop

9.247. List Short Syntax

Usage of short syntax version of list().

<?php

// PHP 7.1 short list syntax
// PHP 7.1 may also use key => value structures with list
[$a, $b, $c] = ['2', 3, '4'];

// PHP 7.0 list syntax
list($a, $b, $c) = ['2', 3, '4'];

?>
Short name Php/ListShortSyntax
Themes CompatibilityPHP53, CompatibilityPHP70, CompatibilityPHP54, CompatibilityPHP55, CompatibilityPHP56
Php Version With PHP 7.1 and more recent
Severity Major
Time To Fix Quick (30 mins)

9.248. List With Appends

List() behavior has changed in PHP 7.0 and it has impact on the indexing when list is used with the [] operator.

<?php

$x = array();
list($x[], $x[], $x[]) = [1, 2, 3];

print_r($x);

?>

In PHP 7.0, results are ::

Array
(
    [0] => 1
    [1] => 2
    [2] => 3
)

In PHP 5.6, results are ::

Array
(
    [0] => 3
    [1] => 2
    [2] => 1
)
Short name Php/ListWithAppends
Themes CompatibilityPHP70
Severity Minor
Time To Fix Slow (1 hour)

9.249. List With Keys

Setting keys when using list() is a PHP 7.1 feature.

<?php

// PHP 7.1 and later only
list('a' => $a, 'b' => $b) = ['b' => 1, 'c' => 2, 'a' => 3];

?>
Short name Php/ListWithKeys
Themes CompatibilityPHP53, CompatibilityPHP70, CompatibilityPHP54, CompatibilityPHP55, CompatibilityPHP56
Php Version With PHP 7.1 and more recent
Severity Major
Time To Fix Quick (30 mins)

9.250. List With Reference

Support for references in list calls is not backward compatible with older versions of PHP. The support was introduced in PHP 7.3.

<?php

$a = [1,2,3];

[$c, $d, $e] = $a;

$d++;
echo $a[2]; // Displays 4

?>

See also list() Reference Assignment.

Short name Php/ListWithReference
Themes CompatibilityPHP53, CompatibilityPHP70, CompatibilityPHP71, CompatibilityPHP72, CompatibilityPHP54, CompatibilityPHP55, CompatibilityPHP56
Php Version With PHP 7.3 and more recent
Severity Major
Time To Fix Slow (1 hour)

9.251. Locally Unused Property

Those properties are defined in a class, and this class doesn’t have any method that makes use of them.

While this is syntactically correct, it is unusual that defined resources are used in a child class. It may be worth moving the definition to another class, or to move accessing methods to the class.

<?php

class foo {
    public $unused, $used;// property $unused is never used in this class

    function bar() {
        $this->used++; // property $used is used in this method
    }
}

class foofoo extends foo {
    function bar() {
        $this->unused++; // property $unused is used in this method, but defined in the parent class
    }
}

?>
Short name Classes/LocallyUnusedProperty
Themes Dead code
Severity Minor
Time To Fix Slow (1 hour)

9.252. Logical Mistakes

Avoid logical mistakes within long expressions.

Sometimes, the logic is not what it seems. It is important to check the actual impact of every part of the logical expression. Do not hesitate to make a table with all possible cases. If those cases are too numerous, it may be time to rethink the whole expression.

<?php

// Always true
if ($a != 1 || $a != 2) { }

// $a == 1 is useless
if ($a == 1 || $a != 2) {}

// Always false
if ($a == 1 && $a == 2) {}

// $a != 2 is useless
if ($a == 1 && $a != 2) {}

?>

Based on article from Andrey Karpov Logical Expressions in C/C++. Mistakes Made by Professionals

9.252.1. Suggestions

  • Change the expressions for them to have a real meaning
Short name Structures/LogicalMistakes
Themes Analyze
Severity Critical
Time To Fix Quick (30 mins)
Examples Dolibarr, Cleverstyle

9.253. Logical Operators Favorite

PHP has two sets of logical operators : letters (and, or, xor) and chars (&&, ||, ^).

The analyzed code has less than 10% of one of the two sets : for consistency reasons, it is recommended to make them all the same.

Warning : the two sets of operators have different precedence levels. Using and or && is not exactly the same, especially and not only, when assigning the results to a variable.

<?php

$a1 = $b and $c;
$a1 = $b and $c;
$a1 = $b and $c;
$a1 = $b or $c;
$a1 = $b OR $c;
$a1 = $b and $c;
$a1 = $b and $c;
$a1 = $b and $c;
$a1 = $b or $c;
$a1 = $b OR $c;
$a1 = $b ^ $c;

?>

Using and or && are also the target of other analysis.

See also Logical Operators and Operators Precedence.

Short name Php/LetterCharsLogicalFavorite
Themes Top10

9.254. Logical Should Use Symbolic Operators

Logical operators come in two flavors : and / &&, || / or, ^ / xor. However, they are not exchangeable, as && and and have different precedence.

<?php

// Avoid lettered operator, as they have lower priority than expected
$a = $b and $c;
// $a === 3 because equivalent to ($a = $b) and $c;

// safe way to write the above :
$a = ($b and $c);

$a = $b && $c;
// $a === 1

?>

It is recommended to use the symbol operators, rather than the letter ones.

See also Logical Operators.

9.254.1. Suggestions

  • Change the letter operators to the symbol one : and => &&, or => ||, xor => ^. Review the new expressions as processing order may have changed.
  • Add parenthesis to make sure that the order is the expected one
Short name Php/LogicalInLetters
Themes Analyze, Suggestions, Top10
Severity Minor
Time To Fix Instant (5 mins)
ClearPHP no-letter-logical
Examples Cleverstyle, OpenConf

9.255. Logical To in_array

Multiples exclusive comparisons may be replaced by in_array().

in_array() makes the alternatives more readable, especially when the number of alternatives is large. In fact, the list of alternative may even be set in a variable, and centralized for easier management.

Even two ‘or’ comparisons are slower than using a in_array() call. More calls are even slower than just two. This is a micro-optimisation : speed gain is low, and marginal. Code centralisation is a more significant advantage.

<?php

// Set the list of alternative in a variable, property or constant.
$valid_values = array(1, 2, 3, 4);
if (in_array($a, $valid_values) ) {
    // doSomething()
}

if ($a == 1 || $a == 2 || $a == 3 || $a == 4) {
    // doSomething()
}

// in_array also works with strict comparisons
if (in_array($a, $valid_values, true) ) {
    // doSomething()
}

if ($a === 1 || $a === 2 || $a === 3 || $a === 4) {
    // doSomething()
}

?>

See also in_array().

9.255.1. Suggestions

  • Replace the list of comparisons with a in_array() call
Short name Performances/LogicalToInArray
Themes Analyze
Severity Minor
Time To Fix Quick (30 mins)
Examples Zencart

9.256. Lone Blocks

Any grouped code without a commanding structure is useless.

Blocks are compulsory when defining a structure, such as a class or a function. They are most often used with flow control instructions, like if then or switch.

Blocks are also valid syntax that group several instructions together, though they have no effect at all, except confuse the reader. Most often, it is a ruin from a previous flow control instruction, whose condition was removed or commented. They should be removed.

<?php

    // Lone block
    //foreach($a as $b)
    {
        $b++;
    }
?>

9.256.1. Suggestions

  • Remove the useless curly brackets
Short name Structures/LoneBlock
Themes Analyze
Severity Minor
Time To Fix Instant (5 mins)
Examples ThinkPHP, Tine20

9.257. Long Arguments

Long arguments should be put in variable, to preserve readability.

When literal arguments are too long, they break the hosting structure by moving the next argument too far on the right. Whenever possible, long arguments should be set in a local variable to keep the readability.

<?php

// Now the call to foo() is easier to read.
$reallyBigNumber = <<<BIGNUMBER
123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890
BIGNUMBER
foo($reallyBigNumber, 2, '12345678901234567890123456789012345678901234567890');

// where are the next arguments ?
foo('123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890', 2, '123456789012345678901234567890123456789012345678901234567890');

// This is still difficult to read
foo(<<<BIGNUMBER
123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890
BIGNUMBER
, 2, '123456789012345678901234567890123456789012345678901234567890');

?>

Literal strings and heredoc strings, including variables, that are over 50 chars longs are reported here.

9.257.1. Suggestions

  • Put the long arguments in a separate variable, and use the variable in the second expression, reducing its total length
Name Default Type Description
codeTooLong 100 integer Minimum size of a functioncall or a methodcall to be considered too long.
Short name Structures/LongArguments
Themes Analyze
Severity Minor
Time To Fix Quick (30 mins)
Examples Cleverstyle, Contao

9.258. Lost References

Either avoid references, or propagate them correctly.

When assigning a referenced variable with another reference, the initial reference is lost, while the intend was to transfer the content.

<?php

function foo(&$lostReference, &$keptReference)
{
    $c = 'c';

    // $lostReference was a reference, but now, it is another
    $lostReference =& $c;
    // $keptReference was a reference : now it contains the actual value
    $keptReference = $c;
}

$bar = 'bar';
$bar2 = 'bar';
foo($bar, $bar2);

//displays bar c, instead of bar bar
print $bar. ' '.$bar2;

?>

Do not reassign a reference with another reference. Assign new content to the reference to change its value.

9.258.1. Suggestions

  • Always assign new value to an referenced argument, and don’t reassign a new reference
Short name Variables/LostReferences
Themes Analyze
Severity Major
Time To Fix Quick (30 mins)
Examples WordPress

9.259. Magic Visibility

The class magic methods must have public visibility and cannot be static.

<?php

class foo{
    // magic method must bt public and non-static
    public static function __clone($name) {    }

    // magic method can't be private
    private function __get($name) {    }

    // magic method can't be protected
    private function __set($name, $value) {    }

    // magic method can't be static
    public static function __isset($name) {    }
}

?>

See also Magic methods.

Short name Classes/toStringPss
Themes CompatibilityPHP70
Php Version With PHP 5.4 and older
Severity Major
Time To Fix Quick (30 mins)

9.260. Make Global A Property

Calling global (or $GLOBALS) in methods is slower and less testable than setting the global to a property, and using this property.

Using properties is slightly faster than calling global or $GLOBALS, though the gain is not important.

Setting the property in the constructor (or in a factory), makes the class easier to test, as there is now a single point of configuration.

<?php

// Wrong way
class fooBad {
    function x() {
        global $a;
        $a->do();
        // Or $GLOBALS['a']->do();
    }
}

class fooGood {
    private $bar = null;

    function __construct() {
        global $bar;
        $this->bar = $bar;
        // Even better, do this via arguments
    }

    function x() {
        $this->a->do();
    }
}

?>

9.260.1. Suggestions

  • Avoid using global variables, and use properties instead
  • Remove the usage of these global variables
Short name Classes/MakeGlobalAProperty
Themes Analyze
Severity Minor
Time To Fix Slow (1 hour)

9.261. Make Magic Concrete

Speed up execution by replacing magic calls by concrete properties.

Magic properties are managed dynamically, with __get``and ``__set. They replace property access by a methodcall, and they are much slower than the first.

When a property name is getting used more often, it is worth creating a concrete property, and skip the method call. The threshold for ‘magicMemberUsage’ is 1, by default.

<?php

class x {
    private $values = array('a' => 1,
                            'b' => 2);

    function __get($name) {
        return $this->values[$name] ?? '';
    }
}

$x = new x();
// Access to 'a' is repeated in the code, at least 'magicMemberUsage' time (cf configuration below)
echo $x->a;

?>

See also Memoize MagicCall.

9.261.1. Suggestions

  • Make frequently used properties concrete; keep the highly dynamic as magic
Name Default Type Description
magicMemberUsage 1 integer Minimal number of magic member usage across the code, to trigger a concrete property.
Short name Classes/MakeMagicConcrete
Themes Performances
Severity Minor
Time To Fix Quick (30 mins)

9.262. Make One Call With Array

Avoid calling the same function several times by batching the calls with arrays.

Calling the same function to chain modifications tends to be slower than calling the same function with all the transformations at the same time. Some PHP functions accept scalars or arrays, and using the later is more efficient.

<?php

$string = 'abcdef';

//str_replace() accepts arrays as arguments
$string = str_replace( ['a', 'b', 'c'],
                       ['A', 'B', 'C'],
                       $string);

// Too many calls to str_replace
$string = str_replace( 'a', 'A', $string);
$string = str_replace( 'b', 'B', $string);
$string = str_replace( 'c', 'C', $string);

// Too many nested calls to str_replace
$string = str_replace( 'a', 'A', str_replace( 'b', 'B', str_replace( 'c', 'C', $string)));

?>

Potential replacements :

Function Replacement
str_replace() str_ireplace() substr_replace() preg_replace() preg_replace_callback() str_replace() str_replace() substr_replace() preg_replace() preg_replace_callback_array()
<?php
$subject = 'Aaaaaa Bbb';


//preg_replace_callback_array() is better than multiple preg_replace_callback :
preg_replace_callback_array(
    [
        '~[a]+~i' => function ($match) {
            echo strlen($match[0]), ' matches for a found', PHP_EOL;
        },
        '~[b]+~i' => function ($match) {
            echo strlen($match[0]), ' matches for b found', PHP_EOL;
        }
    ],
    $subject
);

$result = preg_replace_callback('~[a]+~i', function ($match) {
            echo strlen($match[0]), ' matches for a found', PHP_EOL;
        }, $subject);

$result = preg_replace_callback('~[b]+~i', function ($match) {
            echo strlen($match[0]), ' matches for b found', PHP_EOL;
        }, $subject);

//str_replace() accepts arrays as arguments
$string = str_replace( ['a', 'b', 'c'],
                       ['A', 'B', 'C'],
                       $string);

// Too many calls to str_replace
$string = str_replace( 'a', 'A');
$string = str_replace( 'b', 'B');
$string = str_replace( 'c', 'C');

?>

9.262.1. Suggestions

  • use str_replace() with arrays as arguments.
  • use preg_replace() with arrays as arguments.
  • use preg_replace_callback() for merging multiple complex calls.
Short name Performances/MakeOneCall
Themes Performances
Severity Major
Time To Fix Quick (30 mins)
Examples HuMo-Gen, Edusoho

9.263. Malformed Octal

Those numbers starts with a 0, so they are using the PHP octal convention. Therefore, one can’t use 8 or 9 figures in those numbers, as they don’t belong to the octal base. The resulting number will be truncated at the first erroneous figure. For example, 090 is actually 0, and 02689 is actually 22.

<?php

// A long way to write 0 in PHP 5
$a = 0890;

// A fatal error since PHP 7

?>

Also, note that very large octal, usually with more than 21 figures, will be turned into a real number and undergo a reduction in precision.

See also Integers.

Short name Type/MalformedOctal
Themes CompatibilityPHP53, CompatibilityPHP54, CompatibilityPHP55, CompatibilityPHP56
Php Version With PHP 7.0 and older
Severity Minor
Time To Fix Instant (5 mins)

9.264. Memoize MagicCall

Cache calls to magic methods in local variable. Local cache is faster than calling again the magic method as soon as the second call, provided that the value hasn’t changed.

__get is slower, as it turns a simple member access into a full method call.

<?php

class x {
    private $values = array();

    function __get($name) {
        return $this->values[$name];
    }
    // more code to set values to this class
}

function foo(x $b) {
    $a = $b->a;
    $c = $b->c;

    $d = $c;     // using local cache, no new access to $b->__get($name)
    $e = $b->a;  // Second access to $b->a, through __get
}

function bar(x $b) {
    $a = $b->a;
    $c = $b->c;

    $b->bar2(); // this changes $b->a and $b->c, but we don't see it

    $d = $b->c;
    $e = $b->a;  // Second access to $b->a, through __get
}

?>

The caching is not possible if the processing of the object changes the value of the property.

See also __get performance questions with PHP, Make Magic Concrete and Benchmarking magic.

9.264.1. Suggestions

  • Cache the value in a local variable, and reuse that variable
  • Make the property concrete in the class, so as to avoid __get() altogether
Short name Performances/MemoizeMagicCall
Themes Analyze, ClassReview
Severity Minor
Time To Fix Quick (30 mins)

9.265. Method Collision Traits

Two or more traits are included in the same class, and they have methods collisions.

Those collisions should be solved with a use expression. When they are not, PHP stops execution with a fatal error : Trait method M has not been applied, because there are collisions with other trait methods on C.

<?php

trait A {
    public function A() {}
    public function M() {}
}

trait B {
    public function B() {}
    public function M() {}
}

class C {
    use  A, B;
}

class D {
    use  A, B{
        B::M insteadof A;
    };
}

?>

The code above lints, but doesn’t execute.

See also Traits.

Short name Traits/MethodCollisionTraits
Themes Analyze, LintButWontExec
Severity Critical
Time To Fix Quick (30 mins)

9.266. Method Could Be Private Method

The following methods are never used outside their class of definition. Given the analyzed code, they could be set as private.

<?php

class foo {
    public function couldBePrivate() {}
    public function cantdBePrivate() {}

    function bar() {
        // couldBePrivate is used internally.
        $this->couldBePrivate();
    }
}

class foo2 extends foo {
    function bar2() {
        // cantdBePrivate is used in a child class.
        $this->cantdBePrivate();
    }
}

//couldBePrivate() is not used outside
$foo = new foo();

//cantdBePrivate is used outside the class
$foo->cantdBePrivate();

?>

Note that dynamic properties (such as $x->$y) are not taken into account.

Short name Classes/CouldBePrivateMethod
Themes ClassReview
Severity Minor
Time To Fix Quick (30 mins)

9.267. Method Could Be Static

A method that doesn’t make any usage of $this could be turned into a static method.

While static methods are usually harder to handle, recognizing the static status is a first step before turning the method into a standalone function.

<?php

class foo {
    static $property = 1;

    // legit static method
    static function staticMethod() {
        return self::$property;
    }

    // This is not using $this, and could be static
    function nonStaticMethod() {
        return self::$property;
    }

    // This is not using $this nor self, could be a standalone function
    function nonStaticMethod() {
        return self::$property;
    }
}

?>

9.267.1. Suggestions

  • Make the method static
  • Make the method a standalone function
  • Make use of $this in the method : may be it was forgotten.
Short name Classes/CouldBeStatic
Themes none
Severity Minor
Time To Fix Quick (30 mins)
Examples FuelCMS, ExpressionEngine

9.268. Method Signature Must Be Compatible

Make sure methods signature are compatible.

PHP generates the infamous Fatal error at execution : Declaration of FooParent\:\:Bar() must be compatible with FooChildren\:\:Bar()

<?php

class x {
    function xa() {}
}

class xxx extends xx {
    function xa($a) {}
}

?>

Currently, the analysis doesn’t check for ellipsis nor references.

9.268.1. Suggestions

  • Fix the child class method() signature.
  • Fix the parent class method() signature, after checking that it won’t affect the other children.
Short name Classes/MethodSignatureMustBeCompatible
Themes Analyze, LintButWontExec
Severity Critical
Time To Fix Quick (30 mins)

9.269. Methodcall On New

It is possible to call a method right at object instantiation.

This syntax was added in PHP 5.4+. Before, this was not possible : the object had to be stored in a variable first.

<?php

// Data is collected
$data = data_source();

// Data is saved, but won't be reused from this databaseRow object. It may be ignored.
$result = (new databaseRow($data))->save();

// The actual result of the save() is collected and tested.
if ($result !== true) {
    processSaveError($data);
}

?>

This syntax is interesting when the object is not reused, and may be discarded

Short name Php/MethodCallOnNew
Themes CompatibilityPHP53
Php Version With PHP 5.4 and more recent
Severity Major
Time To Fix Quick (30 mins)

9.270. Methods Without Return

List of all the function, closures, methods that have no explicit return.

Functions that hold the void return type are omitted.

<?php

// With return null : Explicitly not returning
function withExplicitReturn($a = 1) {
    $a++;
    return null;
}

// Without indication
function withoutExplicitReturn($a = 1) {
    $a++;
}

// With return type void : Explicitly not returning
function withExplicitReturnType($a = 1) : void {
    $a++;
}

?>

See also return.

9.270.1. Suggestions

  • Add the returntype ‘void’ to make this explicit behavior
Short name Functions/WithoutReturn
Themes Analyze
Severity Minor
Time To Fix Quick (30 mins)

9.271. Minus One On Error

Some PHP native functions return -1 on error. They also return 1 in case of success, and 0 in case of failure. This leads to confusions.

In case the native function is used as a condition without explicit comparaison, PHP typecase the return value to a boolean. In this case, -1 and 1 are both converted to true, and the condition applies. This means that an error situation is mistaken for a successful event.

<?php

// Proper check of the return value
if (openssl_verify($data, $signature, $public) === 1) {
    $this->loginAsUser($user);
}

// if this call fails, it returns -1, and is confused with true
if (openssl_verify($data, $signature, $public)) {
    $this->loginAsUser($user);
}
?>

This analysis searches for if/then structures, ternary operators dans while() / do…`while() <http://php.net/manual/en/control-structures.while.php>`_ loops.

See also Can you spot the vulnerability? (openssl_verify) and Incorrect Signature Verification.

9.271.1. Suggestions

  • Compare explicitly the return value to 1
Short name Security/MinusOneOnError
Themes Security
Severity Critical
Time To Fix Instant (5 mins)

9.272. Mismatch Type And Default

The argument typehint and its default value don’t match.

The code may lint and load, and even work when the argument are provided. Though, PHP won’t eventually execute it.

Most of the mismatch problems are caught by PHP at linting time. You’ll get the following error message : ‘Argument 1 passed to foo() must be of the type integer, string given’.

The default value may be a constant (normal or class constant) : as such, PHP might find its value only at execution time, from another include. As such, PHP doesn’t report anything about the situation at compile time.

The default value may also be a constant scalar expression : since PHP 7, some of the simple operators such as +, -, , %, `* <http://www.php.net/manual/en/language.operators.arithmetic.php>`_, etc. are available to build default values. Among them, the ternary operator and Coalesce. Again, those expression may be only evaluated at execution time, when the value of the constants are known.

<?php

// bad definition
const STRING = 3;

function foo(string $s = STRING) {
    echo $s;
}

// works without problem
foo('string');

// Fatal error at compile time
foo();

// Fail only at execution time (missing D), and when default is needed
function foo2(string $s = D ? null : array()) {
    echo $s;
}

?>

PHP report typehint and default mismatch at compilation time, unless there is a static expression that can’t be resolved within the compiled file : then it is checked only at runtime, leading to a Fatal error.

See also Type declarations.

9.272.1. Suggestions

  • Match the typehint with the default value
Short name Functions/MismatchTypeAndDefault
Themes Analyze, LintButWontExec
Severity Critical
Time To Fix Slow (1 hour)
Examples WordPress

9.273. Mismatched Default Arguments

Arguments are relayed from one method to the other, and the arguments have different default values.

Although it is possible to have different default values, it is worth checking why this is actually the case.

<?php

function foo($a = null, $b = array() ) {
    // foo method calls directly bar.
    // When argument are provided, it's OK
    // When argument are omited, the default value is not the same as the next method
    bar($a, $b);
}

function bar($c = 1, $d = array() ) {

}

?>

9.273.1. Suggestions

  • Synchronize default values to avoid surprises
  • Drop some of the default values
Short name Functions/MismatchedDefaultArguments
Themes Analyze
Severity Minor
Time To Fix Quick (30 mins)
Examples SPIP

9.274. Mismatched Ternary Alternatives

A ternary operator should yield the same type on both branches.

Ternary operator applies a condition, and yield two different results. Those results will then be processed by code that expects the same types. It is recommended to match the types on both branches of the ternary operator.

<?php

// $object may end up in a very unstable state
$object = ($type == 'Type') ? new $type() : null;

//same result are provided by both alternative, though process is very different
$result = ($type == 'Addition') ? $a + $b : $a * $b;

//Currently, this is omitted
$a = 1;
$result = empty($condition) ? $a : 'default value';
$result = empty($condition) ? $a : getDefaultValue();

?>

9.274.1. Suggestions

  • Use compatible data type in both branch of the alternative
  • Turn the ternary into a if/then, with different processing
Short name Structures/MismatchedTernary
Themes Analyze, Suggestions
Severity Major
Time To Fix Quick (30 mins)
Examples phpadsnew, OpenEMR

9.275. Mismatched Typehint

Relayed arguments don’t have the same typehint.

Typehint acts as a filter method. When an object is checked with a first class, and then checked again with a second distinct class, the whole process is always false : $a can’t be of two different classes at the same time.

<?php

// Foo() calls bar()
function foo(A $a, B $b) {
    bar($a, $b);
}

// $a is of A typehint in both methods, but
// $b is of B then BB typehing
function bar(A $a, BB $b) {

}

?>

Note : This analysis currently doesn’t check generalisation of classes : for example, when B is a child of BB, it is still reported as a mismatch.

9.275.1. Suggestions

  • Ensure that the default value match the expected typehint.
Short name Functions/MismatchedTypehint
Themes Analyze
Severity Major
Time To Fix Quick (30 mins)
Examples WordPress

9.276. Missing Cases In Switch

It seems that some cases are missing in this switch structure.

When comparing two different switch() structures, it appears that some cases are missing in one of them. The set of cases are almost identical, but one of the values are missing.

Switch() structures using strings as literals are compared in this analysis. When the discrepancy between two lists is below 25%, both switches are reported.

<?php

// This switch operates on a, b, c, d and default
switch($a) {
    case 'a': doSomethingA(); break 1;
    case 'b': doSomethingB(); break 1;
    case 'c': doSomethingC(); break 1;
    case 'd': doSomethingD(); break 1;
    default: doNothing();
}

// This switch operates on a, b, d and default
switch($o->p) {
    case 'a': doSomethingA(); break 1;
    case 'b': doSomethingB(); break 1;

    case 'd': doSomethingD(); break 1;
    default: doNothing();
}

?>

In the example, one may argue that the ‘c’ case is actually handled by the ‘default’ case. Otherwise, business logic may request that omission.

9.276.1. Suggestions

  • Add the missing cases
  • Add comments to mention that missing cases are processed in the default case
Short name Structures/MissingCases
Themes Analyze
Severity Minor
Time To Fix Slow (1 hour)
Examples Tikiwiki

9.277. Missing Include

The included files doesn’t exists in the repository. The inclusions target a files that doesn’t exist.

The analysis works with every type of inclusion : include(), require(), include_once() and require_once(). It also works with parenthesis when used as parameter delimiter.

The analysis doesn’t take into account include_path. This may yield false positives.

<?php

include 'non_existent.php';

// variables are not resolved. This won't be reported.
require ($path.'non_existent.php');

?>

Missing included files may lead to a fatal error, a warning or other error later in the execution.

Name Default Type Description
constant_or_variable_name 100 string Literal value to be used when including files. For example, by configuring ‘Files_MissingInclude[“HOME_DIR”] = “/tmp/myDir/”;’, then ‘include HOME_DIR . “my_class.php”; will be actually be used as ‘/tmp/myDir/my_class.php’. Constants must be configured with their correct case. Variable must be configured with their initial ‘$’. Configure any number of variable and constant names.
Short name Files/MissingInclude
Themes Analyze
Severity Critical
Time To Fix Instant (5 mins)

9.278. Missing New ?

This functioncall looks like a class instantiation that is missing the new keyword.

Any function definition was found for that function, but a class with that name was. New is probably missing.

<?php

// Functioncall
$a = foo();

// Class definition
class foo {}
// Function definition
function foo {}


// Functioncall
$a = BAR;

// Function definition
class bar {}
// Constant definition
const BAR = 1;


?>

9.278.1. Suggestions

  • Add the new
  • Rename the class to distinguish it from the function
  • Rename the function to distinguish it from the class
Short name Structures/MissingNew
Themes Analyze
Severity Critical
Time To Fix Instant (5 mins)

9.279. Missing Parenthesis

Add parenthesis to those expression to prevent bugs.

<?php

// Missing some parenthesis!!
if (!$a instanceof Stdclass) {
    print Not\n;
} else {
    print Is\n;
}

// Could this addition be actually
$c = -$a + $b;

// This one ?
$c = -($a + $b);

?>

See also Operators Precedence.

Short name Structures/MissingParenthesis
Themes Analyze
Severity Major
Time To Fix Instant (5 mins)

9.280. Mistaken Concatenation

A unexpected structure is built for initialization. It may be a typo that creates an unwanted expression.

<?php

// This 'cd' is unexpected. Isn't it 'c', 'd' ?
$array = array('a', 'b', 'c'. 'd');
$array = array('a', 'b', 'c', 'd');

// This 4.5 is unexpected. Isn't it 4, 5 ?
$array = array(1, 2, 3, 4.5);
$array = array(1, 2, 3, 4, 5);

?>
Short name Arrays/MistakenConcatenation
Themes Coding Conventions
Severity Major
Time To Fix Instant (5 mins)

9.281. Mixed Concat And Interpolation

Mixed usage of concatenation and string interpolation is error prone. It is harder to read, and leads to overlooking the concatenation or the interpolation.

<?php

// Concatenation string
$a = $b . 'c' . $d;

// Interpolation strings
$a = {$b}c{$d};   // regular form
$a = {$b}c$d;     // irregular form

// Mixed Concatenation and Interpolation string
$a = {$b}c . $d;
$a = $b . c$d;
$a = $b . c{$d};

// Mixed Concatenation and Interpolation string with constant
$a = {$b}c . CONSTANT;

?>

Fixing this issue has no impact on the output. It makes code less error prone.

There are some situations where using concatenation are compulsory : when using a constant, calling a function, running a complex expression or make use of the escape sequence. You may also consider pushing the storing of such expression in a local variable.

9.281.1. Suggestions

  • Only use one type of variable usage : either interpolation, or concatenation
Short name Structures/MixedConcatInterpolation
Themes Coding Conventions, Analyze
Severity Minor
Time To Fix Quick (30 mins)
Examples SuiteCrm, Edusoho

9.282. Mixed Keys Arrays

Avoid mixing constants and literals in array keys.

When defining default values in arrays, it is recommended to avoid mixing constants and literals, as PHP may mistake them and overwrite the previous with the latter.

Either switch to a newer version of PHP (5.5 or newer), or make sure the resulting array hold the expected data. If not, reorder the definitions.

<?php

const ONE = 1;

$a = [ 1   => 2,
       ONE => 3];

?>

9.282.1. Suggestions

  • Use only literals or constants when building the array
Short name Arrays/MixedKeys
Themes CompatibilityPHP53, CompatibilityPHP54
Php Version With PHP 5.6 and more recent
Severity Minor
Time To Fix Slow (1 hour)

9.283. Mkdir Default

mkdir() gives universal access to created folders, by default. It is recommended to gives limited set of rights (0755, 0700), or to explicitly set the rights to 0777.

<?php

// By default, this dir is 777
mkdir('/path/to/dir');

// Explicitely, this is wanted. It may also be audited easily
mkdir('/path/to/dir', 0777);

// This dir is limited to the current user.
mkdir('/path/to/dir', 0700);

?>

See also Why 777 Folder Permissions are a Security Risk.

9.283.1. Suggestions

  • Always use the lowest possible privileges on folders
  • Don’t use the PHP default : at least, make it explicit that the ‘universal’ rights are voluntary
Short name Security/MkdirDefault
Themes Security
Severity Major
Time To Fix Quick (30 mins)
Examples Mautic, OpenEMR

9.284. Modernize Empty With Expression

empty() accept expressions since PHP 5.5. There is no need to store the expression in a variable before testing, unless it is reused later.

<?php

// PHP 5.5+ empty() usage
if (empty(foo($b . $c))) {
    doSomethingWithoutA();
}

// Compatible empty() usage
$a = foo($b . $c);
if (empty($a)) {
    doSomethingWithoutA();
}

// $a2 is reused, storage is legit
$a2 = strtolower($b . $c);
if (empty($a2)) {
    doSomething();
} else {
    echo $a2;
}

?>

See also empty().

9.284.1. Suggestions

  • Aviod the temporary variable, and use directly empty()
Short name Structures/ModernEmpty
Themes Analyze
Php Version With PHP 5.5 and more recent
Severity Minor
Time To Fix Quick (30 mins)

9.285. Multiple Alias Definitions

Some aliases are representing different classes across the repository. This leads to potential confusion.

Across an application, it is recommended to use the same namespace for one alias. Failing to do this lead to the same keyword to represent different values in different files, with different behavior. Those are hard to find bugs.

<?php

namespace A {
    use d\d; // aka D
}

// Those are usually in different files, rather than just different namespaces.

namespace B {
    use b\c as D; // also D. This could be named something else
}

?>

9.285.1. Suggestions

  • Give more specific names to classes
  • Use an alias ‘use AB ac BC’ to give locally another name
Short name Namespaces/MultipleAliasDefinitions
Themes Analyze
Severity Minor
Time To Fix Instant (5 mins)
Examples ChurchCRM, Phinx

9.286. Multiple Alias Definitions Per File

Avoid aliasing the same name with different aliases. This leads to confusion.

<?php

// first occurrence
use name\space\ClasseName;

// when this happens, several other uses are mentionned

// name\space\ClasseName has now two names
use name\space\ClasseName as anotherName;

?>

See also Namespaces/MultipleAliasDefinition.

Short name Namespaces/MultipleAliasDefinitionPerFile
Themes Analyze
Severity Minor
Time To Fix Slow (1 hour)

9.287. Multiple Class Declarations

It is possible to declare several times the same class in the code. PHP will not mention it until execution time, since declarations may be conditional.

<?php

$a = 1;

// Conditional declaration
if ($a == 1) {
    class foo {
        function method() { echo 'class 1';}
    }
} else {
    class foo {
        function method() { echo 'class 2';}
    }
}

(new foo())->method();
?>

It is recommended to avoid declaring several times the same class in the code. The best practice is to separate them with namespaces, they are for here for that purpose. In case those two classes are to be used interchangeably, the best is to use an abstract class or an interface.

Short name Classes/MultipleDeclarations
Themes Analyze
Severity Major
Time To Fix Quick (30 mins)

9.288. Multiple Classes In One File

It is regarded as a bad practice to store several classes in the same file. This is usually done to make life of __autoload() easier.

It is often unexpected to find class foo in the bar.php file. This is also the case for interfaces and traits.

<?php

// three classes in the same file
class foo {}
class bar {}
class foobar{}

?>

One good reason to have multiple classes in one file is to reduce include time by providing everything into one nice include.

See also Is it a bad practice to have multiple classes in the same file?.

Short name Classes/MultipleClassesInFile
Themes Coding Conventions
Severity Minor
Time To Fix Quick (30 mins)

9.289. Multiple Constant Definition

Some constants are defined several times in your code. This will lead to a fatal error, if they are defined during the same execution.

Multiple definitions may happens at bootstrap, when the application code is collecting information about the current environment. It may also happen at inclusion time, which one set of constant being loaded, while other definition are not, avoiding conflict. Both are false positive.

<?php

// OS is defined twice.
if (PHP_OS == 'Windows') {
    define('OS', 'Win');
} else {
    define('OS', 'Other');
}

?>

9.289.1. Suggestions

  • Move the constants to a class, and include the right class based on control flow.
  • Give different names to the constants, and keep the condition close to utilisation.
  • Move the constants to an external configuration file : it will be easier to identify that those constants may change.
Short name Constants/MultipleConstantDefinition
Themes Analyze
Severity Minor
Time To Fix Quick (30 mins)
Examples Dolibarr, OpenConf

9.290. Multiple Definition Of The Same Argument

A method’s signature is holding twice (or more) the same argument. For example, function x ($a, $a) { }.

This is accepted as is by PHP 5, and the last parameter’s value will be assigned to the variable. PHP 7.0 and more recent has dropped this feature, and reports a fatal error when linting the code.

<?php
  function x ($a, $a) { print $a; };
  x(1,2); => display 2

    // special case with a closure :
  function ($a) use ($a) { print $a; };
  x(1,2); => display 2

?>

However, this is not common programming practise : all arguments should be named differently.

See also Prepare for PHP 7 error messages (part 3).

9.290.1. Suggestions

  • Give different names to different parameters
Short name Functions/MultipleSameArguments
Themes CompatibilityPHP53, CompatibilityPHP54, CompatibilityPHP55, CompatibilityPHP56
Php Version With PHP 7.0 and older
Severity Major
Time To Fix Instant (5 mins)
ClearPHP all-unique-arguments

9.291. Multiple Exceptions Catch()

It is possible to have several distinct exceptions class caught by the same catch, preventing code repetition.

This is a new feature since PHP 7.1.

<?php

// PHP 7.1 and more recent
try {
    throw new someException();
} catch (Single $s) {
    doSomething();
} catch (oneType | anotherType $s) {
    processIdentically();
} finally {

}

// PHP 7.0 and older
try {
    throw new someException();
} catch (Single $s) {
    doSomething();
} catch (oneType $s) {
    processIdentically();
} catch (anotherType $s) {
    processIdentically();
} finally {

}

?>

This is a backward incompatible feature of PHP 7.1.

Short name Exceptions/MultipleCatch
Themes CompatibilityPHP53, CompatibilityPHP70, CompatibilityPHP54, CompatibilityPHP55, CompatibilityPHP56
Severity Major
Time To Fix Quick (30 mins)

9.292. Multiple Identical Closure

Several closures are defined with the same code.

It may be interesting to check if a named function could be defined from them.

<?php

// the first squares, with closure
$squares= array_map(function ($a) {return $a * $a; }, range(0, 10) );

// later, in another file...
// another identical closure
$squaring = function ($x) { return $x * $x; };
foo($x, $squaring);

?>

This analysis also reports functions and methods that look like the closures : they may be considered for switch.

9.292.1. Suggestions

  • Create a function with the body of those closures, and replace the closures by the function’s name.
Short name Functions/MultipleIdenticalClosure
Themes Suggestions
Severity Minor
Time To Fix Slow (1 hour)

9.293. Multiple Identical Trait Or Interface

There is no need to use the same trait, or implements the same interface more than once.

Up to PHP 7.1 (at least), this doesn’t raise any warning. Traits are only imported once, and interfaces may be implemented as many times as wanted.

<?php

class foo {
    use t3,t3,t3;
}

class bar implements i,i,i {

}

?>
Short name Classes/MultipleTraitOrInterface
Themes Analyze
Severity Minor
Time To Fix Instant (5 mins)

9.294. Multiple Index Definition

Indexes that are defined multiple times in the same array.

<?php
    // Multiple identical keys
    $x = array(1 => 2,
               2 => 3,
               1 => 3);

    // Multiple identical keys (sneaky version)
    $x = array(1 => 2,
               1.1 => 3,
               true => 4);

    // Multiple identical keys (automated version)
    $x = array(1 => 2,
               3,        // This will be index 2
               2 => 4);  // this index is overwritten
?>

They are indeed overwriting each other. This is most probably a typo.

9.294.1. Suggestions

  • Review your code and check that arrays only have keys defined once.
  • Review carefully your code and check indirect values, like constants, static constants.
Short name Arrays/MultipleIdenticalKeys
Themes Analyze
Severity Minor
Time To Fix Instant (5 mins)
Examples Magento, MediaWiki

9.295. Multiple Type Variable

Avoid using the same variable with different types of data.

It is recommended to use different names for differently typed data, while processing them. This prevents errors where one believe the variable holds the former type, while it has already been cast to the later.

Incrementing variables, with math operations or concatenation, is OK : the content changes, but not the type. And casting the variable without storing it in itself is OK.

<?php

// $x is an array
$x = range('a', 'z');
// $x is now a string
$x = join('', $x);
$c = count($x); // $x is not an array anymore


// $letters is an array
$letters = range('a', 'z');
// $alphabet is a string
$alphabet = join('', $letters);

// Here, $letters is cast by PHP, but the variable is changed.
if ($letters) {
    $count = count($letters); // $letters is still an array
}

?>

9.295.1. Suggestions

  • Use a class that accepts one type of argument, and exports another type of argument.
  • Use different variable for each type of data format : $rows (for array), $list (for implode(‘’, $rows))
  • Pass the final result as argument to another method, avoiding the temporary variable
Short name Structures/MultipleTypeVariable
Themes Analyze
Severity Minor
Time To Fix Quick (30 mins)
Examples Typo3, Vanilla

9.296. Multiple Unset()

Unset() accepts multiple arguments, unsetting them one after each other. It is more efficient to call unset() once, than multiple times.

<?php

// One call to unset only
unset($a, $b, $c, $d);

// Too many calls to unset
unset($a);
unset($b);
unset($c);
unset($d);

?>

See also unset.

9.296.1. Suggestions

  • Merge all unset into one call
Short name Structures/MultipleUnset
Themes Suggestions
Severity Minor
Time To Fix Quick (30 mins)

9.297. Multiple Usage Of Same Trait

The same trait is used several times. One trait usage is sufficient.

<?php

// C is used twice, and could be dropped from B
trait A { use B, C;}
trait B { use C;}

?>

PHP doesn’t raise any error when traits are included multiple times.

See also Traits.

9.297.1. Suggestions

  • Remove any multiple traits from use expressions
  • Review the class tree, and remove any trait mentioned multiple times
Short name Traits/MultipleUsage
Themes Suggestions
Severity Minor
Time To Fix Instant (5 mins)
Examples NextCloud

9.298. Multiples Identical Case

Some cases are defined multiple times, but only one will be processed. Check the list of cases, and remove the extra one.

Exakat tries to find the value of the case as much as possible, and ignore any dynamic cases (using variables).

<?php

const A = 1;

case ($x) {
    case 1 :
        break;
    case true:    // This is a duplicate of the previous
        break;
    case 1 + 0:   // This is a duplicate of the previous
        break;
    case 1.0 :    // This is a duplicate of the previous
        break;
    case A :      // The A constant is actually 1
        break;
    case $y  :    // This is not reported.
        break;
    default:

}
?>

9.298.1. Suggestions

  • Remove the double case
  • Change the case to another and rightfull value
Short name Structures/MultipleDefinedCase
Themes Analyze
Severity Minor
Time To Fix Quick (30 mins)
ClearPHP no-duplicate-case
Examples SugarCrm, ExpressionEngine

9.299. Multiply By One

Multiplying by 1 is a fancy type cast.

If it is used to type cast a value to number, then casting (integer) or (real) is clearer. This behavior may change with PHP 7.1, which has unified the behavior of all hidden casts.

<?php

// Still the same value than $m, but now cast to integer or real
$m = $m * 1;

// Still the same value than $m, but now cast to integer or real
$n *= 1;

// make typecasting clear, and merge it with the producing call.
$n = (int) $n;

?>

See also Type Juggling

9.299.1. Suggestions

  • Typecast to (int) or (float) for better readability
  • Skip useless math operation altogether
Short name Structures/MultiplyByOne
Themes Analyze
Severity Minor
Time To Fix Instant (5 mins)
ClearPHP no-useless-math
Examples SugarCrm, Edusoho

9.300. Must Call Parent Constructor

Some PHP native classes require a call to parent::__construct() to be stable.

As of PHP 7.3, two classes currently need that call : SplTempFileObject and SplFileObject.

The error is only emitted if the class is instantiated, and a parent class is called.

<?php

class mySplFileObject extends \SplFileObject {
    public function __construct()    {
        // Forgottent call to parent::__construct()
    }
}

(new mySplFileObject())->passthru();
?>

See also Why, php? WHY???.

9.300.1. Suggestions

  • Add a call to the parent’s constructor
  • Remove the extension of the parent class
Short name Php/MustCallParentConstructor
Themes Analyze
Severity Major
Time To Fix Quick (30 mins)

9.301. Must Return Methods

The following methods are expected to return a value that will be used later. Without return, they are useless.

Methods that must return are : __get(), __isset(), __sleep(), __toString(), __set_state(), __invoke(), __debugInfo(). Methods that may not return, but are often expected to : __call(), __callStatic().

<?php

class foo {
    public function __isset($a) {
        // returning something useful
        return isset($this->$var[$a]);
    }

    public function __get($a) {
        $this->$a++;
        // not returning...
    }

    public function __call($name, $args) {
        $this->$name(...$args);
        // not returning anything, but that's OK
    }

}
?>

9.301.1. Suggestions

  • Add a return expression, with a valid data type
  • Remove the return typehint
Short name Functions/MustReturn
Themes Analyze, LintButWontExec
Severity Major
Time To Fix Quick (30 mins)

9.302. Named Regex

Captured subpatterns may be named, for easier reference.

From the manual : It is possible to name a subpattern using the syntax (?P<name>pattern). This subpattern will then be indexed in the matches array by its normal numeric position and also by name. PHP 5.2.2 introduced two alternative syntaxes (?<name>pattern) and (?'name'pattern).

Naming subpatterns makes it easier to know what is read from the results of the subpattern : for example, $r['name'] has more meaning than $r[1].

Named subpatterns may also be shifted in the regex without impact on the resulting array.

<?php

$x = 'abc';
preg_match_all('/(?<name>a)/', $x, $r);
print_r($r[1]);
print_r($r['name']);

preg_match("/(?<name>a)(?'sub'b)/", $x, $s);
print $s[2];
print $s['sub'];

?>

See also Subpatterns.

9.302.1. Suggestions

  • Use named regex, and stop using integer-named subpatterns
Short name Structures/NamedRegex
Themes Suggestions
Examples Phinx, shopware

9.303. Negative Power

The power operator ** has higher precedence than the sign operators + and -.

This means that -2 ** 2 == -4. It is in fact, -(2 ** 2).

When using negative power, it is clearer to add parenthesis or to use the pow() function, which has no such ambiguity :

<?php

// -2 to the power of 2 (a square)
pow(-2, 2) == 4;

// minus 2 to the power of 2 (a negative square)
-2 ** 2 == -(2 ** 2) == 4;

?>
Short name Structures/NegativePow
Themes Analyze
Severity Major
Time To Fix Instant (5 mins)

9.304. Nested Ifthen

Three levels of ifthen is too much. The method should be split into smaller functions.

<?php

function foo($a, $b) {
    if ($a == 1) {
        // Second level, possibly too much already
        if ($b == 2) {

        }
    }
}

function bar($a, $b, $c) {
    if ($a == 1) {
        // Second level.
        if ($b == 2) {
            // Third level level.
            if ($c == 3) {
                // Too much
            }
        }
    }
}

?>
Name Default Type Description
nestedIfthen 3 integer Maximal number of acceptable nesting of if-then structures
Short name Structures/NestedIfthen
Themes Analyze
Severity Major
Time To Fix Quick (30 mins)
Examples LiveZilla, MediaWiki

9.305. Nested Ternary

Ternary operators should not be nested too deep.

They are a convenient instruction to apply some condition, and avoid a if() structure. It works best when it is simple, like in a one liner.

However, ternary operators tends to make the syntax very difficult to read when they are nested. It is then recommended to use an if() structure, and make the whole code readable.

<?php

// Simple ternary expression
echo ($a == 1 ? $b : $c) ;

// Nested ternary expressions
echo ($a === 1 ? $d === 2 ? $b : $d : $d === 3 ? $e : $c) ;
echo ($a === 1 ? $d === 2 ? $f ===4 ? $g : $h : $d : $d === 3 ? $e : $i === 5 ? $j : $k) ;

//Previous expressions, written as a if / Then expression
if ($a === 1) {
    if ($d === 2) {
        echo $b;
    } else {
        echo $d;
    }
} else {
    if ($d === 3) {
        echo $e;
    } else {
        echo $c;
    }
}

if ($a === 1) {
    if ($d === 2) {
        if ($f === 4) {
            echo $g;
        } else {
            echo $h;
        }
    } else {
        echo $d;
    }
} else {
    if ($d === 3) {
        echo $e;
    } else {
        if ($i === 5) {
            echo $j;
        } else {
            echo $k;
        }
    }
}

?>

See also Nested Ternaries are Great.

9.305.1. Suggestions

  • Replace ternaries by if/then structures.
  • Replace ternaries by a functioncall : this provides more readability, offset the actual code, and gives room for making it different.
Short name Structures/NestedTernary
Themes Analyze
Severity Major
Time To Fix Quick (30 mins)
ClearPHP no-nested-ternary
Examples SPIP, Zencart

9.306. Never Used Parameter

When a parameter is never used at calltime, it may be turned into a local variable.

It seems that the parameter was set up initially, but never found its practical usage. It is never mentioned, and always fall back on its default value.

Parameter without a default value are reported by PHP, and are usually always filled.

<?php

// $b may be turned into a local var, it is unused
function foo($a, $b = 1) {
    return $a + $b;
}

// whenever foo is called, the 2nd arg is not mentionned
foo($a);
foo(3);
foo('a');
foo($c);

?>