From 4dacfaa44a863e242d2bc74eba3ae9d9701476b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20=C5=BD=C3=A1dn=C3=ADk?= Date: Mon, 4 Oct 2021 20:08:24 +0300 Subject: [PATCH 1/5] Add import pattern support to 'hide' --- crates/nu-parser/src/parse_keywords.rs | 88 ++++++++++++++++++++++++-- src/tests.rs | 40 ++++++++++++ 2 files changed, 124 insertions(+), 4 deletions(-) diff --git a/crates/nu-parser/src/parse_keywords.rs b/crates/nu-parser/src/parse_keywords.rs index 033a21e738..820427f3e2 100644 --- a/crates/nu-parser/src/parse_keywords.rs +++ b/crates/nu-parser/src/parse_keywords.rs @@ -600,12 +600,92 @@ pub fn parse_hide( let (name_expr, err) = parse_string(working_set, spans[1]); error = error.or(err); - let name_bytes: Vec = working_set.get_span_contents(spans[1]).into(); + let (import_pattern, err) = parse_import_pattern(working_set, spans[1]); + error = error.or(err); - // TODO: Do the import pattern stuff for bulk-hiding + let exported_names: Vec> = + if let Some(block_id) = working_set.find_module(&import_pattern.head) { + working_set + .get_block(block_id) + .exports + .iter() + .map(|(name, _)| name.clone()) + .collect() + } else { + if import_pattern.members.is_empty() { + // The pattern head can be e.g. a function name, not just a module + vec![import_pattern.head.clone()] + } else { + return ( + garbage_statement(spans), + Some(ParseError::ModuleNotFound(spans[1])), + ); + } + }; - if working_set.hide_decl(&name_bytes).is_none() { - error = error.or_else(|| Some(ParseError::UnknownCommand(spans[1]))); + // This kind of inverts the import pattern matching found in parse_use() + let names_to_hide = if import_pattern.members.is_empty() { + exported_names + } else { + match &import_pattern.members[0] { + ImportPatternMember::Glob { .. } => exported_names + .into_iter() + .map(|name| { + let mut new_name = import_pattern.head.to_vec(); + new_name.push(b'.'); + new_name.extend(&name); + new_name + }) + .collect(), + ImportPatternMember::Name { name, span } => { + let new_exports: Vec> = exported_names + .into_iter() + .filter(|n| n == name) + .map(|n| { + let mut new_name = import_pattern.head.to_vec(); + new_name.push(b'.'); + new_name.extend(&n); + new_name + }) + .collect(); + + if new_exports.is_empty() { + error = error.or(Some(ParseError::ExportNotFound(*span))) + } + + new_exports + } + ImportPatternMember::List { names } => { + let mut output = vec![]; + + for (name, span) in names { + let mut new_exports: Vec> = exported_names + .iter() + .filter_map(|n| if n == name { Some(n.clone()) } else { None }) + .map(|n| { + let mut new_name = import_pattern.head.to_vec(); + new_name.push(b'.'); + new_name.extend(n); + new_name + }) + .collect(); + + if new_exports.is_empty() { + error = error.or(Some(ParseError::ExportNotFound(*span))) + } else { + output.append(&mut new_exports) + } + } + + output + } + } + }; + + for name in names_to_hide { + if working_set.hide_decl(&name).is_none() { + error = error.or_else(|| Some(ParseError::UnknownCommand(spans[1]))); + } } // Create the Hide command call diff --git a/src/tests.rs b/src/tests.rs index 7416aecb6f..db8fde4bd8 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -441,6 +441,46 @@ fn hide_twice_not_allowed() -> TestResult { ) } +#[test] +fn hides_import_1() -> TestResult { + fail_test( + r#"module spam { export def foo [] { "foo" } }; use spam; hide spam.foo; foo"#, + "not found" + ) +} + +#[test] +fn hides_import_2() -> TestResult { + fail_test( + r#"module spam { export def foo [] { "foo" } }; use spam; hide spam.*; foo"#, + "not found" + ) +} + +#[test] +fn hides_import_3() -> TestResult { + fail_test( + r#"module spam { export def foo [] { "foo" } }; use spam; hide spam.[foo]; foo"#, + "not found" + ) +} + +#[test] +fn hides_import_4() -> TestResult { + fail_test( + r#"module spam { export def foo [] { "foo" } }; use spam.foo; hide foo; foo"#, + "not found" + ) +} + +#[test] +fn hides_import_5() -> TestResult { + fail_test( + r#"module spam { export def foo [] { "foo" } }; use spam.*; hide foo; foo"#, + "not found" + ) +} + #[test] fn def_twice_should_fail() -> TestResult { fail_test( From 0fe525de872d4622a0b7d9c8ed66f9b986e95c0d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20=C5=BD=C3=A1dn=C3=ADk?= Date: Mon, 4 Oct 2021 20:16:43 +0300 Subject: [PATCH 2/5] Add test with TODO note --- src/tests.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/tests.rs b/src/tests.rs index db8fde4bd8..173c2f7ba1 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -489,6 +489,15 @@ fn def_twice_should_fail() -> TestResult { ) } +// TODO: This test fails if executed each command on a separate line in REPL +#[test] +fn use_import_after_hide() -> TestResult { + run_test( + r#"module spam { export def foo [] { "foo" } }; use spam.foo; hide foo; use spam.foo; foo"#, + "foo" + ) +} + #[test] fn from_json_1() -> TestResult { run_test(r#"('{"name": "Fred"}' | from json).name"#, "Fred") From 9737d4a614a49754eeda0f7ef3f375863c9cdcb6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20=C5=BD=C3=A1dn=C3=ADk?= Date: Mon, 4 Oct 2021 20:33:27 +0300 Subject: [PATCH 3/5] Change comments --- crates/nu-protocol/src/engine/engine_state.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/crates/nu-protocol/src/engine/engine_state.rs b/crates/nu-protocol/src/engine/engine_state.rs index bbe6bdb1c1..c5cd679860 100644 --- a/crates/nu-protocol/src/engine/engine_state.rs +++ b/crates/nu-protocol/src/engine/engine_state.rs @@ -375,7 +375,7 @@ impl<'a> StateWorkingSet<'a> { if let Some(decl_id) = scope.decls.get(name) { if !hiding.contains(decl_id) { - // Do not hide already hidden decl + // Hide decl only if it's not already hidden last_scope_frame.hiding.insert(*decl_id); return Some(*decl_id); } @@ -409,8 +409,6 @@ impl<'a> StateWorkingSet<'a> { } pub fn activate_overlay(&mut self, overlay: Vec<(Vec, DeclId)>) { - // TODO: This will overwrite all existing definitions in a scope. When we add deactivate, - // we need to re-think how make it recoverable. let scope_frame = self .delta .scope From 1e1e12b027718ea9cd8b0528f91c0bd0860123d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20=C5=BD=C3=A1dn=C3=ADk?= Date: Mon, 4 Oct 2021 22:17:18 +0300 Subject: [PATCH 4/5] Fmt --- src/tests.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/tests.rs b/src/tests.rs index 173c2f7ba1..0efd0b513a 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -445,7 +445,7 @@ fn hide_twice_not_allowed() -> TestResult { fn hides_import_1() -> TestResult { fail_test( r#"module spam { export def foo [] { "foo" } }; use spam; hide spam.foo; foo"#, - "not found" + "not found", ) } @@ -453,7 +453,7 @@ fn hides_import_1() -> TestResult { fn hides_import_2() -> TestResult { fail_test( r#"module spam { export def foo [] { "foo" } }; use spam; hide spam.*; foo"#, - "not found" + "not found", ) } @@ -461,7 +461,7 @@ fn hides_import_2() -> TestResult { fn hides_import_3() -> TestResult { fail_test( r#"module spam { export def foo [] { "foo" } }; use spam; hide spam.[foo]; foo"#, - "not found" + "not found", ) } @@ -469,7 +469,7 @@ fn hides_import_3() -> TestResult { fn hides_import_4() -> TestResult { fail_test( r#"module spam { export def foo [] { "foo" } }; use spam.foo; hide foo; foo"#, - "not found" + "not found", ) } @@ -477,7 +477,7 @@ fn hides_import_4() -> TestResult { fn hides_import_5() -> TestResult { fail_test( r#"module spam { export def foo [] { "foo" } }; use spam.*; hide foo; foo"#, - "not found" + "not found", ) } @@ -494,7 +494,7 @@ fn def_twice_should_fail() -> TestResult { fn use_import_after_hide() -> TestResult { run_test( r#"module spam { export def foo [] { "foo" } }; use spam.foo; hide foo; use spam.foo; foo"#, - "foo" + "foo", ) } From 6f5f1fa43a5612bfae690448d5136bdae61533ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20=C5=BD=C3=A1dn=C3=ADk?= Date: Mon, 4 Oct 2021 22:37:43 +0300 Subject: [PATCH 5/5] Clippy --- crates/nu-parser/src/parse_keywords.rs | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/crates/nu-parser/src/parse_keywords.rs b/crates/nu-parser/src/parse_keywords.rs index 820427f3e2..d093cd88ce 100644 --- a/crates/nu-parser/src/parse_keywords.rs +++ b/crates/nu-parser/src/parse_keywords.rs @@ -611,16 +611,14 @@ pub fn parse_hide( .iter() .map(|(name, _)| name.clone()) .collect() + } else if import_pattern.members.is_empty() { + // The pattern head can be e.g. a function name, not just a module + vec![import_pattern.head.clone()] } else { - if import_pattern.members.is_empty() { - // The pattern head can be e.g. a function name, not just a module - vec![import_pattern.head.clone()] - } else { - return ( - garbage_statement(spans), - Some(ParseError::ModuleNotFound(spans[1])), - ); - } + return ( + garbage_statement(spans), + Some(ParseError::ModuleNotFound(spans[1])), + ); }; // This kind of inverts the import pattern matching found in parse_use()