From 4e98afe22cd401a114a353e12e54e0be8a67babb Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Wed, 31 Jul 2024 19:07:28 +0200 Subject: [PATCH] Add AvoidNegatedCollectionFilterOrRejectRector (#234) * Add AvoidNegatedCollectionFilterOrRejectRector * Add Collection set, improve docs for sets * cast values that are not surely bool * add return types * try adding stub * convert all Enumerable * reorder sets * Fix annotation * explain * improve example * update docs * update docs * . * Revert sets documentation update --- README.md | 8 +- composer.json | 4 +- config/sets/laravel-collection.php | 11 ++ docs/rector_rules_overview.md | 22 +++- ...dNegatedCollectionFilterOrRejectRector.php | 111 ++++++++++++++++++ src/Set/LaravelSetList.php | 19 +-- stubs/Illuminate/Support/Collection.php | 20 ++++ stubs/Illuminate/Support/Enumerable.php | 23 ++++ ...atedCollectionFilterOrRejectRectorTest.php | 31 +++++ .../Fixture/fixture_cast.php.inc | 27 +++++ .../Fixture/fixture_return_type.php.inc | 27 +++++ .../Fixture/fixture_simple_negation.php.inc | 35 ++++++ .../config/configured_rule.php | 12 ++ 13 files changed, 336 insertions(+), 14 deletions(-) create mode 100644 config/sets/laravel-collection.php create mode 100644 src/Rector/MethodCall/AvoidNegatedCollectionFilterOrRejectRector.php create mode 100644 stubs/Illuminate/Support/Collection.php create mode 100644 tests/Rector/MethodCall/AvoidNegatedCollectionFilterOrRejectRector/AvoidNegatedCollectionFilterOrRejectRectorTest.php create mode 100644 tests/Rector/MethodCall/AvoidNegatedCollectionFilterOrRejectRector/Fixture/fixture_cast.php.inc create mode 100644 tests/Rector/MethodCall/AvoidNegatedCollectionFilterOrRejectRector/Fixture/fixture_return_type.php.inc create mode 100644 tests/Rector/MethodCall/AvoidNegatedCollectionFilterOrRejectRector/Fixture/fixture_simple_negation.php.inc create mode 100644 tests/Rector/MethodCall/AvoidNegatedCollectionFilterOrRejectRector/config/configured_rule.php diff --git a/README.md b/README.md index 20350548..59afca8d 100644 --- a/README.md +++ b/README.md @@ -1,4 +1,5 @@ # Rector Rules for Laravel + [![Tests](https://github.com/driftingly/rector-laravel/actions/workflows/tests.yaml/badge.svg)](https://github.com/driftingly/rector-laravel/actions/workflows/tests.yaml) [![Code Analysis](https://github.com/driftingly/rector-laravel/actions/workflows/code_analysis.yaml/badge.svg)](https://github.com/driftingly/rector-laravel/actions/workflows/code_analysis.yaml) [![Packagist Downloads](https://img.shields.io/packagist/dm/driftingly/rector-laravel)](https://packagist.org/packages/driftingly/rector-laravel/stats) @@ -12,10 +13,10 @@ This package is a [Rector](https://github.com/rectorphp/rector) extension develo Rules for additional first party packages are included as well e.g. Cashier and Livewire. -Install the `RectorLaravel` package as dependency: +Install as a dev dependency: ```bash -composer require driftingly/rector-laravel --dev +composer require --dev driftingly/rector-laravel ``` ## Use Sets @@ -39,15 +40,16 @@ return RectorConfig::configure() | Set | Purpose | |---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| +| [LaravelSetList::LARAVEL_ARRAYACCESS_TO_METHOD_CALL](https://github.com/driftingly/rector-laravel/blob/main/config/sets/laravel-arrayaccess-to-method-call.php) | Converts uses of things like `$app['config']` to `$app->make('config')`. | | [LaravelSetList::LARAVEL_ARRAY_STR_FUNCTION_TO_STATIC_CALL](https://github.com/driftingly/rector-laravel/blob/main/config/sets/laravel-array-str-functions-to-static-call.php) | Converts most string and array helpers into Str and Arr Facades' static calls.
https://laravel.com/docs/11.x/facades#facades-vs-helper-functions | | [LaravelSetList::LARAVEL_CODE_QUALITY](https://github.com/driftingly/rector-laravel/blob/main/config/sets/laravel-code-quality.php) | Replaces magical call on `$this->app["something"]` to standalone variable with PHPDocs. | +| [LaravelSetList::LARAVEL_COLLECTION](https://github.com/driftingly/rector-laravel/blob/main/config/sets/laravel-collection.php) | Improves the usage of Laravel Collections by using simpler, more efficient, or more readable methods. | | [LaravelSetList::LARAVEL_CONTAINER_STRING_TO_FULLY_QUALIFIED_NAME](https://github.com/driftingly/rector-laravel/blob/main/config/sets/laravel-container-string-to-fully-qualified-name.php) | Changes the string or class const used for a service container make call.
https://laravel.com/docs/11.x/container#the-make-method | | [LaravelSetList::LARAVEL_ELOQUENT_MAGIC_METHOD_TO_QUERY_BUILDER](https://github.com/driftingly/rector-laravel/blob/main/config/sets/laravel-eloquent-magic-method-to-query-builder.php) | Transforms magic method calls on Eloquent Models into corresponding Query Builder method calls.
https://laravel.com/docs/11.x/eloquent | | [LaravelSetList::LARAVEL_FACADE_ALIASES_TO_FULL_NAMES](https://github.com/driftingly/rector-laravel/blob/main/config/sets/laravel-facade-aliases-to-full-names.php) | Replaces Facade aliases with full Facade names.
https://laravel.com/docs/11.x/facades#facade-class-reference | | [LaravelSetList::LARAVEL_IF_HELPERS](https://github.com/driftingly/rector-laravel/blob/main/config/sets/laravel-if-helpers.php) | Replaces `abort()`, `report()`, `throw` statements inside conditions with `abort_if()`, `report_if()`, `throw_if()` function calls.
https://laravel.com/docs/11.x/helpers#method-abort-if | | [LaravelSetList::LARAVEL_LEGACY_FACTORIES_TO_CLASSES](https://github.com/driftingly/rector-laravel/blob/main/config/sets/laravel-legacy-factories-to-classes.php) | Migrates Eloquent legacy model factories (with closures) into class based factories.
https://laravel.com/docs/8.x/releases#model-factory-classes | | [LaravelSetList::LARAVEL_STATIC_TO_INJECTION](https://github.com/driftingly/rector-laravel/blob/main/config/sets/laravel-static-to-injection.php) | Replaces Laravel's Facades with Dependency Injection.
https://tomasvotruba.com/blog/2019/03/04/how-to-turn-laravel-from-static-to-dependency-injection-in-one-day/
https://laravel.com/docs/11.x/facades#facades-vs-dependency-injection | -| [LaravelSetList::LARAVEL_ARRAYACCESS_TO_METHOD_CALL](https://github.com/driftingly/rector-laravel/blob/main/config/sets/laravel-arrayaccess-to-method-call.php) | Converts uses of things like `$app['config']` to `$app->make('config')`. | ## Contributors diff --git a/composer.json b/composer.json index e3071c8c..a4884337 100644 --- a/composer.json +++ b/composer.json @@ -38,9 +38,7 @@ "fix": "vendor/bin/duster fix", "rector-dry-run": "vendor/bin/rector process --dry-run --ansi", "rector": "vendor/bin/rector process --ansi", - "docs": [ - "vendor/bin/rule-doc-generator generate src --output-file docs/rector_rules_overview.md --ansi" - ] + "docs": "vendor/bin/rule-doc-generator generate src --output-file docs/rector_rules_overview.md --ansi" }, "minimum-stability": "dev", "prefer-stable": true, diff --git a/config/sets/laravel-collection.php b/config/sets/laravel-collection.php new file mode 100644 index 00000000..84cd852d --- /dev/null +++ b/config/sets/laravel-collection.php @@ -0,0 +1,11 @@ +import(__DIR__ . '/../config.php'); + $rectorConfig->rule(AvoidNegatedCollectionFilterOrRejectRector::class); +}; diff --git a/docs/rector_rules_overview.md b/docs/rector_rules_overview.md index a2180aa8..b38572a5 100644 --- a/docs/rector_rules_overview.md +++ b/docs/rector_rules_overview.md @@ -1,4 +1,4 @@ -# 62 Rules Overview +# 63 Rules Overview ## AbortIfRector @@ -350,6 +350,26 @@ Replace `(new \Illuminate\Testing\TestResponse)->assertStatus(200)` with `(new \
+## AvoidNegatedCollectionFilterOrRejectRector + +Avoid negated conditionals in `filter()` by using `reject()`, or vice versa. + +- class: [`RectorLaravel\Rector\MethodCall\AvoidNegatedCollectionFilterOrRejectRector`](../src/Rector/MethodCall/AvoidNegatedCollectionFilterOrRejectRector.php) + +```diff + use Illuminate\Support\Collection; + + $collection = new Collection([0, 1, null, -1]); +-$collection->filter(fn (?int $number): bool => ! is_null($number)); +-$collection->filter(fn (?int $number): bool => ! $number); +-$collection->reject(fn (?int $number) => ! $number > 0); ++$collection->reject(fn (?int $number): bool => is_null($number)); // Avoid negation ++$collection->reject(fn (?int $number): bool => (bool) $number); // Ensures explicit cast ++$collection->filter(fn (?int $number): bool => $number > 0); // Adds return type +``` + +
+ ## CallOnAppArrayAccessToStandaloneAssignRector Replace magical call on `$this->app["something"]` to standalone type assign variable diff --git a/src/Rector/MethodCall/AvoidNegatedCollectionFilterOrRejectRector.php b/src/Rector/MethodCall/AvoidNegatedCollectionFilterOrRejectRector.php new file mode 100644 index 00000000..a3c00b21 --- /dev/null +++ b/src/Rector/MethodCall/AvoidNegatedCollectionFilterOrRejectRector.php @@ -0,0 +1,111 @@ +filter(fn (?int $number): bool => ! is_null($number)); +$collection->filter(fn (?int $number): bool => ! $number); +$collection->reject(fn (?int $number) => ! $number > 0); +CODE_SAMPLE + , + <<<'CODE_SAMPLE' +use Illuminate\Support\Collection; + +$collection = new Collection([0, 1, null, -1]); +$collection->reject(fn (?int $number): bool => is_null($number)); // Avoid negation +$collection->reject(fn (?int $number): bool => (bool) $number); // Explicitly cast +$collection->filter(fn (?int $number): bool => $number > 0); // Adds return type +CODE_SAMPLE + ), + ] + ); + } + + /** + * @return array> + */ + public function getNodeTypes(): array + { + return [MethodCall::class]; + } + + /** + * @param MethodCall $node + */ + public function refactor(Node $node): ?Node + { + return $this->updateFilterOrRejectCall($node); + } + + private function updateFilterOrRejectCall(MethodCall $methodCall): ?MethodCall + { + if (! $this->isObjectType($methodCall->var, new ObjectType('Illuminate\Support\Enumerable'))) { + return null; + } + + if (! $this->isNames($methodCall->name, ['filter', 'reject'])) { + return null; + } + + $args = $methodCall->getArgs(); + if (count($args) !== 1) { + return null; + } + + $arg = $args[0]; + $argValue = $arg->value; + + if (! $argValue instanceof ArrowFunction) { + return null; + } + + $return = $argValue->expr; + if (! $return instanceof BooleanNot) { + return null; + } + + $methodCall->name = new Identifier( + $this->isName($methodCall->name, 'filter') + ? 'reject' + : 'filter' + ); + + // Since negation implicitly casts to boolean, we need to replace it with an + // explicit cast - unless the value is already a boolean. + $returnExpr = $return->expr; + $argValue->expr = $this->getType($returnExpr)->isBoolean()->yes() + ? $returnExpr + : new Bool_($returnExpr); + + $argValue->returnType = new Identifier('bool'); + + return $methodCall; + } +} diff --git a/src/Set/LaravelSetList.php b/src/Set/LaravelSetList.php index 1e917921..ad49796a 100644 --- a/src/Set/LaravelSetList.php +++ b/src/Set/LaravelSetList.php @@ -91,27 +91,27 @@ final class LaravelSetList implements SetListInterface /** * @var string */ - final public const LARAVEL_STATIC_TO_INJECTION = __DIR__ . '/../../config/sets/laravel-static-to-injection.php'; + final public const LARAVEL_ARRAYACCESS_TO_METHOD_CALL = __DIR__ . '/../../config/sets/laravel-arrayaccess-to-method-call.php'; /** * @var string */ - final public const LARAVEL_CODE_QUALITY = __DIR__ . '/../../config/sets/laravel-code-quality.php'; + final public const LARAVEL_ARRAY_STR_FUNCTION_TO_STATIC_CALL = __DIR__ . '/../../config/sets/laravel-array-str-functions-to-static-call.php'; /** * @var string */ - final public const LARAVEL_ARRAY_STR_FUNCTION_TO_STATIC_CALL = __DIR__ . '/../../config/sets/laravel-array-str-functions-to-static-call.php'; + final public const LARAVEL_CODE_QUALITY = __DIR__ . '/../../config/sets/laravel-code-quality.php'; /** * @var string */ - final public const LARAVEL_LEGACY_FACTORIES_TO_CLASSES = __DIR__ . '/../../config/sets/laravel-legacy-factories-to-classes.php'; + final public const LARAVEL_COLLECTION = __DIR__ . '/../../config/sets/laravel-collection.php'; /** * @var string */ - final public const LARAVEL_FACADE_ALIASES_TO_FULL_NAMES = __DIR__ . '/../../config/sets/laravel-facade-aliases-to-full-names.php'; + final public const LARAVEL_CONTAINER_STRING_TO_FULLY_QUALIFIED_NAME = __DIR__ . '/../../config/sets/laravel-container-string-to-fully-qualified-name.php'; /** * @var string @@ -121,7 +121,7 @@ final class LaravelSetList implements SetListInterface /** * @var string */ - final public const LARAVEL_CONTAINER_STRING_TO_FULLY_QUALIFIED_NAME = __DIR__ . '/../../config/sets/laravel-container-string-to-fully-qualified-name.php'; + final public const LARAVEL_FACADE_ALIASES_TO_FULL_NAMES = __DIR__ . '/../../config/sets/laravel-facade-aliases-to-full-names.php'; /** * @var string @@ -131,5 +131,10 @@ final class LaravelSetList implements SetListInterface /** * @var string */ - final public const LARAVEL_ARRAYACCESS_TO_METHOD_CALL = __DIR__ . '/../../config/sets/laravel-arrayaccess-to-method-call.php'; + final public const LARAVEL_LEGACY_FACTORIES_TO_CLASSES = __DIR__ . '/../../config/sets/laravel-legacy-factories-to-classes.php'; + + /** + * @var string + */ + final public const LARAVEL_STATIC_TO_INJECTION = __DIR__ . '/../../config/sets/laravel-static-to-injection.php'; } diff --git a/stubs/Illuminate/Support/Collection.php b/stubs/Illuminate/Support/Collection.php new file mode 100644 index 00000000..73405ee2 --- /dev/null +++ b/stubs/Illuminate/Support/Collection.php @@ -0,0 +1,20 @@ + + */ +class Collection implements Enumerable +{ +} diff --git a/stubs/Illuminate/Support/Enumerable.php b/stubs/Illuminate/Support/Enumerable.php index cf1f8239..cb7f6a84 100644 --- a/stubs/Illuminate/Support/Enumerable.php +++ b/stubs/Illuminate/Support/Enumerable.php @@ -8,6 +8,29 @@ return; } +/** + * @template TKey of array-key + * + * @template-covariant TValue + * + * @extends \Illuminate\Contracts\Support\Arrayable + * @extends \IteratorAggregate + */ interface Enumerable { + /** + * Run a filter over each of the items. + * + * @param (callable(TValue): bool)|null $callback + * @return static + */ + public function filter(?callable $callback = null); + + /** + * Create a collection of all elements that do not pass a given truth test. + * + * @param (callable(TValue, TKey): bool)|bool|TValue $callback + * @return static + */ + public function reject($callback = true); } diff --git a/tests/Rector/MethodCall/AvoidNegatedCollectionFilterOrRejectRector/AvoidNegatedCollectionFilterOrRejectRectorTest.php b/tests/Rector/MethodCall/AvoidNegatedCollectionFilterOrRejectRector/AvoidNegatedCollectionFilterOrRejectRectorTest.php new file mode 100644 index 00000000..db0d7185 --- /dev/null +++ b/tests/Rector/MethodCall/AvoidNegatedCollectionFilterOrRejectRector/AvoidNegatedCollectionFilterOrRejectRectorTest.php @@ -0,0 +1,31 @@ +doTestFile($filePath); + } + + public function provideConfigFilePath(): string + { + return __DIR__ . '/config/configured_rule.php'; + } +} diff --git a/tests/Rector/MethodCall/AvoidNegatedCollectionFilterOrRejectRector/Fixture/fixture_cast.php.inc b/tests/Rector/MethodCall/AvoidNegatedCollectionFilterOrRejectRector/Fixture/fixture_cast.php.inc new file mode 100644 index 00000000..309d1ea0 --- /dev/null +++ b/tests/Rector/MethodCall/AvoidNegatedCollectionFilterOrRejectRector/Fixture/fixture_cast.php.inc @@ -0,0 +1,27 @@ +filter(fn (?int $number): bool => ! $number); + +(new Collection([0, 1, null, -1])) + ->reject(fn (?int $number): bool => ! $number); + +?> +----- +reject(fn (?int $number): bool => (bool) $number); + +(new Collection([0, 1, null, -1])) + ->filter(fn (?int $number): bool => (bool) $number); + +?> diff --git a/tests/Rector/MethodCall/AvoidNegatedCollectionFilterOrRejectRector/Fixture/fixture_return_type.php.inc b/tests/Rector/MethodCall/AvoidNegatedCollectionFilterOrRejectRector/Fixture/fixture_return_type.php.inc new file mode 100644 index 00000000..6f3ee24e --- /dev/null +++ b/tests/Rector/MethodCall/AvoidNegatedCollectionFilterOrRejectRector/Fixture/fixture_return_type.php.inc @@ -0,0 +1,27 @@ +filter(fn (?int $number) => ! is_null($number)); + +(new Collection([0, 1, null, -1])) + ->reject(fn (?int $number) => ! is_null($number)); + +?> +----- +reject(fn (?int $number): bool => is_null($number)); + +(new Collection([0, 1, null, -1])) + ->filter(fn (?int $number): bool => is_null($number)); + +?> diff --git a/tests/Rector/MethodCall/AvoidNegatedCollectionFilterOrRejectRector/Fixture/fixture_simple_negation.php.inc b/tests/Rector/MethodCall/AvoidNegatedCollectionFilterOrRejectRector/Fixture/fixture_simple_negation.php.inc new file mode 100644 index 00000000..efea1c86 --- /dev/null +++ b/tests/Rector/MethodCall/AvoidNegatedCollectionFilterOrRejectRector/Fixture/fixture_simple_negation.php.inc @@ -0,0 +1,35 @@ +filter(fn (?int $number): bool => ! is_null($number)) + ->reject(fn (int $number): bool => ! ($number > 0)); + +(new Collection([0, 1, null, -1])) + ->filter(fn (?int $number): bool => ! ($number > 0)); + +(new Collection([0, 1, null, -1])) + ->reject(fn (?int $number): bool => ! ($number > 0)); + +?> +----- +reject(fn (?int $number): bool => is_null($number)) + ->filter(fn (int $number): bool => $number > 0); + +(new Collection([0, 1, null, -1])) + ->reject(fn (?int $number): bool => $number > 0); + +(new Collection([0, 1, null, -1])) + ->filter(fn (?int $number): bool => $number > 0); + +?> diff --git a/tests/Rector/MethodCall/AvoidNegatedCollectionFilterOrRejectRector/config/configured_rule.php b/tests/Rector/MethodCall/AvoidNegatedCollectionFilterOrRejectRector/config/configured_rule.php new file mode 100644 index 00000000..de648d42 --- /dev/null +++ b/tests/Rector/MethodCall/AvoidNegatedCollectionFilterOrRejectRector/config/configured_rule.php @@ -0,0 +1,12 @@ +import(__DIR__ . '/../../../../../config/config.php'); + + $rectorConfig->rule(AvoidNegatedCollectionFilterOrRejectRector::class); +};