Add new phan rule PhanMismatchReturn (#4914)

This adds the phan rule PhanMismatchReturn which flags code that is declared to return a type but in reality returns a different type.
aces · Sep 18, 2019 · 0dacece · 0dacece
1 parent b7f942f
commit 0dacece
Show file tree

Hide file tree

Showing 73 changed files with 349 additions and 318 deletions.
diff --git a/.phan/config.php b/.phan/config.php
@@ -21,7 +21,6 @@
 		"PhanUndeclaredVariableDim",
 		"PhanUndeclaredClassMethod",
 		"PhanTypeMismatchArgument",
-		"PhanTypeMismatchReturn",
 		"PhanTypeMismatchProperty",
         "PhanTypeSuspiciousStringExpression",
 	],

diff --git a/htdocs/survey.php b/htdocs/survey.php
@@ -369,7 +369,7 @@ function display()
             $this->tpl_data['complete']  = true;
 
             $this->updateStatus('Complete');
-            $Responses      = $DB->update(
+            $DB->update(
                 $this->TestName,
                 array(
                  'Date_taken' => date('Y-m-d'),
@@ -378,7 +378,7 @@ function display()
                  'CommentID' => $this->CommentID,
                 )
             );
-            $Responses_flag = $DB->update(
+            $DB->update(
                 'flag',
                 array(
                  'Data_entry'     => 'Complete',

diff --git a/modules/create_timepoint/php/create_timepoint.class.inc b/modules/create_timepoint/php/create_timepoint.class.inc
@@ -208,7 +208,9 @@ class Create_Timepoint extends \NDB_Form
                         WHERE CenterID =:cid",
                         array('cid' => $siteID)
                     );
-                    $psc_labelOptions[$siteID] = $center['Name'];
+                    if (!is_null($center)) {
+                        $psc_labelOptions[$siteID] = $center['Name'];
+                    }
                 }
             }
             $this->addSelect('psc', 'Site', $psc_labelOptions);

diff --git a/modules/data_integrity_flag/php/data_integrity_flag.class.inc b/modules/data_integrity_flag/php/data_integrity_flag.class.inc
@@ -76,7 +76,7 @@ class Data_Integrity_Flag extends \NDB_Menu_Filter
             || !isset($req['date'])
             || !isset($req['flagStatus'])
         ) {
-            return "Error";
+            return;
         }
         $req = \Utility::asArray($req);
         // comment may not be set since it is not a required field

diff --git a/modules/data_release/ajax/FileUpload.php b/modules/data_release/ajax/FileUpload.php
@@ -43,7 +43,7 @@
     } else {
         $target_path = $base_path . $fileName;
         if (move_uploaded_file($_FILES["file"]["tmp_name"], $target_path)) {
-            $success = $DB->insert(
+            $DB->insert(
                 'data_release',
                 array(
                  'file_name'   => $fileName,
@@ -68,7 +68,7 @@
                  'upload_date' => $upload_date,
                 )
             );
-            $success = $DB->insert(
+            $DB->insert(
                 'data_release_permissions',
                 array(
                  'userid'          => $user_ID,

diff --git a/modules/datadict/php/datadict.class.inc b/modules/datadict/php/datadict.class.inc
@@ -134,7 +134,6 @@ class Datadict extends \NDB_Menu_Filter
             )
         );
         $this->addBasicText('keyword', "Search keyword");
-        return true;
     }
 
     /**

diff --git a/modules/dicom_archive/php/viewdetails.class.inc b/modules/dicom_archive/php/viewdetails.class.inc
@@ -95,9 +95,9 @@ class ViewDetails extends \NDB_Form
             $array = $this->DB->pselectRow($query, array('ID' => $tarchiveID));
             break;
         case "tarchive_series":
-            $query = "SELECT * FROM $table WHERE TarchiveID =:ID
+            $query        = "SELECT * FROM $table WHERE TarchiveID =:ID
                 ORDER BY :OField";
-            $array = $this->DB->pselect(
+            $tarchiveData = $this->DB->pselect(
                 $query,
                 array(
                  'ID'     => $tarchiveID,
@@ -108,7 +108,7 @@ class ViewDetails extends \NDB_Form
             if ($this->_setProtocols()) {
                 $previousSeriesDescription = '';
                 $previousProtocolName      = '';
-                foreach ($array as &$series) {
+                foreach ($tarchiveData as &$series) {
                     $seriesDescription = $series['SeriesDescription'];
                     // if the same series, do not compute the protocol name again,
                     // use the previous one
@@ -124,9 +124,9 @@ class ViewDetails extends \NDB_Form
             }
             break;
         case "tarchive_files":
-            $query = "SELECT * FROM $table WHERE TarchiveID =:ID
+            $query        = "SELECT * FROM $table WHERE TarchiveID =:ID
                 ORDER BY :OField";
-            $array = $this->DB->pselect(
+            $tarchiveData = $this->DB->pselect(
                 $query,
                 array(
                  'ID'     => $tarchiveID,
@@ -136,7 +136,7 @@ class ViewDetails extends \NDB_Form
             break;
         }
 
-        return $array;
+        return $tarchiveData ?? array();
     }
     /**
     * Validates PatientName and PatientID,
@@ -321,7 +321,7 @@ class ViewDetails extends \NDB_Form
         try {
             $query = "SELECT Scan_type FROM mri_scan_type WHERE ID=:ID";
             $array = $this->DB->pselectRow($query, array('ID' => $id));
-            return $array['Scan_type'];
+            return $array['Scan_type'] ?? 'Unknown';
         } catch (\LorisException $e) {
             return "Unknown";
         }

diff --git a/modules/document_repository/php/doctree.class.inc b/modules/document_repository/php/doctree.class.inc
@@ -75,16 +75,14 @@ class DocTree extends \NDB_Page
     /**
      * This method will return an array of document_repository_categories
      *
-     * @return ResponseInterface The outgoing PSR7 response
+     * @return array All the data from the document_repository_categories table
      */
     public function getTreeData()
     {
-          $DB       = \Database::singleton();
-          $category = $DB->pselect(
-              "SELECT * FROM document_repository_categories",
-              array()
-          );
-          return $category;
+        return \Database::singleton()->pselect(
+            "SELECT * FROM document_repository_categories",
+            array()
+        );
     }
     /**
      * This method will return an array of all children's categories
@@ -119,7 +117,7 @@ class DocTree extends \NDB_Page
      *
      * @return array the result
      */
-    Public function getParents($data, $id)
+    public function getParents($data, $id)
     {
         $arr = array();
         foreach ($data as $v) {
@@ -133,8 +131,7 @@ class DocTree extends \NDB_Page
                 }
             }
         }
-            return $arr;
-
+        return $arr;
     }
     /**
      * This method will return a json of the parent's categroies and

diff --git a/modules/document_repository/php/document_repository.class.inc b/modules/document_repository/php/document_repository.class.inc
@@ -123,11 +123,17 @@ class Document_Repository extends \NDB_Menu_Filter_Form
             $categoryName = $value['category_name'];
         do {
             if ($value['parent_id'] != 0) {
-                $depth       += 1;
-                $parent_id    = $value['parent_id'];
-                $query        = "SELECT * FROM document_repository_categories".
+                $depth    += 1;
+                $parent_id = $value['parent_id'];
+                $query     = "SELECT * FROM document_repository_categories".
                             " where id=$parent_id";
-                $value        = $DB->pselectRow($query, array());
+                $value     = $DB->pselectRow($query, array());
+                if (is_null($value)) {
+                    throw new \LorisException(
+                        'Could not find data for specified parent ID. '
+                        . 'Cannot parse category.'
+                    );
+                }
                 $categoryName = $value['category_name'] . ">" . $categoryName;
             }
         } while ($value['parent_id'] != 0);
@@ -193,7 +199,6 @@ class Document_Repository extends \NDB_Menu_Filter_Form
         }
         $this->group_by = '';
         $this->order_by = 'v.File_name';
-        return true;
     }
 
     /**

diff --git a/modules/document_repository/php/files.class.inc b/modules/document_repository/php/files.class.inc
@@ -285,20 +285,26 @@ class Files extends \NDB_Page
     * @return array An array containing the category name and ID for file
     *               categories in the document repository.
     */
-    function parseCategory($value): array
+    function parseCategory(array $value): array
     {
         $id           = $value['id'];
         $depth        = 0;
         $DB           = \Database::singleton();
         $categoryName = $value['category_name'];
         do {
             if ($value['parent_id'] != 0) {
-                $depth       += 1;
-                $parent_id    = $value['parent_id'];
-                $query        = "SELECT * FROM document_repository_categories".
+                $depth    += 1;
+                $parent_id = $value['parent_id'];
+                $query     = "SELECT * FROM document_repository_categories".
                             " WHERE id=:parent_id";
-                $where        = array('parent_id' => $parent_id);
-                $value        = $DB->pselectRow($query, $where);
+                $where     = array('parent_id' => $parent_id);
+                $value     = $DB->pselectRow($query, $where);
+                if (is_null($value)) {
+                    throw new \LorisException(
+                        'No parent category found for specified parentID. '
+                        . 'Cannot parse categories.'
+                    );
+                }
                 $categoryName = $value['category_name'] . ">" . $categoryName;
             }
         } while ($value['parent_id'] != 0);
@@ -403,7 +409,7 @@ class Files extends \NDB_Page
         ) {
                 $uploadedFile->moveTo($uploadPath.$fileName);
 
-                $success = $DB->insert(
+                $DB->insert(
                     'document_repository',
                     array(
                      'File_category' => $category,

diff --git a/modules/examiner/php/editexaminer.class.inc b/modules/examiner/php/editexaminer.class.inc
@@ -200,6 +200,12 @@ class EditExaminer extends \NDB_Form
                      'TID' => $testID,
                     )
                 );
+                if (is_null($oldVals)) {
+                    throw new \LorisException(
+                        'No old certification values found for specified '
+                        . 'examiner and test IDs.'
+                    );
+                }
 
                 $oldVal  = $oldVals['new'];
                 $oldDate = $oldVals['new_date'];
@@ -214,6 +220,13 @@ class EditExaminer extends \NDB_Form
                     )
                 );
 
+                if (is_null($oldCertification)) {
+                    throw new \LorisException(
+                        'No old data certification found for specified '
+                        . 'examiner and test IDs.'
+                    );
+                }
+
                 // If one of the values was changed
                 if ($oldCertification['pass'] != $certStatus
                     || $oldCertification['comment'] != $comment

diff --git a/modules/examiner/php/examiner.class.inc b/modules/examiner/php/examiner.class.inc
@@ -116,7 +116,6 @@ class Examiner extends \NDB_Menu_Filter_Form
                                'site'        => 'epr.centerID',
                                'radiologist' => 'COALESCE(e.radiologist, 0)',
                               );
-        return true;
     }
 
     /**

diff --git a/modules/genomic_browser/php/cnv_browser.class.inc b/modules/genomic_browser/php/cnv_browser.class.inc
@@ -200,7 +200,6 @@ class CNV_Browser extends \NDB_Menu_Filter
                 'Platform'           => 'genotyping_platform.Name',
                );
         $this->formToFilter = $ftf;
-        return true;
     }
 
     /**
@@ -342,8 +341,6 @@ class CNV_Browser extends \NDB_Menu_Filter
         $this->addSelect('Array_Report', 'Array Report:', $Array_Report_options);
         $this->addBasicText('Markers', 'Markers:');
         $this->addBasicText('Validation_Method', 'Validation Method:');
-
-        return true;
     }
 
     /**

diff --git a/modules/genomic_browser/php/cpg_browser.class.inc b/modules/genomic_browser/php/cpg_browser.class.inc
@@ -243,8 +243,6 @@ class CpG_Browser extends \NDB_Menu_Filter
                                   'gca.design_type',
                                   'gca.Strand',
                                  );
-
-        return true;
     }
 
     /**
@@ -426,8 +424,6 @@ class CpG_Browser extends \NDB_Menu_Filter
             'dmr'
         );
         $this->addSelect('DMR', 'DMR:', $Base_options);
-
-        return true;
     }
 
     /**

diff --git a/modules/genomic_browser/php/genomic_browser.class.inc b/modules/genomic_browser/php/genomic_browser.class.inc
@@ -249,8 +249,6 @@ class Genomic_Browser extends \NDB_Menu_Filter
                                  'full'  => 'All fields',
                                 );
         $this->addSelect('Show_Brief_Results', 'Display:', $show_results_options);
-
-        return true;
     }
 
     /**

diff --git a/modules/genomic_browser/php/genomic_file_uploader.class.inc b/modules/genomic_browser/php/genomic_file_uploader.class.inc
@@ -143,8 +143,6 @@ class Genomic_File_Uploader extends \NDB_Menu_Filter
         $this->tpl_data['upload_allowed'] = $user->hasPermission(
             'genomic_data_manager'
         );
-
-        return true;
     }
 
     /**

diff --git a/modules/genomic_browser/php/gwas_browser.class.inc b/modules/genomic_browser/php/gwas_browser.class.inc
@@ -149,8 +149,6 @@ class GWAS_Browser extends \NDB_Menu_Filter
         $this->addBasicText('Estimate', 'Estimate:');
         $this->addBasicText('StdErr', 'Std Err:');
         $this->addBasicText('Pvalue', 'P-value:');
-
-        return true;
     }
 
     /**

diff --git a/modules/genomic_browser/php/snp_browser.class.inc b/modules/genomic_browser/php/snp_browser.class.inc
@@ -223,8 +223,6 @@ class SNP_Browser extends \NDB_Menu_Filter
                );
 
         $this->formToFilter = $ftf;
-
-        return true;
     }
 
     /**
@@ -376,8 +374,6 @@ class SNP_Browser extends \NDB_Menu_Filter
         $this->addSelect('Array_Report', 'Array Report:', $Array_Report_options);
         $this->addBasicText('Markers', 'Markers:');
         $this->addBasicText('Validation_Method', 'Validation Method:');
-
-        return true;
     }
 
     /**

diff --git a/modules/help_editor/php/edit_help_content.class.inc b/modules/help_editor/php/edit_help_content.class.inc
@@ -40,7 +40,8 @@ class Edit_Help_Content extends \NDB_Form
      /**
       * Returns default help content of the page
       *
-      * @return array
+      * @return array An associative array containing the title and content
+      *         for the default help information.
       */
     function _getDefaults()
     {
@@ -53,6 +54,8 @@ class Edit_Help_Content extends \NDB_Form
         $safeSubsection = $_REQUEST['subsection'] ?
             htmlspecialchars($_REQUEST['subsection']) : '';
 
+        $defaults = array();
+
         if (isset($_REQUEST['helpID'])) {
             $helpID = htmlspecialchars($_REQUEST['helpID']);
         }
-Original file line number
+Diff line change
@@ Expand Up / @@ -134,7 +134,6 @@ class Datadict extends \NDB_Menu_Filter @@
                 )
             );
             $this->addBasicText('keyword', "Search keyword");
-            return true;
         }
         /**
@@ Expand Down @@