From a71f2e3e0393ac46101aac595718bcbdb67b0dcc Mon Sep 17 00:00:00 2001 From: Jeff Ching Date: Thu, 11 Jan 2018 11:18:32 -0800 Subject: [PATCH 01/13] Add test for SourceLocationResolver --- .../Debugger/SourceLocationResolverTest.php | 66 +++++++++++++++++++ 1 file changed, 66 insertions(+) create mode 100644 tests/unit/Debugger/SourceLocationResolverTest.php diff --git a/tests/unit/Debugger/SourceLocationResolverTest.php b/tests/unit/Debugger/SourceLocationResolverTest.php new file mode 100644 index 00000000000..4de0b5d2eb9 --- /dev/null +++ b/tests/unit/Debugger/SourceLocationResolverTest.php @@ -0,0 +1,66 @@ +sourcePath(['src', 'Debugger', 'DebuggerClient.php']), 1); + $resolver = new SourceLocationResolver(); + $resolvedLocation = $resolver->resolve($location); + $this->assertInstanceOf(SourceLocation::class, $resolvedLocation); + $expectedFile = realpath($this->sourcePath([__DIR__, '..', '..', '..', 'src', 'Debugger', 'DebuggerClient.php'])); + $this->assertEquals($expectedFile, $resolvedLocation->path()); + $this->assertEquals(1, $resolvedLocation->line()); + } + + public function testExtraDirectories() + { + $location = new SourceLocation($this->sourcePath(['extra', 'src', 'Debugger', 'DebuggerClient.php']), 1); + $resolver = new SourceLocationResolver(); + $resolvedLocation = $resolver->resolve($location); + $this->assertInstanceOf(SourceLocation::class, $resolvedLocation); + $expectedFile = realpath($this->sourcePath([__DIR__, '..', '..', '..', 'src', 'Debugger', 'DebuggerClient.php'])); + $this->assertEquals($expectedFile, $resolvedLocation->path()); + $this->assertEquals(1, $resolvedLocation->line()); + } + + public function testMissingDirectories() + { + $location = new SourceLocation($this->sourcePath(['Debugger', 'DebuggerClient.php']), 1); + $resolver = new SourceLocationResolver(); + $resolvedLocation = $resolver->resolve($location); + $this->assertInstanceOf(SourceLocation::class, $resolvedLocation); + $expectedFile = realpath($this->sourcePath([__DIR__, '..', '..', '..', 'src', 'Debugger', 'DebuggerClient.php'])); + $this->assertEquals($expectedFile, $resolvedLocation->path()); + $this->assertEquals(1, $resolvedLocation->line()); + } + + private function sourcePath($parts) + { + return implode(DIRECTORY_SEPARATOR, $parts); + } +} From ebcb3276695a402006f8acde09cf374d7f606fc4 Mon Sep 17 00:00:00 2001 From: Jeff Ching Date: Thu, 11 Jan 2018 11:30:02 -0800 Subject: [PATCH 02/13] Add SourceLocationResolver --- src/Debugger/Breakpoint.php | 19 ++- src/Debugger/SourceLocationResolver.php | 166 ++++++++++++++++++++++++ 2 files changed, 184 insertions(+), 1 deletion(-) create mode 100644 src/Debugger/SourceLocationResolver.php diff --git a/src/Debugger/Breakpoint.php b/src/Debugger/Breakpoint.php index 59b1e82c12a..1490b1e40e9 100644 --- a/src/Debugger/Breakpoint.php +++ b/src/Debugger/Breakpoint.php @@ -66,6 +66,13 @@ class Breakpoint implements \JsonSerializable */ private $location; + /** + * @var SourceLocation Resolved breakpoint location. The requested location + * may not exactly match the path to the deployed source. This value + * will be resolved by the Daemon to an existing file (if found). + */ + private $resolvedLocation + /** * @var string Condition that triggers the breakpoint. The condition is a * compound boolean expression composed using expressions in a @@ -314,7 +321,7 @@ public function action() */ public function location() { - return $this->location; + return $this->resolvedLocation ?: $this->location; } /** @@ -597,4 +604,14 @@ private function addVariable($name, $value) $this->variableTable = $this->variableTable ?: new VariableTable(); return $this->variableTable->register($name, $value); } + + private function resolveLocation() + { + if (!$this->location) { + return false; + } + + $dirname = dirname($this->location->path()); + $basename = basename($this->location->path()); + } } diff --git a/src/Debugger/SourceLocationResolver.php b/src/Debugger/SourceLocationResolver.php new file mode 100644 index 00000000000..56002ac4296 --- /dev/null +++ b/src/Debugger/SourceLocationResolver.php @@ -0,0 +1,166 @@ +resolve($location); + * echo $resolvedLocation->path(); + * ``` + * + * Case 2: There are extra folder(s) in the requested breakpoint path + * + * Example: + * ``` + * $location = new SourceLocation('extra/folder/src/Debugger/DebuggerClient.php', 1); + * $resolver = new SourceLocationResolver(); + * $resolvedLocation = $resolver->resolve($location); + * echo $resolvedLocation->path(); + * ``` + * + * Case 3: There are fewer folders in the requested breakpoint path + * + * Example: + * ``` + * $location = new SourceLocation('Debugger/DebuggerClient.php', 1); + * $resolver = new SourceLocationResolver(); + * $resolvedLocation = $resolver->resolve($location); + * echo $resolvedLocation->path(); + * ``` + */ +class SourceLocationResolver +{ + /** + * Resolve the full path of an existing file in the application's source. + * If no matching source file is found, then return null. If found, the + * resolved location will include the full, absolute path to the source + * file. + * + * @param SourceLocation $location The location to resolve. + * @return SourceLocation|null + */ + public function resolve(SourceLocation $location) + { + $basename = basename($location->path()); + $prefixes = $this->searchPrefixes($location->path()); + $includePaths = explode(':', get_include_path()); + + // Phase 1: search for an exact file match and try stripping off extra + // folders + foreach ($prefixes as $prefix) { + foreach ($includePaths as $path) { + $file = implode(DIRECTORY_SEPARATOR, [$path, $prefix, $basename]); + if (file_exists($file)) { + return new SourceLocation(realpath($file), $location->line()); + } + } + } + + // Phase 2: recursively search folders for + foreach ($includePaths as $includePath) { + $iterator = new MatchingFileIterator( + $includePath, + $location->path() + ); + foreach ($iterator as $file => $info) { + return new SourceLocation(realpath($file), $location->line()); + } + } + + return null; + } + + /** + * Returns an array of relative paths for this file by recursively removing + * each leading directory. + * + * @param string $path The source path + * @return string[] + */ + private function searchPrefixes($path) + { + $dirname = dirname($path); + $directoryParts = explode(DIRECTORY_SEPARATOR, $dirname); + $directories = []; + while ($directoryParts) { + $directories[] = implode(DIRECTORY_SEPARATOR, $directoryParts); + array_shift($directoryParts); + } + return $directories; + } +} + +/** + * This iterator returns files that match the provided file in the provided + * search path. + * + * Example: + * ``` + * $iterator = new MatchingFileIterator('.', 'src/Debugger/DebuggerClient.php'); + * $matches = iterator_to_array($iterator); + * ``` + * + * @access private + */ +class MatchingFileIterator extends \FilterIterator +{ + private $file; + + /** + * Create a new MatchingFileIterator. + * + * @param string $searchPath The root path to search in + * @param string $file The file to search for + */ + public function __construct($searchPath, $file) + { + parent::__construct( + new \RecursiveIteratorIterator( + new \RecursiveDirectoryIterator( + realpath($searchPath), + \FilesystemIterator::SKIP_DOTS + ) + ) + ); + $this->file = $file; + } + + /** + * FilterIterator callback to determine whether or not the value should be + * accepted. + * + * @return boolean + * @access private + */ + public function accept() + { + $candidate = $this->getInnerIterator()->current(); + return strrpos($candidate, $this->file) === strlen($candidate) - strlen($this->file); + } +} From cf822cc8ac2a9a393dedb568bad33eee6ac9a56b Mon Sep 17 00:00:00 2001 From: Jeff Ching Date: Thu, 11 Jan 2018 12:22:49 -0800 Subject: [PATCH 03/13] Breakpoint validation attempts to resolve the source location --- src/Debugger/Breakpoint.php | 28 ++++++++++++++++++++------ tests/unit/Debugger/BreakpointTest.php | 14 +++++++++++++ 2 files changed, 36 insertions(+), 6 deletions(-) diff --git a/src/Debugger/Breakpoint.php b/src/Debugger/Breakpoint.php index 1490b1e40e9..aab644db79a 100644 --- a/src/Debugger/Breakpoint.php +++ b/src/Debugger/Breakpoint.php @@ -71,7 +71,7 @@ class Breakpoint implements \JsonSerializable * may not exactly match the path to the deployed source. This value * will be resolved by the Daemon to an existing file (if found). */ - private $resolvedLocation + private $resolvedLocation; /** * @var string Condition that triggers the breakpoint. The condition is a @@ -587,6 +587,24 @@ public function validate() return false; } } + + if (!$this->location) { + $this->setError( + StatusMessage::REFERENCE_BREAKPOINT_SOURCE_LOCATION, + 'No source location specified' + ); + return false; + } + + if (!$this->resolveLocation()) { + $this->setError( + StatusMessage::REFERENCE_BREAKPOINT_SOURCE_LOCATION, + 'Could not find source location: $0', + [$this->location->path()] + ); + return false; + } + return true; } @@ -607,11 +625,9 @@ private function addVariable($name, $value) private function resolveLocation() { - if (!$this->location) { - return false; - } + $resolver = new SourceLocationResolver(); + $this->resolvedLocation = $resolver->resolve($this->location); - $dirname = dirname($this->location->path()); - $basename = basename($this->location->path()); + return $this->resolvedLocation !== null; } } diff --git a/tests/unit/Debugger/BreakpointTest.php b/tests/unit/Debugger/BreakpointTest.php index 1b8deb140ed..f32f6f4ce19 100644 --- a/tests/unit/Debugger/BreakpointTest.php +++ b/tests/unit/Debugger/BreakpointTest.php @@ -173,4 +173,18 @@ public function testAddingEvaluatedExpressions() $this->assertArrayHasKey('evaluatedExpressions', $json); $this->assertCount(2, $json['evaluatedExpressions']); } + + public function testResolvedLocationNotIncludedInJson() + { + $path = 'src/Debugger/DebuggerClient.php'; + $breakpoint = new Breakpoint([ + 'location' => [ + 'path' => $path, + 'line' => 1 + ] + ]); + $this->assertTrue($breakpoint->validate()); + // resolved location should + $this->assertTrue(strlen($path) < strlen($breakpoint->location()->path())); + } } From 2d03d2606f50b0a3ca49001303c877700dd72593 Mon Sep 17 00:00:00 2001 From: Jeff Ching Date: Thu, 11 Jan 2018 12:57:22 -0800 Subject: [PATCH 04/13] Move MatchingFileIterator to its own file for phpcs --- src/Debugger/MatchingFileIterator.php | 66 +++++++++++++++ src/Debugger/SourceLocationResolver.php | 102 +++++++----------------- 2 files changed, 94 insertions(+), 74 deletions(-) create mode 100644 src/Debugger/MatchingFileIterator.php diff --git a/src/Debugger/MatchingFileIterator.php b/src/Debugger/MatchingFileIterator.php new file mode 100644 index 00000000000..7b0f5552795 --- /dev/null +++ b/src/Debugger/MatchingFileIterator.php @@ -0,0 +1,66 @@ +file = $file; + } + + /** + * FilterIterator callback to determine whether or not the value should be + * accepted. + * + * @return boolean + * @access private + */ + public function accept() + { + $candidate = $this->getInnerIterator()->current(); + return strrpos($candidate, $this->file) === strlen($candidate) - strlen($this->file); + } +} diff --git a/src/Debugger/SourceLocationResolver.php b/src/Debugger/SourceLocationResolver.php index 56002ac4296..1def7c90456 100644 --- a/src/Debugger/SourceLocationResolver.php +++ b/src/Debugger/SourceLocationResolver.php @@ -22,36 +22,11 @@ * tree. A debugger breakpoint may be requested for a source path that has * extra or missing folders. * - * There are 3 cases for resolving a SourceLocation: - * - * Case 1: The exact path is found - * * Example: * ``` * $location = new SourceLocation('src/Debugger/DebuggerClient.php', 1); * $resolver = new SourceLocationResolver(); * $resolvedLocation = $resolver->resolve($location); - * echo $resolvedLocation->path(); - * ``` - * - * Case 2: There are extra folder(s) in the requested breakpoint path - * - * Example: - * ``` - * $location = new SourceLocation('extra/folder/src/Debugger/DebuggerClient.php', 1); - * $resolver = new SourceLocationResolver(); - * $resolvedLocation = $resolver->resolve($location); - * echo $resolvedLocation->path(); - * ``` - * - * Case 3: There are fewer folders in the requested breakpoint path - * - * Example: - * ``` - * $location = new SourceLocation('Debugger/DebuggerClient.php', 1); - * $resolver = new SourceLocationResolver(); - * $resolvedLocation = $resolver->resolve($location); - * echo $resolvedLocation->path(); * ``` */ class SourceLocationResolver @@ -62,6 +37,34 @@ class SourceLocationResolver * resolved location will include the full, absolute path to the source * file. * + * There are 3 cases for resolving a SourceLocation: + * + * Case 1: The exact path is found + * + * Example: + * ``` + * $location = new SourceLocation('src/Debugger/DebuggerClient.php', 1); + * $resolver = new SourceLocationResolver(); + * $resolvedLocation = $resolver->resolve($location); + * ``` + * + * Case 2: There are extra folder(s) in the requested breakpoint path + * + * Example: + * ``` + * $location = new SourceLocation('extra/folder/src/Debugger/DebuggerClient.php', 1); + * $resolver = new SourceLocationResolver(); + * $resolvedLocation = $resolver->resolve($location); + * ``` + * + * Case 3: There are fewer folders in the requested breakpoint path + * + * Example: + * ``` + * $location = new SourceLocation('Debugger/DebuggerClient.php', 1); + * $resolver = new SourceLocationResolver(); + * $resolvedLocation = $resolver->resolve($location); + * * @param SourceLocation $location The location to resolve. * @return SourceLocation|null */ @@ -115,52 +118,3 @@ private function searchPrefixes($path) return $directories; } } - -/** - * This iterator returns files that match the provided file in the provided - * search path. - * - * Example: - * ``` - * $iterator = new MatchingFileIterator('.', 'src/Debugger/DebuggerClient.php'); - * $matches = iterator_to_array($iterator); - * ``` - * - * @access private - */ -class MatchingFileIterator extends \FilterIterator -{ - private $file; - - /** - * Create a new MatchingFileIterator. - * - * @param string $searchPath The root path to search in - * @param string $file The file to search for - */ - public function __construct($searchPath, $file) - { - parent::__construct( - new \RecursiveIteratorIterator( - new \RecursiveDirectoryIterator( - realpath($searchPath), - \FilesystemIterator::SKIP_DOTS - ) - ) - ); - $this->file = $file; - } - - /** - * FilterIterator callback to determine whether or not the value should be - * accepted. - * - * @return boolean - * @access private - */ - public function accept() - { - $candidate = $this->getInnerIterator()->current(); - return strrpos($candidate, $this->file) === strlen($candidate) - strlen($this->file); - } -} From 24b23dc4811439f6157f46a666a9a95937330dad Mon Sep 17 00:00:00 2001 From: Jeff Ching Date: Thu, 11 Jan 2018 12:57:32 -0800 Subject: [PATCH 05/13] Add snippet tests for new classes --- .../Debugger/MatchingFileIteratorTest.php | 35 ++++++++++ .../Debugger/SourceLocationResolverTest.php | 64 +++++++++++++++++++ 2 files changed, 99 insertions(+) create mode 100644 tests/snippets/Debugger/MatchingFileIteratorTest.php create mode 100644 tests/snippets/Debugger/SourceLocationResolverTest.php diff --git a/tests/snippets/Debugger/MatchingFileIteratorTest.php b/tests/snippets/Debugger/MatchingFileIteratorTest.php new file mode 100644 index 00000000000..f57065a13ef --- /dev/null +++ b/tests/snippets/Debugger/MatchingFileIteratorTest.php @@ -0,0 +1,35 @@ +snippetFromClass(MatchingFileIterator::class); + $snippet->addUse(MatchingFileIterator::class); + $matches = $snippet->invoke('matches')->returnVal(); + $this->assertCount(1, $matches); + } +} diff --git a/tests/snippets/Debugger/SourceLocationResolverTest.php b/tests/snippets/Debugger/SourceLocationResolverTest.php new file mode 100644 index 00000000000..83f5932bb66 --- /dev/null +++ b/tests/snippets/Debugger/SourceLocationResolverTest.php @@ -0,0 +1,64 @@ +snippetFromClass(SourceLocationResolver::class); + $snippet->addUse(SourceLocation::class); + $snippet->addUse(SourceLocationResolver::class); + $res = $snippet->invoke('resolvedLocation'); + $this->assertInstanceOf(SourceLocation::class, $res->returnVal()); + } + + public function testResolveCase1() + { + $snippet = $this->snippetFromMethod(SourceLocationResolver::class, 'resolve'); + $snippet->addUse(SourceLocation::class); + $snippet->addUse(SourceLocationResolver::class); + $res = $snippet->invoke('resolvedLocation'); + $this->assertInstanceOf(SourceLocation::class, $res->returnVal()); + } + + public function testResolveCase2() + { + $snippet = $this->snippetFromMethod(SourceLocationResolver::class, 'resolve'); + $snippet->addUse(SourceLocation::class); + $snippet->addUse(SourceLocationResolver::class); + $res = $snippet->invoke('resolvedLocation'); + $this->assertInstanceOf(SourceLocation::class, $res->returnVal()); + } + + public function testResolveCase3() + { + $snippet = $this->snippetFromMethod(SourceLocationResolver::class, 'resolve'); + $snippet->addUse(SourceLocation::class); + $snippet->addUse(SourceLocationResolver::class); + $res = $snippet->invoke('resolvedLocation'); + $this->assertInstanceOf(SourceLocation::class, $res->returnVal()); + } +} From c993962f70d82c13d896aa58c22b704a4040b493 Mon Sep 17 00:00:00 2001 From: Jeff Ching Date: Thu, 11 Jan 2018 13:24:01 -0800 Subject: [PATCH 06/13] Install stackdriver_debugger extension on travis. PHPCS fix --- .travis.yml | 1 + src/Debugger/MatchingFileIterator.php | 1 + 2 files changed, 2 insertions(+) diff --git a/.travis.yml b/.travis.yml index 8002b044a84..4e0ca4ae4d3 100644 --- a/.travis.yml +++ b/.travis.yml @@ -15,6 +15,7 @@ matrix: before_script: - pecl install grpc || echo 'Failed to install grpc' + - pecl install stackdriver_debugger-alpha || echo 'Failed to install stackdriver_debugger' - composer install - if [[ $TRAVIS_PHP_VERSION =~ ^hhvm ]]; then composer --no-interaction --dev remove google/protobuf google/gax google/proto-client; fi - ./dev/sh/system-test-credentials diff --git a/src/Debugger/MatchingFileIterator.php b/src/Debugger/MatchingFileIterator.php index 7b0f5552795..e0c41e501bd 100644 --- a/src/Debugger/MatchingFileIterator.php +++ b/src/Debugger/MatchingFileIterator.php @@ -16,6 +16,7 @@ */ namespace Google\Cloud\Debugger; + /** * This iterator returns files that match the provided file in the provided * search path. From 6bb29c3023c611ce07e72dce5b6347b75d38d8e4 Mon Sep 17 00:00:00 2001 From: Jeff Ching Date: Wed, 17 Jan 2018 12:34:12 -0800 Subject: [PATCH 07/13] only try to install stackdriver extension on php 7 --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 4e0ca4ae4d3..b638dd434eb 100644 --- a/.travis.yml +++ b/.travis.yml @@ -15,7 +15,7 @@ matrix: before_script: - pecl install grpc || echo 'Failed to install grpc' - - pecl install stackdriver_debugger-alpha || echo 'Failed to install stackdriver_debugger' + - if [[ $TRAVIS_PHP_VERSION =~ ^7 ]]; then pecl install stackdriver_debugger-alpha || echo 'Failed to install stackdriver_debugger'; fi - composer install - if [[ $TRAVIS_PHP_VERSION =~ ^hhvm ]]; then composer --no-interaction --dev remove google/protobuf google/gax google/proto-client; fi - ./dev/sh/system-test-credentials From e10b59e83fdb99152dea05dd5a9eff70283629fa Mon Sep 17 00:00:00 2001 From: Jeff Ching Date: Thu, 18 Jan 2018 09:51:48 -0800 Subject: [PATCH 08/13] Make resolveLocation public and add tests --- src/Debugger/Breakpoint.php | 27 +++++++++++++++------- tests/snippets/Debugger/BreakpointTest.php | 15 ++++++++++++ tests/unit/Debugger/BreakpointTest.php | 9 ++++++-- 3 files changed, 41 insertions(+), 10 deletions(-) diff --git a/src/Debugger/Breakpoint.php b/src/Debugger/Breakpoint.php index 88ec1b0ce6d..dae9fee61d3 100644 --- a/src/Debugger/Breakpoint.php +++ b/src/Debugger/Breakpoint.php @@ -567,6 +567,25 @@ public function validate() $this->validateExpressions(); } + /** + * Attempts to resolve the real (full) path to the specified source + * location. Returns true if a location was resolved. + * + * Example: + * ``` + * $found = $breakpoint->resolveLocation(); + * ``` + * + * @return bool + */ + public function resolveLocation() + { + $resolver = new SourceLocationResolver(); + $this->resolvedLocation = $resolver->resolve($this->location); + + return $this->resolvedLocation !== null; + } + private function setError($type, $message, array $parameters = []) { $this->status = new StatusMessage( @@ -735,12 +754,4 @@ private function validateExpressions() return true; } - - private function resolveLocation() - { - $resolver = new SourceLocationResolver(); - $this->resolvedLocation = $resolver->resolve($this->location); - - return $this->resolvedLocation !== null; - } } diff --git a/tests/snippets/Debugger/BreakpointTest.php b/tests/snippets/Debugger/BreakpointTest.php index 7a02808c429..b83bc2b94e6 100644 --- a/tests/snippets/Debugger/BreakpointTest.php +++ b/tests/snippets/Debugger/BreakpointTest.php @@ -180,4 +180,19 @@ public function testValidate() $res = $snippet->invoke('valid'); } + + public function testResolveLocation() + { + $breakpoint = new Breakpoint([ + 'location' => [ + 'path' => __FILE__, + 'line' => 1 + ] + ]); + $snippet = $this->snippetFromMethod(Breakpoint::class, 'resolveLocation'); + $snippet->addLocal('breakpoint', $breakpoint); + + $res = $snippet->invoke('found'); + $this->assertTrue($res->returnVal()); + } } diff --git a/tests/unit/Debugger/BreakpointTest.php b/tests/unit/Debugger/BreakpointTest.php index f32f6f4ce19..4471588883a 100644 --- a/tests/unit/Debugger/BreakpointTest.php +++ b/tests/unit/Debugger/BreakpointTest.php @@ -183,8 +183,13 @@ public function testResolvedLocationNotIncludedInJson() 'line' => 1 ] ]); - $this->assertTrue($breakpoint->validate()); - // resolved location should + $this->assertTrue($breakpoint->resolveLocation()); + + // resolved location should have changed the path $this->assertTrue(strlen($path) < strlen($breakpoint->location()->path())); + $json = json_decode(json_encode($breakpoint->jsonSerialize()), true); + + // the serialized location should be unaffected + $this->assertEquals($path, $json['location']['path']); } } From 54a6cdd76c2e7ac969b0bca49e59747e993020c8 Mon Sep 17 00:00:00 2001 From: Jeff Ching Date: Thu, 18 Jan 2018 10:06:26 -0800 Subject: [PATCH 09/13] Fix path separator for windows --- src/Debugger/SourceLocationResolver.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Debugger/SourceLocationResolver.php b/src/Debugger/SourceLocationResolver.php index 1def7c90456..7f41650d615 100644 --- a/src/Debugger/SourceLocationResolver.php +++ b/src/Debugger/SourceLocationResolver.php @@ -72,7 +72,7 @@ public function resolve(SourceLocation $location) { $basename = basename($location->path()); $prefixes = $this->searchPrefixes($location->path()); - $includePaths = explode(':', get_include_path()); + $includePaths = explode(PATH_SEPARATOR, get_include_path()); // Phase 1: search for an exact file match and try stripping off extra // folders From a4fe0fc2005584906f067a39631fc0b4309cbe20 Mon Sep 17 00:00:00 2001 From: Jeff Ching Date: Thu, 18 Jan 2018 10:18:12 -0800 Subject: [PATCH 10/13] Add MatchingFileIterator unit test --- src/Debugger/MatchingFileIterator.php | 5 +- .../Debugger/MatchingFileIteratorTest.php | 53 +++++++++++++++++++ 2 files changed, 57 insertions(+), 1 deletion(-) create mode 100644 tests/unit/Debugger/MatchingFileIteratorTest.php diff --git a/src/Debugger/MatchingFileIterator.php b/src/Debugger/MatchingFileIterator.php index e0c41e501bd..00342d99489 100644 --- a/src/Debugger/MatchingFileIterator.php +++ b/src/Debugger/MatchingFileIterator.php @@ -31,6 +31,9 @@ */ class MatchingFileIterator extends \FilterIterator { + /** + * @var string The file pattern to search for. + */ private $file; /** @@ -56,8 +59,8 @@ public function __construct($searchPath, $file) * FilterIterator callback to determine whether or not the value should be * accepted. * - * @return boolean * @access private + * @return boolean */ public function accept() { diff --git a/tests/unit/Debugger/MatchingFileIteratorTest.php b/tests/unit/Debugger/MatchingFileIteratorTest.php new file mode 100644 index 00000000000..b653d30ee53 --- /dev/null +++ b/tests/unit/Debugger/MatchingFileIteratorTest.php @@ -0,0 +1,53 @@ +sourcePath([__DIR__, '..'])), + $this->sourcePath(['Connection', 'RestTest.php']) + ); + $matches = iterator_to_array($iterator); + $this->assertTrue(count($matches) > 2); + } + + public function testNoMatches() + { + $iterator = new MatchingFileIterator( + realpath($this->sourcePath([__DIR__, '..'])), + $this->sourcePath(['Connection', 'file-not-exists.php']) + ); + $matches = iterator_to_array($iterator); + $this->assertEmpty($matches); + } + + private function sourcePath($parts) + { + return implode(DIRECTORY_SEPARATOR, $parts); + } +} From e01751f3f69b09c2048b4dd8ef92cc53cb2d0e6d Mon Sep 17 00:00:00 2001 From: Jeff Ching Date: Thu, 18 Jan 2018 10:39:20 -0800 Subject: [PATCH 11/13] Fix merge. SourceLocation validations should happen on the resolved source location --- src/Debugger/Breakpoint.php | 37 ++++++++++++++----------------------- 1 file changed, 14 insertions(+), 23 deletions(-) diff --git a/src/Debugger/Breakpoint.php b/src/Debugger/Breakpoint.php index dae9fee61d3..81e8775c7c2 100644 --- a/src/Debugger/Breakpoint.php +++ b/src/Debugger/Breakpoint.php @@ -634,7 +634,16 @@ private function validateSourceLocation() return false; } - $path = $this->location->path(); + if (!$this->resolveLocation()) { + $this->setError( + StatusMessage::REFERENCE_BREAKPOINT_SOURCE_LOCATION, + 'Could not find source location: $0', + [$this->location->path()] + ); + return false; + } + + $path = $this->resolvedLocation->path(); $info = new \SplFileInfo($path); // Ensure the file exists and is readable @@ -658,7 +667,7 @@ private function validateSourceLocation() } $file = $info->openFile('r'); - $file->seek($this->location->line() - 1); + $file->seek($this->resolvedLocation->line() - 1); $line = ltrim($file->current() ?: ''); // Ensure the line exists and is not empty @@ -666,17 +675,17 @@ private function validateSourceLocation() $this->setError( StatusMessage::REFERENCE_BREAKPOINT_SOURCE_LOCATION, 'Invalid breakpoint location - Invalid file line: $0.', - [$this->location->line()] + [$this->resolvedLocation->line()] ); return false; } // Check that the line is not a comment - if ($line[0] == '/' || ($line[0] == '*' && $this->inMultilineComment($file, $this->location->line() - 1))) { + if ($line[0] == '/' || ($line[0] == '*' && $this->inMultilineComment($file, $this->resolvedLocation->line() - 1))) { $this->setError( StatusMessage::REFERENCE_BREAKPOINT_SOURCE_LOCATION, 'Invalid breakpoint location - Invalid file line: $0.', - [$this->location->line()] + [$this->resolvedLocation->line()] ); return false; } @@ -734,24 +743,6 @@ private function validateExpressions() return false; } } - - if (!$this->location) { - $this->setError( - StatusMessage::REFERENCE_BREAKPOINT_SOURCE_LOCATION, - 'No source location specified' - ); - return false; - } - - if (!$this->resolveLocation()) { - $this->setError( - StatusMessage::REFERENCE_BREAKPOINT_SOURCE_LOCATION, - 'Could not find source location: $0', - [$this->location->path()] - ); - return false; - } - return true; } } From fda5ee92760be61873bf1f4f65fd4f40217785cf Mon Sep 17 00:00:00 2001 From: Jeff Ching Date: Thu, 18 Jan 2018 10:54:13 -0800 Subject: [PATCH 12/13] Fix phpcs --- src/Debugger/Breakpoint.php | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/Debugger/Breakpoint.php b/src/Debugger/Breakpoint.php index 81e8775c7c2..02e3775c763 100644 --- a/src/Debugger/Breakpoint.php +++ b/src/Debugger/Breakpoint.php @@ -644,6 +644,7 @@ private function validateSourceLocation() } $path = $this->resolvedLocation->path(); + $lineNumber = $this->resolvedLocation->line(); $info = new \SplFileInfo($path); // Ensure the file exists and is readable @@ -667,7 +668,7 @@ private function validateSourceLocation() } $file = $info->openFile('r'); - $file->seek($this->resolvedLocation->line() - 1); + $file->seek($lineNumber - 1); $line = ltrim($file->current() ?: ''); // Ensure the line exists and is not empty @@ -675,17 +676,17 @@ private function validateSourceLocation() $this->setError( StatusMessage::REFERENCE_BREAKPOINT_SOURCE_LOCATION, 'Invalid breakpoint location - Invalid file line: $0.', - [$this->resolvedLocation->line()] + [$lineNumber] ); return false; } // Check that the line is not a comment - if ($line[0] == '/' || ($line[0] == '*' && $this->inMultilineComment($file, $this->resolvedLocation->line() - 1))) { + if ($line[0] == '/' || ($line[0] == '*' && $this->inMultilineComment($file, $lineNumber - 1))) { $this->setError( StatusMessage::REFERENCE_BREAKPOINT_SOURCE_LOCATION, 'Invalid breakpoint location - Invalid file line: $0.', - [$this->resolvedLocation->line()] + [$lineNumber] ); return false; } From ea5751572bfaf923c50397ee9d253383a978e222 Mon Sep 17 00:00:00 2001 From: Jeff Ching Date: Thu, 18 Jan 2018 13:31:10 -0800 Subject: [PATCH 13/13] Add a comment about the MatchingFileIterator accept logic --- src/Debugger/MatchingFileIterator.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Debugger/MatchingFileIterator.php b/src/Debugger/MatchingFileIterator.php index 00342d99489..3cefe2028ab 100644 --- a/src/Debugger/MatchingFileIterator.php +++ b/src/Debugger/MatchingFileIterator.php @@ -65,6 +65,8 @@ public function __construct($searchPath, $file) public function accept() { $candidate = $this->getInnerIterator()->current(); + + // Check that the candidate file (a full file path) ends in the pattern we are searching for. return strrpos($candidate, $this->file) === strlen($candidate) - strlen($this->file); } }