.oxlintrc.json not found (because insert_package_json() searchs breadth-first instead of depth-first)

[Rick-Phoenix]

Description

I tried switching to the new oxlint lsp config, but there is an issue.
In my monorepo, I have just one .oxlintrc.json file at the root of the repo. The older config with lspconfig.oxlint.setup() was able to correctly identify the root directory and the server would attach. But with the new version, that is not the case, even if I manually replace the new root_dir function with the older one.

[justinmk]

the new config, unlike the old one, checks for a oxlint field in the json:

nvim-lspconfig/lsp/oxlint.lua

Line 27 in 4643407

local root_markers = util.insert_package_json({ '.oxlintrc.json' }, 'oxlint', fname)

even if I manually replace the new root_dir function with the older one.

always show the exact code sample. wastes time to just describe things vaguely.

if you did this:

root_dir = util.root_pattern('.oxlintrc.json'),

then that isn't valid. root_pattern() returns a function, but that function does not call on_dir() which is required by the function form of root_dir() (as documented).

[Rick-Phoenix]

Ah yeah I must have mixed them up.
What I was doing was this:

  root_dir = require("lspconfig.util").root_pattern ".oxlintrc.json",

I assumed this would be correct since this is what the previous config used and because the name is the same, but now I realize the return signature is different.

The same issue remains though. Because if, like in my case, oxlint is indeed in the local package.json file but the .oxlintrc.json is in an upper path, the callback won't be called and the lsp won't attach.

Personally, I think it would be better to revert to the previous approach of searching directly for a .oxlintrc.json because that's more accurate.

[justinmk]

revert to the previous approach of searching directly for a .oxlintrc.json

that would allow the use of root_markers, then we can drop the root_dir() entirely.

wdyt @maximtrp

[maximtrp]

Actually, I can't reproduce this issue. Whether I remove .oxlintrc.json or keep oxlint in package.json, the Oxlint language server still attaches without any problems.

@Rick-Phoenix Could you please share the output of tree showing the directory structure with package.json and .oxlintrc.json files?

@justinmk Removing root_dir would make it more difficult to detect Oxlint-enabled workspaces that don’t use a .oxlintrc.json file. The thing is, Oxlint doesn’t require a configuration file, it can also be configured via inline comments.

[Rick-Phoenix]

I actually found out that the lsp attachment issue was due to a mismatch of version between my global oxlint package managed by mason and the local one used in the project.

The real issue is rather that if the .oxlintrc.json file is outside of the package's root, it does not get detected correctly.

So this is what the file tree looks like for my project.
If I open monorepo-starter/packages/hono-starter/src/index.ts with the default config, the lsp does attach and the root directory for oxlint is monorepo-starter/packages/hono-starter, but since there is no .oxlintrc.json file there, the diagnostics I get from the oxlint server do not reflect the settings of my .oxlintrc.json file in the root of the repo.

If instead I change the root_dir function to one that looks upwards until it can find a .oxlintrc.json file, the root directory is correctly identified and my settings are reflected in the diagnostic messages.

[Rick-Phoenix]

Like I said, do what the older config did and look for a .oxlintrc.json file rather from the file's directory upwards rarther than an oxlint entry in the package.json file.

what exactly do you mean "correctly indentified"? what is the ws root in that case?

monorepo-starter, which is the folder where the first .oxlintrc.json file is found from that buffer's directory upwards

[justinmk]

I understand now.

the root directory for oxlint is monorepo-starter/packages/hono-starter, but since there is no .oxlintrc.json file there

but looking at the logic of

nvim-lspconfig/lua/lspconfig/util.lua

Line 47 in 4643407

function M.insert_package_json(config_files, field, fname)

, it would only stop at monorepo-starter/packages/hono-starter/ if it found a package.json file with the expected field.

and even then, here '.oxlintrc.json' is the first item in the root markers list:

nvim-lspconfig/lsp/oxlint.lua

Line 27 in 4643407

local root_markers = util.insert_package_json({ '.oxlintrc.json' }, 'oxlint', fname)

so all ancestors should be searched for '.oxlintrc.json' first, before trying other things like package.json.

so i don't see how your problem can occur

[maximtrp]

@justinmk Yeah, exactly. This is what I am struggling to understand now. I have reproduced this case locally. And the LS is attached to the folder where package.json is located even though the config file is the first one in the root_markers table.

The table returned by:

local root_markers = util.insert_package_json({ '.oxlintrc.json' }, 'oxlint', fname)
vim.fs.find(root_markers, { path = fname, upward = true })

contains just the path to the root directory of package.json.

[maximtrp]

@justinmk Could it be that vim.fs.find searches for all files in root_markers asynchronously and selects the first one it finds? Seems like this is the case.

[justinmk]

no, it searches in-order: https://github.com/neovim/neovim/blob/b877aa34cf366982bf92aa8b17a8409a1f8eb134/runtime/lua/vim/fs.lua#L304-L305

[maximtrp]

@justinmk Yeah, thank you, I have just dived in there. It returns the first found directory for one of the root markers (in-order) before going upwards. So, it will never find .oxlintrc.json in a such project structure. Is it a good case to implement an option for vim.fs.find to use another algorithm and try all parents for the first item, then for the second and so on?

[justinmk]

oh good point. that's wrong. it's contrary to the behavior of root_markers, which searchs "depth first": https://github.com/neovim/neovim/blob/b877aa34cf366982bf92aa8b17a8409a1f8eb134/runtime/lua/vim/lsp.lua#L726-L731

so to avoid confusion, we should change the behavior of insert_package_json to match that.