Repository: gauge-sh/tach (language: python)

Issue:
disable allowing for <root> in tach.toml?
Hey, is there a way of configuring tach to not allow dependencies on `<root>`? I would like the following behavior:

- when running tach sync, never introduce `<root>` but fail instead
- when tach.toml was manually added to include `<root>`, fail on `tach check`

This would make it easier for people in my repo to start using tach and not by accident introduce `<root>` deps because they forgot to explicitly add a new module.


Discussion / hints:
This makes sense to me as a configurable option! Something like `root_module: 'forbid'`, which would give the behavior you described. There was another request to simply hide all of the `<root>` instances, which could correspond to something like `root_module: 'ignore'`. A middle ground might be to allow `<root>` to have its own listed `depends_on` key, but not allow any other modules to depend on `<root>`, this might be `root_module: 'dependencies-only'`

This (or maybe a simpler version) should go out in the next release.

EDIT: This is a bit delayed! Will ping back in this thread when it's available.

Fix patch:
diff --git a/docs/usage/configuration.mdx b/docs/usage/configuration.mdx
index eab6d3a8..f4104921 100644
--- a/docs/usage/configuration.mdx
+++ b/docs/usage/configuration.mdx
@@ -22,6 +22,8 @@ This is the project-level configuration file which should be in the root of your
 
 `forbid_circular_dependencies` is a boolean which, when enabled, causes `tach check` to fail if any circular dependencies are detected.
 
+`root_module` takes a string enum value, and determines how Tach treats code which lives within the project but is not covered by an explicit module. This is described in detail [below](#the_root_module)
+
 `use_regex_matching` is a boolean which, when enabled, uses regex (default) matching to exclude patterns else uses globs matching.
 
 ```toml
@@ -39,6 +41,8 @@ exact = true
 ignore_type_checking_imports = true
 forbid_circular_dependencies = true
 
+root_module = "allow"
+
 [[modules]]
 path = "tach"
 depends_on = []
@@ -93,6 +97,39 @@ Each module listed under the `modules` key above can accept the following attrib
   Tach also supports [deprecating individual dependencies](../usage/deprecate).
 </Note>
 
+## The Root Module
+
+By default, Tach checks all of the source files beneath all of the configured [source roots](#source_roots).
+This means that some code may not be contained within any configured [module](#modules).
+
+For example, given the file tree below:
+
+```
+my_repo/
+  tach.toml
+  script.py
+  lib/
+    module1.py
+    module2/
+      __init__.py
+      service.py
+    module3.py
+  docs/
+  tests/
+```
+
+If `lib.module1`, `lib.module2`, and `lib.module3` are the only configured modules, then the code in `script.py` would be automatically part of the `<root>` module.
+
+This module can declare its own dependencies with `depends_on` and use the rest of the available module configuration.
+Further, other modules need to declare an explicit dependency on `<root>` to use code which rolls up to the root.
+
+Tach allows configuring how the root module should be treated through the `root_module` key in `tach.toml`. It may take one of the following values:
+
+- **(default)** `"allow"`: Treat `<root>` as a catch-all rollup module which must be explicitly declared as a dependency and must declare its own dependencies on other modules.
+- **(permissive)** `"ignore"`: Disable all checks related to the `<root>` module. `tach check` will never fail due to code in the `<root>` module, and `tach sync` will never add `<root>` to `tach.toml`
+- **(stricter)** `"dependenciesonly"`: Forbid any module from listing `<root>` as a dependency, but allow `<root>` to declare its own dependencies.
+- **(strictest)** `"forbid"`: Forbid any reference to the `<root>` module in tach.toml. This means that all code in [source roots](#source_roots) MUST be contained within an explicitly configured [module](#modules).
+
 ## Source Roots
 
 The `source_roots` key is required for Tach to understand the imports within your project.
diff --git a/src/check_int.rs b/src/check_int.rs
index 623d4207..471464a0 100644
--- a/src/check_int.rs
+++ b/src/check_int.rs
@@ -9,7 +9,7 @@ use thiserror::Error;
 
 use crate::{
     core::{
-        config::ProjectConfig,
+        config::{ProjectConfig, RootModuleTreatment},
         module::{ModuleNode, ModuleTree},
     },
     exclusion::{self, is_path_excluded, set_excluded_paths},
@@ -139,6 +139,7 @@ fn check_import(
     file_mod_path: &str,
     module_tree: &ModuleTree,
     file_nearest_module: Option<Arc<ModuleNode>>,
+    root_module_treatment: RootModuleTreatment,
 ) -> Result<(), ImportCheckError> {
     let import_nearest_module = match module_tree.find_nearest(import_mod_path) {
         Some(module) => module,
@@ -147,6 +148,10 @@ fn check_import(
         None => return Ok(()),
     };
 
+    if import_nearest_module.is_root() && root_module_treatment == RootModuleTreatment::Ignore {
+        return Ok(());
+    }
+
     let file_nearest_module = file_nearest_module
         // Lookup file_mod_path if module not given
         .or_else(|| module_tree.find_nearest(file_mod_path))
@@ -265,6 +270,7 @@ pub fn check(
         &source_roots,
         valid_modules,
         project_config.forbid_circular_dependencies,
+        project_config.root_module.clone(),
     )?;
 
     set_excluded_paths(
@@ -283,6 +289,11 @@ pub fn check(
             let Some(nearest_module) = module_tree.find_nearest(&mod_path) else {
                 continue;
             };
+
+            if nearest_module.is_root() && project_config.root_module == RootModuleTreatment::Ignore
+            {
+                continue;
+            }
             let project_imports = match get_project_imports(
                 &source_roots,
                 abs_file_path,
@@ -319,6 +330,7 @@ pub fn check(
                     &mod_path,
                     &module_tree,
                     Some(Arc::clone(&nearest_module)),
+                    project_config.root_module.clone(),
                 ) else {
                     continue;
                 };
@@ -381,14 +393,26 @@ mod tests {
         #[case] import_mod_path: &str,
         #[case] expected_result: bool,
     ) {
-        let check_error = check_import(import_mod_path, file_mod_path, &module_tree, None);
+        let check_error = check_import(
+            import_mod_path,
+            file_mod_path,
+            &module_tree,
+            None,
+            RootModuleTreatment::Allow,
+        );
         let result = check_error.is_ok();
         assert_eq!(result, expected_result);
     }
 
     #[rstest]
     fn test_check_deprecated_import(module_tree: ModuleTree) {
-        let check_error = check_import("domain_one.subdomain", "domain_one", &module_tree, None);
+        let check_error = check_import(
+            "domain_one.subdomain",
+            "domain_one",
+            &module_tree,
+            None,
+            RootModuleTreatment::Allow,
+        );
         assert!(check_error.is_err());
         assert!(check_error.unwrap_err().is_deprecated());
     }
diff --git a/src/core/config.rs b/src/core/config.rs
index 804ab67f..98b3ee26 100644
--- a/src/core/config.rs
+++ b/src/core/config.rs
@@ -183,6 +183,33 @@ pub struct UnusedDependencies {
     pub dependencies: Vec<DependencyConfig>,
 }
 
+#[derive(Debug, Serialize, Default, Deserialize, Clone, PartialEq)]
+#[serde(rename_all = "lowercase")]
+pub enum RootModuleTreatment {
+    #[default]
+    Allow,
+    Forbid,
+    Ignore,
+    DependenciesOnly,
+}
+
+impl RootModuleTreatment {
+    fn is_default(&self) -> bool {
+        *self == Self::default()
+    }
+}
+
+impl IntoPy<PyObject> for RootModuleTreatment {
+    fn into_py(self, py: Python) -> PyObject {
+        match self {
+            Self::Allow => "allow".to_object(py),
+            Self::Forbid => "forbid".to_object(py),
+            Self::Ignore => "ignore".to_object(py),
+            Self::DependenciesOnly => "dependenciesonly".to_object(py),
+        }
+    }
+}
+
 #[derive(Debug, Serialize, Deserialize, Clone, PartialEq)]
 #[serde(deny_unknown_fields)]
 #[pyclass(get_all, module = "tach.extension")]
@@ -209,6 +236,8 @@ pub struct ProjectConfig {
     pub forbid_circular_dependencies: bool,
     #[serde(default = "default_true", skip_serializing_if = "is_true")]
     pub use_regex_matching: bool,
+    #[serde(default, skip_serializing_if = "RootModuleTreatment::is_default")]
+    pub root_module: RootModuleTreatment,
 }
 
 impl Default for ProjectConfig {
@@ -224,6 +253,7 @@ impl Default for ProjectConfig {
             ignore_type_checking_imports: default_true(),
             forbid_circular_dependencies: Default::default(),
             use_regex_matching: default_true(),
+            root_module: Default::default(),
         }
     }
 }
@@ -291,6 +321,7 @@ impl ProjectConfig {
             ignore_type_checking_imports: self.ignore_type_checking_imports,
             forbid_circular_dependencies: self.forbid_circular_dependencies,
             use_regex_matching: self.use_regex_matching,
+            root_module: self.root_module.clone(),
         }
     }
 
diff --git a/src/core/module.rs b/src/core/module.rs
index a1719e54..f7b30573 100644
--- a/src/core/module.rs
+++ b/src/core/module.rs
@@ -46,6 +46,10 @@ impl ModuleNode {
         }
     }
 
+    pub fn is_root(&self) -> bool {
+        self.full_path == "." && self.is_end_of_path
+    }
+
     pub fn fill(
         &mut self,
         config: ModuleConfig,
diff --git a/src/lib.rs b/src/lib.rs
index 85862946..04a98038 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -95,6 +95,8 @@ impl From<sync::SyncError> for PyErr {
         match err {
             SyncError::FileWrite(err) => PyOSError::new_err(err.to_string()),
             SyncError::TomlSerialize(err) => PyOSError::new_err(err.to_string()),
+            SyncError::CheckError(err) => err.into(),
+            SyncError::RootModuleViolation(err) => PyValueError::new_err(err.to_string()),
         }
     }
 }
@@ -298,7 +300,7 @@ fn sync_dependency_constraints(
     project_config: ProjectConfig,
     exclude_paths: Vec<String>,
     prune: bool,
-) -> ProjectConfig {
+) -> Result<ProjectConfig, SyncError> {
     sync::sync_dependency_constraints(project_root, project_config, exclude_paths, prune)
 }
 
diff --git a/src/parsing/config.rs b/src/parsing/config.rs
index 7a05a4ac..7067f01c 100644
--- a/src/parsing/config.rs
+++ b/src/parsing/config.rs
@@ -40,7 +40,7 @@ mod tests {
 
     use super::*;
     use crate::{
-        core::config::{CacheConfig, DependencyConfig, ExternalDependencyConfig, ModuleConfig},
+        core::config::{DependencyConfig, ModuleConfig},
         tests::fixtures::example_dir,
     };
     use filesystem::DEFAULT_EXCLUDE_PATHS;
@@ -79,7 +79,7 @@ mod tests {
                         ..Default::default()
                     }
                 ],
-                cache: CacheConfig::default(),
+                cache: Default::default(),
                 exclude: DEFAULT_EXCLUDE_PATHS
                     .into_iter()
                     .chain(["domain_four"].into_iter())
@@ -91,7 +91,8 @@ mod tests {
                 ignore_type_checking_imports: true,
                 forbid_circular_dependencies: true,
                 use_regex_matching: true,
-                external: ExternalDependencyConfig::default(),
+                external: Default::default(),
+                root_module: Default::default(),
             }
         );
     }
diff --git a/src/parsing/error.rs b/src/parsing/error.rs
index 9c846eb2..21f0dc51 100644
--- a/src/parsing/error.rs
+++ b/src/parsing/error.rs
@@ -50,6 +50,8 @@ pub enum ModuleTreeError {
     VisibilityViolation(Vec<VisibilityErrorInfo>),
     #[error("Circular dependency detected: {0:?}")]
     CircularDependency(Vec<String>),
+    #[error("Root module violation: {0:?}")]
+    RootModuleViolation(String),
     #[error("Parsing Error while building module tree.\n{0}")]
     ParseError(#[from] ParsingError),
     #[error("Cannot insert module with empty path.")]
diff --git a/src/parsing/module.rs b/src/parsing/module.rs
index e9d03e4b..8b693209 100644
--- a/src/parsing/module.rs
+++ b/src/parsing/module.rs
@@ -1,8 +1,9 @@
 use std::collections::{HashMap, HashSet};
 use std::path::PathBuf;
 
-use crate::core::config::{global_visibility, ModuleConfig};
+use crate::core::config::{global_visibility, ModuleConfig, RootModuleTreatment};
 use crate::core::module::ModuleTree;
+use crate::filesystem::ROOT_MODULE_SENTINEL_TAG;
 use petgraph::algo::kosaraju_scc;
 use petgraph::graphmap::DiGraphMap;
 
@@ -108,10 +109,63 @@ pub fn find_modules_with_cycles(modules: &[ModuleConfig]) -> Vec<&String> {
     modules_with_cycles
 }
 
+fn validate_root_module_treatment(
+    root_module_treatment: RootModuleTreatment,
+    modules: &[ModuleConfig],
+) -> Result<(), ModuleTreeError> {
+    match root_module_treatment {
+        RootModuleTreatment::Allow | RootModuleTreatment::Ignore => Ok(()),
+        RootModuleTreatment::Forbid => {
+            let root_module_violations: Vec<String> = modules
+                .iter()
+                .filter_map(|module| {
+                    if module.path == ROOT_MODULE_SENTINEL_TAG
+                        || module
+                            .depends_on
+                            .iter()
+                            .any(|dep| dep.path == ROOT_MODULE_SENTINEL_TAG)
+                    {
+                        return Some(module.path.clone());
+                    }
+                    None
+                })
+                .collect();
+
+            if root_module_violations.is_empty() {
+                Ok(())
+            } else {
+                Err(ModuleTreeError::RootModuleViolation(format!(
+                    "The root module ('{}') is forbidden, but was found in module configuration for modules: {}.",
+                    ROOT_MODULE_SENTINEL_TAG,
+                    root_module_violations.into_iter().map(|module| format!("'{}'", module)).collect::<Vec<_>>().join(", ")
+                )))
+            }
+        }
+        RootModuleTreatment::DependenciesOnly => {
+            let root_module_violations: Vec<String> = modules
+                .iter()
+                .filter(|module| module.path == ROOT_MODULE_SENTINEL_TAG)
+                .map(|module| module.path.clone())
+                .collect();
+
+            if root_module_violations.is_empty() {
+                Ok(())
+            } else {
+                Err(ModuleTreeError::RootModuleViolation(format!(
+                    "The root module ('{}') is set to allow dependencies only, but was found as a dependency in: {}.",
+                    ROOT_MODULE_SENTINEL_TAG,
+                    root_module_violations.into_iter().map(|module| format!("'{}'", module)).collect::<Vec<_>>().join(", ")
+                )))
+            }
+        }
+    }
+}
+
 pub fn build_module_tree(
     source_roots: &[PathBuf],
     modules: Vec<ModuleConfig>,
     forbid_circular_dependencies: bool,
+    root_module_treatment: RootModuleTreatment,
 ) -> Result<ModuleTree, ModuleTreeError> {
     // Check for duplicate modules
     let duplicate_modules = find_duplicate_modules(&modules);
@@ -127,6 +181,9 @@ pub fn build_module_tree(
         return Err(ModuleTreeError::VisibilityViolation(visibility_error_info));
     }
 
+    // Check for root module treatment errors
+    validate_root_module_treatment(root_module_treatment, &modules)?;
+
     // Check for circular dependencies if forbidden
     if forbid_circular_dependencies {
         let module_paths = find_modules_with_cycles(&modules);
diff --git a/src/sync.rs b/src/sync.rs
index 70a848c8..bf8c195d 100644
--- a/src/sync.rs
+++ b/src/sync.rs
@@ -1,8 +1,10 @@
 use thiserror::Error;
 
-use crate::check_int::check;
-use crate::core::config::{global_visibility, DependencyConfig, ModuleConfig, ProjectConfig};
-use crate::filesystem as fs;
+use crate::check_int::{check, CheckError};
+use crate::core::config::{
+    global_visibility, DependencyConfig, ModuleConfig, ProjectConfig, RootModuleTreatment,
+};
+use crate::filesystem::{self as fs, ROOT_MODULE_SENTINEL_TAG};
 use crate::parsing::config::dump_project_config_to_toml;
 use std::collections::HashMap;
 use std::path::PathBuf;
@@ -13,6 +15,43 @@ pub enum SyncError {
     FileWrite(#[from] std::io::Error),
     #[error("Failed to serialize project configuration to TOML.\n{0}")]
     TomlSerialize(#[from] toml::ser::Error),
+    #[error("Failed to sync project.\n{0}")]
+    CheckError(#[from] CheckError),
+    #[error("Failed to sync project configuration due to root module violation.\n{0}")]
+    RootModuleViolation(String),
+}
+
+fn handle_detected_dependency(
+    module_path: &str,
+    dependency: DependencyConfig,
+    project_config: &mut ProjectConfig,
+) -> Result<(), SyncError> {
+    let module_is_root = module_path == ROOT_MODULE_SENTINEL_TAG;
+    let dependency_is_root = dependency.path == ROOT_MODULE_SENTINEL_TAG;
+
+    if !module_is_root && !dependency_is_root {
+        project_config.add_dependency_to_module(module_path, dependency);
+        return Ok(());
+    }
+
+    match project_config.root_module {
+        RootModuleTreatment::Ignore => Ok(()),
+        RootModuleTreatment::Allow => {
+            project_config.add_dependency_to_module(module_path, dependency);
+            Ok(())
+        }
+        RootModuleTreatment::Forbid => Err(SyncError::RootModuleViolation(format!(
+            "The root module is forbidden, but it was found that '{}' depends on '{}'.",
+            module_path, dependency.path
+        ))),
+        RootModuleTreatment::DependenciesOnly => {
+            if dependency_is_root {
+                return Err(SyncError::RootModuleViolation(format!("No module may depend on the root module, but it was found that '{}' depends on the root module.", module_path)));
+            }
+            project_config.add_dependency_to_module(module_path, dependency);
+            Ok(())
+        }
+    }
 }
 
 /// Update project configuration with auto-detected dependency constraints.
@@ -23,7 +62,7 @@ pub fn sync_dependency_constraints(
     mut project_config: ProjectConfig,
     exclude_paths: Vec<String>,
     prune: bool,
-) -> ProjectConfig {
+) -> Result<ProjectConfig, SyncError> {
     let mut deprecation_map: HashMap<String, Vec<String>> = HashMap::new();
     let mut visibility_map: HashMap<String, Vec<String>> = HashMap::new();
     let mut new_project_config = None;
@@ -67,8 +106,7 @@ pub fn sync_dependency_constraints(
     let mut new_project_config = new_project_config.unwrap_or(project_config);
 
     // If prune is false, the existing project config is reused without changes
-    let check_result = check(project_root, &new_project_config, exclude_paths)
-        .expect("Failed to run the check function");
+    let check_result = check(project_root, &new_project_config, exclude_paths)?;
 
     // Iterate through the check results to add dependencies to the config
     for error in check_result.errors {
@@ -87,7 +125,8 @@ pub fn sync_dependency_constraints(
                 deprecated,
             };
 
-            new_project_config.add_dependency_to_module(source_path, dependency);
+            // The project config determines whether the sync fails, ignores, or adds this dependency
+            handle_detected_dependency(source_path, dependency, &mut new_project_config)?
         }
     }
 
@@ -98,7 +137,7 @@ pub fn sync_dependency_constraints(
         }
     }
 
-    new_project_config
+    Ok(new_project_config)
 }
 
 pub fn sync_project(
@@ -108,7 +147,7 @@ pub fn sync_project(
     add: bool,
 ) -> Result<String, SyncError> {
     let mut project_config =
-        sync_dependency_constraints(project_root, project_config, exclude_paths, !add);
+        sync_dependency_constraints(project_root, project_config, exclude_paths, !add)?;
 
     Ok(dump_project_config_to_toml(&mut project_config)?)
 }


Test patch:
diff --git a/src/test.rs b/src/test.rs
index 360fbd49..9d4ea7a5 100644
--- a/src/test.rs
+++ b/src/test.rs
@@ -59,6 +59,7 @@ impl TachPytestPluginHandler {
             &source_roots,
             valid_modules,
             project_config.forbid_circular_dependencies,
+            project_config.root_module.clone(),
         )
         .unwrap();