Skip to content

Conversation

maks-rafalko
Copy link
Contributor

@maks-rafalko maks-rafalko commented Oct 14, 2025

Hello,

at Infection, we do a lot of nodes replacements during the Mutation Testing process.

For example (see tests in this PR), we replace + with -.

So for the following code:

<?php
echo
    1
        +
            2;

I would expect the following result:

<?php
echo
    1
        -
            2;

However, as PHPUnit test failure says, we get:

Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
 '<?php
 echo
-    1
-        -
-            2;'
+    1 - 2;'

So basically formatting disappears and we get

<?php

echo 
    1 - 2;

Notes: the same code saves formatting when we replace number with another one, not the + expression. It is tested here:

abc1
-----
<?php
echo
1
+
2
+
3;
-----
$stmts[0]->exprs[0]->left->right->value = 42;
-----
<?php
echo
1
+
42
+
3;

  1. Is it a bug or expected behavior?
  2. Is it possible to fix in theory?

I'm working on a new update for Infection to produce format-preserved diffs for developers, and among 5000+ mutations found more interesting cases where format isn't preserved, but let's keep it simple with this one for now. If this one is a bug, I can create separate issues for other cases.

@maks-rafalko maks-rafalko changed the title Format preserving with replacing expression nodes Format preserving with replacing expression nodes doesn't work Oct 14, 2025
@maks-rafalko maks-rafalko changed the title Format preserving with replacing expression nodes doesn't work Formatpreserving pretty-printing with replacing expression nodes doesn't work Oct 14, 2025
@maks-rafalko maks-rafalko changed the title Formatpreserving pretty-printing with replacing expression nodes doesn't work Format-preserving pretty-printing with replacing expression nodes doesn't work Oct 14, 2025
maks-rafalko added a commit to infection/infection that referenced this pull request Oct 21, 2025
Format-preserving pretty printing is hard :| After a week of evenings of
debugging, here we go.


https://github.com/nikic/PHP-Parser/blob/master/doc/component/Pretty_printing.markdown#formatting-preserving-pretty-printing

Why is this needed?

- To correctly highlight mutated code in PhpStorm Plugin (requested
here:
#2425 (comment))
- To have original code in code diffs for each Mutant. Previously, the
code was reformatted by PrettyPrinter from `php-parser`

Instead of 1000 words, examples:

Before:

```diff
@@ @@
     }
     public function logVerbosityDeprecationNotice(string $valueToUse): void
     {
-        $this->logger->notice('Numeric versions of log-verbosity have been deprecated, please use, ' . $valueToUse . ' to keep the same result', ['block' => true]);
+        $this->logger->notice('Numeric versions of log-verbosity have been deprecated, please use, ' . $valueToUse . ' to keep the same result', []);
     }
     public function logUnknownVerbosityOption(string $default): void
     {
```

After:

```diff
@@ @@
     {
         $this->logger->notice(
             'Numeric versions of log-verbosity have been deprecated, please use, ' . $valueToUse . ' to keep the same result',
-            ['block' => true],
+            [],
         );
     }
```

As you can see, previously Infection **modified** the source code
formatting, then created a Mutant, and then compared modified original
code with mutant.

Now, original code is compared with a mutant as is - we ONLY change the
mutated node. No more reformatting.

This gives us much more readable and clear diffs. Long multiline arrays
are no longer changed to lengthy one-liners.

----

During implementation, I found several bugs in `php-parser`:

- nikic/PHP-Parser#1115
- nikic/PHP-Parser#1116
- nikic/PHP-Parser#1117
- nikic/PHP-Parser#1119

These are not blockers, but interesting to see the answers from Nikita.
I have a feeling that either these are bugs or we don't use nodes
replacements correctly (probably we should do them in-place) - let's
see.

----

- [x] check how HTML report works after these changes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant