Page MenuHomePhabricator
Create Task
Maniphest T114213

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

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

SubjectRepoBranchLines +/-
Fix the last Generic.Files.LineLength phpcs failuresmediawiki/coremaster+5 -5
Re-enable Generic.Files.LineLength and exclude failing filesmediawiki/coremaster+6 -1
Customize query in gerrit

Related Objects

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
Amire80 added a parent task: T102614: Fix Generic.Files.LineLength errors in MediaWiki core by breaking long lines.Sep 30 2015, 6:24 AM
gerritbot subscribed.Sep 30 2015, 6:36 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

gerritbot added a project: Patch-For-Review.Sep 30 2015, 6:36 AM
gerritbot added a comment.Sep 30 2015, 6:41 AM

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

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

Legoktm added a comment.Sep 30 2015, 7:19 AM

I think the comment has to be in a specific format: https://github.com/squizlabs/PHP_CodeSniffer/wiki/Advanced-Usage#ignoring-parts-of-a-file

gerritbot added a comment.Sep 30 2015, 7:21 AM

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

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

Amire80 added a comment.Sep 30 2015, 7:38 AM

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!

gerritbot added a comment.Sep 30 2015, 7:40 AM

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

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

Legoktm closed this task as Invalid.Oct 1 2015, 2:30 AM
Legoktm claimed this task.
Legoktm mentioned this in T113852: @codingStandardsIgnoreStart doesn't seem to help avoid phpcs errors inside comments.
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.

Amire80 merged a task: T113852: @codingStandardsIgnoreStart doesn't seem to help avoid phpcs errors inside comments.Oct 1 2015, 7:39 PM
Amire80 added a subscriber: Addshore.
Amire80 mentioned this in T102614: Fix Generic.Files.LineLength errors in MediaWiki core by breaking long lines.Oct 3 2015, 4:23 PM
Umherirrender mentioned this in rMWc50cbe48ab60: Fix remaining Generic.Files.LineLength failures.Oct 3 2015, 6:00 PM
Umherirrender subscribed.Oct 3 2015, 6:05 PM

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

gerritbot added a comment.Oct 3 2015, 6:06 PM

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

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

Legoktm added a comment.Oct 5 2015, 5:07 AM
In T114213#1699845, @Umherirrender wrote:

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
gerritbot added a comment.Oct 5 2015, 8:26 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

gerritbot added a comment.Oct 5 2015, 12:39 PM

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

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

Reedy mentioned this in rMW4490b5b5ec4b: Fix the last Generic.Files.LineLength phpcs failures.Oct 5 2015, 12:40 PM
gerritbot added a comment.Oct 5 2015, 12:41 PM

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 removed a parent task: T102614: Fix Generic.Files.LineLength errors in MediaWiki core by breaking long lines.Oct 5 2015, 12:42 PM
Amire80 edited projects, added Upstream; removed Patch-For-Review.Oct 5 2015, 12:44 PM
ReleaseTaggerBot added projects: MW-1.27-release-notes, MW-1.27-release (WMF-deploy-2015-10-06_(1.27.0-wmf.2)).Oct 5 2015, 1:00 PM
TasneemLo subscribed.Oct 7 2015, 8:16 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

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
In T114213#1708009, @TasneemLo wrote:

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.

Umherirrender unsubscribed.Nov 1 2015, 6:17 PM
TasneemLo unsubscribed.Dec 20 2015, 9:12 AM
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 subscribed.
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL