rfc flag method as having write operation side effects

[SignpostMarv]

I had a brief perusal of the phpdocumenter tag list, and there doesn't seem to be a tag for indicating a method has write operation amongst it list of side effects.

The situation I'm thinking of here is some theoretical static analysis for flagging up write operations taking place before some anti-csrf action, i.e. if the token is re-used, no write operations should've taken place, i.e.

• deleting files
• writing files to a filesystem
• insert/update/delete sql queries

write operations in this context do not include simply calling property setters etc.

if ($request->files->has('thing')) {
    // write file to filesystem
}

$db->query('UPDATE `table` SET `foo` = ? WHERE `id` = ?', ['bar', $id]); 
/*
`$db->query() is either implicitly flagged as an ambiguous operation (or explicitly via tag),
since it has `PDOStatement::execute()` somewhere in it's AST
*/

// check token somewhere around here, halting if token not valid

$anticsrf->removeToken($token);
/*
hypothetical analyser now checks previous chunks of the AST in it's scope for write operations
*/

/*
assume docblock for `$db->update()` has the "write operation" tag
*/
$db->update('table', ['foo' => 'bar'], ['id' => $id]);

the hypthetical analysis could also flag methods that're explicitly tagged as being read-only operations that (in a later commit/ composer package update) end up with a write operation in it's AST.

tl:dr; we have tags for type safety, what about operation safety?

[ondrejmirtes]

@SignpostMarv Hi, I don't see what effect should this have for analysis. Also see: phpstan/phpstan#1157

[SignpostMarv]

@ondrejmirtes one of the intended effects would be to let an analyser flag particular methods as "MUST have no side effects prior to calling", in the above example this would be $anticsrf->removeToken($token);, with the idea being an analyser could throw an error in the if ($request->files->has('thing')) { block when the submitted file gets written to disk, and $db->query('UPDATE tableSETfoo= ? WHEREid= ?', ['bar', $id]); throws an error because it's ambiguous as to whether it has a side effect.

additionally, if the $db->query('UPDATE tableSETfoo= ? WHEREid = ?', ['bar', $id]); example was wrapped inside a transaction, the ambiguous warning could be dropped so long as there's an exception being thrown in // check token somewhere around here, halting if token not valid

since the tl:dr; is regarding operation safety, there's a few patterns in PHP that shouldn't have side effects, and some patterns that should-

• a logger whose AST has no write operations (i.e. to file, to database etc.) can be flagged as "this logger should have side effects but no write operations were detected".
• a psr-7 or symfony/http-foundation request/response filter might be required to have no side effects

for catching developer errors:

• the use of otherwise-valid file write operations that were used for debugging purposes being inadvertently left in place
• a class expecting write side effects being accidentally configured to use a mock class as a dependency that doesn't have side effects (in the logger example, this would be if your app requires a PSR-3 logger, but you've accidentally left it using NullLogger
• an http handler that checks Symfony's Request::$files->has() method or does an Request::$files->get() instanceof \SplFileInfo check but does nothing with the file could be a signal for intent to do something with a file, but draw the developer's attention back to the code if they forgot to do something with the file.