diff --git a/src/Linters/AvoidHigherOrderCollectionProxies.php b/src/Linters/AvoidHigherOrderCollectionProxies.php new file mode 100644 index 0000000..6ed21a3 --- /dev/null +++ b/src/Linters/AvoidHigherOrderCollectionProxies.php @@ -0,0 +1,58 @@ +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 $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/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..8358bae --- /dev/null +++ b/tests/Linters/AvoidHigherOrderCollectionProxiesTest.php @@ -0,0 +1,79 @@ +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(); + } +}