Set tab-width in the base ruleset file
Closed, ResolvedPublic
Actions

Assigned To

Authored By

	Daimona
	Jan 24 2020, 1:02 PM

Description

Since we use tabs to indent lines, we should tell PHPCS how many spaces a tab equals to. Usually (e.g. in IDEs), a tab corresponds to 4 spaces, and we should tell this to PHPCS by setting tab-width.

Not setting this option causes some whitespace-related issues to behave badly, see T243529. People upstream suggested setting the tab width to let the InterfaceWrongIndent sniff and others behave correctly.

However, there's an important side-effect in doing so: the LineLength sniff. Right now, the following line:

\t\t\tFOO

(where \t is a tab) is counted as having 6 characters (3 tabs + 3 letters). By setting tab-width to 4, it will be counted as having 15 characters (4*3 + 3). Hence, we'll start getting lots of new warnings about long lines, see test run. Of note, I've always found it weird to have tabs counted as 1, but I don't know whether it's intentional/there are historical reasons to do so. For sure, it makes more sense (IMHO) to count a tab as 4 characters when computing line length.

Either way, I see three possible outcomes:

We don't set tabWidth, but then we'll have to disable InterfaceWrongIndent, and possibly other space-related sniffs.
We set tabWidth, but we also increase the max line length (e.g. by 8-10 chars, assuming that the average indent is around 2.5-3 tabs).
We set tabWidth, and also fix all the long lines.

IMHO, 3 is the best option, but 2 would be fine for now.

Details

Subject	Repo	Branch	Lines +/-
Add tab-width=4 and increase line length to 120	mediawiki/tools/codesniffer	19.x	+4 -2
Reduce the length of 6 long lines of code	mediawiki/core	master	+17 -6
Add tab-width=4 and increase line length to 120	mediawiki/tools/codesniffer	master	+4 -2

Customize query in gerrit

Related Objects

Mentioned In: T171695: Incorrect indenting moving {} onto newline
T243529: PHPCS should allow implementing interfaces over multiple lines
Mentioned Here: T243529: PHPCS should allow implementing interfaces over multiple lines

Event Timeline

Daimona created this task.Jan 24 2020, 1:02 PM

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJan 24 2020, 1:02 PM

Daimona mentioned this in T243529: PHPCS should allow implementing interfaces over multiple lines.Jan 24 2020, 1:03 PM

DannyS712 subscribed.Jan 24 2020, 3:03 PM

I think https://gerrit.wikimedia.org/r/#/c/mediawiki/core/+/563289/ is running into this.

In T243598#5851512, @aaron wrote:

I think https://gerrit.wikimedia.org/r/#/c/mediawiki/core/+/563289/ is running into this.

Yes, it does. Feel free to disable the rule globally (in the phpcs.xml file), right now it's useless.

Samwilson awarded a token.Mar 12 2020, 1:57 AM

I agree, having tabs count as one character has always felt odd to me: it means that lines can appear longer than 100 chars, but not result in a warning.

If we were to increase the line length to 120 (which is what PSR12 says) then there are only 6 lines in MediaWiki includes/ that are currently longer than that.

Setting the tab-width will also mean we can enable Generic.WhiteSpace.ScopeIndent, which would be good (there is a bunch of incorrectly-indented code).

	<rule ref="Generic.WhiteSpace.ScopeIndent">
		<properties>
			<property name="tabIndent" value="true" />
			<property name="exact" value="true" />
		</properties>
	</rule>

Change 579119 had a related patch set uploaded (by Samwilson; owner: Samwilson):
[mediawiki/tools/codesniffer@master] Add tab-width=4 and increase line length to 120

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

gerritbot added a project: Patch-For-Review.Mar 12 2020, 2:26 AM

Here's the 6 long lines: https://gerrit.wikimedia.org/r/#/c/mediawiki/core/+/579448

Change 579448 had a related patch set uploaded (by Samwilson; owner: Samwilson):
[mediawiki/core@master] Reduce the length of 6 long lines of code

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

Change 579119 merged by jenkins-bot:
[mediawiki/tools/codesniffer@master] Add tab-width=4 and increase line length to 120

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

Change 579448 merged by jenkins-bot:
[mediawiki/core@master] Reduce the length of 6 long lines of code

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

Maintenance_bot removed a project: Patch-For-Review.Mar 13 2020, 10:10 AM

Daimona closed this task as Resolved.Mar 13 2020, 10:17 AM

Daimona assigned this task to Samwilson.

ReleaseTaggerBot added a project: MW-1.35-notes (1.35.0-wmf.24; 2020-03-17).Mar 13 2020, 11:00 AM

Umherirrender mentioned this in T171695: Incorrect indenting moving {} onto newline.Apr 4 2020, 8:43 PM

Change 655681 had a related patch set uploaded (by Reedy; owner: Samwilson):
[mediawiki/tools/codesniffer@19.x] Add tab-width=4 and increase line length to 120

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

gerritbot added a project: Patch-For-Review.Jan 12 2021, 8:23 PM

Change 655681 merged by Reedy:
[mediawiki/tools/codesniffer@19.x] Add tab-width=4 and increase line length to 120