Skip to content

3.x: Fix phpstan up to level 9 - #81

Merged
acoulton merged 16 commits into
3.x-phpstan-l8from
3.x-phpstan-l9
Oct 27, 2026
Merged

acoulton merged 16 commits into
3.x-phpstan-l8from
3.x-phpstan-l9

Conversation

@acoulton

Copy link
Copy Markdown
Member

No description provided.

@acoulton
acoulton added this pull request to stack #71 October 27, 2026 11:24
@acoulton
acoulton force-pushed the 3.x-phpstan-l9 branch 3 times, most recently from d649ca7 to 2de1ba1 Compare October 27, 2026 12:33
@acoulton
acoulton requested a review from craig410 October 27, 2026 12:41

@craig410 craig410 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

That can't be a valid value, so treat it as not being
present at all and use our safe "production" default.
Note that $_SERVER is `array<string,mixed>` because it's possible
to have vars in there that are not strings e.g. the argv array.
It's not valid to write non-scalar values to fputcsv, we don't need to
validate that internally (fputcsv will generate a warning if so) but the
 method typehint should not claim it's supported to allow static
 analysis to find any invalid/unsafe values passed by the caller.
It's possible in PHP to parse a cookie as an arbitrarily nested
array by using a name with `foo[bar]` array syntax.
The type of a given $_SERVER value is consistent over time
(e.g. an HTTP_XXX header key will always be a string) but there
are a mix of value types with different keys.

Make the intent of the code clear when reading string server
values, and add development time assertions to prove these.

This also introduces a new thin `ServerGlobal` wrapper class,
to make dependency injection clearer / simpler when writing
other classes that depend on the $_SERVER global state by
default.
They should take any type of `mixed`. This looks like it might actually
be a phpstan bug - the examples they give for the covariant warning make
sense when the type is an actual type / interface, but `mixed` can't
ever *actually* be added to a collection...
It had the right type for the closure, but didn't carry the closure's
return type back to the function.
It's fine if this just TypeErrors or fails if we got the type
wrong (which we didn't).
Which also allows us to eliminate the old condition that the ::with
had to take at least one item.
It should only be called with a valid trace.
So that callers can enforce they are only putting in / out the expected
types of data.
Avoid runtime failures by checking the types in the test stub.
If we fail to read the file, or if we read it but the content after
decryption is not a string, then throw an exception.
Previously both the code and the tests made some assumptions about
nested array structure within log entries and context providers. In some
cases these wouldn't always be guaranteed to hold - tighten them.
@acoulton
acoulton merged commit 5bdbd1f into 3.x Oct 27, 2026
20 checks passed
@acoulton
acoulton deleted the 3.x-phpstan-l9 branch October 27, 2026 15:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants