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.

Sign up for GitHub

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

Type safe comparison in plugins - second iteration #13272

Merged
merged 9 commits into from
May 31, 2017
Merged

Type safe comparison in plugins - second iteration #13272

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
39a2e07
Type safe string comparison of strings in plugins - second iteration
frankmayer Dec 18, 2016
d8e1eb7
Fixed two bugs introduced in previous commit (one involving multiple …
frankmayer Dec 19, 2016
e058414
Merge branch 'staging' into type-safety-in-plugins-2
frankmayer Dec 19, 2016
cfaca22
Missed a comparison. Thanks @shur!
frankmayer Dec 24, 2016
90055f6
Reversed type safe comparison, as it turns out, that the type of the …
frankmayer Dec 28, 2016
a52d3be
Reversed type safe comparison (#2), as it turns out, that the type of…
frankmayer Dec 28, 2016
669da6f
Reverted this one to not clash with @laoneo's refactoring efforts
frankmayer Dec 29, 2016
4d9479e
Merge branch 'staging' into type-safety-in-plugins-2
frankmayer Jan 12, 2017
572572c
Merge branch 'staging' into type-safety-in-plugins-2
frankmayer May 29, 2017
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
4 changes: 2 additions & 2 deletions plugins/authentication/cookie/cookie.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ public function onUserAuthenticate($credentials, $options, &$response)
$cookieArray = explode('.', $cookieValue);

// Check for valid cookie value
if (count($cookieArray) != 2)
if (count($cookieArray) !== 2)
{
// Destroy the cookie in the browser.
$this->app->input->cookie->set($cookieName, false, time() - 42000, $this->app->get('cookie_path', '/'), $this->app->get('cookie_domain'));
Expand Down Expand Up @@ -279,7 +279,7 @@ public function onUserAfterLogin($options)
$errorCount++;

// We'll let this query fail up to 5 times before giving up, there's probably a bigger issue at this point
if ($errorCount == 5)
if ($errorCount === 5)
{
return false;
}
Expand Down
2 changes: 1 addition & 1 deletion plugins/authentication/gmail/gmail.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ public function onUserAuthenticate($credentials, $options, &$response)
foreach ($localUsers as $localUser)
{
// Local user exists with same username but different email address
if ($email != $localUser->email)
if ($email !== $localUser->email)
{
$response->status = JAuthentication::STATUS_FAILURE;
$response->error_message = JText::sprintf('JGLOBAL_AUTH_FAILED', JText::_('PLG_GMAIL_ERROR_LOCAL_USERNAME_CONFLICT'));
Expand Down
2 changes: 1 addition & 1 deletion plugins/captcha/recaptcha/recaptchalib.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ class JReCaptcha
*/
public function __construct($secret)
{
if ($secret == null || $secret == '')
if ($secret == null || $secret === '')
Copy link
Contributor

Choose a reason for hiding this comment

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

@frankmayer Should this be === null?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think It could, and I also think I have done this in another PR.

{
die("To use reCAPTCHA you must get an API key from <a href='"
. self::$_signupUrl . "'>" . self::$_signupUrl . "</a>");
Expand Down
2 changes: 1 addition & 1 deletion plugins/content/contact/contact.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ protected function getContactId($created_by)
$query->where('contact.published = 1');
$query->where('contact.user_id = ' . (int) $created_by);

if (JLanguageMultilang::isEnabled() == 1)
if (JLanguageMultilang::isEnabled() === true)
{
$query->where('(contact.language in '
. '(' . $this->db->quote(JFactory::getLanguage()->getTag()) . ',' . $this->db->quote('*') . ') '
Expand Down
10 changes: 5 additions & 5 deletions plugins/content/pagebreak/pagebreak.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -311,17 +311,17 @@ protected function _createToc(&$row, &$matches, &$page)
$title = JText::sprintf('PLG_CONTENT_PAGEBREAK_PAGE_NUM', $i);
}

$liClass = ($limitstart == $i - 1) ? ' class="active"' : '';
$class = ($limitstart == $i - 1) ? 'toclink active' : 'toclink';
$liClass = ($limitstart === $i - 1) ? ' class="active"' : '';
$class = ($limitstart === $i - 1) ? 'toclink active' : 'toclink';
$row->toc .= '<li' . $liClass . '><a href="' . $link . '" class="' . $class . '">' . $title . '</a></li>';
$i++;
}

if ($this->params->get('showall'))
{
$link = JRoute::_(ContentHelperRoute::getArticleRoute($row->slug, $row->catid, $row->language) . '&showall=1&limitstart=');
$liClass = ($limitstart == $i - 1) ? ' class="active"' : '';
$class = ($limitstart == $i - 1) ? 'toclink active' : 'toclink';
$liClass = ($limitstart === $i - 1) ? ' class="active"' : '';
$class = ($limitstart === $i - 1) ? 'toclink active' : 'toclink';
$row->toc .= '<li' . $liClass . '><a href="' . $link . '" class="' . $class . '">'
. JText::_('PLG_CONTENT_PAGEBREAK_ALL_PAGES') . '</a></li>';
}
Expand Down Expand Up @@ -365,7 +365,7 @@ protected function _createNavigation(&$row, $page, $n)

if ($page > 0)
{
$page_prev = $page - 1 == 0 ? '' : $page - 1;
$page_prev = $page - 1 === 0 ? '' : $page - 1;

$link_prev = JRoute::_(ContentHelperRoute::getArticleRoute($row->slug, $row->catid, $row->language) . '&showall=&limitstart=' . $page_prev);

Expand Down
2 changes: 1 addition & 1 deletion plugins/content/vote/vote.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ private function displayVotingData($context, &$row, &$params, $page)
include $path;
$html = ob_get_clean();

if ($this->app->input->getString('view', '') === 'article' && $row->state == 1)
if ($this->app->input->getString('view', '') === 'article' && $row->state === 1)
Copy link
Contributor

Choose a reason for hiding this comment

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

doubt: is row state (and access in changes below) an int for sure in all cases?

Copy link
Contributor

Choose a reason for hiding this comment

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

Did you check this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, sorry, it got under. I checked it just now. You were right! Unfortunately it is not guaranteed that it is an int.
So, either we cast it to an int or we revert the change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

BTW, I don't know if this has come up in the past, but if we want to do strict(er) typing, as we should, then I guess the db driver classes should make sure the right types are returned, also.

Copy link
Contributor

Choose a reason for hiding this comment

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

i agree, but a change like that IMHO should be in 4.0 branch
@wilsonge what do you think?

{
// Get the path for the voting form layout file
$path = JPluginHelper::getLayoutPath('content', 'vote', 'vote');
Expand Down
4 changes: 2 additions & 2 deletions plugins/editors-xtd/image/image.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,15 +40,15 @@ public function onDisplay($name, $asset, $author)
$user = JFactory::getUser();
$extension = JFactory::getApplication()->input->get('option');

if ($asset == '')
if ($asset === '')
{
$asset = $extension;
}

if ($user->authorise('core.edit', $asset)
|| $user->authorise('core.create', $asset)
|| (count($user->getAuthorisedCategories($asset, 'core.create')) > 0)
|| ($user->authorise('core.edit.own', $asset) && $author == $user->id)
|| ($user->authorise('core.edit.own', $asset) && $author === $user->id)
|| (count($user->getAuthorisedCategories($extension, 'core.edit')) > 0)
|| (count($user->getAuthorisedCategories($extension, 'core.edit.own')) > 0 && $author == $user->id))
Copy link
Contributor

Choose a reason for hiding this comment

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

and what about line 53 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, yes, missed that one... Thanks!!

{
Expand Down
8 changes: 4 additions & 4 deletions plugins/finder/categories/categories.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ public function onFinderAfterSave($context, $row, $isNew)
if ($context === 'com_categories.category')
{
// Check if the access levels are different.
if (!$isNew && $this->old_access != $row->access)
if (!$isNew && $this->old_access !== $row->access)
{
// Process the change.
$this->itemAccessChange($row);
Expand All @@ -136,7 +136,7 @@ public function onFinderAfterSave($context, $row, $isNew)
$this->reindex($row->id);

// Check if the parent access level is different.
if (!$isNew && $this->old_cataccess != $row->access)
if (!$isNew && $this->old_cataccess !== $row->access)
{
$this->categoryAccessChange($row);
}
Expand Down Expand Up @@ -244,13 +244,13 @@ public function onFinderChangeState($context, $pks, $value)
protected function index(FinderIndexerResult $item, $format = 'html')
{
// Check if the extension is enabled.
if (JComponentHelper::isEnabled($this->extension) == false)
if (JComponentHelper::isEnabled($this->extension) === false)
{
return;
}

// Check if the extension that owns the category is also enabled.
if (JComponentHelper::isEnabled($item->extension) == false)
if (JComponentHelper::isEnabled($item->extension) === false)
{
return;
}
Expand Down
6 changes: 3 additions & 3 deletions plugins/finder/contacts/contacts.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ public function onFinderAfterSave($context, $row, $isNew)
if ($context === 'com_contact.contact')
{
// Check if the access levels are different
if (!$isNew && $this->old_access != $row->access)
if (!$isNew && $this->old_access !== $row->access)
{
// Process the change.
$this->itemAccessChange($row);
Expand All @@ -161,7 +161,7 @@ public function onFinderAfterSave($context, $row, $isNew)
if ($context === 'com_categories.category')
{
// Check if the access levels are different
if (!$isNew && $this->old_cataccess != $row->access)
if (!$isNew && $this->old_cataccess !== $row->access)
{
$this->categoryAccessChange($row);
}
Expand Down Expand Up @@ -251,7 +251,7 @@ public function onFinderChangeState($context, $pks, $value)
protected function index(FinderIndexerResult $item, $format = 'html')
{
// Check if the extension is enabled
if (JComponentHelper::isEnabled($this->extension) == false)
if (JComponentHelper::isEnabled($this->extension) === false)
{
return;
}
Expand Down
4 changes: 2 additions & 2 deletions plugins/finder/content/content.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ public function onFinderAfterSave($context, $row, $isNew)
if ($context === 'com_content.article' || $context === 'com_content.form')
{
// Check if the access levels are different.
if (!$isNew && $this->old_access != $row->access)
if (!$isNew && $this->old_access !== $row->access)
{
// Process the change.
$this->itemAccessChange($row);
Expand All @@ -155,7 +155,7 @@ public function onFinderAfterSave($context, $row, $isNew)
if ($context === 'com_categories.category')
{
// Check if the access levels are different.
if (!$isNew && $this->old_cataccess != $row->access)
if (!$isNew && $this->old_cataccess !== $row->access)
{
$this->categoryAccessChange($row);
}
Expand Down
6 changes: 3 additions & 3 deletions plugins/finder/newsfeeds/newsfeeds.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ public function onFinderAfterSave($context, $row, $isNew)
if ($context === 'com_newsfeeds.newsfeed')
{
// Check if the access levels are different.
if (!$isNew && $this->old_access != $row->access)
if (!$isNew && $this->old_access !== $row->access)
{
// Process the change.
$this->itemAccessChange($row);
Expand All @@ -163,7 +163,7 @@ public function onFinderAfterSave($context, $row, $isNew)
if ($context === 'com_categories.category')
{
// Check if the access levels are different.
if (!$isNew && $this->old_cataccess != $row->access)
if (!$isNew && $this->old_cataccess !== $row->access)
{
$this->categoryAccessChange($row);
}
Expand Down Expand Up @@ -252,7 +252,7 @@ public function onFinderChangeState($context, $pks, $value)
protected function index(FinderIndexerResult $item, $format = 'html')
{
// Check if the extension is enabled.
if (JComponentHelper::isEnabled($this->extension) == false)
if (JComponentHelper::isEnabled($this->extension) === false)
{
return;
}
Expand Down
4 changes: 2 additions & 2 deletions plugins/finder/tags/tags.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ public function onFinderAfterSave($context, $row, $isNew)
if ($context === 'com_tags.tag')
{
// Check if the access levels are different
if (!$isNew && $this->old_access != $row->access)
if (!$isNew && $this->old_access !== $row->access)
{
// Process the change.
$this->itemAccessChange($row);
Expand Down Expand Up @@ -206,7 +206,7 @@ public function onFinderChangeState($context, $pks, $value)
protected function index(FinderIndexerResult $item, $format = 'html')
{
// Check if the extension is enabled
if (JComponentHelper::isEnabled($this->extension) == false)
if (JComponentHelper::isEnabled($this->extension) === false)
{
return;
}
Expand Down
2 changes: 1 addition & 1 deletion plugins/quickicon/extensionupdate/extensionupdate.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ class PlgQuickiconExtensionupdate extends JPlugin
*/
public function onGetIcons($context)
{
if ($context != $this->params->get('context', 'mod_quickicon') || !JFactory::getUser()->authorise('core.manage', 'com_installer'))
if ($context !== $this->params->get('context', 'mod_quickicon') || !JFactory::getUser()->authorise('core.manage', 'com_installer'))
{
return;
}
Expand Down
2 changes: 1 addition & 1 deletion plugins/quickicon/joomlaupdate/joomlaupdate.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ class PlgQuickiconJoomlaupdate extends JPlugin
*/
public function onGetIcons($context)
{
if ($context != $this->params->get('context', 'mod_quickicon') || !JFactory::getUser()->authorise('core.manage', 'com_installer'))
if ($context !== $this->params->get('context', 'mod_quickicon') || !JFactory::getUser()->authorise('core.manage', 'com_installer'))
{
return;
}
Expand Down
2 changes: 1 addition & 1 deletion plugins/search/categories/categories.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ public function onContentSearch($text, $phrase = '', $ordering = '', $areas = nu

$text = trim($text);

if ($text == '')
if ($text === '')
{
return array();
}
Expand Down
2 changes: 1 addition & 1 deletion plugins/search/contacts/contacts.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ public function onContentSearch($text, $phrase = '', $ordering = '', $areas = nu

$text = trim($text);

if ($text == '')
if ($text === '')
{
return array();
}
Expand Down
2 changes: 1 addition & 1 deletion plugins/search/content/content.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ public function onContentSearch($text, $phrase = '', $ordering = '', $areas = nu

$text = trim($text);

if ($text == '')
if ($text === '')
{
return array();
}
Expand Down
2 changes: 1 addition & 1 deletion plugins/search/newsfeeds/newsfeeds.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ public function onContentSearch($text, $phrase = '', $ordering = '', $areas = nu

$text = trim($text);

if ($text == '')
if ($text === '')
{
return array();
}
Expand Down
4 changes: 2 additions & 2 deletions plugins/search/tags/tags.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ public function onContentSearch($text, $phrase = '', $ordering = '', $areas = nu

$text = trim($text);

if ($text == '')
if ($text === '')
{
return array();
}
Expand Down Expand Up @@ -156,7 +156,7 @@ public function onContentSearch($text, $phrase = '', $ordering = '', $areas = nu
foreach ($rows as $key => $row)
{
$rows[$key]->href = TagsHelperRoute::getTagRoute($row->id);
$rows[$key]->text = ($row->description != '' ? $row->description : $row->title);
$rows[$key]->text = ($row->description !== '' ? $row->description : $row->title);
$rows[$key]->text .= $row->note;
$rows[$key]->section = $section;
$rows[$key]->created = $row->created;
Expand Down
2 changes: 1 addition & 1 deletion plugins/system/debug/debug.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1189,7 +1189,7 @@ protected function displayQueries()
$labelClass = 'label-warning';
}

if ($this->totalQueries == 0)
if ($this->totalQueries === 0)
{
$this->totalQueries = $db->getCount();
}
Expand Down
2 changes: 1 addition & 1 deletion plugins/system/fields/fields.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -580,7 +580,7 @@ private function getParts($context)
$db->setQuery($query);
$tmp = $db->loadObjectList();

if (count($tmp) == 1)
if (count($tmp) === 1)
{
$parts = FieldsHelper::extract($tmp[0]->context);

Expand Down
18 changes: 9 additions & 9 deletions plugins/system/languagefilter/languagefilter.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,8 +210,8 @@ public function buildRule(&$router, &$uri)

if ($this->mode_sef
&& (!$this->params->get('remove_default_prefix', 0)
|| $lang != $this->default_lang
|| $lang != $this->current_lang))
|| $lang !== $this->default_lang
|| $lang !== $this->current_lang))
{
$uri->setPath($uri->getPath() . '/' . $sef . '/');
}
Expand Down Expand Up @@ -339,7 +339,7 @@ public function parseRule(&$router, &$uri)
array_shift($parts);

// Empty parts array when "index.php" is the only part left.
if (count($parts) == 1 && $parts[0] === 'index.php')
if (count($parts) === 1 && $parts[0] === 'index.php')
{
$parts = array();
}
Expand Down Expand Up @@ -549,7 +549,7 @@ public function onUserAfterSave($user, $isnew, $success, $msg)
$lang_code = $this->current_lang;
}

if ($lang_code == $this->user_lang_code || !isset($this->lang_codes[$lang_code]))
if ($lang_code === $this->user_lang_code || !isset($this->lang_codes[$lang_code]))
{
if ($this->app->isSite())
{
Expand Down Expand Up @@ -663,15 +663,15 @@ public function onUserLogin($user, $options = array())
// We are on a Home page, we redirect to the user preferred site language Home page.
$item = $menu->getDefault($lang_code);

if ($item && $item->language != $active->language && $item->language !== '*')
if ($item && $item->language !== $active->language && $item->language !== '*')
{
$this->app->setUserState('users.login.form.return', 'index.php?Itemid=' . $item->id);
$foundAssociation = true;
}
}
}

if ($foundAssociation && $lang_code != $this->current_lang)
if ($foundAssociation && $lang_code !== $this->current_lang)
{
// Change language.
$this->current_lang = $lang_code;
Expand Down Expand Up @@ -722,14 +722,14 @@ public function onAfterDispatch()
$current_link = JRoute::_($currentInternalUrl);

// Load menu associations
if ($active_link == $current_link)
if ($active_link === $current_link)
{
$associations = MenusHelper::getAssociations($active->id);
}

// Check if we are on the home page
$is_home = ($active->home
&& ($active_link == $current_link || $active_link == $current_link . 'index.php' || $active_link . '/' == $current_link));
&& ($active_link === $current_link || $active_link === $current_link . 'index.php' || $active_link . '/' === $current_link));
}

// Load component associations.
Expand Down Expand Up @@ -760,7 +760,7 @@ public function onAfterDispatch()
break;

// Current language link
case ($i == $this->current_lang):
case ($i === $this->current_lang):
$language->link = JRoute::_($currentInternalUrl);
break;

Expand Down
2 changes: 1 addition & 1 deletion plugins/system/updatenotification/postinstall/updatecachetime.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ function updatecachetime_postinstall_condition()
$cacheTimeout = (int) JComponentHelper::getComponent('com_installer')->params->get('cachetimeout', 6);

// Check if cachetimeout is eq zero
if ($cacheTimeout == 0 && JPluginHelper::isEnabled('system', 'updatenotification'))
if ($cacheTimeout === 0 && JPluginHelper::isEnabled('system', 'updatenotification'))
{
return true;
}
Expand Down
Loading