From 90d65bb987e9dc02b7d6da6e7116678ab44e0980 Mon Sep 17 00:00:00 2001 From: Yash Thakur <45539777+ysthakur@users.noreply.github.com> Date: Mon, 22 Jan 2024 02:13:48 -0500 Subject: [PATCH] Evaluate string interpolation at parse time (#11562) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #11561 # Description This PR will allow string interpolation at parse time. Since the actual config hasn't been loaded at parse time, this uses the `get_config()` method on `StateWorkingSet`. So file sizes and datetimes (I think those are the only things whose string representations depend on the config) may be formatted differently from how users have configured things, which may come as a surprise to some. It does seem unlikely that anyone would be formatting file sizes or date times at parse time. Still, something to think about if/before this PR merged. Also, I changed the `ModuleNotFound` error to include the name of the module. # User-Facing Changes Users will be able to do stuff like: ```nu const x = [1 2 3] const y = $"foo($x)" // foo[1, 2, 3] ``` The main use case is `use`-ing and `source`-ing files at parse time: ```nu const file = "foo.nu" use $"($file)" ``` If the module isn't found, you'll see an error like this: ``` Error: nu::parser::module_not_found × Module not found. ╭─[entry #3:1:1] 1 │ use $"($file)" · ─────┬──── · ╰── module foo.nu not found ╰──── help: module files and their paths must be available before your script is run as parsing occurs before anything is evaluated ``` # Tests + Formatting # After Submitting Although there's user-facing changes, there's probably no need to change the docs since people probably already expect string interpolation to work at parse time. Edit: @kubouch pointed out that we'd need to document the fact that stuff like file sizes and datetimes won't get formatted according to user's runtime configs, so I'll make a PR to nushell.github.io after this one --- crates/nu-engine/src/eval.rs | 36 ++++++---------------- crates/nu-parser/src/parse_keywords.rs | 42 ++++++++++++++++++++------ crates/nu-protocol/src/eval_base.rs | 26 ++++++++++------ crates/nu-protocol/src/eval_const.rs | 20 ++++++------ crates/nu-protocol/src/parse_error.rs | 4 +-- tests/const_/mod.rs | 13 ++++++++ tests/modules/mod.rs | 2 +- 7 files changed, 84 insertions(+), 59 deletions(-) diff --git a/crates/nu-engine/src/eval.rs b/crates/nu-engine/src/eval.rs index 71ce05ad3d..68add01f37 100644 --- a/crates/nu-engine/src/eval.rs +++ b/crates/nu-engine/src/eval.rs @@ -1,4 +1,4 @@ -use crate::{current_dir_str, get_full_help}; +use crate::{current_dir_str, get_config, get_full_help}; use nu_path::expand_path_with; use nu_protocol::{ ast::{ @@ -7,11 +7,11 @@ use nu_protocol::{ }, engine::{Closure, EngineState, Stack}, eval_base::Eval, - DeclId, IntoInterruptiblePipelineData, IntoPipelineData, PipelineData, ShellError, Span, - Spanned, Type, Value, VarId, ENV_VARIABLE_ID, + Config, DeclId, IntoPipelineData, PipelineData, ShellError, Span, Spanned, Type, Value, VarId, + ENV_VARIABLE_ID, }; -use std::collections::HashMap; use std::thread::{self, JoinHandle}; +use std::{borrow::Cow, collections::HashMap}; pub fn eval_call( engine_state: &EngineState, @@ -913,9 +913,13 @@ impl Eval for EvalRuntime { type MutState = Stack; + fn get_config<'a>(engine_state: Self::State<'a>, stack: &mut Stack) -> Cow<'a, Config> { + Cow::Owned(get_config(engine_state, stack)) + } + fn eval_filepath( - engine_state: Self::State<'_>, - stack: &mut Self::MutState, + engine_state: &EngineState, + stack: &mut Stack, path: String, quoted: bool, span: Span, @@ -1141,26 +1145,6 @@ impl Eval for EvalRuntime { Ok(Value::closure(Closure { block_id, captures }, span)) } - fn eval_string_interpolation( - engine_state: &EngineState, - stack: &mut Stack, - exprs: &[Expression], - span: Span, - ) -> Result { - let mut parts = vec![]; - for expr in exprs { - parts.push(eval_expression(engine_state, stack, expr)?); - } - - let config = engine_state.get_config(); - - parts - .into_iter() - .into_pipeline_data(None) - .collect_string("", config) - .map(|x| Value::string(x, span)) - } - fn eval_overlay(engine_state: &EngineState, span: Span) -> Result { let name = String::from_utf8_lossy(engine_state.get_span_contents(span)).to_string(); diff --git a/crates/nu-parser/src/parse_keywords.rs b/crates/nu-parser/src/parse_keywords.rs index 657b667a15..fd277153fd 100644 --- a/crates/nu-parser/src/parse_keywords.rs +++ b/crates/nu-parser/src/parse_keywords.rs @@ -1969,14 +1969,20 @@ fn parse_module_file( } else if let Some(stem) = path.file_stem() { stem.to_string_lossy().to_string() } else { - working_set.error(ParseError::ModuleNotFound(path_span)); + working_set.error(ParseError::ModuleNotFound( + path_span, + path.path().to_string_lossy().to_string(), + )); return None; }; let contents = if let Some(contents) = path.read(working_set) { contents } else { - working_set.error(ParseError::ModuleNotFound(path_span)); + working_set.error(ParseError::ModuleNotFound( + path_span, + path.path().to_string_lossy().to_string(), + )); return None; }; @@ -2031,20 +2037,26 @@ pub fn parse_module_file_or_dir( if let Some(path) = find_in_dirs(&module_path_str, working_set, &cwd, LIB_DIRS_VAR) { path } else { - working_set.error(ParseError::ModuleNotFound(path_span)); + working_set.error(ParseError::ModuleNotFound(path_span, module_path_str)); return None; }; if module_path.is_dir() { if module_path.read_dir().is_none() { - working_set.error(ParseError::ModuleNotFound(path_span)); + working_set.error(ParseError::ModuleNotFound( + path_span, + module_path.path().to_string_lossy().to_string(), + )); return None; }; let module_name = if let Some(stem) = module_path.file_stem() { stem.to_string_lossy().to_string() } else { - working_set.error(ParseError::ModuleNotFound(path_span)); + working_set.error(ParseError::ModuleNotFound( + path_span, + module_path.path().to_string_lossy().to_string(), + )); return None; }; @@ -2084,7 +2096,10 @@ pub fn parse_module_file_or_dir( } else if module_path.is_file() { parse_module_file(working_set, module_path, path_span, name_override) } else { - working_set.error(ParseError::ModuleNotFound(path_span)); + working_set.error(ParseError::ModuleNotFound( + path_span, + module_path.path().to_string_lossy().to_string(), + )); None } } @@ -2200,7 +2215,10 @@ pub fn parse_module( ) { return (pipeline, Some(module_id)); } else { - working_set.error(ParseError::ModuleNotFound(module_name_or_path_span)); + working_set.error(ParseError::ModuleNotFound( + module_name_or_path_span, + module_name_or_path, + )); return (pipeline, None); } } @@ -2397,7 +2415,10 @@ pub fn parse_use(working_set: &mut StateWorkingSet, spans: &[Span]) -> (Pipeline module_id, ) } else { - working_set.error(ParseError::ModuleNotFound(import_pattern.head.span)); + working_set.error(ParseError::ModuleNotFound( + import_pattern.head.span, + String::from_utf8_lossy(&import_pattern.head.name).to_string(), + )); return ( Pipeline::from_vec(vec![Expression { expr: Expr::Call(call), @@ -2567,7 +2588,10 @@ pub fn parse_hide(working_set: &mut StateWorkingSet, spans: &[Span]) -> Pipeline (false, Module::new(b"tmp".to_vec())) } } else { - working_set.error(ParseError::ModuleNotFound(spans[1])); + working_set.error(ParseError::ModuleNotFound( + spans[1], + String::from_utf8_lossy(&import_pattern.head.name).to_string(), + )); return garbage_pipeline(spans); }; diff --git a/crates/nu-protocol/src/eval_base.rs b/crates/nu-protocol/src/eval_base.rs index a96fcb085e..ca01f00056 100644 --- a/crates/nu-protocol/src/eval_base.rs +++ b/crates/nu-protocol/src/eval_base.rs @@ -3,9 +3,9 @@ use crate::{ eval_operator, Assignment, Bits, Boolean, Call, Comparison, Expr, Expression, ExternalArgument, Math, Operator, RecordItem, }, - Range, Record, ShellError, Span, Value, VarId, + Config, IntoInterruptiblePipelineData, Range, Record, ShellError, Span, Value, VarId, }; -use std::collections::HashMap; +use std::{borrow::Cow, collections::HashMap}; /// To share implementations for regular eval and const eval pub trait Eval { @@ -271,7 +271,18 @@ pub trait Eval { Self::eval_row_condition_or_closure(state, mut_state, *block_id, expr.span) } Expr::StringInterpolation(exprs) => { - Self::eval_string_interpolation(state, mut_state, exprs, expr.span) + let mut parts = vec![]; + for expr in exprs { + parts.push(Self::eval(state, mut_state, expr)?); + } + + let config = Self::get_config(state, mut_state); + + parts + .into_iter() + .into_pipeline_data(None) + .collect_string("", &config) + .map(|x| Value::string(x, expr.span)) } Expr::Overlay(_) => Self::eval_overlay(state, expr.span), Expr::GlobPattern(pattern, quoted) => { @@ -294,6 +305,8 @@ pub trait Eval { } } + fn get_config<'a>(state: Self::State<'a>, mut_state: &mut Self::MutState) -> Cow<'a, Config>; + fn eval_filepath( state: Self::State<'_>, mut_state: &mut Self::MutState, @@ -366,13 +379,6 @@ pub trait Eval { span: Span, ) -> Result; - fn eval_string_interpolation( - state: Self::State<'_>, - mut_state: &mut Self::MutState, - exprs: &[Expression], - span: Span, - ) -> Result; - fn eval_overlay(state: Self::State<'_>, span: Span) -> Result; fn eval_glob_pattern( diff --git a/crates/nu-protocol/src/eval_const.rs b/crates/nu-protocol/src/eval_const.rs index e6e1b58ba8..995a806c28 100644 --- a/crates/nu-protocol/src/eval_const.rs +++ b/crates/nu-protocol/src/eval_const.rs @@ -2,10 +2,13 @@ use crate::{ ast::{Assignment, Block, Call, Expr, Expression, ExternalArgument, PipelineElement}, engine::{EngineState, StateWorkingSet}, eval_base::Eval, - record, HistoryFileFormat, PipelineData, Record, ShellError, Span, Value, VarId, + record, Config, HistoryFileFormat, PipelineData, Record, ShellError, Span, Value, VarId, }; use nu_system::os_info::{get_kernel_version, get_os_arch, get_os_family, get_os_name}; -use std::path::{Path, PathBuf}; +use std::{ + borrow::Cow, + path::{Path, PathBuf}, +}; pub fn create_nu_constant(engine_state: &EngineState, span: Span) -> Result { fn canonicalize_path(engine_state: &EngineState, path: &Path) -> PathBuf { @@ -278,6 +281,10 @@ impl Eval for EvalConst { type MutState = (); + fn get_config<'a>(state: Self::State<'a>, _: &mut ()) -> Cow<'a, Config> { + Cow::Borrowed(state.get_config()) + } + fn eval_filepath( _: &StateWorkingSet, _: &mut (), @@ -377,15 +384,6 @@ impl Eval for EvalConst { Err(ShellError::NotAConstant { span }) } - fn eval_string_interpolation( - _: &StateWorkingSet, - _: &mut (), - _: &[Expression], - span: Span, - ) -> Result { - Err(ShellError::NotAConstant { span }) - } - fn eval_overlay(_: &StateWorkingSet, span: Span) -> Result { Err(ShellError::NotAConstant { span }) } diff --git a/crates/nu-protocol/src/parse_error.rs b/crates/nu-protocol/src/parse_error.rs index f9ad99f5ae..b19cd47d86 100644 --- a/crates/nu-protocol/src/parse_error.rs +++ b/crates/nu-protocol/src/parse_error.rs @@ -207,7 +207,7 @@ pub enum ParseError { code(nu::parser::module_not_found), help("module files and their paths must be available before your script is run as parsing occurs before anything is evaluated") )] - ModuleNotFound(#[label = "module not found"] Span), + ModuleNotFound(#[label = "module {1} not found"] Span, String), #[error("Missing mod.nu file.")] #[diagnostic( @@ -520,7 +520,7 @@ impl ParseError { ParseError::VariableNotValid(s) => *s, ParseError::AliasNotValid(s) => *s, ParseError::CommandDefNotValid(s) => *s, - ParseError::ModuleNotFound(s) => *s, + ParseError::ModuleNotFound(s, _) => *s, ParseError::ModuleMissingModNuFile(_, s) => *s, ParseError::NamedAsModule(_, _, _, s) => *s, ParseError::ModuleDoubleMain(_, s) => *s, diff --git a/tests/const_/mod.rs b/tests/const_/mod.rs index a76c19efdc..aa6debb926 100644 --- a/tests/const_/mod.rs +++ b/tests/const_/mod.rs @@ -108,6 +108,19 @@ fn const_string() { assert_eq!(actual.out, "abc"); } +#[test] +fn const_string_interpolation() { + let actual = nu!(r#" + const x = 2 + const s = $"var: ($x), date: (2021-02-27T13:55:40+00:00), file size: (2kb)" + $s + "#); + assert_eq!( + actual.out, + "var: 2, date: Sat, 27 Feb 2021 13:55:40 +0000 (2 years ago), file size: 2.0 KiB" + ); +} + #[test] fn const_nothing() { let inp = &["const x = null", "$x | describe"]; diff --git a/tests/modules/mod.rs b/tests/modules/mod.rs index 93fa3147e6..2e868c3e5d 100644 --- a/tests/modules/mod.rs +++ b/tests/modules/mod.rs @@ -357,7 +357,7 @@ fn module_cyclical_imports_0() { let actual = nu!(cwd: dirs.test(), &inp.join("; ")); - assert!(actual.err.contains("module not found")); + assert!(actual.err.contains("Module not found")); }) }