Module declaration conflicts?

[melMass]

Describe the bug

This is a "bug" I've been experiencing for quite some time, after helping someone over discord on a similar matter I decided to try and reproduce it in the smallest way possible and this actually quickly revealed the issue which might not be a bug but something the compiler/LSP could probably catch early on: cyclic imports

How to reproduce

Extract the following archive, cd to the folder and run nu --config config.nu
reproduction_module_conflicts.zip

You will get this error:

Error: nu::parser::missing_positional

  × Missing required positional argument.
   ╭─[C:\Users\User\dev\melMass\repro_modules\jedi.nu:5:1]
 5 │ export def "jedi fight" [] {
 6 │     {laser:"blue"} | merge {HP: 100}
   ·                                     ▲
   ·                                     ╰── missing a
 7 │ }
   ╰────
  help: Usage: merge <base> <a> <b> . Use `--help` for more information.

The merge declared in padawan.nu shadows the builtin one for some reason (I'm guessing it has something to do with cyclic imports)

Expected behavior

Not sure to be honnest, I'm using a simple technique to overcome that: include the namespace in the def declaration, so instead of export def merge [] {} I do export def "padawan merge" [] {}, this works and I personally don't mind, I use a few "private" modules so I'm bound to having some level of cyclism.

Screenshots

No response

Configuration

key	value
version	0.89.1
branch	main
commit_hash	f16ac88
build_os	windows-x86_64
build_target	x86_64-pc-windows-msvc
rust_version	rustc 1.73.0 (cc66ad468 2023-10-03)
rust_channel	1.73.0-x86_64-pc-windows-msvc
cargo_version	cargo 1.73.0 (9c4383fb5 2023-08-26)
build_time	2024-01-31 15:54:48 +01:00
build_rust_channel	release
allocator	mimalloc
features	dataframe, default, extra, sqlite, trash, which, zip
installed_plugins	notify, query, query json, query web, query xml

Additional context

No response

[kubouch]

Can you please paste the file contents into the issue? It's easier than downloading and unzipping the archive.

If you define a command that overrides a builtin, save the builtin as an alias with a different name before the definition, e.g., alias old-merge = merge; export def merge ....

[melMass]

The goal is not to replace merge.
The fact that padawan's merge is in scope is the bug as per the imports it shouldn't

Can you please paste the file contents into the issue? It's easier than downloading and unzipping the archive.

I don't see how but sure:

config.nu

use padawan.nu
use jedi.nu *

jedi.nu

export def main [] {
    print (help main)
}

export def "jedi fight" [] {
    {laser:"blue"} | merge {HP: 100}
}

export def "jedi health" [] {
    $in | get HP
}

padawan.nu

use jedi.nu ["jedi health"]

export def main [] {
   print (help main)
}

export def merge [base, a,b] {
    print $"($base)($a)($b)"
}

[kubouch]

I don't see how but sure

So that anyone reading through the issue can immediately see what the problem is without downloading any files.

This is definitely a parser bug. After use padawan.nu, you have commands padawan and padawan merge, so I don't understand why it's overwriting merge in jedi.nu. If you try replacing the jedi fight body with help merge, you'll see both builtin and padawan's merge listed. This is also weird because we removed command overloading, and you shouldn't be able to define two commands with the same name. Very strange...

[kubouch]

It also doesn't look like a cyclic import. We have a parser guard against cyclic import, and this one doesn't cyclically import anything.

But I might actually have a clue what's happening. When parsing padawan.nu, the parser first scans predeclarations, so it registers merge before continuing the parse. So, when parsing use jedi.nu ["jedi health"], there is already merge being present in the parser and thus seen inside jedi.nu.

So technically it works as intended, but for sure it's very confusing. I think modules should be more self-contained and not inherit too much from the parent scope because it can lead to weird situations like this.