Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

bug: phpcbf overwrites php-cs-fixer config with stats #289

Closed
1 task done
Rasmus-Bertell opened this issue Feb 3, 2024 · 10 comments · Fixed by #333
Closed
1 task done

bug: phpcbf overwrites php-cs-fixer config with stats #289

Rasmus-Bertell opened this issue Feb 3, 2024 · 10 comments · Fixed by #333
Labels
bug Something isn't working

Comments

@Rasmus-Bertell
Copy link

Neovim version (nvim -v)

0.9.5

Operating system/version

Archlinux 6.7.3-arch1-1

Add the debug logs

  • I have set log_level = vim.log.levels.DEBUG and pasted the log contents below.

Log file

14:16:10[DEBUG] Running formatters on /home/rasmus/projects/wt-dev/.php-cs-fixer.dist.php: { "phpcbf", "php_cs_fixer", "typos", "trim_newlines", "trim_whitespace" }
14:16:10[INFO] Run phpcbf on /home/rasmus/projects/wt-dev/.php-cs-fixer.dist.php
14:16:10[DEBUG] Run command: { "/home/rasmus/projects/wt-dev/vendor/bin/phpcbf", "-q", "--stdin-path=/home/rasmus/projects/wt-dev/.php-cs-fixer.dist.php", "-" }
14:16:10[DEBUG] phpcbf exited with code 0
14:16:10[INFO] Run php_cs_fixer on /home/rasmus/projects/wt-dev/.php-cs-fixer.dist.php
14:16:10[DEBUG] Creating temp file /home/rasmus/projects/wt-dev/.conform.2575698..php-cs-fixer.dist.php
14:16:10[DEBUG] Run command: { "/home/rasmus/projects/wt-dev/vendor/bin/php-cs-fixer", "fix", "/home/rasmus/projects/wt-dev/.conform.2575698..php-cs-fixer.dist.php" }
14:16:10[DEBUG] php_cs_fixer exited with code 0
14:16:10[DEBUG] Cleaning up temp file /home/rasmus/projects/wt-dev/.conform.2575698..php-cs-fixer.dist.php
14:16:10[INFO] Run typos on /home/rasmus/projects/wt-dev/.php-cs-fixer.dist.php
14:16:10[DEBUG] Creating temp file /home/rasmus/projects/wt-dev/.conform.5625946..php-cs-fixer.dist.php
14:16:10[DEBUG] Run command: { "typos", "--write-changes", "--force-exclude", "/home/rasmus/projects/wt-dev/.conform.5625946..php-cs-fixer.dist.php" }
14:16:10[DEBUG] typos exited with code 0
14:16:10[DEBUG] Cleaning up temp file /home/rasmus/projects/wt-dev/.conform.5625946..php-cs-fixer.dist.php
14:16:10[INFO] Run trim_newlines on /home/rasmus/projects/wt-dev/.php-cs-fixer.dist.php
14:16:10[DEBUG] Run command: { "awk", 'NF{print s $0; s=""; next} {s=s ORS}' }
14:16:10[DEBUG] trim_newlines exited with code 0
14:16:10[INFO] Run trim_whitespace on /home/rasmus/projects/wt-dev/.php-cs-fixer.dist.php
14:16:10[DEBUG] Run command: { "awk", '{ sub(/[ \t]+$/, ""); print }' }
14:16:10[DEBUG] trim_whitespace exited with code 0
14:17:11[DEBUG] Running LSP formatter on /home/rasmus/.config/nvim/lua/plugins/conform-nvim/init.lua
14:17:11[DEBUG] Converting full-file LSP format to piecewise format
14:17:15[DEBUG] Running formatters on /home/rasmus/projects/wt-dev/.php-cs-fixer.dist.php: { "phpcbf", "typos", "trim_newlines", "trim_whitespace" }
14:17:15[INFO] Run phpcbf on /home/rasmus/projects/wt-dev/.php-cs-fixer.dist.php
14:17:15[DEBUG] Run command: { "/home/rasmus/projects/wt-dev/vendor/bin/phpcbf", "-q", "--stdin-path=/home/rasmus/projects/wt-dev/.php-cs-fixer.dist.php", "-" }
14:17:15[DEBUG] phpcbf exited with code 0
14:17:15[INFO] Run typos on /home/rasmus/projects/wt-dev/.php-cs-fixer.dist.php
14:17:15[DEBUG] Creating temp file /home/rasmus/projects/wt-dev/.conform.3044990..php-cs-fixer.dist.php
14:17:15[DEBUG] Run command: { "typos", "--write-changes", "--force-exclude", "/home/rasmus/projects/wt-dev/.conform.3044990..php-cs-fixer.dist.php" }
14:17:15[DEBUG] typos exited with code 0
14:17:15[DEBUG] Cleaning up temp file /home/rasmus/projects/wt-dev/.conform.3044990..php-cs-fixer.dist.php
14:17:15[INFO] Run trim_newlines on /home/rasmus/projects/wt-dev/.php-cs-fixer.dist.php
14:17:15[DEBUG] Run command: { "awk", 'NF{print s $0; s=""; next} {s=s ORS}' }
14:17:15[DEBUG] trim_newlines exited with code 0
14:17:15[INFO] Run trim_whitespace on /home/rasmus/projects/wt-dev/.php-cs-fixer.dist.php
14:17:15[DEBUG] Run command: { "awk", '{ sub(/[ \t]+$/, ""); print }' }
14:17:15[DEBUG] trim_whitespace exited with code 0
14:17:33[DEBUG] Running LSP formatter on /home/rasmus/.config/nvim/lua/plugins/conform-nvim/init.lua
14:17:37[DEBUG] Running formatters on /home/rasmus/projects/wt-dev/.php-cs-fixer.dist.php: { "php_cs_fixer", "typos", "trim_newlines", "trim_whitespace" }
14:17:37[INFO] Run php_cs_fixer on /home/rasmus/projects/wt-dev/.php-cs-fixer.dist.php
14:17:37[DEBUG] Creating temp file /home/rasmus/projects/wt-dev/.conform.7351072..php-cs-fixer.dist.php
14:17:37[DEBUG] Run command: { "/home/rasmus/projects/wt-dev/vendor/bin/php-cs-fixer", "fix", "/home/rasmus/projects/wt-dev/.conform.7351072..php-cs-fixer.dist.php" }
14:17:37[DEBUG] php_cs_fixer exited with code 0
14:17:37[DEBUG] Cleaning up temp file /home/rasmus/projects/wt-dev/.conform.7351072..php-cs-fixer.dist.php
14:17:37[INFO] Run typos on /home/rasmus/projects/wt-dev/.php-cs-fixer.dist.php
14:17:37[DEBUG] Creating temp file /home/rasmus/projects/wt-dev/.conform.5358265..php-cs-fixer.dist.php
14:17:37[DEBUG] Run command: { "typos", "--write-changes", "--force-exclude", "/home/rasmus/projects/wt-dev/.conform.5358265..php-cs-fixer.dist.php" }
14:17:37[DEBUG] typos exited with code 0
14:17:37[DEBUG] Cleaning up temp file /home/rasmus/projects/wt-dev/.conform.5358265..php-cs-fixer.dist.php
14:17:37[INFO] Run trim_newlines on /home/rasmus/projects/wt-dev/.php-cs-fixer.dist.php
14:17:37[DEBUG] Run command: { "awk", 'NF{print s $0; s=""; next} {s=s ORS}' }
14:17:38[DEBUG] trim_newlines exited with code 0
14:17:38[INFO] Run trim_whitespace on /home/rasmus/projects/wt-dev/.php-cs-fixer.dist.php
14:17:38[DEBUG] Run command: { "awk", '{ sub(/[ \t]+$/, ""); print }' }
14:17:38[DEBUG] trim_whitespace exited with code 0
14:18:15[DEBUG] Running LSP formatter on /home/rasmus/.config/nvim/lua/plugins/conform-nvim/init.lua
14:18:15[DEBUG] Converting full-file LSP format to piecewise format
14:22:35[DEBUG] Running formatters on /home/rasmus/projects/wt-dev/app/Models/Locale.php: { "phpcbf", "php_cs_fixer", "typos", "trim_newlines", "trim_whitespace" }
14:22:35[INFO] Run phpcbf on /home/rasmus/projects/wt-dev/app/Models/Locale.php
14:22:35[DEBUG] Run command: { "/home/rasmus/projects/wt-dev/vendor/bin/phpcbf", "-q", "--stdin-path=/home/rasmus/projects/wt-dev/app/Models/Locale.php", "-" }
14:22:35[DEBUG] phpcbf exited with code 1
14:22:35[INFO] Run php_cs_fixer on /home/rasmus/projects/wt-dev/app/Models/Locale.php
14:22:35[DEBUG] Creating temp file /home/rasmus/projects/wt-dev/app/Models/.conform.4922251.Locale.php
14:22:35[DEBUG] Run command: { "/home/rasmus/projects/wt-dev/vendor/bin/php-cs-fixer", "fix", "/home/rasmus/projects/wt-dev/app/Models/.conform.4922251.Locale.php" }
14:22:36[DEBUG] php_cs_fixer exited with code 0
14:22:36[DEBUG] Cleaning up temp file /home/rasmus/projects/wt-dev/app/Models/.conform.4922251.Locale.php
14:22:36[INFO] Run typos on /home/rasmus/projects/wt-dev/app/Models/Locale.php
14:22:36[DEBUG] Creating temp file /home/rasmus/projects/wt-dev/app/Models/.conform.4931772.Locale.php
14:22:36[DEBUG] Run command: { "typos", "--write-changes", "--force-exclude", "/home/rasmus/projects/wt-dev/app/Models/.conform.4931772.Locale.php" }
14:22:36[DEBUG] typos exited with code 0
14:22:36[DEBUG] Cleaning up temp file /home/rasmus/projects/wt-dev/app/Models/.conform.4931772.Locale.php
14:22:36[INFO] Run trim_newlines on /home/rasmus/projects/wt-dev/app/Models/Locale.php
14:22:36[DEBUG] Run command: { "awk", 'NF{print s $0; s=""; next} {s=s ORS}' }
14:22:36[DEBUG] trim_newlines exited with code 0
14:22:36[INFO] Run trim_whitespace on /home/rasmus/projects/wt-dev/app/Models/Locale.php
14:22:36[DEBUG] Run command: { "awk", '{ sub(/[ \t]+$/, ""); print }' }
14:22:36[DEBUG] trim_whitespace exited with code 0

Describe the bug

phpcbf formatter seems to overwrite the file whenever it runs successfully and for some reason the only file it seems to run successfully is .php-cs-fixer.dist.php.

It overwrites the file with


No fixable errors were found

Time: 22ms; Memory: 8MB

What is the severity of this bug?

breaking (some functionality is broken)

Steps To Reproduce

  1. nvim -u repro.lua .php-cs-fixer.dist.php
  2. :w

Expected Behavior

I expect it to format the file according to the rulesets I have defined or if no rulesets are defined by the defaults.

Minimal example file

.php-cs-fixer.dist.php (phpcbf exits with code 0 and overwrites)

<?php

declare(strict_types=1);

app/Models/Locale.php (phpcbf exits with code 1 and does nothing)

<?php

declare(strict_types=1);

Minimal init.lua

-- DO NOT change the paths and don't remove the colorscheme
local root = vim.fn.fnamemodify("./.repro", ":p")

-- set stdpaths to use .repro
for _, name in ipairs({ "config", "data", "state", "cache" }) do
  vim.env[("XDG_%s_HOME"):format(name:upper())] = root .. "/" .. name
end

-- bootstrap lazy
local lazypath = root .. "/plugins/lazy.nvim"
if not vim.loop.fs_stat(lazypath) then
  vim.fn.system({
    "git",
    "clone",
    "--filter=blob:none",
    "--single-branch",
    "https://github.com/folke/lazy.nvim.git",
    lazypath,
  })
end
vim.opt.runtimepath:prepend(lazypath)

-- install plugins
local plugins = {
  "folke/tokyonight.nvim",
  {
    "stevearc/conform.nvim",
    config = function()
      require("conform").setup({
        log_level = vim.log.levels.DEBUG,
        formatters_by_ft = {
          php = { 'phpcbf', 'php_cs_fixer' },
          json = { 'jq' },
          ['*'] = { 'typos', 'trim_newlines', 'trim_whitespace' },
        },
        format_on_save = function()
          if not vim.g.enable_autoformat then return end

          return { timeout_ms = 500, lsp_fallback = true }
        end,
      })
    end,
    init = function()
      vim.g.enable_autoformat = true
      vim.o.formatexpr = "v:lua.require'conform'.formatexpr()"
    end,
  },
  -- add any other plugins here
}
require("lazy").setup(plugins, {
  root = root .. "/plugins",
})

vim.cmd.colorscheme("tokyonight")
-- add anything else here

Additional context

No response

@Rasmus-Bertell Rasmus-Bertell added the bug Something isn't working label Feb 3, 2024
@stevearc
Copy link
Owner

My guess is that phpcbf is not a well-behaved formatter when it comes to stdin/stdout. The correct behavior is, if formatting to stdout, to put all these diagnostic messages in stderr so tools like conform don't parse it as part of the formatted file. My guess is we'll have to either change it to operate on a file instead of stdin, or modify it to not accept exit code 1 as a success. @rawalplawit you added this formatter, any thoughts?

@stevearc stevearc added the question Further information is requested label Feb 21, 2024
@Rasmus-Bertell
Copy link
Author

I have to dig into how php-cs-fixer works for other reasons so I might be able to check how phpcbf does the formatting at the same time and do some more debugging. Will report back with my findings and a possible fix if I'm able to do it.

@github-actions github-actions bot removed the question Further information is requested label Mar 8, 2024
@star-szr
Copy link

star-szr commented Mar 8, 2024

I found the following which seems to confirm that we should treat at least 0 and 1 exit codes as success, I have not dug into the code to see if there’s anything else we should do: squizlabs/PHP_CodeSniffer#1818 (comment)

Note that the exit codes are likely to change with the future release 4.x of phpcbf which has a new maintainer and repo: PHPCSStandards/PHP_CodeSniffer#184

Edit: looks like we are probably already handling the exit codes correctly: https://github.com/stevearc/conform.nvim/blob/5a15cc46e75cad804fd51ec5af9227aeb1d1bdaa/lua/conform/formatters/phpcbf.lua

@Rasmus-Bertell
Copy link
Author

I did some digging and I think the problem might be that when phpcbf exits with a code of 0 it outputs the following message to stdout, it can't be supressed with -q.

No fixable errors were found

Time: 22ms; Memory: 8MB

I haven't however found out why it doesn't work on files when it exits with a code 1. It seems to work when run from the command line fixing the file as intended. I also haven't figured out why it returns with an exit code of 1 when there are no errors for it to fix, but that is not a conform issue in my opinion.

@Rasmus-Bertell
Copy link
Author

Rasmus-Bertell commented Mar 13, 2024

I also tried to change the formatter config to not use stdin but it didn't help. I propably don't know how to configure it properly.

local util = require("conform.util")

---@type conform.FileFormatterConfig
return {
  meta = {
    url = "https://phpqa.io/projects/phpcbf.html",
    description = "PHP Code Beautifier and Fixer fixes violations of a defined coding standard.",
  },
  command = util.find_executable({
    "vendor/bin/phpcbf",
  }, "phpcbf"),
  args = { "$FILENAME" },
  stdin = false,
  -- 0: no errors found
  -- 1: errors found
  -- 2: fixable errors found
  -- 3: processing error
  exit_codes = { 0, 1, 2 },
}

logs:

22:35:36[DEBUG] Running formatters on /home/rasmus/projects/xxxxx/app/Models/Locale.php: { "phpcbf" }
22:35:36[INFO] Run phpcbf on /home/rasmus/projects/xxxxx/app/Models/Locale.php
22:35:36[DEBUG] Creating temp file /home/rasmus/projects/xxxxx/app/Models/.conform.6949964.Locale.php
22:35:36[DEBUG] Run command: { "phpcbf", "/home/rasmus/projects/xxxxx/app/Models/.conform.6949964.Locale.php" }
22:35:36[DEBUG] Run CWD: /home/rasmus/projects/xxxxx
22:35:36[DEBUG] phpcbf exited with code 0
22:35:36[DEBUG] Cleaning up temp file /home/rasmus/projects/xxxxx/app/Models/.conform.6949964.Locale.php

Running the command phpcbf app/Models/Locale.php works and returns exit code 1.

@stevearc
Copy link
Owner

It is possible that phpcbf cares about the filename. If you cp app/Models/Locale.php app/Models/.conform.6949964.Locale.php and then run phpcbf app/Models/.conform.6949964.Locale.php, does it format the file and exit with code 1?

@stevearc stevearc added the question Further information is requested label Mar 14, 2024
@Rasmus-Bertell
Copy link
Author

I got around to testing this and seems like phpcbf doesn't for some reason format the file, it reports there were no errors. The files are identical but the first command has an exit code 0 and the second one 1. This seems to be more of an issue with phpcbf than with conform.

$ phpcbf app/Models/.conform.6949964.Locale.php

No fixable errors were found

Time: 33ms; Memory: 6MB
$ phpcbf app/Models/Locale.php

PHPCBF RESULT SUMMARY
----------------------------------------------------------------------
FILE                                                  FIXED  REMAINING
----------------------------------------------------------------------
/home/rasmus/projects/xxxxx/app/Models/Locale.php     2      9
----------------------------------------------------------------------
A TOTAL OF 2 ERRORS WERE FIXED IN 1 FILE
----------------------------------------------------------------------

Time: 64ms; Memory: 8MB

@github-actions github-actions bot removed the question Further information is requested label Mar 14, 2024
@Rasmus-Bertell
Copy link
Author

Opened up an issue about the ignored hidden files, hopefully it will fix the no stdin solution and I can make a PR out of it.
PHPCSStandards/PHP_CodeSniffer#395

@stevearc
Copy link
Owner

Can you try out #333 and see if that fixes your issue?

@Rasmus-Bertell
Copy link
Author

That seems to fix the problem, thank you. I'll open up a PR if phpcbf decides to accept hidden files at some point.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants