feat(phpstan): add OneThingPerLineRule

phpstan/include-by-php-version.php

@@ -8,6 +8,11 @@
     $includes[] = __DIR__ . '/../rules.neon';
 }
 
+// PHP < 8.0: exclude test fixtures using nullsafe operator syntax
+if (version_compare(PHP_VERSION, '8.0', '<')) {
+    $includes[] = __DIR__ . '/php-below-8.0.neon';
+}
+
 // PHP < 8.1: exclude enums, add ignores for older PHPStan versions
 if (version_compare(PHP_VERSION, '8.1', '<')) {
     $includes[] = __DIR__ . '/php-below-8.1.neon';

phpstan/php-below-8.0.neon

@@ -0,0 +1,4 @@
+parameters:
+  excludePaths:
+    - ../tests/PHPStan/data/method-chain-nullsafe-violations.php
+    - ../tests/PHPStan/data/method-chain-nullsafe-correct.php

phpstan/php-below-8.1.neon

@@ -30,6 +30,7 @@ parameters:
 
     # Return type differences in older PHPStan rule interfaces
     - '#Method MLL\\Utils\\PHPStan\\Rules\\MissingClosureParameterTypehintRule::processNode\(\) should return array<int, PHPStan\\Rules\\IdentifierRuleError> but returns array<int, PHPStan\\Rules\\RuleError>\.#'
+    - '#Method MLL\\Utils\\PHPStan\\Rules\\OneThingPerLineRule::processNode\(\) should return array<int, PHPStan\\Rules\\IdentifierRuleError> but returns array<int, PHPStan\\Rules\\RuleError>\.#'
 
     # Existing code with @phpstan-ignore that older versions don't understand
     - message: '#Cannot access property \$name on SimpleXMLElement\|null\.#'

src/PHPStan/Rules/OneThingPerLineRule.php

@@ -0,0 +1,85 @@
+<?php declare(strict_types=1);
+
+namespace MLL\Utils\PHPStan\Rules;
+
+use PhpParser\Node;
+use PhpParser\Node\Expr;
+use PhpParser\Node\Expr\CallLike;
+use PhpParser\Node\Expr\MethodCall;
+use PhpParser\Node\Expr\NullsafeMethodCall;
+use PhpParser\Node\Expr\NullsafePropertyFetch;
+use PhpParser\Node\Expr\PropertyFetch;
+use PHPStan\Analyser\Scope;
+use PHPStan\Rules\IdentifierRuleError;
+use PHPStan\Rules\Rule;
+use PHPStan\Rules\RuleErrorBuilder;
+
+use function Safe\file_get_contents;
+
+/**
+ * @implements Rule<CallLike>
+ */
+final class OneThingPerLineRule implements Rule
+{
+    /** @var array<string, string> */
+    private array $fileContentsCache = [];
+
+    public function getNodeType(): string
+    {
+        return CallLike::class;
+    }
+
+    /** @return list<IdentifierRuleError> */
+    public function processNode(Node $node, Scope $scope): array
+    {
+        if (! $node instanceof MethodCall
+            && ! $node instanceof NullsafeMethodCall) {
+            return [];
+        }
+
+        $var = $node->var;
+        if (! $var instanceof MethodCall
+            && ! $var instanceof NullsafeMethodCall) {
+            return [];
+        }
+
+        if ($node->name->getStartLine() !== $var->name->getStartLine()) {
+            return [];
+        }
+
+        if ($var->getArgs() === []) {
+            return [];
+        }
+
+        if ($this->isInsideStringInterpolation($scope->getFile(), $node)) {
+            return [];
+        }
+
+        return [
+            RuleErrorBuilder::message('Method chain calls must each be on their own line.')
+                ->identifier('mll.oneThingPerLine')
+                ->line($node->name->getStartLine())
+                ->build(),
+        ];
+    }
+
+    private function isInsideStringInterpolation(string $file, Expr $node): bool
+    {
+        $root = $node;
+        while ($root instanceof MethodCall
+            || $root instanceof NullsafeMethodCall
+            || $root instanceof PropertyFetch
+            || $root instanceof NullsafePropertyFetch) {
+            $root = $root->var;
+        }
+
+        $startPos = $root->getStartFilePos();
+        if ($startPos <= 0) {
+            return false;
+        }
+
+        $this->fileContentsCache[$file] ??= file_get_contents($file);
+
+        return $this->fileContentsCache[$file][$startPos - 1] === '{';
+    }
+}

tests/PHPStan/OneThingPerLineRuleTest.php

@@ -0,0 +1,94 @@
+<?php declare(strict_types=1);
+
+namespace MLL\Utils\Tests\PHPStan;
+
+use PHPStan\Analyser\Analyser;
+use PHPStan\Analyser\Error;
+use PHPStan\Testing\PHPStanTestCase;
+use PHPUnit\Framework\Attributes\DataProvider;
+
+final class OneThingPerLineRuleTest extends PHPStanTestCase
+{
+    private const ERROR_MESSAGE = 'Method chain calls must each be on their own line.';
+
+    /** @return iterable<array{0: string, 1: array<int, array<int, string>>}> */
+    public static function dataIntegrationTests(): iterable
+    {
+        self::getContainer();
+
+        yield [__DIR__ . '/data/method-chain-violations.php', [
+            26 => [self::ERROR_MESSAGE],
+            31 => [self::ERROR_MESSAGE],
+            38 => [self::ERROR_MESSAGE],
+            45 => [self::ERROR_MESSAGE],
+        ]];
+
+        yield [__DIR__ . '/data/method-chain-correct.php', []];
+
+        if (PHP_VERSION_ID >= 80000) {
+            yield [__DIR__ . '/data/method-chain-nullsafe-violations.php', [
+                19 => [self::ERROR_MESSAGE],
+                24 => [self::ERROR_MESSAGE],
+            ]];
+
+            yield [__DIR__ . '/data/method-chain-nullsafe-correct.php', []];
+        }
+    }
+
+    /**
+     * @param array<int, array<int, string>> $expectedErrors
+     *
+     * @dataProvider dataIntegrationTests
+     */
+    #[DataProvider('dataIntegrationTests')]
+    public function testIntegration(string $file, array $expectedErrors): void
+    {
+        $errors = $this->runAnalyse($file);
+
+        $ourErrors = array_filter(
+            $errors,
+            static fn (Error $error): bool => str_contains($error->getMessage(), self::ERROR_MESSAGE),
+        );
+
+        if ($expectedErrors === []) {
+            self::assertEmpty($ourErrors, 'Should not report errors for correct code');
+        } else {
+            self::assertNotEmpty($ourErrors, 'Should detect method chain violations');
+            $this->assertSameErrorMessages($expectedErrors, $ourErrors);
+        }
+    }
+
+    /** @return array<Error> */
+    private function runAnalyse(string $file): array
+    {
+        $file = self::getFileHelper()->normalizePath($file);
+
+        /** @var Analyser $analyser */
+        $analyser = self::getContainer()->getByType(Analyser::class);
+
+        return $analyser->analyse([$file])->getErrors();
+    }
+
+    /**
+     * @param array<int, array<int, string>> $expectedErrors
+     * @param array<Error> $errors
+     */
+    private function assertSameErrorMessages(array $expectedErrors, array $errors): void
+    {
+        foreach ($errors as $error) {
+            $errorLine = $error->getLine() ?? 0;
+            $errorMessage = $error->getMessage();
+
+            self::assertArrayHasKey($errorLine, $expectedErrors, "Unexpected error at line {$errorLine}: {$errorMessage}");
+            self::assertContains($errorMessage, $expectedErrors[$errorLine]);
+        }
+    }
+
+    /** @return array<string> */
+    public static function getAdditionalConfigFiles(): array
+    {
+        return [
+            __DIR__ . '/phpstan-one-thing-per-line-test.neon',
+        ];
+    }
+}

tests/PHPStan/data/method-chain-correct.php

@@ -0,0 +1,123 @@
+<?php declare(strict_types=1);
+
+namespace MLL\Utils\Tests\PHPStan\data;
+
+class MethodChainCorrect
+{
+    public function foo(int $arg = 0): self
+    {
+        return $this;
+    }
+
+    public function bar(): self
+    {
+        return $this;
+    }
+
+    public function baz(): void {}
+
+    public function name(): string
+    {
+        return 'test';
+    }
+
+    public static function create(): self
+    {
+        return new self();
+    }
+
+    /** @var self */
+    public object $relation;
+
+    public function singleMethodCall(): void
+    {
+        $this->foo();
+    }
+
+    public function staticPlusSingleMethod(): void
+    {
+        self::create()->foo();
+    }
+
+    public function propertyAccessPlusMethod(): void
+    {
+        $this->relation->foo();
+    }
+
+    public function properlySplitChain(): void
+    {
+        $this->foo()
+            ->bar();
+    }
+
+    public function properlySplitThreeChain(): void
+    {
+        $this->foo()
+            ->bar()
+            ->baz();
+    }
+
+    public function newExpressionPlusSingleMethod(): void
+    {
+        (new self())->baz();
+    }
+
+    public function multilineArgsWithSplitContinuation(): void
+    {
+        $this->foo(
+            1
+        )->bar();
+    }
+
+    public function noArgAccessorThenAction(): void
+    {
+        $this->foo()->baz();
+    }
+
+    public function noArgAccessorThenActionWithArgs(): void
+    {
+        $this->foo()->foo(1);
+    }
+
+    public function noArgChainAllAccessors(): void
+    {
+        $this->foo()->bar()->baz();
+    }
+
+    public function staticPlusNoArgChain(): void
+    {
+        self::create()->foo()->bar();
+    }
+
+    public function propertyThenNoArgAccessorThenAction(): void
+    {
+        $this->relation->foo()->baz();
+    }
+
+    public function noArgAccessorChainInArrowFunction(): void
+    {
+        /** @var list<self> $items */
+        $items = [];
+        array_map(fn (self $x): self => $x->foo()->bar(), $items);
+    }
+
+    public function noArgAccessorChainInClosure(): void
+    {
+        /** @var list<self> $items */
+        $items = [];
+        array_map(fn (self $x): int => spl_object_id($x->foo()->bar()), $items);
+    }
+
+    public function chainInsideStringInterpolation(): string
+    {
+        return "value: {$this->foo()->name()}";
+    }
+
+    public function properlySplitChainInClosure(): void
+    {
+        /** @var list<self> $items */
+        $items = [];
+        array_map(fn (self $x): self => $x->foo()
+            ->bar(), $items);
+    }
+}

tests/PHPStan/data/method-chain-nullsafe-correct.php

@@ -0,0 +1,32 @@
+<?php declare(strict_types=1);
+
+namespace MLL\Utils\Tests\PHPStan\data;
+
+class MethodChainNullsafeCorrect
+{
+    public function foo(): self
+    {
+        return $this;
+    }
+
+    public function bar(): self
+    {
+        return $this;
+    }
+
+    public function properlySplitNullSafeChain(?self $nullable): void
+    {
+        $nullable?->foo()
+            ?->bar();
+    }
+
+    public function noArgNullSafeChain(?self $nullable): void
+    {
+        $nullable?->foo()?->bar();
+    }
+
+    public function mixedNoArgNullSafeChain(?self $nullable): void
+    {
+        $nullable?->foo()->bar();
+    }
+}

tests/PHPStan/data/method-chain-nullsafe-violations.php

@@ -0,0 +1,26 @@
+<?php declare(strict_types=1);
+
+namespace MLL\Utils\Tests\PHPStan\data;
+
+class MethodChainNullsafeViolations
+{
+    public function foo(int $arg = 0): self
+    {
+        return $this;
+    }
+
+    public function bar(int $arg = 0): self
+    {
+        return $this;
+    }
+
+    public function nullSafeChainWithArgs(?self $nullable): void
+    {
+        $nullable?->foo(1)?->bar(2);
+    }
+
+    public function mixedNullSafeChainWithArgs(?self $nullable): void
+    {
+        $nullable?->foo(1)->bar(2);
+    }
+}