Skip to content

Commit

Permalink
Trim code snippets on both sides when comparing current issues agains…
Browse files Browse the repository at this point in the history
…t baseline

Should fix issue vimeo#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.
  • Loading branch information
bdsl committed Jun 26, 2021
1 parent 19cc4cb commit 0722401
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 3 deletions.
7 changes: 6 additions & 1 deletion src/Psalm/ErrorBaseline.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
}
}
Expand Down Expand Up @@ -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);
}
Expand Down
4 changes: 3 additions & 1 deletion src/Psalm/Internal/Analyzer/IssueData.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

namespace Psalm\Internal\Analyzer;

use function trim;

class IssueData
{
/**
Expand Down Expand Up @@ -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;
Expand Down
28 changes: 28 additions & 0 deletions tests/ErrorBaselineTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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(
"<?xml version=\"1.0\" encoding=\"UTF-8\"?>
<files>
<file src=\"sample/sample-file.php\">
<MixedAssignment occurrences=\"1\">
<code>foo\r</code>
</MixedAssignment>
</file>
</files>"
);

$expectedParsedBaseline = [
'sample/sample-file.php' => [
'MixedAssignment' => ['o' => 1, 's' => ['foo']],
],
];

$this->assertSame(
$expectedParsedBaseline,
ErrorBaseline::read($this->fileProvider->reveal(), $baselineFilePath)
);
}

/**
* @return void
*/
Expand Down
22 changes: 21 additions & 1 deletion tests/IssueBufferTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit 0722401

Please sign in to comment.