Add an ability to ignore Mutators for source code line(s) by regular expression by maks-rafalko · Pull Request #1326 · infection/infection

maks-rafalko · 2020-09-07T14:44:30Z

Fixes Allow adding mutation exclusions for specific calls #697
Covered by tests
Doc PR: Document how to ignore mutations matching the source code by regular expression site#177

Allows to avoid mutating such lines of code as:

- Assert::notNull($variable);

or

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

and similar if developers are not interested in them.

Example of new config setting usage:

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

Also, there is a new setting for mutators - global-ignoreSourceCodeByRegex that will be added to all enabled mutators if present. Works similar to global-ignore

{
    "mutators": {
        "global-ignoreSourceCodeByRegex": [
            "Assert::.*"
        ]
    }
}

From an implementation point of view, it's not possible to use IgnoreConfig and IgnoreMutator to ignore mutators for configured lines of code by regex, because, at the step of applying Mutator, we don't have any information about the source line of code.

While we technically can get it (using PrettyPrinter's prettyPrintExpr($expression) function or by passing source code to each mutator and finding the needed line), for me it seems like not the right solution.

This PR tries to filter out mutants before creating Mutant processes, similar to what is being discussed in pre-/post- hooks here #1323

So, before deciding whether we need to ignore mutator or no, we are applying regular expression (if any) to the diff of Mutant. Easy, yet so powerful.

I will continue adding tests/docs after we agree with the implementation.

I see this feature is most requested during the last months, so I would like to release it asap.

Regarding the suggested plugin system - #1323. When we will do it, this implementation can be redeveloped using plugin system/hooks, while for users configuration will be the same, so no BC here.

PS I'm aware of the failed build, it's ok for now since no tests have been updated

Slamdunk · 2020-09-08T07:44:32Z

I'm very happy with the results. I miss only one functionality: a global-ignoreSourceCodeByRegex, like the already existing global-ignore, as many ignores may be specific to the environment of the mutation, instead of the specific mutation, like the assert one.

maks-rafalko · 2020-09-12T09:46:45Z

@Slamdunk @sanmai great suggestions, thank you very much. Implemented.

Updated description. Also, added global-ignoreSourceCodeByRegex as you requested.

Ready for review.

sanmai · 2020-09-15T01:11:34Z

+        },
+        "MethodCallRemoval": {
+            "ignoreSourceCodeByRegex": [
+                "Assert::.*",


Shouldn't we dog-food this feature already?

As a non-native English speaker, may I ask you what does this mean?

Sure. I'm slightly surprised to find that there's even a Wikipedia article for the term.

Eating your own dog food or dogfooding is the practice of an organization using its own product.

What I meant is that Infection could use exactly this regex to limit removal of these calls.

Shouldn't we dog-food this feature already?

if we need to - we certainly can. I didn't do any analysis for this though

It can certainly wait.

…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" ] } } } ```

sanmai · 2020-09-16T02:30:57Z

If we can extend #1327 to allow injecting arbitrary configuration for a plugin, this feature is a great candidate for a standard plugin. Nevertheless I think this idea shouldn't be holding anything up since it should be possible to refactor this ignorer into a plugin without changing the configuration schema in a bit.

sanmai · 2020-09-16T05:23:12Z

CS Fixer is going nuts 😞

maks-rafalko · 2020-09-16T05:25:51Z

CS Fixer is going nuts 😞

yeah, it's because it's downloading v2.15.8 instead of v2.16.4 as the latest PHAR for https://cs.sensiolabs.org/download/php-cs-fixer-v2.phar URL

2.16.4 works as expected locally. I will try to resolve it as a separate issue somewhere later today

maks-rafalko · 2020-09-16T05:28:02Z

If we can extend #1327 to allow injecting arbitrary configuration for a plugin, this feature is a great candidate for a standard plugin.

Agree. That was my plan - to release this one ASAP, and then, when we are ready with the plugin system (which can take some time) - we will refactor internal implementation.

maks-rafalko · 2020-09-16T05:33:41Z

Regarding php-cs-fixer: there is an open issue, let's see how they will resolve it: PHP-CS-Fixer/PHP-CS-Fixer#5099

I think if they wont' fix it today, we can think about downloading particular hardcoded version for the time being

sanmai · 2020-09-16T05:53:17Z

I did composer require --dev friendsofphp/php-cs-fixer for local CS works.

sanmai · 2020-09-16T06:30:09Z

Shall we merge this?

maks-rafalko · 2020-09-16T07:17:02Z

Thank you @sanmai & @Slamdunk for great review

maks-rafalko mentioned this pull request Sep 7, 2020

Allow adding mutation exclusions for specific calls #697

Closed

sanmai reviewed Sep 7, 2020

View reviewed changes

Comment thread src/Process/Runner/MutationTestingRunner.php Outdated

sanmai reviewed Sep 7, 2020

View reviewed changes

Comment thread src/Process/Runner/MutationTestingRunner.php Outdated

This was referenced Sep 8, 2020

Add mutation testing paratestphp/paratest#510

Merged

Skip mutation testing on assert() paratestphp/paratest#531

Closed

maks-rafalko force-pushed the feature/ignore-code-by-regex branch 2 times, most recently from eba4b3e to 0ab9637 Compare September 12, 2020 09:45

maks-rafalko marked this pull request as ready for review September 12, 2020 09:46

maks-rafalko added this to the 0.18.0 milestone Sep 12, 2020

maks-rafalko added the Feature label Sep 12, 2020

maks-rafalko linked an issue Sep 12, 2020 that may be closed by this pull request

Allow adding mutation exclusions for specific calls #697

Closed

maks-rafalko force-pushed the feature/ignore-code-by-regex branch from 0ab9637 to 0d5e908 Compare September 12, 2020 10:11

sanmai reviewed Sep 12, 2020

View reviewed changes

Comment thread src/Differ/DiffSourceCodeMatcher.php Outdated

sanmai self-requested a review September 13, 2020 08:33

sanmai reviewed Sep 14, 2020

View reviewed changes

Comment thread resources/schema.json

sanmai reviewed Sep 14, 2020

View reviewed changes

Comment thread src/Configuration/ConfigurationFactory.php Outdated

maks-rafalko force-pushed the feature/ignore-code-by-regex branch from cb789a0 to bddf72f Compare September 14, 2020 15:47

sanmai approved these changes Sep 15, 2020

View reviewed changes

maks-rafalko added 3 commits September 15, 2020 19:19

Escape delimiters before building regular expression with user's input

9b38e6f

Rebase on master, use MutatorFactory::getMutatorNameForClassName()

ecc0b61

maks-rafalko force-pushed the feature/ignore-code-by-regex branch from e4d7088 to ecc0b61 Compare September 15, 2020 20:39

sanmai approved these changes Sep 16, 2020

View reviewed changes

Comment thread src/Configuration/ConfigurationFactory.php Outdated

Comment thread src/Configuration/ConfigurationFactory.php Outdated

maks-rafalko mentioned this pull request Sep 16, 2020

Document how to ignore mutations matching the source code by regular expression infection/site#177

Merged

Make public method private, compare with empty array

82d5f3d

maks-rafalko merged commit ee40e43 into master Sep 16, 2020

maks-rafalko deleted the feature/ignore-code-by-regex branch September 16, 2020 07:16

sanmai mentioned this pull request Sep 21, 2020

Add pre-filters configuration option #1327

Closed

5 tasks

Uh oh!

Conversation

maks-rafalko commented Sep 7, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Slamdunk commented Sep 8, 2020

Uh oh!

maks-rafalko commented Sep 12, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sanmai Sep 15, 2020

Choose a reason for hiding this comment

Uh oh!

Slamdunk Sep 15, 2020

Choose a reason for hiding this comment

Uh oh!

sanmai Sep 15, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maks-rafalko Sep 16, 2020

Choose a reason for hiding this comment

Uh oh!

sanmai Sep 16, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

sanmai commented Sep 16, 2020

Uh oh!

sanmai commented Sep 16, 2020

Uh oh!

maks-rafalko commented Sep 16, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

maks-rafalko commented Sep 16, 2020

Uh oh!

maks-rafalko commented Sep 16, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sanmai commented Sep 16, 2020

Uh oh!

sanmai commented Sep 16, 2020

Uh oh!

maks-rafalko commented Sep 16, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

maks-rafalko commented Sep 7, 2020 •

edited

Loading

sanmai Sep 15, 2020 •

edited

Loading

maks-rafalko commented Sep 16, 2020 •

edited

Loading

maks-rafalko commented Sep 16, 2020 •

edited

Loading