enhancement(checks): add check for changing error reporting configuration

includes/Checker/Checks/General/Error_Reporting_Check.php

@@ -0,0 +1,105 @@
+<?php
+/**
+ * Class Error_Reporting_Check.
+ *
+ * @package plugin-check
+ */
+
+namespace WordPress\Plugin_Check\Checker\Checks\General;
+
+use WordPress\Plugin_Check\Checker\Check_Categories;
+use WordPress\Plugin_Check\Checker\Check_Result;
+use WordPress\Plugin_Check\Checker\Checks\Abstract_PHP_CodeSniffer_Check;
+use WordPress\Plugin_Check\Traits\Amend_Check_Result;
+use WordPress\Plugin_Check\Traits\Stable_Check;
+
+/**
+ * Check for detecting modifications to PHP/WordPress debug and error-reporting settings.
+ *
+ * @since 1.9.0
+ */
+class Error_Reporting_Check extends Abstract_PHP_CodeSniffer_Check {
+
+	use Amend_Check_Result;
+	use Stable_Check;
+
+	/**
+	 * Gets the categories for the check.
+	 *
+	 * Every check must have at least one category.
+	 *
+	 * @since 1.9.0
+	 *
+	 * @return array The categories for the check.
+	 */
+	public function get_categories() {
+		return array(
+			Check_Categories::CATEGORY_GENERAL,
+			Check_Categories::CATEGORY_PLUGIN_REPO,
+		);
+	}
+
+	/**
+	 * Returns an associative array of arguments to pass to PHPCS.
+	 *
+	 * @since 1.9.0
+	 *
+	 * @param Check_Result $result The check result to amend, including the plugin context to check.
+	 * @return array An associative array of PHPCS CLI arguments.
+	 */
+	protected function get_args( Check_Result $result ) {
+		return array(
+			'extensions' => '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 );
+	}
+}

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(),

phpcs-sniffs/PluginCheck/Sniffs/CodeAnalysis/ErrorReportingSniff.php

@@ -0,0 +1,127 @@
+<?php
+/**
+ * ErrorReportingSniff
+ *
+ * @package PluginCheck
+ */
+
+namespace PluginCheckCS\PluginCheck\Sniffs\CodeAnalysis;
+
+use PHP_CodeSniffer\Util\Tokens;
+use PHPCSUtils\Utils\MessageHelper;
+use PHPCSUtils\Utils\PassedParameters;
+use PHPCSUtils\Utils\TextStrings;
+use WordPressCS\WordPress\Sniff;
+
+/**
+ * Detects changes to error reporting and debug configurations.
+ *
+ * @since 1.9.0
+ */
+final class ErrorReportingSniff extends Sniff {
+
+	/**
+	 * Returns an array of tokens this test wants to listen for.
+	 *
+	 * @since 1.9.0
+	 *
+	 * @return array<int>
+	 */
+	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;
+	}
+}

phpcs-sniffs/PluginCheck/ruleset.xml

@@ -10,6 +10,7 @@
 	<rule ref="PluginCheck.CodeAnalysis.Offloading" />
 	<rule ref="PluginCheck.CodeAnalysis.RequiredFunctionParameters" />
 	<rule ref="PluginCheck.CodeAnalysis.SettingSanitization" />
+	<rule ref="PluginCheck.CodeAnalysis.ErrorReporting" />
 	<rule ref="PluginCheck.Security.VerifyNonce" />
 	<rule ref="PluginCheck.Security.DirectDB" />
 

tests/phpunit/testdata/plugins/test-plugin-error-reporting-with-errors/load.php

@@ -0,0 +1,12 @@
+<?php
+/**
+ * Plugin Name: Test Plugin Error Reporting With Errors
+ */
+
+error_reporting(0);
+ini_set('display_errors', 1);
+ini_alter('error_reporting', '0');
+define('WP_DEBUG', true);
+define('WP_DEBUG_LOG', true);
+define('SCRIPT_DEBUG', true);
+define('WP_DEBUG_DISPLAY', false);

tests/phpunit/testdata/plugins/test-plugin-error-reporting-without-errors/load.php

@@ -0,0 +1,17 @@
+<?php
+/**
+ * Plugin Name: Test Plugin Error Reporting Without Errors
+ */
+
+// error_reporting(0);
+// ini_set('display_errors', 1);
+// define('WP_DEBUG', true);
+
+$error_reporting = 'some_val';
+$ini_set = 'another_val';
+
+// Checking constants or function names in docstrings/comments should not trigger
+/**
+ * ini_set('display_errors', 1);
+ * define('WP_DEBUG', true);
+ */

tests/phpunit/tests/Checker/Checks/Error_Reporting_Check_Tests.php

@@ -0,0 +1,63 @@
+<?php
+/**
+ * Tests for the Error_Reporting_Check class.
+ *
+ * @package plugin-check
+ */
+
+use WordPress\Plugin_Check\Checker\Check_Context;
+use WordPress\Plugin_Check\Checker\Check_Result;
+use WordPress\Plugin_Check\Checker\Checks\General\Error_Reporting_Check;
+
+class Error_Reporting_Check_Tests extends WP_UnitTestCase {
+
+	/**
+	 * Test running the check on a plugin with error reporting configuration modifications.
+	 *
+	 * @since 1.9.0
+	 */
+	public function test_run_with_errors() {
+		$check        = new Error_Reporting_Check();
+		$context      = new Check_Context( UNIT_TESTS_PLUGIN_DIR . 'test-plugin-error-reporting-with-errors/load.php' );
+		$check_result = new Check_Result( $context );
+
+		$check->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() );
+	}
+}