closed resource is not falsey

[SignpostMarv]

https://psalm.dev/r/5439dec258 https://3v4l.org/rraD2

[psalm-github-bot]

I found these snippets:

https://psalm.dev/r/5439dec258

<?php
$fp = tmpfile();

if ($fp && is_resource($fp)) { // has just opened but not *likely* to be redundant
  echo 'foo', "\n";
} else {
  echo 'bar', "\n";
}

echo var_export([$fp, is_resource($fp), !! $fp], true);

fclose($fp);

if ($fp && is_resource($fp)) { // not a resource but not falsey, should not error in object context
  echo 'baz', "\n";
} else {
  echo 'bat', "\n";
}

echo var_export([$fp, is_resource($fp), !! $fp], true);

Psalm output (using commit 389af1b):

ERROR: RedundantCondition - 4:12 - Found a redundant condition when evaluating $fp and trying to reconcile type 'resource' to resource

ERROR: RedundantCondition - 4:5 - Found a redundant condition when evaluating $fp and trying to reconcile type 'resource' to resource

ERROR: RedundantCondition - 14:12 - Found a redundant condition when evaluating $fp and trying to reconcile type 'resource' to resource

ERROR: RedundantCondition - 14:5 - Found a redundant condition when evaluating $fp and trying to reconcile type 'resource' to resource

[weirdan]

Related: #2266

[SignpostMarv]

suggestion: open-resource pseudo-type, true branch of is_resource() condition nudges variable/parameter from resource to open-resource ?

[TysonAndre]

And fopen()'s signatures (etc.) could be changed to return open-resource.
Another possibility: Emit a different issue type or none at all for is_resource()

This wouldn't solve the original issue, but changing the input argument to closed-resource via plugin or otherwise for native functions such as fclose(), curl_close(), etc and warning about passing closed-resource would detect some bugs.

However, using resources directly is probably uncommon outside of composer libraries such as http clients.

Analyzing $this->file_handle across methods might also be error-prone

[SignpostMarv]

currently tinkering with this, but the param change doesn't seem to stick so I'm guessing it's not the right place to make the change?

diff --git a/src/Psalm/Internal/Analyzer/FunctionAnalyzer.php b/src/Psalm/Internal/Analyzer/FunctionAnalyzer.php
index 9a86bc7a..b2e5e002 100644
--- a/src/Psalm/Internal/Analyzer/FunctionAnalyzer.php
+++ b/src/Psalm/Internal/Analyzer/FunctionAnalyzer.php
@@ -325,6 +325,49 @@ class FunctionAnalyzer extends FunctionLikeAnalyzer
                     // Really this should only work on instances we've created with new Foo(),
                     // but that requires more work
                     break;
+                case 'is_resource':
+                    if (isset($call_args[0])) {
+                        $first_arg_type = $statements_analyzer->node_data->getType($call_args[0]->value);
+
+                        if ($first_arg_type) {
+                            $is_already_closed = $codebase->isTypeContainedByType(
+                                $first_arg_type,
+                                new Type\Union([new Type\Atomic\TResourceClosed])
+                            );
+
+                            if ($is_already_closed) {
+                                return Type::getFalse();
+                            }
+                        }
+                    }
+                    break;
+                case 'fclose':
+                    if (isset($call_args[0])) {
+                        $first_arg_type = $statements_analyzer->node_data->getType($call_args[0]->value);
+
+                        if ($first_arg_type) {
+                            $is_already_closed = $codebase->isTypeContainedByType(
+                                $first_arg_type,
+                                new Type\Union([new Type\Atomic\TResourceClosed, new Type\Atomic\TFalse])
+                            );
+
+                            if ($is_already_closed) {
+                                return Type::getFalse();
+                            }
+
+                            $is_expected_type = $codebase->isTypeContainedByType(
+                                $first_arg_type,
+                                new Type\Union([new Type\Atomic\TResource])
+                            );
+
+                            if ($is_expected_type) {
+                                $first_arg_type = $statements_analyzer->node_data->setType($call_args[0]->value, Type::getClosedResource());
+
+                                return Type::getBool();
+                            }
+                        }
+                    }
+                    break;
             }
         }

testing against:

<?php
$fp = tmpfile();

fclose($fp); // should set $fp to new Union([new TResourceClosed])

if (fclose($fp)) { // expected to be a redundant condition
    echo 'foo';
} else {
    echo 'bar';
}

if (is_resource($fp)) { // expected to be a redundant condition
    echo 'foo';
} else {
    echo 'bar';
}