Fix #13963 with unit tests. #4786

niconoe- · 2026-01-19T13:16:18Z

VincentLanglet · 2026-01-19T15:02:44Z

Hi, I think you're not handling correctly the flag for non-string value

I recommend you to add test for

filter_var(true, options:FILTER_FLAG_EMPTY_STRING_NULL); // non-empty-string
filter_var(false, options:FILTER_FLAG_EMPTY_STRING_NULL); // null
filter_var($bool, options:FILTER_FLAG_EMPTY_STRING_NULL); // null
filter_var(0.0, options:FILTER_FLAG_EMPTY_STRING_NULL); // non-empty-string
filter_var(0, options:FILTER_FLAG_EMPTY_STRING_NULL); // non-empty-string
filter_var(null, options:FILTER_FLAG_EMPTY_STRING_NULL); // null
filter_var($nullable, options:FILTER_FLAG_EMPTY_STRING_NULL); // null|non-empty-string

You certainly need to do the check you did but after a toString call.

niconoe- · 2026-01-19T17:15:59Z

Thanks for your feedback! I added some cases and tried to be as precise as I could on type expectations. I feel like it's starting to get too complex when going deeper on union types. I'm not feeling fluent enough with PHPStan's API but I did my best.

Again, tell me if I got something wrong here 😉

VincentLanglet · 2026-01-19T21:33:38Z

I feel like it could be simpler to look for something like

if ($filterValue === $this->getConstant('FILTER_DEFAULT')) {
             $scalarOrNull = new UnionType([
				new StringType(),
				new FloatType(),
				new BooleanType(),
				new IntegerType(),
				new NullType(),
			]);
			if ($scalarOrNull->isSuperTypeOf($in)->yes()) {
				$canBeSanitized = $this->canStringBeSanitized($filterValue, $flagsType);
				if ($canBeSanitized->no()) {
					$stringType = $in->toString();
				} else {
					$stringType = $in->isString()->no()
						? $in->toString()
						: TypeCombinator::union(TypeCombinator::remove($in, new StringType()), new StringType());
				}

				return $this->handleEmptyStringNullFlag($stringType, $flagsType);
			}
		}

with

private function handleEmptyStringNullFlag(Type $in, ?Type $flagsType): Type
	{
		$hasFlag = $this->hasFlag('FILTER_FLAG_EMPTY_STRING_NULL', $flagsType);
		if ($hasFlag->no()) {
			return $in;
		}

		$hasEmptyString = !$in->isSuperTypeOf(new ConstantStringType(''))->no();
		if ($hasFlag->maybe()) {
			return $hasEmptyString ? TypeCombinator::addNull($in) : $in;
		}

		return $hasEmptyString ? TypeCombinator::remove(TypeCombinator::addNull($in), new ConstantStringType('')) : $in;
	}

Some existing tests will fail, but I think it's an improvement.
There will be only

		$return = filter_var($nullable_string, options: FILTER_FLAG_EMPTY_STRING_NULL);
		assertType('non-empty-string|null', $return);

to fix. This is something related to (I think)

if ($exactType === null || $hasOptions->maybe() || (!$inputType->equals($type) && $inputType->isSuperTypeOf($type)->yes())) {
			if (!$defaultType->isSuperTypeOf($type)->yes()) {
				$type = TypeCombinator::union($type, $defaultType);
			}
		}

VincentLanglet · 2026-01-19T22:16:21Z

I think you could try something like https://github.com/phpstan/phpstan-src/compare/2.1.x...VincentLanglet:phpstan-src:fix/filter_var?expand=1 with all your existing tests @niconoe-

niconoe- · 2026-01-20T08:44:46Z

Thank you very much for your feedback!

I'm giving it a try right now.

I'm just curious about this snippet:

$stringType = $in->isString()->no()
    ? $in->toString()
    : TypeCombinator::union(TypeCombinator::remove($in, new StringType()), new StringType());

In the else part of the ternary, I can't read something else than "Return a union of input type minus 'string', plus 'string'." which is basically the input type itself. Am I missing something?

EDIT: nvm, I got it. Aim is to remove any kind of StringType more specific than just a StringType, like an AccessoryNonEmptyStringType for instance, and replace it by a general StringType. This is due to possible sanitations when using flags that will remove unwanted chars from a non-empty-string that could lead to empty strings, and as we don't know, we infer the general StringType.
Got it ! 😄

niconoe- · 2026-01-20T09:08:49Z

I tried your proposal and I got 141 errors because of LegacyNodeScopeResolverTest, like for instance, filter_var($mixed, FILTER_DEFAULT) is expected to return string|false but know is infered as returning string.

I think this is due to

if ($canBeSanitized->no()) {
	$stringType = $in->toString();
}

Forcing the input type as a String only matters when having the flag FILTER_FLAG_EMPTY_STRING_NULL (at least, for the test cases I want to fix), IMO, so doing this change outside of the context of using such flag breaks the general usecases.
I'll try to get a fix about it.

VincentLanglet · 2026-01-20T09:18:41Z

I tried your proposal and I got 141 errors because of LegacyNodeScopeResolverTest, like for instance, filter_var($mixed, FILTER_DEFAULT) is expected to return string|false but know is infered as returning string.

It's certainly because of the if ($this->isValidationFilter($filterValue)) { I tried to add, but I was wrong since something like
filter_var(new \DateTime()) is false without a ValidationFilter

Maybe you'll have less failure to fix with https://github.com/phpstan/phpstan-src/compare/2.1.x...VincentLanglet:phpstan-src:fix/filter_var_2?expand=1

niconoe- · 2026-01-20T10:02:19Z

It's certainly because of the if ($this->isValidationFilter($filterValue)) { I tried to add, but I was wrong since something like filter_var(new \DateTime()) is false without a ValidationFilter

I figured this out too, but I kept the new method isValidationFilter as it is still used more than once, even when removing its call around

if ($exactType === null || $hasOptions->maybe() || (!$inputType->equals($type) && $inputType->isSuperTypeOf($type)->yes())) {
			if (!$defaultType->isSuperTypeOf($type)->yes()) {
				$type = TypeCombinator::union($type, $defaultType);
			}
		}

I had to fix the remaining failling unit test about filter_var($nullable_string, options: FILTER_FLAG_EMPTY_STRING_NULL); with a special case, as I don't want to add the default type when using FILTER_FLAG_EMPTY_STRING_NULL and input type is null|string.

niconoe- · 2026-01-20T10:30:14Z

I really don't understand why the unit tests are not passing on PHP 7.4 only and why the actual results differ from the expectations. The behavior of filter_var remained unchanged AFAIK.

For the other errors on the pipeline, there are issues about things I did not impact, and memory limit troubles. For both of them, I don't know if I have something to do.

src/Type/Php/FilterFunctionReturnTypeHelper.php

tests/PHPStan/Analyser/nsrt/filter-var-returns-non-empty-string.php

niconoe- · 2026-01-21T11:04:00Z

About Mutation testing, I don't get how could I kill both kind of mutations like this:

- if (!$ternaryLogic->yes()) {
+ if ($ternaryLogic->no()) {

and

- if ($ternaryLogic->no()) {
+ if (!$ternaryLogic->yes()) {

when $ternaryLogic cannot be "maybe".

Therefore, I don't think such mutation is relevant IMO.

And about other failing tests, this is about memory_limit so, I'm not sure I could do something to be fair.

VincentLanglet · 2026-01-21T11:30:06Z

For if ($scalarOrNull->isSuperTypeOf($in)->yes()) you can test the maybe case with something like
filter_var($stringOrObject) I think (or filter_var($mixed) ?)

Maybe some of the other condition can be checked the same way.
I feel like you're getting some mutation testing on some trinary you didn't originally write, so I would focus on the code you introduced first.

niconoe- · 2026-01-26T10:49:40Z

I added unit tests with all combinations of types I got on this unit test file and I added mixed too. I'm getting some unexpected results because as soon as the input might be an object, the analysis of the flag is not done and default return type string|false is given.

I'm gonna need some time to fix this.

niconoe- · 2026-01-27T13:05:53Z

@VincentLanglet I added unit tests with all types and anyOf all types + mixed, which helped into getting even more precise retun types, but some mutants are still alive. I'm not sure how I could kill them by tests.
Probably the code should be optimized in a way I'm not confindent enough to handle it, TBH.

Do you have any ideas about it?

Thanks a lot!

VincentLanglet · 2026-01-27T13:15:50Z

Do you have any ideas about it?

I'm not fully familiar with all those mutation testing errors, I think this is something introduced by @staabm.
I sometimes find the report are false-positive or unpossible case. But maybe he could help you on it

niconoe- · 2026-02-02T09:33:36Z

Is there a way to run the mutation testing locally so I could give it a try to find how to kill the mutants? I really wish this PR not to be ignored because of this 😅

Any advice you could give @staabm ?

Thanks a lot!

staabm · 2026-02-10T07:17:20Z

#4524 adds a way to run mutation tests locally.

sometimes its also useful to apply a mutant manual locally, run the test-suite and make sure it fails with the mutated code.
sometimes mutants are false positives and can/need to be ignored.

when mutations happen on TrinaryLogic object (most ->yes() or ->no() calls), this usually means you are missing a test-case for the "maybe"-case.

…_EMPTY_STRING_NULL.

niconoe- · 2026-02-10T10:15:44Z

I gave it a try by manually append your modifications from #4524 into my branch but since I rebased the 2.1.x branch this morning, I'm experiancing the following error on almost every unit test I could run:

Exception in third-party event subscriber: Missing parameter 'narrowMethodScopeFromConstructor'.

I can't even run the unit tests only for the class I changed to reduce the scope so I guess I'm blocked for now.
I'll give it a try later after another rebase.

ondrejmirtes · 2026-02-10T10:18:31Z

Make sure to run composer install after rebase/checkout.

niconoe- · 2026-02-10T10:44:03Z

Make sure to run composer install after rebase/checkout.

Thanks, I didn't do that and for sure it makes sense. I should have thought about it, my bad.

When running composer install, I'm experiancing this:

Just to give you some context: I'm not having PHP locally installed, I'm inside a docker container when I run PHP. This container is from an image build via this Dockerfile :

FROM ghcr.io/devgine/composer-php:v2-php8.2-alpine

RUN apk update && \
    apk add --no-cache make git && \
    echo 'memory_limit = 2048M' >> /usr/local/etc/php/conf.d/docker-php-memlimit.ini

VOLUME "/app"
WORKDIR "/app"
ENTRYPOINT ["/bin/sh"]

Then I can run a container via
docker container run -it --rm -v $(pwd):/app/ phpstan-src (I'm into my phpstan-src 's project root).

I don't think that's permission related as I tried to give 777 to the whole vendor folder but I still can't apply the patch.

This cause composer install to fail and if I just let it go as my dependencies are loaded anyway, I'm still experiancing the Missing parameter "narrowMethodScopeFromConstructor" error when running unit tests… 😢

ondrejmirtes · 2026-02-10T10:55:40Z

The patch utility is probably missing in the Docker image?

You might have more success with Ubuntu or Debian images. Alpine is very minimalX

niconoe- · 2026-02-10T19:00:35Z

src/Type/Php/FilterFunctionReturnTypeHelper.php


-			if ($in->isBoolean()->yes() || $in->isFloat()->yes() || $in->isInteger()->yes() || $in->isNull()->yes()) {
-				return $in->toString();
+			if ($this->canStringBeSanitized($filterValue, $flagsType)->no()) {


To me, this mutation is a false-positive: the method canStringBeSanitized, in this context, is the exact same value than the TernaryLogic about the FILTER_FLAG_STRIP_* checks. But the problem is that such ternary logic is maybe only when the $flagsType is a non-constant array, and such case is marked as ignored as "Too complicated" (line 150).

Therefore, in this particular context, the canStringBeSanitized returns TernaryLogic::yes or TernaryLogic::no, but can never return TernaryLogic.

If you're right here, the canStringBeSanitized return type could be changed to bool?

That was my feeling too, but I only found out how to make it this morning. Changes just have been pushed. Thanks 🙂

niconoe- · 2026-02-11T13:34:47Z

src/Type/Php/FilterFunctionReturnTypeHelper.php

+		if (
+			$exactType === null
+			|| $hasOptions->maybe()
+			|| ($this->isValidationFilter($filterValue) && (!$inputType->equals($type) && $inputType->isSuperTypeOf($type)->yes()))


For the 2 mutations here, I don't know how to kill them.

I want to find a data-set where only the $inputType->isSuperTypeOf($type)->yes() part toogles, which implies that such data-set must produce the following statements:

$exactType is not null

$hasOptions->maybe() is false (not a big deal)

$this->isValidationFilter($filterValue) is true`

$inputType->equals($type) is false.

Also, as the body of this if statement implies a possible change of the $type to return by adding it the $defaultType, it means that $exactType must not be $defaultType too (or the whole condition would have no effect, and mutants are still alive).

Now, regarding how $exactType is set to something different from defaultType and $this->isValidationFilter(…) must be true, the data-set MUST be via filter_var(???, FILTER_VALIDATE_(BOOLEAN|INT|FLOAT), ???)

(Please note that my PR was only about caring of the flag FILTER_FLAG_EMPTY_STRING_NULL which has no effect when used along one of the 3 validators identified, so I'm pretty sure the mutants to kill are not alive due to my changes, but anyway, let's say I'm a mutant killer 😆 )

About FILTER_VALIDATE_BOOLEAN, as we don't want $exactType to be $defaultType, the only data-set possible to search is when $in IS a boolean. Such situation produce that $exactType === $inputType === $type, therefore $inputType->equals($type) is always true. So this filterValue does not allow me to kill the mutant (no data-set possible).

About FILTER_VALIDATE_FLOAT, searched data-set would be $in is true or an integer (float would lead to the same outcome than any boolean with FILTER_VALIDATE_BOOLEAN and null would lead to $defaultType). In such data-set, $exactType (and therefore, $type) would be a Float or a ConstantFloat. But as $inputType as true is never a super-type of ConstantFloat, nor $inputType as int is never a super-type of Float, there is no way to toogle $inputType->isSuperTypeOf($type)->yes() from any possible data-set. This leads to a dead-end.

Finally, about FILTER_VALIDATE_INT, long story short, $exactType is a different type from $inputType or $defaultType only when:

$in === true => implies $type to be a ConstantInteger and true is never a super-type of a constant integer, therefore no data-set is possible

$in is a float which value matches its "integer" value => implies $type to be an integer and a float is never a super-type of an integer, therefore no sata-set is possible

$in is a numerical constant string => implies $type to be an integer and a string is never a super-type of an integer, therefore no data-set is possible.

I really do have the feeling that finding a data-set that could kill the mutants is not possible here 😢 .

niconoe- mentioned this pull request Jan 19, 2026

filter_var with option FILTER_FLAG_EMPTY_STRING_NULL is not returning the expected type phpstan/phpstan#13963

Open

VincentLanglet reviewed Jan 20, 2026

View reviewed changes

src/Type/Php/FilterFunctionReturnTypeHelper.php Outdated Show resolved Hide resolved

VincentLanglet reviewed Jan 20, 2026

View reviewed changes

src/Type/Php/FilterFunctionReturnTypeHelper.php Outdated Show resolved Hide resolved

VincentLanglet reviewed Jan 20, 2026

View reviewed changes

tests/PHPStan/Analyser/nsrt/filter-var-returns-non-empty-string.php Outdated Show resolved Hide resolved

VincentLanglet reviewed Jan 20, 2026

View reviewed changes

tests/PHPStan/Analyser/nsrt/filter-var-returns-non-empty-string.php Outdated Show resolved Hide resolved

niconoe- added 9 commits February 10, 2026 10:21

Fix #13963 with unit tests.

aaf71de

Improve handling of input type for filter_var with option FILTER_FLAG…

a4a2cb2

…_EMPTY_STRING_NULL.

Fix forgotten import of function.

187e52e

Better usage of PHPStan API.

3983d21

Code clean up.

3998efd

Refactor thanks to @VincentLanglet + add new test cases.

7e0aeb0

Make unit tests complient with PHP 7.4.

b348d10

Apply comments.

84367f3

Try to make unit test OK with PHP 7.4.

191f1a3

niconoe- added 3 commits February 10, 2026 10:21

Try to make unit test OK with PHP 7.4.

5b0796f

Add lots of unit tests and combinations to cover all kinds of types.

ff4f572

Remove forced type mixed on PHPUnit test to be compliant with PHP 7.4.

8567e3a

Simplify some type detection.

7c44caa

niconoe- force-pushed the fix-13963 branch from ecc34ab to 7c44caa Compare February 10, 2026 15:35

niconoe- added 2 commits February 10, 2026 17:06

Fix deprecated RequiresPhp attribute.

7c7379e

Simplify comparisions.

48b0b46

niconoe- commented Feb 10, 2026

View reviewed changes

Removing mutants.

f24e1e2

niconoe- commented Feb 11, 2026

View reviewed changes

Simplify method canStringBeSanitized.

aa4d70a

Fix #13963 with unit tests. #4786

Are you sure you want to change the base?

Fix #13963 with unit tests. #4786

Conversation

niconoe- commented Jan 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

VincentLanglet commented Jan 19, 2026

Uh oh!

niconoe- commented Jan 19, 2026

Uh oh!

VincentLanglet commented Jan 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

VincentLanglet commented Jan 19, 2026

Uh oh!

niconoe- commented Jan 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

niconoe- commented Jan 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

VincentLanglet commented Jan 20, 2026

Uh oh!

niconoe- commented Jan 20, 2026

Uh oh!

niconoe- commented Jan 20, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

niconoe- commented Jan 21, 2026

Uh oh!

VincentLanglet commented Jan 21, 2026

Uh oh!

niconoe- commented Jan 26, 2026

Uh oh!

niconoe- commented Jan 27, 2026

Uh oh!

VincentLanglet commented Jan 27, 2026

Uh oh!

niconoe- commented Feb 2, 2026

Uh oh!

staabm commented Feb 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

niconoe- commented Feb 10, 2026

Uh oh!

ondrejmirtes commented Feb 10, 2026

Uh oh!

niconoe- commented Feb 10, 2026

Uh oh!

ondrejmirtes commented Feb 10, 2026

Uh oh!

niconoe- Feb 10, 2026

Choose a reason for hiding this comment

Uh oh!

ondrejmirtes Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

niconoe- Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

niconoe- Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

niconoe- commented Jan 19, 2026 •

edited

Loading

VincentLanglet commented Jan 19, 2026 •

edited

Loading

niconoe- commented Jan 20, 2026 •

edited

Loading

niconoe- commented Jan 20, 2026 •

edited

Loading

staabm commented Feb 10, 2026 •

edited

Loading