Last modified: 2014-08-16 12:33:08 UTC
CodeSniffer currently tries to enforce a space after a cast[2]. Our documentation states: "When type casting, do not use a space within the cast operator or after the cast"[1] Please update the CodeSniffer configuration to issue the correct warning. [1] https://www.mediawiki.org/wiki/Manual:Coding_conventions/PHP [2] https://integration.wikimedia.org/ci/job/mwext-Translate-phpcs-HEAD/1291/console
Been done with https://gerrit.wikimedia.org/r/#/c/45140/ then reverted with https://gerrit.wikimedia.org/r/#/c/45142/ which actually cause the issue. The SpaceAfterCast has been readded by Timo in https://gerrit.wikimedia.org/r/#/c/58637/
Timo, can you please explain why this is closed WORKSFORME? See https://gerrit.wikimedia.org/r/#/c/66558/ where https://integration.wikimedia.org/ci/job/mwext-Translate-phpcs-HEAD/1361/console throws an error on https://gerrit.wikimedia.org/r/#/c/66558/2/tag/TranslateDeleteJob.php: 73: ERROR A cast statement must be followed by a single space / Generic.Formatting.SpaceAfterCast.NoSpace 73: $pages = (array)$cache->get( wfMemcKey( 'pt-base', $base ) ); This was a few hours ago.
Yes, that's by design. Our code had mostly a 50/50 between cast with and without a space afterwards. There was no explicit rule either way, and we'll have to make changes either way. I made a judgement call and believe that in our coding style it makes most sense to have a space there. Therefor I removed the rule in phpcs that considered code with a space afterwards to be a violation and instead configured it the other way around.
(In reply to comment #3) > Yes, that's by design. > > Our code had mostly a 50/50 between cast with and without a space afterwards. > > There was no explicit rule either way, and we'll have to make changes either > way. > > I made a judgement call and believe that in our coding style it makes most > sense to have a space there. Therefor I removed the rule in phpcs that > considered code with a space afterwards to be a violation and instead > configured it the other way around. Our docs say otherwise, and despite your consistent replies here, you have not made an effort to remedy the inconsistencies between the checker and the docs.
We had that debate at the beginning of 2013. When type casting, do not use a space after the cast: (int)$foo; http://www.mediawiki.org/w/index.php?title=Manual:Coding_conventions/PHP&diff=652478&oldid=648613 Later improved with code to not use (aka anything with spaces): ( int )$bar; ( int ) $bar; ( int ) $bar; http://www.mediawiki.org/w/index.php?title=Manual:Coding_conventions/PHP&diff=674672&oldid=673901 Using the standard from mediawiki/tools/codesniffer.git at commit e1de64e4 , PHPCs processing gives out: (int)$foo; // A cast statement must be followed by a single space ( int )$bar; // A cast statement must be followed by a single space ( int ) $bar; // considered valid, should not ( int ) $bar; // considered valid, should not That is happening since https://gerrit.wikimedia.org/r/#/c/58637/ which did the following change: - <rule ref="Generic.Formatting.NoSpaceAfterCast" /> + <rule ref="Generic.Formatting.SpaceAfterCast" /> We need to revert it again, aka like what I did in https://gerrit.wikimedia.org/r/#/c/45142/ : - <rule ref="Generic.Formatting.SpaceAfterCast" /> + <rule ref="Generic.Formatting.NoSpaceAfterCast" />
Related URL: https://gerrit.wikimedia.org/r/66961 (Gerrit Change I641535209fd477e7d56d8659015bb9c66f10ac68)
Related URL: https://gerrit.wikimedia.org/r/66964 (Gerrit Change Ied10f203af5dcff45893bb0246cb6a0469728cb3)
phpcs no longer warns about (type) casting now.
Timo that bug is a regression, we used to have that sniff in and https://gerrit.wikimedia.org/r/#/c/66961/ reintroduce it.
Change 66961 abandoned by Hashar: Enable Generic.Formatting.NoSpaceAfterCast https://gerrit.wikimedia.org/r/66961
I think the status should be "reopened" now, per comment 9. It's pretty confusing that the bot changes bug status to "patch to review" when a patch is abandoned. I'll have to change status twice to get there...
Hmm, can't get to reopened, so will leave this at new... Messed up...
So what is the state here and what do we want?
Per Timo Comment #8 the rule is abandoned so we do not enforce one way or another. My patch https://gerrit.wikimedia.org/r/#/c/66961/ was to reintroduce one of the style and Timo and I agreed to abandon it (see discussion on Gerrit change). I guess we can mark this bug as FIXED. The fix being that we abandoned that sniff entirely.
