Page MenuHomePhabricator

[Spike 1hr] What technical problems do we need to fix from mobilefrontend
Closed, ResolvedPublic

Description

Event Timeline

PHPMessDetector complains a lot

raynor@DellE6540:~/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes(masterβ—‹) Β» phpmd . text codesize,cleancode,design | grep -v 'Avoid using static' | grep -v 'uses an else'

/home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/MenuBuilder.php:43	The method insert has a boolean flag argument $isJSOnly, which is a certain sign of a Single Responsibility Principle violation.
/home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/MenuBuilder.php:81	The method insertAfter has a boolean flag argument $isJSOnly, which is a certain sign of a Single Responsibility Principle violation.
/home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/MobileContext.php:13	The class MobileContext has 1240 lines of code. Current threshold is 1000. Avoid really long classes.
/home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/MobileContext.php:13	The class MobileContext has 49 public methods and attributes. Consider to reduce the number of public items under 45.
/home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/MobileContext.php:13	The class MobileContext has 21 fields. Consider to redesign MobileContext to keep the number of fields under 15.
/home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/MobileContext.php:13	The class MobileContext has 33 non-getter- and setter-methods. Consider refactoring MobileContext to keep number of methods under 25.
/home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/MobileContext.php:13	The class MobileContext has 23 public methods. Consider refactoring MobileContext to keep number of public methods under 10.
/home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/MobileContext.php:13	The class MobileContext has an overall complexity of 167 which is very high. The configured complexity threshold is 50.
/home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/MobileContext.php:13	The class MobileContext has a coupling between objects value of 13. Consider to reduce the number of dependencies under 13.
/home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/MobileContext.php:812	The method getMobileUrl has a boolean flag argument $forceHttps, which is a certain sign of a Single Responsibility Principle violation.
/home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/MobileFormatter.php:11	The class MobileFormatter has an overall complexity of 89 which is very high. The configured complexity threshold is 50.
/home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/MobileFormatter.php:115	The method enableTOCPlaceholder has a boolean flag argument $flag, which is a certain sign of a Single Responsibility Principle violation.
/home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/MobileFormatter.php:124	The method enableExpandableSections has a boolean flag argument $flag, which is a certain sign of a Single Responsibility Principle violation.
/home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/MobileFormatter.php:132	The method setIsMainPage has a boolean flag argument $value, which is a certain sign of a Single Responsibility Principle violation.
/home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/MobileFormatter.php:146	The method filterContent has a boolean flag argument $removeDefaults, which is a certain sign of a Single Responsibility Principle violation.
/home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/MobileFormatter.php:146	The method filterContent has a boolean flag argument $removeReferences, which is a certain sign of a Single Responsibility Principle violation.
/home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/MobileFormatter.php:146	The method filterContent has a boolean flag argument $removeImages, which is a certain sign of a Single Responsibility Principle violation.
/home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/MobileFormatter.php:147	The method filterContent has a boolean flag argument $showFirstParagraphBeforeInfobox, which is a certain sign of a Single Responsibility Principle violation.
/home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/MobileFormatter.php:458	The method parseMainPage() has a Cyclomatic Complexity of 10. The configured cyclomatic complexity threshold is 10.
/home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/MobileFormatter.php:458	The method parseMainPage() has an NPath complexity of 208. The configured NPath complexity threshold is 200.
/home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/MobileFormatter.php:541	The method makeSections() has a Cyclomatic Complexity of 11. The configured cyclomatic complexity threshold is 10.
/home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/MobileFrontend.body.php:34	The method DOMParse() has a Cyclomatic Complexity of 11. The configured cyclomatic complexity threshold is 10.
/home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/MobileFrontend.body.php:34	The method DOMParse has a boolean flag argument $isBeta, which is a certain sign of a Single Responsibility Principle violation.
/home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/MobileFrontend.body.php:99	The method getUserPageContent() has an NPath complexity of 625. The configured NPath complexity threshold is 200.
/home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/MobileFrontend.hooks.php:17	The class MobileFrontendHooks has 1412 lines of code. Current threshold is 1000. Avoid really long classes.
/home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/MobileFrontend.hooks.php:17	The class MobileFrontendHooks has 44 non-getter- and setter-methods. Consider refactoring MobileFrontendHooks to keep number of methods under 25.
/home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/MobileFrontend.hooks.php:17	The class MobileFrontendHooks has 41 public methods. Consider refactoring MobileFrontendHooks to keep number of public methods under 10.
/home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/MobileFrontend.hooks.php:17	The class MobileFrontendHooks has an overall complexity of 173 which is very high. The configured complexity threshold is 50.
/home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/MobileFrontend.hooks.php:17	The class MobileFrontendHooks has a coupling between objects value of 23. Consider to reduce the number of dependencies under 13.
/home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/MobileFrontend.hooks.php:186	The method onOutputPageBeforeHTML() has a Cyclomatic Complexity of 10. The configured cyclomatic complexity threshold is 10.
/home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/MobileFrontend.hooks.php:591	The method onSpecialPageBeforeExecute() has a Cyclomatic Complexity of 11. The configured cyclomatic complexity threshold is 10.
/home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/MobileFrontend.hooks.php:723	The method onBeforePageDisplay() has a Cyclomatic Complexity of 26. The configured cyclomatic complexity threshold is 10.
/home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/MobileFrontend.hooks.php:723	The method onBeforePageDisplay() has an NPath complexity of 186624. The configured NPath complexity threshold is 200.
/home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/MobileFrontend.hooks.php:723	The method onBeforePageDisplay() has 128 lines of code. Current threshold is set to 100. Avoid really long methods.
/home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/MobileFrontend.skin.hooks.php:311	The method getSitename has a boolean flag argument $withPossibleTrademark, which is a certain sign of a Single Responsibility Principle violation.
/home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/api/ApiMobileView.php:10	The class ApiMobileView has an overall complexity of 156 which is very high. The configured complexity threshold is 50.
/home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/api/ApiMobileView.php:10	The class ApiMobileView has a coupling between objects value of 20. Consider to reduce the number of dependencies under 13.
/home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/api/ApiMobileView.php:49	The method execute() has a Cyclomatic Complexity of 48. The configured cyclomatic complexity threshold is 10.
/home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/api/ApiMobileView.php:49	The method execute() has an NPath complexity of 6252134400. The configured NPath complexity threshold is 200.
/home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/api/ApiMobileView.php:49	The method execute() has 210 lines of code. Current threshold is set to 100. Avoid really long methods.
/home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/api/ApiMobileView.php:387	The method getRequestedSectionIds() has a Cyclomatic Complexity of 15. The configured cyclomatic complexity threshold is 10.
/home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/api/ApiMobileView.php:487	The method parseSectionsData has a boolean flag argument $useTidy, which is a certain sign of a Single Responsibility Principle violation.
/home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/api/ApiMobileView.php:529	The method getData() has a Cyclomatic Complexity of 25. The configured cyclomatic complexity threshold is 10.
/home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/api/ApiMobileView.php:529	The method getData() has an NPath complexity of 179200. The configured NPath complexity threshold is 200.
/home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/api/ApiMobileView.php:529	The method getData() has 148 lines of code. Current threshold is set to 100. Avoid really long methods.
/home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/api/ApiMobileView.php:712	The method addPageImage() has a Cyclomatic Complexity of 22. The configured cyclomatic complexity threshold is 10.
/home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/api/ApiMobileView.php:712	The method addPageImage() has an NPath complexity of 188280. The configured NPath complexity threshold is 200.
/home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/models/MobilePage.php:45	The method __construct has a boolean flag argument $file, which is a certain sign of a Single Responsibility Principle violation.
/home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/models/MobilePage.php:147	The method getSmallThumbnailHtml has a boolean flag argument $useBackgroundImage, which is a certain sign of a Single Responsibility Principle violation.
/home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/models/MobilePage.php:158	The method getPageImageHtml has a boolean flag argument $useBackgroundImage, which is a certain sign of a Single Responsibility Principle violation.
/home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/skins/MinervaTemplate.php:231	The method getPreContentHtml() has an NPath complexity of 625. The configured NPath complexity threshold is 200.
/home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/skins/SkinMinerva.php:13	The class SkinMinerva has 1379 lines of code. Current threshold is 1000. Avoid really long classes.
/home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/skins/SkinMinerva.php:13	The class SkinMinerva has 31 non-getter- and setter-methods. Consider refactoring SkinMinerva to keep number of methods under 25.
/home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/skins/SkinMinerva.php:13	The class SkinMinerva has an overall complexity of 179 which is very high. The configured complexity threshold is 50.
/home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/skins/SkinMinerva.php:13	The class SkinMinerva has a coupling between objects value of 23. Consider to reduce the number of dependencies under 13.
/home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/skins/SkinMinerva.php:216	The method doEditSectionLink has a boolean flag argument $lang, which is a certain sign of a Single Responsibility Principle violation.
/home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/skins/SkinMinerva.php:1244	The method getContextSpecificModules() has a Cyclomatic Complexity of 16. The configured cyclomatic complexity threshold is 10.
/home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/skins/SkinMinerva.php:1244	The method getContextSpecificModules() has an NPath complexity of 1800. The configured NPath complexity threshold is 200.
/home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/specials/MobileSpecialPageFeed.php:86	The method renderFeedItemHtml() has a Cyclomatic Complexity of 10. The configured cyclomatic complexity threshold is 10.
/home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/specials/MobileSpecialPageFeed.php:86	The method renderFeedItemHtml() has an NPath complexity of 288. The configured NPath complexity threshold is 200.
/home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/specials/MobileSpecialPageFeed.php:87	The method renderFeedItemHtml has a boolean flag argument $isAnon, which is a certain sign of a Single Responsibility Principle violation.
/home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/specials/MobileSpecialPageFeed.php:87	The method renderFeedItemHtml has a boolean flag argument $isMinor, which is a certain sign of a Single Responsibility Principle violation.
/home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/specials/SpecialMobileDiff.php:9	The class SpecialMobileDiff has an overall complexity of 56 which is very high. The configured complexity threshold is 50.
/home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/specials/SpecialMobileDiff.php:59	The method getRevisionsToCompare() has a Cyclomatic Complexity of 12. The configured cyclomatic complexity threshold is 10.
/home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/specials/SpecialMobileDiff.php:220	The method showDiff() has a Cyclomatic Complexity of 11. The configured cyclomatic complexity threshold is 10.
/home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/specials/SpecialMobileDiff.php:220	The method showDiff() has an NPath complexity of 1440. The configured NPath complexity threshold is 200.
/home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/specials/SpecialMobileDiff.php:380	The method getMobileUrlFromDesktop() has a Cyclomatic Complexity of 14. The configured cyclomatic complexity threshold is 10.
/home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/specials/SpecialMobileDiff.php:380	The method getMobileUrlFromDesktop() has an NPath complexity of 918. The configured NPath complexity threshold is 200.
/home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/specials/SpecialMobileHistory.php:180	The method showRow() has a Cyclomatic Complexity of 11. The configured cyclomatic complexity threshold is 10.
/home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/specials/SpecialMobileLanguages.php:136	The method executeWhenAvailable() has a Cyclomatic Complexity of 12. The configured cyclomatic complexity threshold is 10.
/home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/specials/SpecialMobileOptions.php:89	The method addSettingsForm() has an NPath complexity of 2000. The configured NPath complexity threshold is 200.
/home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/specials/SpecialMobileOptions.php:296	The method getURL has a boolean flag argument $fullUrl, which is a certain sign of a Single Responsibility Principle violation.
/home/raynor/Work/wmf/mediawiki-vagrant/mediawiki/extensions/MobileFrontend/includes/specials/SpecialMobileWatchlist.php:11	The class SpecialMobileWatchlist has a coupling between objects value of 14. Consider to reduce the number of dependencies under 13

PHPStorm inspection also complains a lot:

    • 9 deprecated calls
    • 6 incosistent return points
    • 4 parameters mismatch in declaration
    • ~40 unsued variables/parameters
  • 12 Type combaility issues
  • 74 undefined issues

Type compability issues and undefined issues might be false positive

CodeCoverage is pretty low (only 30%)

Phan also complains but not that much:

extensions/MobileFrontend/includes/MobileFrontend.hooks.php:555 PhanUndeclaredTypeParameter Parameter of undeclared type \AbuseFilterVariableHolder
extensions/MobileFrontend/includes/MobileFrontend.hooks.php:1001 PhanUndeclaredTypeParameter Parameter of undeclared type \CentralAuthUser
extensions/MobileFrontend/includes/MobileFrontend.hooks.php:1020 PhanUndeclaredTypeParameter Parameter of undeclared type \CentralAuthUser
extensions/MobileFrontend/includes/diff/InlineDifferenceEngine.php:121 PhanUndeclaredFunction Call to undeclared function \wikidiff2_inline_diff()
extensions/MobileFrontend/tests/phpunit/devices/CustomHeaderDeviceDetectorTest.php:18 PhanUndeclaredTypeProperty Property \Tests\MobileFrontend\Devices\CustomHeaderDeviceDetectorTest::config has undeclared type \Tests\MobileFrontend\Devices\GlobalVarConfig
extensions/MobileFrontend/tests/phpunit/devices/CustomHeaderDeviceDetectorTest.php:23 PhanUndeclaredTypeProperty Property \Tests\MobileFrontend\Devices\CustomHeaderDeviceDetectorTest::detector has undeclared type \Tests\MobileFrontend\Devices\UADeviceDetector

Things to improve:

  • high code complexity, long functions, even longer classes
  • lots of coupling with MediaWiki Core, extension changes MW behaviour bit too much
    • time for SOA ?
  • MobileFrontend - is an extension or is it a Skin? - this extension handles so much it could be break into 2-3 separate extensions
  • missing tests (30% code coverage is not enough)
  • on-fly html modifications are error-prone, also very slow
  • scaffolding for ongoing experiments/beta features
  • extension contains logic required only on Wikimedia cluster (not 3rd party instansces)