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

Bug: Iterating preg_match_all() matches no longer allowed in pure functions #4128

Closed
colinodell opened this issue Sep 4, 2020 · 6 comments
Closed
Labels

Comments

@colinodell
Copy link

Iterating over preg_match_all() matches within a pure function used to be allowed but now it causes an ImpureMethodCall error: Cannot call a possibly-mutating iterator from a pure context

Example: https://psalm.dev/r/f170f02c7e

I would expect this example to be allowed from within a pure function since it matches the definition of a pure function and the value of $matches can never be modified outside of that function.

The first time I noticed this error was in version 3.15. This error was not raised in 3.14.2.

I believe this might be related to #4064 but I'm not entirely sure.

@psalm-github-bot
Copy link

I found these snippets:

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

/**
 * @psalm-pure
 *
 * @return string[]
 */
function extractUsernames(string $input): array
{
    preg_match_all('/@[a-z\d](?:[a-z\d]|-(?=[a-z\d])){0,38}(?!\w)/', $input, $matches);
    
    $usernames = [];
    foreach ($matches[0] as $username) {
        $usernames[] = ltrim($username, '@');
    }
    
    return $usernames;
}

var_dump(extractUsernames('This issue was reported by @colinodell'));
Psalm output (using commit 66251d8):

INFO: PossiblyUndefinedIntArrayOffset - 13:14 - Possibly undefined array offset 'int(0)' is risky given expected type 'array-key'. Consider using isset beforehand.

ERROR: ImpureMethodCall - 13:5 - Cannot call a possibly-mutating iterator from a pure context

INFO: MixedAssignment - 13:29 - Unable to determine the type that $username is being assigned to

INFO: MixedArgument - 14:30 - Argument 1 of ltrim cannot be mixed, expecting string

ERROR: ForbiddenCode - 20:1 - Unsafe var_dump

@weirdan
Copy link
Collaborator

weirdan commented Sep 4, 2020

Simplified: https://psalm.dev/r/aa99f2c9fc

@psalm-github-bot
Copy link

I found these snippets:

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

/**
 * @psalm-pure
 *
 * @param mixed $input
 */
function extractUsernames($input): void
{
  foreach ($input as $_v) {}  
}
Psalm output (using commit 66251d8):

ERROR: ImpureMethodCall - 10:3 - Cannot call a possibly-mutating iterator from a pure context

@weirdan weirdan added the bug label Sep 4, 2020
@muglug
Copy link
Collaborator

muglug commented Sep 4, 2020

Yeah, what really needs to happen here is that Psalm understands the type of $matches much better

@weirdan
Copy link
Collaborator

weirdan commented Sep 4, 2020

Shouldn't there be a separate issue type, something like MixedIteration? Then ImpureMethodCall could be made more specific by only emitting it when iterating over a known Traversable?

@muglug
Copy link
Collaborator

muglug commented Sep 4, 2020

Yeah, you're right about that too

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

3 participants