fix: checks for cyclic dependencies (#3699)

# Description

## Problem\*

Resolves #2295 

## Summary\*

Keep track of the processed packages and check against this list before
processing a new one.

## Additional Context



## Documentation\*

Check one:
- [X] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[Exceptional Case]** Documentation to be submitted in a separate
PR.

# PR Checklist\*

- [X] I have tested the changes locally.
- [X] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.

---------

Co-authored-by: kevaundray <[email protected]>

test_programs/compile_failure/cyclic_dep/Nargo.toml

@@ -0,0 +1,7 @@
+[package]
+name = "cyclic_dep"
+type = "bin"
+authors = [""]
+
+[dependencies]
+dep1 = { path= "./dep1"}

test_programs/compile_failure/cyclic_dep/dep1/Nargo.toml

@@ -0,0 +1,7 @@
+[package]
+name = "dep1"
+type = "lib"
+authors = [""]
+
+[dependencies]
+dep1 = { path= "../dep2"}

test_programs/compile_failure/cyclic_dep/dep1/src/lib.nr

@@ -0,0 +1,3 @@
+fn bar() {
+
+}

test_programs/compile_failure/cyclic_dep/dep2/Nargo.toml

@@ -0,0 +1,7 @@
+[package]
+name = "dep2"
+type = "lib"
+authors = [""]
+
+[dependencies]
+dep1 = { path= "../dep1"}

test_programs/compile_failure/cyclic_dep/dep2/src/lib.nr

@@ -0,0 +1,3 @@
+fn foo() {
+
+}

test_programs/compile_failure/cyclic_dep/src/main.nr

@@ -0,0 +1,7 @@
+use dep1::foo;
+use dep2::bar;
+
+fn main() {
+    dep1::foo();
+    dep2::bar();
+}

tooling/nargo_toml/src/errors.rs

@@ -69,6 +69,9 @@ pub enum ManifestError {
 
     #[error(transparent)]
     SemverError(SemverError),
+
+    #[error("Cyclic package dependency found when processing {cycle}")]
+    CyclicDependency { cycle: String },
 }
 
 #[allow(clippy::enum_variant_names)]

tooling/nargo_toml/src/lib.rs

@@ -120,7 +120,11 @@ struct PackageConfig {
 }
 
 impl PackageConfig {
-    fn resolve_to_package(&self, root_dir: &Path) -> Result<Package, ManifestError> {
+    fn resolve_to_package(
+        &self,
+        root_dir: &Path,
+        processed: &mut Vec<String>,
+    ) -> Result<Package, ManifestError> {
         let name: CrateName = if let Some(name) = &self.package.name {
             name.parse().map_err(|_| ManifestError::InvalidPackageName {
                 toml: root_dir.join("Nargo.toml"),
@@ -136,7 +140,7 @@ impl PackageConfig {
                 toml: root_dir.join("Nargo.toml"),
                 name: name.into(),
             })?;
-            let resolved_dep = dep_config.resolve_to_dependency(root_dir)?;
+            let resolved_dep = dep_config.resolve_to_dependency(root_dir, processed)?;
 
             dependencies.insert(name, resolved_dep);
         }
@@ -283,7 +287,11 @@ enum DependencyConfig {
 }
 
 impl DependencyConfig {
-    fn resolve_to_dependency(&self, pkg_root: &Path) -> Result<Dependency, ManifestError> {
+    fn resolve_to_dependency(
+        &self,
+        pkg_root: &Path,
+        processed: &mut Vec<String>,
+    ) -> Result<Dependency, ManifestError> {
         let dep = match self {
             Self::Github { git, tag, directory } => {
                 let dir_path = clone_git_repo(git, tag).map_err(ManifestError::GitError)?;
@@ -300,13 +308,13 @@ impl DependencyConfig {
                     dir_path
                 };
                 let toml_path = project_path.join("Nargo.toml");
-                let package = resolve_package_from_toml(&toml_path)?;
+                let package = resolve_package_from_toml(&toml_path, processed)?;
                 Dependency::Remote { package }
             }
             Self::Path { path } => {
                 let dir_path = pkg_root.join(path);
                 let toml_path = dir_path.join("Nargo.toml");
-                let package = resolve_package_from_toml(&toml_path)?;
+                let package = resolve_package_from_toml(&toml_path, processed)?;
                 Dependency::Local { package }
             }
         };
@@ -325,9 +333,10 @@ fn toml_to_workspace(
     nargo_toml: NargoToml,
     package_selection: PackageSelection,
 ) -> Result<Workspace, ManifestError> {
+    let mut resolved = Vec::new();
     let workspace = match nargo_toml.config {
         Config::Package { package_config } => {
-            let member = package_config.resolve_to_package(&nargo_toml.root_dir)?;
+            let member = package_config.resolve_to_package(&nargo_toml.root_dir, &mut resolved)?;
             match &package_selection {
                 PackageSelection::Selected(selected_name) if selected_name != &member.name => {
                     return Err(ManifestError::MissingSelectedPackage(member.name))
@@ -345,7 +354,7 @@ fn toml_to_workspace(
             for (index, member_path) in workspace_config.members.into_iter().enumerate() {
                 let package_root_dir = nargo_toml.root_dir.join(&member_path);
                 let package_toml_path = package_root_dir.join("Nargo.toml");
-                let member = resolve_package_from_toml(&package_toml_path)?;
+                let member = resolve_package_from_toml(&package_toml_path, &mut resolved)?;
 
                 match &package_selection {
                     PackageSelection::Selected(selected_name) => {
@@ -402,17 +411,43 @@ fn read_toml(toml_path: &Path) -> Result<NargoToml, ManifestError> {
 }
 
 /// Resolves a Nargo.toml file into a `Package` struct as defined by our `nargo` core.
-fn resolve_package_from_toml(toml_path: &Path) -> Result<Package, ManifestError> {
+fn resolve_package_from_toml(
+    toml_path: &Path,
+    processed: &mut Vec<String>,
+) -> Result<Package, ManifestError> {
+    // Checks for cyclic dependencies
+    let str_path = toml_path.to_str().expect("ICE - path is empty");
+    if processed.contains(&str_path.to_string()) {
+        let mut cycle = false;
+        let mut message = String::new();
+        for toml in processed {
+            cycle = cycle || toml == str_path;
+            if cycle {
+                message += &format!("{} referencing ", toml);
+            }
+        }
+        message += str_path;
+        return Err(ManifestError::CyclicDependency { cycle: message });
+    }
+    // Adds the package to the set of resolved packages
+    if let Some(str) = toml_path.to_str() {
+        processed.push(str.to_string());
+    }
+
     let nargo_toml = read_toml(toml_path)?;
 
-    match nargo_toml.config {
+    let result = match nargo_toml.config {
         Config::Package { package_config } => {
-            package_config.resolve_to_package(&nargo_toml.root_dir)
+            package_config.resolve_to_package(&nargo_toml.root_dir, processed)
         }
         Config::Workspace { .. } => {
             Err(ManifestError::UnexpectedWorkspace(toml_path.to_path_buf()))
         }
-    }
+    };
+    let pos =
+        processed.iter().position(|toml| toml == str_path).expect("added package must be here");
+    processed.remove(pos);
+    result
 }
 
 #[derive(Debug, PartialEq, Eq)]