Skip to content

Simplify FileCacheStorage::save() with atomic dumpFile() - #8312

Open
TomasVotruba wants to merge 1 commit into
mainfrom
claude/simplify-file-cache-storage-4bavrw
Open

Simplify FileCacheStorage::save() with atomic dumpFile()#8312
TomasVotruba wants to merge 1 commit into
mainfrom
claude/simplify-file-cache-storage-4bavrw

Conversation

@TomasVotruba

Copy link
Copy Markdown
Member

What

Simplifies FileCacheStorage::save() to write the cache file with a single atomic Filesystem::dumpFile() call, removing the manual temp-file handling.

Why

This finishes the cleanup started in #498 ("Sync FileCacheStorage with changes from phpstan-src"), which had since regressed. The write path currently does its own atomic-write dance:

$tmpPath = sprintf('%s/%s.tmp', $this->directory, Random::generate());
FileSystem::write($tmpPath, ...);   // Nette
$copySuccess = @copy($tmpPath, $filePath);
@unlink($tmpPath);
// + a DIRECTORY_SEPARATOR fallback

But the already-injected Symfony Filesystem::dumpFile() already writes atomically via a temporary file + rename internally, so doing it again on the Rector side is redundant syscalls — the exact overhead #498 set out to remove (it was a notable bottleneck on Windows).

How

  • Replaced the Random temp path + FileSystem::write + @copy + @unlink + DIRECTORY_SEPARATOR fallback with $this->filesystem->dumpFile($filePath, ...).
  • Kept the var_export error guard (catches non-exportable data such as closures).
  • Dropped the now-unused Nette\Utils\Random import.

Behaviour is unchanged; this is a smaller/faster write path.


Generated by Claude Code

Replace the manual temp-file + copy + unlink handling with Symfony's
Filesystem::dumpFile(), which already writes atomically via a temporary
file and rename internally. Finishes the cleanup started in #498 that had
since regressed. Drops the now-unused Nette\Utils\Random import.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Ti6vHRo3xLSHw84rxUw6Lb
@TomasVotruba
TomasVotruba marked this pull request as ready for review August 7, 2026 09:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants