Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

485: Include output from Psalm in report when mutatnt killed by stati… #490

Open
wants to merge 2 commits into
base: 1.35.x
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -5,26 +5,36 @@
namespace Roave\InfectionStaticAnalysis\Psalm;

use Infection\Mutant\Mutant;
use Psalm\Internal\Analyzer\IssueData;
use Psalm\Internal\Analyzer\ProjectAnalyzer;

use function array_key_exists;
use function array_map;
use function count;
use function implode;

/**
* @internal

Check failure on line 16 in src/Roave/InfectionStaticAnalysis/Psalm/RunStaticAnalysisAgainstMutant.php

View workflow job for this annotation

GitHub Actions / QA Checks (PHPCodeSniffer [8.1, locked], ubuntu-latest, laminas/laminas-continuous-integration-ac...

Incorrect annotations group.
* @psalm-suppress InternalProperty - we use Psalm's internal IssueData class here. Afaik the only other way to
* display details of the issues would be to use one of the subclasses of \Psalm\Report, but I think none are exactly
* what we want. Probably we can accept the risk of Psalm's internals changing and breaking this.
Comment on lines +17 to +19
Copy link
Member

Choose a reason for hiding this comment

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

@orklah how stable is this stuff? Just want to see if we need to pin to psalm at patch level, from now on 🤔

Copy link
Author

Choose a reason for hiding this comment

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

I'm also happy to have a go at using one of the subclasses of https://github.com/vimeo/psalm/blob/5.x/src/Psalm/Report.php to avoid the InternalProperty access.

Copy link
Member

Choose a reason for hiding this comment

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

Note: we pin to the patch version of infection/infection because we want to prevent inadverted BC breaks.

If we can avoid depending on @internal in Psalm, we could avoid having to do it for vimeo/psalm

Copy link

Choose a reason for hiding this comment

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

Honestly, Psalm's internals are not easily accessible. We added @internal mainly to have some support for BC breaks checks.

Psalm is not moving very fast currently so the risk of things breaking is low though

*
* @final not explicitly final because we don't yet have a uniform API for this type of analysis
*/
class RunStaticAnalysisAgainstMutant
{
private bool $alreadyVisitedStubs = false;

/** @var IssueData[] */
private array $psalmIssuesFromLastMutant = [];

public function __construct(private ProjectAnalyzer $projectAnalyzer)
{
}

public function isMutantStillValidAccordingToStaticAnalysis(Mutant $mutant): bool
{
$this->psalmIssuesFromLastMutant = [];

$path = $mutant->getFilePath();
$paths = [$mutant->getFilePath()];
$codebase = $this->projectAnalyzer->getCodebase();
Expand All @@ -48,13 +58,18 @@
$codebase->reloadFiles($this->projectAnalyzer, $paths);
$codebase->analyzer->analyzeFiles($this->projectAnalyzer, count($paths), false);

$mutationValid = ! array_key_exists(
$path,
$codebase->file_reference_provider->getExistingIssues(),
);
$this->psalmIssuesFromLastMutant = $codebase->file_reference_provider->getExistingIssues()[$path] ?? [];

$codebase->invalidateInformationForFile($path);

return $mutationValid;
return $this->psalmIssuesFromLastMutant === [];
}

public function formatLastIssues(): string
{
return implode(
", ",

Check failure on line 71 in src/Roave/InfectionStaticAnalysis/Psalm/RunStaticAnalysisAgainstMutant.php

View workflow job for this annotation

GitHub Actions / QA Checks (PHPCodeSniffer [8.1, locked], ubuntu-latest, laminas/laminas-continuous-integration-ac...

String ", " does not require double quotes; use single quotes instead
array_map(static fn (IssueData $issueData) => ($issueData->type . ': ' . $issueData->message), $this->psalmIssuesFromLastMutant),
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,8 @@ public function createFromProcess(MutantProcess $mutantProcess): MutantExecution
assert(is_int($originalEndFilePosition));

return new MutantExecutionResult(
$result->getProcessCommandLine(),
$result->getProcessOutput(),
'Static Analysis',
$this->runStaticAnalysis->formatLastIssues(),
DetectionStatus::KILLED, // Mutant was squished by static analysis
later(static fn () => yield $result->getMutantDiff()),
$result->getMutantHash(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,16 @@ function add(int $a, int $b): int {
}
PHP,
)));

self::assertStringContainsString(
"InvalidReturnStatement: The inferred type 'int<min, max>' does not match the declared return type 'int<1, max>' for add",
$this->runStaticAnalysis->formatLastIssues(),
);

self::assertStringContainsString(
"InvalidReturnType: The declared return type 'int<1, max>' for add is incorrect, got 'int<min, max>'",
$this->runStaticAnalysis->formatLastIssues(),
);
}

public function testWillConsiderMutantReferencingProjectFilesAsValid(): void
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,9 +103,13 @@ public function testWillKillMutantsThatEscapedAndFailedStaticAnalysis(): void
->method('isMutantStillValidAccordingToStaticAnalysis')
->willReturn(false);

$this->staticAnalysis->expects(self::any())
->method('formatLastIssues')
->willReturn('formatted Psalm issues');

$nextFactoryResult = new MutantExecutionResult(
'echo hi',
'output',
'formatted Psalm issues',
DetectionStatus::ESCAPED,
now('diff'),
'a-hash',
Expand Down Expand Up @@ -136,7 +140,7 @@ public function testWillKillMutantsThatEscapedAndFailedStaticAnalysis(): void
self::assertEquals($nextFactoryResult->getMutantDiff(), $result->getMutantDiff());
self::assertEquals($nextFactoryResult->getMutantHash(), $result->getMutantHash());
self::assertEquals($nextFactoryResult->getProcessOutput(), $result->getProcessOutput());
self::assertEquals($nextFactoryResult->getProcessCommandLine(), $result->getProcessCommandLine());
self::assertEquals('Static Analysis', $result->getProcessCommandLine());
self::assertSame(DetectionStatus::KILLED, $result->getDetectionStatus());

$reflectionOriginalStartFileLocation = new ReflectionProperty(MutantExecutionResult::class, 'originalStartFilePosition');
Expand Down