From dedd95e20aab9c7b12d1b3896ea65c2778eface2 Mon Sep 17 00:00:00 2001 From: Mukesh Panchal Date: Tue, 1 Aug 2023 16:07:21 +0530 Subject: [PATCH 1/2] Fix readme error file name --- .../Checker/Checks/Plugin_Readme_Check.php | 73 ++++++++++++------- .../Checks/Plugin_Readme_Check_Tests.php | 38 ++++++++++ .../load.php | 16 ++++ .../readme.md | 13 ++++ 4 files changed, 115 insertions(+), 25 deletions(-) create mode 100644 tests/phpunit/testdata/plugins/test-plugin-plugin-readme-md-with-errors/load.php create mode 100644 tests/phpunit/testdata/plugins/test-plugin-plugin-readme-md-with-errors/readme.md diff --git a/includes/Checker/Checks/Plugin_Readme_Check.php b/includes/Checker/Checks/Plugin_Readme_Check.php index 34895f5a5..68d388b42 100644 --- a/includes/Checker/Checks/Plugin_Readme_Check.php +++ b/includes/Checker/Checks/Plugin_Readme_Check.php @@ -12,7 +12,7 @@ use WordPress\Plugin_Check\Traits\Stable_Check; /** - * Check the plugins readme.txt file and contents. + * Check the plugins readme file and contents. * * @since n.e.x.t */ @@ -34,7 +34,7 @@ public function get_categories() { } /** - * Check the readme.txt file. + * Check the readme file. * * @since n.e.x.t * @@ -45,7 +45,7 @@ protected function check_files( Check_Result $result, array $files ) { // Find the readme file. $readme = self::filter_files_by_regex( $files, '/readme\.(txt|md)$/i' ); - // If the readme.txt does not exist, add a warning and skip other tests. + // If the readme file does not exist, add a warning and skip other tests. if ( empty( $readme ) ) { $result->add_message( false, @@ -59,18 +59,18 @@ protected function check_files( Check_Result $result, array $files ) { return; } - // Check the readme.txt for default text. + // Check the readme file for default text. $this->check_default_text( $result, $readme ); - // Check the readme.txt for a valid license. + // Check the readme file for a valid license. $this->check_license( $result, $readme ); - // Check the readme.txt for a valid version. + // Check the readme file for a valid version. $this->check_stable_tag( $result, $readme ); } /** - * Checks the readme.txt for default text. + * Checks the readme file for default text. * * @since n.e.x.t * @@ -78,24 +78,38 @@ protected function check_files( Check_Result $result, array $files ) { * @param array $files Array of plugin files. */ private function check_default_text( Check_Result $result, array $files ) { - if ( - self::file_str_contains( $files, 'Here is a short description of the plugin.' ) || - self::file_str_contains( $files, 'Tags: tag1' ) || - self::file_str_contains( $files, 'Donate link: http://example.com/' ) - ) { + $default_text_patterns = array( + 'Here is a short description of the plugin.', + 'Tags: tag1', + 'Donate link: http://example.com/', + ); + + $file = ''; + foreach ( $default_text_patterns as $pattern ) { + $file = self::file_str_contains( $files, $pattern ); + if ( $file ) { + break; + } + } + + if ( $file ) { $result->add_message( false, - __( 'The readme.txt appears to contain default text.', 'plugin-check' ), + sprintf( + /* translators: %s: readme file */ + __( 'The %s appears to contain default text.', 'plugin-check' ), + str_replace( $result->plugin()->path( '/' ), '', $file ) + ), array( 'code' => 'default_readme_text', - 'file' => $result->plugin()->path( '/readme.txt' ), + 'file' => $file, ) ); } } /** - * Checks the readme.txt for a valid license. + * Checks the readme file for a valid license. * * @since n.e.x.t * @@ -105,7 +119,7 @@ private function check_default_text( Check_Result $result, array $files ) { private function check_license( Check_Result $result, array $files ) { $matches = array(); // Get the license from the readme file. - self::file_preg_match( '/(License:|License URI:)\s*(.+)*/i', $files, $matches ); + $file = self::file_preg_match( '/(License:|License URI:)\s*(.+)*/i', $files, $matches ); if ( empty( $matches ) ) { return; @@ -115,17 +129,21 @@ private function check_license( Check_Result $result, array $files ) { if ( ! preg_match( '/^([a-z0-9\-\+\.]+)(\sor\s([a-z0-9\-\+\.]+))*$/i', $matches[2] ) ) { $result->add_message( false, - __( 'Your plugin has an invalid license declared. Please update your readme.txt with a valid SPDX license identifier.', 'plugin-check' ), + sprintf( + /* translators: %s: readme file */ + __( 'Your plugin has an invalid license declared. Please update your %s with a valid SPDX license identifier.', 'plugin-check' ), + str_replace( $result->plugin()->path( '/' ), '', $file ) + ), array( 'code' => 'invalid_license', - 'file' => $result->plugin()->path( '/readme.txt' ), + 'file' => $file, ) ); } } /** - * Checks the readme.txt stable tag. + * Checks the readme file stable tag. * * @since n.e.x.t * @@ -134,8 +152,9 @@ private function check_license( Check_Result $result, array $files ) { */ private function check_stable_tag( Check_Result $result, array $files ) { $matches = array(); - // Get the readme.txt Stable tag. - if ( ! self::file_preg_match( '/Stable tag:\s*([a-z0-9\.]+)/i', $files, $matches ) ) { + // Get the Stable tag from readme file. + $file = self::file_preg_match( '/Stable tag:\s*([a-z0-9\.]+)/i', $files, $matches ); + if ( ! $file ) { return; } @@ -147,12 +166,12 @@ private function check_stable_tag( Check_Result $result, array $files ) { __( "It's recommended not to use 'Stable Tag: trunk'.", 'plugin-check' ), array( 'code' => 'trunk_stable_tag', - 'file' => $result->plugin()->path( '/readme.txt' ), + 'file' => $file, ) ); } - // Check the readme.txt Stable tag against the plugin's main file version. + // Check the readme file Stable tag against the plugin's main file version. $plugin_data = get_plugin_data( WP_PLUGIN_DIR . '/' . $result->plugin()->basename() ); if ( @@ -161,10 +180,14 @@ private function check_stable_tag( Check_Result $result, array $files ) { ) { $result->add_message( false, - __( 'The Stable Tag in your readme.txt file does not match the version in your main plugin file.', 'plugin-check' ), + sprintf( + /* translators: %s: readme file */ + __( 'The Stable Tag in your %s file does not match the version in your main plugin file.', 'plugin-check' ), + str_replace( $result->plugin()->path( '/' ), '', $file ) + ), array( 'code' => 'stable_tag_mismatch', - 'file' => $result->plugin()->path( '/readme.txt' ), + 'file' => $file, ) ); } diff --git a/tests/phpunit/Checker/Checks/Plugin_Readme_Check_Tests.php b/tests/phpunit/Checker/Checks/Plugin_Readme_Check_Tests.php index 1c94c1f09..bbdd93946 100644 --- a/tests/phpunit/Checker/Checks/Plugin_Readme_Check_Tests.php +++ b/tests/phpunit/Checker/Checks/Plugin_Readme_Check_Tests.php @@ -128,4 +128,42 @@ public function test_run_md_without_errors() { $this->assertEquals( 0, $check_result->get_error_count() ); $this->assertEquals( 0, $check_result->get_warning_count() ); } + + public function test_run_md_with_errors() { + $readme_check = new Plugin_Readme_Check(); + $check_context = new Check_Context( UNIT_TESTS_PLUGIN_DIR . 'test-plugin-plugin-readme-md-with-errors/load.php' ); + $check_result = new Check_Result( $check_context ); + + $readme_check->run( $check_result ); + + $warnings = $check_result->get_warnings(); + + $this->assertNotEmpty( $warnings ); + $this->assertArrayHasKey( 'readme.md', $warnings ); + $this->assertEquals( 4, $check_result->get_warning_count() ); + + // Check for default text file warning. + $this->assertArrayHasKey( 0, $warnings['readme.md'] ); + $this->assertArrayHasKey( 0, $warnings['readme.md'][0] ); + $this->assertArrayHasKey( 'code', $warnings['readme.md'][0][0][0] ); + $this->assertEquals( 'default_readme_text', $warnings['readme.md'][0][0][0]['code'] ); + + // Check for invalid license warning. + $this->assertArrayHasKey( 0, $warnings['readme.md'] ); + $this->assertArrayHasKey( 0, $warnings['readme.md'][0] ); + $this->assertArrayHasKey( 'code', $warnings['readme.md'][0][0][1] ); + $this->assertEquals( 'invalid_license', $warnings['readme.md'][0][0][1]['code'] ); + + // Check for trunk stable tag warning. + $this->assertArrayHasKey( 0, $warnings['readme.md'] ); + $this->assertArrayHasKey( 0, $warnings['readme.md'][0] ); + $this->assertArrayHasKey( 'code', $warnings['readme.md'][0][0][2] ); + $this->assertEquals( 'trunk_stable_tag', $warnings['readme.md'][0][0][2]['code'] ); + + // Check for stable tag mismatch file warning. + $this->assertArrayHasKey( 0, $warnings['readme.md'] ); + $this->assertArrayHasKey( 0, $warnings['readme.md'][0] ); + $this->assertArrayHasKey( 'code', $warnings['readme.md'][0][0][3] ); + $this->assertEquals( 'stable_tag_mismatch', $warnings['readme.md'][0][0][3]['code'] ); + } } diff --git a/tests/phpunit/testdata/plugins/test-plugin-plugin-readme-md-with-errors/load.php b/tests/phpunit/testdata/plugins/test-plugin-plugin-readme-md-with-errors/load.php new file mode 100644 index 000000000..9677ccd1a --- /dev/null +++ b/tests/phpunit/testdata/plugins/test-plugin-plugin-readme-md-with-errors/load.php @@ -0,0 +1,16 @@ + Date: Wed, 2 Aug 2023 12:21:57 +0530 Subject: [PATCH 2/2] Apply suggestions from code review Co-authored-by: Felix Arntz --- .../Checker/Checks/Plugin_Readme_Check.php | 26 +++++-------------- 1 file changed, 7 insertions(+), 19 deletions(-) diff --git a/includes/Checker/Checks/Plugin_Readme_Check.php b/includes/Checker/Checks/Plugin_Readme_Check.php index 68d388b42..8ce44fb8b 100644 --- a/includes/Checker/Checks/Plugin_Readme_Check.php +++ b/includes/Checker/Checks/Plugin_Readme_Check.php @@ -95,14 +95,10 @@ private function check_default_text( Check_Result $result, array $files ) { if ( $file ) { $result->add_message( false, - sprintf( - /* translators: %s: readme file */ - __( 'The %s appears to contain default text.', 'plugin-check' ), - str_replace( $result->plugin()->path( '/' ), '', $file ) - ), + __( 'The readme appears to contain default text.', 'plugin-check' ), array( 'code' => 'default_readme_text', - 'file' => $file, + 'file' => str_replace( $result->plugin()->path(), '', $file ), ) ); } @@ -129,14 +125,10 @@ private function check_license( Check_Result $result, array $files ) { if ( ! preg_match( '/^([a-z0-9\-\+\.]+)(\sor\s([a-z0-9\-\+\.]+))*$/i', $matches[2] ) ) { $result->add_message( false, - sprintf( - /* translators: %s: readme file */ - __( 'Your plugin has an invalid license declared. Please update your %s with a valid SPDX license identifier.', 'plugin-check' ), - str_replace( $result->plugin()->path( '/' ), '', $file ) - ), + __( 'Your plugin has an invalid license declared. Please update your readme with a valid SPDX license identifier.', 'plugin-check' ), array( 'code' => 'invalid_license', - 'file' => $file, + 'file' => str_replace( $result->plugin()->path(), '', $file ), ) ); } @@ -166,7 +158,7 @@ private function check_stable_tag( Check_Result $result, array $files ) { __( "It's recommended not to use 'Stable Tag: trunk'.", 'plugin-check' ), array( 'code' => 'trunk_stable_tag', - 'file' => $file, + 'file' => str_replace( $result->plugin()->path(), '', $file ), ) ); } @@ -180,14 +172,10 @@ private function check_stable_tag( Check_Result $result, array $files ) { ) { $result->add_message( false, - sprintf( - /* translators: %s: readme file */ - __( 'The Stable Tag in your %s file does not match the version in your main plugin file.', 'plugin-check' ), - str_replace( $result->plugin()->path( '/' ), '', $file ) - ), + __( 'The Stable Tag in your readme file does not match the version in your main plugin file.', 'plugin-check' ), array( 'code' => 'stable_tag_mismatch', - 'file' => $file, + 'file' => str_replace( $result->plugin()->path(), '', $file ), ) ); }