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

Ignore second value of ternary operator #910

Closed
mitsuru793 opened this issue Apr 10, 2018 · 6 comments
Closed

Ignore second value of ternary operator #910

mitsuru793 opened this issue Apr 10, 2018 · 6 comments

Comments

@mitsuru793
Copy link

mitsuru793 commented Apr 10, 2018

Hi, first, thank you very much for this excellent project!

related issues:

Summary of a problem or a feature request

return json_encode($this->data) ?: '';

This code ensures that returns alyways string.
Howerver, phpstan(master) considered that it returns string|false, so phpstan ignores ?: ''.

This problem occur only on master branch, but not on 0.9.2 tag.

Code snippet that reproduces the problem

json_encode returns string or false, so I ensured that it returns always string or null.
http://php.net/manual/en/function.json-encode.php

<?php declare(strict_types = 1);

class Data
{
    private $data;

    public function toJson(): ?string
    {
        return json_encode($this->data) ?: null;
    }
}

master branch
https://phpstan.org/r/8f97c7e5ac57cb845f37071cc7885b0f

  Line   analyzed.php
 ------ ---------------------------------------------------------------------------
  9      Method Data::toJson() should return string|null but returns false|string|null. 
 ------ ---------------------------------------------------------------------------


 [ERROR] Found 1 error

0.9.2 tags
https://phpstan.org/r/12c1a9eda8e04bbd0c4c20d21136e4d0

 [OK] No errors

Expected output

This bug occurs again.
I want phpstan to consider ?: <second value>.

@muglug
Copy link
Contributor

muglug commented Apr 12, 2018

Simpler testcase that fails in all versions of PHPStan:

function weirdEmpty(string $s): ?string {
  return (rand(0, 1) ? $s : false) ?: null;
}

Problem goes away if you put the string|false var into separate assignment:

function weirdEmpty(string $s): ?string {
  $s2 = rand(0, 1) ? $s : false;
  return $s2 ?: null;
}

@mitsuru793 you can use the same workaround

@ondrejmirtes
Copy link
Member

ondrejmirtes commented Apr 12, 2018 via email

@mitsuru793
Copy link
Author

Sorry, I replied as wrong another account.
I post it again.


Hi, @muglug. Thank you very much for replaying.

I confirmed your code and it worked. A left of ?: must be variable.
I should write code along the following.

https://phpstan.org/r/008068a6be84e300959ddb89ec983478

<?php declare(strict_types = 1);

class Data
{
	private $data;
	
	public function toJson(): ?string
	{
		$json = json_encode($this->data);
		return $json ?: null;
	}
}

return json_encode($this->data) ?: null;
This doesn't work. Is this a bug?
Is this a bad coding?

@mitsuru793
Copy link
Author

Hi, @ondrejmirtes.
Thank you very much for to reply.

I see that it's a bug. I'm looking forward to fix it.

@ondrejmirtes
Copy link
Member

Fixed: ea47684

It wasn't about null vs. false, it was about function type-specifying - normally, you can't rely on the fact that the function will return always the same value when called again in a truthy branch of a condition, except for the short ternary operator, where the value itself is used in the if branch.

@mitsuru793
Copy link
Author

mitsuru793 commented Apr 16, 2018

where the value itself is used in the if branch

Sorry, I couldn't understood it correctly, but I could do loosely.
(I'm not good at English)

I have confirmed your fix.
Thank you very much, @ondrejmirtes and @muglug !
I was helped with you.

@lock lock bot locked as resolved and limited conversation to collaborators Dec 20, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants