From 24cba89c4d4a7bedb319d27815d769572c2714d1 Mon Sep 17 00:00:00 2001 From: Jared Hancock <jared@osticket.com> Date: Sat, 12 Oct 2013 03:45:51 +0000 Subject: [PATCH] Address various lint related bugs, typos, and hints Including adding support to the TCL uninitialized variable reader to ignore class-static variable access as well as detect inline functions and their closure arguments. --- include/ajax.draft.php | 10 ++++---- include/class.dynamic_forms.php | 5 ++-- include/class.file.php | 2 +- include/class.forms.php | 9 +++---- include/class.mailparse.php | 1 + include/class.orm.php | 11 +++++---- .../client/templates/dynamic-form.tmpl.php | 2 +- include/html2text.php | 2 +- include/staff/templates/dynamic-form.tmpl.php | 2 +- scp/tickets.php | 2 +- setup/test/tests/lib/phplint.tcl | 15 +++++++++--- setup/test/tests/stubs.php | 24 +++++++++++++++++++ 12 files changed, 63 insertions(+), 22 deletions(-) diff --git a/include/ajax.draft.php b/include/ajax.draft.php index 8ab99c946..37df05f77 100644 --- a/include/ajax.draft.php +++ b/include/ajax.draft.php @@ -7,8 +7,9 @@ require_once(INCLUDE_DIR.'class.draft.php'); class DraftAjaxAPI extends AjaxController { function _createDraft($vars) { - foreach (array('response', 'note', 'answer', 'body', 'message', - 'issue') as $field) { + $field_list = array('response', 'note', 'answer', 'body', + 'message', 'issue'); + foreach ($field_list as $field) { if (isset($_POST[$field])) { $vars['body'] = urldecode($_POST[$field]); break; @@ -48,8 +49,9 @@ class DraftAjaxAPI extends AjaxController { } function _updateDraft($draft) { - foreach (array('response', 'note', 'answer', 'body', 'message', - 'issue') as $field) { + $field_list = array('response', 'note', 'answer', 'body', + 'message', 'issue'); + foreach ($field_list as $field) { if (isset($_POST[$field])) { $body = urldecode($_POST[$field]); break; diff --git a/include/class.dynamic_forms.php b/include/class.dynamic_forms.php index 55020e192..e803235c8 100644 --- a/include/class.dynamic_forms.php +++ b/include/class.dynamic_forms.php @@ -457,8 +457,9 @@ class DynamicFormEntry extends VerySimpleModel { } // Sort the form the way it is declared to be sorted if ($this->_fields) - usort($this->_fields, function($a, $b) { - return $a->get('sort') - $b->get('sort'); + usort($this->_fields, + function($a, $b) { + return $a->get('sort') - $b->get('sort'); }); } } diff --git a/include/class.file.php b/include/class.file.php index 03ea04736..6664b4f82 100644 --- a/include/class.file.php +++ b/include/class.file.php @@ -151,7 +151,7 @@ class AttachmentFile { } } - function display() { + function display($scale=false) { $this->makeCacheable(); if ($scale && extension_loaded('gd')) { diff --git a/include/class.forms.php b/include/class.forms.php index adb525e80..48e1e9047 100644 --- a/include/class.forms.php +++ b/include/class.forms.php @@ -682,6 +682,7 @@ class ThreadEntryField extends FormField { } function renderExtras($mode=null) { if ($mode == 'client') + // TODO: Pass errors arrar into showAttachments $this->getWidget()->showAttachments(); } } @@ -785,7 +786,7 @@ class TextboxWidget extends Widget { class TextareaWidget extends Widget { function render() { $config = $this->field->getConfiguration(); - $class = ""; + $class = $cols = $rows = $maxlength = ""; if (isset($config['rows'])) $rows = "rows=\"{$config['rows']}\""; if (isset($config['cols'])) @@ -796,7 +797,7 @@ class TextareaWidget extends Widget { $class = 'class="richtext no-bar small"'; ?> <span style="display:inline-block;width:100%"> - <textarea <?php echo $rows." ".$cols." ".$length." ".$class; ?> + <textarea <?php echo $rows." ".$cols." ".$maxlength." ".$class; ?> name="<?php echo $this->name; ?>"><?php echo Format::htmlchars($this->value); ?></textarea> @@ -892,7 +893,7 @@ class DatetimePickerWidget extends Widget { strtotime($this->value)); if ($config['gmt']) $this->value += 3600 * - $_SESSION['TZ_OFFSET']+($_SESSION['TZ_DST']?date('I',$time):0); + $_SESSION['TZ_OFFSET']+($_SESSION['TZ_DST']?date('I',$this->value):0); list($hr, $min) = explode(':', date('H:i', $this->value)); $this->value = date('m/d/Y', $this->value); @@ -971,7 +972,7 @@ class ThreadEntryWidget extends Widget { <?php } - function showAttachments() { + function showAttachments($errors=array()) { global $cfg, $thisclient; if(($cfg->allowOnlineAttachments() diff --git a/include/class.mailparse.php b/include/class.mailparse.php index 145d10bf7..953435d5a 100644 --- a/include/class.mailparse.php +++ b/include/class.mailparse.php @@ -157,6 +157,7 @@ class Mail_Parse { } function getBody(){ + global $cfg; if ($body=$this->getPart($this->struct,'text/html')) { //Cleanup the html. diff --git a/include/class.orm.php b/include/class.orm.php index 8aa8e5d39..90557fa76 100644 --- a/include/class.orm.php +++ b/include/class.orm.php @@ -299,7 +299,8 @@ class QuerySet implements IteratorAggregate, ArrayAccess { } function count() { - $compiler = new $this->compiler(); + $class = $this->compiler; + $compiler = new $class(); return $compiler->compileCount($this); } @@ -309,8 +310,9 @@ class QuerySet implements IteratorAggregate, ArrayAccess { // IteratorAggregate interface function getIterator() { + $class = $this->iterator; if (!isset($this->_iterator)) - $this->_iterator = new $this->iterator($this); + $this->_iterator = new $class($this); return $this->_iterator; } @@ -341,7 +343,8 @@ class QuerySet implements IteratorAggregate, ArrayAccess { if (!$this->ordering && isset($model::$meta['ordering'])) $this->ordering = $model::$meta['ordering']; - $compiler = new $this->compiler($options); + $class = $this->compiler; + $compiler = new $class($options); $this->query = $compiler->compileSelect($this); return $this->query; @@ -364,7 +367,7 @@ class ModelInstanceIterator implements Iterator, ArrayAccess { function buildModel($row) { // TODO: Traverse to foreign keys - return new $this->model($row); + return new $this->model($row); # nolint } function fillTo($index) { diff --git a/include/client/templates/dynamic-form.tmpl.php b/include/client/templates/dynamic-form.tmpl.php index 5eac1d38a..c8e7090ff 100644 --- a/include/client/templates/dynamic-form.tmpl.php +++ b/include/client/templates/dynamic-form.tmpl.php @@ -23,7 +23,7 @@ <?php } else { ?> - <td><label for="<?php echo $field->getFormname(); ?>" class="<?php + <td><label for="<?php echo $field->getFormName(); ?>" class="<?php if ($field->get('required')) echo 'required'; ?>"> <?php echo Format::htmlchars($field->get('label')); ?>:</label></td><td> <?php diff --git a/include/html2text.php b/include/html2text.php index ba1e1389d..a4997d1b2 100644 --- a/include/html2text.php +++ b/include/html2text.php @@ -627,7 +627,7 @@ class HtmlTable extends HtmlBlockElement { # Stash the computed width so it doesn't need to be # recomputed again below $cell->width = $cwidth; - unset($data); + unset($data); # nolint $data = explode("\n", $cell->render($cwidth, $options)); $heights[$y] = max(count($data), $heights[$y]); $contents[$y][$i] = &$data; diff --git a/include/staff/templates/dynamic-form.tmpl.php b/include/staff/templates/dynamic-form.tmpl.php index c60cb8e0a..f77fa928a 100644 --- a/include/staff/templates/dynamic-form.tmpl.php +++ b/include/staff/templates/dynamic-form.tmpl.php @@ -1,4 +1,4 @@ - <?php if ($form->getTitle()) { ?> +<?php if ($form->getTitle()) { ?> <tr><th colspan="2"> <em><strong><?php echo Format::htmlchars($form->getTitle()); ?></strong>: <?php echo Format::htmlchars($form->getInstructions()); ?></em> diff --git a/scp/tickets.php b/scp/tickets.php index 0fde54a63..455cbb41a 100644 --- a/scp/tickets.php +++ b/scp/tickets.php @@ -181,7 +181,7 @@ if($_POST && !$errors): // Cleanup drafts for the ticket. If not closed, only clean // for this staff. Else clean all drafts for the ticket. - Draft::deleteForTicket($ticket->getId(), + Draft::deleteForNamespace('ticket.%.' . $ticket->getId(), $ticket->isClosed() ? false : $thisstaff->getId()); } else { diff --git a/setup/test/tests/lib/phplint.tcl b/setup/test/tests/lib/phplint.tcl index 6b857c750..30e10074b 100755 --- a/setup/test/tests/lib/phplint.tcl +++ b/setup/test/tests/lib/phplint.tcl @@ -7,10 +7,10 @@ proc scan file { set fd [open $file] set infunc 0 set linenr 0 - set fnre {(^\s*)((public|private|protected|static)\s*)*function\s+([^(]+)\s*\((.*)\).*} + set fnre {(^\s*)((public|private|protected|static)\s*)*function(?=\s|\()\s*([^(]+)?\s*\(((\(\)|[^)])*)\)(\s*use\s*\((.*)\))?} while {[gets $fd line] != -1} { incr linenr - if {[regexp $fnre $line - ind - - - fa]} { + if {[regexp $fnre $line - ind - - - fa - - ua]} { # If $infunc is true we miss the end of the last function # so we analyze it now. if {$infunc} { @@ -26,6 +26,11 @@ proc scan file { set arg [string trim $arg " $&"] lappend arglist $arg } + # Support closure variables + foreach arg [split $ua ,] { + set arg [string trim $arg " $"] + lappend arglist $arg + } set infunc 1 } elseif {$infunc && [regexp "^$ind\}" $line]} { set infunc 0 @@ -185,11 +190,15 @@ proc analyze {file arglist body} { } # Check for var accesses - set varsimplere {\$[_A-Za-z]+[_A-Za-z0-9]*} + set varsimplere {(?:::)?\$[_A-Za-z]+[_A-Za-z0-9]*} set l [regexp -all -inline $varsimplere $line] foreach a $l { set a [string trim $a "=$ "] regsub -all {\[.*\]} $a {} a + # Skip access to class-static variables + if {[string first :: $a] == 0} { + continue + } #puts "access of $a" if {![info exists initialized($a)] && ![info exists ignore($a)]} { diff --git a/setup/test/tests/stubs.php b/setup/test/tests/stubs.php index 38cd382cd..bfb686074 100644 --- a/setup/test/tests/stubs.php +++ b/setup/test/tests/stubs.php @@ -5,11 +5,35 @@ class mysqli { function query() {} function real_escape_string() {} function fetch_row() {} + function prepare() {} +} +class mysqli_stmt { + function store_result() {} + function data_seek() {} + function fetch() {} + function fetch_field() {} + function result_metadata() {} } class ReflectionClass { function getMethods() {} } +class DomNode { + function hasChildNodes() {} +} + +class DomNodeList { + function item() {} +} + +class DomElement { + function getAttribute() {} +} + +class DomDocument { + function loadHTML() {} +} + ?> -- GitLab