Add support for package field under dependencies. If unspecified,…

… require that dependency name matches package name. (#1002)

* Add `kebab_to_snake_case` function to `forc-util` for dependency names

* Add support for `package` field under `dependencies`

This allows for the dependency name to differ from the dependency's
package name.

Closes #977.

* Add test for forc's dependency `package` field support

* Ensure that pinned package names match their associated manifest

* Update forc lock files of tests that depend on std

It is no longer valid for packages to contain a manifest with a name
that does not match the name of the pinned package within the lock file.
As a result, the lock files of these tests have been invalidated and
regenerated.

* Update context_testing_contract test

* Update caller_context_test

* Fix some minor documenation nits

forc-pkg/src/lock.rs

@@ -33,11 +33,23 @@ pub struct PkgLock {
     // `Option`.
     version: Option<semver::Version>,
     source: Option<String>,
-    // Dependency string is "<name> <source_string>". The source string is included in order to be
-    // able to uniquely distinguish between multiple different versions of the same package.
-    dependencies: Vec<String>,
+    dependencies: Vec<PkgDepLine>,
 }
 
+/// `PkgDepLine` is a terse, single-line, git-diff-friendly description of a package's
+/// dependency. It is formatted like so:
+///
+/// ```ignore
+/// (<dep_name>) <pkg_name> <source_string>
+/// ```
+///
+/// The `(<dep_name>)` segment is only included in the uncommon case that the dependency name does
+/// not match the package name, i.e. if the `package` field was specified for the dependency.
+///
+/// The source string is included in order to be able to uniquely distinguish between multiple
+/// different versions of the same package.
+pub type PkgDepLine = String;
+
 /// Convert the given package source to a string for use in the package lock.
 ///
 /// Returns `None` for sources that refer to a direct `Path`.
@@ -74,10 +86,16 @@ impl PkgLock {
         let mut dependencies: Vec<String> = graph
             .edges_directed(node, Direction::Outgoing)
             .map(|edge| {
+                let dep_name = edge.weight();
                 let dep_node = edge.target();
-                let dep = &graph[dep_node];
-                let source_string = source_to_string(&dep.source);
-                pkg_unique_string(&dep.name, source_string.as_deref())
+                let dep_pkg = &graph[dep_node];
+                let dep_name = if *dep_name != dep_pkg.name {
+                    Some(&dep_name[..])
+                } else {
+                    None
+                };
+                let source_string = source_to_string(&dep_pkg.source);
+                pkg_dep_line(dep_name, &dep_pkg.name, source_string.as_deref())
             })
             .collect();
         dependencies.sort();
@@ -123,9 +141,6 @@ impl Lock {
         for pkg in &self.package {
             let key = pkg.unique_string();
             let name = pkg.name.clone();
-            // TODO: We shouldn't use `pkg::SourcePinned` as we don't actually know the `Path`
-            // until we follow the dependency graph. Use something like a `ParsedSource` type here
-            // instead.
             let pkg_source_string = pkg.source.clone();
             let source = match &pkg_source_string {
                 None => pkg::SourcePinned::Path,
@@ -142,12 +157,15 @@ impl Lock {
         for pkg in &self.package {
             let key = pkg.unique_string();
             let node = pkg_to_node[&key];
-            for dep_key in &pkg.dependencies {
+            for dep_line in &pkg.dependencies {
+                let (dep_name, dep_key) = parse_pkg_dep_line(dep_line)
+                    .map_err(|e| anyhow!("failed to parse dependency \"{}\": {}", dep_line, e))?;
                 let dep_node = pkg_to_node
-                    .get(&dep_key[..])
+                    .get(dep_key)
                     .cloned()
                     .ok_or_else(|| anyhow!("found dep {} without node entry in graph", dep_key))?;
-                graph.add_edge(node, dep_node, ());
+                let dep_name = dep_name.unwrap_or(&graph[dep_node].name).to_string();
+                graph.add_edge(node, dep_node, dep_name);
             }
         }
 
@@ -171,6 +189,38 @@ fn pkg_unique_string(name: &str, source: Option<&str>) -> String {
     }
 }
 
+fn pkg_dep_line(dep_name: Option<&str>, name: &str, source: Option<&str>) -> PkgDepLine {
+    let pkg_string = pkg_unique_string(name, source);
+    match dep_name {
+        None => pkg_string,
+        Some(dep_name) => format!("({}) {}", dep_name, pkg_string),
+    }
+}
+
+// Parse the given `PkgDepLine` into its dependency name and unique string segments.
+//
+// I.e. given "(<dep_name>) <name> <source>", returns ("<dep_name>", "<name> <source>").
+fn parse_pkg_dep_line(pkg_dep_line: &str) -> anyhow::Result<(Option<&str>, &str)> {
+    let s = pkg_dep_line.trim();
+
+    // Check for the open bracket.
+    if !s.starts_with('(') {
+        return Ok((None, s));
+    }
+
+    // If we have the open bracket, grab everything until the closing bracket.
+    let s = &s["(".len()..];
+    let mut iter = s.split(')');
+    let dep_name = iter
+        .next()
+        .ok_or_else(|| anyhow!("missing closing parenthesis"))?;
+
+    // The rest is the unique package string.
+    let s = &s[dep_name.len() + ")".len()..];
+    let pkg_str = s.trim_start();
+    Ok((Some(dep_name), pkg_str))
+}
+
 pub fn print_diff(proj_name: &str, diff: &Diff) {
     print_removed_pkgs(proj_name, diff.removed.iter().cloned());
     print_added_pkgs(proj_name, diff.added.iter().cloned());

forc-pkg/src/manifest.rs

@@ -56,6 +56,17 @@ pub struct DependencyDetails {
     pub(crate) git: Option<String>,
     pub(crate) branch: Option<String>,
     pub(crate) tag: Option<String>,
+    pub(crate) package: Option<String>,
+}
+
+impl Dependency {
+    /// The string of the `package` field if specified.
+    pub fn package(&self) -> Option<&str> {
+        match *self {
+            Self::Simple(_) => None,
+            Self::Detailed(ref det) => det.package.as_deref(),
+        }
+    }
 }
 
 impl Manifest {

forc-pkg/src/pkg.rs

@@ -4,8 +4,8 @@ use crate::{
 };
 use anyhow::{anyhow, bail, Result};
 use forc_util::{
-    find_file_name, git_checkouts_directory, print_on_failure, print_on_success,
-    print_on_success_library, println_yellow_err,
+    find_file_name, git_checkouts_directory, kebab_to_snake_case, print_on_failure,
+    print_on_success, print_on_success_library, println_yellow_err,
 };
 use petgraph::{self, visit::EdgeRef, Directed, Direction};
 use serde::{Deserialize, Serialize};
@@ -24,7 +24,7 @@ use url::Url;
 
 type GraphIx = u32;
 type Node = Pinned;
-type Edge = ();
+type Edge = DependencyName;
 pub type Graph = petgraph::Graph<Node, Edge, Directed, GraphIx>;
 pub type NodeIx = petgraph::graph::NodeIndex<GraphIx>;
 pub type PathMap = HashMap<PinnedId, PathBuf>;
@@ -44,7 +44,7 @@ pub struct Compiled {
 /// A package uniquely identified by name along with its source.
 #[derive(Clone, Debug, Eq, Hash, Ord, PartialEq, PartialOrd, Deserialize, Serialize)]
 pub struct Pkg {
-    /// The unique name of the package.
+    /// The unique name of the package as declared in its manifest.
     pub name: String,
     /// Where the package is sourced from.
     pub source: Source,
@@ -145,6 +145,26 @@ pub enum SourceGitPinnedParseError {
     CommitHash,
 }
 
+/// The name specified on the left hand side of the `=` in a depenedency declaration under
+/// `[dependencies]` within a forc manifest.
+///
+/// The name of a dependency may differ from the package name in the case that the dependency's
+/// `package` field is specified.
+///
+/// For example, in the following, `foo` is assumed to be both the package name and the dependency
+/// name:
+///
+/// ```toml
+/// foo = { git = "https://github.com/owner/repo", branch = "master" }
+/// ```
+///
+/// In the following case however, `foo` is the package name, but the dependency name is `foo-alt`:
+///
+/// ```toml
+/// foo-alt = { git = "https://github.com/owner/repo", branch = "master", package = "foo" }
+/// ```
+pub type DependencyName = String;
+
 impl BuildPlan {
     /// Create a new build plan for the project by fetching and pinning dependenies.
     pub fn new(manifest_dir: &Path, offline: bool) -> Result<Self> {
@@ -189,7 +209,11 @@ impl BuildPlan {
         let plan_dep_pkgs: BTreeSet<_> = self
             .graph
             .edges_directed(proj_node, Direction::Outgoing)
-            .map(|e| self.graph[e.target()].unpinned(&self.path_map))
+            .map(|e| {
+                let dep_name = e.weight();
+                let dep_pkg = self.graph[e.target()].unpinned(&self.path_map);
+                (dep_name, dep_pkg)
+            })
             .collect();
 
         // Collect dependency `Source`s from manifest.
@@ -200,22 +224,23 @@ impl BuildPlan {
             .as_ref()
             .into_iter()
             .flat_map(|deps| deps.iter())
-            .map(|(name, dep)| {
+            .map(|(dep_name, dep)| {
                 // NOTE: Temporarily warn about `version` until we have support for registries.
                 if let Dependency::Detailed(det) = dep {
                     if det.version.is_some() {
                         println_yellow_err(&format!(
                             "  WARNING! Dependency \"{}\" specifies the unused `version` field: \
                             consider using `branch` or `tag` instead",
-                            name
+                            dep_name
                         ))
                         .unwrap();
                     }
                 }
 
-                let name = name.clone();
+                let name = dep.package().unwrap_or(dep_name).to_string();
                 let source = dep_to_source(proj_path, dep)?;
-                Ok(Pkg { name, source })
+                let dep_pkg = Pkg { name, source };
+                Ok((dep_name, dep_pkg))
             })
             .collect::<Result<BTreeSet<_>>>()?;
 
@@ -224,6 +249,21 @@ impl BuildPlan {
             bail!("Manifest dependencies do not match");
         }
 
+        // Ensure the pkg names of all nodes match their associated manifests.
+        for node in self.graph.node_indices() {
+            let pkg = &self.graph[node];
+            let id = pkg.id();
+            let path = &self.path_map[&id];
+            let manifest = Manifest::from_dir(path)?;
+            if pkg.name != manifest.project.name {
+                bail!(
+                    "package name {:?} does not match the associated manifest project name {:?}",
+                    pkg.name,
+                    manifest.project.name,
+                );
+            }
+        }
+
         Ok(())
     }
 
@@ -448,10 +488,12 @@ pub(crate) fn fetch_deps(
     // TODO: Convert this recursion to use loop & stack to ensure deps can't cause stack overflow.
     let fetch_ts = std::time::Instant::now();
     let fetch_id = fetch_id(&path_map[&pkg_id], fetch_ts);
+    let manifest = Manifest::from_dir(&path_map[&pkg_id])?;
     fetch_children(
         fetch_id,
         offline_mode,
         root,
+        &manifest,
         &mut graph,
         &mut path_map,
         &mut visited,
@@ -475,34 +517,49 @@ fn fetch_children(
     fetch_id: u64,
     offline_mode: bool,
     node: NodeIx,
+    manifest: &Manifest,
     graph: &mut Graph,
     path_map: &mut PathMap,
     visited: &mut HashMap<Pinned, NodeIx>,
 ) -> Result<()> {
     let parent = &graph[node];
     let parent_path = path_map[&parent.id()].clone();
-    let manifest = Manifest::from_dir(&parent_path)?;
-    let deps = match &manifest.dependencies {
-        None => return Ok(()),
-        Some(deps) => deps,
-    };
-    for (name, dep) in deps {
-        let name = name.clone();
+    for (dep_name, dep) in manifest.deps() {
+        let name = dep.package().unwrap_or(dep_name).to_string();
         let source = dep_to_source(&parent_path, dep)?;
         if offline_mode && !matches!(source, Source::Path(_)) {
             bail!("Unable to fetch pkg {:?} in offline mode", source);
         }
         let pkg = Pkg { name, source };
         let pinned = pin_pkg(fetch_id, &pkg, path_map)?;
+        let pkg_id = pinned.id();
+        let manifest = Manifest::from_dir(&path_map[&pkg_id])?;
+        if pinned.name != manifest.project.name {
+            bail!(
+                "dependency name {:?} must match the manifest project name {:?} \
+                unless `package = {:?}` is specified in the dependency declaration",
+                pinned.name,
+                manifest.project.name,
+                manifest.project.name,
+            );
+        }
         let dep_node = if let hash_map::Entry::Vacant(entry) = visited.entry(pinned.clone()) {
             let node = graph.add_node(pinned);
             entry.insert(node);
-            fetch_children(fetch_id, offline_mode, node, graph, path_map, visited)?;
+            fetch_children(
+                fetch_id,
+                offline_mode,
+                node,
+                &manifest,
+                graph,
+                path_map,
+                visited,
+            )?;
             node
         } else {
             visited[&pinned]
         };
-        graph.add_edge(node, dep_node, ());
+        graph.add_edge(node, dep_node, dep_name.to_string());
     }
     Ok(())
 }
@@ -764,11 +821,20 @@ pub fn dependency_namespace(
 
     // In order of compilation, accumulate dependency namespace refs.
     let namespace = sway_core::create_module();
-    for dep_node in compilation_order.iter().filter(|n| deps.contains(n)) {
-        if *dep_node == node {
+    for &dep_node in compilation_order.iter().filter(|n| deps.contains(n)) {
+        if dep_node == node {
             break;
         }
-        namespace.insert_module_ref(graph[*dep_node].name.clone(), namespace_map[dep_node]);
+        // Add the namespace once for each of its names.
+        let namespace_ref = namespace_map[&dep_node];
+        let dep_names: BTreeSet<_> = graph
+            .edges_directed(dep_node, Direction::Incoming)
+            .map(|e| e.weight())
+            .collect();
+        for dep_name in dep_names {
+            let dep_name = kebab_to_snake_case(dep_name);
+            namespace.insert_module_ref(dep_name.to_string(), namespace_ref);
+        }
     }
 
     namespace

forc-util/src/lib.rs

@@ -106,6 +106,11 @@ pub fn validate_name(name: &str, use_case: &str) -> Result<()> {
     Ok(())
 }
 
+/// Simple function to convert kebab-case to snake_case.
+pub fn kebab_to_snake_case(s: &str) -> String {
+    s.replace('-', "_")
+}
+
 pub fn default_output_directory(manifest_dir: &Path) -> PathBuf {
     manifest_dir.join(DEFAULT_OUTPUT_DIRECTORY)
 }

test/src/e2e_vm_tests/mod.rs

@@ -12,6 +12,10 @@ pub fn run(filter_regex: Option<regex::Regex>) {
     // programs that should successfully compile and terminate
     // with some known state
     let positive_project_names = vec![
+        (
+            "should_pass/forc/dependency_package_field",
+            ProgramState::Return(0),
+        ),
         (
             "should_pass/language/asm_expr_basic",
             ProgramState::Return(6),

test/src/e2e_vm_tests/test_programs/should_fail/match_expressions_non_exhaustive/Forc.lock

@@ -1,16 +1,16 @@
 [[package]]
 name = 'core'
-source = 'git+http://github.com/FuelLabs/sway-lib-core?reference=master#c331ed20ebc9d646acec6b8ee8f408627ce3b573'
+source = 'git+https://github.com/FuelLabs/sway-lib-core?reference=master#30274cf817c1848e28f984c2e8703eb25e7a3a44'
 dependencies = []
 
 [[package]]
 name = 'match_expressions_non_exhaustive'
 dependencies = [
-    'core git+http://github.com/FuelLabs/sway-lib-core?reference=master#c331ed20ebc9d646acec6b8ee8f408627ce3b573',
-    'std git+http://github.com/FuelLabs/sway-lib-std?reference=master#7081d2e1ac2401f22a0de0673e8a01535180ba3a',
+    'core git+https://github.com/FuelLabs/sway-lib-core?reference=master#30274cf817c1848e28f984c2e8703eb25e7a3a44',
+    'std git+https://github.com/FuelLabs/sway-lib-std?reference=master#7b973a638d5220228be616f1f89a249846001549',
 ]
 
 [[package]]
 name = 'std'
-source = 'git+http://github.com/FuelLabs/sway-lib-std?reference=master#7081d2e1ac2401f22a0de0673e8a01535180ba3a'
-dependencies = ['core git+http://github.com/FuelLabs/sway-lib-core?reference=master#c331ed20ebc9d646acec6b8ee8f408627ce3b573']
+source = 'git+https://github.com/FuelLabs/sway-lib-std?reference=master#7b973a638d5220228be616f1f89a249846001549'
+dependencies = ['core git+https://github.com/FuelLabs/sway-lib-core?reference=master#30274cf817c1848e28f984c2e8703eb25e7a3a44']