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

Match expression does not handle remaining value with an array value #4451

Closed
jkuchar opened this issue Jan 28, 2021 · 24 comments
Closed

Match expression does not handle remaining value with an array value #4451

jkuchar opened this issue Jan 28, 2021 · 24 comments
Labels

Comments

@jkuchar
Copy link

jkuchar commented Jan 28, 2021

Bug report

match() definitely handles all values, but phpstan do not think so.

Code snippet that reproduces the problem

https://phpstan.org/r/b07c57ab-5084-4149-9c8b-21c1e14d2377

Expected output

No error.

@phpstan-bot
Copy link
Contributor

@jkuchar After the latest commit in dev-master, PHPStan now reports different result with your code snippet:

@@ @@
 PHP 8.0 (1 error)
 ==========
 
- 9: Match expression does not handle remaining value: array(bool, bool)
+-1: Internal error: PHPStan\Rules\Methods\WrongCaseOfInheritedMethodRule::findMethod(): Argument #2 ($classReflection) must be of type PHPStan\Reflection\ClassReflection, null given, called in /var/task/vendor/phpstan/phpstan-strict-rules/src/Rules/Methods/WrongCaseOfInheritedMethodRule.php on line 40
+Run PHPStan with --debug option and post the stack trace to:
+https://github.com/phpstan/phpstan/issues/new?template=Bug_report.md
 
 PHP 7.4 (4 errors)
 ==========
Full report

PHP 8.0 (1 error)

Line Error
-1 Internal error: PHPStan\Rules\Methods\WrongCaseOfInheritedMethodRule::findMethod(): Argument #2 ($classReflection) must be of type PHPStan\Reflection\ClassReflection, null given, called in /var/task/vendor/phpstan/phpstan-strict-rules/src/Rules/Methods/WrongCaseOfInheritedMethodRule.php on line 40Run PHPStan with --debug option and post the stack trace to:https://github.com/phpstan/phpstan/issues/new?template=Bug_report.md

PHP 7.4 (4 errors)

Line Error
10 Syntax error, unexpected T_DOUBLE_ARROW on line 10
11 Syntax error, unexpected T_DOUBLE_ARROW on line 11
12 Syntax error, unexpected T_DOUBLE_ARROW on line 12
13 Syntax error, unexpected T_DOUBLE_ARROW on line 13

PHP 7.1 – 7.3 (5 errors)

Line Error
7 Syntax error, unexpected ':' on line 7
10 Syntax error, unexpected T_DOUBLE_ARROW on line 10
11 Syntax error, unexpected T_DOUBLE_ARROW on line 11
12 Syntax error, unexpected T_DOUBLE_ARROW on line 12
13 Syntax error, unexpected T_DOUBLE_ARROW on line 13

@phpstan-bot
Copy link
Contributor

@jkuchar After the latest commit in dev-master, PHPStan now reports different result with your code snippet:

@@ @@
 PHP 8.0 (1 error)
 ==========
 
- 9: Match expression does not handle remaining value: array(bool, bool)
+ 5: Method HelloWorld::sayHello() should return int but return statement is missing.
 
 PHP 7.4 (4 errors)
 ==========
Full report

PHP 8.0 (1 error)

Line Error
5 Method HelloWorld::sayHello() should return int but return statement is missing.

PHP 7.4 (4 errors)

Line Error
10 Syntax error, unexpected T_DOUBLE_ARROW on line 10
11 Syntax error, unexpected T_DOUBLE_ARROW on line 11
12 Syntax error, unexpected T_DOUBLE_ARROW on line 12
13 Syntax error, unexpected T_DOUBLE_ARROW on line 13

PHP 7.1 – 7.3 (5 errors)

Line Error
7 Syntax error, unexpected ':' on line 7
10 Syntax error, unexpected T_DOUBLE_ARROW on line 10
11 Syntax error, unexpected T_DOUBLE_ARROW on line 11
12 Syntax error, unexpected T_DOUBLE_ARROW on line 12
13 Syntax error, unexpected T_DOUBLE_ARROW on line 13

@solverat
Copy link

same goes for string:

$value *= match ($identifier) {
  'g', 'm', 'k' => 1024,
};

https://phpstan.org/r/37736108-b211-4406-a94e-cec1e5b14c18

@ondrejmirtes
Copy link
Member

@solverat Your issue is unrelated, and the reported error is legit. If someone passes 'a' into your method, the match expression will throw an exception. You can either handle all possible strings, or use advanced types in PHPDocs: https://phpstan.org/writing-php-code/phpdoc-types#literals-and-constants

@solverat
Copy link

solverat commented Oct 12, 2021

@ondrejmirtes damn you're right - I guess I misinterpreted the message then. Thanks for clarifying!

@phpstan-bot
Copy link
Contributor

@jkuchar After the latest commit in dev-master, PHPStan now reports different result with your code snippet:

@@ @@
 PHP 8.0 (1 error)
 ==========
 
- 9: Match expression does not handle remaining value: array(bool, bool)
+ 9: Match expression does not handle remaining value: array{bool, bool}
 
 PHP 7.4 (4 errors)
 ==========
Full report

PHP 8.0 (1 error)

Line Error
9 Match expression does not handle remaining value: array{bool, bool}

PHP 7.4 (4 errors)

Line Error
10 Syntax error, unexpected T_DOUBLE_ARROW on line 10
11 Syntax error, unexpected T_DOUBLE_ARROW on line 11
12 Syntax error, unexpected T_DOUBLE_ARROW on line 12
13 Syntax error, unexpected T_DOUBLE_ARROW on line 13

PHP 7.1 – 7.3 (5 errors)

Line Error
7 Syntax error, unexpected ':' on line 7
10 Syntax error, unexpected T_DOUBLE_ARROW on line 10
11 Syntax error, unexpected T_DOUBLE_ARROW on line 11
12 Syntax error, unexpected T_DOUBLE_ARROW on line 12
13 Syntax error, unexpected T_DOUBLE_ARROW on line 13

@da-mask
Copy link

da-mask commented May 11, 2022

For those who come from google, you probably just want to add a default arm to your match expression.

@eexit
Copy link

eexit commented Jun 7, 2022

AFAIK, the default pattern is not mandatory so PHPStan should allow expressions when it's missing.

@allan-simon
Copy link

AFAIK, the default pattern is not mandatory so PHPStan should allow expressions when it's missing.

exactly currently we have to do this

        default => throw new UnhandledMatchError($type),

to please phpstan while keeping the default (no pun intended) behaviour

@mostafa-rz
Copy link

same for spaceship operator

return match ($foo <=> $bar) {
            1 => TrendEnum::UP,
            0 => TrendEnum::NEUTRAL,
            -1 => TrendEnum::DOWN,
        };

https://phpstan.org/r/71c1b6b0-cab7-49e4-8f3a-813900ba68e4

@phpstan-bot
Copy link
Contributor

@mostafa-rz After the latest commit in 1.7.x, PHPStan now reports different result with your code snippet:

@@ @@
-PHP 8.1 (1 error)
+PHP 8.1
 ==========
 
-16: Match expression does not handle remaining value: int<-1, 1>
+No errors
 
 PHP 7.1 – 8.0 (2 errors)
 ==========
Full report

PHP 8.1

No errors

PHP 7.1 – 8.0 (2 errors)

Line Error
2 Syntax error, unexpected T_STRING on line 2
4 Syntax error, unexpected T_CASE on line 4

@SomeBdyElse
Copy link

I agree with #4451 (comment) that it should be possible to use the default "\UnhandeledMatchError" without explicitly adding a default case.

@ondrejmirtes
Copy link
Member

@SomeBdyElse There's a feature request about this: #8042 (it has a hint on how to implement it)

@jaroslavtyc
Copy link

jaroslavtyc commented Feb 27, 2023

What about this

            $propertyValue = match ($property) {
                ReceiveOperation::OPERATION_ID => $value->getOperationId(),
                ReceiveOperation::OPERATION_CODE => $value->getOperationCode(),
                ReceiveOperation::DATA => $value->getData(),
                'default' => throw new \LogicException('Unknown property ' . var_export($property, true)),
            };
  Line   Api/Validation/ConstraintReceiveOperationValidator.php   
 ------ --------------------------------------------------------- 
  28     Match expression does not handle remaining value: mixed  

Is it intended?

PHPStan - PHP Static Analysis Tool 1.10.3

@ondrejmirtes
Copy link
Member

@jaroslavtyc Please reproduce your problem on phpstan.org/try and open a separate issue.

@jaroslavtyc
Copy link

@jaroslavtyc Please reproduce your problem on phpstan.org/try and open a separate issue.

#8969

@phpstan-bot
Copy link
Contributor

@mostafa-rz After the latest push in 1.11.x, PHPStan now reports different result with your code snippet:

@@ @@
-PHP 8.1 (1 error)
+PHP 8.1 (3 errors)
 ==========
 
-16: Match expression does not handle remaining value: int<-1, 1>
+ 4: Enum case TrendEnum::UP without type doesn't match the "string" type.
+ 5: Enum case TrendEnum::NEUTRAL without type doesn't match the "string" type.
+ 6: Enum case TrendEnum::DOWN without type doesn't match the "string" type.
 
 PHP 7.1 – 8.0 (2 errors)
 ==========
Full report

PHP 8.1 (3 errors)

Line Error
4 Enum case TrendEnum::UP without type doesn't match the "string" type.
5 Enum case TrendEnum::NEUTRAL without type doesn't match the "string" type.
6 Enum case TrendEnum::DOWN without type doesn't match the "string" type.

PHP 7.1 – 8.0 (2 errors)

Line Error
2 Syntax error, unexpected T_STRING on line 2
4 Syntax error, unexpected T_CASE on line 4

@phpstan-bot
Copy link
Contributor

@mostafa-rz After the latest push in 1.11.x, PHPStan now reports different result with your code snippet:

@@ @@
-PHP 8.1 (1 error)
+PHP 8.1 (3 errors)
 ==========
 
-16: Match expression does not handle remaining value: int<-1, 1>
+ 4: Enum case TrendEnum::UP does not have a value but the enum is backed with the "string" type.
+ 5: Enum case TrendEnum::NEUTRAL does not have a value but the enum is backed with the "string" type.
+ 6: Enum case TrendEnum::DOWN does not have a value but the enum is backed with the "string" type.
 
 PHP 7.1 – 8.0 (2 errors)
 ==========
Full report

PHP 8.1 (3 errors)

Line Error
4 Enum case TrendEnum::UP does not have a value but the enum is backed with the "string" type.
5 Enum case TrendEnum::NEUTRAL does not have a value but the enum is backed with the "string" type.
6 Enum case TrendEnum::DOWN does not have a value but the enum is backed with the "string" type.

PHP 7.1 – 8.0 (2 errors)

Line Error
2 Syntax error, unexpected T_STRING on line 2
4 Syntax error, unexpected T_CASE on line 4

@homersimpsons
Copy link

homersimpsons commented May 29, 2023

Note that this is now fixed for the spaceship operator with a correct syntax: https://phpstan.org/r/200e398c-d2a5-4370-aaf9-2cfa6864a409

<?php declare(strict_types = 1);
enum TrendEnum
{
    case UP;
    case NEUTRAL;
    case DOWN;
}

class HelloWorld
{
	public function sayHello(): mixed
	{
		$foo = fn(): int => rand();
		$bar = fn(): int => rand();

		return match ($foo <=> $bar) {
                    1 => TrendEnum::UP,
                    0 => TrendEnum::NEUTRAL,
                   -1 => TrendEnum::DOWN,
              };
	}
}

Commenting one match-arm also lead to the expected result. I think this can be closed.

But the array of bool still is not handled.

<?php declare(strict_types = 1);

class HelloWorld
{
	public function sayHello(): int
	{
		$verified = fn(): bool => rand() === 1;
		
		return match([$verified(), $verified()]) {
			[true, true] => 1,
			[true, false] => 2,
			[false, true] => 3,
			[false, false] => 4,
		};
		
	}
}

@bwaidelich
Copy link

@ondrejmirtes
Copy link
Member

@bwaidelich No, your code would be correct only if those classes were final.

@bwaidelich
Copy link

your code would be correct only if those classes were final.

Oh, of course.. https://phpstan.org/r/10db3101-89fa-44dd-92c1-682fe650f865
You're the man!

@ondrejmirtes
Copy link
Member

This was possible to solve thanks to: phpstan/phpstan-src@7912caf

I realized that [bool, bool] is in fact [true, true]|[true, false]|[false, true]|[false, false]. So I wrote Type::getFiniteTypes() to generate these combinations.

After that I took advantage of the new method when removing types: phpstan/phpstan-src@b5cf52b

And finally I made it work inside match condition: phpstan/phpstan-src@0cdda0b

@github-actions
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests