Page MenuHomePhabricator

Fix multiple PHP class declarations in one file
Open, Needs TriagePublic

Description

There should be one PHP class per file, not multiple. Any files where this isn't the case should be fixed up.

Find a PHP file that has more than one class/interface in it (in MediaWiki core, a WMF deployed extension, or any other extension), and split the classes into individual files, that are named the same as the class itself.

Update any autoloader references to the class in the old file to point in the new file. In core this is autoload.php, in extensions this may be in the PHP entry point, or in extension.json

Details

Related Gerrit Patches:
mediawiki/extensions/CentralNotice : wmf_deployFix multiple PHP class declarations in one file
mediawiki/extensions/CentralNotice : masterFix multiple PHP class declarations in one file
mediawiki/extensions/CirrusSearch : masterFix multiple PHP class declarations in one file
mediawiki/extensions/BlueSpiceFoundation : masterFix multiple PHP class declarations in one file
mediawiki/extensions/Wikispeech : masterFix multiple PHP class declarations in one file
mediawiki/extensions/UploadWizard : masterFix multiple PHP class declarations in one file
mediawiki/extensions/WikiLexicalData : masterFix multiple PHP class declarations in one file
mediawiki/extensions/Echo : masterFix multiple PHP class declarations in one file in tests
mediawiki/extensions/Echo : masterFix multiple PHP class declarations in one file
mediawiki/core : masterFix multiple PHP class declarations in one file in includes/htmlform
mediawiki/core : masterFix multiple PHP class declarations in one file in includes/diff
mediawiki/core : masterFix multiple PHP class declarations in one file in includes/specialse
mediawiki/core : masterFix multiple PHP class declarations in one file in includes/auth
mediawiki/core : masterFix multiple PHP class declarations in one file in includes/utils
mediawiki/core : masterFix multiple PHP class declarations in one file in includes/logging
mediawiki/core : masterFix multiple PHP class declarations in one file in includes/parser
mediawiki/core : masterFix multiple PHP class declarations in one file in includes/cache

Related Objects

Event Timeline

Reedy created this task.Oct 10 2017, 12:34 AM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptOct 10 2017, 12:34 AM
Reedy updated the task description. (Show Details)Oct 10 2017, 1:01 AM

Find a PHP file that has more than one class/interface in it

@Reedy: How would I or someone else do that? What to grep for, basically?

Aklapper renamed this task from Multiple class declarations in one file to Fix multiple PHP class declarations in one file.Nov 20 2017, 11:01 PM
Reedy added a comment.Dec 5 2017, 4:11 PM

I suspect, it's possible to grep for this... But I imagine it would need some post-processing...

Grep for php files with a class in it, get the file name, group and order by file name.... and exclude if after grouping it's just 1?

Seems excessively complex unless we provided them the command to run

Much easier option is to just (install and) use phpcs, and disable the "MediaWiki.Files.OneClassPerFile.MultipleFound" rule, and fix some....

So... PhpStorm makes a "nice" HTML output of this. I've uploaded a copy for mediawiki (core) includes folder to https://people.wikimedia.org/~reedy/inspections/dupeclasses/includes

Then there's 27 more in maintenance.. 84 in tests (I don't think they're both quite so important... Maybe?)

Ideally, outside of core, we want to target WMF deployed extensions/skins first...

1288 in all extensions. 33 in skins

Various breakdowns/sub folders scanned in https://people.wikimedia.org/~reedy/inspections/dupeclasses/

Reedy updated the task description. (Show Details)Dec 5 2017, 4:11 PM
Pppery added a subscriber: Pppery.

<adding parent task because the presence of multiple classes in the same file is a phpcs violation>

I suspect, it's possible to grep for this... But I imagine it would need some post-processing...
Grep for php files with a class in it, get the file name, group and order by file name.... and exclude if after grouping it's just 1?

It's much more simple:

grep "^[a-z]* *class " -rc includes/ | grep -v ':[01]$'

Who would mentor this in Google-Code-in-2017?

It's much more simple:
grep "^[a-z]* *class " -rc includes/ | grep -v ':[01]$'

Output for /includes subfolder in git master of MediaWiki extensions listed below.
Should I start creating tasks on the GCI website for these? Asking as I'm very clueless about these things.

1$:acko\> more ./T177809.sh
2for repos in $(ls); do
3 if [ -d $repos ]; then
4 cd $repos
5 if [ -d "includes" ]; then
6 grep "^[a-z]* *class " -rc includes/ | grep -v ':[01]$'
7 fi
8 cd ..
9 fi
10done
11$:acko\> ./T177809.sh
12==== BlueSpiceFoundation
13includes/ExtJSStoreParams.php:2
14includes/validator/Validator.class.php:9
15includes/html/htmlformfields/HTMLFormFieldOverrides.php:6
16includes/html/htmlformfields/HTMLMultiSelectEx.php:2
17includes/Common.php:9
18
19==== CentralNotice
20includes/Campaign.php:2
21includes/Banner.php:4
22includes/CNBannerPager.php:2
23includes/MixinController.php:2
24
25==== CirrusSearch
26includes/BuildDocument/Builder.php:2
27includes/BuildDocument/Completion/SuggestScoring.php:4
28includes/Sanity/Remediator.php:2
29includes/Search/RescoreBuilders.php:17
30includes/Search/CrossProjectBlockScorer.php:6
31includes/Search/ResultsType.php:6
32
33==== CommunityTwitter
34includes/OAuth.php:11
35
36==== DataTransfer
37includes/DT_XMLParser.php:3
38includes/DT_PageStructure.php:2
39
40==== Echo
41includes/DiffParser.php:2
42includes/ContainmentSet.php:4
43
44==== EditNotify
45includes/ENPageStructure.php:2
46
47==== EducationProgram
48includes/CourseUndeletionHelper.php:2
49includes/LogFormatter.php:3
50includes/OrgDeletionHelper.php:2
51includes/Events/TimelineGroup.php:4
52
53==== EventLogging
54includes/JsonSchema.php:5
55
56==== Flow
57includes/Import/LiquidThreadsApi/Objects.php:10
58includes/Import/LiquidThreadsApi/CachedData.php:4
59includes/Import/LiquidThreadsApi/Iterators.php:3
60includes/Import/LiquidThreadsApi/Source.php:4
61includes/Import/Importer.php:5
62includes/Model/UUID.php:2
63includes/Formatter/ChangesListQuery.php:2
64includes/Formatter/ContributionsQuery.php:3
65includes/Formatter/CheckUserQuery.php:2
66includes/Formatter/AbstractQuery.php:2
67includes/Formatter/RevisionViewQuery.php:4
68includes/Data/Listener/WatchTopicListener.php:2
69includes/Exception/ExceptionHandling.php:17
70
71==== FundraisingEmailUnsubscribe
72includes/MediaWikiTwig.php:2
73
74==== MarkAsHelpful
75includes/MarkAsHelpfulItem.php:3
76
77==== NSFileRepo
78includes/filerepo/file/NSLocalFile.php:2
79
80==== OOUIPlayground
81includes/argument-filters/GroupElementFilter.php:2
82includes/WidgetInfo.php:3
83includes/CodeRenderer.php:7
84includes/WidgetFactory.php:2
85
86==== PageTriage
87includes/PageTriage.php:2
88includes/ArticleMetadata.php:9
89includes/PageTriageUtil.php:2
90
91==== PhpTags
92includes/Renderer.php:2
93
94==== SecurePoll
95includes/main/Store.php:3
96includes/user/Auth.php:3
97includes/crypt/Crypt.php:2
98includes/pages/CreatePage.php:3
99includes/pages/DetailsPage.php:2
100includes/pages/ListPage.php:2
101includes/pages/EntryPage.php:2
102includes/ballots/Ballot.php:2
103
104==== SemanticExpressiveness
105includes/ShortQueryResult.php:2
106
107==== SemanticPageMaker
108includes/widgets/SPM_WidgetDesignPage2.php:2
109includes/widgets/SPM_WidgetDesignPage.php:2
110includes/widgets/SPM_WidgetPage.php:2
111includes/widgets/datatype/SPM_WT_File.php:2
112includes/widgets/datatype/SPM_WT_Widget.php:2
113includes/widgets/datatype/SPM_WT_Page.php:2
114includes/widgets/datatype/SPM_WT_Media.php:2
115
116==== SpellingDictionary
117includes/AdminRights.php:4
118
119==== UploadWizard
120includes/specials/SpecialUploadWizard.php:2
121
122==== WikiLexicalData
123includes/specials/SpecialOWAddFromExternalAPI.php:2
124includes/specials/ExternalWordnik.php:3
125
126==== Wikispeech
127includes/CleanedContent.php:2

nikitavbv added a subscriber: nikitavbv.

I will work on fixing this

There are quite a lot of such files in core so to make things simplier to manage, review and merge I will split it into multiple patches (by directories)

Change 405536 had a related patch set uploaded (by Phantom42; owner: Phantom42):
[mediawiki/core@master] Fix multiple PHP class declarations in one file in includes/cache

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

Change 405545 had a related patch set uploaded (by Phantom42; owner: Phantom42):
[mediawiki/core@master] Fix multiple PHP class declarations in one file in includes/parser

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

Change 405547 had a related patch set uploaded (by Phantom42; owner: Phantom42):
[mediawiki/core@master] Fix multiple PHP class declarations in one file in includes/logging

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

Change 405548 had a related patch set uploaded (by Phantom42; owner: Phantom42):
[mediawiki/core@master] Fix multiple PHP class declarations in one file in includes/utils

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

Change 405551 had a related patch set uploaded (by Phantom42; owner: Phantom42):
[mediawiki/core@master] Fix multiple PHP class declarations in one file in includes/specialse

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

Change 405552 had a related patch set uploaded (by Phantom42; owner: Phantom42):
[mediawiki/core@master] Fix multiple PHP class declarations in one file in includes/auth

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

Change 405554 had a related patch set uploaded (by Phantom42; owner: Phantom42):
[mediawiki/core@master] Fix multiple PHP class declarations in one file in includes/diff

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

Change 405555 had a related patch set uploaded (by Phantom42; owner: Phantom42):
[mediawiki/core@master] Fix multiple PHP class declarations in one file in includes/htmlform

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

Anomie added a subscriber: Anomie.Jan 20 2018, 11:08 PM

Considering this will already be done by T166010: The Great Namespaceization and Reorg, it seems somewhat redundant to split files now only to rename them later. See discussion on this topic on T173799: Implement a PSR-4 autoloader in MediaWiki core.

nikitavbv added a comment.EditedJan 20 2018, 11:24 PM

@Anomie Thank you for giving links to those tasks! You are right, there is no need to split files now if general reogranization will be done. So can we close this task as "Declined"?

There's still various extensions that need fixing up... I think those bugs are just for MW Core

@Reedy Thank you! Then I will abandon my changes to core and fix extensions instead.

Change 405536 abandoned by Phantom42:
Fix multiple PHP class declarations in one file in includes/cache

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

Change 405545 abandoned by Phantom42:
Fix multiple PHP class declarations in one file in includes/parser

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

Change 405547 abandoned by Phantom42:
Fix multiple PHP class declarations in one file in includes/logging

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

Change 405548 abandoned by Phantom42:
Fix multiple PHP class declarations in one file in includes/utils

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

Change 405552 abandoned by Phantom42:
Fix multiple PHP class declarations in one file in includes/auth

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

Change 405551 abandoned by Phantom42:
Fix multiple PHP class declarations in one file in includes/specialse

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

Change 405554 abandoned by Phantom42:
Fix multiple PHP class declarations in one file in includes/diff

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

Change 405555 abandoned by Phantom42:
Fix multiple PHP class declarations in one file in includes/htmlform

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

Change 405608 had a related patch set uploaded (by Phantom42; owner: Phantom42):
[mediawiki/extensions/Echo@master] Fix multiple PHP class declarations in one file

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

Change 405608 merged by jenkins-bot:
[mediawiki/extensions/Echo@master] Fix multiple PHP class declarations in one file

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

Change 405663 had a related patch set uploaded (by Phantom42; owner: Phantom42):
[mediawiki/extensions/Wikispeech@master] Fix multiple PHP class declarations in one file

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

Change 405665 had a related patch set uploaded (by Phantom42; owner: Phantom42):
[mediawiki/extensions/WikiLexicalData@master] Fix multiple PHP class declarations in one file

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

Pppery removed a subscriber: Pppery.Jan 22 2018, 1:49 AM

Change 405718 had a related patch set uploaded (by Phantom42; owner: Phantom42):
[mediawiki/extensions/Echo@master] Fix multiple PHP class declarations in one file in tests

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

Anomie removed a subscriber: Anomie.Jan 22 2018, 4:04 PM

Change 405718 merged by jenkins-bot:
[mediawiki/extensions/Echo@master] Fix multiple PHP class declarations in one file in tests

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

While working on this, I have noticed that in some extensions (for example WikiLexicalData) some classes are outside includes directory (need to take this into account while grep-ing). Additionally, some files are not loaded with autoloader, but are required from other files with require_once. I am not sure what to do with those. Do we need to replace require_once with autoloading first?

Change 406136 had a related patch set uploaded (by Phantom42; owner: Phantom42):
[mediawiki/extensions/UploadWizard@master] Fix multiple PHP class declarations in one file

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

Change 406136 merged by jenkins-bot:
[mediawiki/extensions/UploadWizard@master] Fix multiple PHP class declarations in one file

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

Change 405663 merged by jenkins-bot:
[mediawiki/extensions/Wikispeech@master] Fix multiple PHP class declarations in one file

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

Change 444371 had a related patch set uploaded (by Phantom42; owner: Phantom42):
[mediawiki/extensions/BlueSpiceFoundation@master] Fix multiple PHP class declarations in one file

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

Change 444371 merged by jenkins-bot:
[mediawiki/extensions/BlueSpiceFoundation@master] Fix multiple PHP class declarations in one file

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

Change 445779 had a related patch set uploaded (by Phantom42; owner: Phantom42):
[mediawiki/extensions/CentralNotice@master] Fix multiple PHP class declarations in one file

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

Change 453603 had a related patch set uploaded (by Phantom42; owner: Phantom42):
[mediawiki/extensions/CirrusSearch@master] Fix multiple PHP class declarations in one file

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

Change 445779 merged by jenkins-bot:
[mediawiki/extensions/CentralNotice@master] Fix multiple PHP class declarations in one file

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

Change 474396 had a related patch set uploaded (by AndyRussG; owner: Phantom42):
[mediawiki/extensions/CentralNotice@wmf_deploy] Fix multiple PHP class declarations in one file

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

Change 474396 merged by jenkins-bot:
[mediawiki/extensions/CentralNotice@wmf_deploy] Fix multiple PHP class declarations in one file

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