Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add new phan rule PhanMismatchReturn #4914

Merged
merged 25 commits into from
Sep 18, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion .phan/config.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
"PhanUndeclaredVariableDim",
"PhanUndeclaredClassMethod",
"PhanTypeMismatchArgument",
"PhanTypeMismatchReturn",
"PhanTypeMismatchProperty",
"PhanTypeSuspiciousStringExpression",
],
Expand Down
4 changes: 2 additions & 2 deletions htdocs/survey.php
Original file line number Diff line number Diff line change
Expand Up @@ -369,7 +369,7 @@ function display()
$this->tpl_data['complete'] = true;

$this->updateStatus('Complete');
$Responses = $DB->update(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unused. same with below.

$DB->update(
$this->TestName,
array(
'Date_taken' => date('Y-m-d'),
Expand All @@ -378,7 +378,7 @@ function display()
'CommentID' => $this->CommentID,
)
);
$Responses_flag = $DB->update(
$DB->update(
'flag',
array(
'Data_entry' => 'Complete',
Expand Down
4 changes: 3 additions & 1 deletion modules/create_timepoint/php/create_timepoint.class.inc
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another theme in this PR is verifying that DB results are not null. Since the pselect* family does not have any return types it is possible for them to return null instead of the expected array of data. This causes phan to get upset when we try to access an array key without first ensuring that the DB result is not null.

$psc_labelOptions[$siteID] = $center['Name'];
}
}
}
$this->addSelect('psc', 'Site', $psc_labelOptions);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions modules/data_release/ajax/FileUpload.php
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@
} else {
$target_path = $base_path . $fileName;
if (move_uploaded_file($_FILES["file"]["tmp_name"], $target_path)) {
$success = $DB->insert(
Copy link
Contributor Author

@johnsaigle johnsaigle Jun 27, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

insert + update always return void so they should not be assigned to a value.

$DB->insert(
'data_release',
array(
'file_name' => $fileName,
Expand All @@ -68,7 +68,7 @@
'upload_date' => $upload_date,
)
);
$success = $DB->insert(
$DB->insert(
'data_release_permissions',
array(
'userid' => $user_ID,
Expand Down
1 change: 0 additions & 1 deletion modules/datadict/php/datadict.class.inc
Original file line number Diff 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
14 changes: 7 additions & 7 deletions modules/dicom_archive/php/viewdetails.class.inc
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
Expand All @@ -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,
Expand All @@ -136,7 +136,7 @@ class ViewDetails extends \NDB_Form
break;
}

return $array;
return $tarchiveData ?? array();
}
/**
* Validates PatientName and PatientID,
Expand Down Expand Up @@ -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";
}
Expand Down
17 changes: 7 additions & 10 deletions modules/document_repository/php/doctree.class.inc
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Expand All @@ -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
Expand Down
15 changes: 10 additions & 5 deletions modules/document_repository/php/document_repository.class.inc
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should use label instead of direct value in query.

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);
Expand Down Expand Up @@ -193,7 +199,6 @@ class Document_Repository extends \NDB_Menu_Filter_Form
}
$this->group_by = '';
$this->order_by = 'v.File_name';
return true;
}

/**
Expand Down
20 changes: 13 additions & 7 deletions modules/document_repository/php/files.class.inc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -403,7 +409,7 @@ class Files extends \NDB_Page
) {
$uploadedFile->moveTo($uploadPath.$fileName);

$success = $DB->insert(
$DB->insert(
'document_repository',
array(
'File_category' => $category,
Expand Down
13 changes: 13 additions & 0 deletions modules/examiner/php/editexaminer.class.inc
Original file line number Diff line number Diff line change
Expand Up @@ -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'];
Expand All @@ -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
Expand Down
1 change: 0 additions & 1 deletion modules/examiner/php/examiner.class.inc
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,6 @@ class Examiner extends \NDB_Menu_Filter_Form
'site' => 'epr.centerID',
'radiologist' => 'COALESCE(e.radiologist, 0)',
);
return true;
}

/**
Expand Down
3 changes: 0 additions & 3 deletions modules/genomic_browser/php/cnv_browser.class.inc
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,6 @@ class CNV_Browser extends \NDB_Menu_Filter
'Platform' => 'genotyping_platform.Name',
);
$this->formToFilter = $ftf;
return true;
}

/**
Expand Down Expand Up @@ -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;
}

/**
Expand Down
4 changes: 0 additions & 4 deletions modules/genomic_browser/php/cpg_browser.class.inc
Original file line number Diff line number Diff line change
Expand Up @@ -243,8 +243,6 @@ class CpG_Browser extends \NDB_Menu_Filter
'gca.design_type',
'gca.Strand',
);

return true;
}

/**
Expand Down Expand Up @@ -426,8 +424,6 @@ class CpG_Browser extends \NDB_Menu_Filter
'dmr'
);
$this->addSelect('DMR', 'DMR:', $Base_options);

return true;
}

/**
Expand Down
2 changes: 0 additions & 2 deletions modules/genomic_browser/php/genomic_browser.class.inc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

/**
Expand Down
2 changes: 0 additions & 2 deletions modules/genomic_browser/php/genomic_file_uploader.class.inc
Original file line number Diff line number Diff line change
Expand Up @@ -143,8 +143,6 @@ class Genomic_File_Uploader extends \NDB_Menu_Filter
$this->tpl_data['upload_allowed'] = $user->hasPermission(
'genomic_data_manager'
);

return true;
}

/**
Expand Down
2 changes: 0 additions & 2 deletions modules/genomic_browser/php/gwas_browser.class.inc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

/**
Expand Down
4 changes: 0 additions & 4 deletions modules/genomic_browser/php/snp_browser.class.inc
Original file line number Diff line number Diff line change
Expand Up @@ -223,8 +223,6 @@ class SNP_Browser extends \NDB_Menu_Filter
);

$this->formToFilter = $ftf;

return true;
}

/**
Expand Down Expand Up @@ -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;
}

/**
Expand Down
5 changes: 4 additions & 1 deletion modules/help_editor/php/edit_help_content.class.inc
Original file line number Diff line number Diff line change
Expand Up @@ -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()
{
Expand All @@ -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']);
}
Expand Down
Loading