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

False-positive ImpureMethodCall when calling method on an object that created inside of a pure method #8413

Open
fluffycondor opened this issue Aug 16, 2022 · 3 comments

Comments

@fluffycondor
Copy link
Contributor

https://psalm.dev/r/0310c9c472

If I understand correctly, this code shouldn't yield an error. It blocks psalm/psalm-plugin-doctrine#125 now, so I'd like to know can I safely suppress it.

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/0310c9c472
<?php

class Collection {
    /** @psalm-mutation-free Returns a new collection, doesn't mutate the existing one */
	public function matching(Criteria $criteria): Collection
    {
    	return new Collection();
    }
}

class Criteria {
    private ?string $key = null;
    
    /** @psalm-external-mutation-free Mutates only its own state */
	public function orderBy(string $key): Criteria
    {
        $this->key = $key;
        return $this;
    }
}

/** @psalm-pure */
function foo(Collection $collection): Collection
{
    return $collection->matching((new Criteria())->orderBy('key'));
}
Psalm output (using commit 42462b8):

ERROR: ImpureMethodCall - 25:52 - Cannot call a non-mutation-free method Criteria::orderBy from a pure context

@AndrolGenhald
Copy link
Collaborator

Yeah, this can be safely suppressed, duplicate of #5615. As weirdan mentioned there, it seems cloning fixes this, so we need to make it so that creating a new object works the same way as cloning.

If you trick Psalm by creating and then cloning it works fine: https://psalm.dev/r/c75ed63c79

I think that issue will probably have to be fixed before merging that PR though, as I understand it the PR will cause everyone that uses Criteria::orderBy to have the false positive that they have to suppress, rather than just being a suppression in the plugin itself.

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/c75ed63c79
<?php

class Collection {
    /** @psalm-mutation-free Returns a new collection, doesn't mutate the existing one */
	public function matching(Criteria $criteria): Collection
    {
    	return new Collection();
    }
}

class Criteria {
    private ?string $key = null;
    
    /** @psalm-external-mutation-free Mutates only its own state */
	public function orderBy(string $key): Criteria
    {
        $this->key = $key;
        return $this;
    }
}

/** @psalm-pure */
function foo(Collection $collection): Collection
{
    $criteria = new Criteria();
    $criteria = clone $criteria;
    $criteria = $criteria->orderBy('key');
    return $collection->matching($criteria);
}
Psalm output (using commit 42462b8):

No issues!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants