From 647d1e38b6fb1d2b0ec4efccee24e15b5cf45a0f Mon Sep 17 00:00:00 2001 From: Nilambar Sharma Date: Sat, 7 Sep 2024 21:16:05 +0545 Subject: [PATCH 1/8] Fix path issue in CLI output --- includes/Traits/Amend_Check_Result.php | 3 ++- tests/behat/features/plugin-check-remote.feature | 4 ++++ tests/behat/features/plugin-check-severity.feature | 4 ++++ 3 files changed, 10 insertions(+), 1 deletion(-) diff --git a/includes/Traits/Amend_Check_Result.php b/includes/Traits/Amend_Check_Result.php index 4dc5a1951..8eba7f48c 100644 --- a/includes/Traits/Amend_Check_Result.php +++ b/includes/Traits/Amend_Check_Result.php @@ -34,13 +34,14 @@ trait Amend_Check_Result { * @param int $severity Severity level. Default is 5. */ protected function add_result_message_for_file( Check_Result $result, $error, $message, $code, $file, $line = 0, $column = 0, string $docs = '', $severity = 5 ) { + $filename = explode( $result->plugin()->path(), $file ); $result->add_message( (bool) $error, $message, array( 'code' => $code, - 'file' => str_replace( $result->plugin()->path(), '', $file ), + 'file' => ( 1 === count( $filename ) ) ? reset( $filename ) : $filename[1], 'line' => $line, 'column' => $column, 'link' => $this->get_file_editor_url( $result, $file, $line ), diff --git a/tests/behat/features/plugin-check-remote.feature b/tests/behat/features/plugin-check-remote.feature index 6499f0663..4f1040a61 100644 --- a/tests/behat/features/plugin-check-remote.feature +++ b/tests/behat/features/plugin-check-remote.feature @@ -35,6 +35,10 @@ Feature: Test that the WP-CLI plugin check command works with remote ZIP url. Scenario: Test with valid ZIP When I run the WP-CLI command `plugin check https://github.com/WordPress/plugin-check/raw/trunk/tests/behat/testdata/foo-bar-wp.zip --fields=code,type --format=csv` Then STDOUT should contain: + """ + FILE: foo-bar-wp.php + """ + And STDOUT should contain: """ WordPress.WP.AlternativeFunctions.rand_mt_rand,ERROR """ diff --git a/tests/behat/features/plugin-check-severity.feature b/tests/behat/features/plugin-check-severity.feature index 38323ae62..bf86f9f28 100644 --- a/tests/behat/features/plugin-check-severity.feature +++ b/tests/behat/features/plugin-check-severity.feature @@ -64,6 +64,10 @@ Feature: Test that the severity level in plugin check works. When I run the WP-CLI command `plugin check foo-bar-wp --format=csv --fields=code,type,severity` Then STDOUT should contain: + """ + FILE: foo-bar-wp.php + """ + And STDOUT should contain: """ allow_unfiltered_uploads_detected,ERROR,7 """ From 5180f4f5deb9ea2ffca5608a2c6f236043ff7964 Mon Sep 17 00:00:00 2001 From: Nilambar Sharma Date: Mon, 9 Sep 2024 10:23:19 +0545 Subject: [PATCH 2/8] Update basename conditionals --- includes/Checker/Abstract_Check_Runner.php | 14 +++++++++++--- includes/Traits/Amend_Check_Result.php | 3 +-- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/includes/Checker/Abstract_Check_Runner.php b/includes/Checker/Abstract_Check_Runner.php index 8cb612c5b..c621088cc 100644 --- a/includes/Checker/Abstract_Check_Runner.php +++ b/includes/Checker/Abstract_Check_Runner.php @@ -541,6 +541,8 @@ private function get_check_exclude_slugs() { * @since 1.0.0 * * @return string The plugin basename to check. + * + * @throws Exception Thrown if an plugin basename could not be set. */ final public function get_plugin_basename() { if ( null === $this->plugin_basename ) { @@ -550,10 +552,16 @@ final public function get_plugin_basename() { $this->plugin_basename = Plugin_Request_Utility::download_plugin( $plugin ); $this->delete_plugin_folder = true; - } elseif ( Plugin_Request_Utility::is_directory_valid_plugin( $plugin ) ) { - $this->plugin_basename = $plugin; } else { - $this->plugin_basename = Plugin_Request_Utility::get_plugin_basename_from_input( $plugin ); + try { + $this->plugin_basename = Plugin_Request_Utility::get_plugin_basename_from_input( $plugin ); + } catch ( Exception $exception ) { + if ( Plugin_Request_Utility::is_directory_valid_plugin( $plugin ) ) { + $this->plugin_basename = $plugin; + } else { + throw $exception; + } + } } } diff --git a/includes/Traits/Amend_Check_Result.php b/includes/Traits/Amend_Check_Result.php index 8eba7f48c..4dc5a1951 100644 --- a/includes/Traits/Amend_Check_Result.php +++ b/includes/Traits/Amend_Check_Result.php @@ -34,14 +34,13 @@ trait Amend_Check_Result { * @param int $severity Severity level. Default is 5. */ protected function add_result_message_for_file( Check_Result $result, $error, $message, $code, $file, $line = 0, $column = 0, string $docs = '', $severity = 5 ) { - $filename = explode( $result->plugin()->path(), $file ); $result->add_message( (bool) $error, $message, array( 'code' => $code, - 'file' => ( 1 === count( $filename ) ) ? reset( $filename ) : $filename[1], + 'file' => str_replace( $result->plugin()->path(), '', $file ), 'line' => $line, 'column' => $column, 'link' => $this->get_file_editor_url( $result, $file, $line ), From ed046a783f1d445c9be700c1d4168dfd224ba047 Mon Sep 17 00:00:00 2001 From: Pascal Birchler Date: Mon, 9 Sep 2024 11:44:54 +0200 Subject: [PATCH 3/8] Use `realpath` --- includes/Plugin_Context.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/includes/Plugin_Context.php b/includes/Plugin_Context.php index e0c4923a8..d4f6921b6 100644 --- a/includes/Plugin_Context.php +++ b/includes/Plugin_Context.php @@ -67,6 +67,8 @@ public function __construct( $main_file ) { } } } + + $this->main_file = realpath( $this->main_file ); } /** From e9cdd3a3db24c08f0475f5748479932068e70c1e Mon Sep 17 00:00:00 2001 From: Pascal Birchler Date: Mon, 9 Sep 2024 11:49:26 +0200 Subject: [PATCH 4/8] Move the `realpath` call --- includes/Plugin_Context.php | 6 +++--- includes/Traits/Amend_Check_Result.php | 3 +-- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/includes/Plugin_Context.php b/includes/Plugin_Context.php index d4f6921b6..e75d8a112 100644 --- a/includes/Plugin_Context.php +++ b/includes/Plugin_Context.php @@ -68,7 +68,7 @@ public function __construct( $main_file ) { } } - $this->main_file = realpath( $this->main_file ); + $this->main_file = $this->main_file; } /** @@ -103,9 +103,9 @@ public function main_file() { */ public function path( $relative_path = '/' ) { if ( is_dir( $this->main_file ) ) { - return trailingslashit( $this->main_file ) . ltrim( $relative_path, '/' ); + return realpath( trailingslashit( $this->main_file ) . ltrim( $relative_path, '/' ) ); } else { - return plugin_dir_path( $this->main_file ) . ltrim( $relative_path, '/' ); + return realpath( plugin_dir_path( $this->main_file ) . ltrim( $relative_path, '/' ) ); } } diff --git a/includes/Traits/Amend_Check_Result.php b/includes/Traits/Amend_Check_Result.php index 4dc5a1951..b9c05e979 100644 --- a/includes/Traits/Amend_Check_Result.php +++ b/includes/Traits/Amend_Check_Result.php @@ -34,13 +34,12 @@ trait Amend_Check_Result { * @param int $severity Severity level. Default is 5. */ protected function add_result_message_for_file( Check_Result $result, $error, $message, $code, $file, $line = 0, $column = 0, string $docs = '', $severity = 5 ) { - $result->add_message( (bool) $error, $message, array( 'code' => $code, - 'file' => str_replace( $result->plugin()->path(), '', $file ), + 'file' => str_replace( trailingslashit( $result->plugin()->path() ), '', $file ), 'line' => $line, 'column' => $column, 'link' => $this->get_file_editor_url( $result, $file, $line ), From b098883c32df1cd0e812c9f3d646a4fae4c1a0fb Mon Sep 17 00:00:00 2001 From: Nilambar Sharma Date: Mon, 9 Sep 2024 17:34:26 +0545 Subject: [PATCH 5/8] Add slash in path which realpath ate --- includes/Plugin_Context.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/includes/Plugin_Context.php b/includes/Plugin_Context.php index e75d8a112..cb5eafd9e 100644 --- a/includes/Plugin_Context.php +++ b/includes/Plugin_Context.php @@ -103,9 +103,9 @@ public function main_file() { */ public function path( $relative_path = '/' ) { if ( is_dir( $this->main_file ) ) { - return realpath( trailingslashit( $this->main_file ) . ltrim( $relative_path, '/' ) ); + return realpath( trailingslashit( $this->main_file ) . ltrim( $relative_path, '/' ) ) . '/'; } else { - return realpath( plugin_dir_path( $this->main_file ) . ltrim( $relative_path, '/' ) ); + return realpath( plugin_dir_path( $this->main_file ) . ltrim( $relative_path, '/' ) ) . '/'; } } From 7a6dcb5480e50dc19e5819a5d994ad45283a14fc Mon Sep 17 00:00:00 2001 From: Nilambar Sharma Date: Mon, 9 Sep 2024 18:09:26 +0545 Subject: [PATCH 6/8] Remove trailingslashit in Amend trait --- includes/Traits/Amend_Check_Result.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/includes/Traits/Amend_Check_Result.php b/includes/Traits/Amend_Check_Result.php index b9c05e979..4dc5a1951 100644 --- a/includes/Traits/Amend_Check_Result.php +++ b/includes/Traits/Amend_Check_Result.php @@ -34,12 +34,13 @@ trait Amend_Check_Result { * @param int $severity Severity level. Default is 5. */ protected function add_result_message_for_file( Check_Result $result, $error, $message, $code, $file, $line = 0, $column = 0, string $docs = '', $severity = 5 ) { + $result->add_message( (bool) $error, $message, array( 'code' => $code, - 'file' => str_replace( trailingslashit( $result->plugin()->path() ), '', $file ), + 'file' => str_replace( $result->plugin()->path(), '', $file ), 'line' => $line, 'column' => $column, 'link' => $this->get_file_editor_url( $result, $file, $line ), From fc2835799afa5d64c8d9d77504181b488235a4dc Mon Sep 17 00:00:00 2001 From: Nilambar Sharma Date: Wed, 11 Sep 2024 15:47:13 +0545 Subject: [PATCH 7/8] Use actual test file instead of virtual filename string in Check_Result_Tests --- .../testdata/plugins/test-plugin/test-plugin.php | 16 ++++++++++++++++ .../phpunit/tests/Checker/Check_Result_Tests.php | 12 ++++++------ 2 files changed, 22 insertions(+), 6 deletions(-) create mode 100644 tests/phpunit/testdata/plugins/test-plugin/test-plugin.php diff --git a/tests/phpunit/testdata/plugins/test-plugin/test-plugin.php b/tests/phpunit/testdata/plugins/test-plugin/test-plugin.php new file mode 100644 index 000000000..2d88c1458 --- /dev/null +++ b/tests/phpunit/testdata/plugins/test-plugin/test-plugin.php @@ -0,0 +1,16 @@ +check_result = new Check_Result( $check_context ); } @@ -28,7 +28,7 @@ public function test_plugin() { $this->assertInstanceOf( Check_Context::class, $this->check_result->plugin() ); // Check the Check_Context has the correct basename. - $this->assertSame( 'test-plugin/test-plugin.php', $this->check_result->plugin()->basename() ); + $this->assertStringEndsWith( 'test-plugin/test-plugin.php', $this->check_result->plugin()->basename() ); } public function test_add_message_with_warning() { @@ -37,7 +37,7 @@ public function test_add_message_with_warning() { 'Warning message', array( 'code' => 'test_warning', - 'file' => 'test-plugin/test-plugin.php', + 'file' => UNIT_TESTS_PLUGIN_DIR . 'test-plugin/test-plugin.php', 'line' => 12, 'column' => 40, ) @@ -73,7 +73,7 @@ public function test_add_message_with_error() { 'Error message', array( 'code' => 'test_error', - 'file' => 'test-plugin/test-plugin.php', + 'file' => UNIT_TESTS_PLUGIN_DIR . 'test-plugin/test-plugin.php', 'line' => 22, 'column' => 30, ) @@ -113,7 +113,7 @@ public function test_get_errors_with_errors() { 'Error message', array( 'code' => 'test_error', - 'file' => 'test-plugin/test-plugin.php', + 'file' => UNIT_TESTS_PLUGIN_DIR . 'test-plugin/test-plugin.php', 'line' => 22, 'column' => 30, ) @@ -146,7 +146,7 @@ public function test_get_warnings_with_warnings() { 'Warning message', array( 'code' => 'test_warning', - 'file' => 'test-plugin/test-plugin.php', + 'file' => UNIT_TESTS_PLUGIN_DIR . 'test-plugin/test-plugin.php', 'line' => 22, 'column' => 30, ) From 028ea1aafc3151e51ff6cf67d49e128b3c9b80d8 Mon Sep 17 00:00:00 2001 From: Nilambar Sharma Date: Wed, 11 Sep 2024 16:20:10 +0545 Subject: [PATCH 8/8] Use actual existing folder in Contexts test --- tests/phpunit/tests/Checker/Check_Context_Tests.php | 4 ++-- tests/phpunit/tests/Plugin_Context_Tests.php | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/phpunit/tests/Checker/Check_Context_Tests.php b/tests/phpunit/tests/Checker/Check_Context_Tests.php index ad6046d5b..17dc4e874 100644 --- a/tests/phpunit/tests/Checker/Check_Context_Tests.php +++ b/tests/phpunit/tests/Checker/Check_Context_Tests.php @@ -34,7 +34,7 @@ public function test_path() { } public function test_path_with_parameter() { - $this->assertSame( WP_PLUGIN_DIR . '/' . $this->plugin_name . '/another/folder', $this->check_context->path( '/another/folder' ) ); + $this->assertSame( WP_PLUGIN_DIR . '/' . $this->plugin_name . '/test-content/themes', $this->check_context->path( '/test-content/themes' ) ); } public function test_url() { @@ -42,6 +42,6 @@ public function test_url() { } public function test_url_with_parameter() { - $this->assertSame( WP_PLUGIN_URL . '/' . $this->plugin_name . '/folder/file.css', $this->check_context->url( '/folder/file.css' ) ); + $this->assertSame( WP_PLUGIN_URL . '/' . $this->plugin_name . '/assets/js/plugin-check-admin.js', $this->check_context->url( '/assets/js/plugin-check-admin.js' ) ); } } diff --git a/tests/phpunit/tests/Plugin_Context_Tests.php b/tests/phpunit/tests/Plugin_Context_Tests.php index e930aecb0..db5f01a53 100644 --- a/tests/phpunit/tests/Plugin_Context_Tests.php +++ b/tests/phpunit/tests/Plugin_Context_Tests.php @@ -34,7 +34,7 @@ public function test_path() { } public function test_path_with_parameter() { - $this->assertSame( WP_PLUGIN_DIR . '/' . $this->plugin_name . '/another/folder', $this->plugin_context->path( '/another/folder' ) ); + $this->assertSame( WP_PLUGIN_DIR . '/' . $this->plugin_name . '/test-content/themes', $this->plugin_context->path( '/test-content/themes' ) ); } public function test_url() { @@ -42,7 +42,7 @@ public function test_url() { } public function test_url_with_parameter() { - $this->assertSame( WP_PLUGIN_URL . '/' . $this->plugin_name . '/folder/file.css', $this->plugin_context->url( '/folder/file.css' ) ); + $this->assertSame( WP_PLUGIN_URL . '/' . $this->plugin_name . '/assets/js/plugin-check-admin.js', $this->plugin_context->url( '/assets/js/plugin-check-admin.js' ) ); } public function test_location() {