diff -urpN coder/coder.install coder.simpletests.added/coder.install --- coder/coder.install 2008-08-20 19:10:24.000000000 -0400 +++ coder.simpletests.added/coder.install 2008-08-25 21:23:56.000000000 -0400 @@ -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; +} diff -urpN coder/coder.module coder.simpletests.added/coder.module --- coder/coder.module 2008-08-24 07:30:47.000000000 -0400 +++ coder.simpletests.added/coder.module 2008-08-25 21:23:56.000000000 -0400 @@ -961,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) { @@ -968,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); } } @@ -976,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. @@ -1494,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] ++; } @@ -1685,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 /** @@ -1694,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')), ); @@ -1724,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 diff -urpN coder/CVS/Entries coder.simpletests.added/CVS/Entries --- coder/CVS/Entries 2008-08-26 13:07:28.000000000 -0400 +++ coder.simpletests.added/CVS/Entries 1969-12-31 19:00:00.000000000 -0500 @@ -1,8 +0,0 @@ -/README.txt/1.7/Fri Jun 1 00:47:57 2007//TDRUPAL-6--1 -/coder.css/1.6/Sat Aug 11 04:54:04 2007//TDRUPAL-6--1 -/coder.drush.inc/1.1.4.10/Wed Aug 20 16:10:16 2008//TDRUPAL-6--1 -/coder.info/1.9.2.1/Sun Apr 27 18:35:16 2008//TDRUPAL-6--1 -/coder.install/1.1.4.3/Wed Aug 20 23:10:24 2008//TDRUPAL-6--1 -/coder.js/1.3/Sat Apr 7 17:28:03 2007//TDRUPAL-6--1 -/coder.module/1.88.2.44/Sun Aug 24 11:30:47 2008//TDRUPAL-6--1 -D diff -urpN coder/CVS/Entries.Log coder.simpletests.added/CVS/Entries.Log --- coder/CVS/Entries.Log 2008-08-26 13:07:29.000000000 -0400 +++ coder.simpletests.added/CVS/Entries.Log 1969-12-31 19:00:00.000000000 -0500 @@ -1,6 +0,0 @@ -A D/coder_format//// -A D/images//// -A D/includes//// -A D/scripts//// -A D/tests//// -R D/coder_format//// diff -urpN coder/CVS/Repository coder.simpletests.added/CVS/Repository --- coder/CVS/Repository 2008-08-26 13:07:28.000000000 -0400 +++ coder.simpletests.added/CVS/Repository 1969-12-31 19:00:00.000000000 -0500 @@ -1 +0,0 @@ -contributions/modules/coder diff -urpN coder/CVS/Root coder.simpletests.added/CVS/Root --- coder/CVS/Root 2008-08-26 13:07:28.000000000 -0400 +++ coder.simpletests.added/CVS/Root 1969-12-31 19:00:00.000000000 -0500 @@ -1 +0,0 @@ -:pserver:anonymous:anonymous@cvs.drupal.org:/cvs/drupal-contrib diff -urpN coder/CVS/Tag coder.simpletests.added/CVS/Tag --- coder/CVS/Tag 2008-08-26 13:07:28.000000000 -0400 +++ coder.simpletests.added/CVS/Tag 1969-12-31 19:00:00.000000000 -0500 @@ -1 +0,0 @@ -TDRUPAL-6--1 diff -urpN coder/includes/coder_50.inc coder.simpletests.added/includes/coder_50.inc --- coder/includes/coder_50.inc 2008-01-30 05:44:48.000000000 -0500 +++ coder.simpletests.added/includes/coder_50.inc 2008-08-25 21:39:09.000000000 -0400 @@ -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( diff -urpN coder/includes/coder_6x.inc coder.simpletests.added/includes/coder_6x.inc --- coder/includes/coder_6x.inc 2008-07-14 09:31:50.000000000 -0400 +++ coder.simpletests.added/includes/coder_6x.inc 2008-08-25 21:39:25.000000000 -0400 @@ -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.'), diff -urpN coder/includes/coder_style.inc coder.simpletests.added/includes/coder_style.inc --- coder/includes/coder_style.inc 2008-07-14 09:36:29.000000000 -0400 +++ coder.simpletests.added/includes/coder_style.inc 2008-08-25 21:43:23.000000000 -0400 @@ -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)); } } diff -urpN coder/tests/coder_style.test coder.simpletests.added/tests/coder_style.test --- coder/tests/coder_style.test 1969-12-31 19:00:00.000000000 -0500 +++ coder.simpletests.added/tests/coder_style.test 2008-08-25 22:52:17.000000000 -0400 @@ -0,0 +1,72 @@ + 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 tests + '$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 tests + '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 tests + 'print \'
\';' => CODER_NOT_OK, + '?>
CODER_NOT_OK, + // heredoc tests + '$var = <<< __EOD__' => CODER_NOT_OK, // a php error and not a camelCase error + '__EOD__' => CODER_NOT_OK, // + // dot tests + 'if ($file = file_check_upload($fieldname . \'_upload\')) {}' => CODER_NOT_OK, // Not ok + '$v .= \'bugger\';' => CODER_OK, // Ok. + '$a = $v .\'bugger\';' => CODER_OK, // Ok. + '$a = $v .\"bugger\";' => CODER_OK, // Ok, but will throw performance warning. + '$a = $v.\'bugger\';' => CODER_NOT_OK, // Not ok. + '$a = $some_func().\'bugger\';' => CODER_NOT_OK, // Not ok. + '$a = 1.0 * .1 * 1. * (0.1) * (1.) * (.1) * (1.0);' => CODER_OK, // Ok. + // array indexes tests + '$a[hello] = \'this is bad\';' => CODER_NOT_OK, + '$a[\'hello\'] = \'this is no\';' => CODER_NOT_OK, + // trailing spaces + '$left = \'this is bad\';' => CODER_NOT_OK, + '$left = \'this is ok\';' => CODER_OK, + // control structures tests + /* Not sure how to implement this one */ + + // lower case true tests + '$a = TRUE;' => CODER_OK, // Ok. + '$atrue = "true";' => CODER_OK, // Ok. + '$a = true;' => CODER_NOT_OK, // Not ok. + '$a =true;' => CODER_NOT_OK, // Not ok. + 'if ($a == true) {' => CODER_NOT_OK, // Not ok. + 'return false;' => CODER_NOT_OK, // Not ok. + '}' => CODER_OK, + ); + } +}