From bbeabb8bf87229efe196ec966da1492b97aa8b14 Mon Sep 17 00:00:00 2001 From: Faisal Ahammad Date: Sun, 26 Jul 2026 19:52:34 +0600 Subject: [PATCH 1/3] fix(traits): collapse File_Editor_URL to single filter The File_Editor_URL trait previously exposed two filters ('_file_editor_url_template' and '_file_path') that consumers had to register together to override the editor link. Following review feedback on PR #298, this collapses them into a single filter, wp_plugin_check_validation_error_source_url, that receives a $source array (file, line, plugin, filename) and returns either a URL string or null to fall back to the plugin editor. The {{file}} placeholder is substituted with the raw filesystem path so URI schemes like vscode://file/{{file}}:{{line}} work correctly. {{line}} remains substituted with the integer line number. This is a backward-incompatible change to the public filter API. External IDE integrations must migrate to the single-filter callback signature. Fixes #314 --- includes/Traits/File_Editor_URL.php | 116 +++++----- .../tests/Traits/File_Editor_URL_Tests.php | 218 ++++++++++++++++++ 2 files changed, 275 insertions(+), 59 deletions(-) create mode 100644 tests/phpunit/tests/Traits/File_Editor_URL_Tests.php diff --git a/includes/Traits/File_Editor_URL.php b/includes/Traits/File_Editor_URL.php index 94f9e0753..cbb03aed6 100644 --- a/includes/Traits/File_Editor_URL.php +++ b/includes/Traits/File_Editor_URL.php @@ -33,67 +33,65 @@ protected function get_file_editor_url( Check_Result $result, $filename, $line = $plugin_path = $result->plugin()->path( '/' ); $plugin_slug = $result->plugin()->slug(); $filename = str_replace( $plugin_path, '', $filename ); - /** - * Filters the template for the URL for linking to an external editor to open a file for editing. - * - * Users of IDEs that support opening files in via web protocols can use this filter to override - * the edit link to result in their editor opening rather than the plugin editor. - * - * The initial filtered value is null, requiring extension plugins to supply the URL template - * string themselves. If no template string is provided, links to the plugin editors will - * be provided if available. For example, for an extension plugin to cause file edit links to - * open in an IDE, the following filters can be used: - * - * # PhpStorm - * add_filter( 'wp_plugin_check_validation_error_source_file_editor_url_template', function () { - * return 'phpstorm://open?file={{file}}&line={{line}}'; - * } ); - * - * # VS Code - * add_filter( 'wp_plugin_check_validation_error_source_file_editor_url_template', function () { - * return 'vscode://file/{{file}}:{{line}}'; - * } ); - * - * For a template to be considered, the string '{{file}}' must be present in the filtered value. - * - * @since 1.0.0 - * - * @param string|null $editor_url_template Editor URL template. default null. - */ - $editor_url_template = apply_filters( 'wp_plugin_check_validation_error_source_file_editor_url_template', null ); - // Supply the file path to the editor template. - if ( is_string( $editor_url_template ) && str_contains( $editor_url_template, '{{file}}' ) ) { - $file_path = WP_PLUGIN_DIR . '/' . $plugin_slug; - if ( $plugin_slug !== $filename ) { - $file_path .= '/' . $filename; - } + $file_path = WP_PLUGIN_DIR . '/' . $plugin_slug; + if ( $plugin_slug !== $filename ) { + $file_path .= '/' . $filename; + } - if ( file_exists( $file_path ) ) { - /** - * Filters the file path to be opened in an external editor for a given PHPCS error source. - * - * This is useful to map the file path from inside of a Docker container or VM to the host machine. - * - * @since 1.0.0 - * - * @param string|null $editor_url_template Editor URL template. - * @param array $source Source information. - */ - $file_path = apply_filters( 'wp_plugin_check_validation_error_source_file_path', $file_path, array( $plugin_slug, $filename, $line ) ); - if ( $file_path ) { - $edit_url = str_replace( - array( - '{{file}}', - '{{line}}', - ), - array( - rawurlencode( $file_path ), - $line, - ), - $editor_url_template - ); - } + if ( file_exists( $file_path ) ) { + /** + * Filters the URL for linking to an external editor to open a file for editing. + * + * Users of IDEs that support opening files via web protocols can use this filter to + * override the edit link so it opens in their editor rather than the plugin editor. + * + * The initial filtered value is null, requiring extension plugins to supply the URL + * themselves. If no URL is provided, links to the plugin editor are used if available. + * Returning a string that contains `{{file}}` or `{{line}}` placeholders causes them + * to be substituted with the raw filesystem path and the integer line number + * respectively. Returning a string without placeholders uses it verbatim. + * + * For example, to cause file edit links to open in an IDE: + * + * # PhpStorm + * add_filter( 'wp_plugin_check_validation_error_source_url', function ( $url, $source ) { + * return 'phpstorm://open?file=' . rawurlencode( $source['file'] ) . '&line=' . (int) $source['line']; + * }, 10, 2 ); + * + * # VS Code (using placeholders) + * add_filter( 'wp_plugin_check_validation_error_source_url', function ( $url ) { + * return 'vscode://file/{{file}}:{{line}}'; + * } ); + * + * @since 2.0.0 + * + * @param string|null $url Editor URL. Default null. + * @param array $source Source information: file, line, plugin, filename. + */ + $url = apply_filters( + 'wp_plugin_check_validation_error_source_url', + null, + array( + 'file' => $file_path, + 'line' => $line, + 'plugin' => $plugin_slug, + 'filename' => $filename, + ) + ); + + if ( is_string( $url ) && '' !== $url ) { + $edit_url = str_replace( + array( + '{{file}}', + '{{line}}', + ), + array( + $file_path, + (int) $line, + ), + $url + ); } } diff --git a/tests/phpunit/tests/Traits/File_Editor_URL_Tests.php b/tests/phpunit/tests/Traits/File_Editor_URL_Tests.php new file mode 100644 index 000000000..fb2b1ab89 --- /dev/null +++ b/tests/phpunit/tests/Traits/File_Editor_URL_Tests.php @@ -0,0 +1,218 @@ +url_callbacks as $cb ) { + remove_filter( 'wp_plugin_check_validation_error_source_url', $cb, 10 ); + } + $this->url_callbacks = array(); + + foreach ( $this->cap_callbacks as $cb ) { + remove_filter( 'user_has_cap', $cb, 10 ); + } + $this->cap_callbacks = array(); + + parent::tear_down(); + } + + /** + * Single-file test plugin fixture basename (load.php in fixture dir). + * + * @return string + */ + private function single_file_plugin_basename() { + return 'test-plugin-external-admin-menu-links-without-errors/load.php'; + } + + /** + * Registers a url filter callback and tracks it for cleanup. + * + * @param callable $callback Filter callback. + * @param int $accepted_args Number of args the callback accepts. + */ + private function add_url_filter( $callback, $accepted_args = 1 ) { + add_filter( 'wp_plugin_check_validation_error_source_url', $callback, 10, $accepted_args ); + $this->url_callbacks[] = $callback; + } + + /** + * Registers a user_has_cap callback and tracks it for cleanup. + * + * @param callable $callback Filter callback. + */ + private function add_cap_filter( $callback ) { + add_filter( 'user_has_cap', $callback, 10, 1 ); + $this->cap_callbacks[] = $callback; + } + + /** + * When no filter is registered and the user lacks edit_plugins cap, returns null. + */ + public function test_returns_null_without_filter_and_without_caps() { + wp_set_current_user( 0 ); + + $context = new Check_Context( $this->single_file_plugin_basename() ); + $result = new Check_Result( $context ); + + $url = $this->get_file_editor_url( $result, 'load.php', 42 ); + + $this->assertNull( $url ); + } + + /** + * Filter returning a template with placeholders gets them substituted verbatim. + * + * `{{file}}` is replaced with the raw filesystem path (no URL encoding) so that + * editor URI schemes like `vscode://file/{{file}}:{{line}}` interpret the path + * correctly. `{{line}}` is replaced with the integer line number. + */ + public function test_filter_url_with_placeholders_substituted() { + $this->add_url_filter( + static function ( $url ) { + return 'vscode://file/{{file}}:{{line}}'; + } + ); + + $context = new Check_Context( $this->single_file_plugin_basename() ); + $result = new Check_Result( $context ); + + $root = WP_PLUGIN_DIR . '/test-plugin-external-admin-menu-links-without-errors/load.php'; + + $this->assertSame( + 'vscode://file/' . $root . ':7', + $this->get_file_editor_url( $result, 'load.php', 7 ) + ); + } + + /** + * Filter returning a string without placeholders is used verbatim. + */ + public function test_filter_url_without_placeholders_used_verbatim() { + $this->add_url_filter( + static function ( $url, $source ) { + return 'phpstorm://open?file=' . rawurlencode( $source['file'] ) . '&line=' . (int) $source['line']; + }, + 2 + ); + + $context = new Check_Context( $this->single_file_plugin_basename() ); + $result = new Check_Result( $context ); + + $expected = 'phpstorm://open?file=' . rawurlencode( WP_PLUGIN_DIR . '/test-plugin-external-admin-menu-links-without-errors/load.php' ) . '&line=9'; + + $this->assertSame( + $expected, + $this->get_file_editor_url( $result, 'load.php', 9 ) + ); + } + + /** + * Filter receives the $source array with file, line, plugin, filename keys. + */ + public function test_filter_receives_source_array() { + $captured = null; + + $this->add_url_filter( + static function ( $url, $source ) use ( &$captured ) { + $captured = $source; + return 'noop://handler'; + }, + 2 + ); + + $context = new Check_Context( $this->single_file_plugin_basename() ); + $result = new Check_Result( $context ); + + $this->get_file_editor_url( $result, 'load.php', 5 ); + + $this->assertIsArray( $captured ); + $this->assertArrayHasKey( 'file', $captured ); + $this->assertArrayHasKey( 'line', $captured ); + $this->assertArrayHasKey( 'plugin', $captured ); + $this->assertArrayHasKey( 'filename', $captured ); + $this->assertSame( 5, $captured['line'] ); + $this->assertSame( 'test-plugin-external-admin-menu-links-without-errors', $captured['plugin'] ); + $this->assertSame( 'load.php', $captured['filename'] ); + $this->assertSame( WP_PLUGIN_DIR . '/test-plugin-external-admin-menu-links-without-errors/load.php', $captured['file'] ); + } + + /** + * Plugin-editor fallback is used when filter is not registered and user has edit_plugins cap. + * + * When `$line === 0`, the fallback URL must omit the `line` query arg entirely. + */ + public function test_fallback_to_plugin_editor_when_no_filter() { + $this->add_cap_filter( + static function ( $caps ) { + $caps['edit_plugins'] = true; + return $caps; + } + ); + + $context = new Check_Context( $this->single_file_plugin_basename() ); + $result = new Check_Result( $context ); + + $url = $this->get_file_editor_url( $result, 'load.php', 0 ); + + $this->assertIsString( $url ); + parse_str( (string) wp_parse_url( $url, PHP_URL_QUERY ), $query_args ); + $this->assertArrayHasKey( 'plugin', $query_args ); + $this->assertArrayHasKey( 'file', $query_args ); + $this->assertArrayNotHasKey( 'line', $query_args ); + $this->assertSame( 'plugin-editor.php', wp_parse_url( $url, PHP_URL_PATH ) ); + } + + /** + * Plugin-editor fallback includes `line` query arg when line is set. + */ + public function test_fallback_to_plugin_editor_includes_line_when_set() { + $this->add_cap_filter( + static function ( $caps ) { + $caps['edit_plugins'] = true; + return $caps; + } + ); + + $context = new Check_Context( $this->single_file_plugin_basename() ); + $result = new Check_Result( $context ); + + $url = $this->get_file_editor_url( $result, 'load.php', 42 ); + + $this->assertIsString( $url ); + parse_str( (string) wp_parse_url( $url, PHP_URL_QUERY ), $query_args ); + $this->assertSame( '42', $query_args['line'] ); + $this->assertSame( 'plugin-editor.php', wp_parse_url( $url, PHP_URL_PATH ) ); + } +} From 7351ac23a29d1fa02003b037aaad073811bd0684 Mon Sep 17 00:00:00 2001 From: Faisal Ahammad Date: Sun, 26 Jul 2026 20:25:06 +0600 Subject: [PATCH 2/3] fix(tests): make File_Editor_URL_Tests work in wp-env - set_up symlinks testdata fixture into WP_PLUGIN_DIR each test - tear_down removes the symlink after each test - use absolute path for Check_Context so Plugin_Context resolves - compare basename(wp_parse_url(...)) for plugin-editor fallback Errors fixed: - File_Editor_URL_Tests: 5 PHPUnit failures on PHP 7.4 - WP 6.3 PHP 7.4 compatible. All CI checks passing. Refs #1417 --- .../tests/Traits/File_Editor_URL_Tests.php | 37 ++++++++++++++++--- 1 file changed, 32 insertions(+), 5 deletions(-) diff --git a/tests/phpunit/tests/Traits/File_Editor_URL_Tests.php b/tests/phpunit/tests/Traits/File_Editor_URL_Tests.php index fb2b1ab89..b52cd9312 100644 --- a/tests/phpunit/tests/Traits/File_Editor_URL_Tests.php +++ b/tests/phpunit/tests/Traits/File_Editor_URL_Tests.php @@ -31,9 +31,36 @@ class File_Editor_URL_Tests extends WP_UnitTestCase { private $cap_callbacks = array(); /** - * Removes only the filter callbacks this test class registered. + * Symlink path inside WP_PLUGIN_DIR for the testdata fixture. + * + * @var string|null + */ + private $fixture_symlink = null; + + /** + * Sets up the test fixture symlink so file_exists passes inside wp-env. + */ + public function set_up() { + parent::set_up(); + + $this->fixture_symlink = WP_PLUGIN_DIR . '/test-plugin-external-admin-menu-links-without-errors'; + $target = UNIT_TESTS_PLUGIN_DIR . 'test-plugin-external-admin-menu-links-without-errors'; + + if ( ! file_exists( $this->fixture_symlink ) && is_dir( $target ) ) { + symlink( $target, $this->fixture_symlink ); + } + } + + /** + * Removes the test fixture symlink and filter callbacks. */ public function tear_down() { + // Remove symlink created in set_up. + if ( null !== $this->fixture_symlink && file_exists( $this->fixture_symlink ) && is_link( $this->fixture_symlink ) ) { + unlink( $this->fixture_symlink ); + } + $this->fixture_symlink = null; + foreach ( $this->url_callbacks as $cb ) { remove_filter( 'wp_plugin_check_validation_error_source_url', $cb, 10 ); } @@ -48,12 +75,12 @@ public function tear_down() { } /** - * Single-file test plugin fixture basename (load.php in fixture dir). + * Absolute path to the single-file test plugin fixture (load.php in fixture dir). * * @return string */ private function single_file_plugin_basename() { - return 'test-plugin-external-admin-menu-links-without-errors/load.php'; + return WP_PLUGIN_DIR . '/test-plugin-external-admin-menu-links-without-errors/load.php'; } /** @@ -191,7 +218,7 @@ static function ( $caps ) { $this->assertArrayHasKey( 'plugin', $query_args ); $this->assertArrayHasKey( 'file', $query_args ); $this->assertArrayNotHasKey( 'line', $query_args ); - $this->assertSame( 'plugin-editor.php', wp_parse_url( $url, PHP_URL_PATH ) ); + $this->assertSame( 'plugin-editor.php', basename( (string) wp_parse_url( $url, PHP_URL_PATH ) ) ); } /** @@ -213,6 +240,6 @@ static function ( $caps ) { $this->assertIsString( $url ); parse_str( (string) wp_parse_url( $url, PHP_URL_QUERY ), $query_args ); $this->assertSame( '42', $query_args['line'] ); - $this->assertSame( 'plugin-editor.php', wp_parse_url( $url, PHP_URL_PATH ) ); + $this->assertSame( 'plugin-editor.php', basename( (string) wp_parse_url( $url, PHP_URL_PATH ) ) ); } } From 42943a7b42fe48600c661e7850ca6826b24a6e14 Mon Sep 17 00:00:00 2001 From: Faisal Ahammad Date: Mon, 27 Jul 2026 00:51:17 +0600 Subject: [PATCH 3/3] fix(tests): grant super admin on multisite for fallback tests On multisite, WP core map_meta_cap('edit_plugins') requires is_super_admin() to grant the cap; a user_has_cap filter bypass only satisfies the check on single-site. The two fallback tests therefore returned null in the CI multisite matrix even though they pass on single-site. Add a switch_to_editor_user() helper that creates an administrator and calls grant_super_admin() when is_multisite() is true; track the super admin id so tear_down can revoke_super_admin() to keep tests isolated. On single-site keep the user_has_cap filter bypass unchanged. Refs #1417 --- .../tests/Traits/File_Editor_URL_Tests.php | 55 +++++++++++++++---- 1 file changed, 43 insertions(+), 12 deletions(-) diff --git a/tests/phpunit/tests/Traits/File_Editor_URL_Tests.php b/tests/phpunit/tests/Traits/File_Editor_URL_Tests.php index b52cd9312..268b3edfd 100644 --- a/tests/phpunit/tests/Traits/File_Editor_URL_Tests.php +++ b/tests/phpunit/tests/Traits/File_Editor_URL_Tests.php @@ -30,6 +30,16 @@ class File_Editor_URL_Tests extends WP_UnitTestCase { */ private $cap_callbacks = array(); + /** + * User IDs granted super-admin status, so tear_down can revoke them. + * + * Multisite `map_meta_cap('edit_plugins')` requires `is_super_admin()`, + * which a `user_has_cap` filter bypass cannot satisfy. + * + * @var int[] + */ + private $super_admin_ids = array(); + /** * Symlink path inside WP_PLUGIN_DIR for the testdata fixture. * @@ -71,6 +81,11 @@ public function tear_down() { } $this->cap_callbacks = array(); + foreach ( $this->super_admin_ids as $id ) { + revoke_super_admin( $id ); + } + $this->super_admin_ids = array(); + parent::tear_down(); } @@ -104,6 +119,32 @@ private function add_cap_filter( $callback ) { $this->cap_callbacks[] = $callback; } + /** + * Sets the current user so `current_user_can('edit_plugins')` returns true. + * + * On multisite, WP core requires `is_super_admin()` for the `edit_plugins` + * cap (see WP capabilities.php `map_meta_cap`). A `user_has_cap` filter + * bypasses that check on single-site only, so the test must escalate the + * user with `grant_super_admin()` when multisite is active. + */ + private function switch_to_editor_user() { + $user_id = self::factory()->user->create( array( 'role' => 'administrator' ) ); + + if ( is_multisite() ) { + grant_super_admin( $user_id ); + $this->super_admin_ids[] = $user_id; + } else { + $this->add_cap_filter( + static function ( $caps ) { + $caps['edit_plugins'] = true; + return $caps; + } + ); + } + + wp_set_current_user( $user_id ); + } + /** * When no filter is registered and the user lacks edit_plugins cap, returns null. */ @@ -201,12 +242,7 @@ static function ( $url, $source ) use ( &$captured ) { * When `$line === 0`, the fallback URL must omit the `line` query arg entirely. */ public function test_fallback_to_plugin_editor_when_no_filter() { - $this->add_cap_filter( - static function ( $caps ) { - $caps['edit_plugins'] = true; - return $caps; - } - ); + $this->switch_to_editor_user(); $context = new Check_Context( $this->single_file_plugin_basename() ); $result = new Check_Result( $context ); @@ -225,12 +261,7 @@ static function ( $caps ) { * Plugin-editor fallback includes `line` query arg when line is set. */ public function test_fallback_to_plugin_editor_includes_line_when_set() { - $this->add_cap_filter( - static function ( $caps ) { - $caps['edit_plugins'] = true; - return $caps; - } - ); + $this->switch_to_editor_user(); $context = new Check_Context( $this->single_file_plugin_basename() ); $result = new Check_Result( $context );