Skip to content

Commit b3b577d

Browse files
author
Test
committed
Merge branch '3.3.2' into rc-3.3.2
2 parents 4c2683c + a28770f commit b3b577d

4 files changed

Lines changed: 300 additions & 1 deletion

File tree

‎docs/changelog.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,10 @@ All notable changes to this project are documented here.
88

99
- Code Climate and SARIF reports requested via `flows.options.reports.codeclimate` / `reports.sarif` in config or via `--report-codeclimate=PATH` / `--report-sarif=PATH` CLI flags came out empty (`[]` / no findings) when the primary `--format` was anything other than `codeclimate` / `sarif`. The flag that asks each tool for JSON output only activated on `--format=codeclimate|sarif`, so every tool ran with its default human-text format and the report parsers (which all do `json_decode()` over stdout) found nothing to extract. Affects every tool with a JSON-dependent parser: PHPStan (`--error-format=json`), PHPCS (`--report=json`), PHPMD (positional `json` format), Psalm (`--output-format=json`) and parallel-lint (`--json`). Fixed: tool-level JSON output is now requested whenever a codeclimate or sarif payload will be produced, regardless of how it was requested.
1010

11+
### Improved
12+
13+
- JUnit `<failure>` payloads now pretty-print embedded JSON so GitLab/Jenkins viewers render each finding on its own indented block. PHPMD already emits `JSON_PRETTY_PRINT` natively; PHPStan/PHPCS/Psalm/parallel-lint emit compact one-liner JSON. When a pipeline triggers tool JSON output (typical GitLab setup pairs JUnit + Code Climate), the JUnit `<failure>` arrived as a single 1000+ char line. The formatter now detects a parseable JSON span inside `<failure>` and re-encodes it with indentation; non-JSON outputs (custom jobs, scripts) and JSON with prologue/epilogue (PHPStan's "Instructions for interpreting errors" stderr block) are preserved verbatim except for the JSON span itself. Idempotent for already-pretty payloads (semantics preserved; bytes may differ because `JSON_UNESCAPED_SLASHES` turns `\/` into `/`).
14+
1115
## [3.3.1]
1216

1317
### Fixed

‎src/Output/JunitResultFormatter.php‎

Lines changed: 78 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,8 @@ public function format(FlowResult $result): string
4242
} elseif (!$jobResult->isSuccess()) {
4343
$failure = $dom->createElement('failure');
4444
$failure->setAttribute('message', $jobResult->getJobName() . ' failed');
45-
$failure->appendChild($dom->createTextNode($this->stripAnsi($jobResult->getOutput())));
45+
$cleaned = $this->stripAnsi($jobResult->getOutput());
46+
$failure->appendChild($dom->createTextNode($this->prettyJsonIfApplicable($cleaned)));
4647
$testcase->appendChild($failure);
4748
}
4849

@@ -57,6 +58,82 @@ private function stripAnsi(string $text): string
5758
return (string) preg_replace('/\x1B(?:\[[0-9;]*[A-Za-z]|\][^\x07]*\x07)|\r/', '', $text);
5859
}
5960

61+
/**
62+
* Pretty-print JSON outputs so JUnit viewers (GitLab, Jenkins) render
63+
* each tool's payload in a readable shape.
64+
*
65+
* Rationale: phpmd already emits JSON_PRETTY_PRINT by default, so a JUnit
66+
* viewer renders it nicely. phpstan/phpcs/psalm/parallel-lint emit
67+
* compact JSON (one line). When phpstan also writes informational text
68+
* to stderr (instructions block), githooks captures stdout+stderr
69+
* combined, so `<failure>` arrives as `<compact-json><explanatory-text>`.
70+
* We detect the JSON document (first `{` or `[` to its matching closer),
71+
* pretty-print only that span, and preserve any non-JSON prologue and
72+
* epilogue verbatim.
73+
*
74+
* Edge-cases:
75+
* - phpmd already pretty stays pretty (semantics preserved; bytes may
76+
* differ because JSON_UNESCAPED_SLASHES turns `\/` into `/`).
77+
* - non-JSON output (custom job, script) passes through unchanged.
78+
* - JSON we can't parse passes through unchanged.
79+
*/
80+
private function prettyJsonIfApplicable(string $text): string
81+
{
82+
if ($text === '') {
83+
return $text;
84+
}
85+
86+
$bounds = $this->findJsonBounds($text);
87+
if ($bounds === null) {
88+
return $text;
89+
}
90+
[$start, $end] = $bounds;
91+
92+
$jsonPart = substr($text, $start, $end - $start + 1);
93+
$decoded = json_decode($jsonPart, true);
94+
if ($decoded === null && json_last_error() !== JSON_ERROR_NONE) {
95+
return $text;
96+
}
97+
98+
$pretty = json_encode($decoded, JSON_PRETTY_PRINT | JSON_UNESCAPED_UNICODE | JSON_UNESCAPED_SLASHES);
99+
if ($pretty === false) {
100+
return $text;
101+
}
102+
103+
return substr($text, 0, $start) . $pretty . substr($text, $end + 1);
104+
}
105+
106+
/**
107+
* Locate the outermost JSON document inside `$text`. Picks the earliest
108+
* opener (`{` or `[`) and pairs it with the matching last closer of the
109+
* same kind. Returns [start, end] inclusive, or null when no plausible
110+
* JSON span exists.
111+
*
112+
* @return array{0:int,1:int}|null
113+
*/
114+
private function findJsonBounds(string $text): ?array
115+
{
116+
$objStart = strpos($text, '{');
117+
$arrStart = strpos($text, '[');
118+
119+
if ($objStart !== false && ($arrStart === false || $objStart < $arrStart)) {
120+
return $this->boundsFor($text, (int) $objStart, '}');
121+
}
122+
if ($arrStart !== false) {
123+
return $this->boundsFor($text, (int) $arrStart, ']');
124+
}
125+
return null;
126+
}
127+
128+
/**
129+
* @return array{0:int,1:int}|null
130+
*/
131+
private function boundsFor(string $text, int $start, string $closer): ?array
132+
{
133+
$end = strrpos($text, $closer);
134+
return ($end !== false && $end > $start) ? [$start, (int) $end] : null;
135+
}
136+
60137
/**
61138
* Convert formatted time strings (e.g. "234ms", "1.23s", "2m 30s") to seconds.
62139
*

‎tests/System/Release/V32FeaturesReleaseTest.php‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -737,6 +737,69 @@ public function bug16_cli_report_sarif_captures_phpstan_issues_without_format_fl
737737
@unlink($reportPath);
738738
}
739739

740+
// ========================================================================
741+
// 3.3.2 JUnit pretty-print: tools that emit compact JSON (phpstan, phpcs,
742+
// psalm, parallel-lint) ship one-liner payloads; GitLab/Jenkins viewers
743+
// render <failure> verbatim, leaving the panel unreadable. The formatter
744+
// now pretty-prints JSON inside <failure> so each finding gets its own
745+
// indented block. phpmd already pretty-prints natively — its semantics
746+
// are preserved across the round-trip.
747+
// ========================================================================
748+
749+
/** @test */
750+
public function junit_failure_pretty_prints_phpstan_compact_json_output()
751+
{
752+
$srcDir = self::TESTS_PATH . '/src';
753+
@mkdir($srcDir, 0777, true);
754+
$phpstanFile = $this->phpFileBuilder->buildWithErrors([\Tests\Utils\PhpFileBuilder::PHPSTAN]);
755+
file_put_contents("$srcDir/Bad.php", $phpstanFile);
756+
757+
$junitPath = self::TESTS_PATH . '/qa.junit.xml';
758+
$codeclimatePath = self::TESTS_PATH . '/qa.cc.json';
759+
@unlink($junitPath);
760+
@unlink($codeclimatePath);
761+
762+
$this->configurationFileBuilder
763+
->setV3Flows(['qa' => ['jobs' => ['phpstan_src']]])
764+
->setV3Jobs([
765+
'phpstan_src' => ['type' => 'phpstan', 'level' => 0, 'paths' => [$srcDir]],
766+
]);
767+
file_put_contents($this->configPath, $this->configurationFileBuilder->buildV3Php());
768+
769+
// structuredFormat must be active so phpstan emits JSON. We trigger it
770+
// via --report-codeclimate (typical GitLab pipeline shape: JUnit + CC
771+
// generated together). The bug only manifests when phpstan actually
772+
// emits compact JSON into the JUnit <failure>.
773+
shell_exec(
774+
"$this->githooks flow qa --format=junit --output=$junitPath"
775+
. " --report-codeclimate=$codeclimatePath --config=$this->configPath 2>/dev/null"
776+
);
777+
778+
$this->assertFileExists($junitPath);
779+
$dom = new \DOMDocument();
780+
$this->assertTrue($dom->load($junitPath), 'junit output must be valid XML');
781+
782+
$failures = $dom->getElementsByTagName('failure');
783+
$this->assertGreaterThan(0, $failures->length, 'phpstan errors must produce a <failure> element');
784+
785+
$failureText = $failures->item(0)->textContent;
786+
$bodyNewlines = substr_count(trim($failureText), "\n");
787+
$this->assertGreaterThan(
788+
5,
789+
$bodyNewlines,
790+
'phpstan JSON inside <failure> must be pretty-printed (>5 newlines); got ' . $bodyNewlines
791+
);
792+
793+
$jsonEnd = strrpos($failureText, '}');
794+
$jsonOnly = substr($failureText, 0, $jsonEnd + 1);
795+
$decoded = json_decode(trim($jsonOnly), true);
796+
$this->assertIsArray($decoded);
797+
$this->assertArrayHasKey('files', $decoded);
798+
799+
@unlink($junitPath);
800+
@unlink($codeclimatePath);
801+
}
802+
740803
/** @test */
741804
public function dashboard_falls_back_to_streaming_when_stdout_is_not_a_tty()
742805
{

‎tests/Unit/Output/JunitResultFormatterTest.php‎

Lines changed: 155 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -242,4 +242,159 @@ function parse_seconds_handles_each_format_branch()
242242
$this->assertSame('150', $reflection->invoke($formatter, '2m 30s'));
243243
$this->assertSame('60', $reflection->invoke($formatter, '1m 0s'));
244244
}
245+
246+
// ========================================================================
247+
// Pretty-print JSON in <failure> (3.3.2):
248+
// GitLab/Jenkins JUnit viewers render <failure> verbatim. Tools that emit
249+
// compact JSON (phpstan, phpcs, psalm, parallel-lint) arrive unreadable;
250+
// tools that already pretty-print (phpmd) must keep their semantics.
251+
// ========================================================================
252+
253+
/** @test */
254+
function compact_json_failure_output_is_pretty_printed()
255+
{
256+
$compactPhpstan = '{"totals":{"errors":0,"file_errors":4},"files":{"src/Foo.php":{"errors":4,"messages":[{"message":"Method has no return type.","line":8,"identifier":"missingType.return"}]}},"errors":[]}';
257+
258+
$result = new FlowResult('qa', [
259+
new JobResult('phpstan_src', false, $compactPhpstan, '1s'),
260+
], '1s');
261+
262+
$formatter = new JunitResultFormatter();
263+
$dom = new DOMDocument();
264+
$dom->loadXML($formatter->format($result));
265+
266+
$failureText = $dom->getElementsByTagName('failure')->item(0)->textContent;
267+
268+
$this->assertStringContainsString("\n", $failureText);
269+
$this->assertStringContainsString(' ', $failureText);
270+
$decoded = json_decode($failureText, true);
271+
$this->assertIsArray($decoded);
272+
$this->assertSame(4, $decoded['totals']['file_errors']);
273+
}
274+
275+
/** @test */
276+
function pretty_printed_json_failure_output_stays_pretty_semantically()
277+
{
278+
$payload = [
279+
'version' => '2.15.0',
280+
'package' => 'phpmd',
281+
'files' => [['file' => '/abs/path/Foo.php', 'violations' => [['beginLine' => 8, 'description' => 'Avoid unused parameters.']]]],
282+
];
283+
$prettyPhpmdEscaped = json_encode($payload, JSON_PRETTY_PRINT);
284+
285+
$result = new FlowResult('qa', [
286+
new JobResult('phpmd_src', false, $prettyPhpmdEscaped, '500ms'),
287+
], '500ms');
288+
289+
$formatter = new JunitResultFormatter();
290+
$dom = new DOMDocument();
291+
$dom->loadXML($formatter->format($result));
292+
293+
$failureText = $dom->getElementsByTagName('failure')->item(0)->textContent;
294+
295+
$this->assertStringContainsString("\n", $failureText);
296+
$this->assertStringContainsString(' ', $failureText);
297+
$this->assertSame($payload, json_decode($failureText, true));
298+
}
299+
300+
/** @test */
301+
function non_json_failure_output_passes_through_unchanged()
302+
{
303+
$rawOutput = "Test failed: expected 5, got 4\nStack trace:\n at line 23";
304+
305+
$result = new FlowResult('qa', [
306+
new JobResult('custom_test', false, $rawOutput, '100ms'),
307+
], '100ms');
308+
309+
$formatter = new JunitResultFormatter();
310+
$dom = new DOMDocument();
311+
$dom->loadXML($formatter->format($result));
312+
313+
$failureText = $dom->getElementsByTagName('failure')->item(0)->textContent;
314+
315+
$this->assertSame($rawOutput, $failureText);
316+
}
317+
318+
/** @test */
319+
function json_with_prologue_or_epilogue_pretty_prints_only_the_json_span()
320+
{
321+
$compactJson = '{"totals":{"errors":0,"file_errors":1},"files":{"src/Bad.php":{"errors":1,"messages":[{"message":"Undefined variable","line":9}]}}}';
322+
$epilogue = "\nInstructions for interpreting errors\n---------\nSee https://phpstan.org/...";
323+
$combined = $compactJson . $epilogue;
324+
325+
$result = new FlowResult('qa', [
326+
new JobResult('phpstan_src', false, $combined, '1s'),
327+
], '1s');
328+
329+
$formatter = new JunitResultFormatter();
330+
$dom = new DOMDocument();
331+
$dom->loadXML($formatter->format($result));
332+
333+
$failureText = $dom->getElementsByTagName('failure')->item(0)->textContent;
334+
335+
$this->assertGreaterThan(3, substr_count($failureText, "\n"));
336+
$this->assertStringContainsString('Instructions for interpreting errors', $failureText);
337+
$this->assertStringContainsString('https://phpstan.org/', $failureText);
338+
$jsonEnd = strrpos($failureText, '}');
339+
$jsonOnly = substr($failureText, 0, $jsonEnd + 1);
340+
$decoded = json_decode($jsonOnly, true);
341+
$this->assertSame(1, $decoded['totals']['file_errors']);
342+
}
343+
344+
/** @test */
345+
function empty_failure_output_passes_through_unchanged()
346+
{
347+
$result = new FlowResult('qa', [
348+
new JobResult('silent_failure', false, '', '50ms'),
349+
], '50ms');
350+
351+
$formatter = new JunitResultFormatter();
352+
$dom = new DOMDocument();
353+
$dom->loadXML($formatter->format($result));
354+
355+
$failure = $dom->getElementsByTagName('failure')->item(0);
356+
$this->assertSame('', $failure->textContent);
357+
}
358+
359+
/** @test */
360+
function json_array_payload_is_also_pretty_printed()
361+
{
362+
$compactPsalm = '[{"file_name":"src/Foo.php","line_from":8,"message":"Type X is undefined","severity":"error"}]';
363+
364+
$result = new FlowResult('qa', [
365+
new JobResult('psalm_src', false, $compactPsalm, '1s'),
366+
], '1s');
367+
368+
$formatter = new JunitResultFormatter();
369+
$dom = new DOMDocument();
370+
$dom->loadXML($formatter->format($result));
371+
372+
$failureText = $dom->getElementsByTagName('failure')->item(0)->textContent;
373+
374+
$this->assertStringContainsString("\n", $failureText);
375+
$decoded = json_decode($failureText, true);
376+
$this->assertIsArray($decoded);
377+
$this->assertSame('Type X is undefined', $decoded[0]['message']);
378+
}
379+
380+
/** @test */
381+
function pretty_print_runs_after_ansi_strip()
382+
{
383+
$jsonWithAnsi = "\e[31m" . '{"errors":1}' . "\e[0m";
384+
385+
$result = new FlowResult('qa', [
386+
new JobResult('phpcs_src', false, $jsonWithAnsi, '1s'),
387+
], '1s');
388+
389+
$formatter = new JunitResultFormatter();
390+
$dom = new DOMDocument();
391+
$dom->loadXML($formatter->format($result));
392+
393+
$failureText = $dom->getElementsByTagName('failure')->item(0)->textContent;
394+
395+
$this->assertStringNotContainsString("\e[", $failureText);
396+
$this->assertStringContainsString("\n", $failureText);
397+
$decoded = json_decode($failureText, true);
398+
$this->assertSame(1, $decoded['errors']);
399+
}
245400
}

0 commit comments

Comments
 (0)