From 0722401aaf4b41928438fb7a921d5c85ca6221f9 Mon Sep 17 00:00:00 2001 From: Barney Laurance Date: Sat, 26 Jun 2021 21:59:40 +0100 Subject: [PATCH] Trim code snippets on both sides when comparing current issues against baseline Should fix issue #5979 We could also trim code issues when writing to baseline, but I think that's a minor BC break, so probably shouldn't happen until Psalm 9. --- src/Psalm/ErrorBaseline.php | 7 +++++- src/Psalm/Internal/Analyzer/IssueData.php | 4 +++- tests/ErrorBaselineTest.php | 28 +++++++++++++++++++++++ tests/IssueBufferTest.php | 22 +++++++++++++++++- 4 files changed, 58 insertions(+), 3 deletions(-) diff --git a/src/Psalm/ErrorBaseline.php b/src/Psalm/ErrorBaseline.php index 9ab0cbfc7d6..5d589d9d062 100644 --- a/src/Psalm/ErrorBaseline.php +++ b/src/Psalm/ErrorBaseline.php @@ -19,6 +19,7 @@ use function preg_replace_callback; use function str_replace; use function strpos; +use function trim; use function usort; use const LIBXML_NOBLANKS; @@ -115,7 +116,7 @@ public static function read(FileProvider $fileProvider, string $baselineFile): a $codeSamples = $issue->getElementsByTagName('code'); foreach ($codeSamples as $codeSample) { - $files[$fileName][$issueType]['s'][] = $codeSample->textContent; + $files[$fileName][$issueType]['s'][] = trim($codeSample->textContent); } } } @@ -273,6 +274,10 @@ function (string $extension) : string { foreach ($existingIssueType['s'] as $selection) { $codeNode = $baselineDoc->createElement('code'); + /** @todo in major version release (e.g. 5.0.0) replace $selection with trim($selection) + * This will be a minor BC break as baselines generated will then not be compatible with Psalm + * versions from before PR https://github.com/vimeo/psalm/pull/6000 + */ $codeNode->textContent = $selection; $issueNode->appendChild($codeNode); } diff --git a/src/Psalm/Internal/Analyzer/IssueData.php b/src/Psalm/Internal/Analyzer/IssueData.php index 363e8578e7f..4dddfbb7426 100644 --- a/src/Psalm/Internal/Analyzer/IssueData.php +++ b/src/Psalm/Internal/Analyzer/IssueData.php @@ -2,6 +2,8 @@ namespace Psalm\Internal\Analyzer; +use function trim; + class IssueData { /** @@ -154,7 +156,7 @@ public function __construct( $this->file_name = $file_name; $this->file_path = $file_path; $this->snippet = $snippet; - $this->selected_text = $selected_text; + $this->selected_text = trim($selected_text); $this->from = $from; $this->to = $to; $this->snippet_from = $snippet_from; diff --git a/tests/ErrorBaselineTest.php b/tests/ErrorBaselineTest.php index 92e846f2a1f..56be18758f3 100644 --- a/tests/ErrorBaselineTest.php +++ b/tests/ErrorBaselineTest.php @@ -67,6 +67,34 @@ public function testLoadShouldParseXmlBaselineToPhpArray() ); } + public function testLoadShouldIgnoreLineEndingsInIssueSnippet(): void + { + $baselineFilePath = 'baseline.xml'; + + $this->fileProvider->fileExists($baselineFilePath)->willReturn(true); + $this->fileProvider->getContents($baselineFilePath)->willReturn( + " + + + + foo\r + + + " + ); + + $expectedParsedBaseline = [ + 'sample/sample-file.php' => [ + 'MixedAssignment' => ['o' => 1, 's' => ['foo']], + ], + ]; + + $this->assertSame( + $expectedParsedBaseline, + ErrorBaseline::read($this->fileProvider->reveal(), $baselineFilePath) + ); + } + /** * @return void */ diff --git a/tests/IssueBufferTest.php b/tests/IssueBufferTest.php index d350906e295..6cef39c9d9e 100644 --- a/tests/IssueBufferTest.php +++ b/tests/IssueBufferTest.php @@ -54,11 +54,31 @@ public function testFinishDoesNotCorruptInternalState(): void 0, 0 ) - ] + ], + '/path/three.php' => [ + new \Psalm\Internal\Analyzer\IssueData( + "error", + 0, + 0, + "MissingPropertyType", + 'Message', + "three.php", + "/path/three.php", + "snippet-3-has-carriage-return" . "\r", + "snippet-3-has-carriage-return" . "\r", + 0, + 0, + 0, + 0, + 0, + 0 + ) + ], ]); $baseline = [ 'one.php' => ['MissingPropertyType' => ['o' => 1, 's' => ['snippet-1']] ], 'two.php' => ['MissingPropertyType' => ['o' => 1, 's' => ['snippet-2']] ], + 'three.php' => ['MissingPropertyType' => ['o' => 1, 's' => ['snippet-3-has-carriage-return']] ], ]; $analyzer = $this->createMock(Analyzer::class);