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

Detect syntax errors during mutation analysis and differentiate them from all errors #1555

Merged
merged 5 commits into from Aug 17, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion composer.json
Expand Up @@ -47,7 +47,7 @@
"ext-json": "*",
"ext-libxml": "*",
"composer/xdebug-handler": "^2.0",
"infection/abstract-testframework-adapter": "^0.3.1",
"infection/abstract-testframework-adapter": "^0.5.0",
"infection/extension-installer": "^0.1.0",
"infection/include-interceptor": "^0.2.4",
"justinrainbow/json-schema": "^5.2.10",
Expand Down
30 changes: 20 additions & 10 deletions composer.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions resources/schema.json
Expand Up @@ -339,6 +339,7 @@
"SpreadOneItem": { "$ref": "#/definitions/default-mutator-config" },
"SpreadAssignment": { "$ref": "#/definitions/default-mutator-config" },
"SpreadRemoval": { "$ref": "#/definitions/default-mutator-config" },
"SyntaxError": { "$ref": "#/definitions/default-mutator-config" },
"Foreach_": { "$ref": "#/definitions/default-mutator-config" },
"For_": { "$ref": "#/definitions/default-mutator-config" },
"DoWhile": { "$ref": "#/definitions/default-mutator-config" },
Expand Down
5 changes: 5 additions & 0 deletions src/Console/OutputFormatter/DotFormatter.php
Expand Up @@ -66,6 +66,7 @@ public function start(int $mutationCount): void
. '<escaped>M</escaped>: escaped, '
. '<uncovered>U</uncovered>: uncovered, '
. '<with-error>E</with-error>: fatal error, '
. '<with-syntax-error>X</with-syntax-error>: syntax error, '
. '<timeout>T</timeout>: timed out, '
. '<skipped>S</skipped>: skipped',
'',
Expand Down Expand Up @@ -100,6 +101,10 @@ public function advance(MutantExecutionResult $executionResult, int $mutationCou
case DetectionStatus::ERROR:
$this->output->write('<with-error>E</with-error>');

break;
case DetectionStatus::SYNTAX_ERROR:
$this->output->write('<with-syntax-error>X</with-syntax-error>');

break;
}

Expand Down
1 change: 1 addition & 0 deletions src/Console/OutputFormatterStyleConfigurator.php
Expand Up @@ -59,6 +59,7 @@ public static function configure(OutputInterface $output): void
private static function configureMutantStyle(OutputFormatterInterface $formatter): void
{
$formatter->setStyle('with-error', new OutputFormatterStyle('green'));
$formatter->setStyle('with-syntax-error', new OutputFormatterStyle('red', null, ['bold']));
$formatter->setStyle(
'uncovered',
new OutputFormatterStyle('blue', null, ['bold'])
Expand Down
Expand Up @@ -150,6 +150,7 @@ private function showMetrics(): void
$this->output->writeln('<options=bold>' . $this->getPadded($this->metricsCalculator->getNotTestedCount()) . '</options=bold> mutants were not covered by tests');
$this->output->writeln('<options=bold>' . $this->getPadded($this->metricsCalculator->getEscapedCount()) . '</options=bold> covered mutants were not detected');
$this->output->writeln('<options=bold>' . $this->getPadded($this->metricsCalculator->getErrorCount()) . '</options=bold> errors were encountered');
$this->output->writeln('<options=bold>' . $this->getPadded($this->metricsCalculator->getSyntaxErrorCount()) . '</options=bold> syntax errors were encountered');
$this->output->writeln('<options=bold>' . $this->getPadded($this->metricsCalculator->getTimedOutCount()) . '</options=bold> time outs were encountered');
$this->output->writeln('<options=bold>' . $this->getPadded($this->metricsCalculator->getSkippedCount()) . '</options=bold> mutants required more time than configured');

Expand Down
5 changes: 5 additions & 0 deletions src/Logger/DebugFileLogger.php
Expand Up @@ -84,6 +84,11 @@ public function getLogLines(): array
'Errors',
$separateSections
);
$logs[] = $this->getResultsLine(
$this->resultsCollector->getSyntaxErrorExecutionResults(),
'Syntax Errors',
$separateSections
);
$logs[] = $this->getResultsLine(
$this->resultsCollector->getEscapedExecutionResults(),
'Escaped',
Expand Down
2 changes: 1 addition & 1 deletion src/Logger/FederatedLogger.php
Expand Up @@ -43,7 +43,7 @@ final class FederatedLogger implements MutationTestingResultsLogger
/**
* @var MutationTestingResultsLogger[]
*/
private $loggers;
private array $loggers;

public function __construct(MutationTestingResultsLogger ...$loggers)
{
Expand Down
2 changes: 2 additions & 0 deletions src/Logger/JsonLogger.php
Expand Up @@ -74,6 +74,7 @@ public function getLogLines(): array
'notCoveredCount' => $this->metricsCalculator->getNotTestedCount(),
'escapedCount' => $this->metricsCalculator->getEscapedCount(),
'errorCount' => $this->metricsCalculator->getErrorCount(),
'syntaxErrorCount' => $this->metricsCalculator->getSyntaxErrorCount(),
'skippedCount' => $this->metricsCalculator->getSkippedCount(),
'timeOutCount' => $this->metricsCalculator->getTimedOutCount(),
'msi' => $this->metricsCalculator->getMutationScoreIndicator(),
Expand All @@ -84,6 +85,7 @@ public function getLogLines(): array
'timeouted' => $this->getResultsLine($this->resultsCollector->getTimedOutExecutionResults()),
'killed' => $this->getResultsLine($this->resultsCollector->getKilledExecutionResults()),
'errored' => $this->getResultsLine($this->resultsCollector->getErrorExecutionResults()),
'syntaxErrors' => $this->getResultsLine($this->resultsCollector->getSyntaxErrorExecutionResults()),
'uncovered' => $this->onlyCoveredMode ? [] : $this->getResultsLine($this->resultsCollector->getNotCoveredExecutionResults()),
];

Expand Down
3 changes: 2 additions & 1 deletion src/Logger/PerMutatorLogger.php
Expand Up @@ -73,7 +73,7 @@ public function getLogLines(): array
$calculatorPerMutator = $this->createMetricsPerMutators();

$table = [
['Mutator', 'Mutations', 'Killed', 'Escaped', 'Errors', 'Timed Out', 'Skipped', 'MSI (%s)', 'Covered MSI (%s)'],
['Mutator', 'Mutations', 'Killed', 'Escaped', 'Errors', 'Syntax Errors', 'Timed Out', 'Skipped', 'MSI (%s)', 'Covered MSI (%s)'],
];

foreach ($calculatorPerMutator as $mutatorName => $calculator) {
Expand All @@ -85,6 +85,7 @@ public function getLogLines(): array
(string) $calculator->getKilledCount(),
(string) $calculator->getEscapedCount(),
(string) $calculator->getErrorCount(),
(string) $calculator->getSyntaxErrorCount(),
(string) $calculator->getTimedOutCount(),
(string) $calculator->getSkippedCount(),
self::formatScore($calculator->getMutationScoreIndicator()),
Expand Down
1 change: 1 addition & 0 deletions src/Logger/SummaryFileLogger.php
Expand Up @@ -59,6 +59,7 @@ public function getLogLines(): array
'',
'Killed: ' . $this->metricsCalculator->getKilledCount(),
'Errored: ' . $this->metricsCalculator->getErrorCount(),
'Syntax Errors: ' . $this->metricsCalculator->getSyntaxErrorCount(),
'Escaped: ' . $this->metricsCalculator->getEscapedCount(),
'Timed Out: ' . $this->metricsCalculator->getTimedOutCount(),
'Skipped: ' . $this->metricsCalculator->getSkippedCount(),
Expand Down
6 changes: 6 additions & 0 deletions src/Logger/TextFileLogger.php
Expand Up @@ -102,6 +102,12 @@ public function getLogLines(): array
'Errors',
$separateSections
);

$logs[] = $this->getResultsLine(
$this->resultsCollector->getSyntaxErrorExecutionResults(),
'Syntax Errors',
$separateSections
);
}

if (!$this->onlyCoveredMode) {
Expand Down
5 changes: 5 additions & 0 deletions src/Metrics/MetricsCalculator.php
Expand Up @@ -103,6 +103,11 @@ public function getErrorCount(): int
return $this->countByStatus[DetectionStatus::ERROR];
}

public function getSyntaxErrorCount(): int
{
return $this->countByStatus[DetectionStatus::SYNTAX_ERROR];
}

public function getSkippedCount(): int
{
return $this->countByStatus[DetectionStatus::SKIPPED];
Expand Down
8 changes: 8 additions & 0 deletions src/Metrics/ResultsCollector.php
Expand Up @@ -105,6 +105,14 @@ public function getErrorExecutionResults(): array
return $this->getResultListForStatus(DetectionStatus::ERROR)->getSortedExecutionResults();
}

/**
* @return MutantExecutionResult[]
*/
public function getSyntaxErrorExecutionResults(): array
{
return $this->getResultListForStatus(DetectionStatus::SYNTAX_ERROR)->getSortedExecutionResults();
}

/**
* @return MutantExecutionResult[]
*/
Expand Down
4 changes: 4 additions & 0 deletions src/Metrics/TargetDetectionStatusesProvider.php
Expand Up @@ -123,6 +123,8 @@ private function findRequired(): Generator

yield DetectionStatus::ERROR;

yield DetectionStatus::SYNTAX_ERROR;

yield DetectionStatus::TIMED_OUT;

if (!$this->onlyCoveredMode) {
Expand All @@ -138,6 +140,8 @@ private function findRequired(): Generator

yield DetectionStatus::SKIPPED;

yield DetectionStatus::SYNTAX_ERROR;

if ($this->logVerbosity === LogVerbosity::DEBUG) {
yield DetectionStatus::KILLED;

Expand Down
2 changes: 2 additions & 0 deletions src/Mutant/DetectionStatus.php
Expand Up @@ -49,6 +49,7 @@ final class DetectionStatus
public const ERROR = 'error';
public const TIMED_OUT = 'timed out';
public const SKIPPED = 'skipped';
public const SYNTAX_ERROR = 'syntax error';
public const NOT_COVERED = 'not covered';

public const ALL = [
Expand All @@ -57,6 +58,7 @@ final class DetectionStatus
self::ERROR,
self::TIMED_OUT,
self::SKIPPED,
self::SYNTAX_ERROR,
self::NOT_COVERED,
];
}
9 changes: 8 additions & 1 deletion src/Mutant/MutantExecutionResultFactory.php
Expand Up @@ -35,6 +35,7 @@

namespace Infection\Mutant;

use Infection\AbstractTestFramework\SyntaxErrorAware;
use Infection\AbstractTestFramework\TestFrameworkAdapter;
use Infection\Process\MutantProcess;
use function Safe\sprintf;
Expand Down Expand Up @@ -103,10 +104,16 @@ private function retrieveDetectionStatus(MutantProcess $mutantProcess): string
return DetectionStatus::ERROR;
}

if ($this->testFrameworkAdapter->testsPass($this->retrieveProcessOutput($process))) {
$output = $this->retrieveProcessOutput($process);

if ($this->testFrameworkAdapter->testsPass($output)) {
return DetectionStatus::ESCAPED;
}

if ($this->testFrameworkAdapter instanceof SyntaxErrorAware && $this->testFrameworkAdapter->isSyntaxError($output)) {
return DetectionStatus::SYNTAX_ERROR;
}

return DetectionStatus::KILLED;
}
}
3 changes: 3 additions & 0 deletions src/Mutator/ProfileList.php
Expand Up @@ -464,6 +464,9 @@ final class ProfileList
// Extensions
'BCMath' => Mutator\Extensions\BCMath::class,
'MBString' => Mutator\Extensions\MBString::class,

// Internal only usage
Copy link
Member

Choose a reason for hiding this comment

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

Internal usage only

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed in #1557

'SyntaxError' => Mutator\SyntaxError::class,
];

/** @var array<int, string>|null */
Expand Down
81 changes: 81 additions & 0 deletions src/Mutator/SyntaxError.php
@@ -0,0 +1,81 @@
<?php
/**
* This code is licensed under the BSD 3-Clause License.
*
* Copyright (c) 2017, Maks Rafalko
* All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions are met:
*
* * Redistributions of source code must retain the above copyright notice, this
* list of conditions and the following disclaimer.
*
* * Redistributions in binary form must reproduce the above copyright notice,
* this list of conditions and the following disclaimer in the documentation
* and/or other materials provided with the distribution.
*
* * Neither the name of the copyright holder nor the names of its
* contributors may be used to endorse or promote products derived from
* this software without specific prior written permission.
*
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
* AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
* IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
* DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE
* FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
* DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
* SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
* CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
* OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
* OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
*/

declare(strict_types=1);

namespace Infection\Mutator;

use PhpParser\Node;

/**
* @internal
*
* @implements Mutator<Node\Expr\Variable>
*/
final class SyntaxError implements Mutator
Copy link
Member

Choose a reason for hiding this comment

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

From what i understand it's for testing only right? So why registering the mutator here in source code? I guess we don't have (yet) a way to register mutators only the fly for tests only?

Copy link
Member Author

Choose a reason for hiding this comment

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

it's for testing only, yes

as far as I know, we can't register mutators on the fly atm. If we remove SyntaxError from ProfileList.php, tests fail with "Uknown Mutator" error raised here

$knownMutatorClassNames = array_flip(ProfileList::ALL_MUTATORS);
foreach ($resolvedMutators as $mutatorClassName => $config) {
Assert::keyExists(
$knownMutatorClassNames,
$mutatorClassName,
sprintf('Unknown mutator "%s"', $mutatorClassName)

{
use GetMutatorName;

public static function getDefinition(): ?Definition
{
return new Definition(
'Replaces a `$this` with `false` to produce a syntax error. Internal usage only.',
MutatorCategory::ORTHOGONAL_REPLACEMENT,
null,
<<<'DIFF'
class X {
function foo()
{
- $this->method();
+ $->method();
}
}
DIFF
);
}

/**
* @psalm-mutation-free
*
* @return iterable<Node\Expr\ConstFetch>
*/
public function mutate(Node $node): iterable
{
yield new Node\Expr\ConstFetch(new Node\Name('$'));
}

public function canMutate(Node $node): bool
{
return $node instanceof Node\Expr\Variable && $node->name === 'this';
}
}
8 changes: 7 additions & 1 deletion src/TestFramework/PhpUnit/Adapter/PestAdapter.php
Expand Up @@ -36,6 +36,7 @@
namespace Infection\TestFramework\PhpUnit\Adapter;

use Infection\AbstractTestFramework\MemoryUsageAware;
use Infection\AbstractTestFramework\SyntaxErrorAware;
use Infection\AbstractTestFramework\TestFrameworkAdapter;
use Infection\TestFramework\ProvidesInitialRunOnlyOptions;
use function Safe\preg_match;
Expand All @@ -44,7 +45,7 @@
/**
* @internal
*/
final class PestAdapter implements MemoryUsageAware, ProvidesInitialRunOnlyOptions, TestFrameworkAdapter
final class PestAdapter implements MemoryUsageAware, ProvidesInitialRunOnlyOptions, SyntaxErrorAware, TestFrameworkAdapter
{
private const NAME = 'Pest';

Expand Down Expand Up @@ -76,6 +77,11 @@ public function testsPass(string $output): bool
return $isOk || $isOkRisked;
}

public function isSyntaxError(string $output): bool
{
return preg_match('/ParseError\s*syntax error/i', $output) === 1;
}

public function hasJUnitReport(): bool
{
return $this->phpUnitAdapter->hasJUnitReport();
Expand Down