search

junstyle / vscode-php-cs-fixer

PHP CS Fixer extension for VS Code
https://marketplace.visualstudio.com/items?itemName=junstyle.php-cs-fixer
MIT License
357 stars 45 forks source link

fix: spawn args need to be quoted only on Windows #218

Closed mengino closed 3 months ago

mengino commented 3 months ago

JSON string can be used directly on Linux shell.

mengino commented 3 months ago

v0.3.17 doesn’t work on Linux, whether “rules” is a string or JSON.

runAsync: spawn /workspace/project/vendor/bin/php-cs-fixer
[
  "fix",
  "--using-cache=no",
  "--format=json",
  "--rules=\"@PSR12\"",
  "--path-mode=override",
  "/tmp/pcf-tmp0.5098910296327399/Example.php"
]
{
  "cwd": "/workspace/project/app",
  "shell": false
}
runAsync: error
{
  "exitCode": 16,
  "stdout": null,
  "stderr": "Loaded config default.\n\nIn ConfigurationResolver.php line 793:\n                                                 \n  The rules contain unknown fixers: \"\"@PSR12\"\".  \n                                                 \n\nfix [--path-mode PATH-MODE] [--allow-risky ALLOW-RISKY] [--config CONFIG] [--dry-run] [--rules RULES] [--using-cache USING-CACHE] [--cache-file CACHE-FILE] [--diff] [--format FORMAT] [--stop-on-violation] [--show-progress SHOW-PROGRESS] [--] [<path>...]\n\n"
}
runAsync: reject promise
Loaded config default.

In ConfigurationResolver.php line 793:

  The rules contain unknown fixers: ""@PSR12"".  

fix [--path-mode PATH-MODE] [--allow-risky ALLOW-RISKY] [--config CONFIG] [--dry-run] [--rules RULES] [--using-cache USING-CACHE] [--cache-file CACHE-FILE] [--diff] [--format FORMAT] [--stop-on-violation] [--show-progress SHOW-PROGRESS] [--] [<path>...]
runAsync: spawn /workspace/project/vendor/bin/php-cs-fixer
[
  "fix",
  "--using-cache=no",
  "--format=json",
  "--rules=\"{\\\"@PSR12\\\":true,\\\"no_extra_blank_lines\\\":true,\\\"binary_operator_spaces\\\":{\\\"default\\\":\\\"align_single_space\\\",\\\"operators\\\":{\\\"=>\\\":\\\"align_single_space_minimal\\\"}}}\"",
  "--path-mode=override",
  "/tmp/pcf-tmp0.5017557214467123/Example.php"
]
{
  "cwd": "/workspace/project/app",
  "shell": false
}

runAsync: error
{
  "exitCode": 16,
  "stdout": null,
  "stderr": "Loaded config default.\n\nIn ConfigurationResolver.php line 793:\n                                                                               \n  The rules contain unknown fixers: \"\"{\\\"@PSR12\\\":true\", \"\\\"no_extra_blank_li  \n  nes\\\":true\" (did you mean \"no_extra_blank_lines\"?), \"\\\"binary_operator_spac  \n  es\\\":{\\\"default\\\":\\\"align_single_space\\\"\", \"\\\"operators\\\":{\\\"=>\\\":\\\"align_s  \n  ingle_space_minimal\\\"}}}\"\".                                                  \n                                                                               \n\nfix [--path-mode PATH-MODE] [--allow-risky ALLOW-RISKY] [--config CONFIG] [--dry-run] [--rules RULES] [--using-cache USING-CACHE] [--cache-file CACHE-FILE] [--diff] [--format FORMAT] [--stop-on-violation] [--show-progress SHOW-PROGRESS] [--] [<path>...]\n\n"
}
runAsync: reject promise
Loaded config default.

In ConfigurationResolver.php line 793:

  The rules contain unknown fixers: ""{\"@PSR12\":true", "\"no_extra_blank_li  
  nes\":true" (did you mean "no_extra_blank_lines"?), "\"binary_operator_spac  
  es\":{\"default\":\"align_single_space\"", "\"operators\":{\"=>\":\"align_s  
  ingle_space_minimal\"}}}"".                                                  

fix [--path-mode PATH-MODE] [--allow-risky ALLOW-RISKY] [--config CONFIG] [--dry-run] [--rules RULES] [--using-cache USING-CACHE] [--cache-file CACHE-FILE] [--diff] [--format FORMAT] [--stop-on-violation] [--show-progress SHOW-PROGRESS] [--] [<path>...]
junstyle commented 3 months ago

if rules has blank string, maybe can't work. double quote not work on Linux, can you test single quote?

mengino commented 3 months ago

if rules has blank string, maybe can't work. double quote not work on Linux, can you test single quote?

Unfortunately, single quote also doesn't work.

runAsync: spawn /workspace/project/vendor/bin/php-cs-fixer
[
  "fix",
  "--using-cache=no",
  "--format=json",
  "--rules='@PSR12'",
  "--path-mode=override",
  "/tmp/pcf-tmp0.2760509868463954/Example.php"
]
{
  "cwd": "/workspace/project/app",
  "shell": false
}
runAsync: error
{
  "exitCode": 16,
  "stdout": null,
  "stderr": "Loaded config default.\n\nIn ConfigurationResolver.php line 793:\n                                                 \n  The rules contain unknown fixers: \"'@PSR12'\".  \n                                                 \n\nfix [--path-mode PATH-MODE] [--allow-risky ALLOW-RISKY] [--config CONFIG] [--dry-run] [--rules RULES] [--using-cache USING-CACHE] [--cache-file CACHE-FILE] [--diff] [--format FORMAT] [--stop-on-violation] [--show-progress SHOW-PROGRESS] [--] [<path>...]\n\n"
}
runAsync: reject promise
Loaded config default.

In ConfigurationResolver.php line 793:

  The rules contain unknown fixers: "'@PSR12'".  

fix [--path-mode PATH-MODE] [--allow-risky ALLOW-RISKY] [--config CONFIG] [--dry-run] [--rules RULES] [--using-cache USING-CACHE] [--cache-file CACHE-FILE] [--diff] [--format FORMAT] [--stop-on-violation] [--show-progress SHOW-PROGRESS] [--] [<path>...]

In fact, “rules” is a JSON or a string that is supported by php-cs-fixer. It never contains any blank string without quotes in normal.

217 is caused by v0.3.14 src/runAsync.ts:5. { shell: true } make the program cannot recognize the system’s shell and parse strings with quotes correctly.This problem was fixed in v0.3.15 which released at 13th Aug. So author of #217 who is a Linux user said “works fine” yesterday.

And Then, because of this comment, v0.3.16 quoted "commands" in order to compatible with Windows. v0.3.17 quoted "rules", although it works normally on Windows, but causes error on Linux. Its all details about #217. As the title says, “rules” only need to be quoted on Windows.

junstyle commented 3 months ago

but if rules contains blank string, actually can't work on linux os. i don't think php-cs-fixer can parse the --rules arg with blank strings.

https://github.com/junstyle/vscode-php-cs-fixer/issues/217#issuecomment-2288419775 this guy use rules with blank string.

so i removed the blank strings on linux in new version

mengino commented 3 months ago

but if rules contains blank string, actually can't work on linux os. i don't think php-cs-fixer can parse the --rules arg with blank strings.

#217 (comment) this guy use rules with blank string.

so i removed the blank strings on linux in new version

不好意思，已经关掉的话我暂时先用中文，明天再翻译一下。到家比较晚，刚测完想撤回PR，结果发现合并了🤣

晚上用 Windows 和 Linux 从 v0.3.14 开始到最新都进行了测试。核心关注点在 v0.3.15 之后的 src/runAsync.ts:5 的配置 { shell: process.platform == 'win32' }。

该配置在 Linux 下 shell 为 false。此时 "command" 不需要做任何处理，哪怕路径中带有空格也能正常识别，而 "rules" 也不需要做任何转译，哪怕是 JSON 也可以直接使用。Windows 下 shell 为 true。此时 "command" 需要用引号包裹，否则路径中带有空格将导致程序失败，而 "rules" 也需要转译并用引号包裹才能正常使用。

由此可猜测， 是否支持路径带空格以及是否要转译 JSON，其实是由 shell 的真假值控制，而非基于系统。再尝试将 src/runAsync.ts:5 配置为 { shell: false }，此时 Windows 下恒失败，Linux 下与上文表现一致。反之如果配置为 { shell: true }， Windows 下与上文表现一致， Linux 下无论是 "command" 还是 "rules" 此时也需要进行转译，否则程序执行失败。

由此推测则无论哪个系统，其实只需将 src/runAsync.ts:5 配置为 { shell: true }，一律按上段 Windows 的处理方式均可使用，应该是 node 会帮忙处理系统兼容性。

也就是说。对于 "command"，将 src/runAsync.ts:8 的 if (process.platform == 'win32') 去掉，即双系统都对路径进行引号包裹，即可正确处理。而对于 "rules" 在 Windows 和 Linux 下都可以使用 v0.3.17 src/index.ts:155 的 args.push('--rules="' + (this.rules as string).replace(/"/g, "\\\"") + '"') 的转译方案处理。这样在代码上避免处理系统差异，应该会更合适一些。

附 v0.3.17 下， src/runAsync.ts:5 配置为 { shell: true } ，并将 src/runAsync.ts:8 的 if (process.platform == 'win32') 去掉的测试结果（Windows 和 Linux 的 JSON 配置直接使用 #217 的两位用户在 issue 中的配置）：

Windows 下 "command" 含有空格， "rules" 为字符串

runAsync: spawn "C:\Users\username\Desktop\test\a a\vendor\bin\php-cs-fixer.bat"
[
  "fix",
  "--using-cache=no",
  "--format=json",
  "--rules=\"@PSR12\"",
  "--path-mode=override",
  "C:\\Users\\username\\AppData\\Local\\Temp\\pcf-tmp0.2901456905828468\\Example.php"
]
{
  "cwd": "c:\\Users\\username\\Desktop\\test\\a a",
  "shell": true
}
runAsync: success
{
  "stdout": "{\"about\":\"PHP CS Fixer 3.62.0 7th Gear by Fabien Potencier, Dariusz Ruminski and contributors.\",\"files\":[{\"name\":\"C:\\\\Users\\\\username\\\\AppData\\\\Local\\\\Temp\\\\pcf-tmp0.2901456905828468\\\\Example.php\"}],\"time\":{\"total\":0.014},\"memory\":14}",
  "stderr": "PHP CS Fixer 3.62.0 7th Gear by Fabien Potencier, Dariusz Ruminski and contributors.\nPHP runtime: 8.3.10\r\nRunning analysis on 1 core sequentially.\r\nYou can enable parallel runner and speed up the analysis! Please see https://cs.symfony.com/doc/usage.html for more information.\r\nLoaded config default.\r\n"
}
runAsync: resolve promise
{"about":"PHP CS Fixer 3.62.0 7th Gear by Fabien Potencier, Dariusz Ruminski and contributors.","files":[{"name":"C:\\Users\\username\\AppData\\Local\\Temp\\pcf-tmp0.2901456905828468\\Example.php"}],"time":{"total":0.014},"memory":14}

Windows 下 "command" 含有空格， "rules" 为 JSON


runAsync: spawn "C:\Users\username\Desktop\test\a a\vendor\bin\php-cs-fixer.bat"
[
  "fix",
  "--using-cache=no",
  "--format=json",
  "--rules=\"{\\\"@PhpCsFixer\\\":true,\\\"@PSR12\\\":true,\\\"array_syntax\\\":{\\\"syntax\\\":\\\"short\\\"},\\\"blank_line_after_opening_tag\\\":true,\\\"blank_line_before_statement\\\":{\\\"statements\\\":[\\\"case\\\",\\\"continue\\\",\\\"default\\\",\\\"declare\\\",\\\"return\\\",\\\"throw\\\",\\\"try\\\"]},\\\"cast_spaces\\\":{\\\"space\\\":\\\"none\\\"},\\\"concat_space\\\":{\\\"spacing\\\":\\\"one\\\"},\\\"linebreak_after_opening_tag\\\":true,\\\"method_argument_space\\\":{\\\"on_multiline\\\":\\\"ensure_fully_multiline\\\"},\\\"no_break_comment\\\":{\\\"comment_text\\\":\\\"no break...\\\"},\\\"no_trailing_comma_in_list_call\\\":true,\\\"no_trailing_comma_in_singleline_array\\\":true,\\\"phpdoc_summary\\\":false,\\\"phpdoc_align\\\":false,\\\"phpdoc_no_alias_tag\\\":false,\\\"phpdoc_annotation_without_dot\\\":true,\\\"phpdoc_no_empty_return\\\":true,\\\"single_line_comment_style\\\":{\\\"comment_types\\\":[\\\"asterisk\\\"]},\\\"single_quote\\\":{\\\"strings_containing_single_quote_chars\\\":true},\\\"trailing_comma_in_multiline\\\":{\\\"elements\\\":[]},\\\"yoda_style\\\":{\\\"always_move_variable\\\":true}}\"",
  "--path-mode=override",
  "C:\\Users\\username\\AppData\\Local\\Temp\\pcf-tmp0.0977728712257997\\Example.php"
]
{
  "cwd": "c:\\Users\\username\\Desktop\\test\\a a",
  "shell": true
}
runAsync: success
{
  "stdout": "{\"about\":\"PHP CS Fixer 3.62.0 7th Gear by Fabien Potencier, Dariusz Ruminski and contributors.\",\"files\":[{\"name\":\"C:\\\\Users\\\\username\\\\AppData\\\\Local\\\\Temp\\\\pcf-tmp0.0977728712257997\\\\Example.php\"}],\"time\":{\"total\":0.042},\"memory\":14}",
  "stderr": "PHP CS Fixer 3.62.0 7th Gear by Fabien Potencier, Dariusz Ruminski and contributors.\nPHP runtime: 8.3.10\r\nRunning analysis on 1 core sequentially.\r\nYou can enable parallel runner and speed up the analysis! Please see https://cs.symfony.com/doc/usage.html for more information.\r\nLoaded config default.\r\n"
}
runAsync: resolve promise
{"about":"PHP CS Fixer 3.62.0 7th Gear by Fabien Potencier, Dariusz Ruminski and contributors.","files":[{"name":"C:\\Users\\username\\AppData\\Local\\Temp\\pcf-tmp0.0977728712257997\\Example.php"}],"time":{"total":0.042},"memory":14}

Linux 下 "command" 含有空格， "rules" 为字符串

runAsync: spawn "/workspaces/username/a a/vendor/bin/php-cs-fixer"
[
  "fix",
  "--using-cache=no",
  "--format=json",
  "--rules=\"@PSR12\"",
  "--path-mode=override",
  "/tmp/pcf-tmp0.3247631733940495/Example.php"
]
{
  "cwd": "/workspaces/username/a a",
  "shell": true
}
runAsync: success
{
  "stdout": "{\"about\":\"PHP CS Fixer 3.62.0 7th Gear by Fabien Potencier, Dariusz Ruminski and contributors.\",\"files\":[{\"name\":\"\\/tmp\\/pcf-tmp0.3247631733940495\\/Example.php\"}],\"time\":{\"total\":0.006},\"memory\":14}",
  "stderr": "PHP CS Fixer 3.62.0 7th Gear by Fabien Potencier, Dariusz Ruminski and contributors.\nPHP runtime: 8.2.14\nRunning analysis on 1 core sequentially.\nYou can enable parallel runner and speed up the analysis! Please see https://cs.symfony.com/doc/usage.html for more information.\nLoaded config default.\n"
}
runAsync: resolve promise
{"about":"PHP CS Fixer 3.62.0 7th Gear by Fabien Potencier, Dariusz Ruminski and contributors.","files":[{"name":"\/tmp\/pcf-tmp0.3247631733940495\/Example.php"}],"time":{"total":0.006},"memory":14}

Linux 下 "command" 含有空格， "rules" 为 JSON

runAsync: spawn "/workspaces/username/a a/vendor/bin/php-cs-fixer"
[
  "fix",
  "--using-cache=no",
  "--format=json",
  "--rules=\"{\\\"@PSR12\\\":true,\\\"no_extra_blank_lines\\\":true,\\\"binary_operator_spaces\\\":{\\\"default\\\":\\\"align_single_space\\\",\\\"operators\\\":{\\\"=>\\\":\\\"align_single_space_minimal\\\"}}}\"",
  "--path-mode=override",
  "/tmp/pcf-tmp0.8814402388895575/Example.php"
]
{
  "cwd": "/workspaces/username/a a",
  "shell": true
}
runAsync: success
{
  "stdout": "{\"about\":\"PHP CS Fixer 3.62.0 7th Gear by Fabien Potencier, Dariusz Ruminski and contributors.\",\"files\":[{\"name\":\"\\/tmp\\/pcf-tmp0.8814402388895575\\/Example.php\"}],\"time\":{\"total\":0.005},\"memory\":14}",
  "stderr": "PHP CS Fixer 3.62.0 7th Gear by Fabien Potencier, Dariusz Ruminski and contributors.\nPHP runtime: 8.2.14\nRunning analysis on 1 core sequentially.\nYou can enable parallel runner and speed up the analysis! Please see https://cs.symfony.com/doc/usage.html for more information.\nLoaded config default.\n"
}
runAsync: resolve promise
{"about":"PHP CS Fixer 3.62.0 7th Gear by Fabien Potencier, Dariusz Ruminski and contributors.","files":[{"name":"\/tmp\/pcf-tmp0.8814402388895575\/Example.php"}],"time":{"total":0.005},"memory":14}
junstyle commented 2 months ago

谢谢啊，已经更新了代码