From b1e0d87f6cc35b0c58683177ad5ddea4b5b9e1a7 Mon Sep 17 00:00:00 2001
From: Mike Kasberg <kasberg.mike@gmail.com>
Date: Sun, 1 Apr 2018 22:28:42 -0600
Subject: [PATCH] Fix StringMatchesFormatDescription with \r\n

This is a 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.
---
 .../StringMatchesFormatDescription.php        | 26 ++++++++--
 .../StringMatchesFormatDescriptionTest.php    | 50 ++++++++++++++++---
 2 files changed, 67 insertions(+), 9 deletions(-)

diff --git a/src/Framework/Constraint/StringMatchesFormatDescription.php b/src/Framework/Constraint/StringMatchesFormatDescription.php
index fc161956801..0726ca21b09 100644
--- a/src/Framework/Constraint/StringMatchesFormatDescription.php
+++ b/src/Framework/Constraint/StringMatchesFormatDescription.php
@@ -29,13 +29,28 @@ public function __construct($string)
     {
         parent::__construct(
             $this->createPatternFromFormat(
-                \preg_replace('/\r\n/', "\n", $string)
+                $this->convertNewlines($string)
             )
         );
 
         $this->string = $string;
     }
 
+    /**
+     * Evaluates the constraint for parameter $other. Returns true if the
+     * constraint is met, false otherwise.
+     *
+     * @param mixed $other Value or object to evaluate.
+     *
+     * @return bool
+     */
+    protected function matches($other): bool
+    {
+        return parent::matches(
+            $this->convertNewlines($other)
+        );
+    }
+
     protected function failureDescription($other): string
     {
         return 'string matches format description';
@@ -43,8 +58,8 @@ protected function failureDescription($other): string
 
     protected function additionalFailureDescription($other): string
     {
-        $from = \preg_split('(\r\n|\r|\n)', $this->string);
-        $to   = \preg_split('(\r\n|\r|\n)', $other);
+        $from = \explode("\n", $this->string);
+        $to   = \explode("\n", $this->convertNewlines($other));
 
         foreach ($from as $index => $line) {
             if (isset($to[$index]) && $line !== $to[$index]) {
@@ -100,4 +115,9 @@ private function createPatternFromFormat(string $string): string
 
         return '/^' . $string . '$/s';
     }
+
+    private function convertNewlines($text): string
+    {
+        return \preg_replace('/\r\n/', "\n", $text);
+    }
 }
diff --git a/tests/Framework/Constraint/StringMatchesFormatDescriptionTest.php b/tests/Framework/Constraint/StringMatchesFormatDescriptionTest.php
index a5c5c35df46..de9bf60069c 100644
--- a/tests/Framework/Constraint/StringMatchesFormatDescriptionTest.php
+++ b/tests/Framework/Constraint/StringMatchesFormatDescriptionTest.php
@@ -10,9 +10,11 @@
 
 namespace PHPUnit\Framework\Constraint;
 
+use PHPUnit\Framework\ExpectationFailedException;
+
 class StringMatchesFormatDescriptionTest extends ConstraintTestCase
 {
-    public function testConstraintStringMatches(): void
+    public function testConstraintStringMatchesCharacter(): void
     {
         $constraint = new StringMatchesFormatDescription('*%c*');
 
@@ -22,7 +24,7 @@ public function testConstraintStringMatches(): void
         $this->assertCount(1, $constraint);
     }
 
-    public function testConstraintStringMatches2(): void
+    public function testConstraintStringMatchesString(): void
     {
         $constraint = new StringMatchesFormatDescription('*%s*');
 
@@ -32,7 +34,7 @@ public function testConstraintStringMatches2(): void
         $this->assertCount(1, $constraint);
     }
 
-    public function testConstraintStringMatches3(): void
+    public function testConstraintStringMatchesInteger(): void
     {
         $constraint = new StringMatchesFormatDescription('*%i*');
 
@@ -42,7 +44,7 @@ public function testConstraintStringMatches3(): void
         $this->assertCount(1, $constraint);
     }
 
-    public function testConstraintStringMatches4(): void
+    public function testConstraintStringMatchesUnsignedInt(): void
     {
         $constraint = new StringMatchesFormatDescription('*%d*');
 
@@ -52,7 +54,7 @@ public function testConstraintStringMatches4(): void
         $this->assertCount(1, $constraint);
     }
 
-    public function testConstraintStringMatches5(): void
+    public function testConstraintStringMatchesHexadecimal(): void
     {
         $constraint = new StringMatchesFormatDescription('*%x*');
 
@@ -62,7 +64,7 @@ public function testConstraintStringMatches5(): void
         $this->assertCount(1, $constraint);
     }
 
-    public function testConstraintStringMatches6(): void
+    public function testConstraintStringMatchesFloat(): void
     {
         $constraint = new StringMatchesFormatDescription('*%f*');
 
@@ -71,4 +73,40 @@ public function testConstraintStringMatches6(): void
         $this->assertEquals('matches PCRE pattern "/^\*[+-]?\.?\d+\.?\d*(?:[Ee][+-]?\d+)?\*$/s"', $constraint->toString());
         $this->assertCount(1, $constraint);
     }
+
+    public function testConstraintStringMatchesNewline(): void
+    {
+        $constraint = new StringMatchesFormatDescription("\r\n");
+
+        $this->assertFalse($constraint->evaluate("*\r\n", '', true));
+        $this->assertTrue($constraint->evaluate("\r\n", '', true));
+        $this->assertEquals("matches PCRE pattern \"/^\n$/s\"", $constraint->toString());
+        $this->assertCount(1, $constraint);
+    }
+
+    public function testFailureMessageWithNewlines(): void
+    {
+        $constraint = new StringMatchesFormatDescription("%c\nfoo\n%c");
+
+        try
+        {
+            $constraint->evaluate("*\nbar\n*");
+            $this->fail('Expected ExpectationFailedException, but it was not thrown.');
+        }
+        catch (ExpectationFailedException $e)
+        {
+            $expected = <<<EOD
+Failed asserting that string matches format description.
+--- Expected
++++ Actual
+@@ @@
+ *
+-foo
++bar
+ *
+
+EOD;
+            $this->assertEquals($expected, $e->getMessage());
+        }
+    }
 }