? 281399.patch Index: coder.install =================================================================== RCS file: /cvs/drupal-contrib/contributions/modules/coder/Attic/coder.install,v retrieving revision 1.1.4.3 diff -u -u -p -r1.1.4.3 coder.install --- coder.install 20 Aug 2008 23:10:24 -0000 1.1.4.3 +++ coder.install 24 Aug 2008 11:25:44 -0000 @@ -15,3 +15,9 @@ function coder_uninstall() { cache_clear_all('coder:', 'cache', true); } +function coder_update_1() { + $ret = array(); + // This update adds a theming function, so we need to clear the entire cache. + $ret[] = update_sql("DELETE FROM {cache}"); + return $ret; +} Index: coder.module =================================================================== RCS file: /cvs/drupal-contrib/contributions/modules/coder/coder.module,v retrieving revision 1.88.2.43 diff -u -u -p -r1.88.2.43 coder.module --- coder.module 20 Aug 2008 23:12:20 -0000 1.88.2.43 +++ coder.module 24 Aug 2008 11:25:44 -0000 @@ -959,6 +961,7 @@ function do_coder_reviews($coder_args) { // Read the file. if (_coder_read_and_parse_file($coder_args)) { // Do all of the code reviews. + $errors = array(); foreach ($coder_args['#reviews'] as $review) { if ($result = do_coder_review($coder_args, $review)) { foreach (array('critical', 'normal', 'minor') as $severity_level) { @@ -966,7 +969,14 @@ function do_coder_reviews($coder_args) { $results['#stats'][$severity_level] += $result['#stats'][$severity_level]; } } - $results += $result; + $errors += $result; + } + } + + // Theme the error messages. + foreach ($errors as $key => $error) { + if (is_numeric($key)) { + $results[$key] = theme('coder_warning_msg', $error); } } @@ -974,7 +984,7 @@ function do_coder_reviews($coder_args) { ksort($results, SORT_NUMERIC); } else { - _coder_error_msg($results, t('Could not read the file'), 'critical'); + $results[] = theme('coder_warning', t('Could not read the file'), 'critical'); } // Save the results in the cache. @@ -1492,51 +1502,13 @@ function do_coder_review_regex(&$coder_a * @param $original * Deprecated. */ -function _coder_error(&$results, $rule, $severity_name, $lineno = -1, $line = '', $original = '') { - if (isset($rule['#warning_callback'])) { - if (function_exists($rule['#warning_callback'])) { - $warning = $rule['#warning_callback'](); - } - else { // If this happens, there is an error in the rule definition. - $warning = t('please report this !warning', - array( - '@report' => 'http://drupal.org/node/add/project_issue/coder/bug', - '!warning' => $rule['#warning_callback'], - ) - ); - } - } - else { - $warning = t($rule['#warning']); - } - - return _coder_error_msg($results, $warning, $severity_name, $lineno, $line); -} - -/** - * Does the actual saving of error to results array and generating its - * unique numeric id. - * - * @param $results - * Results array variable to save errors to. - * @param $warning - * Warning array/string to be themed, returned from '#warning_callback' or - * is a translated string from the rule. See theme_coder_warning() for - * array format. - * @param $severity_name - * String severity of error. - * @param $lineno - * Integer line number error occured on. - * @param $line - * String line contents. - */ -function _coder_error_msg(&$results, $warning, $severity_name, $lineno = -1, $line = '') { +function _coder_error(&$results, $rule, $severity_name, $lineno = -1, $line = '') { // Note: The use of the $key allows multiple errors on one line. // This assumes that no line of source has more than 10000 lines of code // and that we have fewer than 10000 errors. global $_coder_errno; $key = ($lineno + 1) * 10000 + ($_coder_errno ++); - $results[$key] = theme('coder_warning', $warning, $severity_name, $lineno + 1, $line); + $results[$key] = array('rule' => $rule, 'severity_name' => $severity_name, 'lineno' => $lineno, 'line' => $line); $results['#stats'][$severity_name] ++; } @@ -1683,6 +1655,26 @@ function _coder_is_drupal_core($module) return isset($core[$module->name]) ? 1 : 0; } +/** + * Implementation of hook_simpletest(). + */ +function coder_simpletest() { + return array_keys(file_scan_directory(drupal_get_path('module', 'coder') .'/tests', '\.test')); +} + +/** + * Helper function to run the review on the code snippet. + */ +function coder_test($code, $review, $severity = SEVERITY_MINOR) { + $coder_args = array( + '#severity' => $severity, + '#filename' => 'snippet', + '#patch' => $code, + ); + _coder_read_and_parse_file($coder_args); + return do_coder_review($coder_args, $reviews['style']); +} + // Theming functions /** @@ -1692,6 +1684,7 @@ function coder_theme() { return array( 'coder' => array('arguments' => array('name', 'filename', 'results')), 'coder_warning' => array('arguments' => array('warning', 'severity_name', 'lineno', 'line')), + 'coder_warning_msg' => array('arguments' => array('error')), 'cols' => array('arguments' => array('form')), 'drupalapi' => array('arguments' => array('function', 'version')), ); @@ -1722,6 +1715,37 @@ function theme_coder($name, $filename, $ } /** + * Format a coder warning to be included in results, creating the text. + * + * @param $results + * Results array variable to save errors to. + * @param $error + * Error array from _coder_error(). + */ +function theme_coder_warning_msg($error) { + if (isset($error['rule']['#warning_callback'])) { + if (function_exists($error['rule']['#warning_callback'])) { + $warning = $error['rule']['#warning_callback'](); + } + else { // If this happens, there is an error in the rule definition. + $warning = t('please report this !warning', + array( + '@report' => 'http://drupal.org/node/add/project_issue/coder/bug', + '!warning' => $error['rule']['#warning_callback'], + ) + ); + } + } + else { + $warning = t($error['rule']['#warning']); + } + + // @TODO: we can combine theme_coder_warning() and theme_coder_warning_msg(), + // But let's do this on the 7.x upgrade in case anyone's actually using it. + return theme('coder_warning', $warning, $error['severity_name'], $error['lineno'], $error['line']); +} + +/** * Format a coder warning to be included in results. * * @param $warning Index: includes/coder_50.inc =================================================================== RCS file: /cvs/drupal-contrib/contributions/modules/coder/includes/coder_50.inc,v retrieving revision 1.21.2.6 diff -u -u -p -r1.21.2.6 coder_50.inc --- includes/coder_50.inc 30 Jan 2008 10:44:48 -0000 1.21.2.6 +++ includes/coder_50.inc 24 Aug 2008 11:25:44 -0000 @@ -61,6 +61,7 @@ function coder_50_reviews() { array( '#type' => 'callback', '#value' => '_coder_50_callback', + '#warning_callback' => '_coder_50_callback_warning', ), array( '#type' => 'regex', @@ -95,8 +96,7 @@ function _coder_50_callback(&$coder_args $filename = drupal_substr(realpath($filename), 0, -7) .'.info'; if (!file_exists($filename)) { $severity_name = _coder_severity_name($coder_args, $review, $rule); - $warning = t('All modules now need to have a modulename.info file', array('@info' => 'http://drupal.org/node/101009')); - $results[0] = theme('coder_warning', $warning, $severity_name); + _coder_error($results, $rule, $severity_name); } } } @@ -105,6 +105,10 @@ function _coder_50_callback(&$coder_args * Define the warning callbacks. */ +function _coder_50_callback_warning() { + return t('All modules now need to have a modulename.info file', array('@info' => 'http://drupal.org/node/101009')); +} + function _coder_50_user_mail_warning() { return t('!user_mail() is replaced by !drupal_mail()', array( Index: includes/coder_6x.inc =================================================================== RCS file: /cvs/drupal-contrib/contributions/modules/coder/includes/coder_6x.inc,v retrieving revision 1.17.2.51 diff -u -u -p -r1.17.2.51 coder_6x.inc --- includes/coder_6x.inc 14 Jul 2008 13:31:50 -0000 1.17.2.51 +++ includes/coder_6x.inc 24 Aug 2008 11:25:44 -0000 @@ -103,6 +103,8 @@ function coder_6x_reviews() { array( '#type' => 'callback', '#value' => '_coder_6x_callback', + // @NOTE: This is not used. It only exists to catch potential errors in this code. + '#warning_callback' => '_coder_6x_callback_warning', ), // FAPI Rules. array( @@ -485,8 +487,9 @@ function _coder_6x_callback(&$coder_args } if ($theme && !$hook_theme) { $severity_name = _coder_severity_name($coder_args, $review, $rule); - $warning = _coder_6x_hook_theme_warning(); - $results[0] = theme('coder_warning', $warning, $severity_name, $theme_lineno, $theme_line); + $tmprule = $rule; + $tmprule['#warning_callback'] = '_coder_6x_hook_theme_warning'; + _coder_error($results, $tmprule, $severity_name, $theme_lineno, $theme_line); } // Read the .info file. @@ -496,8 +499,9 @@ function _coder_6x_callback(&$coder_args foreach ($lines as $lineno => $line) { if (preg_match('/^dependencies\s*=/', $line)) { $severity_name = _coder_severity_name($coder_args, $review, $rule); - $warning = t('New syntax for .info files, use dependencies[]', array('!uri' => 'http://drupal.org/node/114774#info')); - $results[-1] = theme('coder_warning', $warning, $severity_name, $lineno, $line); + $tmprule = $rule; + $tmprule['#warning_callback'] = '_coder_6x_dependency_warning'; + _coder_error($results, $tmprule, $severity_name, $lineno, $line); } if (preg_match('/^core\s*=/', $line)) { $core = TRUE; @@ -505,8 +509,9 @@ function _coder_6x_callback(&$coder_args } if (!isset($core)) { $severity_name = _coder_severity_name($coder_args, $review, $rule); - $warning = t('New syntax for .info files files requires core=6.x', array('!uri' => 'http://drupal.org/node/114774#info')); - $results[-1] = theme('coder_warning', $warning, $severity_name, $lineno, $line); + $tmprule = $rule; + $tmprule['#warning_callback'] = '_coder_6x_core_warning'; + _coder_error($results, $tmprule, $severity_name, $lineno, $line); } } } @@ -515,11 +520,9 @@ function _coder_6x_callback(&$coder_args $dir = dirname($filename); if (file_exists($dir .'/po') && !file_exists($dir .'/translations')) { $severity_name = _coder_severity_name($coder_args, $review, $rule); - $warning = array( - '#warning' => t('Translations moved from ./po to ./translations'), - '#link' => 'http://drupal.org/node/114774#translations_directory', - ); - $results[-2] = theme('coder_warning', $warning, $severity_name, -1, $dir); + $tmprule = $rule; + $tmprule['#warning_callback'] = '_coder_6x_translations_warning'; + _coder_error($results, $tmprule, $severity_name, -1, $dir); } } } @@ -528,6 +531,27 @@ function _coder_6x_callback(&$coder_args * Define the warning callbacks. */ +function _coder_6x_core_warning() { + return array( + '#warning' => t('New syntax for .info files files requires core=6.x'), + '#link' => 'http://drupal.org/node/114774#info', + ); +} + +function _coder_6x_dependency_warning() { + return array( + '#warning' => t('New syntax for .info files, use dependencies[]'), + '#link' => 'http://drupal.org/node/114774#info', + ); +} + +function _coder_6x_translations_warning() { + return array( + '#warning' => t('Translations moved from ./po to ./translations'), + '#link' => 'http://drupal.org/node/114774#translations_directory', + ); +} + function _coder_6x_new_menu_system_warning() { return array( '#warning' => t('The menu system has been completely over-hauled in 6.x.'), Index: includes/coder_style.inc =================================================================== RCS file: /cvs/drupal-contrib/contributions/modules/coder/includes/coder_style.inc,v retrieving revision 1.16.2.17 diff -u -u -p -r1.16.2.17 coder_style.inc --- includes/coder_style.inc 14 Jul 2008 13:36:29 -0000 1.16.2.17 +++ includes/coder_style.inc 24 Aug 2008 11:25:44 -0000 @@ -71,6 +71,7 @@ function coder_style_reviews() { '#type' => 'callback', '#source' => 'all', '#value' => '_coder_style_callback', + '#warning' => 'the final ?> should be omitted from all code files', ), array( '#type' => 'regex', @@ -165,7 +166,7 @@ function _coder_style_callback(&$coder_a } if ($last && $lastline && preg_match('/\?>\s*$/i', $lastline)) { $severity_name = _coder_severity_name($coder_args, $review, $rule); - _coder_error_msg($results, 'the final ?> should be omitted from all code files', $severity_name, count($lines)); + _coder_error($results, $rule, $severity_name, count($lines)); } } Index: tests/coder_style.test =================================================================== RCS file: tests/coder_style.test diff -N tests/coder_style.test --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ tests/coder_style.test 24 Aug 2008 11:25:44 -0000 @@ -0,0 +1,45 @@ + t('Coder Style Tests'), + 'desc' => t('Tests for the coder style review.'), + 'group' => 'Coder' + ); + } + + function setUp() { + } + + function testStyle() { + $snippets = array( + '$some_array[FOO_BAR] = $baz;' => CODER_OK, + '$some_array[foo_bar] = $baz;' => CODER_NOT_OK, + '// Tab in comment' => CODER_OK, + '$var = "tab in double quote"' => CODER_OK, + '$var = \'tab in single quote\'' => CODER_OK, + ' $var = "tab in line";' => CODER_NOT_OK, + '$var = new stdClass();' => CODER_OK, + '$var = new StdClass();' => CODER_NOT_OK, // This is not. + '$var = new stdclassToo();' => CODER_NOT_OK, // Should be camelcase rule. + // camelCase + '$camelCaseVar = \'whatever\';' => CODER_NOT_OK, // Camel case functions and vars not allowed. + '$var = $obj->camelCase;' => CODER_OK, // But camel case objects elements are. + 'new camcelCase();' => CODER_OK, // Is ok. + // hex number + '$var = 0xFF;' => CODER_OK, // Should NOT be camelcase. + // multiline quote + 'return t(\'

Blocks are boxes of content that may be rendered into certain regions of your web pages, for example, into sidebars. They are usually generated automatically by modules, but administrators can create blocks manually.

+

Only enabled blocks are shown. You can position blocks by specifying which area of the page they should appear in (e.g., a sidebar). Highlighted labels on this page show the regions into which blocks can be rendered. You can specify where within a region a block will appear by adjusting its weight.

+

If you want certain blocks to disable themselves temporarily during high server loads, check the "Throttle" box. You can configure the auto-throttle on the throttle configuration page after having enabled the throttle module.

+

You can configure the behaviour of each block (for example, specifying on which pages and for what users it will appear) by clicking the "configure" link for each block.

\', array(\'@throttle\' => url(\'admin/settings/throttle\')));' => CODER_OK, + // break + 'print \'
\';' => CODER_NOT_OK, + '?>
CODER_NOT_OK, + // heredoc + // dot + ); + + $reviews = coder_reviews(); + foreach ($snippets as $code => $status) { + $coder_args = array( + '#severity' => SEVERITY_MINOR, + '#filename' => 'coder_style.test', + '#patch' => $code, + ); + _coder_read_and_parse_file($coder_args); + $results = do_coder_review($coder_args, $reviews['style']); + + if ($status == CODER_OK) { + $this->assertTrue(count($results) == 1, 'Expect NO warning: '. $code); + } + else { + $this->assertTrue(count($results) > 1, 'Expect a warning: '. $code); + } + } + } +}