feat!: remove dep:: prefix

[michaeljklein]

Description

Problem*

Resolves #2307

Summary*

dep:: is unnecessary to distinguish between internal and external crates, so this PR removes it.

Additional Context

Documentation*

Check one:

• No documentation needed.
• Documentation included in this PR.
• [For Experimental Features] Documentation to be submitted in a separate PR.

PR Checklist*

• I have tested the changes locally.
• I have formatted the changes with Prettier and/or cargo fmt on default settings.

[TomAFrench]

Would be nice if we could maintain some backwards compatibility on this as we'd be nuking basically every noir program out there

dep:: is unnecessary to distinguish between internal and external crates, so this PR removes it.

Not sure this is true as we can have a crate with a dependency foo but also a submodule foo. Rust diambiguates these with ::foo to select the dependency rather the submodule (or vice versa). We should have a test for this case in any case.

[github-actions]

🚀 Deployed on https://66437e82ef96833c07eaba82--noir-docs.netlify.app

[michaeljklein]

@TomAFrench I made it backwards compatible and added a deprecation warning when dep:: is used.

R.e. dependency vs. submodule, this test (run from compile_success_empty):

// main.nr
use reexporting_lib::{FooStruct, MyStruct, lib};

mod lib;
use crate::lib::{FooStruct, MyStruct};

fn main() {
    let x: FooStruct = MyStruct { inner: 0 };
    assert(lib::is_struct_zero(x));
}

// lib.nr
struct MyStruct {
    inner: Field
}

type FooStruct = MyStruct;

// Nargo.toml
[package]
name = "dep_submodule_overlap"
type = "bin"
authors = [""]
compiler_version = ">=0.28.0"

[dependencies]
reexporting_lib = { path = "../../test_libraries/reexporting_lib" }

fails with overlapping name errors:

error: Duplicate definitions of import with name lib found
  ┌─ /Users/michaelklein/Coding/rust/noir/test_programs/compile_success_empty/dep_submodule_overlap/src/main.nr:1:44
  │
1 │ use reexporting_lib::{FooStruct, MyStruct, lib};
  │                                            --- Second import found here
2 │ 
3 │ mod lib;
  │     --- First import found here
  │

error: Duplicate definitions of import with name FooStruct found
  ┌─ /Users/michaelklein/Coding/rust/noir/test_programs/compile_success_empty/dep_submodule_overlap/src/main.nr:1:23
  │
1 │ use reexporting_lib::{FooStruct, MyStruct, lib};
  │                       --------- First import found here
  ·
4 │ use crate::lib::{FooStruct, MyStruct};
  │                  --------- Second import found here
  │

error: Duplicate definitions of import with name MyStruct found
  ┌─ /Users/michaelklein/Coding/rust/noir/test_programs/compile_success_empty/dep_submodule_overlap/src/main.nr:1:34
  │
1 │ use reexporting_lib::{FooStruct, MyStruct, lib};
  │                                  -------- First import found here
  ·
4 │ use crate::lib::{FooStruct, MyStruct};
  │                             -------- Second import found here

so we currently have conflicts between dependencies and submodules and could distinguish with ::foo.

new issue here

[github-actions]

FYI @noir-lang/developerrelations on Noir doc changes.

[TomAFrench]

The current behaviour is going to lead to a bunch of footguns/confusing error messages.

[TomAFrench]

We shouldn't automatically retry looking for an external dependency if we can't resolve locally here. Consider a Noir program with both a submodule and a dependency both called foo, we can then write:

// foo/lib.nr

pub fn baz() -> Field {
    6
}

// bin/main.nr

fn main() -> pub Field {
    foo::bar() + foo::baz()
}

mod foo {
    pub(crate) fn bar() -> Field {
        5
    }
}

This compiles completely fine on this branch despite the fact that foo::bar() + foo::baz() is very confusing as we've got two completely different foos here. If I were to define a baz function within the foo submodule I'm then silently changing the call to baz in main.

[TomAFrench]

We should check the first section of the path to see if we can resolve it locally, if so then we should commit to that and raise any errors encountered. Only if we don't see any foo namespace locally should we start searching through dependencies.

[TomAFrench]

This code should fail to compile unless we modified to foo::bar() + ::foo::baz()

[TomAFrench]

R.e. dependency vs. submodule, this test (run from compile_success_empty):

fails with overlapping name errors

This is a different case as we're trying to import the same name into the same namespace twice. The issue we're wanting to solve is two different paths which share a common prefix but we're loading different names from.

[TomAFrench]

Can you add a test to show that this fails to compile without dep::?

[michaeljklein]

done 8a4d138

[TomAFrench]

We should be able to make this a compile_failure test rather than execution_failure test

[michaeljklein]

ok, I'll look into updating the test. as is, skipping the dep:: makes it load everything from the current module and then compilation succeeds.

[TomAFrench]

Huh, strange. I can take a look at this next week as this behaviour isn't what I expect.

[michaeljklein]

the most recent version has foo::bar and dep::foo::bar. two foo::bar's don't conflict, so I've updated the test to conflict on foo, viz. foo::bar vs. foo::baz

[michaeljklein]

(you could have been looking at a previous version that had bar/baz? I've changed it around several times)