From 4d1d2ed4f65d2bd70464503b9ed88c9b924b0f2f Mon Sep 17 00:00:00 2001 From: Evan Doyle Date: Thu, 5 Dec 2024 13:32:43 -0800 Subject: [PATCH] Remove dead code related to interface members in ModuleTree --- python/tach/extension.pyi | 1 - src/commands/check_internal.rs | 22 -------- src/lib.rs | 10 ---- src/modules/parsing.rs | 4 +- src/modules/tree.rs | 26 +++------- src/tests/check_internal.rs | 6 --- src/tests/module.rs | 6 --- src/tests/test.rs | 95 ---------------------------------- 8 files changed, 7 insertions(+), 163 deletions(-) diff --git a/python/tach/extension.pyi b/python/tach/extension.pyi index 60d3e00b..18c1d168 100644 --- a/python/tach/extension.pyi +++ b/python/tach/extension.pyi @@ -54,7 +54,6 @@ def update_computation_cache( project_root: str, cache_key: str, value: tuple[list[tuple[int, str]], int] ) -> None: ... def parse_project_config(filepath: Path) -> tuple[ProjectConfig, bool]: ... -def parse_interface_members(source_roots: list[Path], path: str) -> list[str]: ... def dump_project_config_to_toml(project_config: ProjectConfig) -> str: ... def check( project_root: Path, diff --git a/src/commands/check_internal.rs b/src/commands/check_internal.rs index 622f3181..c5e80e13 100644 --- a/src/commands/check_internal.rs +++ b/src/commands/check_internal.rs @@ -123,28 +123,6 @@ impl ImportCheckError { } } -fn is_top_level_module_import(mod_path: &str, module: &ModuleNode) -> bool { - mod_path == module.full_path -} - -fn import_matches_interface_members(mod_path: &str, module: &ModuleNode) -> bool { - let mod_path_segments: Vec<&str> = mod_path.rsplitn(2, '.').collect(); - - if mod_path_segments.len() == 1 { - // If there's no '.' in the path, compare the whole path with the module's full path. - mod_path_segments[0] == module.full_path - } else { - // If there's a '.', split into package path and member name. - let mod_pkg_path = mod_path_segments[1]; - let mod_member_name = mod_path_segments[0]; - - mod_pkg_path == module.full_path - && module - .interface_members - .contains(&mod_member_name.to_string()) - } -} - fn check_import( import_mod_path: &str, module_tree: &ModuleTree, diff --git a/src/lib.rs b/src/lib.rs index 4fd0c6a0..00d5906c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -317,15 +317,6 @@ fn update_computation_cache( cache::update_computation_cache(project_root, cache_key, value) } -#[pyfunction] -#[pyo3(signature = (source_roots, path))] -fn parse_interface_members( - source_roots: Vec, - path: String, -) -> python::parsing::Result> { - python::parsing::parse_interface_members(&source_roots, &path) -} - #[pyfunction] #[pyo3(signature = (project_root, project_config, dependencies, interfaces, exclude_paths))] fn check( @@ -385,7 +376,6 @@ fn extension(py: Python, m: &Bound<'_, PyModule>) -> PyResult<()> { m.add_function(wrap_pyfunction_bound!(create_computation_cache_key, m)?)?; m.add_function(wrap_pyfunction_bound!(check_computation_cache, m)?)?; m.add_function(wrap_pyfunction_bound!(update_computation_cache, m)?)?; - m.add_function(wrap_pyfunction_bound!(parse_interface_members, m)?)?; m.add_function(wrap_pyfunction_bound!(dump_project_config_to_toml, m)?)?; m.add_function(wrap_pyfunction_bound!(check, m)?)?; m.add_function(wrap_pyfunction_bound!(sync_dependency_constraints, m)?)?; diff --git a/src/modules/parsing.rs b/src/modules/parsing.rs index 5b4cfc19..3c8e7dd2 100644 --- a/src/modules/parsing.rs +++ b/src/modules/parsing.rs @@ -4,7 +4,6 @@ use std::path::PathBuf; use crate::core::config::{ global_visibility, ModuleConfig, RootModuleTreatment, ROOT_MODULE_SENTINEL_TAG, }; -use crate::python::parsing::parse_interface_members; use petgraph::algo::kosaraju_scc; use petgraph::graphmap::DiGraphMap; @@ -203,9 +202,8 @@ pub fn build_module_tree( // Construct the ModuleTree let mut tree = ModuleTree::new(); for module in modules { - let interface_members = parse_interface_members(source_roots, &module.path)?; let mod_path = module.mod_path(); - tree.insert(module, mod_path, interface_members)?; + tree.insert(module, mod_path)?; } Ok(tree) diff --git a/src/modules/tree.rs b/src/modules/tree.rs index 26f37439..7d07c72c 100644 --- a/src/modules/tree.rs +++ b/src/modules/tree.rs @@ -19,7 +19,6 @@ pub struct ModuleNode { pub is_end_of_path: bool, pub full_path: String, pub config: Option, - pub interface_members: Vec, pub children: HashMap>, } @@ -29,7 +28,6 @@ impl ModuleNode { is_end_of_path: false, full_path: String::new(), config: None, - interface_members: vec![], children: HashMap::new(), } } @@ -40,7 +38,6 @@ impl ModuleNode { is_end_of_path: true, full_path: ".".to_string(), config: Some(config), - interface_members: vec![], children: HashMap::new(), } } @@ -53,16 +50,10 @@ impl ModuleNode { self.config.as_ref().map_or(false, |c| c.unchecked) } - pub fn fill( - &mut self, - config: ModuleConfig, - full_path: String, - interface_members: Vec, - ) { + pub fn fill(&mut self, config: ModuleConfig, full_path: String) { self.is_end_of_path = true; self.config = Some(config); self.full_path = full_path; - self.interface_members = interface_members; } } @@ -115,12 +106,7 @@ impl ModuleTree { } } - pub fn insert( - &mut self, - config: ModuleConfig, - path: String, - interface_members: Vec, - ) -> Result<(), ModuleTreeError> { + pub fn insert(&mut self, config: ModuleConfig, path: String) -> Result<(), ModuleTreeError> { if path.is_empty() { return Err(ModuleTreeError::InsertNodeError); } @@ -135,7 +121,7 @@ impl ModuleTree { .unwrap(); } - node.fill(config, path, interface_members); + node.fill(config, path); Ok(()) } @@ -253,7 +239,7 @@ mod tests { #[rstest] fn test_insert_empty_path(test_config: ModuleConfig) { let mut tree = ModuleTree::new(); - let result = tree.insert(test_config, "".to_string(), vec![]); + let result = tree.insert(test_config, "".to_string()); assert!(matches!( result.unwrap_err(), ModuleTreeError::InsertNodeError @@ -263,7 +249,7 @@ mod tests { #[rstest] fn test_insert_single_level_path(test_config: ModuleConfig) { let mut tree = ModuleTree::new(); - let result = tree.insert(test_config, "domain".to_string(), vec![]); + let result = tree.insert(test_config, "domain".to_string()); assert!(result.is_ok()); let paths: Vec = tree.iter().map(|node| node.full_path.clone()).collect(); assert_eq!(paths, [".", "domain"]); @@ -272,7 +258,7 @@ mod tests { #[rstest] fn test_insert_multi_level_path(test_config: ModuleConfig) { let mut tree = ModuleTree::new(); - let result = tree.insert(test_config, "domain.subdomain".to_string(), vec![]); + let result = tree.insert(test_config, "domain.subdomain".to_string()); assert!(result.is_ok()); let paths: Vec = tree.iter().map(|node| node.full_path.clone()).collect(); assert_eq!(paths, [".", "domain.subdomain"]); diff --git a/src/tests/check_internal.rs b/src/tests/check_internal.rs index b611135b..13b305ec 100644 --- a/src/tests/check_internal.rs +++ b/src/tests/check_internal.rs @@ -21,7 +21,6 @@ pub mod fixtures { is_end_of_path: false, full_path: String::new(), config: None, - interface_members: vec![], children: HashMap::from([ ( "domain_one".to_string(), @@ -37,14 +36,12 @@ pub mod fixtures { strict: false, ..Default::default() }), - interface_members: vec![], children: HashMap::from([( "subdomain".to_string(), Arc::new(ModuleNode { is_end_of_path: true, full_path: "domain_one.subdomain".to_string(), config: Some(ModuleConfig::new("domain_one.subdomain", false)), - interface_members: vec![], children: HashMap::new(), }), )]), @@ -61,7 +58,6 @@ pub mod fixtures { strict: false, ..Default::default() }), - interface_members: vec![], children: HashMap::from([( "subdomain".to_string(), Arc::new(ModuleNode { @@ -73,7 +69,6 @@ pub mod fixtures { strict: false, ..Default::default() }), - interface_members: vec![], children: HashMap::new(), }), )]), @@ -85,7 +80,6 @@ pub mod fixtures { is_end_of_path: true, full_path: "domain_three".to_string(), config: Some(ModuleConfig::new("domain_three", false)), - interface_members: vec![], children: HashMap::new(), }), ), diff --git a/src/tests/module.rs b/src/tests/module.rs index 1c580c5d..b505303f 100644 --- a/src/tests/module.rs +++ b/src/tests/module.rs @@ -12,7 +12,6 @@ pub mod fixtures { is_end_of_path: true, full_path: ".".to_string(), config: Some(ModuleConfig::new_root_config()), - interface_members: vec![], children: HashMap::from([ ( "domain_one".to_string(), @@ -20,14 +19,12 @@ pub mod fixtures { is_end_of_path: true, full_path: "domain_one".to_string(), config: Some(ModuleConfig::new("test", false)), - interface_members: vec!["public_fn".to_string()], children: HashMap::from([( "subdomain".to_string(), Arc::new(ModuleNode { is_end_of_path: true, full_path: "domain_one.subdomain".to_string(), config: Some(ModuleConfig::new("test", false)), - interface_members: vec![], children: HashMap::new(), }), )]), @@ -39,14 +36,12 @@ pub mod fixtures { is_end_of_path: true, full_path: "domain_two".to_string(), config: Some(ModuleConfig::new("test", false)), - interface_members: vec![], children: HashMap::from([( "subdomain".to_string(), Arc::new(ModuleNode { is_end_of_path: true, full_path: "domain_two.subdomain".to_string(), config: Some(ModuleConfig::new("test", false)), - interface_members: vec![], children: HashMap::new(), }), )]), @@ -58,7 +53,6 @@ pub mod fixtures { is_end_of_path: true, full_path: "domain_three".to_string(), config: Some(ModuleConfig::new("test", false)), - interface_members: vec![], children: HashMap::new(), }), ), diff --git a/src/tests/test.rs b/src/tests/test.rs index f65e0cd7..5110c65a 100644 --- a/src/tests/test.rs +++ b/src/tests/test.rs @@ -179,14 +179,12 @@ pub mod fixtures { is_end_of_path: true, full_path: ".".to_string(), config: Some(ModuleConfig::new_root_config()), - interface_members: vec![], children: HashMap::from([( "tach".to_string(), Arc::new(ModuleNode { is_end_of_path: true, full_path: "tach".to_string(), config: Some(ModuleConfig::new("tach", true)), - interface_members: vec!["__version__".to_string()], children: HashMap::from([ ( "__main__".to_string(), @@ -199,7 +197,6 @@ pub mod fixtures { strict: true, ..Default::default() }), - interface_members: vec![], children: HashMap::new(), }), ), @@ -216,11 +213,6 @@ pub mod fixtures { strict: true, ..Default::default() }), - interface_members: vec![ - "get_uid".to_string(), - "update_latest_version".to_string(), - "get_latest_version".to_string(), - ], children: HashMap::new(), }), ), @@ -241,10 +233,6 @@ pub mod fixtures { strict: true, ..Default::default() }), - interface_members: vec![ - "BoundaryError".to_string(), - "check".to_string(), - ], children: HashMap::new(), }), ), @@ -277,7 +265,6 @@ pub mod fixtures { strict: true, ..Default::default() }), - interface_members: vec!["main".to_string()], children: HashMap::new(), }), ), @@ -287,7 +274,6 @@ pub mod fixtures { is_end_of_path: true, full_path: "tach.colors".to_string(), config: Some(ModuleConfig::new("tach.colors", true)), - interface_members: vec!["BCOLORS".to_string()], children: HashMap::new(), }), ), @@ -297,16 +283,6 @@ pub mod fixtures { is_end_of_path: true, full_path: "tach.constants".to_string(), config: Some(ModuleConfig::new("tach.constants", true)), - interface_members: [ - "PACKAGE_NAME", - "TOOL_NAME", - "CONFIG_FILE_NAME", - "PACKAGE_FILE_NAME", - "ROOT_MODULE_SENTINEL_TAG", - "DEFAULT_EXCLUDE_PATHS", - ] - .map(str::to_string) - .into(), children: HashMap::new(), }), ), @@ -323,15 +299,6 @@ pub mod fixtures { strict: true, ..Default::default() }), - interface_members: [ - "ProjectConfig", - "ModuleConfig", - "ModuleNode", - "ModuleTree", - "UnusedDependencies", - ] - .map(str::to_string) - .into(), children: HashMap::new(), }), ), @@ -341,13 +308,6 @@ pub mod fixtures { is_end_of_path: true, full_path: "tach.errors".to_string(), config: Some(ModuleConfig::new("tach.errors", true)), - interface_members: [ - "TachError", - "TachParseError", - "TachSetupError", - ] - .map(str::to_string) - .into(), children: HashMap::new(), }), ), @@ -370,27 +330,6 @@ pub mod fixtures { strict: true, ..Default::default() }), - interface_members: [ - "get_cwd", - "chdir", - "canonical", - "read_file", - "write_file", - "delete_file", - "parse_ast", - "walk", - "walk_pyfiles", - "file_to_module_path", - "module_to_file_path_no_members", - "module_to_pyfile_or_dir_path", - "get_project_config_path", - "find_project_config_root", - "install_pre_commit", - "validate_project_modules", - "ProjectModuleValidationResult", - ] - .map(str::to_string) - .into(), children: HashMap::from([( "git_ops".to_string(), Arc::new(ModuleNode { @@ -404,7 +343,6 @@ pub mod fixtures { strict: true, ..Default::default() }), - interface_members: vec!["get_changed_files".to_string()], children: HashMap::new(), }), )]), @@ -423,9 +361,6 @@ pub mod fixtures { strict: true, ..Default::default() }), - interface_members: vec![ - "build_pre_commit_hook_content".to_string() - ], children: HashMap::new(), }), ), @@ -442,12 +377,6 @@ pub mod fixtures { strict: true, ..Default::default() }), - interface_members: [ - "get_selected_modules_interactive", - "InteractiveModuleConfiguration", - ] - .map(str::to_string) - .into(), children: HashMap::new(), }), ), @@ -464,11 +393,6 @@ pub mod fixtures { strict: true, ..Default::default() }), - interface_members: [ - "logger".to_string(), - "LogDataModel".to_string(), - ] - .into(), children: HashMap::new(), }), ), @@ -492,7 +416,6 @@ pub mod fixtures { strict: true, ..Default::default() }), - interface_members: vec!["mod_edit_interactive".to_string()], children: HashMap::new(), }), ), @@ -513,14 +436,6 @@ pub mod fixtures { strict: true, ..Default::default() }), - interface_members: [ - "parse_project_config", - "dump_project_config_to_yaml", - "parse_interface_members", - "build_module_tree", - ] - .map(str::to_string) - .into(), children: HashMap::new(), }), ), @@ -537,7 +452,6 @@ pub mod fixtures { strict: true, ..Default::default() }), - interface_members: vec!["report".to_string()], children: HashMap::new(), }), ), @@ -547,7 +461,6 @@ pub mod fixtures { is_end_of_path: true, full_path: "tach.show".to_string(), config: Some(ModuleConfig::new("tach.show", true)), - interface_members: vec!["generate_show_url".to_string()], children: HashMap::new(), }), ), @@ -562,7 +475,6 @@ pub mod fixtures { strict: true, ..Default::default() }), - interface_members: vec!["start".to_string()], children: HashMap::new(), }), ), @@ -584,12 +496,6 @@ pub mod fixtures { strict: true, ..Default::default() }), - interface_members: [ - "sync_project", - "prune_dependency_constraints", - ] - .map(str::to_string) - .into(), children: HashMap::new(), }), ), @@ -611,7 +517,6 @@ pub mod fixtures { strict: false, ..Default::default() }), - interface_members: vec![], children: HashMap::new(), }), ),