From 5a533e7a48bfaa19e803e467437eaa16947f7bbf Mon Sep 17 00:00:00 2001 From: kek kek kek Date: Mon, 24 Jul 2023 11:53:43 -0700 Subject: [PATCH] chore: full path when running tests (#2017) * basic impl * fix naming --- crates/lsp/src/lib.rs | 4 +-- crates/lsp/src/lib_hacky.rs | 4 +-- crates/nargo_cli/src/cli/test_cmd.rs | 5 ++-- .../src/hir/def_collector/dc_crate.rs | 3 +- crates/noirc_frontend/src/hir/def_map/mod.rs | 21 ++++++++++++-- crates/noirc_frontend/src/hir/mod.rs | 29 +++++++++++++++---- .../src/hir/resolution/resolver.rs | 24 +++++++++++---- .../noirc_frontend/src/hir/type_check/mod.rs | 4 ++- crates/noirc_frontend/src/hir_def/function.rs | 3 ++ 9 files changed, 72 insertions(+), 25 deletions(-) diff --git a/crates/lsp/src/lib.rs b/crates/lsp/src/lib.rs index 953540226ad..1b93de288d9 100644 --- a/crates/lsp/src/lib.rs +++ b/crates/lsp/src/lib.rs @@ -201,7 +201,7 @@ fn on_code_lens_request( let tests = context.get_all_test_functions_in_crate_matching(&crate_id, ""); let mut lenses: Vec = vec![]; - for func_id in tests { + for (func_name, func_id) in tests { let location = context.function_meta(&func_id).name.location; let file_id = location.file; // TODO(#1681): This file_id never be 0 because the "path" where it maps is the directory, not a file @@ -209,8 +209,6 @@ fn on_code_lens_request( continue; } - let func_name = context.function_name(&func_id); - let range = byte_span_to_range(files, file_id.as_usize(), location.span.into()).unwrap_or_default(); diff --git a/crates/lsp/src/lib_hacky.rs b/crates/lsp/src/lib_hacky.rs index 0fea6c43144..5a12f0c3b6a 100644 --- a/crates/lsp/src/lib_hacky.rs +++ b/crates/lsp/src/lib_hacky.rs @@ -94,7 +94,7 @@ pub fn on_code_lens_request( let tests = context.get_all_test_functions_in_crate_matching(&crate_id, ""); let mut lenses: Vec = vec![]; - for func_id in tests { + for (func_name, func_id) in tests { let location = context.function_meta(&func_id).name.location; let file_id = location.file; // TODO(#1681): This file_id never be 0 because the "path" where it maps is the directory, not a file @@ -102,8 +102,6 @@ pub fn on_code_lens_request( continue; } - let func_name = context.function_name(&func_id); - let range = byte_span_to_range(files, file_id.as_usize(), location.span.into()).unwrap_or_default(); diff --git a/crates/nargo_cli/src/cli/test_cmd.rs b/crates/nargo_cli/src/cli/test_cmd.rs index afb4481f5c5..cd511464581 100644 --- a/crates/nargo_cli/src/cli/test_cmd.rs +++ b/crates/nargo_cli/src/cli/test_cmd.rs @@ -61,12 +61,11 @@ fn run_tests( let writer = StandardStream::stderr(ColorChoice::Always); let mut writer = writer.lock(); - for test_function in test_functions { - let test_name = context.function_name(&test_function); + for (test_name, test_function) in test_functions { writeln!(writer, "Testing {test_name}...").expect("Failed to write to stdout"); writer.flush().ok(); - match run_test(backend, test_name, test_function, &context, compile_options) { + match run_test(backend, &test_name, test_function, &context, compile_options) { Ok(_) => { writer.set_color(ColorSpec::new().set_fg(Some(Color::Green))).ok(); writeln!(writer, "ok").ok(); diff --git a/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs b/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs index d5fc0251d6e..3f30a4990e4 100644 --- a/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/crates/noirc_frontend/src/hir/def_collector/dc_crate.rs @@ -450,6 +450,7 @@ fn resolve_function_set( let file_id = unresolved_functions.file_id; vecmap(unresolved_functions.functions, |(mod_id, func_id, func)| { + let module_id = ModuleId { krate: crate_id, local_id: mod_id }; let path_resolver = StandardPathResolver::new(ModuleId { local_id: mod_id, krate: crate_id }); @@ -460,7 +461,7 @@ fn resolve_function_set( resolver.set_generics(impl_generics.clone()); resolver.set_self_type(self_type.clone()); - let (hir_func, func_meta, errs) = resolver.resolve_function(func, func_id); + let (hir_func, func_meta, errs) = resolver.resolve_function(func, func_id, module_id); interner.push_fn_meta(func_meta, func_id); interner.update_fn(func_id, hir_func); extend_errors(errors, file_id, errs); diff --git a/crates/noirc_frontend/src/hir/def_map/mod.rs b/crates/noirc_frontend/src/hir/def_map/mod.rs index cc16746daed..daffadc7e3b 100644 --- a/crates/noirc_frontend/src/hir/def_map/mod.rs +++ b/crates/noirc_frontend/src/hir/def_map/mod.rs @@ -39,6 +39,12 @@ pub struct ModuleId { pub local_id: LocalModuleId, } +impl ModuleId { + pub fn dummy_id() -> ModuleId { + ModuleId { krate: CrateId::dummy_id(), local_id: LocalModuleId::dummy_id() } + } +} + impl ModuleId { pub fn module(self, def_maps: &HashMap) -> &ModuleData { &def_maps[&self.krate].modules()[self.local_id.0] @@ -185,7 +191,16 @@ impl CrateDefMap { /// Find a child module's name by inspecting its parent. /// Currently required as modules do not store their own names. - fn get_module_path(&self, child_id: Index, parent: Option) -> String { + pub fn get_module_path(&self, child_id: Index, parent: Option) -> String { + self.get_module_path_with_separator(child_id, parent, ".") + } + + pub fn get_module_path_with_separator( + &self, + child_id: Index, + parent: Option, + separator: &str, + ) -> String { if let Some(id) = parent { let parent = &self.modules[id.0]; let name = parent @@ -195,11 +210,11 @@ impl CrateDefMap { .map(|(name, _)| &name.0.contents) .expect("Child module was not a child of the given parent module"); - let parent_name = self.get_module_path(id.0, parent.parent); + let parent_name = self.get_module_path_with_separator(id.0, parent.parent, separator); if parent_name.is_empty() { name.to_string() } else { - format!("{parent_name}.{name}") + format!("{parent_name}{separator}{name}") } } else { String::new() diff --git a/crates/noirc_frontend/src/hir/mod.rs b/crates/noirc_frontend/src/hir/mod.rs index 6c9237bff48..5937f57a8c7 100644 --- a/crates/noirc_frontend/src/hir/mod.rs +++ b/crates/noirc_frontend/src/hir/mod.rs @@ -87,16 +87,35 @@ impl Context { &self, crate_id: &CrateId, pattern: &str, - ) -> Vec { + ) -> Vec<(String, FuncId)> { let interner = &self.def_interner; - self.def_map(crate_id) - .expect("The local crate should be analyzed already") + let def_map = self.def_map(crate_id).expect("The local crate should be analyzed already"); + + def_map .get_all_test_functions(interner) - .filter_map(|id| interner.function_name(&id).contains(pattern).then_some(id)) + .filter_map(|id| { + let name = interner.function_name(&id); + + let meta = interner.function_meta(&id); + let module = self.module(meta.module_id); + + let parent = def_map.get_module_path_with_separator( + meta.module_id.local_id.0, + module.parent, + "::", + ); + let path = + if parent.is_empty() { name.into() } else { format!("{parent}::{name}") }; + + path.contains(pattern).then_some((path, id)) + }) .collect() } - pub fn get_all_test_functions_in_workspace_matching(&self, pattern: &str) -> Vec { + pub fn get_all_test_functions_in_workspace_matching( + &self, + pattern: &str, + ) -> Vec<(String, FuncId)> { let mut tests = Vec::new(); for crate_id in self.crate_graph.iter_keys() { diff --git a/crates/noirc_frontend/src/hir/resolution/resolver.rs b/crates/noirc_frontend/src/hir/resolution/resolver.rs index cb40ac32082..5553669b746 100644 --- a/crates/noirc_frontend/src/hir/resolution/resolver.rs +++ b/crates/noirc_frontend/src/hir/resolution/resolver.rs @@ -22,7 +22,7 @@ use std::collections::{HashMap, HashSet}; use std::rc::Rc; use crate::graph::CrateId; -use crate::hir::def_map::{ModuleDefId, TryFromModuleDefId, MAIN_FUNCTION}; +use crate::hir::def_map::{ModuleDefId, ModuleId, TryFromModuleDefId, MAIN_FUNCTION}; use crate::hir_def::stmt::{HirAssignStatement, HirLValue, HirPattern}; use crate::node_interner::{ DefinitionId, DefinitionKind, ExprId, FuncId, NodeInterner, StmtId, StructId, @@ -137,6 +137,7 @@ impl<'a> Resolver<'a> { mut self, func: NoirFunction, func_id: FuncId, + module_id: ModuleId, ) -> (HirFunction, FuncMeta, Vec) { self.scopes.start_function(); @@ -145,7 +146,7 @@ impl<'a> Resolver<'a> { self.add_generics(&func.def.generics); - let (hir_func, func_meta) = self.intern_function(func, func_id); + let (hir_func, func_meta) = self.intern_function(func, func_id, module_id); let func_scope_tree = self.scopes.end_function(); self.check_for_unused_variables_in_scope_tree(func_scope_tree); @@ -304,8 +305,13 @@ impl<'a> Resolver<'a> { } } - fn intern_function(&mut self, func: NoirFunction, id: FuncId) -> (HirFunction, FuncMeta) { - let func_meta = self.extract_meta(&func, id); + fn intern_function( + &mut self, + func: NoirFunction, + id: FuncId, + module_id: ModuleId, + ) -> (HirFunction, FuncMeta) { + let func_meta = self.extract_meta(&func, id, module_id); let hir_func = match func.kind { FunctionKind::Builtin | FunctionKind::LowLevel | FunctionKind::Oracle => { HirFunction::empty() @@ -586,7 +592,12 @@ impl<'a> Resolver<'a> { /// to be used in analysis and intern the function parameters /// Prerequisite: self.add_generics() has already been called with the given /// function's generics, including any generics from the impl, if any. - fn extract_meta(&mut self, func: &NoirFunction, func_id: FuncId) -> FuncMeta { + fn extract_meta( + &mut self, + func: &NoirFunction, + func_id: FuncId, + module_id: ModuleId, + ) -> FuncMeta { let location = Location::new(func.name_ident().span(), self.file); let id = self.interner.function_definition_id(func_id); let name_ident = HirIdent { id, location }; @@ -652,6 +663,7 @@ impl<'a> Resolver<'a> { name: name_ident, kind: func.kind, attributes, + module_id, contract_function_type: self.handle_function_type(func), is_internal: self.handle_is_function_internal(func), is_unconstrained: func.def.is_unconstrained, @@ -1361,7 +1373,7 @@ mod test { let id = interner.push_fn(HirFunction::empty()); interner.push_function_definition(func.name().to_string(), id); let resolver = Resolver::new(&mut interner, &path_resolver, &def_maps, file); - let (_, _, err) = resolver.resolve_function(func, id); + let (_, _, err) = resolver.resolve_function(func, id, ModuleId::dummy_id()); errors.extend(err); } diff --git a/crates/noirc_frontend/src/hir/type_check/mod.rs b/crates/noirc_frontend/src/hir/type_check/mod.rs index b8bb6c788e9..a36c1ea67bc 100644 --- a/crates/noirc_frontend/src/hir/type_check/mod.rs +++ b/crates/noirc_frontend/src/hir/type_check/mod.rs @@ -229,6 +229,7 @@ mod test { let func_meta = FuncMeta { name, kind: FunctionKind::Normal, + module_id: ModuleId::dummy_id(), attributes: None, location, contract_function_type: None, @@ -378,7 +379,8 @@ mod test { let func_meta = vecmap(program.functions, |nf| { let resolver = Resolver::new(&mut interner, &path_resolver, &def_maps, file); - let (hir_func, func_meta, resolver_errors) = resolver.resolve_function(nf, main_id); + let (hir_func, func_meta, resolver_errors) = + resolver.resolve_function(nf, main_id, ModuleId::dummy_id()); assert_eq!(resolver_errors, vec![]); (hir_func, func_meta) }); diff --git a/crates/noirc_frontend/src/hir_def/function.rs b/crates/noirc_frontend/src/hir_def/function.rs index 304772f1d0e..a69e8bb08b5 100644 --- a/crates/noirc_frontend/src/hir_def/function.rs +++ b/crates/noirc_frontend/src/hir_def/function.rs @@ -4,6 +4,7 @@ use noirc_errors::{Location, Span}; use super::expr::{HirBlockExpression, HirExpression, HirIdent}; use super::stmt::HirPattern; +use crate::hir::def_map::ModuleId; use crate::node_interner::{ExprId, NodeInterner}; use crate::{token::Attribute, FunctionKind}; use crate::{ContractFunctionType, Type}; @@ -116,6 +117,8 @@ pub struct FuncMeta { pub kind: FunctionKind, + pub module_id: ModuleId, + /// A function's attributes are the `#[...]` items above the function /// definition, if any. Currently, this is limited to a maximum of only one /// Attribute per function.