Skip to content

fix: phpstan stderr note line causes parser_error; add phpstan JSON parser#420

Open
nickperkins wants to merge 3 commits intopeteromallet:mainfrom
nickperkins:fix/phpstan-stderr-and-json-parser
Open

fix: phpstan stderr note line causes parser_error; add phpstan JSON parser#420
nickperkins wants to merge 3 commits intopeteromallet:mainfrom
nickperkins:fix/phpstan-stderr-and-json-parser

Conversation

@nickperkins
Copy link

Problem

PHPStan output was silently discarded on every scan, logged as phpstan tooling unavailable (parser_error) then (tool_failed_unparsed_output).

Root cause 1 — tool_runner.py:
Line 94 combined stdout + stderr before JSON parsing. PHPStan writes a diagnostic note to stderr:

Note: Using configuration file /path/to/phpstan.neon.dist.

This gets prepended to the stdout JSON, making the combined string invalid JSON → parser_error.

Root cause 2 — PHP plugin fmt='json':
After fixing root cause 1, parsing still returned an empty list. The plugin declared fmt='json', which routes to parse_json() — a flat-array parser. PHPStan outputs a nested object:

{"totals": {...}, "files": {"/path/to/file.php": {"messages": [{"message": "...", "line": 42}]}}}

parse_json() expects a top-level array and returns []tool_failed_unparsed_output.

Fix

  1. tool_runner.py: Use result.stdout as the parse input when stdout is non-empty; only fall back to stdout + stderr for the emptiness check.

  2. parsers.py: Add parse_phpstan() which correctly handles the nested {"files": {path: {"messages": [...]}}}} format. Register it in PARSERS as "phpstan".

  3. php/__init__.py: Change fmt='json'fmt='phpstan'.

Verification

After applying fixes, phpstan output is correctly parsed — 17 real errors from the test project are now surfaced by the detector. All 5363 existing tests pass.

…arser

Two bugs prevented phpstan output from being parsed:

1. tool_runner.py combined stdout+stderr before parsing. PHPStan writes a
   'Note: Using configuration file ...' line to stderr, which gets prepended
   to the stdout JSON, causing a json.JSONDecodeError classified as
   parser_error. Fix: use stdout only as the parse_input when stdout is
   non-empty; fall back to combined output only for the emptiness check.

2. The PHP plugin declared fmt='json' which routes to parse_json(), a flat
   array parser. PHPStan outputs a nested object:
     {"files": {"<path>": {"messages": [{"message", "line"}]}}}
   parse_json() treats this as an empty list, causing tool_failed_unparsed_output.
   Fix: add a dedicated parse_phpstan() parser and register it as 'phpstan',
   then update the PHP plugin to use fmt='phpstan'.
Copilot AI review requested due to automatic review settings March 13, 2026 20:45
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes PHPStan integration in the generic tool framework so PHPStan findings are no longer dropped due to stderr “Note:” output and mismatched JSON shape parsing.

Changes:

  • Adjust tool execution parsing to prefer stdout for structured parsing while still using combined output for emptiness checks and error previews.
  • Add a dedicated PHPStan JSON parser to handle PHPStan’s nested {"files": {...}} output format.
  • Update the PHP language plugin to use the new phpstan parser format instead of the generic json parser.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.

File Description
desloppify/languages/php/__init__.py Switch PHPStan tool spec from fmt="json" to fmt="phpstan" so output is parsed correctly.
desloppify/languages/_framework/generic_parts/tool_runner.py Prevent stderr diagnostics from corrupting JSON parsing by parsing stdout when available.
desloppify/languages/_framework/generic_parts/parsers.py Introduce and register parse_phpstan() to extract entries from PHPStan’s nested JSON schema.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

- Reword tool_runner.py comment to accurately describe parse_input logic
  (parse stdout when present; fall back to combined only when stdout empty)
- Add isinstance(msg, dict) guard in parse_phpstan to skip malformed entries
- Fix invalid JSON example in parse_phpstan docstring
- Add TestParsePhpstan unit tests (normal output, empty files, empty
  messages, non-dict messages, non-dict file values, invalid JSON, non-dict root)
- Add run_tool_result tests: stderr preamble ignored when stdout has JSON,
  and error preview uses combined output when parsing fails

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes PHPStan integration in the generic tool runner by preventing stderr diagnostics from corrupting JSON parsing and by adding a dedicated parser for PHPStan’s nested JSON schema.

Changes:

  • Update run_tool_result() to parse stdout preferentially (avoids stderr preambles breaking JSON) while still using combined output for “no output” checks and error previews.
  • Add parse_phpstan() parser and register it as a new "phpstan" format.
  • Switch the PHP plugin’s PHPStan tool spec to fmt="phpstan" and add unit tests for the new parser and runner behavior.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
desloppify/languages/_framework/generic_parts/tool_runner.py Adjusts parse input selection to avoid JSON corruption from stderr noise; keeps combined output for emptiness/preview.
desloppify/languages/_framework/generic_parts/parsers.py Adds a PHPStan-specific JSON parser and registers it in PARSERS.
desloppify/languages/php/__init__.py Updates PHPStan tool spec to use the new "phpstan" parser format.
desloppify/tests/lang/common/test_generic_plugin.py Adds coverage for parse_phpstan() and for parsing stdout successfully despite stderr diagnostics.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +451 to +461
stdout="not json at all",
stderr="Some stderr diagnostic\n",
)
result = run_tool_result(
"fake",
tmp_path,
parse_json,
run_subprocess=lambda *_a, **_k: result_bad,
)
assert result.status == "error"
assert result.error_kind == "parser_error"
…utput

The previous test hit the parser_error path (no preview in message).
Switch to using parse_json with a non-array object + non-zero returncode
to exercise tool_failed_unparsed_output, where _output_preview(combined)
is embedded in the message. Assert both stdout and stderr appear in it.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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