diff --git a/includes/Checker/Checks/General/Error_Reporting_Check.php b/includes/Checker/Checks/General/Error_Reporting_Check.php new file mode 100644 index 000000000..320409d84 --- /dev/null +++ b/includes/Checker/Checks/General/Error_Reporting_Check.php @@ -0,0 +1,105 @@ + 'php', + 'standard' => 'PluginCheck', + 'sniffs' => 'PluginCheck.CodeAnalysis.ErrorReporting', + ); + } + + /** + * Gets the description for the check. + * + * Every check must have a short description explaining what the check does. + * + * @since 1.9.0 + * + * @return string Description. + */ + public function get_description(): string { + return __( 'Checks if the plugin changes error reporting or debug configurations.', 'plugin-check' ); + } + + /** + * Gets the documentation URL for the check. + * + * Every check must have a URL with further information about the check. + * + * @since 1.9.0 + * + * @return string The documentation URL. + */ + public function get_documentation_url(): string { + return __( 'https://developer.wordpress.org/plugins/wordpress-org/detailed-plugin-guidelines/', 'plugin-check' ); + } + + /** + * Amends the given result with a message for the specified file, including error information. + * + * @since 1.9.0 + * + * @param Check_Result $result The check result to amend, including the plugin context to check. + * @param bool $error Whether it is an error or notice. + * @param string $message Error message. + * @param string $code Error code. + * @param string $file Absolute path to the file where the issue was found. + * @param int $line The line on which the message occurred. Default is 0 (unknown line). + * @param int $column The column on which the message occurred. Default is 0 (unknown column). + * @param string $docs URL for further information about the message. + * @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 ) { + // Set high priority severity. + $severity = 7; + + parent::add_result_message_for_file( $result, $error, $message, $code, $file, $line, $column, $docs, $severity ); + } +} diff --git a/includes/Checker/Default_Check_Repository.php b/includes/Checker/Default_Check_Repository.php index c22371044..16bb29e4b 100644 --- a/includes/Checker/Default_Check_Repository.php +++ b/includes/Checker/Default_Check_Repository.php @@ -72,6 +72,7 @@ private function register_default_checks() { 'wp_plugin_check_checks', array( 'i18n_usage' => new Checks\General\I18n_Usage_Check(), + 'error_reporting' => new Checks\General\Error_Reporting_Check(), 'enqueued_scripts_size' => new Checks\Performance\Enqueued_Scripts_Size_Check(), 'enqueued_styles_size' => new Checks\Performance\Enqueued_Styles_Size_Check(), 'code_obfuscation' => new Checks\Plugin_Repo\Code_Obfuscation_Check(), diff --git a/phpcs-sniffs/PluginCheck/Sniffs/CodeAnalysis/ErrorReportingSniff.php b/phpcs-sniffs/PluginCheck/Sniffs/CodeAnalysis/ErrorReportingSniff.php new file mode 100644 index 000000000..4d31f0395 --- /dev/null +++ b/phpcs-sniffs/PluginCheck/Sniffs/CodeAnalysis/ErrorReportingSniff.php @@ -0,0 +1,127 @@ + + */ + public function register() { + return array( T_STRING ); + } + + /** + * Processes this test, when one of its tokens is encountered. + * + * @since 1.9.0 + * + * @param int $stackPtr The position of the current token in the stack. + * @return int|void Integer stack pointer to skip forward or void to continue normal file processing. + */ + public function process_token( $stackPtr ) { + $content = strtolower( $this->tokens[ $stackPtr ]['content'] ); + + // 1. Check for error_reporting(...) call. + if ( 'error_reporting' === $content ) { + if ( $this->is_target_function( $stackPtr ) ) { + $this->phpcsFile->addWarning( + 'Changing error reporting configuration via error_reporting() is discouraged.', + $stackPtr, + 'ChangingErrorReportingFound' + ); + } + return; + } + + // 2. Check for ini_set(...) or ini_alter(...) call. + if ( in_array( $content, array( 'ini_set', 'ini_alter' ), true ) ) { + if ( $this->is_target_function( $stackPtr ) ) { + $parameters = PassedParameters::getParameters( $this->phpcsFile, $stackPtr ); + $first_param = PassedParameters::getParameterFromStack( $parameters, 1, 'option' ); + if ( false !== $first_param ) { + $param_value = TextStrings::stripQuotes( $first_param['clean'] ); + if ( in_array( $param_value, array( 'error_reporting', 'display_errors' ), true ) ) { + $this->phpcsFile->addWarning( + 'Changing error reporting configuration via %s() is discouraged.', + $stackPtr, + 'ChangingIniSettingFound', + array( $this->tokens[ $stackPtr ]['content'] ) + ); + } + } + } + return; + } + + // 3. Check for define(...) call. + if ( 'define' === $content ) { + if ( $this->is_target_function( $stackPtr ) ) { + $parameters = PassedParameters::getParameters( $this->phpcsFile, $stackPtr ); + $first_param = PassedParameters::getParameterFromStack( $parameters, 1, 'constant_name' ); + if ( false !== $first_param ) { + $param_value = TextStrings::stripQuotes( $first_param['clean'] ); + $debug_constants = array( + 'WP_DEBUG', + 'WP_DEBUG_LOG', + 'SCRIPT_DEBUG', + 'WP_DEBUG_DISPLAY', + ); + if ( in_array( $param_value, $debug_constants, true ) ) { + $this->phpcsFile->addWarning( + 'Defining %s in plugin code is discouraged.', + $stackPtr, + 'ChangingDebugConstantFound', + array( $param_value ) + ); + } + } + } + return; + } + } + + /** + * Determine if the token is a function call. + * + * @since 1.9.0 + * + * @param int $stackPtr The position of the current token in the stack. + * @return bool True if a function call, false otherwise. + */ + private function is_target_function( $stackPtr ) { + // Ensure it has an opening parenthesis next (ignoring whitespace). + $next = $this->phpcsFile->findNext( Tokens::$emptyTokens, ( $stackPtr + 1 ), null, true ); + if ( false === $next || T_OPEN_PARENTHESIS !== $this->tokens[ $next ]['code'] ) { + return false; + } + + // Ensure it's not a method call or static method call. + $prev = $this->phpcsFile->findPrevious( Tokens::$emptyTokens, ( $stackPtr - 1 ), null, true ); + if ( false !== $prev && in_array( $this->tokens[ $prev ]['code'], array( T_OBJECT_OPERATOR, T_DOUBLE_COLON ), true ) ) { + return false; + } + + return true; + } +} diff --git a/phpcs-sniffs/PluginCheck/ruleset.xml b/phpcs-sniffs/PluginCheck/ruleset.xml index 18666cc40..55b16302e 100644 --- a/phpcs-sniffs/PluginCheck/ruleset.xml +++ b/phpcs-sniffs/PluginCheck/ruleset.xml @@ -10,6 +10,7 @@ + diff --git a/tests/phpunit/testdata/plugins/test-plugin-error-reporting-with-errors/load.php b/tests/phpunit/testdata/plugins/test-plugin-error-reporting-with-errors/load.php new file mode 100644 index 000000000..744a211ed --- /dev/null +++ b/tests/phpunit/testdata/plugins/test-plugin-error-reporting-with-errors/load.php @@ -0,0 +1,12 @@ +run( $check_result ); + + $warnings = $check_result->get_warnings(); + + $this->assertNotEmpty( $warnings ); + $this->assertArrayHasKey( 'load.php', $warnings ); + + $file_warnings = $warnings['load.php']; + + // Assert we caught all error_reporting, ini_set, ini_alter, define calls. + $codes = array(); + foreach ( $file_warnings as $line => $columns ) { + foreach ( $columns as $column => $messages ) { + foreach ( $messages as $message ) { + $codes[] = $message['code']; + } + } + } + + $this->assertContains( 'PluginCheck.CodeAnalysis.ErrorReporting.ChangingErrorReportingFound', $codes ); + $this->assertContains( 'PluginCheck.CodeAnalysis.ErrorReporting.ChangingIniSettingFound', $codes ); + $this->assertContains( 'PluginCheck.CodeAnalysis.ErrorReporting.ChangingDebugConstantFound', $codes ); + } + + /** + * Test running the check on a plugin without error reporting configuration modifications. + * + * @since 1.9.0 + */ + public function test_run_without_errors() { + $check = new Error_Reporting_Check(); + $context = new Check_Context( UNIT_TESTS_PLUGIN_DIR . 'test-plugin-error-reporting-without-errors/load.php' ); + $check_result = new Check_Result( $context ); + + $check->run( $check_result ); + + $this->assertEmpty( $check_result->get_errors() ); + $this->assertEmpty( $check_result->get_warnings() ); + } +}