Merge pull request #546 from gauge-sh/optional-depends-on

Make `depends_on` optional

src/commands/check_internal/checks.rs

@@ -23,15 +23,19 @@ fn check_dependencies(
         LayerCheckResult::LayerViolation(e) | LayerCheckResult::UnknownLayer(e) => return Err(e),
     };
 
+    if file_module_config.depends_on.is_none() {
+        return Ok(());
+    }
+
     if import_module_config.utility {
         return Ok(());
     }
+
     let file_nearest_module_path = &file_module_config.path;
     let import_nearest_module_path = &import_module_config.path;
 
     match file_module_config
-        .depends_on
-        .iter()
+        .dependencies_iter()
         .find(|dep| &dep.path == import_nearest_module_path)
     {
         Some(DependencyConfig {
@@ -368,4 +372,41 @@ mod tests {
         let result = check_layers(&layers, &source_config, &target_config);
         assert!(matches!(result, LayerCheckResult::LayerNotSpecified));
     }
+
+    #[rstest]
+    #[case("unrestricted", "domain_one", false)] // middle->top denied due to layer check
+    #[case("unrestricted", "service_one", true)] // same layer allowed
+    #[case("unrestricted", "data_one", true)] // middle->bottom allowed
+    fn test_check_import_unrestricted_dependencies(
+        module_tree: ModuleTree,
+        module_config: Vec<ModuleConfig>,
+        interface_config: Vec<InterfaceConfig>,
+        layers: Vec<String>,
+        #[case] file_mod_path: &str,
+        #[case] import_mod_path: &str,
+        #[case] expected_result: bool,
+    ) {
+        let file_module = module_tree.find_nearest(file_mod_path).unwrap();
+        let interface_checker = Some(
+            InterfaceChecker::new(&interface_config)
+                .with_type_check_cache(&module_config, &[PathBuf::from(".")])
+                .unwrap(),
+        );
+
+        let check_error = check_import(
+            import_mod_path,
+            &module_tree,
+            file_module.clone(),
+            &layers,
+            RootModuleTreatment::Allow,
+            &interface_checker,
+            true,
+        );
+        let result = check_error.is_ok();
+        assert_eq!(
+            result, expected_result,
+            "Expected import from '{}' to '{}' to be {}, but got {}",
+            file_mod_path, import_mod_path, expected_result, result
+        );
+    }
 }

src/commands/sync.rs

@@ -88,7 +88,7 @@ pub fn sync_dependency_constraints(
                 new_modules.push(module.with_no_dependencies());
             }
             // Track deprecations for each module
-            for dependency in module.depends_on.iter() {
+            for dependency in module.dependencies_iter() {
                 if dependency.deprecated {
                     deprecation_map
                         .entry(module.path.clone())

src/commands/test.rs

@@ -104,7 +104,7 @@ impl TachPytestPluginHandler {
 fn build_module_consumer_map(modules: &Vec<ModuleConfig>) -> HashMap<&String, Vec<String>> {
     let mut consumer_map: HashMap<&String, Vec<String>> = HashMap::new();
     for module in modules {
-        for dependency in &module.depends_on {
+        for dependency in module.dependencies_iter() {
             consumer_map
                 .entry(&dependency.path)
                 .or_default()

src/config/modules.rs

@@ -124,7 +124,7 @@ pub struct ModuleConfig {
     pub path: String,
     #[serde(default)]
     #[pyo3(set)]
-    pub depends_on: Vec<DependencyConfig>,
+    pub depends_on: Option<Vec<DependencyConfig>>,
     #[serde(default)]
     pub layer: Option<String>,
     #[serde(
@@ -152,7 +152,7 @@ impl Default for ModuleConfig {
     fn default() -> Self {
         Self {
             path: Default::default(),
-            depends_on: Default::default(),
+            depends_on: Some(vec![]),
             layer: Default::default(),
             visibility: default_visibility(),
             utility: Default::default(),
@@ -165,9 +165,10 @@ impl Default for ModuleConfig {
 
 impl ModuleConfig {
     pub fn new_with_layer(path: &str, layer: &str) -> Self {
+        // shorthand for test fixtures
         Self {
             path: path.to_string(),
-            depends_on: vec![],
+            depends_on: Some(vec![]),
             layer: Some(layer.to_string()),
             visibility: default_visibility(),
             utility: false,
@@ -176,6 +177,13 @@ impl ModuleConfig {
             group_id: None,
         }
     }
+
+    pub fn dependencies_iter(&self) -> impl Iterator<Item = &DependencyConfig> {
+        self.depends_on
+            .as_ref()
+            .into_iter()
+            .flat_map(|deps| deps.iter())
+    }
 }
 
 #[pymethods]
@@ -184,7 +192,7 @@ impl ModuleConfig {
     pub fn new(path: &str, strict: bool) -> Self {
         Self {
             path: path.to_string(),
-            depends_on: vec![],
+            depends_on: Some(vec![]),
             layer: None,
             visibility: default_visibility(),
             utility: false,
@@ -196,7 +204,7 @@ impl ModuleConfig {
 
     pub fn with_no_dependencies(&self) -> Self {
         let mut new_module = self.clone();
-        new_module.depends_on = vec![];
+        new_module.depends_on = Some(vec![]);
         new_module
     }
 
@@ -216,8 +224,7 @@ impl ModuleConfig {
 struct BulkModule {
     paths: Vec<String>,
     #[serde(default)]
-    #[serde(skip_serializing_if = "Vec::is_empty")]
-    depends_on: Vec<DependencyConfig>,
+    depends_on: Option<Vec<DependencyConfig>>,
     #[serde(default)]
     layer: Option<String>,
     #[serde(
@@ -242,7 +249,7 @@ impl TryFrom<&[&ModuleConfig]> for BulkModule {
         let first = modules[0];
         let mut bulk = BulkModule {
             paths: modules.iter().map(|m| m.path.clone()).collect(),
-            depends_on: Vec::new(),
+            depends_on: None,
             layer: first.layer.clone(),
             visibility: first.visibility.clone(),
             utility: first.utility,
@@ -251,7 +258,9 @@ impl TryFrom<&[&ModuleConfig]> for BulkModule {
 
         let mut unique_deps: HashSet<DependencyConfig> = HashSet::new();
         for module in modules {
-            unique_deps.extend(module.depends_on.clone());
+            if let Some(depends_on) = module.depends_on.clone() {
+                unique_deps.extend(depends_on);
+            }
 
             // Validate that other fields match the first module
             if module.layer != first.layer {
@@ -286,7 +295,9 @@ impl TryFrom<&[&ModuleConfig]> for BulkModule {
             }
         }
 
-        bulk.depends_on = unique_deps.into_iter().collect();
+        if !unique_deps.is_empty() {
+            bulk.depends_on = Some(unique_deps.into_iter().collect());
+        }
         Ok(bulk)
     }
 }

src/config/project.rs

@@ -100,7 +100,7 @@ impl ProjectConfig {
         self.modules
             .iter()
             .find(|mod_config| mod_config.path == module)
-            .map(|mod_config| &mod_config.depends_on)
+            .map(|mod_config| mod_config.depends_on.as_ref())?
     }
     pub fn prepend_roots(&self, project_root: &Path) -> Vec<PathBuf> {
         // don't prepend if root is "."
@@ -178,9 +178,9 @@ impl ProjectConfig {
 
         for new_module_path in &new_module_paths {
             if let Some(mut original_module) = original_modules_by_path.remove(new_module_path) {
-                original_module
-                    .depends_on
-                    .retain(|dep| new_module_paths.contains(&dep.path));
+                if let Some(deps) = original_module.depends_on.as_mut() {
+                    deps.retain(|dep| new_module_paths.contains(&dep.path))
+                }
                 new_modules.push(original_module);
             } else {
                 new_modules.push(ModuleConfig {
@@ -205,17 +205,18 @@ impl ProjectConfig {
             .iter_mut()
             .find(|mod_config| mod_config.path == module)
         {
-            if !module_config
-                .depends_on
-                .iter()
-                .any(|dep| dep.path == dependency.path)
-            {
-                module_config.depends_on.push(dependency);
+            match &mut module_config.depends_on {
+                Some(depends_on) => {
+                    if !depends_on.iter().any(|dep| dep.path == dependency.path) {
+                        depends_on.push(dependency);
+                    }
+                }
+                None => module_config.depends_on = Some(vec![dependency]),
             }
         } else {
             self.modules.push(ModuleConfig {
                 path: module.to_string(),
-                depends_on: vec![dependency],
+                depends_on: Some(vec![dependency]),
                 ..Default::default()
             });
         }
@@ -230,7 +231,7 @@ impl ProjectConfig {
             if !own_module_paths.contains(&module_config.path) {
                 all_unused_dependencies.push(UnusedDependencies {
                     path: module_config.path.clone(),
-                    dependencies: module_config.depends_on.clone(),
+                    dependencies: module_config.depends_on.clone().unwrap_or_default(),
                 });
                 continue;
             }
@@ -241,8 +242,7 @@ impl ProjectConfig {
                 .unwrap_or_default();
 
             let current_dependency_paths: HashSet<&String> = module_config
-                .depends_on
-                .iter()
+                .dependencies_iter()
                 .map(|dep| &dep.path)
                 .collect();
 
@@ -252,8 +252,7 @@ impl ProjectConfig {
 
             if !extra_dependency_paths.is_empty() {
                 let extra_dependencies: Vec<DependencyConfig> = module_config
-                    .depends_on
-                    .iter()
+                    .dependencies_iter()
                     .filter(|dep| extra_dependency_paths.contains(&&dep.path))
                     .cloned()
                     .collect();

src/modules/parsing.rs

@@ -61,7 +61,7 @@ pub fn find_visibility_violations(modules: &[ModuleConfig]) -> Vec<VisibilityErr
 
     let mut results: Vec<VisibilityErrorInfo> = Vec::new();
     for module in modules.iter() {
-        for dependency_config in module.depends_on.iter() {
+        for dependency_config in module.dependencies_iter() {
             if let Some(visibility) = visibility_by_path.get(&dependency_config.path) {
                 // check if visibility of this dependency doesn't match the current module
                 if !visibility.iter().any(|visibility_pattern| {
@@ -90,7 +90,7 @@ pub fn find_modules_with_cycles(modules: &[ModuleConfig]) -> Vec<&String> {
 
     // Add dependency edges
     for module in modules {
-        for dependency in &module.depends_on {
+        for dependency in module.dependencies_iter() {
             graph.add_edge(&module.path, &dependency.path, None::<()>);
         }
     }
@@ -121,8 +121,7 @@ fn validate_root_module_treatment(
                 .filter_map(|module| {
                     if module.path == ROOT_MODULE_SENTINEL_TAG
                         || module
-                            .depends_on
-                            .iter()
+                            .dependencies_iter()
                             .any(|dep| dep.path == ROOT_MODULE_SENTINEL_TAG)
                     {
                         return Some(module.path.clone());
@@ -144,13 +143,15 @@ fn validate_root_module_treatment(
         RootModuleTreatment::DependenciesOnly => {
             let root_module_violations: Vec<String> = modules
                 .iter()
-                .filter(|module| {
-                    module
-                        .depends_on
-                        .iter()
+                .filter_map(|module| {
+                    if module
+                        .dependencies_iter()
                         .any(|dep| dep.path == ROOT_MODULE_SENTINEL_TAG)
+                    {
+                        return Some(module.path.clone());
+                    }
+                    None
                 })
-                .map(|module| module.path.clone())
                 .collect();
 
             if root_module_violations.is_empty() {

src/parsing/config.rs

@@ -29,7 +29,9 @@ pub fn dump_project_config_to_toml(
     });
 
     for module in &mut config.modules {
-        module.depends_on.sort_by(|a, b| a.path.cmp(&b.path));
+        if let Some(depends_on) = &mut module.depends_on {
+            depends_on.sort_by(|a, b| a.path.cmp(&b.path));
+        }
     }
 
     config.exclude.sort();
@@ -102,25 +104,27 @@ mod tests {
                 modules: vec![
                     ModuleConfig {
                         path: "domain_one".to_string(),
-                        depends_on: vec![DependencyConfig::from_deprecated_path("domain_two")],
+                        depends_on: Some(vec![DependencyConfig::from_deprecated_path(
+                            "domain_two"
+                        )]),
                         strict: false,
                         ..Default::default()
                     },
                     ModuleConfig {
                         path: "domain_three".to_string(),
-                        depends_on: vec![],
+                        depends_on: Some(vec![]),
                         strict: false,
                         ..Default::default()
                     },
                     ModuleConfig {
                         path: "domain_two".to_string(),
-                        depends_on: vec![DependencyConfig::from_path("domain_three")],
+                        depends_on: Some(vec![DependencyConfig::from_path("domain_three")]),
                         strict: false,
                         ..Default::default()
                     },
                     ModuleConfig {
                         path: ROOT_MODULE_SENTINEL_TAG.to_string(),
-                        depends_on: vec![DependencyConfig::from_path("domain_one")],
+                        depends_on: Some(vec![DependencyConfig::from_path("domain_one")]),
                         strict: false,
                         ..Default::default()
                     }