Disallow relative include/require

[kkmuffme]

Is your feature request related to a problem?

require 'some/file.php';

is including a file from a relative path. Depending on the server configuration this can lead to: Fatal error: Failed opening required ...

because: a) the way how PHP resolves relatives paths can be changed in the php.ini (via include_path) b) the file might not actually be there in specific installations (e.g. symlinked installs)

Since these are things that are install dependent, the only way to fix this issue is to ALWAYS require absolute paths.

I think this might be a bit complicated with phpcs, but we could at least check if include/require is followed by __DIR__ or basename( (or any variable, which we cant chekc in phpcs) => or maybe the other way round: pure string includes/requires should be disallowed, e.g. if "include" is followed by a string (= either ' or " essentially)

Latest WPCS & phpcs

[jrfnl]

Previously discussed in #1630.

I agree that always using an absolute path should be the recommended way of including/requiring.

maybe the other way round: pure string includes/requires should be disallowed, e.g. if "include" is followed by a string (= either ' or " essentially)

This last suggestion I could get behind, though I suspect that will only catch a small portion of the potential issues.

Anything else is far too prone to false positives though, as plugins/themes often declare their own "root dir" in a constant/variable and we wouldn't have access to the runtime value of those.

Implementation notes:

• Beware of use with parentheses - `include ( 'something.php' );.
• Beware that when used with parentheses, concatenation can still happen after this.

Also loosely related to #462

[kkmuffme]

Yes I think the disallow string solution would make most sense.

Anything else is far too prone to false positives though, as plugins/themes often declare their own "root dir" in a constant/variable and we wouldn't have access to the runtime value of those.

Totally agree.

That maybe something we want to shut off too - CONSTANTS should also not be allowed in the include/require as this is badly compatible with other code sniffing tools (psalm, phpstan) and unnecessary when we can just use __DIR__...

Beware that when used with parentheses, concatenation can still happen after this.

Yep, but is irrelevant, isn't it?

[jrfnl]

That maybe something we want to shut off too - CONSTANTS should also not be allowed in the include/require as this is badly compatible with other code sniffing tools (psalm, phpstan) and unnecessary when we can just use DIR

I very much don't agree with this. Declaring a plugin/theme "root dir" in a constant using something like plugin_dir_path( __FILE__ ) is actually good practice and should not be discouraged.

Beware that when used with parentheses, concatenation can still happen after this. Yep, but is irrelevant, isn't it?

No, it's not. While code like that may be rare (as it is inefficient), the sniff should still handle it correctly.

[kkmuffme]

I very much don't agree with this. Declaring a plugin/theme "root dir" in a constant using something like plugin_dir_path( FILE ) is actually good practice and should not be discouraged.

Where does it say this is good practice? Also, as I mentioned above, using constants for includes isn't (easily) compatible with static code analyzers like psalm/phpstan, which means worse code quality.

Beware that when used with parentheses, concatenation can still happen after this.

Yep, but is irrelevant, isn't it?

No, it's not. While code like that may be rare (as it is inefficient), the sniff should still handle it correctly.

Ok sorry I wasn't aware of that you have to keep this in mind when creating the sniff, I thought you meant that the sniff should disallow this :-)