Unable to set paths relative to workspace, crashes on Windows

musosoft commented 1 year ago

Description

Hi,

I'm trying to bring lint and beautify functionality based on WPCS standards to VS Code. Your extension is the best choice as it's the most recent, well-integrated with VS Code, and made by a WordPress contributor. However, I have encountered some issues that I would like to resolve.

PHPCS and PHPCBF are installed locally, the file structure is as can be seen in this repo: https://github.com/musosoft/vscode-lint-beautify-wordpress-code

If I execute ./vendor/bin/phpcbf .\test \bad-example.php -vvv or ./vendor/bin/phpcs .\test\bad-example.php -vvv I can confirm that CLI works. But if I open the same file with your PHP Code_Sniffer extension enabled I encounter multiple issues:

sometimes after starting VS Code the "Problems section" is empty
when I change file content, errors disappear
the extension is unable to find executables automatically with phpCodeSniffer.autoExecutable enabled. I had to manually specify full paths (phpcs.bat as the one without extension didn't work) which for this project I need to put relative (I tried ${workspaceFolder} settings.json variable, but it didn't work)
formatting doesn't work

Reproduction Steps

git clone https://github.com/musosoft/vscode-lint-beautify-wordpress-code.git
composer install
Change path for phpCodeSniffer.standardCustom and phpCodeSniffer.executable in .vscode/settings.json
Open test/bad-example.php from in VS Code
Try formatting, look for the problems section, and extension logs - it should all be broken

I couldn't find any error in the extension output, that's why I'm posting this video recording: Code_1OcF82EW9f

Expected Behavior

The extension should find the paths in the project's workspace folder when phpCodeSniffer.autoExecutable is enabled (vendor/bin in case of Composer, phpcs.bat in case of Windows),
linter/extension should not hang, if so print some errors,
Formatting should work

I have tested all the above and it works very well on my Linux machine, most probably it's happening only on Windows. If there's any error log I could possibly collect somewhere please let me know. I am more than happy to share it and help.

ObliviousHarmony commented 1 year ago

Hi @musosoft,

When I wrote this extension I assumed that anyone using Windows would be running PHP via WSL2. It is the preferred way of developing in Windows these days and so I didn't test a native Windows execution context. It seems like using the batch file might be broken due to the way argument passing (or maybe input piping?) is handled. I haven't really had a great reason to dive further into it.

At some point, sooner rather than later, I think I will have time to take a look at this. I am on parental leave right now and so I have free time to play around with this.

ObliviousHarmony commented 1 year ago

Hi @musosoft,

I just wanted to let you know that I've released a new version resolving bugs with running PHPCS natively in Windows. You should no longer experience any problems.

musosoft commented 1 year ago

Hi @ObliviousHarmony,

Thanks for resolving the issues on Windows. All works, but:

The extension should find the paths in the project's workspace folder when phpCodeSniffer.autoExecutable is enabled (vendor/bin in case of Composer, phpcs.bat in case of Windows),

This issue is still present, I had to specify the paths manually like so:

    "phpCodeSniffer.standard": "Custom",
    "phpCodeSniffer.autoExecutable": true,
    "phpCodeSniffer.standardCustom": "phpcs.xml.dist",
    "phpCodeSniffer.exec.windows": "C:\\Users\\User\\Documents\\repo\\vendor\\bin\\phpcs.bat"

Interestingly, I also had to specify standardCustom and it had to be a relative path, while exec.windows had to be absolute.

ObliviousHarmony commented 1 year ago

Thanks for the follow-up @musosoft.

I think I'll go ahead and add more tests to try and nail down any problems with path resolution on Windows. Realistically speaking, there shouldn't be a good reason that relative paths don't work with phpCodeSniffer.exec.windows. Maybe you can try using POSIX directory separators? Assuming that you have VS Code opened to C:\\Users\\User\\Documents\\repo, does vendor/bin/phpcs.bat work?

musosoft commented 1 year ago

Thanks for your suggestions @ObliviousHarmony, Strangely, autoExecutable is working well now without changing anything in the same workspace since last time. Tested POSIX and Win path separators, relative and absolute paths - that all works well for phpCodeSniffer.exec.windows also. I suppose it was because that time I just installed Composer on my fresh setup, maybe some paths were not set yet until I restarted. Not sure, really, that time running phpcs manually in the terminal worked. It would be a big plus if the extension could catch some error in the output.

The problem that still remains is if I use "Default" or "Automatic" as phpCodeSniffer.standard the extension won't work nor output anything. I still have to specify the path of the standard:

    "phpCodeSniffer.standard": "Custom",
    "phpCodeSniffer.autoExecutable": true,
    "phpCodeSniffer.standardCustom": "phpcs.xml.dist"

ObliviousHarmony commented 1 year ago

Looks like I made a mistake @musosoft! I misread the PHPCS documentation and it's looking for phpcs.dist.xml, so, it's not finding the right standard. It should be giving an error if it can't find a standard but it doesn't seem like that's happening. I'll get this fixed!

ObliviousHarmony commented 1 year ago

Okay @musosoft, this should be working for you now with the latest version. Let me know if you have any more issues!

musosoft commented 1 year ago

Works perfectly, thanks! Have a nice weekend

ObliviousHarmony / vscode-php-codesniffer