Fix StringMatchesFormatDescription with \r\n

[mkasberg]

This is a proposed fix for #3040.

There was a bug in that the following assertion would fail, even though
it should pass:

$this->assertStringMatchesFormat("\r\n", "\r\n");

Obviously, we would expect a string to match itself. The bug was due to
newline conversion - \r\n was being converted to \n on the pattern,
but not on the string to be matched against.

In order to be compatible with existing behavior, we need to continue
converting \r\n to \n in the pattern. (Otherwise, users who expect
this behavior may have tests fail that would have previously passed.)
Therefore, the only option for fixing the bug is to also sanitize the
string to be matched. This commit makes that change, and provides some
additional tests to verify the behavior.

[codecov-io]

Codecov Report

Merging #3059 into 7.0 will increase coverage by 0.01%.
The diff coverage is 100%.

@@             Coverage Diff              @@
##                7.0    #3059      +/-   ##
============================================
+ Coverage     80.43%   80.45%   +0.01%     
- Complexity     2828     2830       +2     
============================================
  Files           107      107              
  Lines          7438     7443       +5     
============================================
+ Hits           5983     5988       +5     
  Misses         1455     1455

Impacted Files	Coverage Δ	Complexity Δ
...work/Constraint/StringMatchesFormatDescription.php	100% <100%> (ø)	10 <2> (+2)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9f0b379...b1e0d87. Read the comment docs.

[jjeising]

In order to be compatible with existing behavior, we need to continue
converting \r\n to \n in the pattern. (Otherwise, users who expect
this behavior may have tests fail that would have previously passed.)

As far as I see the changes, this would lead to:

$this->assertStringMatchesFormat("\r\n", "\r\n"); // true

$this->assertStringMatchesFormat("\r", "\r\n"); // false
$this->assertStringMatchesFormat("\r\n", "\r"); // false

$this->assertStringMatchesFormat("\n", "\r\n"); // true
$this->assertStringMatchesFormat("\r\n", "\n"); // true

Right? Not ideal, but better than before…

[mkasberg]

That is correct.

Perhaps it is worth some discussion whether this behavior should be changed or not:

$this->assertStringMatchesFormat("\r", "\r\n"); // false
$this->assertStringMatchesFormat("\r\n", "\r"); // false

[sebastianbergmann]

How does the "official" PHPT test runner handle this? Because that is where this comes from.

[mkasberg]

Huh, I didn't realize this was based on PHPT. I will see if I can do some experiments and report back in a few days.

PHPT docs: https://qa.php.net/phpt_details.php#expectf_section

[jjeising]

run-tests.php replaces newlines like the proposed patch:

https://github.com/php/php-src/blob/14de058086d76ac344fde67fc343023fc00279a9/run-tests.php#L2039

[sebastianbergmann]

Merged manually, thanks.