Skip to content

Report by-reference parameters given the left-hand side of a pipe - #12971

Draft
alies-dev wants to merge 7 commits into
vimeo:masterfrom
alies-dev:alies-dev/pipe-byref
Draft

alies-dev wants to merge 7 commits into
vimeo:masterfrom
alies-dev:alies-dev/pipe-byref

Conversation

@alies-dev

Copy link
Copy Markdown
Contributor

PHP passes the left-hand side of a pipe by value and throws when the callee takes that parameter by reference, but Psalm accepted it.

Before

<?php
$a = [3, 1];
$a |> sort(...); // no issue (PHP: Error: sort(): Argument #1 ($array) could not be passed by reference)

After

$a |> sort(...); // InvalidPassByReference: Parameter 1 of sort is passed by reference, but the pipe operator passes its left-hand side by value

The piped variable keeps its type, also for array_push, array_unshift and array_splice, and for callees whose parameters are unknown.

Part of #12966

- The pipe is lowered to one virtual call analysed by the existing call
  analysers, so its type, argument checks, purity, unused-variable
  tracking and left-hand taint match the hand-desugared call
- A partial application PHP compiles to a direct call gets the piped
  value in its placeholder; other partial applications stay mixed until
  closures for them are supported
- The left-hand side is analysed once, before the right-hand side
  whenever the order can matter
- Each call of a chain starts at its own callee, as calls are told apart
  by their start (issue dedupe, taint specialization)
- `??` and psalter's unused-assignment removal treat a pipe as a call
- The scanner and visitor guards against partial applications are the
  ones of vimeo#12915, so neither PR conflicts with the other
PHP passes the left-hand side of a pipe by value, so a by-reference
parameter can't take it: `$a |> sort(...)` throws. Report
InvalidPassByReference for it and leave the piped variable (type and
taint) untouched. array_multisort() prefers a reference, so it sorts a
copy, as for `array_multisort(f())`.


A pipe is a call: like `increment(counter(1))`, `increment(1 |> counter(...))`
passes the reference the callable returns, so it is not an
InvalidPassByReference.
…array_splice vimeo#12966

Their dedicated handlers wrote the new elements into the first argument
before the call was rejected, so `$a |> array_push(?, 4)` still changed
`$a` and carried the taint of the pushed values into it. The piped value
now takes the generic path, which rejects it.
A callee whose parameters aren't known was assumed to take the piped
variable by reference: `$e = []; $e |> $closure` widened `$e` to
`array<array-key, mixed>`, and a callable array of a contracted class
was charged write-refs for it. PHP passes the piped value by value,
whatever the callee.
…-pipe-operator

# Conflicts:
#	src/Psalm/Internal/Analyzer/Statements/Expression/BinaryOp/CoalesceAnalyzer.php
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

1 participant