From e222caf8e69e07988eb4c58f0e1bdb3fb9fd7863 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Sun, 20 Feb 2022 17:33:15 +0100 Subject: [PATCH 1/5] SyntaxErrorGetNormalizeMessageTest: refactor to dataprovider Refactor the tests in the `SyntaxErrorGetNormalizeMessageTest` class to use data providers, which allow for adding more tests in a straight forward manner. Note: this is a 1-on-1 refactor of the test with all existing test cases still being tested but no other changes. --- .../SyntaxErrorGetNormalizeMessageTest.php | 70 +++++++++++++++++-- 1 file changed, 63 insertions(+), 7 deletions(-) diff --git a/tests/Unit/Errors/SyntaxErrorGetNormalizeMessageTest.php b/tests/Unit/Errors/SyntaxErrorGetNormalizeMessageTest.php index 6e3a88e..0be387b 100644 --- a/tests/Unit/Errors/SyntaxErrorGetNormalizeMessageTest.php +++ b/tests/Unit/Errors/SyntaxErrorGetNormalizeMessageTest.php @@ -7,17 +7,73 @@ class SyntaxErrorGetNormalizeMessageTest extends UnitTestCase { - public function testInWordInErrorMessage() + const FILEPATH_MSG_TEMPLATE = "Parse error: unexpected 'Foo' (T_STRING) in %s on line 2"; + const FILEPATH_MSG_EXPECTED = "Unexpected 'Foo' (T_STRING)"; + + /** + * Test retrieving a normalized error message. + * + * @dataProvider dataMessageNormalization + * + * @param string $message The message input to run the test with. + * @param string $expected The expected method return value. + * + * @return void + */ + public function testMessageNormalization($message, $expected) { - $message = 'Fatal error: \'break\' not in the \'loop\' or \'switch\' context in test.php on line 2'; $error = new SyntaxError('test.php', $message); - $this->assertSame('\'break\' not in the \'loop\' or \'switch\' context', $error->getNormalizedMessage()); + $this->assertSame($expected, $error->getNormalizedMessage()); + } + + /** + * Data provider. + * + * @return array + */ + public function dataMessageNormalization() + { + return array( + 'Strip leading and trailing information' => array( + 'message' => "Fatal error: 'break' not in the 'loop' or 'switch' context in test.php on line 2", + 'expected' => "'break' not in the 'loop' or 'switch' context", + ), + ); + } + + /** + * Test retrieving a normalized error message with variations for the file path. + * + * @dataProvider dataFilePathHandling + * + * @param string $filePath The file path input to run the test with. + * @param string $fileName The file name which is expected to be in the error message. + * + * @return void + */ + public function testFilePathHandling($filePath, $fileName) + { + $message = sprintf(self::FILEPATH_MSG_TEMPLATE, $fileName); + $error = new SyntaxError($filePath, $message); + $this->assertSame(self::FILEPATH_MSG_EXPECTED, $error->getNormalizedMessage()); } - public function testInWordInErrorMessageAndInFileName() + /** + * Data provider. + * + * @return array + */ + public function dataFilePathHandling() { - $message = 'Fatal error: \'break\' not in the \'loop\' or \'switch\' context in test in file.php on line 2'; - $error = new SyntaxError('test in file.php', $message); - $this->assertSame('\'break\' not in the \'loop\' or \'switch\' context', $error->getNormalizedMessage()); + return array( + 'Plain file name' => array( + 'filePath' => 'test.php', + 'fileName' => 'test.php', + ), + 'File name containing spaces' => array( + 'filePath' => 'test in file.php', + 'fileName' => 'test in file.php', + ), + ); } } From 273d3795b8af3e221d3be6a80f90c522e116a334 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Sun, 20 Feb 2022 18:36:50 +0100 Subject: [PATCH 2/5] SyntaxError::getNormalizedMessage(): bug fix - file name containing regex delimiter If a file name would contain the delimiter used in the replacement regex, it would cause a PHP error like `preg_replace(): Unknown modifier 'f'`. Fixed by telling `preg_quote()` explicitly which delimiter is being used. Includes unit test. Ref: https://www.php.net/manual/en/function.preg-quote.php --- src/Errors/SyntaxError.php | 2 +- tests/Unit/Errors/SyntaxErrorGetNormalizeMessageTest.php | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/Errors/SyntaxError.php b/src/Errors/SyntaxError.php index 9a4a36e..9e39159 100644 --- a/src/Errors/SyntaxError.php +++ b/src/Errors/SyntaxError.php @@ -30,7 +30,7 @@ public function getLine() public function getNormalizedMessage($translateTokens = false) { $message = preg_replace('~^(Parse|Fatal) error: (syntax error, )?~', '', $this->message); - $message = preg_replace('~ in ' . preg_quote(basename($this->filePath)) . ' on line [0-9]+$~', '', $message); + $message = preg_replace('~ in ' . preg_quote(basename($this->filePath), '~') . ' on line [0-9]+$~', '', $message); $message = ucfirst($message); if ($translateTokens) { diff --git a/tests/Unit/Errors/SyntaxErrorGetNormalizeMessageTest.php b/tests/Unit/Errors/SyntaxErrorGetNormalizeMessageTest.php index 0be387b..0e14a64 100644 --- a/tests/Unit/Errors/SyntaxErrorGetNormalizeMessageTest.php +++ b/tests/Unit/Errors/SyntaxErrorGetNormalizeMessageTest.php @@ -74,6 +74,10 @@ public function dataFilePathHandling() 'filePath' => 'test in file.php', 'fileName' => 'test in file.php', ), + 'File name containing regex delimiter' => array( + 'filePath' => 'test~file.php', + 'fileName' => 'test~file.php', + ), ); } } From e55c9188927f99705e9a1fbcf8b01fca9635a9ed Mon Sep 17 00:00:00 2001 From: jrfnl Date: Sun, 20 Feb 2022 18:39:32 +0100 Subject: [PATCH 3/5] SyntaxError::getNormalizedMessage(): bug fix - "in file on line" doesn't always get stripped In certain cases, PHP puts the full file name in the error message. In those cases the "in filename.php on line .." trailing part of the error message did not get stripped off. Also, in some cases, when Windows slashes are used in the file path, the "in filename.php on line .." trailing part of the error message may not get stripped off. This last case will be exceedingly rare as on Windows, those file paths are handled correctly and the chances of a non-Windows user passing a path using Windows slashes will be minuscule. Even so, I've fixed both cases by making the path handling in the function more robust. * When the initial stripping of the trailing part of the error message fails, it will be retried up to two times. * The first time, if Windows slashes would be found in the file path _after_ the `basename()` function has been applied, a "manual" basename extraction is done and the stripping of the trailing part is retried. * The second time, the full file path is used in a last attempt to strip the trailing part of the error message. Includes unit tests. Fixes 94 --- src/Errors/SyntaxError.php | 20 +++++++++++++++++-- .../SyntaxErrorGetNormalizeMessageTest.php | 20 +++++++++++++++++++ 2 files changed, 38 insertions(+), 2 deletions(-) diff --git a/src/Errors/SyntaxError.php b/src/Errors/SyntaxError.php index 9e39159..ac856c0 100644 --- a/src/Errors/SyntaxError.php +++ b/src/Errors/SyntaxError.php @@ -6,6 +6,8 @@ class SyntaxError extends ParallelLintError { + const IN_ON_REGEX = '~ in %s on line [0-9]+$~'; + /** @var Blame */ private $blame; @@ -29,8 +31,22 @@ public function getLine() */ public function getNormalizedMessage($translateTokens = false) { - $message = preg_replace('~^(Parse|Fatal) error: (syntax error, )?~', '', $this->message); - $message = preg_replace('~ in ' . preg_quote(basename($this->filePath), '~') . ' on line [0-9]+$~', '', $message); + $message = preg_replace('~^(Parse|Fatal) error: (syntax error, )?~', '', $this->message); + $baseName = basename($this->filePath); + $regex = sprintf(self::IN_ON_REGEX, preg_quote($baseName, '~')); + $message = preg_replace($regex, '', $message, -1, $count); + + if ($count === 0 && strpos($baseName, '\\') !== false) { + $baseName = ltrim(strrchr($this->filePath, '\\'), '\\'); + $regex = sprintf(self::IN_ON_REGEX, preg_quote($baseName, '~')); + $message = preg_replace($regex, '', $message, -1, $count); + } + + if ($count === 0) { + $regex = sprintf(self::IN_ON_REGEX, preg_quote($this->filePath, '~')); + $message = preg_replace($regex, '', $message); + } + $message = ucfirst($message); if ($translateTokens) { diff --git a/tests/Unit/Errors/SyntaxErrorGetNormalizeMessageTest.php b/tests/Unit/Errors/SyntaxErrorGetNormalizeMessageTest.php index 0e14a64..3614db7 100644 --- a/tests/Unit/Errors/SyntaxErrorGetNormalizeMessageTest.php +++ b/tests/Unit/Errors/SyntaxErrorGetNormalizeMessageTest.php @@ -78,6 +78,26 @@ public function dataFilePathHandling() 'filePath' => 'test~file.php', 'fileName' => 'test~file.php', ), + 'Full file path, linux slashes' => array( + 'filePath' => 'path/to/subdir/file.php', + 'fileName' => 'file.php', + ), + 'File path, windows slashes' => array( + 'filePath' => 'path\to\subdir\file.php', + 'fileName' => 'file.php', + ), + 'Absolute file path, windows slashes' => array( + 'filePath' => 'C:\path\to\subdir\file.php', + 'fileName' => 'C:\path\to\subdir\file.php', + ), + 'Relative file path, windows slashes' => array( + 'filePath' => '.\subdir\file.php', + 'fileName' => '.\subdir\file.php', + ), + 'Phar file name' => array( + 'filePath' => 'phar://031.phar.php/a.php', + 'fileName' => 'phar://031.phar.php/a.php', + ), ); } } From bdd5a894d8456df84a7e5b732024a57faa6b7008 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Sun, 20 Feb 2022 18:54:04 +0100 Subject: [PATCH 4/5] SyntaxErrorGetNormalizeMessageTest: add some additional test cases Including documenting that a `Deprecated:` prefix will not be removed. --- .../SyntaxErrorGetNormalizeMessageTest.php | 26 +++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/tests/Unit/Errors/SyntaxErrorGetNormalizeMessageTest.php b/tests/Unit/Errors/SyntaxErrorGetNormalizeMessageTest.php index 3614db7..8eb47aa 100644 --- a/tests/Unit/Errors/SyntaxErrorGetNormalizeMessageTest.php +++ b/tests/Unit/Errors/SyntaxErrorGetNormalizeMessageTest.php @@ -20,7 +20,7 @@ class SyntaxErrorGetNormalizeMessageTest extends UnitTestCase * * @return void */ - public function testMessageNormalization($message, $expected) + public function testMessageNormalizationWithoutTokenTranslation($message, $expected) { $error = new SyntaxError('test.php', $message); $this->assertSame($expected, $error->getNormalizedMessage()); @@ -34,13 +34,35 @@ public function testMessageNormalization($message, $expected) public function dataMessageNormalization() { return array( - 'Strip leading and trailing information' => array( + 'Strip leading and trailing information - fatal error' => array( 'message' => "Fatal error: 'break' not in the 'loop' or 'switch' context in test.php on line 2", 'expected' => "'break' not in the 'loop' or 'switch' context", ), + 'Strip leading and trailing information - parse error' => array( + 'message' => "Parse error: unexpected 'Foo' (T_STRING) in test.php on line 2", + 'expected' => "Unexpected 'Foo' (T_STRING)", // Also verifies ucfirst() call is being made. + ), + 'Strip trailing information, not leading - deprecation' => array( + 'message' => "Deprecated: The (real) cast is deprecated, use (float) instead in test.php on line 2", + 'expected' => "Deprecated: The (real) cast is deprecated, use (float) instead", + ), ); } + /** + * Test retrieving a normalized error message with token translation. + * + * @return void + */ + public function testMessageNormalizationWithTokenTranslation() + { + $message = 'Parse error: unexpected T_FILE, expecting T_STRING in test.php on line 2'; + $expected = 'Unexpected __FILE__ (T_FILE), expecting T_STRING'; + + $error = new SyntaxError('test.php', $message); + $this->assertSame($expected, $error->getNormalizedMessage(true)); + } + /** * Test retrieving a normalized error message with variations for the file path. * From 0da93c941c408b53a876ba5e0251a0c673c18663 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Sun, 20 Feb 2022 18:55:47 +0100 Subject: [PATCH 5/5] SyntaxError::getNormalizedMessage(): minor regex tweak By using `?:` at the start of a parenthesis group, the group will not be "remembered"/saved to memory. As this regex does not use the subgroups anyway, we don't need to remember them. --- src/Errors/SyntaxError.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Errors/SyntaxError.php b/src/Errors/SyntaxError.php index ac856c0..18023e2 100644 --- a/src/Errors/SyntaxError.php +++ b/src/Errors/SyntaxError.php @@ -31,7 +31,7 @@ public function getLine() */ public function getNormalizedMessage($translateTokens = false) { - $message = preg_replace('~^(Parse|Fatal) error: (syntax error, )?~', '', $this->message); + $message = preg_replace('~^(?:Parse|Fatal) error: (?:syntax error, )?~', '', $this->message); $baseName = basename($this->filePath); $regex = sprintf(self::IN_ON_REGEX, preg_quote($baseName, '~')); $message = preg_replace($regex, '', $message, -1, $count);