Skip to content

Commit

Permalink
chore: full path when running tests (#2017)
Browse files Browse the repository at this point in the history
* basic impl

* fix naming
  • Loading branch information
kek kek kek authored Jul 24, 2023
1 parent de905cc commit 5a533e7
Show file tree
Hide file tree
Showing 9 changed files with 72 additions and 25 deletions.
4 changes: 1 addition & 3 deletions crates/lsp/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -201,16 +201,14 @@ fn on_code_lens_request(
let tests = context.get_all_test_functions_in_crate_matching(&crate_id, "");

let mut lenses: Vec<CodeLens> = 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
if file_id.as_usize() != 0 {
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();

Expand Down
4 changes: 1 addition & 3 deletions crates/lsp/src/lib_hacky.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,16 +94,14 @@ pub fn on_code_lens_request(
let tests = context.get_all_test_functions_in_crate_matching(&crate_id, "");

let mut lenses: Vec<CodeLens> = 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
if file_id.as_usize() != 0 {
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();

Expand Down
5 changes: 2 additions & 3 deletions crates/nargo_cli/src/cli/test_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,12 +61,11 @@ fn run_tests<B: Backend>(
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();
Expand Down
3 changes: 2 additions & 1 deletion crates/noirc_frontend/src/hir/def_collector/dc_crate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 });

Expand All @@ -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);
Expand Down
21 changes: 18 additions & 3 deletions crates/noirc_frontend/src/hir/def_map/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<CrateId, CrateDefMap>) -> &ModuleData {
&def_maps[&self.krate].modules()[self.local_id.0]
Expand Down Expand Up @@ -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<LocalModuleId>) -> String {
pub fn get_module_path(&self, child_id: Index, parent: Option<LocalModuleId>) -> String {
self.get_module_path_with_separator(child_id, parent, ".")
}

pub fn get_module_path_with_separator(
&self,
child_id: Index,
parent: Option<LocalModuleId>,
separator: &str,
) -> String {
if let Some(id) = parent {
let parent = &self.modules[id.0];
let name = parent
Expand All @@ -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()
Expand Down
29 changes: 24 additions & 5 deletions crates/noirc_frontend/src/hir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,16 +87,35 @@ impl Context {
&self,
crate_id: &CrateId,
pattern: &str,
) -> Vec<FuncId> {
) -> 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<FuncId> {
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() {
Expand Down
24 changes: 18 additions & 6 deletions crates/noirc_frontend/src/hir/resolution/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -137,6 +137,7 @@ impl<'a> Resolver<'a> {
mut self,
func: NoirFunction,
func_id: FuncId,
module_id: ModuleId,
) -> (HirFunction, FuncMeta, Vec<ResolverError>) {
self.scopes.start_function();

Expand All @@ -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);
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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 };
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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);
}

Expand Down
4 changes: 3 additions & 1 deletion crates/noirc_frontend/src/hir/type_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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)
});
Expand Down
3 changes: 3 additions & 0 deletions crates/noirc_frontend/src/hir_def/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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.
Expand Down

0 comments on commit 5a533e7

Please sign in to comment.