Home

Awesome

ShipMonk PHPStan strict rules

About 40 super-strict rules we found useful in ShipMonk. We tend to have PHPStan set up as strict as possible, but that still was not strict enough for us. This set of rules should fill the missing gaps we found.

If you find some rules opinionated, you can easily disable them.

Installation:

composer require --dev shipmonk/phpstan-rules

Use official extension-installer or enable all rules manually by:

# phpstan.neon
includes:
    - vendor/shipmonk/phpstan-rules/rules.neon

Configuration:

You can easily disable or reconfigure any rule, for example:

parameters:
    shipmonkRules:
        enforceReadonlyPublicProperty:
            enabled: false
        forbidUnsafeArrayKey:
            reportMixed: false

Or you can disable all rules and enable only those you want:

parameters:
    shipmonkRules:
        enableAllRules: false

        allowComparingOnlyComparableTypes:
            enabled: true

When you try to configure any default array, PHPStan config is merged by default, so if you want to enforce only your values and not to include our defaults, use exclamation mark:

parameters:
    shipmonkRules:
        forbidCast:
            enabled: true
            blacklist!: ['(unset)'] # force the blacklist to be only (unset)

Few rules are enabled, but do nothing unless configured, those are marked with *.

Rules:

allowComparingOnlyComparableTypes

function example1(Money $fee1, Money $fee2) {
    if ($fee1 > $fee2) {} // comparing objects is denied
}

new DateTime() > '2040-01-02'; // comparing different types is denied
200 > '1e2'; // comparing different types is denied

backedEnumGenerics *

parameters:
    stubFiles:
        - BackedEnum.php.stub # see article or BackedEnumGenericsRuleTest
    ignoreErrors:
        - '#^Enum .*? has @implements tag, but does not implement any interface.$#'

enum MyEnum: string { // missing @implements tag
    case MyCase = 'case1';
}

classSuffixNaming *

    shipmonkRules:
        classSuffixNaming:
            superclassToSuffixMapping!:
                \Exception: Exception
                \PHPStan\Rules\Rule: Rule
                \PHPUnit\Framework\TestCase: Test
                \Symfony\Component\Console\Command\Command: Command

enforceClosureParamNativeTypehint

/**
 * @param list<Entity> $entities
 * @return list<Uuid>
 */
public function getIds(array $entities): array {
    return array_map(
        function ($entity) { // missing native typehint; not reported with allowMissingTypeWhenInferred: true
            return $entity->id;
        },
        $entities
    );
}

enforceEnumMatchRule

enum MyEnum {
    case Foo;
    case Bar;
}

if ($enum === MyEnum::Foo) {
    // ...
} elseif ($enum === MyEnum::Bar) { // always true reported by phpstan (for versions 1.10.0 - 1.10.34)
    // ...
} else {
    throw new LogicException('Unknown case'); // phpstan knows it cannot happen
}

Which someone might fix as:

if ($enum === MyEnum::Foo) {
    // ...
} elseif ($enum === MyEnum::Bar) {
    // ...
}

Or even worse as:

if ($enum === MyEnum::Foo) {
    // ...
} else {
    // ...
}

We believe that this leads to more error-prone code since adding new enum case may not fail in tests. Very good approach within similar cases is to use match construct so that (ideally with forbidMatchDefaultArmForEnums enabled) phpstan fails once new case is added. PHPStan even adds tip about match in those cases since 1.10.11. For those reasons, this rule detects any always-true/false enum comparisons and forces you to rewrite it to match ($enum).

Since PHPStan 1.10.34, the behaviour is much better as it does not report error on the last elseif in case that it is followed by else with thrown exception. Such case raises exception in your tests if you add new enum case, but it is still silent in PHPStan. This leaves space for error being deployed to production. So we still believe this rule makes sense even in latest PHPStan.

enforceIteratorToArrayPreserveKeys

$fn = function () {
    yield new stdClass => 1;
};

iterator_to_array($fn()); // denied, would fail

enforceListReturn

/**
 * @return array<string>
 */
public function returnList(): array // error, return phpdoc is generic array, but list is always returned
{
    return ['item'];
}

enforceNativeReturnTypehint

class NoNativeReturnTypehint {
    /**
     * @return list<string>
     */
    public function returnList() // error, missing array typehint
    {
        return ['item'];
    }
}

enforceReadonlyPublicProperty

class EnforceReadonlyPublicPropertyRule {
    public int $foo; // fails, no readonly modifier
    public readonly int $bar;
}

forbidArithmeticOperationOnNonNumber

function add(string $a, string $b) {
    return $a + $b; // denied, non-numeric types are allowed
}

forbidCast

$empty = (array) null; // denied cast
$notEmpty = (array) 0; // denied cast

parameters:
    shipmonkRules:
        forbidCast:
            blacklist!: ['(array)', '(object)', '(unset)']

forbidCheckedExceptionInCallable

parameters:
    shipmonkRules:
        forbidCheckedExceptionInCallable:
            allowedCheckedExceptionCallables:
                'Symfony\Component\Console\Question::setValidator': 0 # symfony automatically converts all thrown exceptions to error output, so it is safe to throw anything here

parameters:
    exceptions:
        check:
            missingCheckedExceptionInThrows: true # enforce checked exceptions to be stated in @throws
            tooWideThrowType: true # report invalid @throws (exceptions that are not actually thrown in annotated method)
        implicitThrows: false # no @throws means nothing is thrown (otherwise Throwable is thrown)
        checkedExceptionClasses:
            - YourApp\TopLevelRuntimeException # track only your exceptions (children of some, typically RuntimeException)

class TransactionManager {
    /**
     * @param-immediately-invoked-callable $callback
     */
    public function transactional(callable $callback): void {
        // ...
        $callback();
        // ...
    }
}

class UserEditFacade
{
    /**
     * @throws UserNotFoundException
     */
    public function updateUserEmail(UserId $userId, Email $email): void
    {
        $this->transactionManager->transactional(function () use ($userId, $email) {
            $user = $this->userRepository->get($userId); // can throw checked UserNotFoundException
            $user->updateEmail($email);
        })
    }

    public function getUpdateEmailCallback(UserId $userId, Email $email): callable
    {
        return function () use ($userId, $email) {
            $user = $this->userRepository->get($userId); // this usage is denied, it throws checked exception, but you don't know when, thus it cannot be tracked by phpstan
            $user->updateEmail($email);
        };
    }
}

forbidCheckedExceptionInYieldingMethod

class Provider {
    /** @throws CheckedException */
    public static function generate(): iterable
    {
        yield 1;
        throw new CheckedException(); // denied, gets thrown once iterated
    }
}

forbidCustomFunctions *

parameters:
    shipmonkRules:
        forbidCustomFunctions:
            list:
                'Namespace\SomeClass::*': 'Please use different class' # deny all methods by using * (including constructor)
                'Namespace\AnotherClass::someMethod': 'Please use anotherMethod' # deny single method
                'var_dump': 'Please remove debug code' # deny function

new SomeClass(); // Class SomeClass is forbidden. Please use different class
(new AnotherClass())->someMethod(); // Method AnotherClass::someMethod() is forbidden. Please use anotherMethod

forbidEnumInFunctionArguments

enum MyEnum: string {
    case MyCase = 'case1';
}

implode('', [MyEnum::MyCase]); // denied, would fail on implicit toString conversion

forbidFetchOnMixed

function example($unknown) {
    $unknown->property; // cannot fetch property on mixed
}

forbidIdenticalClassComparison

function isEqual(DateTimeImmutable $a, DateTimeImmutable $b): bool {
    return $a === $b;  // comparing denied classes
}

parameters:
    shipmonkRules:
        forbidIdenticalClassComparison:
            blacklist!:
                - DateTimeInterface
                - Brick\Money\MoneyContainer
                - Brick\Math\BigNumber
                - Ramsey\Uuid\UuidInterface

forbidIncrementDecrementOnNonInteger

$value = '2e0';
$value++; // would be float(3), denied

forbidMatchDefaultArmForEnums

match ($enum) {
    MyEnum::Case: 1;
    default: 2; // default arm forbidden
}

forbidMethodCallOnMixed

function example($unknown) {
    $unknown->call(); // cannot call method on mixed
}

forbidNotNormalizedType

/**
 * @return mixed|false   // denied, this is still just mixed
 */
public function getAttribute(string $name)
{
    return $this->attributes[$name] ?? false;
}

forbidNullInAssignOperations

function getCost(int $cost, ?int $surcharge): int {
    $cost += $surcharge;  // denied, adding possibly-null value
    return $cost;
}

forbidNullInBinaryOperations

parameters:
    shipmonkRules:
        forbidNullInBinaryOperations:
            blacklist!: [
                '**', '!=', '==', '+', 'and', 'or', '&&', '||', '%', '-', '/', '*', # checked by phpstan-strict-rules
                '>', '>=', '<', '<=', '<=>', # checked by AllowComparingOnlyComparableTypesRule
                '===', '!==', '??' # valid with null involved
            ]

function getFullName(?string $firstName, string $lastName): string {
    return $firstName . ' ' . $lastName; // denied, null involved in binary operation
}

forbidNullInInterpolatedString

public function output(?string $name) {
    echo "Hello $name!"; // denied, possibly null value
}

forbidPhpDocNullabilityMismatchWithNativeTypehint

/**
 * @param string $param
 */
public function sayHello(?string $param) {} // invalid phpdoc not containing null

forbidProtectedEnumMethod

enum MyEnum {
    protected function isOpen(): bool {} // protected enum method denied
}

forbidReturnValueInYieldingMethod

class Get {
    public static function oneTwoThree(): iterable { // marked as iterable, caller cannot access the return value by Generator::getReturn
        yield 1;
        yield 2;
        return 3;
    }
}

iterator_to_array(Get::oneTwoThree()); // [1, 2] - see https://3v4l.org/Leu9j

parameters:
    shipmonkRules:
        forbidReturnValueInYieldingMethod:
            reportRegardlessOfReturnType: true # optional stricter mode, defaults to false

forbidUnsafeArrayKey

$taxRates = [ // denied, float key gets casted to int (causing $taxRates to contain one item)
    1.15 => 'reduced',
    1.21 => 'standard',
];

parameters:
    shipmonkRules:
        forbidUnsafeArrayKey:
            reportMixed: false # defaults to true
            reportInsideIsset: false # defaults to true

forbidVariableTypeOverwriting

function example(OrderId $id) {
    $id = $id->getStringValue(); // denied, type changed from object to string
}

forbidUnsetClassField

function example(MyClass $class) {
    unset($class->field); // denied
}

forbidUselessNullableReturn

public function example(int $foo): ?int { // null never returned
    if ($foo < 0) {
        return 0;
    }
    return $foo;
}

forbidUnusedException

function validate(): void {
    new Exception(); // forgotten throw
}

forbidUnusedMatchResult

match ($foo) { // unused match result
    case 'foo' => 1;
}

requirePreviousExceptionPass

try {
    // some code
} catch (RuntimeException $e) {
    throw new LogicException('Cannot happen'); // $e not passed as previous
}

parameters:
    shipmonkRules:
        requirePreviousExceptionPass:
            reportEvenIfExceptionIsNotAcceptableByRethrownOne: true

class MyException extends RuntimeException {
    public function __construct() {
        parent::__construct('My error');
    }
}

try {
    // some code
} catch (RuntimeException $e) {
    throw new MyException(); // reported even though MyException cannot accept it yet
}

uselessPrivatePropertyDefaultValue:

class Example
{
    private ?int $field = null; // useless default value

    public function __construct()
    {
        $this->field = 1;
    }
}

uselessPrivatePropertyNullability:

class Example
{
    private ?int $field; // useless nullability

    public function __construct()
    {
        $this->field = 1;
    }

    public function setField(int $value)
    {
        $this->field = $value;
    }
}

Native PHPStan extra strictness

Some strict behaviour in PHPStan is not enabled by default, consider enabling extra strictness even there:

includes:
    - phar://phpstan.phar/conf/config.levelmax.neon
    - phar://phpstan.phar/conf/bleedingEdge.neon      # https://phpstan.org/blog/what-is-bleeding-edge
    - vendor/phpstan/phpstan-strict-rules/rules.neon  # https://github.com/phpstan/phpstan-strict-rules
parameters:
    checkImplicitMixed: true                                  # https://phpstan.org/config-reference#checkimplicitmixed
    checkBenevolentUnionTypes: true                           # https://phpstan.org/config-reference#checkbenevolentuniontypes
    checkUninitializedProperties: true                        # https://phpstan.org/config-reference#checkuninitializedproperties
    checkMissingCallableSignature: true                       # https://phpstan.org/config-reference#vague-typehints
    checkTooWideReturnTypesInProtectedAndPublicMethods: true  # https://phpstan.org/config-reference#checktoowidereturntypesinprotectedandpublicmethods
    reportAnyTypeWideningInVarTag: true                       # https://phpstan.org/config-reference#reportanytypewideninginvartag
    reportPossiblyNonexistentConstantArrayOffset: true        # https://phpstan.org/config-reference#reportpossiblynonexistentconstantarrayoffset
    reportPossiblyNonexistentGeneralArrayOffset: true         # https://phpstan.org/config-reference#reportpossiblynonexistentgeneralarrayoffset

Contributing