Page MenuHomePhabricator

@codingStandardsIgnoreStart only works with some types of comments
Closed, ResolvedPublic

Description

The file tests/phpunit/includes/password/BcryptPasswordTest.php has long lines that probably shouldn't be broken.

It has a /** @codingStandardsIgnoreStart Generic.Files.LineLength.TooLong */ comment, but it fails phpcs nevertheless:
https://integration.wikimedia.org/ci/job/mediawiki-core-phpcs/473/consoleFull

Details

Event Timeline

Amire80 created this task.Sep 30 2015, 6:23 AM
Amire80 raised the priority of this task from to Low.
Amire80 updated the task description. (Show Details)
Amire80 added a project: MediaWiki-Codesniffer.
Amire80 added subscribers: Amire80, Reedy, polybuildr, Legoktm.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptSep 30 2015, 6:23 AM

Change 242450 had a related patch set uploaded (by Amire80):
WIP Trying to see if changing a comment resolves bug T114213

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

Change 242450 abandoned by Amire80:
WIP Trying to see if changing a comment resolves bug T114213

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

Change 242450 restored by Amire80:
WIP Trying to see if changing a comment resolves bug T114213

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

Ha! Indeed: I changed to // @codingStandardsIgnoreStart Generic.Files.LineLength and BcryptPasswordTest.php is not in https://integration.wikimedia.org/ci/job/mediawiki-core-phpcs/536/consoleFull .

This fixes T113852, too.

Thanks!

Change 242450 had a related patch set uploaded (by Amire80):
Use correct comment format for codingStandardsIgnoreStart

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

Amire80 reopened this task as Open.Oct 1 2015, 7:38 PM
Amire80 set Security to None.

According to the comments in https://gerrit.wikimedia.org/r/#/c/242483/ , there's no consensus for this solution.

maybe this commit on github[1] is a fix for phpcs or it was already fixed by [2]

I have no idea which version is on the test infrastructure

[1] https://github.com/squizlabs/PHP_CodeSniffer/commit/6d4b2be755fa3f89dcfdf33043fb7f00e1d4fed6
[2] https://github.com/squizlabs/PHP_CodeSniffer/commit/a511929aed46ec95ae98c89141488daa6f07d19a

Change 243418 had a related patch set uploaded (by Amire80):
Fix the last Generic.Files.LineLength phpcs failures

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

maybe this commit on github[1] is a fix for phpcs or it was already fixed by [2]
I have no idea which version is on the test infrastructure

It uses 2.3.4, meaning it includes [2], but not [1].

[1] https://github.com/squizlabs/PHP_CodeSniffer/commit/6d4b2be755fa3f89dcfdf33043fb7f00e1d4fed6
[2] https://github.com/squizlabs/PHP_CodeSniffer/commit/a511929aed46ec95ae98c89141488daa6f07d19a

Amire80 renamed this task from @codingStandardsIgnoreStart Generic.Files.LineLength.TooLong doesn't seem to work in tests/phpunit/includes/password/BcryptPasswordTest.php to @codingStandardsIgnoreStart only works with // comments.Oct 5 2015, 8:23 AM

Change 243616 had a related patch set uploaded (by Amire80):
Re-enable Generic.Files.LineLength and exclude failing files

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

Change 243418 merged by jenkins-bot:
Fix the last Generic.Files.LineLength phpcs failures

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

Change 243616 abandoned by Amire80:
Re-enable Generic.Files.LineLength and exclude failing files

Reason:
Looks like If853510b55d787765a84bac22b2dbff2e6c526c0 is the direction.

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

Amire80 edited projects, added Upstream; removed Patch-For-Review.Oct 5 2015, 12:44 PM

This seems unusual. I was testing this, and it looks as though

// @codingStandardsIgnoreStart works
/* @codingStandardsIgnoreStart */ works
/** @codingStandardsIgnoreStart */ does NOT work
but ... /*** @codingStandardsIgnoreStart */ works

So, it isn't an issue with // comments as mentioned in the current title. It's an issue with it assuming it's a documentation comment.

I'm using PHCS v2.3.4

Amire80 renamed this task from @codingStandardsIgnoreStart only works with // comments to @codingStandardsIgnoreStart only works with some types of comments.Oct 7 2015, 8:17 AM

This seems unusual. I was testing this, and it looks as though
// @codingStandardsIgnoreStart works
/* @codingStandardsIgnoreStart */ works
/** @codingStandardsIgnoreStart */ does NOT work
but ... /*** @codingStandardsIgnoreStart */ works
So, it isn't an issue with // comments as mentioned in the current title. It's an issue with it assuming it's a documentation comment.
I'm using PHCS v2.3.4

Thanks for testing. I updated the title.

Legoktm removed Legoktm as the assignee of this task.Aug 10 2016, 4:13 AM
Umherirrender closed this task as Resolved.Sep 24 2017, 2:24 PM
Umherirrender claimed this task.

All ways works with "squizlabs/php_codesniffer": "3.0.2"

Umherirrender removed Umherirrender as the assignee of this task.Sep 24 2017, 2:24 PM
Umherirrender added a subscriber: Umherirrender.