diff --git a/crates/nu-cli/src/config_files.rs b/crates/nu-cli/src/config_files.rs index 2b0c46cb04..3519dfc711 100644 --- a/crates/nu-cli/src/config_files.rs +++ b/crates/nu-cli/src/config_files.rs @@ -108,9 +108,9 @@ pub fn eval_config_contents( let config_filename = config_path.to_string_lossy(); if let Ok(contents) = std::fs::read(&config_path) { - // Change currently parsed directory - let prev_currently_parsed_cwd = engine_state.currently_parsed_cwd.clone(); - engine_state.start_in_file(Some(&config_filename)); + // Set the current active file to the config file. + let prev_file = engine_state.file.take(); + engine_state.file = Some(config_path.clone()); eval_source( engine_state, @@ -121,8 +121,8 @@ pub fn eval_config_contents( false, ); - // Restore the currently parsed directory back - engine_state.currently_parsed_cwd = prev_currently_parsed_cwd; + // Restore the current active file. + engine_state.file = prev_file; // Merge the environment in case env vars changed in the config match nu_engine::env::current_dir(engine_state, stack) { diff --git a/crates/nu-cli/src/eval_file.rs b/crates/nu-cli/src/eval_file.rs index f69b14113e..d4061f33d6 100644 --- a/crates/nu-cli/src/eval_file.rs +++ b/crates/nu-cli/src/eval_file.rs @@ -13,7 +13,10 @@ use nu_protocol::{ use nu_utils::stdout_write_all_and_flush; use std::sync::Arc; -/// Main function used when a file path is found as argument for nu +/// Entry point for evaluating a file. +/// +/// If the file contains a main command, it is invoked with `args` and the pipeline data from `input`; +/// otherwise, the pipeline data is forwarded to the first command in the file, and `args` are ignored. pub fn evaluate_file( path: String, args: &[String], @@ -21,7 +24,7 @@ pub fn evaluate_file( stack: &mut Stack, input: PipelineData, ) -> Result<()> { - // Translate environment variables from Strings to Values + // Convert environment variables from Strings to Values and store them in the engine state. if let Some(e) = convert_env_values(engine_state, stack) { let working_set = StateWorkingSet::new(engine_state); report_error(&working_set, &e); @@ -74,8 +77,7 @@ pub fn evaluate_file( ); std::process::exit(1); }); - - engine_state.start_in_file(Some(file_path_str)); + engine_state.file = Some(file_path.clone()); let parent = file_path.parent().unwrap_or_else(|| { let working_set = StateWorkingSet::new(engine_state); @@ -104,17 +106,19 @@ pub fn evaluate_file( let source_filename = file_path .file_name() - .expect("internal error: script missing filename"); + .expect("internal error: missing filename"); let mut working_set = StateWorkingSet::new(engine_state); trace!("parsing file: {}", file_path_str); let block = parse(&mut working_set, Some(file_path_str), &file, false); + // If any parse errors were found, report the first error and exit. if let Some(err) = working_set.parse_errors.first() { report_error(&working_set, err); std::process::exit(1); } + // Look for blocks whose name starts with "main" and replace it with the filename. for block in working_set.delta.blocks.iter_mut().map(Arc::make_mut) { if block.signature.name == "main" { block.signature.name = source_filename.to_string_lossy().to_string(); @@ -124,19 +128,21 @@ pub fn evaluate_file( } } - let _ = engine_state.merge_delta(working_set.delta); + // Merge the changes into the engine state. + engine_state + .merge_delta(working_set.delta) + .expect("merging delta into engine_state should succeed"); + // Check if the file contains a main command. if engine_state.find_decl(b"main", &[]).is_some() { - let args = format!("main {}", args.join(" ")); - + // Evaluate the file, but don't run main yet. let pipeline_data = eval_block::(engine_state, stack, &block, PipelineData::empty()); let pipeline_data = match pipeline_data { Err(ShellError::Return { .. }) => { - // allows early exists before `main` is run. + // Allow early return before main is run. return Ok(()); } - x => x, } .unwrap_or_else(|e| { @@ -145,12 +151,12 @@ pub fn evaluate_file( std::process::exit(1); }); + // Print the pipeline output of the file. + // The pipeline output of a file is the pipeline output of its last command. let result = pipeline_data.print(engine_state, stack, true, false); - match result { Err(err) => { let working_set = StateWorkingSet::new(engine_state); - report_error(&working_set, &err); std::process::exit(1); } @@ -161,6 +167,9 @@ pub fn evaluate_file( } } + // Invoke the main command with arguments. + // Arguments with whitespaces are quoted, thus can be safely concatenated by whitespace. + let args = format!("main {}", args.join(" ")); if !eval_source( engine_state, stack, diff --git a/crates/nu-command/src/system/nu_check.rs b/crates/nu-command/src/system/nu_check.rs index 8876e82c08..260a1c7a59 100644 --- a/crates/nu-command/src/system/nu_check.rs +++ b/crates/nu-command/src/system/nu_check.rs @@ -1,6 +1,6 @@ use nu_engine::{command_prelude::*, env::get_config, find_in_dirs_env, get_dirs_var_from_call}; use nu_parser::{parse, parse_module_block, parse_module_file_or_dir, unescape_unquote_string}; -use nu_protocol::engine::StateWorkingSet; +use nu_protocol::engine::{FileStack, StateWorkingSet}; use std::path::Path; #[derive(Clone)] @@ -112,15 +112,6 @@ impl Command for NuCheck { Err(error) => return Err(error), }; - // 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() - }; - let result = if as_module || path.is_dir() { parse_file_or_dir_module( path.to_string_lossy().as_bytes(), @@ -130,12 +121,13 @@ impl Command for NuCheck { call.head, ) } else { + // Unlike `parse_file_or_dir_module`, `parse_file_script` parses the content directly, + // without adding the file to the stack. Therefore we need to handle this manually. + working_set.files = FileStack::with_file(path.clone()); parse_file_script(&path, &mut working_set, is_debug, path_span, call.head) + // The working set is not merged, so no need to pop the file from the stack. }; - // Restore the currently parsed directory back - working_set.currently_parsed_cwd = prev_currently_parsed_cwd; - result } else { Err(ShellError::GenericError { diff --git a/crates/nu-lsp/src/lib.rs b/crates/nu-lsp/src/lib.rs index 6c347ff3d4..adee58349a 100644 --- a/crates/nu-lsp/src/lib.rs +++ b/crates/nu-lsp/src/lib.rs @@ -250,8 +250,7 @@ impl LanguageServer { ) -> Option<(&Rope, &PathBuf, StateWorkingSet<'a>)> { let (file, path) = self.rope(file_url)?; - // TODO: AsPath thingy - engine_state.start_in_file(Some(&path.to_string_lossy())); + engine_state.file = Some(path.to_owned()); let working_set = StateWorkingSet::new(engine_state); diff --git a/crates/nu-parser/src/parse_keywords.rs b/crates/nu-parser/src/parse_keywords.rs index 9ca3cba470..7fece081a5 100644 --- a/crates/nu-parser/src/parse_keywords.rs +++ b/crates/nu-parser/src/parse_keywords.rs @@ -1871,32 +1871,16 @@ pub fn parse_module_block( (block, module, module_comments) } +/// Parse a module from a file. +/// +/// The module name is inferred from the stem of the file, unless specified in `name_override`. fn parse_module_file( working_set: &mut StateWorkingSet, path: ParserPath, path_span: Span, name_override: Option, ) -> Option { - if let Some(i) = working_set - .parsed_module_files - .iter() - .rposition(|p| p == path.path()) - { - let mut files: Vec = working_set - .parsed_module_files - .split_off(i) - .iter() - .map(|p| p.to_string_lossy().to_string()) - .collect(); - - files.push(path.path().to_string_lossy().to_string()); - - let msg = files.join("\nuses "); - - working_set.error(ParseError::CyclicalModuleImport(msg, path_span)); - return None; - } - + // Infer the module name from the stem of the file, unless overridden. let module_name = if let Some(name) = name_override { name } else if let Some(stem) = path.file_stem() { @@ -1909,6 +1893,7 @@ fn parse_module_file( return None; }; + // Read the content of the module. let contents = if let Some(contents) = path.read(working_set) { contents } else { @@ -1922,29 +1907,23 @@ fn parse_module_file( let file_id = working_set.add_file(path.path().to_string_lossy().to_string(), &contents); let new_span = working_set.get_span_for_file(file_id); + // Check if we've parsed the module before. if let Some(module_id) = working_set.find_module_by_span(new_span) { return Some(module_id); } - // Change the currently parsed directory - let prev_currently_parsed_cwd = if let Some(parent) = path.parent() { - working_set.currently_parsed_cwd.replace(parent.into()) - } else { - working_set.currently_parsed_cwd.clone() - }; - - // Add the file to the stack of parsed module files - working_set.parsed_module_files.push(path.path_buf()); + // Add the file to the stack of files being processed. + if let Err(e) = working_set.files.push(path.path_buf(), path_span) { + working_set.error(e); + return None; + } // Parse the module let (block, module, module_comments) = parse_module_block(working_set, new_span, module_name.as_bytes()); - // Remove the file from the stack of parsed module files - working_set.parsed_module_files.pop(); - - // Restore the currently parsed directory back - working_set.currently_parsed_cwd = prev_currently_parsed_cwd; + // Remove the file from the stack of files being processed. + working_set.files.pop(); let _ = working_set.add_block(Arc::new(block)); let module_id = working_set.add_module(&module_name, module, module_comments); @@ -3425,12 +3404,11 @@ pub fn parse_source(working_set: &mut StateWorkingSet, lite_command: &LiteComman if let Some(path) = find_in_dirs(&filename, working_set, &cwd, LIB_DIRS_VAR) { if let Some(contents) = path.read(working_set) { - // Change currently parsed directory - let prev_currently_parsed_cwd = if let Some(parent) = path.parent() { - working_set.currently_parsed_cwd.replace(parent.into()) - } else { - working_set.currently_parsed_cwd.clone() - }; + // Add the file to the stack of files being processed. + if let Err(e) = working_set.files.push(path.clone().path_buf(), spans[1]) { + working_set.error(e); + return garbage_pipeline(spans); + } // This will load the defs from the file into the // working set, if it was a successful parse. @@ -3441,8 +3419,8 @@ pub fn parse_source(working_set: &mut StateWorkingSet, lite_command: &LiteComman scoped, ); - // Restore the currently parsed directory back - working_set.currently_parsed_cwd = prev_currently_parsed_cwd; + // Remove the file from the stack of files being processed. + working_set.files.pop(); // Save the block into the working set let block_id = working_set.add_block(block); @@ -3831,11 +3809,10 @@ pub fn find_in_dirs( dirs_var_name: &str, ) -> Option { // Choose whether to use file-relative or PWD-relative path - let actual_cwd = if let Some(currently_parsed_cwd) = &working_set.currently_parsed_cwd { - currently_parsed_cwd.as_path() - } else { - Path::new(cwd) - }; + let actual_cwd = working_set + .files + .current_working_directory() + .unwrap_or(Path::new(cwd)); // Try if we have an existing virtual path if let Some(virtual_path) = working_set.find_virtual_path(filename) { @@ -3895,11 +3872,10 @@ pub fn find_in_dirs( dirs_env: &str, ) -> Option { // Choose whether to use file-relative or PWD-relative path - let actual_cwd = if let Some(currently_parsed_cwd) = &working_set.currently_parsed_cwd { - currently_parsed_cwd.as_path() - } else { - Path::new(cwd) - }; + let actual_cwd = working_set + .files + .current_working_directory() + .unwrap_or(Path::new(cwd)); if let Ok(p) = canonicalize_with(filename, actual_cwd) { Some(p) diff --git a/crates/nu-protocol/src/engine/engine_state.rs b/crates/nu-protocol/src/engine/engine_state.rs index 82af39d093..17a4ccb5eb 100644 --- a/crates/nu-protocol/src/engine/engine_state.rs +++ b/crates/nu-protocol/src/engine/engine_state.rs @@ -98,8 +98,8 @@ pub struct EngineState { config_path: HashMap, pub history_enabled: bool, pub history_session_id: i64, - // If Nushell was started, e.g., with `nu spam.nu`, the file's parent is stored here - pub currently_parsed_cwd: Option, + // Path to the file Nushell is currently evaluating, or None if we're in an interactive session. + pub file: Option, pub regex_cache: Arc>>, pub is_interactive: bool, pub is_login: bool, @@ -161,7 +161,7 @@ impl EngineState { config_path: HashMap::new(), history_enabled: true, history_session_id: 0, - currently_parsed_cwd: None, + file: None, regex_cache: Arc::new(Mutex::new(LruCache::new( NonZeroUsize::new(REGEX_CACHE_SIZE).expect("tried to create cache of size zero"), ))), @@ -322,15 +322,6 @@ impl EngineState { Ok(()) } - /// Mark a starting point if it is a script (e.g., nu spam.nu) - pub fn start_in_file(&mut self, file_path: Option<&str>) { - self.currently_parsed_cwd = if let Some(path) = file_path { - Path::new(path).parent().map(PathBuf::from) - } else { - None - }; - } - pub fn has_overlay(&self, name: &[u8]) -> bool { self.scope .overlays diff --git a/crates/nu-protocol/src/engine/state_working_set.rs b/crates/nu-protocol/src/engine/state_working_set.rs index b784b042c6..4340521224 100644 --- a/crates/nu-protocol/src/engine/state_working_set.rs +++ b/crates/nu-protocol/src/engine/state_working_set.rs @@ -10,7 +10,7 @@ use crate::{ use core::panic; use std::{ collections::{HashMap, HashSet}, - path::PathBuf, + path::{Path, PathBuf}, sync::Arc, }; @@ -26,10 +26,7 @@ pub struct StateWorkingSet<'a> { pub permanent_state: &'a EngineState, pub delta: StateDelta, pub external_commands: Vec>, - /// Current working directory relative to the file being parsed right now - pub currently_parsed_cwd: Option, - /// All previously parsed module files. Used to protect against circular imports. - pub parsed_module_files: Vec, + pub files: FileStack, /// Whether or not predeclarations are searched when looking up a command (used with aliases) pub search_predecls: bool, pub parse_errors: Vec, @@ -38,12 +35,18 @@ pub struct StateWorkingSet<'a> { impl<'a> StateWorkingSet<'a> { pub fn new(permanent_state: &'a EngineState) -> Self { + // Initialize the file stack with the top-level file. + let files = if let Some(file) = permanent_state.file.clone() { + FileStack::with_file(file) + } else { + FileStack::new() + }; + Self { delta: StateDelta::new(permanent_state), permanent_state, external_commands: vec![], - currently_parsed_cwd: permanent_state.currently_parsed_cwd.clone(), - parsed_module_files: vec![], + files, search_predecls: true, parse_errors: vec![], parse_warnings: vec![], @@ -1100,3 +1103,65 @@ impl<'a> miette::SourceCode for &StateWorkingSet<'a> { Err(miette::MietteError::OutOfBounds) } } + +/// Files being evaluated, arranged as a stack. +/// +/// The current active file is on the top of the stack. +/// When a file source/import another file, the new file is pushed onto the stack. +/// Attempting to add files that are already in the stack (circular import) results in an error. +/// +/// Note that file paths are compared without canonicalization, so the same +/// physical file may still appear multiple times under different paths. +/// This doesn't affect circular import detection though. +#[derive(Debug, Default)] +pub struct FileStack(Vec); + +impl FileStack { + /// Creates an empty stack. + pub fn new() -> Self { + Self(vec![]) + } + + /// Creates a stack with a single file on top. + /// + /// This is a convenience method that creates an empty stack, then pushes the file onto it. + /// It skips the circular import check and always succeeds. + pub fn with_file(path: PathBuf) -> Self { + Self(vec![path]) + } + + /// Adds a file to the stack. + /// + /// If the same file is already present in the stack, returns `ParseError::CircularImport`. + pub fn push(&mut self, path: PathBuf, span: Span) -> Result<(), ParseError> { + // Check for circular import. + if let Some(i) = self.0.iter().rposition(|p| p == &path) { + let filenames: Vec = self.0[i..] + .iter() + .chain(std::iter::once(&path)) + .map(|p| p.to_string_lossy().to_string()) + .collect(); + let msg = filenames.join("\nuses "); + return Err(ParseError::CircularImport(msg, span)); + } + + self.0.push(path); + Ok(()) + } + + /// Removes a file from the stack and returns its path, or None if the stack is empty. + pub fn pop(&mut self) -> Option { + self.0.pop() + } + + /// Returns the active file (that is, the file on the top of the stack), or None if the stack is empty. + pub fn top(&self) -> Option<&Path> { + self.0.last().map(PathBuf::as_path) + } + + /// Returns the parent directory of the active file, or None if the stack is empty + /// or the active file doesn't have a parent directory as part of its path. + pub fn current_working_directory(&self) -> Option<&Path> { + self.0.last().and_then(|path| path.parent()) + } +} diff --git a/crates/nu-protocol/src/errors/parse_error.rs b/crates/nu-protocol/src/errors/parse_error.rs index 80ff2d5154..d9928df58f 100644 --- a/crates/nu-protocol/src/errors/parse_error.rs +++ b/crates/nu-protocol/src/errors/parse_error.rs @@ -227,9 +227,9 @@ pub enum ParseError { #[label = "module directory is missing a mod.nu file"] Span, ), - #[error("Cyclical module import.")] - #[diagnostic(code(nu::parser::cyclical_module_import), help("{0}"))] - CyclicalModuleImport(String, #[label = "detected cyclical module import"] Span), + #[error("Circular import.")] + #[diagnostic(code(nu::parser::circular_import), help("{0}"))] + CircularImport(String, #[label = "detected circular import"] Span), #[error("Can't export {0} named same as the module.")] #[diagnostic( @@ -506,7 +506,7 @@ impl ParseError { ParseError::NamedAsModule(_, _, _, s) => *s, ParseError::ModuleDoubleMain(_, s) => *s, ParseError::ExportMainAliasNotAllowed(s) => *s, - ParseError::CyclicalModuleImport(_, s) => *s, + ParseError::CircularImport(_, s) => *s, ParseError::ModuleOrOverlayNotFound(s) => *s, ParseError::ActiveOverlayNotFound(s) => *s, ParseError::OverlayPrefixMismatch(_, _, s) => *s, diff --git a/crates/nu-protocol/src/eval_const.rs b/crates/nu-protocol/src/eval_const.rs index 724a1c0841..ff57ac9e85 100644 --- a/crates/nu-protocol/src/eval_const.rs +++ b/crates/nu-protocol/src/eval_const.rs @@ -11,6 +11,7 @@ use std::{ path::{Path, PathBuf}, }; +/// Create a Value for `$nu`. pub fn create_nu_constant(engine_state: &EngineState, span: Span) -> Result { fn canonicalize_path(engine_state: &EngineState, path: &Path) -> PathBuf { let cwd = engine_state.current_work_dir(); diff --git a/crates/nu-std/src/lib.rs b/crates/nu-std/src/lib.rs index 9389d6a0d0..fea5cc2f2a 100644 --- a/crates/nu-std/src/lib.rs +++ b/crates/nu-std/src/lib.rs @@ -3,7 +3,7 @@ use nu_engine::{env::current_dir, eval_block}; use nu_parser::parse; use nu_protocol::{ debugger::WithoutDebug, - engine::{Stack, StateWorkingSet, VirtualPath}, + engine::{FileStack, Stack, StateWorkingSet, VirtualPath}, report_error, PipelineData, }; use std::path::PathBuf; @@ -50,10 +50,9 @@ pub fn load_standard_library( } let std_dir = std_dir.to_string_lossy().to_string(); - let source = format!( - r#" + let source = r#" # Define the `std` module -module {std_dir} +module std # Prelude use std dirs [ @@ -65,14 +64,14 @@ use std dirs [ dexit ] use std pwd -"# - ); +"#; let _ = working_set.add_virtual_path(std_dir, VirtualPath::Dir(std_virt_paths)); - // Change the currently parsed directory - let prev_currently_parsed_cwd = working_set.currently_parsed_cwd.clone(); - working_set.currently_parsed_cwd = Some(PathBuf::from(NU_STDLIB_VIRTUAL_DIR)); + // Add a placeholder file to the stack of files being evaluated. + // The name of this file doesn't matter; it's only there to set the current working directory to NU_STDLIB_VIRTUAL_DIR. + let placeholder = PathBuf::from(NU_STDLIB_VIRTUAL_DIR).join("loading stdlib"); + working_set.files = FileStack::with_file(placeholder); let block = parse( &mut working_set, @@ -81,13 +80,13 @@ use std pwd false, ); + // Remove the placeholder file from the stack of files being evaluated. + working_set.files.pop(); + if let Some(err) = working_set.parse_errors.first() { report_error(&working_set, err); } - // Restore the currently parsed directory back - working_set.currently_parsed_cwd = prev_currently_parsed_cwd; - (block, working_set.render()) }; diff --git a/src/ide.rs b/src/ide.rs index f65fb88218..8e0a60421b 100644 --- a/src/ide.rs +++ b/src/ide.rs @@ -8,7 +8,7 @@ use nu_protocol::{ }; use reedline::Completer; use serde_json::{json, Value as JsonValue}; -use std::sync::Arc; +use std::{path::PathBuf, sync::Arc}; #[derive(Debug)] enum Id { @@ -67,7 +67,7 @@ fn read_in_file<'a>( std::process::exit(1); }); - engine_state.start_in_file(Some(file_path)); + engine_state.file = Some(PathBuf::from(file_path)); let working_set = StateWorkingSet::new(engine_state); diff --git a/tests/parsing/mod.rs b/tests/parsing/mod.rs index e7c21d1ee6..3bbed9b7c4 100644 --- a/tests/parsing/mod.rs +++ b/tests/parsing/mod.rs @@ -33,6 +33,15 @@ fn source_const_file() { assert_eq!(actual.out, "5"); } +#[test] +fn source_circular() { + let actual = nu!(cwd: "tests/parsing/samples", " + nu source_circular_1.nu + "); + + assert!(actual.err.contains("nu::parser::circular_import")); +} + #[test] fn run_nu_script_single_line() { let actual = nu!(cwd: "tests/parsing/samples", " diff --git a/tests/parsing/samples/source_circular_1.nu b/tests/parsing/samples/source_circular_1.nu new file mode 100644 index 0000000000..6779904900 --- /dev/null +++ b/tests/parsing/samples/source_circular_1.nu @@ -0,0 +1 @@ +source source_circular_2.nu diff --git a/tests/parsing/samples/source_circular_2.nu b/tests/parsing/samples/source_circular_2.nu new file mode 100644 index 0000000000..8d2e131a09 --- /dev/null +++ b/tests/parsing/samples/source_circular_2.nu @@ -0,0 +1 @@ +source source_circular_1.nu