squizlabs / PHP_CodeSniffer

PHP_CodeSniffer tokenizes PHP files and detects violations of a defined set of coding standards.
BSD 3-Clause "New" or "Revised" License
10.66k stars 1.48k forks source link

Warning generated by a require_once outside a function #3888

Closed so-o closed 11 months ago

so-o commented 11 months ago

Can you tell me what is wrong with calling require_once before declaring a function?

<?php

require_once 'file.php';

function foobar() {
    return true;
}

$ phpcs --report=checkstyle --standard=PSR1 foobar.php

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 5 and the first side effect is on line 3." source="PSR1.Files.SideEffects.FoundWithSymbols".

If the require_once is removed, no error.

$ phpcs --version
PHP_CodeSniffer version 3.6.2 (stable) by Squiz (http://www.squiz.net)

This is reported as a warning by Flycheck in Emacs.

jrfnl commented 11 months ago

Whether there is something wrong with that or not is a matter of opinion, but you are using the PSR-1 sniffs, so in that case, you are expected to comply with the rules outlined by PSR-1.

Here is the detailed information about the rule in question.

Keep in mind: PHP_CodeSniffer did not make that rule, PHP_CodeSniffer just contains the tooling to check for compliance with the rule.

so-o commented 11 months ago

If a function needs another function, the require_once must be in the function? What about a file with all the constants in my code, same limitation? If I write a script in PHP because bash is a pain, I can't define functions in that code? Note that I can use define.

jrfnl commented 11 months ago

@so-o Please read the description of the rule in the PSR definition. If you don't agree with the rule, you can open a discussion on the FIG mailinglist. If you want to ignore the rule for your codebase, you can <exclude name=...> it from your ruleset.

so-o commented 11 months ago

I read the PSR definition and I'm not going to start arguing about how PHP is under the control of people who think Java is the bible. More rules don't make better programmers.

I found the --exclude command-line option:

$ phpcs --report=checkstyle --standard=PSR1 --exclude=PSR1.Files.SideEffects foobar.php

To exclude the rule by default, I edited the file /usr/share/php/PHP/CodeSniffer/src/Standards/PSR1/ruleset.xml:

   <!-- 2.3. Side Effects -->

    <!-- 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. -->
    <!-- checked by PSR1.Files.SideEffects -->

    <rule ref="PSR1.Files.SideEffects">
        <exclude name="PSR1.Files.SideEffects"/>
    </rule>
jrfnl commented 11 months ago

To exclude the rule by default, I edited the file /usr/share/php/PHP/CodeSniffer/src/Standards/PSR1/ruleset.xml:

Please don't edit the files from this project as your changes will be overwritten whenever you update.

The better way is to add a [.]phpcs.xml[.dist] file to your own project and exclude the rule from there instead.

Either way, I think this issue can now be closed ?

so-o commented 11 months ago

What should be the content of this [.]phpcs.xml file? Will flycheck in Emacs read it? Can I put it in my home directory so it's read every time from anywhere?

jrfnl commented 11 months ago

You can find documentation about using a custom ruleset in the wiki:

so-o commented 11 months ago

Thanks to your fast and pertinent replies, I could remove this annoying warning in Emacs, the only error ever reported by phpcs in my million lines of PHP. Just great!

jrfnl commented 11 months ago

@so-o Happy to hear your issue is solved now 👍🏻