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

ForeachToInArrayRector should take into account Doctrine's Collection #1533

Closed
Aerendir opened this issue Jun 3, 2019 · 2 comments
Closed
Labels

Comments

@Aerendir
Copy link
Contributor

Aerendir commented Jun 3, 2019

     /**
@@ -141,12 +135,6 @@
      */
     private function isConsignee(RemoteOrder $order, User $user): bool
     {
-        foreach ($user->getEmails() as $email) {
-            if ($email === $order->getSrcShippingEmail()) {
-                return true;
-            }
-        }
-
-        return false;
+        return in_array($order->getSrcShippingEmail(), $user->getEmails(), true);
     }
 }
    ----------- end diff -----------

Applied rectors:

 * Rector\CodeQuality\Rector\Foreach_\ForeachToInArrayRector
/**
 * @ORM\Entity
 * @ORM\Table(name="users")
 */
class User extends BaseUser
{
    /**
     * @var ArrayCollection
     *
     * @ORM\OneToMany(targetEntity="App\Entity\Email", mappedBy="forUser")
     */
    protected $emails;

    /**
     * @return Collection|Email[]
     */
    public function getEmails(): Collection
    {
        return $this->emails;
    }

As you can see, and as pointed out in phpstan/phpstan#2176, Collection cannot be passed to in_array().

Here there are two problems:

  1. Rector changes the code to use in_array();
  2. In case of Doctrine's Collection, it should be better to use Collection::contains().
@TomasVotruba
Copy link
Member

I see. In that case, it should be skipped completely, because ForeachToInArrayRector should not handle any kinds of collections.

Could you send failing test (no change is expected)?

@Aerendir
Copy link
Contributor Author

Aerendir commented Jun 4, 2019

Failing test case created!

TomasVotruba added a commit that referenced this issue Jun 4, 2019
[CodeQuality] Skip collections ForeachToInArrayRector [closes #1533]
TomasVotruba added a commit that referenced this issue Dec 20, 2021
rectorphp/rector-src@c51fcbf [Core] [DeadCode] Refactor RectifiedAnalyzer and ExprUsedInNextNodeAnalyzer (#1533)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants