PHP was the one language with real code in two shapes here and no skill: a Composer library held at PHPStan level max, and a plain PHP-FPM application behind nginx with no framework. The rules come from reading both rather than from general PHP advice, so each one names the failure it prevents. The two halves differ enough to be separate sections. A library requires PSR interfaces and ships no implementation; an application's rules are about escaping, keeping the document root unwritable, atomic writes and sessions. Everything in §1-§8 applies to both. Three findings are worth the space they take because they were each paid for once already: `json_encode` with JSON_UNESCAPED_SLASHES into an inline <script> is an XSS hole, a web user that cannot write its own cache fails silently on every request, and inside the dev container the host's own `php` bridge shadows the real interpreter so every vendor/bin tool dies naming distrobox. docs(python): cover MCP servers, Red cogs and HA components, and correct three rules the owner's own code contradicts The skill described a toolchain and was silent on the four things most of this Python actually is. Three of its rules are also wrong in context, and the code is right: getLogger(__name__) puts a cog outside the red.* tree where Red's log controls apply; TaskGroup's cancel-on-first-error is the wrong default for fan-out over N providers, where the real pattern is a semaphore plus gather(return_exceptions=True); and the gate runs locally and through pre-commit, not through CI, which never runs under this account. Also: ruff ALL-minus-documented-ignores is the preferred shape rather than the explicit select list, lockfiles split by artifact type, and bare `python` and bare `pip` are different interpreters on this host. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
23 KiB
name, description
| name | description |
|---|---|
| php | Use this skill when writing, linting, analysing, testing, or releasing PHP — libraries, Composer packages, generated SDKs, and plain PHP-FPM web applications behind nginx. Covers strict_types, PHPStan at max, PER-CS 2.0 with risky rules, final readonly value objects, backed enums, PSR interfaces over implementations, output escaping, and PHPUnit strictness. |
PHP — Rules & Preferences
Extends coding-dev.md. Everything there applies; this adds PHP specifics.
PHP here means one of two things, and they differ: a library published through Composer, which depends on PSR interfaces and ships no implementation, and a plain PHP-FPM application behind nginx with no framework, where the filesystem is the router. Both carry §1–§8; §9 is the library half and §10 the application half.
1. The Gate
composer lint # php-cs-fixer fix --dry-run --diff
composer stan # phpstan analyse
composer test # phpunit
composer check # all three, in that order
Define those four as scripts in composer.json so the gate has one name. composer check
must be clean before anything is called done — zero fixer diffs, zero PHPStan errors, zero
test failures, and no warnings, notices or deprecations (see §7).
- Use
@php tools/x.phprather thanphp tools/x.phpin a composer script: it runs the interpreter Composer itself is running under, which matters on a machine with several. composer testmust never touch the network. PointdefaultTestSuiteat the unit suite so a bare run is safe to fire blind.- Name tool configs
.dist—phpstan.neon.dist,phpunit.xml.dist,.php-cs-fixer.dist.php— so a developer can drop a local override beside them without dirtying the repo. Add the caches (.php-cs-fixer.cache,.phpunit.cache) to.gitignore. - Lockfile by artifact type: an application commits
composer.lock, a library ignores it. Getting this backwards shows up in every diff.
2. There Is No PHP On The Host
On this workstation PHP is not installed on the host at all. ~/.local/bin/php is a three-line
bridge that execs into the development container, and composer has no bridge — see
distrobox-buildbox-dev.md for the container, and
coding-dev.md §13 for why toolchains never land on an immutable
host root.
# php works from the host. composer does not — go through the container, and chain the cd,
# because entering one resets the working directory.
distrobox enter <container> -- bash -c "cd /abs/path/to/project && composer check"
Caution
Inside the container, prepend
PATH=/usr/bin:/bin. The host~/.local/binis on PATH ahead of/usr/binthere, so the host's ownphpbridge shadows the container's real interpreter and dies with/usr/bin/distrobox: No such file or directory. Everyvendor/bin/*tool is a PHP script, sovendor/bin/phpstanfails this way and the error names distrobox rather than anything to do with PHP.php vendor/bin/phpstanwith the fixed PATH works.
Edit PHP sources with Read/Edit/Write, not with sed or a php -r patch script. The
nested quoting inside distrobox enter … bash -c "…" mangles $, backslashes and quotes in
exactly the places PHP uses them, and a half-applied patch script reports success.
3. Strictness — strict_types In Every File, PHPStan At Max
<?php
declare(strict_types=1);
Every file, with no exceptions, enforced mechanically by the fixer's declare_strict_types
rule rather than by review. Without it a "7" from an API quietly becomes int 7 and a
hydrator whose whole purpose is to fail loudly on drift stops failing at all.
# phpstan.neon.dist
parameters:
level: max
paths:
- src
- tests
- tools
- examples
treatPhpDocTypesAsCertain: false
- Analyse tests, tools and examples too, not just
src. Those are the three places authors assume are exempt, and they are where a convention actually breaks first. treatPhpDocTypesAsCertain: falsestops PHPStan trusting a PHPDoc that is narrower than the signature, so genuine runtimeis_string/is_arrayguards are not reported as always-true.- No baseline and no
ignoreErrors. Level max is reachable with standard tags and real guards; a baseline is a list of things nobody will ever go back to. - A suppression, when genuinely needed, is the narrowest
@phpstan-ignore-next-linewith the reason on it. Reaching for@phpstan-*or@psalm-*to describe a type is a smell — the standard tags in §5 carry everything level max needs. - Never relax a strictness flag without an adjacent comment saying why. This is the house habit across every config file in the repo, and it is what makes a relaxation reviewable instead of load-bearing folklore.
4. Formatting — PER-CS 2.0, Risky Rules On
// .php-cs-fixer.dist.php
->setRiskyAllowed(true)
->setRules([
'@PER-CS2.0' => true,
'@PHP82Migration' => true,
'declare_strict_types' => true,
'strict_param' => true,
'strict_comparison' => true,
'global_namespace_import' => ['import_classes' => false, 'import_functions' => false],
'no_unused_imports' => true,
'ordered_imports' => ['sort_algorithm' => 'alpha'],
'php_unit_test_case_static_method_calls' => ['call_type' => 'self'],
])
php-cs-fixer and PHPStan, not Pint and not Psalm. The fixer config does more normative work than the prose does: these rules are where five of the conventions below stop being review items.
strict_comparison+strict_parammean every comparison is===/!==and everyin_array()carries its third argument. Loosein_arraymatches numeric strings.global_namespace_import => falseis why every global function, constant and class is written with a leading backslash:\sprintf,\JSON_THROW_ON_ERROR,\DateTimeImmutable,\RuntimeException. Inside a namespace an unqualified call makes PHP look up the namespaced name first and fall back to global — a per-call cost, and a hazard the day something definesApp\json_decode.- Guard a fixer API that newer releases added, rather than calling it unconditionally and
breaking the lint step on the oldest version
composer.jsonstill allows:if (\method_exists($config, 'setUnsupportedPhpVersionAllowed')) { … }.
5. Banned Constructs
| Never | Use instead |
|---|---|
== / != |
=== / !== (the fixer enforces it) |
in_array($x, $a) |
\in_array($x, $a, true) |
stdClass from json_decode |
\json_decode($b, true, 512, \JSON_THROW_ON_ERROR) into array<string, mixed>, then a typed model |
json_last_error() checks |
\JSON_THROW_ON_ERROR on every encode and decode |
\DateTime |
\DateTimeImmutable — a mutable date in a readonly class is a hole in the guarantee |
setX() on a value object |
withX() returning a new instance |
http_build_query() |
a builder that drops nulls, spells booleans true/false, and repeats a key instead of emitting k[0]= |
extract(), eval(), $$var |
named variables |
@ error suppression |
check the return value |
array() long syntax |
[] |
A pure enum for a wire value |
a backed enum X: string (§6) |
echo $x in HTML |
\htmlspecialchars($x, \ENT_QUOTES, 'UTF-8') (§10) |
Unqualified strlen/count/sprintf inside a namespace |
\strlen, \count, \sprintf |
- PHPDoc only says what the signature cannot: array shapes, generics,
class-string,list<T>,@template,@implements,@return never. A@param string $xrestating a declared scalar is a second source of truth that will drift from the first. @return neverin a docblock over a native: mixedis the deliberate exception: it gives PHPStan the unreachability while still letting the helper sit in a ternary tail position (return \is_string($v) ? $v : $this->fail(…);), which a native: neverforbids.
6. Types — final readonly By Default
final readonly classis the default shape for anything carrying data. Plainfinal classonly for things that memoise, hold mutable state, or are pure behaviour.readonlyon the class is what makes "models are immutable, build a new one" enforced by the engine instead of promised in a docblock.- Constructor property promotion for every property, with
private readonlywritten out explicitly even inside afinalclass. The one property declared the long way should be the one the constructor has to run through a normaliser — promotion cannot transform a value, so that asymmetry is a readable signal rather than an inconsistency. newin a parameter default, not nullable-plus-??:private readonly LoggerInterface $logger = new NullLogger(). The signature then states the real type and PHPStan never sees a nullable to guard. Keep?X $x = nullonly where a static factory forwards the argument and "the caller said nothing" has to stay distinguishable.- Named arguments wherever a call has more than two or three arguments or skips one. A wide constructor built positionally silently mis-wires the day two same-typed fields swap order.
- First-class callable syntax (
Model::fromArray(...)) over a'Model::fromArray'string — it is checked at compile time. Mark a closure that needs no bindingstatic, so it cannot capture$thisinto a long-lived callable and keep the owner alive. - Model properties may keep the API's own
snake_casenames rather than being renamed to camelCase. A rename table has to be maintained in both directions, silently drops whatever the mapping forgot, and makes a devtools capture stop matching the code.
7. Enums & Wire Values
- Backed enums only (
enum Provider: string). A backed case is what lets one branch collapse any enum to its wire form and a generic reader be typedclass-string<T of \BackedEnum>. - An unrecognised wire value throws, naming the model, the field, the value, the enum class
and the remedy.
::from()raises a bare\ValueErrorwith no field name; anUnknownfallback case hands the caller a value the API's own type does not have and defers the failure to someone else'smatch. - Where a field genuinely carries any JSON value, type it
mixedand explain it in prose. A PHPDoc union nothing actually validates is a lie PHPStan will then trust.
8. Errors & Exceptions
- The root of the hierarchy is an interface,
interface AppException extends \Throwable {}, not a base class. Callers can thencatch (AppException $e)without catching unrelated\RuntimeExceptions, while each exception still extends the most apt SPL class — which an abstract base makes impossible the moment two of them want different parents. - One static factory mapping status to class (
match (true)over 400/401/403/404/409/422/ 429/>= 500). Declare the base's constructorfinal public function __constructsonew $class(…)in that factory stays safe. - Carry the parsed error envelope, the raw body, and the request method and URI on the exception, and truncate the raw body — a 2 MB proxy error page does not belong in a log line. Document that callers branch on an error code, never on the message, which is prose.
- A failure to parse the error body must never mask the real status. A 502 from a load balancer returns HTML; if the envelope parser throws, the caller sees a JSON parse error instead of "the service is down". Return null from the parser on every one of those paths.
- Retry only idempotent methods —
GET,HEAD,OPTIONS,PUT,DELETE. POST and PATCH need an explicit opt-in, because a timeout on a request that actually succeeded is how a giveaway gets drawn twice. HonourRetry-Afterover the computed backoff, and inject the sleeper so the retry test does not take 30 seconds of wall clock.
9. Libraries — PSR Interfaces, Not Implementations
Follow the global ordering (reuse → fork → write, coding-dev.md §3).
For a published package specifically:
- Require PSR interfaces and ship no implementation.
psr/http-client(18),psr/http-factory(17),psr/http-message(7),psr/log(3), plusphp-http/discovery. Guzzle and a PSR-7 implementation go inrequire-devwith asuggestentry saying any implementation works. Pinning a client forces a version on every consuming application. - Widen the constraint on an interface package that has had a major (
"psr/http-message": "^1.1 || ^2.0") so a consumer's choice does not conflict with yours. - Wrap the PSR client behind your own one-method transport interface. That is what lets the whole unit suite run against a fake and assert on exact bytes, instead of building PSR-7 mocks in every test.
- Convert a discovery miss into your own configuration exception whose message says what to
composer require. - Secrets enter through an interface, never a string parameter: a token provider whose
implementation reads the environment, with a null return treated as legitimate because some
endpoints are public. A token passed as a constructor literal lands in every stack trace and
var_dump. Seecoding-dev.md§10 for the general rule. - Partial updates: distinguish "field absent" from "field is null" per field. Serialise a
fetched model, PATCH it back, and a blanket
array_filter(…, fn($v) => $v !== null)wipes every server-side value the caller never mentioned — while the opposite default breaks the fields for which null is the value. - Accept an
intwhere the schema saysnumber. JSON has one number type, so a whole number arrives as an int and strictis_float()rejects every such value. This wants a named regression test.
Generated code lives in directories that are wiped and rewritten whole, never overwritten file by file — otherwise a deleted endpoint lingers as a class that still compiles and 404s. Every generated file opens with a banner carrying the exact regeneration command, generated and hand-written namespaces never share a directory, and nothing is added to a generated namespace — a method on a generated enum is deleted by the next run, so it belongs on a hand-written sibling instead.
10. Web Applications — Escaping, Writes, Sessions
A plain PHP-FPM app with no framework still gets one require __DIR__ . '/../vendor/autoload.php'
per entry point and PSR-4 for everything else. Hand-rolled require graphs between
application files are what produce double-inclusion and load-order bugs.
- Entry points are three lines over a class. Two copies of the same handler are how one of them loses its CSRF check.
- Compute first, echo second. With no template engine the discipline is the separation: all
logic above the
?>, then markup with<?= ?>echoes of values already shaped. Logic mixed into markup is where an unescaped echo hides. - Escape at every echo, with the explicit form
\htmlspecialchars($t, \ENT_QUOTES, 'UTF-8'). There are no trusted values.
Caution
json_encodeinto an inline<script>is an XSS hole, andJSON_UNESCAPED_SLASHESopens it by removing the<\/script>escaping that would otherwise have saved you. Any</script>or<!--inside the data closes the element and the rest is parsed as HTML. UseJSON_HEX_TAG | JSON_HEX_AMP | JSON_HEX_APOS | JSON_HEX_QUOT, or emit a<script type="application/json">block andJSON.parseit.
- The document root stays unwritable. A web server that can create files in its own
document root is one bug away from writing a
.phpthere. Anything PHP writes goes undersys_get_temp_dir(); a file it must update in place gets permission on the file, not on the directory. If an upload directory must live under the root anyway, sniff content (getimagesize()plus an image-type→extension allowlist, never the uploaded name) and deny.phpunder it in the server config. - Check the return value of every write. Both halves of the uid trap recorded in this
machine's memory were silent: a cache the web user could not create meant every page view
re-scraped three third-party APIs, and an
.envthe web user could not read looked exactly like "no token configured". - Atomic writes, two routes. Write a temp file beside the target and
rename()it, copying the victim's mode and group first — a rename-created file lands in PHP's group and locks everyone else out of editing it. Fall back tor+plusflock(LOCK_EX)plusftruncate, neverw, which truncates before the lock and happily creates a stray file at a typo'd path. Treat a short write as a failure. - Optimistic concurrency by content hash, not mtime — the mtime of a file on a network mount is not something to bet a save on. Validate the whole document before writing; a partial save is never attempted.
- Validate a filename against a listing, not by sanitising it:
in_array($name, array_column($this->backups(), 'name'), true). Cleaning the string is how../../.envgets through, and for a path under a base directory only comparingrealpath()results catches a symlink. - Sessions in one place. One guarded
session_start(), an explicitsave_pathyou know is writable (a session that silently fails to persist looks exactly like a login that silently fails),use_strict_mode=1,use_only_cookies=1, and anhttponly+samesite=Lax+securecookie. Decidesecureby defaulting to true and excepting loopback: behind a terminating proxy$_SERVER['HTTPS']is empty on a real HTTPS visit, and readingX-Forwarded-Protolets a forged header strip the flag. Expire by inactivity, which the client does not control, andsession_regenerate_id(true)the moment a visitor gains privilege. - Re-check authorisation per request, never from an
is_adminflag stamped into the session at login — otherwise revoking access means waiting for a session to expire. Request the minimum OAuth scope and discard the access token after use; storing it turns a session leak into an account compromise. - Discard unsafe input rather than cleaning it: a redirect target must start with
/, must not start with//, and must have no control characters or backslashes; strip CR/LF from any header value built from a query string. - An authenticated JSON API answers 401 with JSON, not a 302 to an HTML login page, which is
useless to
fetch()and surfaces as a parse error. Every admin call is a POST carrying a CSRF token compared withhash_equals— including the reads, since a GET is reachable from another site's page — dispatched throughmatchwith adefaultthat 400s. - One response shape,
{ok: bool, …}with a snake_casereasoncode, and set the content type,X-Content-Type-Options: nosniffand the cache policy before anything can fail so the error path carries them too. Choose per endpoint whether upstream trouble is a 200 withok: false(a public endpoint the page polls) or a real status code (a command channel). - A multipart request has already been consumed into
$_POST/$_FILES, so check for the upload action before readingphp://input— otherwise the generic JSON path reports "not valid JSON" for a body PHP ate. - Every outbound call gets a timeout and a connect timeout,
http_errors => falseso a 4xx body can be read for its reason, and redirects pinned tohttps. No timeout means a hung third party holding an FPM worker. - A missing credential is a supported state: separate "configured" from "not configured", answer with a machine-readable reason, and render a page saying it is not set up rather than a 500 nobody can diagnose.
11. Testing
- PHPUnit with attributes, never docblock annotations:
#[CoversClass],#[DataProvider],#[Group]. Annotations are deprecated in PHPUnit 11 and the config below turns that into a failure rather than a warning. - Turn on every escalation in
phpunit.xml.dist:failOnWarning,failOnNotice,failOnDeprecation,failOnRisky,failOnEmptyTestSuite,beStrictAboutOutputDuringTests.failOnSkipped="false"is the one justified exception, because a suite that skips itself without credentials is the intended outcome on a machine without them — say so in an XML comment next to it. - Test classes are
final; assertions areself::assertSame(…)(the fixer enforces it), while expectation setup, which is genuinely instance state, stays on$this. - Test method names are full sentences:
testAWrongTypeIsReportedRatherThanCoerced(). - Mock at the transport boundary with a fake that fails loudly. Running out of queued responses throws, naming the method, the URI and how many were already served — a fake returning null turns "one more request than expected" into a type error three frames away. Record every send so a test can assert on exact bytes.
- Fixtures are sanitised real captures, not hand-written JSON. A hand-written fixture encodes what the author thinks the API returns, so a drift test passes against an imagined payload. Keep shape, keys and enum values exactly; replace every identifier, handle, display name, avatar URL and message body with a deterministic placeholder so the diff stays stable. The raw capture stays out of the repository.
- A suite needing credentials is a separate testsuite, tagged, not the default, and skips
per test by re-reading the environment in a helper — so the skip message is attached to the
test that needed it.
\getenv()returnsfalsewhen unset, so guard withis_stringbeforetrim. Everything in it is a read, because the only credentials anyone will run it with belong to something live. Regression test every fixed bug. - Put the file and function size limits in a test, since neither PHPStan nor the fixer has a
length rule — see
coding-dev.md§4. Measure function length by walking\PhpToken::tokenize(), not by counting braces in text: a{inside a string or a comment looks exactly like a block opener. Skip generated files, whose length is a property of the API surface. Carry over-limit files in anACCEPTEDmap whose value is the written reason, and fail when a listed file drops back under so the entry gets removed.
12. CI
A PHP workflow runs shivammathur/setup-php@v2 with tools: composer:v2, then
composer install --prefer-dist --no-interaction --no-progress, then the gate in order: fixer
dry-run, PHPStan, unit suite. Set permissions: contents: read explicitly. Put a
credential-needing suite behind a workflow_dispatch input and pass the secret as env:,
never as a command-line argument (cli-tooling.md §7).
Important
Under this account, only
workflow_dispatchmay be live. Every automatic trigger —push,pull_request,schedule— stays in the file but commented out, under a comment saying why, so a fork restores CI by deleting#characters. See the GitHub Actions section of the agent profile; a workflow written withon: pushlooks correct and never runs.
A job that reports upstream drift should ::warning:: rather than fail: the API moving is
information, not a defect in this repository.