Add AvoidHigherOrderCollectionProxies linter

src/Linters/AvoidHigherOrderCollectionProxies.php

@@ -0,0 +1,58 @@
+<?php
+
+namespace Glhd\LaraLint\Linters;
+
+use Glhd\LaraLint\Contracts\Matcher;
+use Glhd\LaraLint\Linters\Strategies\MatchingLinter;
+use Glhd\LaraLint\Result;
+use Illuminate\Support\Collection;
+use Microsoft\PhpParser\Node\Expression\MemberAccessExpression;
+use Microsoft\PhpParser\Token;
+use ReflectionProperty;
+
+class AvoidHigherOrderCollectionProxies extends MatchingLinter
+{
+	protected array $proxy_methods;
+
+	protected function matcher(): Matcher
+	{
+		return $this->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()), $this->proxyMethods(), true)) {
+					return false;
+				}
+
+				return $node->parent instanceof MemberAccessExpression
+					&& $node->parent->dereferencableExpression === $node;
+			});
+	}
+
+	/** @param \Illuminate\Support\Collection<int, MemberAccessExpression> $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();
+	}
+}

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,

tests/Linters/AvoidHigherOrderCollectionProxiesTest.php

@@ -0,0 +1,79 @@
+<?php
+
+namespace Glhd\LaraLint\Tests\Linters;
+
+use Glhd\LaraLint\Linters\AvoidHigherOrderCollectionProxies;
+use Glhd\LaraLint\Tests\TestCase;
+
+class AvoidHigherOrderCollectionProxiesTest extends TestCase
+{
+	public function test_it_flags_a_proxied_method_call(): void
+	{
+		$source = <<<'END_SOURCE'
+		$post->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();
+	}
+
+	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();
+	}
+}