From 482e0ed1d1ab742b91a03f49b1a3f45485aa8214 Mon Sep 17 00:00:00 2001 From: Bogdan Kharchenko Date: Mon, 8 Jun 2026 09:50:13 -0400 Subject: [PATCH 1/5] Add AvoidHigherOrderCollectionProxies linter --- .../AvoidHigherOrderCollectionProxies.php | 64 +++++++++++++++++ src/Presets/LaraLint.php | 2 + .../AvoidHigherOrderCollectionProxiesTest.php | 68 +++++++++++++++++++ 3 files changed, 134 insertions(+) create mode 100644 src/Linters/AvoidHigherOrderCollectionProxies.php create mode 100644 tests/Linters/AvoidHigherOrderCollectionProxiesTest.php diff --git a/src/Linters/AvoidHigherOrderCollectionProxies.php b/src/Linters/AvoidHigherOrderCollectionProxies.php new file mode 100644 index 0000000..5cddd9e --- /dev/null +++ b/src/Linters/AvoidHigherOrderCollectionProxies.php @@ -0,0 +1,64 @@ +treeMatcher() + ->withChild(function(MemberAccessExpression $node) { + // We only care about a proxy method accessed as a property, e.g. + // the `->each` in `$collection->each->delete()`. A real method + // call (`$collection->each(...)`) parses with this node as the + // callable of a CallExpression, so requiring the parent to chain + // another member access off of us excludes it cleanly. + $member_name = $node->memberName; + + if (! $member_name instanceof Token) { + return false; + } + + if (! in_array($member_name->getText($node->getFileContents()), static::PROXIES, true)) { + return false; + } + + return $node->parent instanceof MemberAccessExpression + && $node->parent->dereferencableExpression === $node; + }); + } + + protected function onMatch(Collection $nodes): ?Result + { + $node = $nodes->first(); + $method = $node->memberName->getText($node->getFileContents()); + + return new Result( + $this, + $node, + "Call ->{$method}() with a closure rather than using the magic higher-order collection proxy." + ); + } +} diff --git a/src/Presets/LaraLint.php b/src/Presets/LaraLint.php index 705accd..4eaa21b 100644 --- a/src/Presets/LaraLint.php +++ b/src/Presets/LaraLint.php @@ -4,6 +4,7 @@ use Glhd\LaraLint\Contracts\Preset; use Glhd\LaraLint\Linters\AvoidGlobalFacadeAliases; +use Glhd\LaraLint\Linters\AvoidHigherOrderCollectionProxies; use Glhd\LaraLint\Linters\AvoidViewCompact; use Glhd\LaraLint\Linters\AvoidViewWith; use Glhd\LaraLint\Linters\DoNotApplyMiddlewareInControllers; @@ -35,6 +36,7 @@ public function linters(): Collection AvoidViewCompact::class, PreferAuthId::class, AvoidGlobalFacadeAliases::class, + AvoidHigherOrderCollectionProxies::class, OrderClassMembers::class, OrderModelMembers::class, PreferFullyRestfulControllers::class, diff --git a/tests/Linters/AvoidHigherOrderCollectionProxiesTest.php b/tests/Linters/AvoidHigherOrderCollectionProxiesTest.php new file mode 100644 index 0000000..5e6c3b9 --- /dev/null +++ b/tests/Linters/AvoidHigherOrderCollectionProxiesTest.php @@ -0,0 +1,68 @@ +comments() + ->where('user_id', $post->user_id) + ->get() + ->each + ->delete(); + END_SOURCE; + + $this->withLinter(AvoidHigherOrderCollectionProxies::class) + ->lintSource($source) + ->assertLintingResult('Call ->each() with a closure rather than using the magic higher-order collection proxy.'); + } + + public function test_it_flags_a_proxied_property_access(): void + { + $source = <<<'END_SOURCE' + $names = $users->map->name; + END_SOURCE; + + $this->withLinter(AvoidHigherOrderCollectionProxies::class) + ->lintSource($source) + ->assertLintingResult('Call ->map() with a closure rather than using the magic higher-order collection proxy.'); + } + + public function test_it_does_not_flag_a_normal_method_call(): void + { + $source = <<<'END_SOURCE' + $users->each(fn ($user) => $user->delete()); + END_SOURCE; + + $this->withLinter(AvoidHigherOrderCollectionProxies::class) + ->lintSource($source) + ->assertNoLintingResults(); + } + + public function test_it_does_not_flag_unrelated_nested_property_access(): void + { + $source = <<<'END_SOURCE' + $value = $config->database->host; + END_SOURCE; + + $this->withLinter(AvoidHigherOrderCollectionProxies::class) + ->lintSource($source) + ->assertNoLintingResults(); + } + + public function test_it_does_not_flag_a_dynamic_member_name(): void + { + $source = <<<'END_SOURCE' + $result = $collection->$method->value; + END_SOURCE; + + $this->withLinter(AvoidHigherOrderCollectionProxies::class) + ->lintSource($source) + ->assertNoLintingResults(); + } +} From 19aaf7eb953a04d510d941bec903db69396dfb78 Mon Sep 17 00:00:00 2001 From: Bogdan Kharchenko Date: Mon, 8 Jun 2026 10:18:41 -0400 Subject: [PATCH 2/5] Pin testbench versions per Laravel matrix branch and split composer require/update in CI Add testbench version to each Laravel matrix branch, require laravel/framework and orchestra/testbench together with --no-update before a full composer update, disable audit.block-insecure, and fix the coverage-publish condition to match '12.*' --- .github/workflows/phpunit.yml | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/.github/workflows/phpunit.yml b/.github/workflows/phpunit.yml index 2d19d44..3bd6a3e 100644 --- a/.github/workflows/phpunit.yml +++ b/.github/workflows/phpunit.yml @@ -16,6 +16,13 @@ jobs: laravel: [ 11.*, 12.*, 13.x] dependency-version: [ lowest, stable ] php: [ 8.3, 8.4, 8.5 ] + include: + - laravel: 11.* + testbench: ^9.16.0 + - laravel: 12.* + testbench: ^10.9.0 + - laravel: 13.x + testbench: ^11.0.0 name: ${{ matrix.laravel }} on ${{ matrix.php }} (${{ matrix.dependency-version }} deps) @@ -47,13 +54,22 @@ jobs: - name: Install dependencies run: | - composer require --no-interaction --prefer-dist --update-with-all-dependencies --prefer-${{ matrix.dependency-version }} "illuminate/support:${{ matrix.laravel }}" + # The matrix exercises older-but-still-supported Laravel branches, so + # don't let advisories against those branches block the test install. + composer config audit.block-insecure false + # Pin the framework and testbench together so the whole graph (testbench + # included) moves to the matrix's Laravel version rather than leaving + # testbench pinned high and creating an unsolvable conflict. + composer require --no-interaction --no-update \ + "laravel/framework:${{ matrix.laravel }}" \ + "orchestra/testbench:${{ matrix.testbench }}" + composer update --no-interaction --prefer-dist --prefer-${{ matrix.dependency-version }} --with-all-dependencies - name: Execute tests run: vendor/bin/phpunit --coverage-html "${{ github.workspace }}/code-coverage" - name: Publish code coverage - if: matrix.laravel == 12 && matrix.dependency-version == 'stable' + if: matrix.laravel == '12.*' && matrix.dependency-version == 'stable' uses: actions/upload-artifact@v4 with: name: code-coverage From 50f819ea77497762fdae126181837ef1aa17f97b Mon Sep 17 00:00:00 2001 From: Bogdan Kharchenko Date: Mon, 8 Jun 2026 11:05:17 -0400 Subject: [PATCH 3/5] Update orchestra/testbench dev requirement to ^v9.17.0|^v10.11.0|^v11.1.0 --- composer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/composer.json b/composer.json index 4e1c3a7..c28addc 100644 --- a/composer.json +++ b/composer.json @@ -21,7 +21,7 @@ }, "require-dev": { "laravel/framework": "^11.0|^12.0|^13.0", - "orchestra/testbench": "^v9.16.0|^v10.9.0|^11.0.0", + "orchestra/testbench": "^v9.17.0|^v10.11.0|^v11.1.0", "phpunit/phpunit": "^11.0|^12.5|^13.0-dev" }, "autoload": { From 5c6042155f2d51066d7f233614450c3631a0ba0f Mon Sep 17 00:00:00 2001 From: Bogdan Kharchenko Date: Mon, 8 Jun 2026 11:05:51 -0400 Subject: [PATCH 4/5] Revert "Merge branch 'avoid-higher-order-proxies' of https://github.com/glhd/laralint into avoid-higher-order-proxies" This reverts commit 6917b9288f106910776a11e75afa9cd95b4ed6c2, reversing changes made to 50f819ea77497762fdae126181837ef1aa17f97b. --- .github/workflows/phpunit.yml | 20 ++------------------ 1 file changed, 2 insertions(+), 18 deletions(-) diff --git a/.github/workflows/phpunit.yml b/.github/workflows/phpunit.yml index 3bd6a3e..2d19d44 100644 --- a/.github/workflows/phpunit.yml +++ b/.github/workflows/phpunit.yml @@ -16,13 +16,6 @@ jobs: laravel: [ 11.*, 12.*, 13.x] dependency-version: [ lowest, stable ] php: [ 8.3, 8.4, 8.5 ] - include: - - laravel: 11.* - testbench: ^9.16.0 - - laravel: 12.* - testbench: ^10.9.0 - - laravel: 13.x - testbench: ^11.0.0 name: ${{ matrix.laravel }} on ${{ matrix.php }} (${{ matrix.dependency-version }} deps) @@ -54,22 +47,13 @@ jobs: - name: Install dependencies run: | - # The matrix exercises older-but-still-supported Laravel branches, so - # don't let advisories against those branches block the test install. - composer config audit.block-insecure false - # Pin the framework and testbench together so the whole graph (testbench - # included) moves to the matrix's Laravel version rather than leaving - # testbench pinned high and creating an unsolvable conflict. - composer require --no-interaction --no-update \ - "laravel/framework:${{ matrix.laravel }}" \ - "orchestra/testbench:${{ matrix.testbench }}" - composer update --no-interaction --prefer-dist --prefer-${{ matrix.dependency-version }} --with-all-dependencies + composer require --no-interaction --prefer-dist --update-with-all-dependencies --prefer-${{ matrix.dependency-version }} "illuminate/support:${{ matrix.laravel }}" - name: Execute tests run: vendor/bin/phpunit --coverage-html "${{ github.workspace }}/code-coverage" - name: Publish code coverage - if: matrix.laravel == '12.*' && matrix.dependency-version == 'stable' + if: matrix.laravel == 12 && matrix.dependency-version == 'stable' uses: actions/upload-artifact@v4 with: name: code-coverage From 8c2833c9bc8a6eefc97f5a1eb322b58f01e4d8e0 Mon Sep 17 00:00:00 2001 From: Chris Morrell Date: Mon, 8 Jun 2026 12:26:03 -0400 Subject: [PATCH 5/5] Use reflection for proxy method names --- .../AvoidHigherOrderCollectionProxies.php | 36 ++++++++----------- .../AvoidHigherOrderCollectionProxiesTest.php | 11 ++++++ 2 files changed, 26 insertions(+), 21 deletions(-) diff --git a/src/Linters/AvoidHigherOrderCollectionProxies.php b/src/Linters/AvoidHigherOrderCollectionProxies.php index 5cddd9e..6ed21a3 100644 --- a/src/Linters/AvoidHigherOrderCollectionProxies.php +++ b/src/Linters/AvoidHigherOrderCollectionProxies.php @@ -8,24 +8,12 @@ use Illuminate\Support\Collection; use Microsoft\PhpParser\Node\Expression\MemberAccessExpression; use Microsoft\PhpParser\Token; +use ReflectionProperty; class AvoidHigherOrderCollectionProxies extends MatchingLinter { - /** - * The methods Laravel exposes as higher-order collection proxies. Accessing - * any of these as a property (rather than calling them) returns a proxy that - * forwards a subsequent member access to every item in the collection. - * - * @see \Illuminate\Collections\Traits\EnumeratesValues::$proxies - */ - protected const array PROXIES = [ - 'average', 'avg', 'contains', 'doesntContain', 'each', 'every', 'filter', - 'first', 'flatMap', 'groupBy', 'hasMany', 'hasSole', 'keyBy', 'last', 'map', - 'max', 'min', 'partition', 'percentage', 'reject', 'skipUntil', 'skipWhile', - 'some', 'sortBy', 'sortByDesc', 'sum', 'takeUntil', 'takeWhile', 'unique', - 'unless', 'until', 'when', - ]; - + protected array $proxy_methods; + protected function matcher(): Matcher { return $this->treeMatcher() @@ -36,29 +24,35 @@ protected function matcher(): Matcher // callable of a CallExpression, so requiring the parent to chain // another member access off of us excludes it cleanly. $member_name = $node->memberName; - + if (! $member_name instanceof Token) { return false; } - - if (! in_array($member_name->getText($node->getFileContents()), static::PROXIES, true)) { + + if (! in_array($member_name->getText($node->getFileContents()), $this->proxyMethods(), true)) { return false; } - + return $node->parent instanceof MemberAccessExpression && $node->parent->dereferencableExpression === $node; }); } - + + /** @param \Illuminate\Support\Collection $nodes */ protected function onMatch(Collection $nodes): ?Result { $node = $nodes->first(); $method = $node->memberName->getText($node->getFileContents()); - + return new Result( $this, $node, "Call ->{$method}() with a closure rather than using the magic higher-order collection proxy." ); } + + protected function proxyMethods(): array + { + return $this->proxy_methods ??= (new ReflectionProperty(Collection::class, 'proxies'))->getValue(); + } } diff --git a/tests/Linters/AvoidHigherOrderCollectionProxiesTest.php b/tests/Linters/AvoidHigherOrderCollectionProxiesTest.php index 5e6c3b9..8358bae 100644 --- a/tests/Linters/AvoidHigherOrderCollectionProxiesTest.php +++ b/tests/Linters/AvoidHigherOrderCollectionProxiesTest.php @@ -65,4 +65,15 @@ public function test_it_does_not_flag_a_dynamic_member_name(): void ->lintSource($source) ->assertNoLintingResults(); } + + public function test_it_does_not_flag_a_variable_named_a_proxy_method(): void + { + $source = <<<'END_SOURCE' + $result = $map->get(); + END_SOURCE; + + $this->withLinter(AvoidHigherOrderCollectionProxies::class) + ->lintSource($source) + ->assertNoLintingResults(); + } }