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.67k stars 1.48k forks source link

[feature] Automatically create directory for cache file in configuration #3834

Open asispts opened 1 year ago

asispts commented 1 year ago

PHPCS: 3.7.2

Description

Currently, when attempting to use a directory to store a cache file by adding the following line to the configuration file:

<arg name="cache" value="dir/phpcs-cache" />

an error is thrown:

ERROR: The specified cache file path "dir/phpcs-cache" points to a non-existent directory

Expected behavior

The expected behavior is that the directory should be created. It's worth noting that phpunit also creates the directory before storing the cache file.

jrfnl commented 1 year ago

@asispts This is not a bug, but a design choice at the time and may be open for discussion to change that design choice.

However, please keep in mind that while your example is relatively simple when one presumes the intended path is relative to the directory containing the ruleset and PHPCS is run from that directory, things suddenly get a lot more complex when you factor in absolute path, path traversal and the fact that PHPCS does ruleset discovery from the current directory upwards.

P.S.: PHPUnit is a completely different project and what they do has no bearing on this project.

asispts commented 1 year ago

First of all, I don't think this is a bug either. Unfortunately, the issue template only provides two options: bug report or security vulnerability. That's why I added [feature] to the title.

I am aware that both PHPCS and PHPUnit are separate projects with different roadmaps and design choices.

Let me explain my use case. I prefer to keep my root project as clean as possible, so I store all temp, cache, and build files (including code coverage) in a build directory. In my workflow, PHPCS is the first dev tool to be executed. This is why the problem arises. I need to ensure that the build directory exists before executing PHPCS. I know that this is just a minor issue. However, I now have to add a command to check the directory to a build script; maintain both local and CI build scripts; and also mention it somewhere in readme or contribution file because the local build script is not committed to the repository.

Considering the complexity you mentioned earlier, I'm uncertain if implementing this feature is worthwhile. Additionally, issues #2802 and #1992 should be taken into consideration as well.

ldebrouwer commented 1 year ago

@asispts What's stopping you from creating the directory in your repository, and dropping a .gitignore file in there to ensure its creation when you check out the repository? This is a fairly common practice.

asispts commented 1 year ago

The directory is not part of the project. It's just a temp directory, similar to var directory in a Symfony project, or dist and build dirs in public directory. Do you add a .gitignore or a .gitkeep in those directories?

ldebrouwer commented 1 year ago

I don't look after any Symfony projects, but yes, that would be my most likely approach. You can see it in projects like Laravel (the storage directory) as well. If you take this approach, and you're explicit about directory creation, you are also alerted (through failed builds) when you've accidentally changed a config value (like a cache directory location) but didn't cater for it in your setup. Think of it as your canary in the coal mine.

In my opinion it's better to be explicit about something, rather than relying on a tool to have the desired side effect that will get you to your end goal.

asispts commented 1 year ago

Then you can think of /public/build/ directory in Laravel. You can easily remove this directory, unlike directories inside /storage/ directory, because you have to recreate or restore .gitignore files for each folder.