From c92211c01689a4eb76c5a3f755506ab4bf9f8804 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20=C5=BD=C3=A1dn=C3=ADk?= Date: Wed, 27 Jul 2022 18:36:56 +0300 Subject: [PATCH] Use relative paths as file-relative when parsing a file (#6150) * Make function local (not used anywhere else) * Use path relative to the parsed file * Do not use real cwd at all --- Cargo.lock | 1 + crates/nu-command/src/system/nu_check.rs | 23 +++- crates/nu-command/tests/commands/nu_check.rs | 107 +++++++++++------ crates/nu-parser/src/parse_keywords.rs | 67 ++++++++++- crates/nu-protocol/Cargo.toml | 1 + crates/nu-protocol/src/engine/engine_state.rs | 11 ++ tests/parsing/mod.rs | 109 +++++++++++++++++- 7 files changed, 277 insertions(+), 42 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 67e40d92e9..23cbea65a9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2731,6 +2731,7 @@ dependencies = [ "indexmap", "miette 5.1.1", "nu-json", + "nu-path", "nu-utils", "num-format", "regex", diff --git a/crates/nu-command/src/system/nu_check.rs b/crates/nu-command/src/system/nu_check.rs index 02020859da..4a65746936 100644 --- a/crates/nu-command/src/system/nu_check.rs +++ b/crates/nu-command/src/system/nu_check.rs @@ -6,6 +6,8 @@ use nu_protocol::{ Category, Example, PipelineData, ShellError, Signature, Span, Spanned, SyntaxShape, Value, }; +use std::path::Path; + #[derive(Clone)] pub struct NuCheck; @@ -117,13 +119,30 @@ impl Command for NuCheck { )); } - if is_all { + // Change currently parsed directory + let prev_currently_parsed_cwd = if let Some(parent) = Path::new(&path).parent() + { + let prev = working_set.currently_parsed_cwd.clone(); + + working_set.currently_parsed_cwd = Some(parent.into()); + + prev + } else { + working_set.currently_parsed_cwd.clone() + }; + + let result = if is_all { heuristic_parse_file(path, &mut working_set, call, is_debug) } else if is_module { parse_file_module(path, &mut working_set, call, is_debug) } else { parse_file_script(path, &mut working_set, call, is_debug) - } + }; + + // Restore the currently parsed directory back + working_set.currently_parsed_cwd = prev_currently_parsed_cwd; + + result } else { Err(ShellError::GenericError( "Failed to execute command".to_string(), diff --git a/crates/nu-command/tests/commands/nu_check.rs b/crates/nu-command/tests/commands/nu_check.rs index 30063d2724..320430fcd4 100644 --- a/crates/nu-command/tests/commands/nu_check.rs +++ b/crates/nu-command/tests/commands/nu_check.rs @@ -260,7 +260,7 @@ fn parse_script_success_with_raw_stream() { let actual = nu!( cwd: dirs.test(), pipeline( r#" - open script.nu | nu-check + open script.nu | nu-check "# )); @@ -368,7 +368,7 @@ fn parse_script_success_with_complex_internal_stream() { # #Examples #grep-nu search file.txt - #ls **/* | some_filter | grep-nu search + #ls **/* | some_filter | grep-nu search #open file.txt | grep-nu search ] { if ($entrada | empty?) { @@ -380,11 +380,11 @@ fn parse_script_success_with_complex_internal_stream() { } else { grep -ihHn $search $entrada } - | lines + | lines | parse "{file}:{line}:{match}" | str trim - | update match {|f| - $f.match + | update match {|f| + $f.match | nu-highlight } | rename "source file" "line number" @@ -417,9 +417,9 @@ fn parse_script_failure_with_complex_internal_stream() { # #Examples #grep-nu search file.txt - #ls **/* | some_filter | grep-nu search + #ls **/* | some_filter | grep-nu search #open file.txt | grep-nu search - ] + ] if ($entrada | empty?) { if ($in | column? name) { grep -ihHn $search ($in | get name) @@ -429,11 +429,11 @@ fn parse_script_failure_with_complex_internal_stream() { } else { grep -ihHn $search $entrada } - | lines + | lines | parse "{file}:{line}:{match}" | str trim - | update match {|f| - $f.match + | update match {|f| + $f.match | nu-highlight } | rename "source file" "line number" @@ -466,7 +466,7 @@ fn parse_script_success_with_complex_external_stream() { # #Examples #grep-nu search file.txt - #ls **/* | some_filter | grep-nu search + #ls **/* | some_filter | grep-nu search #open file.txt | grep-nu search ] { if ($entrada | empty?) { @@ -478,11 +478,11 @@ fn parse_script_success_with_complex_external_stream() { } else { grep -ihHn $search $entrada } - | lines + | lines | parse "{file}:{line}:{match}" | str trim - | update match {|f| - $f.match + | update match {|f| + $f.match | nu-highlight } | rename "source file" "line number" @@ -515,7 +515,7 @@ fn parse_module_success_with_complex_external_stream() { # #Examples #grep-nu search file.txt - #ls **/* | some_filter | grep-nu search + #ls **/* | some_filter | grep-nu search #open file.txt | grep-nu search ] { if ($entrada | empty?) { @@ -527,11 +527,11 @@ fn parse_module_success_with_complex_external_stream() { } else { grep -ihHn $search $entrada } - | lines + | lines | parse "{file}:{line}:{match}" | str trim - | update match {|f| - $f.match + | update match {|f| + $f.match | nu-highlight } | rename "source file" "line number" @@ -564,7 +564,7 @@ fn parse_with_flag_all_success_for_complex_external_stream() { # #Examples #grep-nu search file.txt - #ls **/* | some_filter | grep-nu search + #ls **/* | some_filter | grep-nu search #open file.txt | grep-nu search ] { if ($entrada | empty?) { @@ -576,11 +576,11 @@ fn parse_with_flag_all_success_for_complex_external_stream() { } else { grep -ihHn $search $entrada } - | lines + | lines | parse "{file}:{line}:{match}" | str trim - | update match {|f| - $f.match + | update match {|f| + $f.match | nu-highlight } | rename "source file" "line number" @@ -592,7 +592,7 @@ fn parse_with_flag_all_success_for_complex_external_stream() { let actual = nu!( cwd: dirs.test(), pipeline( r#" - open grep.nu | nu-check -ad + open grep.nu | nu-check -ad "# )); @@ -607,13 +607,13 @@ fn parse_with_flag_all_failure_for_complex_external_stream() { "grep.nu", r#" #grep for nu - def grep-nu + def grep-nu search #search term entrada? #file or pipe # #Examples #grep-nu search file.txt - #ls **/* | some_filter | grep-nu search + #ls **/* | some_filter | grep-nu search #open file.txt | grep-nu search ] { if ($entrada | empty?) { @@ -625,11 +625,11 @@ fn parse_with_flag_all_failure_for_complex_external_stream() { } else { grep -ihHn $search $entrada } - | lines + | lines | parse "{file}:{line}:{match}" | str trim - | update match {|f| - $f.match + | update match {|f| + $f.match | nu-highlight } | rename "source file" "line number" @@ -641,7 +641,7 @@ fn parse_with_flag_all_failure_for_complex_external_stream() { let actual = nu!( cwd: dirs.test(), pipeline( r#" - open grep.nu | nu-check -ad + open grep.nu | nu-check -ad "# )); @@ -656,13 +656,13 @@ fn parse_with_flag_all_failure_for_complex_list_stream() { "grep.nu", r#" #grep for nu - def grep-nu + def grep-nu search #search term entrada? #file or pipe # #Examples #grep-nu search file.txt - #ls **/* | some_filter | grep-nu search + #ls **/* | some_filter | grep-nu search #open file.txt | grep-nu search ] { if ($entrada | empty?) { @@ -674,11 +674,11 @@ fn parse_with_flag_all_failure_for_complex_list_stream() { } else { grep -ihHn $search $entrada } - | lines + | lines | parse "{file}:{line}:{match}" | str trim - | update match {|f| - $f.match + | update match {|f| + $f.match | nu-highlight } | rename "source file" "line number" @@ -690,7 +690,7 @@ fn parse_with_flag_all_failure_for_complex_list_stream() { let actual = nu!( cwd: dirs.test(), pipeline( r#" - open grep.nu | lines | nu-check -ad + open grep.nu | lines | nu-check -ad "# )); @@ -724,3 +724,40 @@ fn parse_failure_due_conflicted_flags() { .contains("You cannot have both `--all` and `--as-module` on the same command line")); }) } + +#[test] +fn parse_script_with_nested_scripts_success() { + Playground::setup("nu_check_test_24", |dirs, sandbox| { + sandbox + .mkdir("lol") + .with_files(vec![FileWithContentToBeTrimmed( + "lol/lol.nu", + r#" + source ../foo.nu + use lol_shell.nu + overlay add ../lol/lol_shell.nu + "#, + )]) + .with_files(vec![FileWithContentToBeTrimmed( + "lol/lol_shell.nu", + r#" + export def ls [] { "lol" } + "#, + )]) + .with_files(vec![FileWithContentToBeTrimmed( + "foo.nu", + r#" + let-env FOO = 'foo' + "#, + )]); + + let actual = nu!( + cwd: dirs.test(), pipeline( + r#" + nu-check lol/lol.nu + "# + )); + + assert_eq!(actual.out, "true"); + }) +} diff --git a/crates/nu-parser/src/parse_keywords.rs b/crates/nu-parser/src/parse_keywords.rs index cedb202aa0..9dc548b79f 100644 --- a/crates/nu-parser/src/parse_keywords.rs +++ b/crates/nu-parser/src/parse_keywords.rs @@ -1333,6 +1333,7 @@ pub fn parse_use( let (module_filename, err) = unescape_unquote_string(&import_pattern.head.name, import_pattern.head.span); + if err.is_none() { if let Some(module_path) = find_in_dirs(&module_filename, working_set, &cwd, LIB_DIRS_ENV) @@ -1351,11 +1352,22 @@ pub fn parse_use( ); }; - if let Ok(contents) = std::fs::read(module_path) { + if let Ok(contents) = std::fs::read(&module_path) { let span_start = working_set.next_span_start(); working_set.add_file(module_filename, &contents); let span_end = working_set.next_span_start(); + // Change currently parsed directory + let prev_currently_parsed_cwd = if let Some(parent) = module_path.parent() { + let prev = working_set.currently_parsed_cwd.clone(); + + working_set.currently_parsed_cwd = Some(parent.into()); + + prev + } else { + working_set.currently_parsed_cwd.clone() + }; + let (block, module, err) = parse_module_block( working_set, Span::new(span_start, span_end), @@ -1363,6 +1375,9 @@ pub fn parse_use( ); error = error.or(err); + // Restore the currently parsed directory back + working_set.currently_parsed_cwd = prev_currently_parsed_cwd; + let _ = working_set.add_block(block); let module_id = working_set.add_module(&module_name, module.clone()); @@ -2069,11 +2084,22 @@ pub fn parse_overlay_add( ); }; - if let Ok(contents) = std::fs::read(module_path) { + if let Ok(contents) = std::fs::read(&module_path) { let span_start = working_set.next_span_start(); working_set.add_file(module_filename, &contents); let span_end = working_set.next_span_start(); + // Change currently parsed directory + let prev_currently_parsed_cwd = if let Some(parent) = module_path.parent() { + let prev = working_set.currently_parsed_cwd.clone(); + + working_set.currently_parsed_cwd = Some(parent.into()); + + prev + } else { + working_set.currently_parsed_cwd.clone() + }; + let (block, module, err) = parse_module_block( working_set, Span::new(span_start, span_end), @@ -2081,6 +2107,9 @@ pub fn parse_overlay_add( ); error = error.or(err); + // Restore the currently parsed directory back + working_set.currently_parsed_cwd = prev_currently_parsed_cwd; + let _ = working_set.add_block(block); let module_id = working_set.add_module(&overlay_name, module.clone()); @@ -2370,6 +2399,7 @@ pub fn parse_source( if name == b"source" { if let Some(decl_id) = working_set.find_decl(b"source", &Type::Any) { let cwd = working_set.get_cwd(); + // Is this the right call to be using here? // Some of the others (`parse_let`) use it, some of them (`parse_hide`) don't. let ParsedInternalCall { @@ -2401,9 +2431,21 @@ pub fn parse_source( if spans.len() >= 2 { let name_expr = working_set.get_span_contents(spans[1]); let (filename, err) = unescape_unquote_string(name_expr, spans[1]); + if err.is_none() { if let Some(path) = find_in_dirs(&filename, working_set, &cwd, LIB_DIRS_ENV) { if let Ok(contents) = std::fs::read(&path) { + // Change currently parsed directory + let prev_currently_parsed_cwd = if let Some(parent) = path.parent() { + let prev = working_set.currently_parsed_cwd.clone(); + + working_set.currently_parsed_cwd = Some(parent.into()); + + prev + } else { + working_set.currently_parsed_cwd.clone() + }; + // This will load the defs from the file into the // working set, if it was a successful parse. let (block, err) = parse( @@ -2414,6 +2456,9 @@ pub fn parse_source( expand_aliases_denylist, ); + // Restore the currently parsed directory back + working_set.currently_parsed_cwd = prev_currently_parsed_cwd; + if err.is_some() { // Unsuccessful parse of file return ( @@ -2695,13 +2740,27 @@ pub fn parse_register( ) } -pub fn find_in_dirs( +/// This helper function is used to find files during parsing +/// +/// Checks whether the file is: +/// 1. relative to the directory of the file currently being parsed +/// 2. relative to the current working directory +/// 3. within one of the NU_LIB_DIRS entries +/// +/// Always returns absolute path +fn find_in_dirs( filename: &str, working_set: &StateWorkingSet, cwd: &str, dirs_env: &str, ) -> Option { - if let Ok(p) = canonicalize_with(filename, cwd) { + if let Some(currently_parsed_cwd) = &working_set.currently_parsed_cwd { + if let Ok(p) = canonicalize_with(filename, currently_parsed_cwd) { + Some(p) + } else { + None + } + } else if let Ok(p) = canonicalize_with(filename, cwd) { Some(p) } else { let path = Path::new(filename); diff --git a/crates/nu-protocol/Cargo.toml b/crates/nu-protocol/Cargo.toml index af14f39590..3f1acb129f 100644 --- a/crates/nu-protocol/Cargo.toml +++ b/crates/nu-protocol/Cargo.toml @@ -10,6 +10,7 @@ version = "0.66.1" [dependencies] nu-utils = { path = "../nu-utils", version = "0.66.1" } +nu-path = { path = "../nu-path", version = "0.66.1" } thiserror = "1.0.31" miette = { version = "5.1.0", features = ["fancy"] } serde = {version = "1.0.130", features = ["derive"]} diff --git a/crates/nu-protocol/src/engine/engine_state.rs b/crates/nu-protocol/src/engine/engine_state.rs index a9b3e3122c..6ebf7146f5 100644 --- a/crates/nu-protocol/src/engine/engine_state.rs +++ b/crates/nu-protocol/src/engine/engine_state.rs @@ -729,6 +729,14 @@ impl EngineState { self.num_files() - 1 } + pub fn get_cwd(&self) -> Option { + if let Some(pwd_value) = self.get_env_var(PWD_ENV) { + pwd_value.as_string().ok() + } else { + None + } + } + #[cfg(not(windows))] pub fn get_sig_quit(&self) -> &Option> { &self.sig_quit @@ -755,6 +763,8 @@ pub struct StateWorkingSet<'a> { pub delta: StateDelta, pub external_commands: Vec>, pub type_scope: TypeScope, + /// Current working directory relative to the file being parsed right now + pub currently_parsed_cwd: Option, } /// A temporary placeholder for expression types. It is used to keep track of the input types @@ -927,6 +937,7 @@ impl<'a> StateWorkingSet<'a> { permanent_state, external_commands: vec![], type_scope: TypeScope::default(), + currently_parsed_cwd: None, } } diff --git a/tests/parsing/mod.rs b/tests/parsing/mod.rs index 9915b76242..af70cad23e 100644 --- a/tests/parsing/mod.rs +++ b/tests/parsing/mod.rs @@ -1,4 +1,6 @@ -use nu_test_support::nu; +use nu_test_support::fs::Stub::FileWithContentToBeTrimmed; +use nu_test_support::playground::Playground; +use nu_test_support::{nu, pipeline}; #[test] fn run_nu_script_single_line() { @@ -44,3 +46,108 @@ fn run_nu_script_multiline_end_pipe_win() { assert_eq!(actual.out, "3"); } + +#[test] +fn parse_file_relative_to_parsed_file() { + Playground::setup("relative_files", |dirs, sandbox| { + sandbox + .mkdir("lol") + .mkdir("lol/lol") + .with_files(vec![FileWithContentToBeTrimmed( + "lol/lol/lol.nu", + r#" + source ../../foo.nu + use ../lol_shell.nu + overlay add ../../lol/lol_shell.nu + + $'($env.FOO) (lol_shell ls) (ls)' + "#, + )]) + .with_files(vec![FileWithContentToBeTrimmed( + "lol/lol_shell.nu", + r#" + export def ls [] { "lol" } + "#, + )]) + .with_files(vec![FileWithContentToBeTrimmed( + "foo.nu", + r#" + let-env FOO = 'foo' + "#, + )]); + + let actual = nu!( + cwd: dirs.test(), pipeline( + r#" + source lol/lol/lol.nu + "# + )); + + assert_eq!(actual.out, "foo lol lol"); + }) +} + +#[test] +fn parse_file_relative_to_parsed_file_dont_use_cwd_1() { + Playground::setup("relative_files", |dirs, sandbox| { + sandbox + .mkdir("lol") + .with_files(vec![FileWithContentToBeTrimmed( + "lol/lol.nu", + r#" + source foo.nu + "#, + )]) + .with_files(vec![FileWithContentToBeTrimmed( + "lol/foo.nu", + r#" + let-env FOO = 'good' + "#, + )]) + .with_files(vec![FileWithContentToBeTrimmed( + "foo.nu", + r#" + let-env FOO = 'bad' + "#, + )]); + + let actual = nu!( + cwd: dirs.test(), pipeline( + r#" + source lol/lol.nu; + $env.FOO + "# + )); + + assert_eq!(actual.out, "good"); + }) +} + +#[test] +fn parse_file_relative_to_parsed_file_dont_use_cwd_2() { + Playground::setup("relative_files", |dirs, sandbox| { + sandbox + .mkdir("lol") + .with_files(vec![FileWithContentToBeTrimmed( + "lol/lol.nu", + r#" + source foo.nu + "#, + )]) + .with_files(vec![FileWithContentToBeTrimmed( + "foo.nu", + r#" + let-env FOO = 'bad' + "#, + )]); + + let actual = nu!( + cwd: dirs.test(), pipeline( + r#" + source lol/lol.nu + "# + )); + + assert!(actual.err.contains("File not found")); + }) +}