Ready to be merged.

There are tons of reworks, as there is a bug in taint (#8477) that confused me, as I haven't used/worked with taint before and thought there's an issue in my code, while it was a bug in taint all along.

We might have to think about this for the same reasons given in #1087. Mutating $_GET and other superglobals is a bad idea, but it's a bad idea that's still somewhat common unfortunately.

You can still mutate $_GET just like before. The behavior for this isn't changed, is it?

Nevertheless, I think psalm should not account for niche, bad coding practices (since there's psalm-suppress and @var anyway) - there are very limited maintainer resources it being open source, so we shouldn't put too much thought into what is a 0.01% niche issue and a (very) bad coding practice in the first place to be honest. (but if anybody wants to fix it, we can still be open for it, I just don't think we should waste time with those things)

One way could be to make the more precise type inference a opt-in feature and also add a rule which reports errors when code modifies superglobals

add a rule which reports errors when code modifies superglobals

Definitely, but not within this PR. Please feel free to create a separate PR for that.

One way could be to make the more precise type inference a opt-in feature

It already is. Psalm already supports declaring any globals in your config. However this PR is not about that - it's about declaring what those ~6 generally available superglobals can contain in every environment (if you didn't specify that yourself in your config already).

This might be something we'd want to consider.

This might be something we'd want to consider.

Actually, that's not the only issue. You can also have an auto_prepend_file in php.ini which changes ALL of them (including these), therefore nothing is safe here. Adding any conditions/checks for these few, would create a false sense of "security", as all of them can be changed in an auto_prepend_file or any file that runs before the current one in fact - which is an issue with all globals anyway.
Alas, don't need to consider this for this PR.

which is an issue with all globals anyway

If we don't have one already, we should open an issue to make superglobals immutable.

@orklah all changes done, branch rebased.

Shepherd has a bug it seems, bc I don't get an error when I do this here: https://psalm.dev/r/66e2a12517 (also this error makes no sense)

$this->argv with declared type 'list' cannot be assigned type 'non-empty-list'

I think this is caused by having "possibly_undefined" and "ignore_nullable_issues" added as you suggested.
I mean I can just fix this in the test by changing it to non-empty-list, but it will be a pain for everybody else.
What do you suggest?

I found these snippets:

<?php

class Foo {
    /**
     * @var array<int, string>
     */
    private $argv = [];
    
    /**
     * @param non-empty-list<string> $arg
     */
    public function __construct($arg) {
    	$this->argv = $arg;
    }
}

Psalm output (using commit d957ff2):

No issues!

I think this is caused by having "possibly_undefined" and "ignore_nullable_issues" added as you suggested.

Yeah, it's probably due to the check for InvalidPropertyAssignmentValue considering that possibly_undefined could lead to null and not checking ignore_nullable. Can you coalesce with empty array here? It should solve the issue

Yes, but that's what I had before with TNull already, however the error made much more sense then.

Because the error now doesn't make any sense:

$this->argv with declared type 'list' cannot be assigned type 'non-empty-list'

The reason why it reports this is because it's nullable, but psalm hides the fact that it could be "null".

Maybe putting back TNull and removing possibly_undefined make it better? let me try that. If not, I think I'll revert it as it was, bc at least the error made sense then.

@orklah yeah it works how you want it when I do it with TNull and $argv_nullable->ignore_nullable_issues = true;

When using without TNull but doing:

$argv_nullable->possibly_undefined = true;
$argv_nullable->ignore_nullable_issues = true;

It reports those wrong errors.

Changed now.

Ready to be merged now.

Thanks!

I think this PR broke our CI. See my latest bug here: #8559