Page MenuHomePhabricator
Create Task
Maniphest T26159

Remove uses of the error suppression operator
Closed, ResolvedPublic
Actions

Description

"Error suppression operators have lots of implementation issues in PHP, they are evil and should almost never be used."

and

"Don't use the error suppression (@) operator for any reason ever. It's broken when E_STRICT is enabled and it causes an unlogged, unexplained error if there is a fatal, which is hard to support. Use wfSuppressWarnings() and wfRestoreWarnings() instead."

Somebody should go through MediaWiki and remove the 87 uses of it and replace it with appropriate error checking. Things like array indexes can be checked with isset(), some things should actually get wfSuppressWarnings() and wfRestoreWarnings() (permission errors on file operations are a good example).

Tagging this easy for somebody with some spare time.

Version: unspecified
Severity: minor

Details

Reference
bz24159
SubjectRepoBranchLines +/-
Avoid the use of silence operator (@) and use AtEase methodsmediawiki/coremaster+5 -6
Customize query in gerrit

Related Objects

Event Timeline

• bzimport raised the priority of this task from to Low.Nov 21 2014, 11:06 PM
• bzimport added a project: MediaWiki-General.
• bzimport set Reference to bz24159.
• bzimport added a subscriber: Unknown Object (MLST).
• demon created this task.Jun 28 2010, 3:04 PM
Reedy added a comment.Jun 29 2010, 6:30 AM

Dump of the what files/lines for the lazy? :P

• demon added a comment.Jun 29 2010, 12:28 PM

Created attachment 7523
List for the lazy

$ grep -irPn '@[a-z]+\(' . | grep -v .svn | grep -v 'Binary file' > ~/uses-of-@.txt

attachment uses-of-@.txt ignored as obsolete

• demon added a comment.Jun 29 2010, 12:29 PM

It has one false positive, an @import in CSS, but it's commented anyway :)

• demon added a comment.Jun 29 2010, 12:34 PM

Created attachment 7524
Try 2

Forgot to include _ in [a-z]+. This version includes it :)

Attached:

uses-of-@.txt7 KBDownload

Reedy added a comment.Jun 29 2010, 12:42 PM

./includes/normal/UtfNormalTest.php:77: if( preg_match( '/@Part([\d])/', $data, $matches ) ) {

Would be another ;)

• demon added a comment.Jul 2 2010, 7:46 PM

$ grep -irPn '@[a-z_]+\(' .

This only includes global function calls, which isn't the only place @ can be used :)

$ grep -irPn '@[a-z_]+::'

Yields 1 static method call. Similar grep for method on object calls came back negative, yay :)

$ grep -irn '@\$' . | grep -v .svn | grep -v 'Binary file'

Gives you a bunch of variable accessing with @ that I didn't include above :(

• bzimport added a comment.Oct 3 2010, 4:29 PM

thetooth wrote:

A better solution is to use php's scream flag which overrides the suppression symbol.

Add a statement to the main construct after the configuration has been loaded and have it call "ini_set('scream.enabled', true);" if a debug flag is set, that way developers can catch all errors regardless of another dev's coding style.

Nikerabbit added a comment.Oct 3 2010, 4:47 PM

It's an PECL extension which requires PHP 5.2.0. I don't see how that is a solution to this bug.

• demon added a comment.Oct 3 2010, 6:48 PM

(In reply to comment #8)

It's an PECL extension which requires PHP 5.2.0. I don't see how that is a
solution to this bug.

It's not. We shouldn't be using a PECL extension to get around poor coding.

Reedy added a comment.Feb 10 2011, 4:53 PM

How many more of these are still about....?

• demon added a comment.Feb 10 2011, 5:04 PM

Too many :(

• demon added a comment.May 15 2011, 2:35 PM

Using r88187:

$ php checkSyntax.php
Building file list...done.
Checking syntax (using php -l, this can take a long time)
Warning in file /opt/local/apache2/htdocs/phase3/includes/api/ApiParse.php: Error supression operator (@) found.
Warning in file /opt/local/apache2/htdocs/phase3/includes/api/ApiQuery.php: Error supression operator (@) found.
Warning in file /opt/local/apache2/htdocs/phase3/includes/api/ApiQueryBacklinks.php: Error supression operator (@) found.
Warning in file /opt/local/apache2/htdocs/phase3/includes/api/ApiQueryExternalLinks.php: Error supression operator (@) found.
Warning in file /opt/local/apache2/htdocs/phase3/includes/cache/MessageCache.php: Error supression operator (@) found.
Warning in file /opt/local/apache2/htdocs/phase3/includes/ConfEditor.php: Error supression operator (@) found.
Warning in file /opt/local/apache2/htdocs/phase3/includes/db/Database.php: Error supression operator (@) found.
Warning in file /opt/local/apache2/htdocs/phase3/includes/db/DatabaseIbm_db2.php: Error supression operator (@) found.
Warning in file /opt/local/apache2/htdocs/phase3/includes/db/DatabaseMysql.php: Error supression operator (@) found.
Warning in file /opt/local/apache2/htdocs/phase3/includes/db/DatabasePostgres.php: Error supression operator (@) found.
Warning in file /opt/local/apache2/htdocs/phase3/includes/diff/DairikiDiff.php: Error supression operator (@) found.
Warning in file /opt/local/apache2/htdocs/phase3/includes/filerepo/ForeignAPIFile.php: Error supression operator (@) found.
Warning in file /opt/local/apache2/htdocs/phase3/includes/filerepo/FSRepo.php: Error supression operator (@) found.
Warning in file /opt/local/apache2/htdocs/phase3/includes/filerepo/LocalFile.php: Error supression operator (@) found.
Warning in file /opt/local/apache2/htdocs/phase3/includes/filerepo/LocalRepo.php: Error supression operator (@) found.
Warning in file /opt/local/apache2/htdocs/phase3/includes/GlobalFunctions.php: Error supression operator (@) found.
Warning in file /opt/local/apache2/htdocs/phase3/includes/HistoryBlob.php: Error supression operator (@) found.
Warning in file /opt/local/apache2/htdocs/phase3/includes/Import.php: Error supression operator (@) found.
Warning in file /opt/local/apache2/htdocs/phase3/includes/json/Services_JSON.php: Error supression operator (@) found.
Warning in file /opt/local/apache2/htdocs/phase3/includes/media/DjVu.php: Error supression operator (@) found.
Warning in file /opt/local/apache2/htdocs/phase3/includes/media/Exif.php: Error supression operator (@) found.
Warning in file /opt/local/apache2/htdocs/phase3/includes/media/SVG.php: Error supression operator (@) found.
Warning in file /opt/local/apache2/htdocs/phase3/includes/objectcache/SqlBagOStuff.php: Error supression operator (@) found.
Warning in file /opt/local/apache2/htdocs/phase3/includes/parser/Parser.php: Error supression operator (@) found.
Warning in file /opt/local/apache2/htdocs/phase3/includes/parser/Parser_LinkHooks.php: Error supression operator (@) found.
Warning in file /opt/local/apache2/htdocs/phase3/includes/profiler/Profiler.php: Error supression operator (@) found.
Warning in file /opt/local/apache2/htdocs/phase3/includes/Revision.php: Error supression operator (@) found.
Warning in file /opt/local/apache2/htdocs/phase3/includes/Sanitizer.php: Error supression operator (@) found.
Warning in file /opt/local/apache2/htdocs/phase3/includes/search/SearchMySQL.php: Error supression operator (@) found.
Warning in file /opt/local/apache2/htdocs/phase3/includes/search/SearchSqlite.php: Error supression operator (@) found.
Warning in file /opt/local/apache2/htdocs/phase3/includes/Setup.php: Error supression operator (@) found.
Warning in file /opt/local/apache2/htdocs/phase3/includes/specials/SpecialLockdb.php: Error supression operator (@) found.
Warning in file /opt/local/apache2/htdocs/phase3/includes/specials/SpecialUnlockdb.php: Error supression operator (@) found.
Warning in file /opt/local/apache2/htdocs/phase3/includes/specials/SpecialWantedpages.php: Error supression operator (@) found.
Warning in file /opt/local/apache2/htdocs/phase3/includes/StreamFile.php: Error supression operator (@) found.
Warning in file /opt/local/apache2/htdocs/phase3/includes/templates/Userlogin.php: Error supression operator (@) found.
Warning in file /opt/local/apache2/htdocs/phase3/includes/User.php: Error supression operator (@) found.
Warning in file /opt/local/apache2/htdocs/phase3/languages/Language.php: Error supression operator (@) found.
Warning in file /opt/local/apache2/htdocs/phase3/maintenance/backup.inc: Error supression operator (@) found.
Warning in file /opt/local/apache2/htdocs/phase3/maintenance/checkImages.php: Error supression operator (@) found.
Warning in file /opt/local/apache2/htdocs/phase3/maintenance/checkSyntax.php: Error supression operator (@) found.
Warning in file /opt/local/apache2/htdocs/phase3/maintenance/importImages.php: Error supression operator (@) found.
Warning in file /opt/local/apache2/htdocs/phase3/maintenance/importUseModWiki.php: Error supression operator (@) found.
Warning in file /opt/local/apache2/htdocs/phase3/maintenance/language/StatOutputs.php: Error supression operator (@) found.
Warning in file /opt/local/apache2/htdocs/phase3/maintenance/Maintenance.php: Error supression operator (@) found.
Warning in file /opt/local/apache2/htdocs/phase3/maintenance/proxy_check.php: Error supression operator (@) found.
Warning in file /opt/local/apache2/htdocs/phase3/maintenance/rebuildFileCache.php: Error supression operator (@) found.
Warning in file /opt/local/apache2/htdocs/phase3/maintenance/storage/fixBug20757.php: Error supression operator (@) found.
Warning in file /opt/local/apache2/htdocs/phase3/maintenance/updateSearchIndex.php: Error supression operator (@) found.
Warning in file /opt/local/apache2/htdocs/phase3/maintenance/updateSpecialPages.php: Error supression operator (@) found.
Warning in file /opt/local/apache2/htdocs/phase3/maintenance/userOptions.inc: Error supression operator (@) found.

• bzimport added a comment.May 16 2011, 8:45 AM

thetooth wrote:

haha, oh wow

• demon added a comment.Jul 6 2011, 10:02 PM

As of r91609:

$ php maintenance/checkSyntax.php
Building file list...done.
Checking syntax (using php -l, this can take a long time)
Warning in file /www/phase3/includes/filerepo/ForeignAPIFile.php: Error supression operator (@) found.
Warning in file /www/phase3/includes/json/Services_JSON.php: Error supression operator (@) found.
Warning in file /www/phase3/includes/libs/jsminplus.php: trailing ?> found.
Warning in file /www/phase3/includes/media/Exif.php: Error supression operator (@) found.
Warning in file /www/phase3/includes/parser/Parser.php: Error supression operator (@) found.
Warning in file /www/phase3/includes/parser/Parser_LinkHooks.php: Error supression operator (@) found.
Warning in file /www/phase3/includes/profiler/Profiler.php: Error supression operator (@) found.
Warning in file /www/phase3/includes/Revision.php: Error supression operator (@) found.
Warning in file /www/phase3/includes/Sanitizer.php: Error supression operator (@) found.
Warning in file /www/phase3/includes/search/SearchMySQL.php: Error supression operator (@) found.
Warning in file /www/phase3/includes/search/SearchSqlite.php: Error supression operator (@) found.
Warning in file /www/phase3/includes/Setup.php: Error supression operator (@) found.
Warning in file /www/phase3/includes/specials/SpecialWantedpages.php: Error supression operator (@) found.
Warning in file /www/phase3/includes/StreamFile.php: Error supression operator (@) found.
Warning in file /www/phase3/includes/User.php: Error supression operator (@) found.
Warning in file /www/phase3/maintenance/backup.inc: Error supression operator (@) found.
Warning in file /www/phase3/maintenance/checkImages.php: Error supression operator (@) found.
Warning in file /www/phase3/maintenance/importImages.php: Error supression operator (@) found.
Warning in file /www/phase3/maintenance/importUseModWiki.php: Error supression operator (@) found.
Warning in file /www/phase3/maintenance/language/StatOutputs.php: Error supression operator (@) found.
Warning in file /www/phase3/maintenance/Maintenance.php: Error supression operator (@) found.
Warning in file /www/phase3/maintenance/proxy_check.php: Error supression operator (@) found.
Warning in file /www/phase3/maintenance/storage/fixBug20757.php: Error supression operator (@) found.
Warning in file /www/phase3/maintenance/userOptions.inc: Error supression operator (@) found.

Done! 1134 files checked, 0 failures and 24 warnings found

• demon added a comment.Jul 6 2011, 10:07 PM

(In reply to comment #14)

As of r91609:

$ php maintenance/checkSyntax.php
Building file list...done.
Checking syntax (using php -l, this can take a long time)
Warning in file /www/phase3/includes/filerepo/ForeignAPIFile.php: Error
supression operator (@) found.

Scratch that one, fixed in r91611.

Krinkle added a project: Technical-Debt.Dec 10 2014, 2:15 PM
Krinkle set Security to None.
Krinkle removed a subscriber: Unknown Object (MLST).
lcawte subscribed.EditedDec 11 2014, 7:21 PM

As of 90fd90d8ce3926e448c525138f644b74231e6402, we have four core files using the @ operator.

lewiscawte@lewis-laptop:/code/mediawiki$ php maintenance/checkSyntax.php
Building file list...done.
Checking syntax (using  php -l, this can take a long time)
Warning in file /code/mediawiki/includes/GlobalFunctions.php: Error supression operator (@) found.
Warning in file /code/mediawiki/includes/Setup.php: Error supression operator (@) found.
Warning in file /code/mediawiki/maintenance/userOptions.inc: Error supression operator (@) found.
Warning in file /code/mediawiki/maintenance/Maintenance.php: Error supression operator (@) found.
found.

Done! 1511 files checked, 0 failures and 4 warnings found
Ricordisamoa subscribed.Aug 10 2015, 3:41 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptAug 10 2015, 3:41 PM
Ricordisamoa added a comment.Aug 10 2015, 4:12 PM

Warnings in Setup.php and Maintenance.php have been fixed by rMWf6e5079a6964: Use mediawiki/at-ease library for suppressing warnings

Danny_B moved this task from Unsorted to Needs removal on the Technical-Debt board.Jan 22 2016, 11:51 PM
Krinkle edited projects, added Technical-Debt (Deprecation process); removed Technical-Debt.Jul 13 2018, 11:11 PM
Krinkle edited projects, added Technical-Debt; removed Technical-Debt (Deprecation process).Jul 13 2018, 11:23 PM
gerritbot added a comment.Jul 18 2019, 11:47 AM

Change 516619 had a related patch set uploaded (by D3r1ck01; owner: Derick Alangi):
[mediawiki/core@master] Avoid the use of silence operator (@) and use AtEase methods

https://gerrit.wikimedia.org/r/516619

gerritbot added a project: Patch-For-Review.Jul 18 2019, 11:47 AM
gerritbot added a comment.Jul 23 2019, 4:23 PM

Change 516619 merged by jenkins-bot:
[mediawiki/core@master] Avoid the use of silence operator (@) and use AtEase methods

https://gerrit.wikimedia.org/r/516619

ReleaseTaggerBot added a project: MW-1.34-notes (1.34.0-wmf.16; 2019-07-30).Jul 23 2019, 5:00 PM
Maintenance_bot removed a project: Patch-For-Review.Jul 23 2019, 5:10 PM
WDoranWMF added a project: Platform Team Workboards (Clinic Duty Team).Jul 24 2019, 6:29 PM
WDoranWMF moved this task from Inbox to External Code Review Completed on the Platform Team Workboards (Clinic Duty Team) board.
WDoranWMF moved this task from External Code Review Completed to Done on the Platform Team Workboards (Clinic Duty Team) board.
mgrabovsky unsubscribed.Jul 25 2019, 6:14 AM
CCicalese_WMF closed this task as Resolved.Apr 28 2020, 8:49 PM
CCicalese_WMF claimed this task.
CCicalese_WMF subscribed.

Marking as Resolved as it is in the Done column. Feel free to reopen if there is remaining work.

Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL