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.

Sign up for GitHub

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

Jump to bottom

Adding Line Numbers To Mutator Ignore List #663

Merged
merged 3 commits into from Mar 16, 2019
Merged

Adding Line Numbers To Mutator Ignore List #663

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
91ff17f
Adding Line Numbers To Mutator Ignore List
Fenikkusu Mar 10, 2019
c6f6bdf
Pretty CI Fix
Fenikkusu Mar 10, 2019
d406e80
Merge branch 'master' into master-ignore-line
sanmai Mar 11, 2019
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
7 changes: 7 additions & 0 deletions infection.json.dist
View file
Open in desktop
Expand Up @@ -10,5 +10,12 @@
"badge": {
"branch": "master"
}
},
"mutators": {
"MethodCallRemoval": {
"ignore": [
"Infection\\Finder\\SourceFilesFinder::__construct::63"
Copy link
Member

Choose a reason for hiding this comment

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

This is for parent's __construct. It does some things, but they're quite hard to test, and that is kind of out of scope of our project.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's the reason I put it in there. Granted, I do have to disagree to some point. It is quite easy to test. I had already commented the test code in #660. The problem is that you have to test using reflection, something most people tend to frown upon.

Copy link
Member

@sanmai sanmai Mar 11, 2019 •
edited

Choose a reason for hiding this comment

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

Yeah, true. What do you think about adding this rationale to the PR description? It is not immediately obvious, that why I had to comment.

(It's kind of award more than just hard to test this without reflection.)

]
}
}
}
6 changes: 5 additions & 1 deletion src/Mutator/Util/Mutator.php
View file
Open in desktop
Expand Up @@ -67,7 +67,11 @@ final public function shouldMutate(Node $node): bool
return true;
}

return !$this->config->isIgnored($reflectionClass->getName(), $node->getAttribute(ReflectionVisitor::FUNCTION_NAME, ''));
return !$this->config->isIgnored(
$reflectionClass->getName(),
$node->getAttribute(ReflectionVisitor::FUNCTION_NAME, ''),
$node->getLine()
);
}

final public static function getName(): string
Expand Down
7 changes: 5 additions & 2 deletions src/Mutator/Util/MutatorConfig.php
View file
Open in desktop
Expand Up @@ -56,7 +56,7 @@ public function __construct(array $config)
$this->mutatorSettings = $config['settings'] ?? [];
}

public function isIgnored(string $class, string $method): bool
public function isIgnored(string $class, string $method, int $lineNumber = null): bool
{
if (\in_array($class, $this->ignoreConfig)) {
return true;
Expand All @@ -67,7 +67,10 @@ public function isIgnored(string $class, string $method): bool
}

foreach ($this->ignoreConfig as $ignorePattern) {
if (fnmatch($ignorePattern, $class, FNM_NOESCAPE) || fnmatch($ignorePattern, $class . '::' . $method, FNM_NOESCAPE)) {
if (fnmatch($ignorePattern, $class, FNM_NOESCAPE)
|| fnmatch($ignorePattern, $class . '::' . $method, FNM_NOESCAPE)
|| ($lineNumber !== null && fnmatch($ignorePattern, $class . '::' . $method . '::' . $lineNumber, FNM_NOESCAPE))
) {
return true;
}
}
Expand Down
11 changes: 9 additions & 2 deletions tests/Mutator/Util/MutatorConfigTest.php
View file
Open in desktop
Expand Up @@ -46,11 +46,11 @@ final class MutatorConfigTest extends TestCase
/**
* @dataProvider providesIgnoredValues
*/
public function test_is_ignored_returns_true_if_there_is_a_match(array $ignored, string $class, string $method): void
public function test_is_ignored_returns_true_if_there_is_a_match(array $ignored, string $class, string $method, int $lineNumber = null): void
{
$config = new MutatorConfig(['ignore' => $ignored]);

$this->assertTrue($config->isIgnored($class, $method));
$this->assertTrue($config->isIgnored($class, $method, $lineNumber));
}

public function providesIgnoredValues(): \Generator
Expand Down Expand Up @@ -90,6 +90,13 @@ public function providesIgnoredValues(): \Generator
'Foo\Bar\Test',
'method',
];

yield 'It ignores a specific line number' => [
['Foo\Bar\Test::method::63'],
'Foo\Bar\Test',
'method',
63,
];
}

/**
Expand Down