Page MenuHomePhabricator

Create a sniff to enforce alphabetical order of literal arrays
Closed, ResolvedPublic

Description

I suspect we have many places in code with associative arrays where the best sorting order is alphabetic order. It would be useful to enforce that with a simple annotation so that if the order is not maintained, the tests would fail.

For example:

// Sniffer: Sorted array
protected static $validators = [
	'BraceBalance' => BraceBalanceValidator::class,
	'EscapeCharacter' => EscapeCharacterValidator::class,::class,
	'UnicodePlural' => UnicodePluralValidator::class,
	'WikiLink' => WikiLinkValidator::class,
	'WikiParameter' => WikiParameterValidator::class
];

Would something like this be possible?

Event Timeline

Change 606799 had a related patch set uploaded (by Umherirrender; owner: Umherirrender):
[mediawiki/tools/codesniffer@master] Sort arrays with new @require-sorted-array

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

Given that this new sniff does nothing unless manually applied with @require-sorted-array, unless there are any objections soon I plan to +2 the patch

Given that this new sniff does nothing unless manually applied with @require-sorted-array, unless there are any objections soon I plan to +2 the patch

"soon" - I just gave a +2

Change 606799 merged by jenkins-bot:
[mediawiki/tools/codesniffer@master] Sort arrays with new @phpcs-require-sorted-array

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

DannyS712 assigned this task to Umherirrender.
DannyS712 removed a project: Patch-For-Review.

In Wikibase, the autofixer of this sniff seems to indent the array with spaces instead of tabs. That doesn’t seem to happen in the sniff’s test, and as far as I can tell the sniff doesn’t deal with the indentation directly – do we need to do something to tell phpcs (in general) to use tabs instead of spaces?

phpcs replaces all spaces with tabs before processing internally. The sniff itself does not change the tabs to spaces, but when the spaces are moved around the tabs are not preserved and lost.

When enable Generic.WhiteSpace.DisallowSpaceIndent.SpacesUsed it works, because that sniff repairs it back to tabs.

But it also easy to fix on call of File::getTokensAsString, will upload a patch set for it.

Change 668136 had a related patch set uploaded (by Umherirrender; owner: Umherirrender):
[mediawiki/tools/codesniffer@master] Preserve tabs when autofix @phpcs-require-sorted-array

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

Odd, we should have that DisallowSpaceIndent in the Wikibase ruleset (it’s in the MediaWiki one and we don’t disable it). But that fix looks good, thanks!

…and of course, when I tried to test your fix, I couldn’t reproduce the issue in Wikibase with or without it :D but I +2ed it anyways.

Odd, we should have that DisallowSpaceIndent in the Wikibase ruleset (it’s in the MediaWiki one and we don’t disable it). But that fix looks good, thanks!

It was disabled and the patch set to enable it was merged today - https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Wikibase/+/667323

Change 668136 merged by jenkins-bot:
[mediawiki/tools/codesniffer@master] Preserve tabs when autofix @phpcs-require-sorted-array

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