ikvasnica/phpstan-clean-test

PHPStan extension with opinionated strict rules for better code in tests.

0.4 2023-08-23 19:30 UTC

Requires

• php: >=8.1
• nette/utils: ~3.0
• nikic/php-parser: ^4.3
• phpstan/phpstan: ^1.10

Requires (Dev)

• ergebnis/phpstan-rules: ^2.1.0
• php-coveralls/php-coveralls: 2.*
• phpstan/phpstan-strict-rules: ^1.5.1
• phpunit/phpunit: ^9.0.0
• symplify/easy-coding-standard: ^12.0.6

Suggests

• ergebnis/phpstan-rules: Even more PHPStan strict and opinionated rules.
• phpstan/phpstan-deprecation-rules: Rules for disallowing deprecation code.
• phpstan/phpstan-phpunit: PHPStan extension and rules to integrate with PHPUnit testing framework.
• phpstan/phpstan-strict-rules: Strict and opinionated PHPStan rules.

MIT

• Ivan Kvasnica

• tests
• PHPStan
• phpstan-rules
• phpstan-strict-rules

README

• PHPStan
• PHPStan-PHPUnit extension

This extension provides highly opinionated and strict rules for test cases for the PHPStan static analysis tool.

Installation

Run

$ composer require --dev ikvasnica/phpstan-clean-test

Usage

All of the rules provided by this library are included in rules.neon.

When you are using phpstan/extension-installer, rules.neon will be automatically included.

Otherwise you need to include rules.neon in your phpstan.neon:

# phpstan.neon
includes:
    - vendor/ikvasnica/phpstan-clean-test/rules.neon

Rules

This package provides the following rules for use with phpstan/phpstan:

• ikvasnica\PHPStan\Rules\UnitExtendsFromTestCaseRule
• ikvasnica\PHPStan\Rules\DisallowSetupAndConstructorRule
• ikvasnica\PHPStan\Rules\AssertSameOverAssertEqualsRule
• ikvasnica\PHPStan\Rules\StaticAssertOverThisAndStaticRule
• ikvasnica\PHPStan\Rules\NoNullableArgumentRule

UnitExtendsFromTestCaseRule

This rule forces you to extend only from allowed classes in unit tests (default: PHPUnit\Framework\TestCase).

Why:

1. It prevents developers i.e. from using a dependency injection container in unit tests ($this->getContainer()) and other tools from integration/functional tests.
2. You should extend only from a class when a child class satisfies the "is a" relationship. That said, if you need only a subset of a parent's functionality, you should use composition over inheritance (i.e. by traits or helpers).

❌

// tests/ExampleTestCase/Unit/UnitExtendsInvalidTest.php
namespace ExampleTestCase\Unit;

final class UnitExtendsInvalidTest extends \Dummy\FunctionalDummyTest {}

✅

// tests/ExampleTestCase/Unit/UnitExtendsUnitTest.php
namespace ExampleTestCase\Unit;

final class UnitExtendsUnitTest extends \PHPUnit\Framework\TestCase {}

Defaults

• By default, this rule detects unit tests by checking the namespace (it must contain the string Unit) and the class name ending (it must end with the string Test).
• The following class is allowed to be extended: PHPUnit\Framework\TestCase

Allowing classes to be extended

If you want to allow additional classes to be extended, you can add it to the classesAllowedToBeExtendedInTests parameter to a list of class names.

Detecting unit tests namespace

If you want to change the namespace string check described above, you can set your own string to be checked in the unitTestNamespaceContainsString parameter.

# phpstan.neon
parameters:
    ikvasnica:
        classesAllowedToBeExtendedInTests:
          - MyNamespace\AbstractTest
        unitTestNamespaceContainsString: CustomTestPath

DisallowSetupAndConstructorRule

Neither of methods __construct nor setUp can be declared in a unit test.

Why: Each test scenario should create its dependencies on its own. Method setUp is useful for setting up i.e. database transaction in a functional test. In a unit test, you should put all the preparation into a testing method or a data provider itself. It increases readability and clearly shows the code intention.

Detecting unit tests namespace

If you want to change the namespace string check described above, you can set your own string to be checked in the unitTestNamespaceContainsString parameter.

Allowing setUp() method

If you really want to use the setUp() method, you can whitelist it by setting the parameter allowSetupInUnitTests to true.

# phpstan.neon
parameters:
    ikvasnica:
        unitTestNamespaceContainsString: CustomTestPath
        allowSetupInUnitTests: true

❌

// tests/ExampleTestCase/Unit/DisallowSetupConstructInvaliTest.php
namespace ExampleTestCase\Unit;

use PHPUnit\Framework\Assert;

final class DisallowSetupConstructInvaliTest extends \PHPUnit\Framework\TestCase
{
    private $something;

    public function __construct($name = null, array $data = [], $dataName = '')
    {
        parent::__construct($name, $data, $dataName);
    }

    protected function setUp(): void
    {
        parent::setUp();

        $this->something = true;
    }

    public function testSomeThing(): void
    {
        Assert::assertTrue($this->something);
    }
}

✅

// tests/ExampleTestCase/Unit/DisallowSetupConstructOkTest.php
namespace ExampleTestCase\Unit;

use PHPUnit\Framework\Assert;

final class DisallowSetupConstructOkTest extends \PHPUnit\Framework\TestCase
{
    public function testSomeThing(): void
    {
        Assert::assertTrue(true);
    }
}

AssertSameOverAssertEqualsRule

Calling assertEquals in tests is forbidden in favor of assertSame.

Why: When using assertEquals, data types are not considered. On the other hand, assertSame checks whether two variables are of the same type and references the same object. Therefore, assertEquals can be valid when comparing objects or arrays, but not scalar values.

Using assertEquals with scalar values might lead to an unexpected behaviour (e.g. assertEquals(null, '') evaluates to true, whereas assertSame(null, '') evaluates to false).

❌

// tests/ExampleTestCase/Unit/InvalidAssertEqualsUses.php
use PHPUnit\Framework\Assert;

$booleanValue = false;
$exception = new Exception('A bad thing has happened.');

Assert::assertEquals(true, $booleanValue);
Assert::assertEquals('exception message', (string) $exception);

✅

// tests/ExampleTestCase/Unit/ValidAsserts.php
use PHPUnit\Framework\Assert;

$booleanValue = false;
$exception = new Exception('A bad thing has happened.');
$emptyArray = [];

Assert::assertTrue($booleanValue);
Assert::assertSame('exception message', (string) $exception);

Assert::assertEquals([], $emptyArray);

StaticAssertOverThisAndStaticRule

Calling $this->assert*, self::assert* or static::assert* in tests is forbidden in favor of PHPUnit\Framework\Assert::assert*.

Why: When you use PHPUnit, your test cases extend from \PHPUnit\Framework\TestCase. Assert methods are declared as static there, therefore it does not make sense to call them dynamically. Using static::assert* is discouraged, because it is a misuse of inheritance and assertion methods are more like a helper's methods.

❌

// tests/ExampleTestCase/Unit/InvalidAssertUsage.php
namespace ExampleTestCase;

final class InvalidAssertUsageTest extends \PHPUnit\Framework\TestCase
{
    public function dummyTest(): void
    {
        // will fail
        $this->assertSame(5, 5);
        $this->assertTrue(false);
        self::assertArrayHasKey(5, [5]);
        static::assertCount(0, []);
        \ExampleTestCase\StaticAssertOverThisAndStaticRule::assertTrue(true);
        InvalidAssertUsageTest::assertTrue(true);
    }
}

✅

// tests/ExampleTestCase/Unit/ValidAssertsUsage.php
namespace ExampleTestCase;

use PHPUnit\Framework\Assert;

final class ValidAssertUsageTest extends \PHPUnit\Framework\TestCase
{
    public function dummyTest(): void
    {
        // Assert::anything is OK
        Assert::assertEquals(5, 5);
        Assert::assertCount(1, [1, 2]);
        Assert::assertTrue(false);
        \PHPUnit\Framework\Assert::assertTrue(true);
    }
}

NoNullableArgumentRule

Nullable arguments and arguments with no type passed to test methods from data providers are forbidden.

Why: A nullable argument from a data provider is a code smell. Usually it means that you test two different scenarios in one test. You should divide the test into two scenarios, i.e. one for testing valid data input and one for invalid data when an exception is expected to be thrown.

❌

// tests/ExampleTestCase/Unit/NullableArgumentsInTest.php
namespace ExampleTestCase;

final class NullableArgumentsInTest extends \PHPUnit\Framework\TestCase
{
    public function testSomething($mixedTypeArgument, ?string $stringOrNullArgument): void
    {
        // will fail, because the first argument has no type and the second one is nullable
    }

    /**
     * @test
     * @param string|null $maybeString
     */
    public function someTestMethod(?string $maybeString): void
    {
        // will fail too, because this one is a test method due to the annotation
    }
}

✅

// tests/ExampleTestCase/Unit/ValidArgumentsInTest.php
namespace ExampleTestCase;

final class ValidArgumentsInTest extends \PHPUnit\Framework\TestCase
{
    public function testSomething(string $definitelyString): void
    {
        // this is a way to go!
    }

    public function testSomethingElse(): void
    {
        // no arguments are, of course, allowed
    }

    public function anotherPublicMethod(?bool $maybeTrueOrFalse): void
    {
        // although weird, you may have other non-test public methods with a nullable type as well
    }

    private function testHelperMethod(?string $maybeString): void
    {
        // private and protected methods are allowed to have nullable arguments
    }
}