search

tweag / nickel

Better configuration for less
https://nickel-lang.org/
MIT License
2.23k stars 85 forks source link

Unresolved import causes `nls` to panic #1935

Closed mkatychev closed 1 week ago

mkatychev commented 1 month ago

Describe the bug

[ERROR] .../vim/lsp/rpc.lua:789 "rpc"   "~/.nix-profile/bin/nls"    "stderr"    "thread 'main' panicked at core/src/typecheck/mod.rs:2819:26:Internal error: resolved import not found during typechecking.
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
"

To Reproduce Declare an import in an ncl that has no resolution: let ObjectThatDoesNotExist = import "real_file.ncl in

Expected behavior nls should be fault tolerant to an unresolved dependency tree and turn the panic below into an error or log for NLS: https://github.com/tweag/nickel/blob/c8e840180d6006707afa68c309df4009f0a497a2/core/src/typecheck/mod.rs#L2817-L2819

Environment

Additional context neovim default LSP

yannham commented 1 month ago

Hi, thanks for the report. The issue isn't that the import isn't resolved (which is usually entirely fine for the LSP and the typechecker, if it can't be found), but rather that the import has been resolved at some point but the corresponding source isn't in the source cache anymore.

I wonder how that could have happened, as besides a few exceptions (temporary holders for error messages) we never remove stuff from the source cache; have you done anything special, like having a valid import that you deleted at some point while NLS was active?

I just tried your example, both with the error (you're missing a closing ") and without, and adding stuff to make it a proper program, but I can't trigger the bug - I just get a proper "import not found" error.

image

Would you have a minimal, complete and reproducible example that triggers the bug for you, to help us address the issue?

mkatychev commented 1 month ago

Hi @yannham I will try to produce an example

yannham commented 2 weeks ago

Hello @mkatychev, even without a small example, does this still happen on 1.7 ? We suspect #1944 might have solved the problem. Let us know!

mkatychev commented 2 weeks ago

Thanks for the followup @yannham let me get to it tomorrow. The issue was a bit hard to reproduce but that ticket looks like the right steps to make it happen since I was heavily refactoring imported modules

mkatychev commented 1 week ago

I'm not able to reproduce the issue w/ 1.7, thanks for the fix @jneem!