Skip to content

Commit

Permalink
Add an ability to ignore Mutators for source code line(s) by regular …
Browse files Browse the repository at this point in the history
…expression (#1326)

* Add an ability to ignore Mutators for source code line(s) by regular expression.

Fixes #697

Allows to avoid mutating such lines of code as:

```diff
- Assert::notNull($variable);
```

or

```diff
- $this->logger->error($message, ['user' => $user]);
+ $this->logger->error($message, []);
```

and so on.

Example of new config setting usage:

```json
{
    "mutators": {
        "MethodCallRemoval": {
            "ignoreSourceCodeByRegex": [
                "Assert::",
                "\\$this->logger"
            ]
        }
    }
}
```

* Escape delimiters before building regular expression with user's input

* Rebase on master, use `MutatorFactory::getMutatorNameForClassName()`

* Make public method private, compare with empty array
  • Loading branch information
maks-rafalko committed Sep 16, 2020
1 parent d6bd395 commit ee40e43
Show file tree
Hide file tree
Showing 21 changed files with 736 additions and 42 deletions.
12 changes: 12 additions & 0 deletions resources/schema.json
Expand Up @@ -122,6 +122,12 @@
"type": "string"
}
},
"global-ignoreSourceCodeByRegex": {
"type": "array",
"items": {
"type": "string"
}
},

"@arithmetic": { "$ref": "#/definitions/default-mutator-config" },
"@boolean": { "$ref": "#/definitions/default-mutator-config" },
Expand Down Expand Up @@ -506,6 +512,12 @@
"items": {
"type": "string"
}
},
"ignoreSourceCodeByRegex": {
"type": "array",
"items": {
"type": "string"
}
}
}
}
Expand Down
14 changes: 13 additions & 1 deletion src/Configuration/Configuration.php
Expand Up @@ -81,12 +81,14 @@ class Configuration
private $msiPrecision;
private $threadCount;
private $dryRun;
private $ignoreSourceCodeMutatorsMap;

/**
* @param string[] $sourceDirectories
* @param string[] $sourceFilesExcludes
* @param iterable<SplFileInfo> $sourceFiles
* @param array<string, Mutator> $mutators
* @param array<string, array<int, string>> $ignoreSourceCodeMutatorsMap
*/
public function __construct(
float $timeout,
Expand Down Expand Up @@ -115,7 +117,8 @@ public function __construct(
?float $minCoveredMsi,
int $msiPrecision,
int $threadCount,
bool $dryRun
bool $dryRun,
array $ignoreSourceCodeMutatorsMap
) {
Assert::nullOrGreaterThanEq($timeout, 0);
Assert::allString($sourceDirectories);
Expand Down Expand Up @@ -152,6 +155,7 @@ public function __construct(
$this->msiPrecision = $msiPrecision;
$this->threadCount = $threadCount;
$this->dryRun = $dryRun;
$this->ignoreSourceCodeMutatorsMap = $ignoreSourceCodeMutatorsMap;
}

public function getProcessTimeout(): float
Expand Down Expand Up @@ -300,4 +304,12 @@ public function isDryRun(): bool
{
return $this->dryRun;
}

/**
* @return array<string, array<int, string>>
*/
public function getIgnoreSourceCodeMutatorsMap(): array
{
return $this->ignoreSourceCodeMutatorsMap;
}
}
82 changes: 55 additions & 27 deletions src/Configuration/ConfigurationFactory.php
Expand Up @@ -36,12 +36,13 @@
namespace Infection\Configuration;

use function array_fill_keys;
use function count;
use function array_key_exists;
use function dirname;
use Infection\Configuration\Entry\PhpUnit;
use Infection\Configuration\Schema\SchemaConfiguration;
use Infection\FileSystem\SourceFileCollector;
use Infection\FileSystem\TmpDirProvider;
use Infection\Mutator\ConfigurableMutator;
use Infection\Mutator\Mutator;
use Infection\Mutator\MutatorFactory;
use Infection\Mutator\MutatorParser;
Expand All @@ -50,6 +51,7 @@
use OndraM\CiDetector\CiDetector;
use function Safe\sprintf;
use function sys_get_temp_dir;
use Webmozart\Assert\Assert;
use Webmozart\PathUtil\Path;

/**
Expand Down Expand Up @@ -121,6 +123,11 @@ public function create(
$namespacedTmpDir
);

$resolvedMutatorsArray = $this->resolveMutators($schema->getMutators(), $mutatorsInput);

This comment has been minimized.

Copy link
@theofidry

theofidry Sep 17, 2020

Member

$resolvedMutators? It's a bit weird to specify the type in the name


$mutators = $this->mutatorFactory->create($resolvedMutatorsArray);
$ignoreSourceCodeMutatorsMap = $this->retrieveIgnoreSourceCodeMutatorsMap($resolvedMutatorsArray);

return new Configuration(
$schema->getTimeout() ?? self::DEFAULT_TIMEOUT,
$schema->getSource()->getDirectories(),
Expand All @@ -134,7 +141,7 @@ public function create(
$logVerbosity,
$namespacedTmpDir,
$this->retrievePhpUnit($schema, $configDir),
$this->retrieveMutators($schema->getMutators(), $mutatorsInput),
$mutators,
$testFramework,
$schema->getBootstrap(),
$initialTestsPhpOptions ?? $schema->getInitialTestsPhpOptions(),
Expand All @@ -151,10 +158,33 @@ public function create(
self::retrieveMinCoveredMsi($minCoveredMsi, $schema),
$msiPrecision,
$threadCount,
$dryRun
$dryRun,
$ignoreSourceCodeMutatorsMap
);
}

/**
* @param array<string, mixed> $schemaMutators
*
* @return array<class-string<Mutator&ConfigurableMutator>, mixed[]>
*/
private function resolveMutators(array $schemaMutators, string $mutatorsInput): array
{
if ($schemaMutators === []) {
$schemaMutators = ['@default' => true];
}

$parsedMutatorsInput = $this->mutatorParser->parse($mutatorsInput);

if ($parsedMutatorsInput === []) {
$mutatorsList = $schemaMutators;
} else {
$mutatorsList = array_fill_keys($parsedMutatorsInput, true);
}

return $this->mutatorResolver->resolve($mutatorsList);
}

private function retrieveTmpDir(
SchemaConfiguration $schema,
string $configDir
Expand Down Expand Up @@ -187,30 +217,6 @@ private function retrievePhpUnit(SchemaConfiguration $schema, string $configDir)
return $phpUnit;
}

/**
* @param array<string, mixed> $schemaMutators
*
* @return array<string, Mutator>
*/
private function retrieveMutators(array $schemaMutators, string $mutatorsInput): array
{
if (count($schemaMutators) === 0) {
$schemaMutators = ['@default' => true];
}

$parsedMutatorsInput = $this->mutatorParser->parse($mutatorsInput);

if ($parsedMutatorsInput === []) {
$mutatorsList = $schemaMutators;
} else {
$mutatorsList = array_fill_keys($parsedMutatorsInput, true);
}

return $this->mutatorFactory->create(
$this->mutatorResolver->resolve($mutatorsList)
);
}

private static function retrieveCoverageBasePath(
?string $existingCoveragePath,
string $configDir,
Expand Down Expand Up @@ -255,4 +261,26 @@ private static function retrieveMinCoveredMsi(?float $minCoveredMsi, SchemaConfi
{
return $minCoveredMsi ?? $schema->getMinCoveredMsi();
}

/**
* @param array<string, mixed[]> $resolvedMutatorsMap
*
* @return array<string, array<int, string>>
*/
private function retrieveIgnoreSourceCodeMutatorsMap(array $resolvedMutatorsMap): array

This comment has been minimized.

Copy link
@theofidry

theofidry Sep 17, 2020

Member

retrieveIgnoreSourceCodeByMutatorNameMap? (could be propagated); I don't feel strongly about that one, but it might remove the impression that you may deal with actual Mutator instances here which is not the case

{
$map = [];

foreach ($resolvedMutatorsMap as $mutatorClassName => $config) {
if (array_key_exists('ignoreSourceCodeByRegex', $config)) {

This comment has been minimized.

Copy link
@theofidry

theofidry Sep 17, 2020

Member

I would suggest inverting the condition to avoid a level of nesting:

if (!array_key_exists('ignoreSourceCodeByRegex', $config)) {
    continue;
}

// ...
$mutatorName = MutatorFactory::getMutatorNameForClassName($mutatorClassName);

This comment has been minimized.

Copy link
@theofidry

theofidry Sep 17, 2020

Member

Maybe an alternative, since you have mutators created before, is to create the list of mutators mapped by their names (IIRC the ignore mutators use the decorated mutator name right?) hence you can remove that dependency on the MutatorFactory and instead use directly the mutator instance.

IMO there is 3 good approaches:

  • Have a 1-1 relation between mutators and mutator name – this has been made non-viable with the introduction of IgnoreMutator
  • Rely on the factory to provide the names for a given mutator, but then Mutator::getName() makes little sense as the source of truth should be the factory, not the mutator instance
  • Rely on the mutator instance (precisely because it can manage to provide the relevant name despite or usage of mutator decorators such as the IgnoreMutator)

And it feels right now we are a bit borderline between two approaches.

That being said, don't get me wrong it's still at the end of the day not a big deal at this point


Assert::isArray($config['ignoreSourceCodeByRegex']);

$map[$mutatorName] = $config['ignoreSourceCodeByRegex'];
}
}

return $map;
}
}
19 changes: 16 additions & 3 deletions src/Container.php
Expand Up @@ -55,6 +55,7 @@
use Infection\Console\OutputFormatter\OutputFormatter;
use Infection\Differ\DiffColorizer;
use Infection\Differ\Differ;
use Infection\Differ\DiffSourceCodeMatcher;
use Infection\Event\EventDispatcher\EventDispatcher;
use Infection\Event\EventDispatcher\SyncEventDispatcher;
use Infection\Event\Subscriber\ChainSubscriberFactory;
Expand Down Expand Up @@ -557,16 +558,20 @@ public static function create(): self
);
},
MutationTestingRunner::class => static function (self $container): MutationTestingRunner {
$configuration = $container->getConfiguration();

return new MutationTestingRunner(
$container->getMutantProcessFactory(),
$container->getMutantFactory(),
$container->getProcessRunner(),
$container->getEventDispatcher(),
$container->getConfiguration()->isDryRun()
$configuration->isDryRun()
? new DummyFileSystem()
: $container->getFileSystem(),
$container->getConfiguration()->noProgress(),
$container->getConfiguration()->getProcessTimeout()
$container->getDiffSourceCodeMatcher(),
$configuration->noProgress(),
$configuration->getProcessTimeout(),
$configuration->getIgnoreSourceCodeMutatorsMap()
);
},
LineRangeCalculator::class => static function (): LineRangeCalculator {
Expand All @@ -590,6 +595,9 @@ public static function create(): self
FormatterFactory::class => static function (self $container): FormatterFactory {
return new FormatterFactory($container->getOutput());
},
DiffSourceCodeMatcher::class => static function (): DiffSourceCodeMatcher {
return new DiffSourceCodeMatcher();
},
]);

return $container->withValues(
Expand Down Expand Up @@ -1134,6 +1142,11 @@ public function getOutputFormatter(): OutputFormatter
return $this->get(OutputFormatter::class);
}

public function getDiffSourceCodeMatcher(): DiffSourceCodeMatcher
{
return $this->get(DiffSourceCodeMatcher::class);
}

/**
* @param class-string<object> $id
* @param Closure(self): object $value
Expand Down
51 changes: 51 additions & 0 deletions src/Differ/DiffSourceCodeMatcher.php
@@ -0,0 +1,51 @@
<?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\Differ;

use function Safe\preg_match;

/**
* @internal
*/
final class DiffSourceCodeMatcher
{
public function matches(string $diff, string $sourceCodeRegex): bool
{
$regexWithEscapedDelimiters = str_replace('/', '\/', $sourceCodeRegex);

return preg_match("/^-\s*{$regexWithEscapedDelimiters}$/mu", $diff) === 1;
}
}
2 changes: 1 addition & 1 deletion src/Mutator/IgnoreConfig.php
Expand Up @@ -47,7 +47,7 @@
class IgnoreConfig
{
private $patterns;
/** @var int[] */
/** @var array<string, int> */
private $hashtable = [];

/**
Expand Down
11 changes: 11 additions & 0 deletions src/Mutator/MutatorResolver.php
Expand Up @@ -49,6 +49,7 @@
final class MutatorResolver
{
private const GLOBAL_IGNORE_SETTING = 'global-ignore';
private const GLOBAL_IGNORE_SOURCE_CODE_BY_REGEX_SETTING = 'global-ignoreSourceCodeByRegex';

/**
* Resolves the given hashmap of enabled, disabled or configured mutators
Expand All @@ -75,6 +76,16 @@ public function resolve(array $mutatorSettings): array

break;
}

if ($mutatorOrProfileOrGlobalSettingKey === self::GLOBAL_IGNORE_SOURCE_CODE_BY_REGEX_SETTING) {
/** @var string[] $globalSetting */
$globalSetting = $setting;

$globalSettings = ['ignoreSourceCodeByRegex' => $globalSetting];
unset($mutatorSettings[self::GLOBAL_IGNORE_SOURCE_CODE_BY_REGEX_SETTING]);

break;
}
}

foreach ($mutatorSettings as $mutatorOrProfile => $setting) {
Expand Down

0 comments on commit ee40e43

Please sign in to comment.