Update warning message for file system operations

[dd32]

Update the message to indicate that WP_Filesystem should not be used for general file IO.

Description

The coding standards suggest using WP_Filesystem instead of all file-operations, which often sends developers down a path that they really shouldn't have.

This just clarifies that perhaps they shouldn't perform file operations, and to use WP_Filesystem for upgrade-routines (Which the classes are intended for).

This might get some pushback from some people who feel WP_Filesystem should be used if it exists, but it's an abstraction that applies for a constantly reducing set of sites, which run on read-only filesystems and require FTP to interact with the WordPress files.
Generally, WP_Filesystem either uses direct IO, or, FTP which requires interactive prompting, which most plugins/themes that use WP_Filesystem are not handling anyway.

I will note it's possible for WP_Filesystem to be configured with hard-coded credentials, but that's even more rare.

Pushing people onto this class just doesn't benefit anyone.

I would almost suggest that the mention of WP_Filesystem should be removed entirely, and this just be left as a "You really shouldn't do file operations if this is running often" type warning.

Suggested changelog entry

Related issues/external references

Fixes #

[jrfnl]

@dd32 Thanks for the PR. I'm in two minds about this.

The new text:

File operations are discouraged, and should use WP_Filesystem methods for Upgrade routines. Found: %s()

... would leave users with an inactionable warning - if a file operation is rightfully used, but it's not an upgrade routine, what to do ? This is counter to the principle of PHPCS (everything should be fixable, automatically or manually).

There is some precedent for inactionable warnings in WPCS, but only for security related issues, not for anything else and inactionable warnings is one of the biggest sources of irritation for end-users of WPCS.

So, that leaves us with a conundrum.... is there any auto-detection which can be done to limit when the warning is shown to only actionable situations ?
And if not, should all file system functions which do not involve upgrade routines be removed from the list of functions this warning applies to ?

On another, very loosely related note - does the FileSystem interact with the PHP OpCache and if so, how ? and will the change in PHP 8.5 of OpCache becoming a non-optional part of PHP have any impact ?
I'd be interested to hear your take on that.

[dd32]

... would leave users with an inactionable warning - if a file operation is rightfully used, but it's not an upgrade routine, what to do ? This is counter to the principle of PHPCS (everything should be fixable, automatically or manually).

I agree. I initially came here to suggest that the rule be removed entirely.

And if not, should all file system functions which do not involve upgrade routines be removed from the list of functions this warning applies to ?

The list of functions is what caused me to change tact from remove to clarify.

These functions can be used properly:
'fclose', 'file_put_contents', 'fopen', 'fputs', 'fread','fwrite','readfile',

This warning has caused a lot of people to switch from f{open,read first x bytes,close}() calls to using file_get_contents(), and likewise Plugin Check appears to disable the file_put_contents check.

These probably shouldn't be needed:
'chgrp', 'chmod', 'chown', 'fsockopen', 'pfsockopen',

These I'm on the rails about, and err towards allow (at least for writable)
'is_writable', 'is_writeable','mkdir', 'rmdir', 'touch',

is there any auto-detection which can be done to limit when the warning is shown to only actionable situations ?

I don't really think so, it's probably possible, but I can't imagine it's worth it.

On another, very loosely related note - does the FileSystem interact with the PHP OpCache and if so, how ? and will the change in PHP 8.5 of OpCache becoming a non-optional part of PHP have any impact ?

No. However, the Upgrader (which uses Filesystem) does. See wp_opcache_invalidate_directory() and https://github.com/search?q=repo%3AWordPress%2Fwordpress-develop+opcache_invalidate&type=code (note: I've not worked on this component in a while, and I'm not overly familiar with those additions)