declare assert-if-true for filesystem functions

[staabm]

closes phpstan/phpstan#6788

[ondrejmirtes]

Two problems here:

1. chdir($dir) ? 'foo' : 'bar' - in the else branch $dir will be considered ''
2. If you pass non-empty-string to chdir(...), you'll get "always true" error.

Both can be solved with =non-empty-string. See: phpstan/phpstan#8348 + phpstan/phpstan#8351

[ondrejmirtes]

To make sure that = actually fixes these problems, you should have tested the else condition types and also put a test about this in ImpossibleCheckTypeFunctionCallRule (before adding the = everywhere). Please verify this was originally a problem and that it's fixed after the change. I'm sure you can find the previous version in your reflog :) It's e117fce or 49564b7.

[staabm]

good point.

2) PHPStan\Rules\Comparison\ImpossibleCheckTypeFunctionCallRuleTest::testBug6788
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-'
+'13: Call to function chdir() with non-empty-string will always evaluate to true.
+    💡 Because the type is coming from a PHPDoc, you can turn off this check by setting <fg=cyan>treatPhpDocTypesAsCertain: false</> in your <fg=cyan>%configurationFile%</>.
+15: Call to function chdir() with non-falsy-string will always evaluate to true.
+    💡 Because the type is coming from a PHPDoc, you can turn off this check by setting <fg=cyan>treatPhpDocTypesAsCertain: false</> in your <fg=cyan>%configurationFile%</>.
 '

[staabm]

the last commit is fixing cases like

<?php

namespace Bug4816;

use function PHPStan\Testing\assertType;

function (string $x): void {
	if (is_dir($x)) {
		assertType('true', is_dir($x));
	}
};

(so we don't regress 1eaef04)

---

my understanding is, that because this PR adds stubs with @phpstan-assert the specifier does no longer invoke handleDefaultTruthyOrFalseyContext in

phpstan-src/src/Analyser/TypeSpecifier.php

Lines 455 to 472 in 7cd03f0

	$assertions = $functionReflection->getAsserts();
	if ($assertions->getAll() !== []) {
	$parametersAcceptor ??= ParametersAcceptorSelector::selectFromArgs($scope, $expr->getArgs(), $functionReflection->getVariants(), $functionReflection->getNamedArgumentsVariants());
	$asserts = $assertions->mapTypes(static fn (Type $type) => TemplateTypeHelper::resolveTemplateTypes(
	$type,
	$parametersAcceptor->getResolvedTemplateTypeMap(),
	$parametersAcceptor instanceof ParametersAcceptorWithPhpDocs ? $parametersAcceptor->getCallSiteVarianceMap() : TemplateTypeVarianceMap::createEmpty(),
	TemplateTypeVariance::createInvariant(),
	));
	$specifiedTypes = $this->specifyTypesFromAsserts($context, $expr, $asserts, $parametersAcceptor, $scope);
	if ($specifiedTypes !== null) {
	return $specifiedTypes;
	}
	}
	}
	return $this->handleDefaultTruthyOrFalseyContext($context, $rootExpr, $expr, $scope);

and that the reason why my previous code sample did regress. After adding handleDefaultTruthyOrFalseyContext I can see other things regress though :).

@rvanvelzen could you give me a hand? why do we not invoke handleDefaultTruthyOrFalseyContext when taking the path thru specifyTypesFromAsserts. is this expected or an oversight?

btw: I noticed we also don't invoke handleDefaultTruthyOrFalseyContext when we take the path thru specifyTypesFromConditionalReturnType.

[thg2k]

I had no idea that the assertion would actually assert the opposite if the condition is not met. This is very counter-intuitive for me.. I think the behavior with "=" should be the default and there should rather be another modifier to request the double behavior.