Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New Rule to use the whereLike clause in Laravel 11.x #267

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

GeniJaho
Copy link
Collaborator

@GeniJaho GeniJaho commented Oct 28, 2024

Adds a new rule WhereToWhereLike (open for a better name) that will migrate all the ->where('name', 'like', '%Rector%') clauses into the new whereLike() introduced in Laravel 11.x.

  • The rule has an optional configuration for the Postgres driver, since it uses a 'case-sensitive' search by default.
  • I would love to add it to the Laravel 11 and Code Quality sets, but we'd have to make a check in the rule for the Laravel version. In any case, it could cause issues for Postgres users if the rule is enabled by default, they'd have to configure it manually... and skip the one provided by the sets, somehow.

Comment on lines +85 to +87
if (! $this->isObjectType($node->var, new ObjectType('Illuminate\Database\Query\Builder'))) {
return null;
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You would be better off expanding this a little. Namely using the contract for the query builder instead of the object type.

*/
public function getNodeTypes(): array
{
return [MethodCall::class];
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could be expanded to Static calls for a Model as well, as Model::where('a', 'like', 'b') wouldn't be covered by this rule as it uses a proxy system.


use Illuminate\Database\Query\Builder;

class Fixture
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's best not to name classes in tests the same. I've found it causes weird issues. It's best to name the class WithPostgresDriver to match the file name.

return;
}

Assert::count($configuration, 1);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this line seems a bit redundant as we know it's an array and the key is checked. It should affect anything if the config contains more keys

Comment on lines +122 to +124
Assert::keyExists($configuration, 'usingPostgresDriver');
Assert::boolean($configuration['usingPostgresDriver']);
$this->usingPostgresDriver = $configuration['usingPostgresDriver'];
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be useful to introduce a const for the array key that's public or just have a static method that creates the array e.g. WhereToWhereLikeRector::usingPostgresDriver() Then update the code sample for it.

@GeniJaho GeniJaho added the enhancement New feature or request label Nov 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants