Psalm — working conventions

master is the 7.x development branch and the base of feature PRs; 6.x is maintained separately and periodically merged into master (skill merge-6x-into-master). Remotes: origin = vimeo/psalm. PR branches go to your own fork's remote (find it with git remote -v and gh api user --jq .login; add it if missing). Never push PR branches to origin.

Domain skills (load them before touching the area):

• taint-analysis: taint graph and resolver, sinks, taint repros and TaintTest pitfalls
• purity-capabilities: capability bitmask, purity templates, builtin/SPL purity stubs, memoization
• hack-conformance: HHVM oracle harness, Hack coeffect facts, type-variable rules
• psalm-repro: scratch-project repros, comparing against master/old releases, timing and memory
• merge-6x-into-master: the 6.x → master merge procedure
• pr-description: the PR description format, and reformatting existing descriptions
• plugin-api-bc: a failing BC check, the roave XML, and the psalm-plugin-api PR

Fixing bugs and writing features

These come first, before every other rule here:

1. Fix the root cause. Find why the bug happens and fix that, not the symptom or the one input that showed it. If the same mistake exists elsewhere (sibling classes, other call sites), fix it there too.
2. Think through every way the change could cause bugs, in edge cases and ordinary ones: other callers of the changed code, subclasses and overrides, both analysis modes, other PHP versions, caching, ordering. Check each one by adding a test.
3. Add an explicit test case for each of those cases, not only for the reported bug.

Validating changes: CI, not local test runs

• Never run the PHPUnit suite locally to validate a branch. That includes whole test files, directories or broad --filter patterns, re-running batches of existing provider cases through the CLI, and sweeps over provider snippets. Commit, push, and let the PR's CI run it. Local runs are slow (~7 min) and noisy: ~73 InternalCallMapHandlerTest cases and ReportOutputTest::testSarifReport fail locally for environment reasons.

• Running a single test case is fine, and recommended for reproducing an issue or showing that a new test fails on the base and passes on the branch. Run just that one case, never the whole file:

  • provider case: vendor/bin/phpunit tests/CapabilitiesTest.php --filter 'testValidCode@caseName';
  • TaintTest provider case (sharded over tests/Taint/TaintCases{1..8}Test.php): vendor/bin/phpunit tests/Taint --filter '@caseName'. This runs the case in both modes (plain and TrackingUnusedVariables);
  • test method: vendor/bin/phpunit tests/TaintTest.php --filter 'testMethodName$'.

  @caseName matches the data-set name exactly; don't append $ (it then matches nothing). "No tests executed!" means a wrong filter, or a stale vendor/ (PHPUnit must be ≥ 10.5: check vendor/bin/phpunit --version, and run composer install if it's older).

• Also fine locally: targeted ./psalm --no-cache repros on a scratch project (skill psalm-repro), single-file self-analysis, and one full self-analysis.
• When briefing a subagent, spell all of this out ("single test cases only, no test files or suites, no sweeps, timeout 300 on self-analysis"); a vague "don't run phpunit" was not enough.

Pull requests

• Push the branch to your fork, then gh pr create --repo vimeo/psalm --base master.
• Add a release:* label (release:fix, release:feature, release:internal, release:docs, …) or the label check fails.
• Then poll gh pr checks <n>. For gh run list, --commit <sha> is reliable; --branch sometimes returns stale runs.
• Prefer a PR even when asked to fix something on master. A direct push to master was once reverted and redone as a PR.
• Description: follow skill pr-description (one-line bug + Before/After code blocks from a test case of the PR that asserts the fix, nothing else). Use it to write new descriptions and to reformat existing ones when asked.
• "Check backward compatibility" fails: follow skill plugin-api-bc. Real BC breaks get a PR to psalm/psalm-plugin-api that bumps its version and adds the changelog entry. False positives (e.g. bit-constant values and aggregates that change because a member was added, which is not a BC issue) are check bugs: fix them in .roave-backward-compatibility-check.xml in the same PR. Never leave the check red as "expected".

Merging and releasing (maintainers with merge rights)

• In any repo of github.com/psalm (plugins, codeception-psalm-module, psalm-plugin-api, …), review the diff yourself, wait for green CI, merge, and tag a release of every plugin you changed (fixture/CI/dependency-only changes too), following each repo's versioning. This includes PRs whose maintainer of record is someone else. It does not extend to third-party orgs (php-stubs, …).
• Never use --admin to bypass red required checks unless asked for that specific PR.
• Shipping a plugin whose users see InvalidDocblock errors from broken upstream stubs is fine: users report them upstream. Don't hold releases for upstream stub fixes.

Task lists (actions.md, actions_2.md: untracked, in the repo root)

• PRs in the Merge section have already been reviewed by whoever wrote the list. Follow the file's bugbot gating, then merge them all into one temporary branch, check it contains each PR's current head, and fast-forward push it to origin/master yourself in the main session. Don't relay the push authorization to a subagent.
• Modify section PRs are not authorized for merge: make the changes in the current chat and open the PR in Brave (see the platform table).
• If the auto-mode classifier denies the master push ("Merge Without Review"), stop and ask for approval rather than retrying.

Agents and subagents

• Do fixes, reviews and PR work in the main session. Spawn a subagent only when asked to or for a genuinely independent, bounded task, and say so first.
• To review several PRs, use one subagent with an ordered target list (most urgent first, saying which to drop if time runs short), not one subagent per PR.
• Never use the Workflow tool unless explicitly asked for a workflow.
• Parallel agents running Psalm contend for RAM (one self-analysis can take most of an 18 GB machine): follow the self-analysis lock rule below, and tell subagents the testing rules above explicitly.

No workarounds

Never hide an issue instead of fixing it:

• no @psalm-suppress, issueHandlers or baseline entries added to dodge a problem;
• no ignoreFiles to dodge DuplicateClass, and no BeforeAddIssue filtering plugins;
• no skipping stub docblock issues, no COMPOSER_NO_SECURITY_BLOCKING-style env bypasses;
• no regenerating a baseline to paper over a Psalm bug;
• no changing fixtures to dodge behaviour, and no reading Psalm internals via reflection because an API is missing.

Fix the root cause in the code, the stubs (send stub bugs upstream) or Psalm itself, or add the missing plugin API. If a proper fix is out of reach, say so and ask. List any workaround explicitly in the report.

Self-analysis (./psalm on this repo)

• psalm.xml.dist has runTaintAnalysis="true", so self-analysis includes taint resolution. Removing the attribute doesn't turn it off; set it to "false". Shepherd (./psalm --shepherd) is the only CI job that runs it.
• Full run: timeout 300 ./psalm --no-cache --output-format=compact. Don't pass --threads: Psalm detects the CPU count itself. Master takes roughly 30–180 s. A run past ~5 min is a hang, not slowness: in Shepherd's log a "runner has received a shutdown signal" during "Resolving taint graph" means a resolver blowup, not issues.
• One self-analysis can use many GB of RAM (up to ~16 GB has been seen), so never run two at once. Parallel agents serialize on the lock file /tmp/psalm-self-analysis.lock with the platform's lock command (see the table): flock /tmp/psalm-self-analysis.lock timeout 300 ./psalm ... on Linux, lockf -k /tmp/psalm-self-analysis.lock timeout 300 ./psalm ... on macOS.
• A file-subset run (./psalm <files>) misses class-level issues (MissingImmutableAnnotation) and reports spurious UnusedPsalmSuppress / UnusedBaselineEntry. Before removing a suppression or claiming self-analysis is clean, do a full run and diff it against a full run on origin/master.
• ComplexMethod/ComplexFunction are computed from the variable-use graph's edge stats (StatementsAnalyzer, getEdgeStats(): size > 200, mean path > 70, convergence > 1.1). A data-flow change can flip them on methods the PR never touched, making an existing @psalm-suppress ComplexMethod unused. Print the method's getEdgeStats() under base and branch to confirm no flow was lost before touching the suppression.
• Master enforces purity annotations on its own code (MissingPureAnnotation, enabled by suggestPurityAnnotations in psalm.xml.dist; off by default for users). ./psalm --alter --issues=MissingPureAnnotation <files> adds them.
• --set-baseline still printing new issues and exiting 2 is deliberate (#5838, a "fix" was reverted in #6051); the written baseline is complete. --update-baseline only shrinks entries.

Tests

• Tests must assert the change. Every test case a PR adds or changes for the bug or feature itself must fail on the base branch and pass on the PR, because it checks the exact behaviour being fixed or added. Prove it by running that single test case on both (see above). Edge-case tests that guard the change may pass on the base. Ways a test silently asserts nothing:
  • a valid-code case for a missed-issue fix (it passes on master too): use an invalid case asserting the issue;
  • an inferred-type fix without 'assertions' => ['$x' => 'expected type'] (key '$x===' compares the exact type id);
  • error_message only matches the first issue raised, as a substring ('InvalidReturnType' matches any line). If master already reports that issue type somewhere in the snippet, pin it with the location ('Issue - src' . DIRECTORY_SEPARATOR . 'somefile.php:13:31') or message text;
  • ignored_issues/error_levels suppressing the issue under test, or a higher php_version/missing extension making the code path not run;
  • harness differences: e.g. TaintTest provider cases don't initialize plugins (skill taint-analysis).
• Data-provider entries are passed by position; string keys are ignored. To pin a PHP version write 'assertions' => [], 'ignored_issues' => [], 'php_version' => '8.1' (valid) or 'error_levels' => [], 'php_version' => ... (invalid). 'php_version' alone becomes $assertions and TypeErrors. TaintTest is the exception: testInvalidCode(string $code, string $error_message, string $php_version = '8.0').
• Test cases analyse PHP 7.4 by default, and native symbols are version-gated, so tests using newer classes/methods need php_version.
• Put each docblock tag on its own line. /** @psalm-pure @param ... */ silently drops every tag after the first.
• Never start a docblock line in src/ with an example @psalm-... tag; Psalm parses it as a real annotation.
• DocumentationTest and invalid-code provider cases only see the first issue raised. When a change makes one location report several kinds, check every case and docs example that reaches it, and assert multi-issue results in a dedicated test method (throw_exception = false + IssueBuffer::getIssuesDataForFile()).
• IssueBuffer::finish() calls exit(2) when errors remain; test reported errors end to end (tests/EndToEnd/PsalmEndToEndTest.php).
• Resolving merge conflicts in big provider files (TaintTest especially): git factors shared trailing lines ('error_message' => ...,, ],) out of the hunk, so "keep both" can drop an entry's error_message or duplicate a ],, and php -l still passes. Afterwards, check with a script that every 12-space-indented 'name' => [ ... ] entry of both parents appears verbatim in the result, unless the other side changed it on purpose (compare with the merge base).

Code style

• Match the surrounding code: comment density, naming, idiom.
• Comments are concise and only explain what isn't obvious from the code:
  • don't restate the code in prose;
  • don't mention approaches that were tried, considered, replaced or removed, or anything that only makes sense in the context of an agent's iteration history;
  • prefer deleting an unnecessary comment over expanding or rewording it;
  • document an idea once, where it naturally belongs.
• Documentation (docs/, issue pages, user-facing docblocks): more examples, less text. Show behaviour with a code example whose lines end in their result (// TaintedNosql, // safe, // no issue), cases that stay reported included, and keep the prose around it to a sentence. Check each line's result on the branch before writing it.
• Hot loops (TaintFlowGraph::connectSinksAndSources(), TaintFlowResolution): keep the branch structure inline and call getChildNodes() directly in each branch. Don't collect nodes into intermediate arrays (allocation per iteration), and avoid new single-use helpers (they make diffs harder to read). Keep the order of side-effecting calls. Verify refactors by diffing full --monochrome output, traces included, on probe projects.

Debugging pitfalls

• STDERR from analysis gets swallowed by --output-format=compact/grep. Debug with file_put_contents('/path', ..., FILE_APPEND).
• Empty Psalm output means it didn't run (wrong cwd, missing vendor/, crash), not "clean". Only trust "No errors found" or an errors summary. Detect crashes (exit 11, segfault) when counting issues.

Shell, worktrees and git

• Write commands that work in both zsh (macOS default) and bash (usual on Linux); check with echo $0 if unsure:
  • Quote globs meant for the tool, not the shell (grep --include='*.php'). In zsh an unmatched glob aborts the whole && chain ("no matches found"); in bash it is passed on literally, so the command quietly acts on a path that doesn't exist. Either way the files you expect may not be there. Clean scratch dirs with find dir -name '*.php' -delete, and ls repro files before concluding anything from an absent issue.
  • Brace variables next to a colon: "${b}:branch", "${r}:tests/TaintTest.php". In zsh $b:l, $r:t, … are history modifiers and mangle refspecs; braces are harmless in bash.
  • Word-splitting a variable of args: zsh needs ${=args}, bash splits unquoted $args. Prefer an array (args=(a b); cmd "${args[@]}"), which behaves the same in both.
• The working directory resets between Bash calls. Start every command that edits, commits or runs Psalm in a worktree with cd <worktree> && (or use absolute paths), and check git status in the intended worktree before committing.
• All worktrees of this repository (sibling ../psalm-* checkouts, /tmp/psalm-*, …) share one stash list. Never use git stash there, and never pop or drop entries you didn't create; the list may hold other people's old stashes. To compare with and without a change, use git diff > f.patch + git apply -R, or a second worktree.
• Put scratch worktrees and probe dirs under /tmp (on macOS it is /private/tmp; both paths work) or the session scratchpad. /tmp may be wiped on reboot on both platforms, so don't keep anything there that must survive.
• A worktree that runs Psalm needs its own vendor/. Copy it with cp -R; a symlink makes composer's autoloader load the main checkout's classes. If the worktree will run phpunit or the main vendor/ may be stale against composer.json, composer install in it.

Platform differences

The instructions here and in .agents/skills/ target macOS and Linux. Where a command differs, use the right column for the current OS (uname prints Darwin or Linux):

Every agent on one machine must use the same lock command, so pick it by OS, not by whatever happens to be installed.