PSR1.Files.SideEffects.FoundWithSymbols not being ignored

[quasipickle]

Describe the bug
A line that violates the PSR1.Files.SideEffects.FoundWithSymbols rule still generates a warning about it even when preceded with // phpcs:ignore

Code sample

<?php

// phpcs:ignore
define('MYCONST', 'myval');
$var = 'test';

To reproduce
Steps to reproduce the behavior:

1. Create a file called test.php with the code sample above...
2. Run phpcs test.php ...
3. See error message displayed

-----------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
-----------------------------------------------------------------------------------------------
 1 | WARNING | A file should declare new symbols (classes, functions, constants, etc.) and
   |         | cause no other side effects, or it should execute logic with side effects, but
   |         | should not do both. The first symbol is defined on line 4 and the first side
   |         | effect is on line 5. (PSR1.Files.SideEffects.FoundWithSymbols)
-----------------------------------------------------------------------------------------------

Expected behavior
No warnings get generated

Versions (please complete the following information):

• OS: Debian 8.11
• PHP: 7.3.11
• PHPCS: 3.6.0
• Standard: PSR12

Additional context
When the offending line has a space removed between the parameters, thereby violating Generic.Functions.FunctionCallArgumentSpacing.NoSpaceAfterComma, that error does get ignored.

If I instead wrap the line in disable/enable, the warning does not get generated

<?php

// phpcs:disable
define('MYCONST', 'myval');
// phpcs:enable
$var = 'test';

If I disable just that rule, the warning does get generated:

<?php

// phpcs:disable PSR1.Files.SideEffects.FoundWithSymbols
define('MYCONST', 'myval');
// phpcs:enable
$var = 'test';

[gsherwood]

Using the enable/disable method is exactly how this is done, as was included in the 3.2.0 release. The ignore comment just tells PHPCS to ignore any violations on that line, but there aren't any as the sniff adds it to the first line of the file.

Replicating the release notes here for clarrity:

    - PSR1.Files.SideEffects now respects the new phpcs:disable comment syntax
      -- The sniff will no longer check any code that is between phpcs:disable and phpcs:enable comments
      -- The sniff does not support phpcs:ignore; you must wrap code structures with disable/enable comments
      -- Previously, there was no way to have this sniff ignore parts of a file

[quasipickle]

Ah, I see. Then maybe instead of a bug report, this could be added to documentation somewhere?

[yakatz]

@gsherwood I found this issue while searching to see why FoundWithSymbols wasn't respected as part of // phpcs:disable.

My file has this as the first two lines:

<?php
// phpcs:disable PSR1.Files.SideEffects.FoundWithSymbols

If I have just // phpcs:disable, it ignores everything in the file. This seems to be contrary to the release notes for 3.2.0.

I am using version 3.7.1.

[mindriven]

I'm facing same inconsistency as @yakatz. Any movement on that?

[nwehrhan]

Having this issue as well. I am using version 3.7.1.

[jrfnl]

I have opened a PR to hopefully fix this issue. Please have a look at PR #3772.

Once that PR has been merged, the below code sample will work:

<?php

// phpcs:disable PSR1.Files.SideEffects.FoundWithSymbols
define('MYCONST', 'myval');
// phpcs:enable
$var = 'test';

Testing appreciated.

[jrfnl]

FYI: the fix for this issue is included in today's PHP_CodeSniffer 3.8.0 release.

As per #3932, development on PHP_CodeSniffer will continue in the PHPCSStandards/PHP_CodeSniffer repository. If you want to stay informed, you may want to start "watching" that repo (or watching releases from that repo).