Specify detailed return type of parse()

[phil-davis]

If I increase phpstan level to 9, then it detects that function parse(string $uri): array can return array<string, int|string|null> and that is correct.

Key "port" is an int value. Other keys have a string value. "Unused" keys have value null.

If I actually document:

 * @return array<string, int|string|null>

Then phpstan complains about:

------ ------------------------------------------------------------------------------------------------- 
  Line   lib/functions.php                                                                                
 ------ ------------------------------------------------------------------------------------------------- 
  60     Parameter #1 $string of function substr expects string, int|string|null given.                   
  68     Parameter #2 $str of function explode expects string, int<min, -1>|int<1, max>|string given.     
  117    Parameter #1 $str of function ltrim expects string, int<min, -1>|int<1, max>|string given.       
  138    Parameter #1 $str of function strtolower expects string, int<min, -1>|int<1, max>|string given.  
  161    Parameter #1 $str of function strtolower expects string, int<min, -1>|int<1, max>|string given.  
 ------ ------------------------------------------------------------------------------------------------- 

because it has not been told the detail that:
Key "port" can have value int|null
All other keys have value string|null

If there is some way to express this in PHPdoc, then that would be good - because the line numbers above are processing keys that do not have int value.

For now, I set the type of the keys to mixed - at least that recognizes that the values are definitely not all string.

[codecov]

Codecov Report

Merging #79 (602106d) into master (4df7398) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master      #79   +/-   ##
=======================================
  Coverage   98.54%   98.54%           
=======================================
  Files           1        1           
  Lines         137      137           
=======================================
  Hits          135      135           
  Misses          2        2           

Impacted Files	Coverage Δ
lib/functions.php	98.54% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[phil-davis]

Note: if I go up to phpstan level 9, then it does not like me just using mixed. but it also seems to not have a way for me to express the full detail of the values that can be int|string|null. So I get a bit stuck - phpstan insists on telling me the 5 things in the top post. To shut it up, I would have to add (string) casts in those places, even though I am confident that the values are already string.

[staabm]

Level9 is very brutal. It does not allow mixed.

Most of the time this means you need a lot of typing and/or ignores

[phil-davis]

with the detailed types, and level 9, phpstan found this.
To be really safe, we cast $path to a string straight away. Then its use at line 60 - passing to substr() is also safe.

[staabm]

I guess my recent fix regarding + will resolve this phpstan error on the next release

phpstan/phpstan-src#1579

[phil-davis]

If the type can be expressed in detail, then phpstan at a high level seems good at finding places where something is used in the wrong way. It takes effort to specify everything, but in the end it seems a good way to avoid dumb-type-errors and regressions.

[phil-davis]

I will make a patch release to get this published, so that other repos that pull in sabre/uri as a dependency will get the updated type information and code...

[staabm]

👍

thx for doing all this.. library users will benefit a lot from it