emmylua_check as main linter by ibhagwan · Pull Request #2776 · ibhagwan/fzf-lua

ibhagwan · 2026-07-07T17:13:48Z

IDK how coderabbit came out with this summary but it's totally wrong lol

What this PR does

Removes lua_language_server from lint step
Sets emmylua_check as main linter and errors on warnings
Adds a makefile step to automatically download emmylua_check on make emmylua_check
Uses emmyua_check 0.23.1 due to a freeze issue with 0.23.2 (will use latest on next release)
Fixes all emmylua_check warnings

The below is the garbage generated by coderabbitai:

Summary by CodeRabbit

Bug Fixes
- Fixed an issue where the grep_lgrep action could ignore the multiprocess setting.
- Improved the health report formatting for FZF_DEFAULT_OPTS.
Chores
- Updated CI linting and Makefile targets to install/run the EmmyLua linter via a dedicated dependency step and the project’s make workflow.
- Enhanced lint behavior to treat more linter warnings as errors and ignore generated dependency paths.
- Applied project-wide type/diagnostic annotation cleanups for editor tooling.

coderabbitai · 2026-07-07T17:14:31Z

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

@coderabbitai resume to resume automatic reviews.
@coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

▶️ Resume reviews
🔍 Trigger review

📝 Walkthrough

Walkthrough

This PR updates lint tooling to provision emmylua_check from the Makefile, narrows several option/result type annotations, and adds many LuaLS/EmmyLua casts and diagnostic suppressions across runtime code, previewers, providers, scripts, and tests. It also fixes one multiprocess field reference.

Changes

Lint Tooling

Layer / File(s)	Summary
Workflow and dependency setup `.github/workflows/lint.yaml`, `.gitignore`	Replaces the workflow’s Rust/cargo-based `emmylua_check` install with a Makefile-backed install step and ignores the `.emmylua/` directory.
Makefile lint targets `Makefile`	Adds `deps/emmylua_check`, updates `emmylua-check` to use the local binary with warnings-as-errors, and keeps the `lint` command behavior intact.

Lua Annotations and Fixes

Layer / File(s)	Summary
Public type contracts `lua/fzf-lua/actions.lua`, `lua/fzf-lua/core.lua`, `lua/fzf-lua/shell.lua`, `lua/fzf-lua/win/border.lua`, `lua/fzf-lua/types.lua`	Narrows several option annotations to `fzf-lua.config.Resolved` and expands config/result type fields used elsewhere.
Core and helper annotations `lua/fzf-lua/_health.lua`, `lua/fzf-lua/config.lua`, `lua/fzf-lua/core.lua`, `lua/fzf-lua/fzf.lua`, `lua/fzf-lua/init.lua`, `lua/fzf-lua/libuv.lua`, `lua/fzf-lua/spawn.lua`, `lua/fzf-lua/shell.lua`, `lua/fzf-lua/utils.lua`, `lua/fzf-lua/complete.lua`, `lua/fzf-lua/devicons.lua`, `lua/fzf-lua/make_entry.lua`, `lua/fzf-lua/path.lua`, `lua/fzf-lua/profiles/*`, `lua/fzf-lua/defaults.lua`, `lua/fzf-lua/win.lua`	Adds casts and diagnostic suppressions around coroutine handling, option normalization, debug-info helpers, path handling, and window/layout helpers without changing control flow.
Actions and providers annotations `lua/fzf-lua/actions.lua`, `lua/fzf-lua/cmd.lua`, `lua/fzf-lua/providers/*`	Adds casts and diagnostic suppressions across action/provider modules, and corrects `opts.multiprcess` to `opts.multiprocess` in `grep_lgrep`.
Previewer annotations `lua/fzf-lua/previewer/*`	Adds casts and diagnostic suppressions in previewer helpers, markdown rendering, tree-sitter context requires, and screenshot/error paths.
Scripts and tests annotations `scripts/`, `tests/`, `lua/fzf-lua/test/*`	Adds diagnostic suppressions and typed locals in generator scripts and tests, plus small test-stub adjustments.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Possibly related PRs

ibhagwan/fzf-lua#2764: Both PRs change the lint workflow and emmylua_check installation path.
ibhagwan/fzf-lua#2580: Both PRs touch lua/fzf-lua/previewer/tscontext.lua and related previewer loading paths.
ibhagwan/fzf-lua#2667: Both PRs modify lua/fzf-lua/actions.lua, including M.vimcmd_entry.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title matches the main change: making emmylua_check the primary linter in CI and the Makefile.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch ci/emmylua_check

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

coderabbitai

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@lua/fzf-lua/providers/tags.lua`:
- Around line 260-268: The `M.grep_cword` and `M.grep_cWORD` search assignments
are malformed because the inline `--[[`@as` string]]` cast consumes the closing
parentheses. Fix this by hoisting each `vim.fn.expand("<cword>")` and
`vim.fn.expand("<cWORD>")` call into a local variable, casting the local
instead, and then passing that variable into `utils.rg_escape_cword` and
`utils.rg_escape` so the `opts.search` expressions remain syntactically correct.

In `@lua/fzf-lua/test/screenshot.lua`:
- Around line 86-87: The screenshot helper in nvim_buf_get_lines still adds 1 to
opts.end_line before checking whether it exists, so missing end_line can crash
the call. Update the line-range calculation in the screenshot.lua helper to
guard opts.end_line first and only apply +1 when it is present, keeping the
existing opts.start_line behavior intact and avoiding reliance on the nil-check
suppression.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: de1f3f4e-3f06-4b45-98ba-3cc7000b82fd

📥 Commits

Reviewing files that changed from the base of the PR and between ed4059d and 8ff0093.

📒 Files selected for processing (55)

.github/workflows/lint.yaml
Makefile
lua/fzf-lua/_health.lua
lua/fzf-lua/actions.lua
lua/fzf-lua/cmd.lua
lua/fzf-lua/complete.lua
lua/fzf-lua/config.lua
lua/fzf-lua/core.lua
lua/fzf-lua/defaults.lua
lua/fzf-lua/devicons.lua
lua/fzf-lua/fzf.lua
lua/fzf-lua/init.lua
lua/fzf-lua/libuv.lua
lua/fzf-lua/make_entry.lua
lua/fzf-lua/path.lua
lua/fzf-lua/previewer/builtin.lua
lua/fzf-lua/previewer/codeaction.lua
lua/fzf-lua/previewer/fzf.lua
lua/fzf-lua/previewer/init.lua
lua/fzf-lua/previewer/tscontext.lua
lua/fzf-lua/profiles/cli.lua
lua/fzf-lua/profiles/hide.lua
lua/fzf-lua/providers/buffers.lua
lua/fzf-lua/providers/colorschemes.lua
lua/fzf-lua/providers/dap.lua
lua/fzf-lua/providers/diagnostic.lua
lua/fzf-lua/providers/files.lua
lua/fzf-lua/providers/git.lua
lua/fzf-lua/providers/helptags.lua
lua/fzf-lua/providers/lsp.lua
lua/fzf-lua/providers/meta.lua
lua/fzf-lua/providers/nvim.lua
lua/fzf-lua/providers/oldfiles.lua
lua/fzf-lua/providers/quickfix.lua
lua/fzf-lua/providers/tags.lua
lua/fzf-lua/providers/ui_select.lua
lua/fzf-lua/providers/undotree.lua
lua/fzf-lua/shell.lua
lua/fzf-lua/spawn.lua
lua/fzf-lua/test/exec_lua.lua
lua/fzf-lua/test/helpers.lua
lua/fzf-lua/test/screenshot.lua
lua/fzf-lua/types.lua
lua/fzf-lua/utils.lua
lua/fzf-lua/win.lua
lua/fzf-lua/win/border.lua
scripts/cli.lua
scripts/gen_options.lua
scripts/gen_types.lua
tests/api_spec.lua
tests/cache_spec.lua
tests/codeaction_spec.lua
tests/path_spec.lua
tests/ui_select_spec.lua
tests/win_spec.lua

💤 Files with no reviewable changes (1)

.github/workflows/lint.yaml

coderabbitai

Actionable comments posted: 1

🧹 Nitpick comments (1)

Makefile (1)
69-87: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Target body length exceeds recommended line count.

Static analysis flags both deps/emmylua_check and emmylua-check targets for exceeding the recommended 5-line body length. This is stylistic only; consider extracting the download logic into a shell script under scripts/ if readability becomes a concern, but not essential for this PR.

Also applies to: 94-101
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Makefile` around lines 69 - 87, The Makefile target bodies for
deps/emmylua_check and emmylua-check are too long and trigger the line-count
warning. Refactor the download/install logic out of these targets into a
reusable shell script or helper target, and keep the Makefile recipe bodies
minimal while preserving the existing behavior of the deps/emmylua_check and
emmylua-check workflows.
Source: Linters/SAST tools

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@Makefile`:
- Around line 71-75: The fast path in the Makefile for locating emmylua_check
creates an empty executable placeholder, which causes the emmylua-check target
to use a bogus binary instead of the real tool on PATH. Update the branch that
handles command -v emmylua_check so it records the actual executable path for
EMMY_CHECK_BIN, ideally by symlinking or otherwise pointing to the discovered
binary instead of touch-ing an empty file; also ensure the emmylua-check target
continues to prefer a valid binary via the EMMY and EMMY_CHECK_BIN variables.

---

Nitpick comments:
In `@Makefile`:
- Around line 69-87: The Makefile target bodies for deps/emmylua_check and
emmylua-check are too long and trigger the line-count warning. Refactor the
download/install logic out of these targets into a reusable shell script or
helper target, and keep the Makefile recipe bodies minimal while preserving the
existing behavior of the deps/emmylua_check and emmylua-check workflows.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 29814686-198d-4ca6-9de8-78bdd64871e6

📥 Commits

Reviewing files that changed from the base of the PR and between 8ff0093 and a08fb72.

📒 Files selected for processing (55)

.github/workflows/lint.yaml
Makefile
lua/fzf-lua/_health.lua
lua/fzf-lua/actions.lua
lua/fzf-lua/cmd.lua
lua/fzf-lua/complete.lua
lua/fzf-lua/config.lua
lua/fzf-lua/core.lua
lua/fzf-lua/defaults.lua
lua/fzf-lua/devicons.lua
lua/fzf-lua/fzf.lua
lua/fzf-lua/init.lua
lua/fzf-lua/libuv.lua
lua/fzf-lua/make_entry.lua
lua/fzf-lua/path.lua
lua/fzf-lua/previewer/builtin.lua
lua/fzf-lua/previewer/codeaction.lua
lua/fzf-lua/previewer/fzf.lua
lua/fzf-lua/previewer/init.lua
lua/fzf-lua/previewer/tscontext.lua
lua/fzf-lua/profiles/cli.lua
lua/fzf-lua/profiles/hide.lua
lua/fzf-lua/providers/buffers.lua
lua/fzf-lua/providers/colorschemes.lua
lua/fzf-lua/providers/dap.lua
lua/fzf-lua/providers/diagnostic.lua
lua/fzf-lua/providers/files.lua
lua/fzf-lua/providers/git.lua
lua/fzf-lua/providers/helptags.lua
lua/fzf-lua/providers/lsp.lua
lua/fzf-lua/providers/meta.lua
lua/fzf-lua/providers/nvim.lua
lua/fzf-lua/providers/oldfiles.lua
lua/fzf-lua/providers/quickfix.lua
lua/fzf-lua/providers/tags.lua
lua/fzf-lua/providers/ui_select.lua
lua/fzf-lua/providers/undotree.lua
lua/fzf-lua/shell.lua
lua/fzf-lua/spawn.lua
lua/fzf-lua/test/exec_lua.lua
lua/fzf-lua/test/helpers.lua
lua/fzf-lua/test/screenshot.lua
lua/fzf-lua/types.lua
lua/fzf-lua/utils.lua
lua/fzf-lua/win.lua
lua/fzf-lua/win/border.lua
scripts/cli.lua
scripts/gen_options.lua
scripts/gen_types.lua
tests/api_spec.lua
tests/cache_spec.lua
tests/codeaction_spec.lua
tests/path_spec.lua
tests/ui_select_spec.lua
tests/win_spec.lua

✅ Files skipped from review due to trivial changes (47)

lua/fzf-lua/_health.lua
lua/fzf-lua/providers/quickfix.lua
tests/ui_select_spec.lua
lua/fzf-lua/fzf.lua
lua/fzf-lua/providers/helptags.lua
lua/fzf-lua/test/exec_lua.lua
lua/fzf-lua/cmd.lua
lua/fzf-lua/providers/lsp.lua
tests/codeaction_spec.lua
lua/fzf-lua/providers/files.lua
lua/fzf-lua/previewer/codeaction.lua
tests/cache_spec.lua
lua/fzf-lua/spawn.lua
lua/fzf-lua/win/border.lua
lua/fzf-lua/profiles/cli.lua
scripts/cli.lua
scripts/gen_options.lua
lua/fzf-lua/test/screenshot.lua
lua/fzf-lua/providers/dap.lua
tests/win_spec.lua
lua/fzf-lua/previewer/tscontext.lua
lua/fzf-lua/defaults.lua
lua/fzf-lua/providers/meta.lua
lua/fzf-lua/providers/colorschemes.lua
lua/fzf-lua/complete.lua
lua/fzf-lua/providers/oldfiles.lua
lua/fzf-lua/providers/diagnostic.lua
lua/fzf-lua/providers/buffers.lua
lua/fzf-lua/devicons.lua
scripts/gen_types.lua
lua/fzf-lua/providers/undotree.lua
lua/fzf-lua/make_entry.lua
lua/fzf-lua/test/helpers.lua
lua/fzf-lua/config.lua
tests/api_spec.lua
lua/fzf-lua/shell.lua
lua/fzf-lua/providers/ui_select.lua
lua/fzf-lua/path.lua
lua/fzf-lua/previewer/fzf.lua
lua/fzf-lua/libuv.lua
lua/fzf-lua/core.lua
lua/fzf-lua/win.lua
lua/fzf-lua/providers/nvim.lua
lua/fzf-lua/providers/git.lua
lua/fzf-lua/previewer/builtin.lua
lua/fzf-lua/init.lua
lua/fzf-lua/providers/tags.lua

🚧 Files skipped from review as they are similar to previous changes (7)

lua/fzf-lua/previewer/init.lua
lua/fzf-lua/profiles/hide.lua
.github/workflows/lint.yaml
tests/path_spec.lua
lua/fzf-lua/types.lua
lua/fzf-lua/utils.lua
lua/fzf-lua/actions.lua

ibhagwan · 2026-07-07T19:13:22Z

@phanen, comments?

before I merge this I want to also add an automatic download of emmylua_format/luafmt and setup a format check as well - what do you think?

ibhagwan · 2026-07-08T01:08:41Z

One issue I’m adding is that luafmt apparently does not take into account lua block comments, take for example the section below gets formatted, the —[ turns into — [ which breaks the comment (causing errors). If I add space_after_comment_dash = false it won’t break the comment but it still formats the commented block as if it was code and also creates other inconsistencies with comments+space.

Any ideas @phanen? Or should we open an upstream issue as this is IMHO a bug.

fzf-lua/lua/fzf-lua/utils.lua

Lines 743 to 788 in df054af

    
           -- auto generate ansi escape sequence from RGB or neovim highlights 
        
           --[[ M.ansi_auto = setmetatable({}, { 
        
             -- __index metamethod only gets called when the item does not exist 
        
             -- we use this to auto-cache the ansi escape sequence 
        
             __index = function(self, k) 
        
               print("get", k) 
        
               local escseq 
        
               -- if not an existing highlight group lookup 
        
               -- in the neovim colormap and convert to RGB 
        
               if not k:match("^#") and vim.fn.hlexists(k) ~= 1 then 
        
                 local col = M.COLORMAP()[k:sub(1, 1):upper() .. k:sub(2):lower()] 
        
                 if col then 
        
                   -- format as 6 digit hex for hex2rgb() 
        
                   k = ("#%06x"):format(col) 
        
                 end 
        
               end 
        
               if k:match("#%x%x%x%x%x%x") then -- index is RGB 
        
                 -- cache the sequence as all lowercase 
        
                 k = k:lower() 
        
                 local v = rawget(self, k) 
        
                 if v then return v end 
        
                 local r, g, b = hex2rgb(k) 
        
                 escseq = string.format("�[38;2;%d;%d;%dm", r, g, b) 
        
               else -- index is neovim hl 
        
                 _, escseq = M.ansi_from_hl(k, "foo") 
        
                 print("esc", k, escseq) 
        
               end 
        
               -- We always set the item, if not RGB and hl isn't valid 
        
               -- create a dummy function that returns the string instead 
        
               local v = type(escseq) == "string" and #escseq > 0 
        
                   and function(s) 
        
                     if type(s) ~= "string" or #s == 0 then return "" end 
        
                     return escseq .. s .. M.ansi_escseq.clear 
        
                   end 
        
                   or function(s) return s end 
        
               rawset(self, k, v) 
        
               return v 
        
             end, 
        
             __newindex = function(self, k, v) 
        
               assert(false, 
        
                 string.format("modifying the ansi cache directly isn't allowed [index: %s]", k)) 
        
               -- rawset doesn't trigger __new_index, otherwise stack overflow 
        
               -- we never get here but this masks the "unused local" warnings 
        
               rawset(self, k, v) 
        
             end 
        
           }) ]]

ibhagwan · 2026-07-08T01:13:32Z

We can also merge this and wait for upstream fixes before we enable make fmtcheck in lint.yaml

phanen · 2026-07-08T02:25:40Z

We can also merge this and wait for upstream fixes before we enable make fmtcheck in lint.yaml

yeah the new formatter currently have less feature than emmylua_codestyle

coderabbitai Bot reviewed Jul 7, 2026

View reviewed changes

Comment thread lua/fzf-lua/providers/tags.lua

Comment thread lua/fzf-lua/test/screenshot.lua Outdated

ibhagwan force-pushed the ci/emmylua_check branch from 8ff0093 to a08fb72 Compare July 7, 2026 17:27

coderabbitai Bot reviewed Jul 7, 2026

View reviewed changes

Comment thread Makefile Outdated

ibhagwan force-pushed the ci/emmylua_check branch 7 times, most recently from 8ae04ff to 895a7ed Compare July 7, 2026 18:35

ibhagwan added 2 commits July 7, 2026 14:56

ci(lint): download emmylua_check with make deps

4e47929

chore(types): emmylua_check lint warnings

d64308f

ibhagwan force-pushed the ci/emmylua_check branch from 895a7ed to d64308f Compare July 7, 2026 18:56

ibhagwan force-pushed the ci/emmylua_check branch from 18b64df to df054af Compare July 7, 2026 22:54

ci: formatting with emmylua luafmt

0847c90

ibhagwan force-pushed the ci/emmylua_check branch from df054af to 0847c90 Compare July 8, 2026 01:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

emmylua_check as main linter#2776

emmylua_check as main linter#2776
ibhagwan wants to merge 3 commits into
mainfrom
ci/emmylua_check

ibhagwan commented Jul 7, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Jul 7, 2026 •

edited

Loading

Reviews paused

Walkthrough

Changes

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

ibhagwan commented Jul 7, 2026

Uh oh!

ibhagwan commented Jul 8, 2026

Uh oh!

ibhagwan commented Jul 8, 2026

Uh oh!

phanen commented Jul 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

ibhagwan commented Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

IDK how coderabbit came out with this summary but it's totally wrong lol

The below is the garbage generated by coderabbitai:

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviews paused

Walkthrough

Changes

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ibhagwan commented Jul 7, 2026

Uh oh!

ibhagwan commented Jul 8, 2026

Uh oh!

ibhagwan commented Jul 8, 2026

Uh oh!

phanen commented Jul 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ibhagwan commented Jul 7, 2026 •

edited

Loading

coderabbitai Bot commented Jul 7, 2026 •

edited

Loading