Skip to content
This repository has been archived by the owner on Aug 12, 2023. It is now read-only.

file is mangled when the formatter (erblint) found no errors #1559

Open
5 tasks done
eleith opened this issue May 13, 2023 · 4 comments
Open
5 tasks done

file is mangled when the formatter (erblint) found no errors #1559

eleith opened this issue May 13, 2023 · 4 comments
Labels
bug Something isn't working

Comments

@eleith
Copy link

eleith commented May 13, 2023

FAQ

  • I have checked the FAQ and it didn't resolve my problem.

Issues

  • I have checked existing issues and there are no issues with the same problem.

Neovim Version

v0.8.1

Dev Version?

  • I am using a stable Neovim release version, or if I am using a dev version of Neovim I have confirmed that my issue is reproducible on a stable version.

Operating System

linux

Minimal Config

-- this template is borrowed from nvim-lspconfig
local on_windows = vim.loop.os_uname().version:match("Windows")

local function join_paths(...)
    local path_sep = on_windows and "\\" or "/"
    local result = table.concat({ ... }, path_sep)
    return result
end

vim.g.loaded_remote_plugins = ""
vim.cmd([[set runtimepath=$VIMRUNTIME]])

local temp_dir = vim.loop.os_getenv("TEMP") or "/tmp"

vim.cmd("set packpath=" .. join_paths(temp_dir, "nvim", "site"))

local package_root = join_paths(temp_dir, "nvim", "site", "pack")
local install_path = join_paths(package_root, "packer", "start", "packer.nvim")
local compile_path = join_paths(install_path, "plugin", "packer_compiled.lua")

local null_ls_config = function()
    local null_ls = require("null-ls")
    -- add only what you need to reproduce your issue
    null_ls.setup({
       debug = true,
       on_attach = function(client, bufnr)
         if client.supports_method("textDocument/formatting") then
           vim.api.nvim_clear_autocmds({ group = augroup, buffer = bufnr })
           vim.api.nvim_create_autocmd("BufWritePre", {
             group = augroup,
             buffer = bufnr,
             callback = function()
               vim.lsp.buf.format({ bufnr = bufnr })
             end,
           })
         end
       end,
        border = "single",
        sources = {
          null_ls.builtins.diagnostics.erb_lint.with({}),
          null_ls.builtins.formatting.erb_lint.with({}),
       },
    })
end

local function load_plugins()
    -- only add other plugins if they are necessary to reproduce the issue
    require("packer").startup({
        {
            "wbthomason/packer.nvim",
            {
                "jose-elias-alvarez/null-ls.nvim",
                requires = { "nvim-lua/plenary.nvim" },
                config = null_ls_config,
            },
        },
        config = {
            package_root = package_root,
            compile_path = compile_path,
        },
    })
end

if vim.fn.isdirectory(install_path) == 0 then
    vim.fn.system({ "git", "clone", "https://github.com/wbthomason/packer.nvim", install_path })
    load_plugins()
    require("packer").sync()
else
    load_plugins()
    require("packer").sync()
end

Steps to Reproduce

<% if t == t %>
   <div>help</div>
<% end %>

Reproducibility Check

  • I confirm that my minimal config is based on the minimal_init.lua template and that my issue is reproducible by running nvim --clean -u minimal_init.lua and following the steps above.

Expected Behavior

since erblint reports no errors, the file doesn't need to be formatted and there should be no change to the file after saving

Actual Behavior

when the condition includes x == x the file is formatted to

t %>
  <div />
<% end %>

but if the conditional doesn't have a comparison, this behavior isn't seen and the file is preserved as expected

Debug Log

TRACE Sat 13 May 2023 09:46:24 AM PDT] ...y/null-ls.nvim/lua/null-ls/helpers/generator_fact
  ory.lua:205: output: Linting and autocorrecting 1 files with 15 linters (12 autocorrectable)
  ...

  No errors were found in ERB files
  ================ /home/folder/test.erb ==================
  <% if t == t %>
    <div />
  <% end %>

  [DEBUG Sat 13 May 2023 09:46:24 AM PDT] .../share/nvim/lazy/null-ls.nvim/lua/null-ls/formatt
  ing.lua:89: received edits from generators
  [TRACE Sat 13 May 2023 09:46:24 AM PDT] .../share/nvim/lazy/null-ls.nvim/lua/null-ls/formatt
  ing.lua:90: {
    newText = "",
    range = {
      ["end"] = {
        character = 11,
        line = 0
      },
      start = {
        character = 0,
        line = 0
      }
    },
    rangeLength = 11
  }
  [TRACE Sat 13 May 2023 09:46:24 AM PDT] .../.local/share/nvim/lazy/null-ls.nvim/lua/null-ls/
  rpc.lua:127: received LSP notification for method textDocument/didChange
  [TRACE Sat 13 May 2023 09:46:24 AM PDT] .../share/nvim/lazy/null-ls.nvim/lua/null-ls/generat
  ors.lua:21: running generators for method NULL_LS_DIAGNOSTICS
  [DEBUG Sat 13 May 2023 09:46:24 AM PDT] ...y/null-ls.nvim/lua/null-ls/helpers/generator_fact
  ory.lua:320: spawning command "erblint" at /home/folder/monorail with args { "--fo
  rmat", "json", "--stdin", "/home/folder/test.erb" }
  [TRACE Sat 13 May 2023 09:46:24 AM PDT] .../.local/share/nvim/lazy/null-ls.nvim/lua/null-ls/
  rpc.lua:127: received LSP notification for method textDocument/didSave
  [TRACE Sat 13 May 2023 09:46:24 AM PDT] .../share/nvim/lazy/null-ls.nvim/lua/null-ls/generat
  ors.lua:21: running generators for method NULL_LS_DIAGNOSTICS_ON_SAVE
  [DEBUG Sat 13 May 2023 09:46:24 AM PDT] .../share/nvim/lazy/null-ls.nvim/lua/null-ls/generat
  ors.lua:24: no generators available
  [TRACE Sat 13 May 2023 09:46:25 AM PDT] ...y/null-ls.nvim/lua/null-ls/helpers/generator_fact
  ory.lua:204: error output: nil
[TRACE Sat 13 May 2023 09:46:25 AM PDT] ...y/null-ls.nvim/lua/null-ls/helpers/generator_fact
  ory.lua:205: output: {"metadata":{"erb_lint_version":"0.4.0","ruby_engine":"ruby","ruby_vers
  ion":"3.2.2","ruby_patchlevel":"53","ruby_platform":"x86_64-linux"},"files":[{"path":"test.e
  rb","offenses":[]}],"summary":{"offenses":0,"inspected_files":1,"corrected":0}}

Help

Yes, but I don't know how to start. I would need guidance

Implementation Help

No response

Requirements

  • I have read and followed the instructions above and understand that my issue will be closed if I did not provide the required information.
@eleith eleith added the bug Something isn't working label May 13, 2023
@eleith eleith changed the title file is formatted when no errors were found by erb-lint file is formatted (mangled) even though formatted (erblint) found no errors May 13, 2023
@eleith eleith changed the title file is formatted (mangled) even though formatted (erblint) found no errors file is mangled when the formatter (erblint) found no errors May 13, 2023
@jose-elias-alvarez
Copy link
Owner

Looks like the exit code doesn't account for errors: Shopify/erb_lint#118

Ideally the executable itself would be better behaved, but a potential fix from our end is something like this:

diff --git a/lua/null-ls/builtins/formatting/erb_lint.lua b/lua/null-ls/builtins/formatting/erb_lint.lua
index 78e9144..3445943 100644
--- a/lua/null-ls/builtins/formatting/erb_lint.lua
+++ b/lua/null-ls/builtins/formatting/erb_lint.lua
@@ -20,6 +20,10 @@ return h.make_builtin({
         output = "raw",
         on_output = function(params, done)
             local output = params.output
+            if output:find("No errors were found in ERB files") then
+                return done()
+            end
+
             local metadata_end = output:match(".*==()") + 1
             return done({ { text = output:sub(metadata_end) } })
         end,

I'm not a user, so I can't say for sure if this is a good fix / how this would handle non-English messages.

@eleith
Copy link
Author

eleith commented May 16, 2023

wow. thanks for narrowing that down.

is it possible to overload this from the source setup in anyway? or is modifying the built-in the only approach that will work?

for now, i'll explore the possibility of adding an exit code to erb-lint

@jose-elias-alvarez
Copy link
Owner

jose-elias-alvarez commented May 16, 2023

It's dirty, but you should be able to do this in the meantime:

-- add to table of sources and register
local erb_lint = null_ls.builtins.formatting.erb_lint.with({
    on_output = function(params, done)
        local output = params.output
        if output:find("No errors were found in ERB files") then
            return done()
        end

        local metadata_end = output:match(".*==()") + 1
        return done({ { text = output:sub(metadata_end) } })
    end,
})

If the solution works for you it might be worth putting in a PR, though, since as far as I can tell this might be affecting other users (and I'm surprised this is the first report).

@fidalgo
Copy link

fidalgo commented Aug 5, 2023

Since the erb_lint has already implemented different exit codes, can we close this?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants