From dff87fd5c2d58f1135ad6019138a8bd8e65e9de1 Mon Sep 17 00:00:00 2001 From: pagedmov Date: Sun, 1 Mar 2026 02:20:58 -0500 Subject: [PATCH] More progress on integrating ariadne's error reporting --- src/builtin/alias.rs | 18 +---- src/builtin/arrops.rs | 3 +- src/builtin/cd.rs | 31 +++------ src/builtin/complete.rs | 13 +--- src/builtin/dirstack.rs | 134 +++++++++++++----------------------- src/builtin/exec.rs | 11 +-- src/builtin/flowctl.rs | 6 +- src/builtin/jobctl.rs | 50 +++----------- src/builtin/shift.rs | 6 +- src/builtin/source.rs | 12 +--- src/builtin/test.rs | 18 +---- src/builtin/varcmds.rs | 12 +--- src/jobs.rs | 4 ++ src/libsh/error.rs | 24 +++++-- src/libsh/utils.rs | 22 +++++- src/parse/execute.rs | 46 +++++++------ src/parse/lex.rs | 32 ++++----- src/parse/mod.rs | 148 ++++++++++++++++++++++++++++++++++------ src/tests/error.rs | 2 +- 19 files changed, 297 insertions(+), 295 deletions(-) diff --git a/src/builtin/alias.rs b/src/builtin/alias.rs index dd93a3e..3cef8da 100644 --- a/src/builtin/alias.rs +++ b/src/builtin/alias.rs @@ -38,19 +38,11 @@ pub fn alias(node: Node, io_stack: &mut IoStack, job: &mut JobBldr) -> ShResult< } else { for (arg, span) in argv { if arg == "command" || arg == "builtin" { - return Err(ShErr::full( - ShErrKind::ExecFail, - format!("alias: Cannot assign alias to reserved name '{arg}'"), - span, - )); + return Err(ShErr::at(ShErrKind::ExecFail, span, format!("alias: Cannot assign alias to reserved name '{arg}'"))); } let Some((name, body)) = arg.split_once('=') else { - return Err(ShErr::full( - ShErrKind::SyntaxErr, - "alias: Expected an assignment in alias args", - span, - )); + return Err(ShErr::at(ShErrKind::SyntaxErr, span, "alias: Expected an assignment in alias args")); }; write_logic(|l| l.insert_alias(name, body)); } @@ -88,11 +80,7 @@ pub fn unalias(node: Node, io_stack: &mut IoStack, job: &mut JobBldr) -> ShResul } else { for (arg, span) in argv { if read_logic(|l| l.get_alias(&arg)).is_none() { - return Err(ShErr::full( - ShErrKind::SyntaxErr, - format!("unalias: alias '{arg}' not found"), - span, - )); + return Err(ShErr::at(ShErrKind::SyntaxErr, span, format!("unalias: alias '{arg}' not found"))); }; write_logic(|l| l.remove_alias(&arg)) } diff --git a/src/builtin/arrops.rs b/src/builtin/arrops.rs index 88ff6f0..ea83b8f 100644 --- a/src/builtin/arrops.rs +++ b/src/builtin/arrops.rs @@ -84,6 +84,7 @@ fn arr_pop_inner(node: Node, io_stack: &mut IoStack, job: &mut JobBldr, end: End } fn arr_push_inner(node: Node, io_stack: &mut IoStack, job: &mut JobBldr, end: End) -> ShResult<()> { + let blame = node.get_span().clone(); let NdRule::Command { assignments: _, argv, @@ -99,7 +100,7 @@ fn arr_push_inner(node: Node, io_stack: &mut IoStack, job: &mut JobBldr, end: En let mut argv = argv.into_iter(); let Some((name, _)) = argv.next() else { - return Err(ShErr::simple(ShErrKind::ExecFail, "push: missing array name".to_string())); + return Err(ShErr::at(ShErrKind::ExecFail, blame, "push: missing array name".to_string())); }; for (val, _) in argv { diff --git a/src/builtin/cd.rs b/src/builtin/cd.rs index 0fcef5c..784b8dc 100644 --- a/src/builtin/cd.rs +++ b/src/builtin/cd.rs @@ -1,4 +1,5 @@ -use ariadne::{Fmt, Label, Span}; +use ariadne::Fmt; +use yansi::Color; use crate::{ jobs::JobBldr, @@ -12,7 +13,6 @@ use super::setup_builtin; pub fn cd(node: Node, job: &mut JobBldr) -> ShResult<()> { let span = node.get_span(); - let src = span.source(); let NdRule::Command { assignments: _, argv, @@ -32,39 +32,28 @@ pub fn cd(node: Node, job: &mut JobBldr) -> ShResult<()> { }; if !new_dir.exists() { - let color = next_color(); let mut err = ShErr::new( ShErrKind::ExecFail, span.clone(), - ).with_label(src.clone(), Label::new(cd_span.clone()).with_color(color).with_message("Failed to change directory")); + ).labeled(cd_span.clone(), "Failed to change directory"); if let Some(span) = arg_span { - let color = next_color(); - err = err.with_label(src.clone(), Label::new(span).with_color(color).with_message(format!("No such file or directory '{}'", new_dir.display().fg(color)))); + err = err.labeled(span, format!("No such file or directory '{}'", new_dir.display().fg(next_color()))); } return Err(err); } if !new_dir.is_dir() { - return Err(ShErr::full( - ShErrKind::ExecFail, - format!("cd: Not a directory '{}'", new_dir.display()), - span, - )); + return Err(ShErr::new(ShErrKind::ExecFail, span.clone()) + .labeled(cd_span.clone(), format!("cd: Not a directory '{}'", new_dir.display().fg(next_color())))); } if let Err(e) = env::set_current_dir(new_dir) { - return Err(ShErr::full( - ShErrKind::ExecFail, - format!("cd: Failed to change directory: {}", e), - span, - )); + return Err(ShErr::new(ShErrKind::ExecFail, span.clone()) + .labeled(cd_span.clone(), format!("cd: Failed to change directory: '{}'", e.fg(Color::Red)))); } let new_dir = env::current_dir().map_err(|e| { - ShErr::full( - ShErrKind::ExecFail, - format!("cd: Failed to get current directory: {}", e), - span, - ) + ShErr::new(ShErrKind::ExecFail, span.clone()) + .labeled(cd_span.clone(), format!("cd: Failed to get current directory: '{}'", e.fg(Color::Red))) })?; unsafe { env::set_var("PWD", new_dir) }; diff --git a/src/builtin/complete.rs b/src/builtin/complete.rs index 56f6f0c..d6acb44 100644 --- a/src/builtin/complete.rs +++ b/src/builtin/complete.rs @@ -206,11 +206,7 @@ pub fn complete_builtin(node: Node, io_stack: &mut IoStack, job: &mut JobBldr) - if argv.is_empty() { state::set_status(1); - return Err(ShErr::full( - ShErrKind::ExecFail, - "complete: no command specified", - blame, - )); + return Err(ShErr::at(ShErrKind::ExecFail, blame, "complete: no command specified")); } let comp_spec = BashCompSpec::from_comp_opts(comp_opts).with_source(src); @@ -285,11 +281,8 @@ pub fn get_comp_opts(opts: Vec) -> ShResult { "dirnames" => comp_opts.opt_flags |= CompOptFlags::DIRNAMES, "space" => comp_opts.opt_flags |= CompOptFlags::SPACE, _ => { - return Err(ShErr::full( - ShErrKind::InvalidOpt, - format!("complete: invalid option: {}", opt_flag), - Default::default(), - )); + let span: crate::parse::lex::Span = Default::default(); + return Err(ShErr::at(ShErrKind::InvalidOpt, span, format!("complete: invalid option: {}", opt_flag))); } }, diff --git a/src/builtin/dirstack.rs b/src/builtin/dirstack.rs index 840c10a..9c8c531 100644 --- a/src/builtin/dirstack.rs +++ b/src/builtin/dirstack.rs @@ -47,26 +47,18 @@ fn print_dirs() -> ShResult<()> { fn change_directory(target: &PathBuf, blame: Span) -> ShResult<()> { if !target.is_dir() { - return Err(ShErr::full( - ShErrKind::ExecFail, - format!("not a directory: {}", target.display()), - blame, - )); + return Err( + ShErr::at(ShErrKind::ExecFail, blame, format!("not a directory: {}", target.display())) + ); } if let Err(e) = env::set_current_dir(target) { - return Err(ShErr::full( - ShErrKind::ExecFail, - format!("Failed to change directory: {}", e), - blame, - )); + return Err( + ShErr::at(ShErrKind::ExecFail, blame, format!("Failed to change directory: {}", e)) + ); } let new_dir = env::current_dir().map_err(|e| { - ShErr::full( - ShErrKind::ExecFail, - format!("Failed to get current directory: {}", e), - blame, - ) + ShErr::at(ShErrKind::ExecFail, blame, format!("Failed to get current directory: {}", e)) })?; unsafe { env::set_var("PWD", new_dir) }; Ok(()) @@ -82,32 +74,24 @@ fn parse_stack_idx(arg: &str, blame: Span, cmd: &str) -> ShResult { }; if digits.is_empty() { - return Err(ShErr::full( - ShErrKind::ExecFail, - format!( + return Err( + ShErr::at(ShErrKind::ExecFail, blame, format!( "{cmd}: missing index after '{}'", if from_top { "+" } else { "-" } - ), - blame, - )); + )) + ); } for ch in digits.chars() { if !ch.is_ascii_digit() { - return Err(ShErr::full( - ShErrKind::ExecFail, - format!("{cmd}: invalid argument: {arg}"), - blame, - )); + return Err( + ShErr::at(ShErrKind::ExecFail, blame, format!("{cmd}: invalid argument: {arg}")) + ); } } let n = digits.parse::().map_err(|e| { - ShErr::full( - ShErrKind::ExecFail, - format!("{cmd}: invalid index: {e}"), - blame, - ) + ShErr::at(ShErrKind::ExecFail, blame, format!("{cmd}: invalid index: {e}")) })?; if from_top { @@ -142,26 +126,20 @@ pub fn pushd(node: Node, io_stack: &mut IoStack, job: &mut JobBldr) -> ShResult< } else if arg == "-n" { no_cd = true; } else if arg.starts_with('-') { - return Err(ShErr::full( - ShErrKind::ExecFail, - format!("pushd: invalid option: {arg}"), - blame.clone(), - )); + return Err( + ShErr::at(ShErrKind::ExecFail, blame, format!("pushd: invalid option: {arg}")) + ); } else { if dir.is_some() { - return Err(ShErr::full( - ShErrKind::ExecFail, - "pushd: too many arguments".to_string(), - blame.clone(), - )); + return Err( + ShErr::at(ShErrKind::ExecFail, blame, "pushd: too many arguments") + ); } let target = PathBuf::from(&arg); if !target.is_dir() { - return Err(ShErr::full( - ShErrKind::ExecFail, - format!("pushd: not a directory: {arg}"), - blame.clone(), - )); + return Err( + ShErr::at(ShErrKind::ExecFail, blame, format!("pushd: not a directory: {arg}")) + ); } dir = Some(target); } @@ -228,11 +206,9 @@ pub fn popd(node: Node, io_stack: &mut IoStack, job: &mut JobBldr) -> ShResult<( } else if arg == "-n" { no_cd = true; } else if arg.starts_with('-') { - return Err(ShErr::full( - ShErrKind::ExecFail, - format!("popd: invalid option: {arg}"), - blame.clone(), - )); + return Err( + ShErr::at(ShErrKind::ExecFail, blame, format!("popd: invalid option: {arg}")) + ); } } @@ -245,11 +221,9 @@ pub fn popd(node: Node, io_stack: &mut IoStack, job: &mut JobBldr) -> ShResult<( if let Some(dir) = dir { change_directory(&dir, blame.clone())?; } else { - return Err(ShErr::full( - ShErrKind::ExecFail, - "popd: directory stack empty".to_string(), - blame.clone(), - )); + return Err( + ShErr::at(ShErrKind::ExecFail, blame, "popd: directory stack empty") + ); } } } @@ -259,11 +233,9 @@ pub fn popd(node: Node, io_stack: &mut IoStack, job: &mut JobBldr) -> ShResult<( let dirs = m.dirs_mut(); let idx = n - 1; if idx >= dirs.len() { - return Err(ShErr::full( - ShErrKind::ExecFail, - format!("popd: directory index out of range: +{n}"), - blame.clone(), - )); + return Err( + ShErr::at(ShErrKind::ExecFail, blame.clone(), format!("popd: directory index out of range: +{n}")) + ); } dirs.remove(idx); Ok(()) @@ -273,11 +245,7 @@ pub fn popd(node: Node, io_stack: &mut IoStack, job: &mut JobBldr) -> ShResult<( write_meta(|m| -> ShResult<()> { let dirs = m.dirs_mut(); let actual = dirs.len().checked_sub(n + 1).ok_or_else(|| { - ShErr::full( - ShErrKind::ExecFail, - format!("popd: directory index out of range: -{n}"), - blame.clone(), - ) + ShErr::at(ShErrKind::ExecFail, blame.clone(), format!("popd: directory index out of range: -{n}")) })?; dirs.remove(actual); Ok(()) @@ -297,11 +265,9 @@ pub fn popd(node: Node, io_stack: &mut IoStack, job: &mut JobBldr) -> ShResult<( change_directory(&dir, blame.clone())?; print_dirs()?; } else { - return Err(ShErr::full( - ShErrKind::ExecFail, - "popd: directory stack empty".to_string(), - blame.clone(), - )); + return Err( + ShErr::at(ShErrKind::ExecFail, blame, "popd: directory stack empty") + ); } } @@ -340,18 +306,14 @@ pub fn dirs(node: Node, io_stack: &mut IoStack, job: &mut JobBldr) -> ShResult<( target_idx = Some(parse_stack_idx(&arg, blame.clone(), "dirs")?); } _ if arg.starts_with('-') => { - return Err(ShErr::full( - ShErrKind::ExecFail, - format!("dirs: invalid option: {arg}"), - blame.clone(), - )); + return Err( + ShErr::at(ShErrKind::ExecFail, blame, format!("dirs: invalid option: {arg}")) + ); } _ => { - return Err(ShErr::full( - ShErrKind::ExecFail, - format!("dirs: unexpected argument: {arg}"), - blame.clone(), - )); + return Err( + ShErr::at(ShErrKind::ExecFail, blame, format!("dirs: unexpected argument: {arg}")) + ); } } } @@ -396,17 +358,15 @@ pub fn dirs(node: Node, io_stack: &mut IoStack, job: &mut JobBldr) -> ShResult<( if let Some(dir) = target { dirs = vec![dir.clone()]; } else { - return Err(ShErr::full( - ShErrKind::ExecFail, - format!( + return Err( + ShErr::at(ShErrKind::ExecFail, blame, format!( "dirs: directory index out of range: {}", match idx { StackIdx::FromTop(n) => format!("+{n}"), StackIdx::FromBottom(n) => format!("-{n}"), } - ), - blame.clone(), - )); + )) + ); } } diff --git a/src/builtin/exec.rs b/src/builtin/exec.rs index 949d2b1..981d9d5 100644 --- a/src/builtin/exec.rs +++ b/src/builtin/exec.rs @@ -1,4 +1,3 @@ -use ariadne::Label; use nix::{errno::Errno, unistd::execvpe}; use crate::{ @@ -43,13 +42,9 @@ pub fn exec_builtin(node: Node, io_stack: &mut IoStack, job: &mut JobBldr) -> Sh let cmd_str = cmd.to_str().unwrap().to_string(); match e { Errno::ENOENT => Err( - ShErr::full(ShErrKind::CmdNotFound, "", span.clone()) - .with_label( - span.span_source().clone(), - Label::new(span.clone()) - .with_message(format!("exec: command not found: {}", cmd_str)) - ) + ShErr::new(ShErrKind::CmdNotFound, span.clone()) + .labeled(span, format!("exec: command not found: {}", cmd_str)) ), - _ => Err(ShErr::full(ShErrKind::Errno(e), format!("{e}"), span)), + _ => Err(ShErr::at(ShErrKind::Errno(e), span, format!("{e}"))), } } diff --git a/src/builtin/flowctl.rs b/src/builtin/flowctl.rs index ae565b0..d5034f9 100644 --- a/src/builtin/flowctl.rs +++ b/src/builtin/flowctl.rs @@ -21,11 +21,7 @@ pub fn flowctl(node: Node, kind: ShErrKind) -> ShResult<()> { let (arg, span) = argv.into_iter().next().unwrap(); let Ok(status) = arg.parse::() else { - return Err(ShErr::full( - ShErrKind::SyntaxErr, - format!("{cmd}: Expected a number"), - span, - )); + return Err(ShErr::at(ShErrKind::SyntaxErr, span, format!("{cmd}: Expected a number"))); }; code = status; diff --git a/src/builtin/jobctl.rs b/src/builtin/jobctl.rs index bf1c009..597c637 100644 --- a/src/builtin/jobctl.rs +++ b/src/builtin/jobctl.rs @@ -33,17 +33,13 @@ pub fn continue_job(node: Node, job: &mut JobBldr, behavior: JobBehavior) -> ShR let mut argv = argv.into_iter(); if read_jobs(|j| j.get_fg().is_some()) { - return Err(ShErr::full( - ShErrKind::InternalErr, - format!("Somehow called '{}' with an existing foreground job", cmd), - blame, - )); + return Err(ShErr::at(ShErrKind::InternalErr, blame, format!("Somehow called '{}' with an existing foreground job", cmd))); } let curr_job_id = if let Some(id) = read_jobs(|j| j.curr_job()) { id } else { - return Err(ShErr::full(ShErrKind::ExecFail, "No jobs found", blame)); + return Err(ShErr::at(ShErrKind::ExecFail, blame, "No jobs found")); }; let tabid = match argv.next() { @@ -57,11 +53,7 @@ pub fn continue_job(node: Node, job: &mut JobBldr, behavior: JobBehavior) -> ShR if query_result.is_some() { Ok(j.remove_job(id.clone()).unwrap()) } else { - Err(ShErr::full( - ShErrKind::ExecFail, - format!("Job id `{}' not found", tabid), - blame, - )) + Err(ShErr::at(ShErrKind::ExecFail, blame.clone(), format!("Job id `{}' not found", tabid))) } })?; @@ -96,11 +88,7 @@ fn parse_job_id(arg: &str, blame: Span) -> ShResult { }); match result { Some(id) => Ok(id), - None => Err(ShErr::full( - ShErrKind::InternalErr, - "Found a job but no table id in parse_job_id()", - blame, - )), + None => Err(ShErr::at(ShErrKind::InternalErr, blame, "Found a job but no table id in parse_job_id()")), } } } else if arg.chars().all(|ch| ch.is_ascii_digit()) { @@ -120,18 +108,10 @@ fn parse_job_id(arg: &str, blame: Span) -> ShResult { match result { Some(id) => Ok(id), - None => Err(ShErr::full( - ShErrKind::InternalErr, - "Found a job but no table id in parse_job_id()", - blame, - )), + None => Err(ShErr::at(ShErrKind::InternalErr, blame, "Found a job but no table id in parse_job_id()")), } } else { - Err(ShErr::full( - ShErrKind::SyntaxErr, - format!("Invalid arg: {}", arg), - blame, - )) + Err(ShErr::at(ShErrKind::SyntaxErr, blame, format!("Invalid arg: {}", arg))) } } @@ -151,11 +131,7 @@ pub fn jobs(node: Node, io_stack: &mut IoStack, job: &mut JobBldr) -> ShResult<( for (arg, span) in argv { let mut chars = arg.chars().peekable(); if chars.peek().is_none_or(|ch| *ch != '-') { - return Err(ShErr::full( - ShErrKind::SyntaxErr, - "Invalid flag in jobs call", - span, - )); + return Err(ShErr::at(ShErrKind::SyntaxErr, span, "Invalid flag in jobs call")); } chars.next(); for ch in chars { @@ -166,11 +142,7 @@ pub fn jobs(node: Node, io_stack: &mut IoStack, job: &mut JobBldr) -> ShResult<( 'r' => JobCmdFlags::RUNNING, 's' => JobCmdFlags::STOPPED, _ => { - return Err(ShErr::full( - ShErrKind::SyntaxErr, - "Invalid flag in jobs call", - span, - )); + return Err(ShErr::at(ShErrKind::SyntaxErr, span, "Invalid flag in jobs call")); } }; flags |= flag @@ -199,11 +171,7 @@ pub fn disown(node: Node, io_stack: &mut IoStack, job: &mut JobBldr) -> ShResult let curr_job_id = if let Some(id) = read_jobs(|j| j.curr_job()) { id } else { - return Err(ShErr::full( - ShErrKind::ExecFail, - "disown: No jobs to disown", - blame, - )); + return Err(ShErr::at(ShErrKind::ExecFail, blame, "disown: No jobs to disown")); }; let mut tabid = curr_job_id; diff --git a/src/builtin/shift.rs b/src/builtin/shift.rs index e6c90b1..373a381 100644 --- a/src/builtin/shift.rs +++ b/src/builtin/shift.rs @@ -22,11 +22,7 @@ pub fn shift(node: Node, job: &mut JobBldr) -> ShResult<()> { if let Some((arg, span)) = argv.next() { let Ok(count) = arg.parse::() else { - return Err(ShErr::full( - ShErrKind::ExecFail, - "Expected a number in shift args", - span, - )); + return Err(ShErr::at(ShErrKind::ExecFail, span, "Expected a number in shift args")); }; for _ in 0..count { write_vars(|v| v.cur_scope_mut().fpop_arg()); diff --git a/src/builtin/source.rs b/src/builtin/source.rs index 976be2e..8c67949 100644 --- a/src/builtin/source.rs +++ b/src/builtin/source.rs @@ -23,18 +23,10 @@ pub fn source(node: Node, job: &mut JobBldr) -> ShResult<()> { for (arg, span) in argv { let path = PathBuf::from(arg); if !path.exists() { - return Err(ShErr::full( - ShErrKind::ExecFail, - format!("source: File '{}' not found", path.display()), - span, - )); + return Err(ShErr::at(ShErrKind::ExecFail, span, format!("source: File '{}' not found", path.display()))); } if !path.is_file() { - return Err(ShErr::full( - ShErrKind::ExecFail, - format!("source: Given path '{}' is not a file", path.display()), - span, - )); + return Err(ShErr::at(ShErrKind::ExecFail, span, format!("source: Given path '{}' is not a file", path.display()))); } source_file(path)?; } diff --git a/src/builtin/test.rs b/src/builtin/test.rs index d64815f..14e7439 100644 --- a/src/builtin/test.rs +++ b/src/builtin/test.rs @@ -138,11 +138,7 @@ pub fn double_bracket_test(node: Node) -> ShResult { let operand = operand.expand()?.get_words().join(" "); conjunct_op = conjunct; let TestOp::Unary(op) = TestOp::from_str(operator.as_str())? else { - return Err(ShErr::full( - ShErrKind::SyntaxErr, - "Invalid unary operator", - err_span, - )); + return Err(ShErr::at(ShErrKind::SyntaxErr, err_span, "Invalid unary operator")); }; match op { UnaryOp::Exists => { @@ -245,11 +241,7 @@ pub fn double_bracket_test(node: Node) -> ShResult { let test_op = operator.as_str().parse::()?; match test_op { TestOp::Unary(_) => { - return Err(ShErr::full( - ShErrKind::SyntaxErr, - "Expected a binary operator in this test call; found a unary operator", - err_span, - )); + return Err(ShErr::at(ShErrKind::SyntaxErr, err_span, "Expected a binary operator in this test call; found a unary operator")); } TestOp::StringEq => { let pattern = crate::expand::glob_to_regex(rhs.trim(), true); @@ -265,11 +257,7 @@ pub fn double_bracket_test(node: Node) -> ShResult { | TestOp::IntGe | TestOp::IntLe | TestOp::IntEq => { - let err = ShErr::full( - ShErrKind::SyntaxErr, - format!("Expected an integer with '{}' operator", operator), - err_span.clone(), - ); + let err = ShErr::at(ShErrKind::SyntaxErr, err_span.clone(), format!("Expected an integer with '{}' operator", operator)); let Ok(lhs) = lhs.trim().parse::() else { return Err(err); }; diff --git a/src/builtin/varcmds.rs b/src/builtin/varcmds.rs index 62922d3..d45a96d 100644 --- a/src/builtin/varcmds.rs +++ b/src/builtin/varcmds.rs @@ -75,20 +75,12 @@ pub fn unset(node: Node, io_stack: &mut IoStack, job: &mut JobBldr) -> ShResult< let argv = argv.unwrap(); if argv.is_empty() { - return Err(ShErr::full( - ShErrKind::SyntaxErr, - "unset: Expected at least one argument", - blame, - )); + return Err(ShErr::at(ShErrKind::SyntaxErr, blame, "unset: Expected at least one argument")); } for (arg, span) in argv { if !read_vars(|v| v.var_exists(&arg)) { - return Err(ShErr::full( - ShErrKind::ExecFail, - format!("unset: No such variable '{arg}'"), - span, - )); + return Err(ShErr::at(ShErrKind::ExecFail, span, format!("unset: No such variable '{arg}'"))); } write_vars(|v| v.unset_var(&arg))?; } diff --git a/src/jobs.rs b/src/jobs.rs index d6c3ebd..48c1f5a 100644 --- a/src/jobs.rs +++ b/src/jobs.rs @@ -704,7 +704,11 @@ pub fn term_ctlr() -> Pid { /// Calls attach_tty() on the shell's process group to retake control of the /// terminal pub fn take_term() -> ShResult<()> { + // take the terminal back attach_tty(getpgrp())?; + + // send SIGWINCH to tell readline to update its window size in case it changed while we were in the background + killpg(getpgrp(), Signal::SIGWINCH)?; Ok(()) } diff --git a/src/libsh/error.rs b/src/libsh/error.rs index 1853795..8b1bdd1 100644 --- a/src/libsh/error.rs +++ b/src/libsh/error.rs @@ -1,5 +1,5 @@ use std::cell::RefCell; -use std::collections::HashMap; +use std::collections::{HashMap, VecDeque}; use std::fmt::Display; use ariadne::Color; use ariadne::{Report, ReportKind}; @@ -13,7 +13,9 @@ use crate::{ pub type ShResult = Result; -pub struct ColorRng; +pub struct ColorRng { + last_color: Option, +} impl ColorRng { fn get_colors() -> &'static [Color] { @@ -51,7 +53,7 @@ impl Iterator for ColorRng { } thread_local! { - static COLOR_RNG: RefCell = const { RefCell::new(ColorRng) }; + static COLOR_RNG: RefCell = const { RefCell::new(ColorRng { last_color: None }) }; } pub fn next_color() -> Color { @@ -144,8 +146,18 @@ impl ShErr { pub fn simple(kind: ShErrKind, msg: impl Into) -> Self { Self { kind, src_span: None, labels: vec![], sources: vec![], notes: vec![msg.into()] } } - pub fn full(kind: ShErrKind, msg: impl Into, span: Span) -> Self { - Self { kind, src_span: Some(span), labels: vec![], sources: vec![], notes: vec![msg.into()] } + pub fn at(kind: ShErrKind, span: Span, msg: impl Into) -> Self { + let color = next_color(); + let src = span.span_source().clone(); + let msg: String = msg.into(); + Self::new(kind, span.clone()) + .with_label(src, ariadne::Label::new(span).with_color(color).with_message(msg)) + } + pub fn labeled(self, span: Span, msg: impl Into) -> Self { + let color = next_color(); + let src = span.span_source().clone(); + let msg: String = msg.into(); + self.with_label(src, ariadne::Label::new(span).with_color(color).with_message(msg)) } pub fn blame(self, span: Span) -> Self { let ShErr { kind, src_span: _, labels, sources, notes } = self; @@ -172,7 +184,7 @@ impl ShErr { labels.push(label); Self { kind, src_span, labels, sources, notes } } - pub fn with_context(self, ctx: Vec<(SpanSource, ariadne::Label)>) -> Self { + pub fn with_context(self, ctx: VecDeque<(SpanSource, ariadne::Label)>) -> Self { let ShErr { kind, src_span, mut labels, mut sources, notes } = self; for (src, label) in ctx { sources.push(src); diff --git a/src/libsh/utils.rs b/src/libsh/utils.rs index fb77b3a..6ad45ab 100644 --- a/src/libsh/utils.rs +++ b/src/libsh/utils.rs @@ -1,7 +1,9 @@ use std::collections::VecDeque; +use ariadne::Span as AriadneSpan; + use crate::parse::lex::{Span, Tk, TkRule}; -use crate::parse::{Redir, RedirType}; +use crate::parse::{Node, Redir, RedirType}; use crate::prelude::*; pub trait VecDequeExt { @@ -27,6 +29,10 @@ pub trait RedirVecUtils { fn split_by_channel(self) -> (Vec, Vec); } +pub trait NodeVecUtils { + fn get_span(&self) -> Option; +} + impl VecDequeExt for VecDeque { fn to_vec(self) -> Vec { self.into_iter().collect::>() @@ -124,3 +130,17 @@ impl RedirVecUtils for Vec { (input, output) } } + +impl NodeVecUtils for Vec { + fn get_span(&self) -> Option { + if let Some(first_nd) = self.first() + && let Some(last_nd) = self.last() { + let first_start = first_nd.get_span().range().start; + let last_end = last_nd.get_span().range().end; + if first_start <= last_end { + return Some(Span::new(first_start..last_end, first_nd.get_span().source().content())); + } + } + None + } +} diff --git a/src/parse/execute.rs b/src/parse/execute.rs index 80e7cbb..1000793 100644 --- a/src/parse/execute.rs +++ b/src/parse/execute.rs @@ -2,7 +2,8 @@ use std::{ cell::Cell, collections::{HashSet, VecDeque}, os::unix::fs::PermissionsExt }; -use ariadne::{Fmt, Label}; + +use ariadne::{Fmt, Label, Span as AriadneSpan}; use crate::{ builtin::{ @@ -294,10 +295,10 @@ impl Dispatcher { let name = name.span.as_str().strip_suffix("()").unwrap(); if KEYWORDS.contains(&name) { - return Err(ShErr::full( + return Err(ShErr::at( ShErrKind::SyntaxErr, - format!("function: Forbidden function name `{name}`"), blame, + format!("function: Forbidden function name `{name}`"), )); } @@ -342,6 +343,8 @@ impl Dispatcher { } fn exec_func(&mut self, func: Node) -> ShResult<()> { let mut blame = func.get_span().clone(); + let func_name = func.get_command().unwrap().to_string(); + let func_ctx = func.get_context(format!("in call to function '{}'",func_name.fg(next_color()))); let NdRule::Command { assignments, mut argv, @@ -358,24 +361,25 @@ impl Dispatcher { }); if depth > max_depth { RECURSE_DEPTH.with(|d| d.set(d.get() - 1)); - return Err(ShErr::full( + return Err(ShErr::at( ShErrKind::InternalErr, - format!("maximum recursion depth ({max_depth}) exceeded"), blame, + format!("maximum recursion depth ({max_depth}) exceeded"), )); } let env_vars = self.set_assignments(assignments, AssignBehavior::Export)?; + let func_name = argv.remove(0).to_string(); let _var_guard = VarCtxGuard::new(env_vars.into_iter().collect()); self.io_stack.append_to_frame(func.redirs); - let func_name = argv.remove(0).span.as_str().to_string(); blame.rename(func_name.clone()); let argv = prepare_argv(argv)?; let result = if let Some(ref mut func_body) = read_logic(|l| l.get_func(&func_name)) { let _guard = ScopeGuard::exclusive_scope(Some(argv)); + func_body.body_mut().propagate_context(func_ctx); func_body.body_mut().flags = func.flags; if let Err(e) = self.exec_brc_grp(func_body.body().clone()) { @@ -384,16 +388,16 @@ impl Dispatcher { state::set_status(*code); Ok(()) } - _ => Err(e), + _ => Err(e) } } else { Ok(()) } } else { - Err(ShErr::full( + Err(ShErr::at( ShErrKind::InternalErr, - format!("Failed to find function '{}'", func_name), blame, + format!("Failed to find function '{}'", func_name), )) }; @@ -747,6 +751,7 @@ impl Dispatcher { } fn dispatch_builtin(&mut self, mut cmd: Node) -> ShResult<()> { let cmd_raw = cmd.get_command().unwrap().to_string(); + let context = cmd.context.clone(); let NdRule::Command { assignments, argv } = &mut cmd.class else { unreachable!() }; @@ -773,7 +778,7 @@ impl Dispatcher { } return self.exec_cmd(cmd); } - match cmd_raw.as_str() { + let result = match cmd_raw.as_str() { "echo" => echo(cmd, io_stack_mut, curr_job_mut), "cd" => cd(cmd, curr_job_mut), "export" => export(cmd, io_stack_mut, curr_job_mut), @@ -819,7 +824,13 @@ impl Dispatcher { Ok(()) } _ => unimplemented!("Have not yet added support for builtin '{}'", cmd_raw), - } + }; + + if let Err(e) = result { + Err(e.with_context(context)) + } else { + Ok(()) + } } fn exec_cmd(&mut self, cmd: Node) -> ShResult<()> { let context = cmd.context.clone(); @@ -859,20 +870,13 @@ impl Dispatcher { let cmd_str = cmd.to_str().unwrap().to_string(); match e { Errno::ENOENT => { - let source = span.span_source().clone(); - let color = next_color(); - ShErr::full(ShErrKind::CmdNotFound, "", span.clone()) - .with_label( - source, - Label::new(span) - .with_color(color) - .with_message(format!("{}: command not found", cmd_str.fg(color))) - ) + ShErr::new(ShErrKind::CmdNotFound, span.clone()) + .labeled(span, format!("{cmd_str}: command not found")) .with_context(context) .print_error(); } _ => { - ShErr::full(ShErrKind::Errno(e), format!("{e}"), span) + ShErr::at(ShErrKind::Errno(e), span, format!("{e}")) .with_context(context) .print_error(); } diff --git a/src/parse/lex.rs b/src/parse/lex.rs index b29ef3d..e984380 100644 --- a/src/parse/lex.rs +++ b/src/parse/lex.rs @@ -354,10 +354,10 @@ impl LexStream { if !found_fd && !self.flags.contains(LexFlags::LEX_UNFINISHED) { let span_start = self.cursor; self.cursor = pos; - return Some(Err(ShErr::full( + return Some(Err(ShErr::at( ShErrKind::ParseErr, - "Invalid redirection", Span::new(span_start..pos, self.source.clone()), + "Invalid redirection", ))); } else { tk = self.get_token(self.cursor..pos, TkRule::Redir); @@ -471,10 +471,10 @@ impl LexStream { } if !paren_count == 0 && !self.flags.contains(LexFlags::LEX_UNFINISHED) { self.cursor = pos; - return Err(ShErr::full( + return Err(ShErr::at( ShErrKind::ParseErr, - "Unclosed subshell", Span::new(paren_pos..paren_pos + 1, self.source.clone()), + "Unclosed subshell", )); } } @@ -539,10 +539,10 @@ impl LexStream { } if !paren_count == 0 && !self.flags.contains(LexFlags::LEX_UNFINISHED) { self.cursor = pos; - return Err(ShErr::full( + return Err(ShErr::at( ShErrKind::ParseErr, - "Unclosed subshell", Span::new(paren_pos..paren_pos + 1, self.source.clone()), + "Unclosed subshell", )); } } @@ -575,10 +575,10 @@ impl LexStream { } if !paren_count == 0 && !self.flags.contains(LexFlags::LEX_UNFINISHED) { self.cursor = pos; - return Err(ShErr::full( + return Err(ShErr::at( ShErrKind::ParseErr, - "Unclosed subshell", Span::new(paren_pos..paren_pos + 1, self.source.clone()), + "Unclosed subshell", )); } } @@ -610,10 +610,10 @@ impl LexStream { } if paren_count != 0 && !self.flags.contains(LexFlags::LEX_UNFINISHED) { self.cursor = pos; - return Err(ShErr::full( + return Err(ShErr::at( ShErrKind::ParseErr, - "Unclosed subshell", Span::new(paren_pos..paren_pos + 1, self.source.clone()), + "Unclosed subshell", )); } let mut subsh_tk = self.get_token(self.cursor..pos, TkRule::Str); @@ -677,10 +677,10 @@ impl LexStream { let mut new_tk = self.get_token(self.cursor..pos, TkRule::Str); if self.quote_state.in_quote() && !self.flags.contains(LexFlags::LEX_UNFINISHED) { self.cursor = pos; - return Err(ShErr::full( + return Err(ShErr::at( ShErrKind::ParseErr, - "Unterminated quote", new_tk.span, + "Unterminated quote", )); } @@ -750,10 +750,10 @@ impl Iterator for LexStream { if self.in_brc_grp() && !self.flags.contains(LexFlags::LEX_UNFINISHED) { let start = self.brc_grp_start.unwrap_or(self.cursor.saturating_sub(1)); self.flags |= LexFlags::STALE; - return Err(ShErr::full( + return Err(ShErr::at( ShErrKind::ParseErr, - "Unclosed brace group", Span::new(start..self.cursor, self.source.clone()), + "Unclosed brace group", )) .into(); } @@ -789,10 +789,10 @@ impl Iterator for LexStream { if self.cursor == self.source.len() { if self.in_brc_grp() && !self.flags.contains(LexFlags::LEX_UNFINISHED) { let start = self.brc_grp_start.unwrap_or(self.cursor.saturating_sub(1)); - return Err(ShErr::full( + return Err(ShErr::at( ShErrKind::ParseErr, - "Unclosed brace group", Span::new(start..self.cursor, self.source.clone()), + "Unclosed brace group", )) .into(); } diff --git a/src/parse/mod.rs b/src/parse/mod.rs index a9afde4..ce27e35 100644 --- a/src/parse/mod.rs +++ b/src/parse/mod.rs @@ -1,6 +1,6 @@ -use std::{fmt::Debug, str::FromStr, sync::Arc}; +use std::{collections::VecDeque, fmt::Debug, str::FromStr, sync::Arc}; -use ariadne::{Fmt, Label}; +use ariadne::{Fmt, Label, Span as AriadneSpan}; use bitflags::bitflags; use fmt::Display; use lex::{LexFlags, LexStream, Span, SpanSource, Tk, TkFlags, TkRule}; @@ -9,7 +9,7 @@ use yansi::Color; use crate::{ libsh::{ error::{Note, ShErr, ShErrKind, ShResult, next_color}, - utils::TkVecUtils, + utils::{NodeVecUtils, TkVecUtils}, }, prelude::*, procio::IoMode, @@ -56,7 +56,7 @@ impl ParsedSrc { src, ast: Ast::new(vec![]), lex_flags: LexFlags::empty(), - context: vec![], + context: VecDeque::new(), } } pub fn with_lex_flags(mut self, flags: LexFlags) -> Self { @@ -97,7 +97,7 @@ impl ParsedSrc { } } -#[derive(Clone, Debug)] +#[derive(Default, Clone, Debug)] pub struct Ast(Vec); impl Ast { @@ -112,7 +112,7 @@ impl Ast { } } -pub type LabelCtx = Vec<(SpanSource, Label)>; +pub type LabelCtx = VecDeque<(SpanSource, Label)>; #[derive(Clone, Debug)] pub struct Node { @@ -135,6 +135,108 @@ impl Node { None } } + pub fn get_context(&self, msg: String) -> (SpanSource, Label) { + let color = next_color(); + let span = self.get_span().clone(); + ( + span.clone().source().clone(), + Label::new(span).with_color(color).with_message(msg) + ) + } + fn walk_tree(&mut self, f: &F) { + f(self); + + match self.class { + NdRule::IfNode { + ref mut cond_nodes, + ref mut else_block, + } => { + for node in cond_nodes { + let CondNode { cond, body } = node; + cond.walk_tree(f); + for body_node in body { + body_node.walk_tree(f); + } + } + + for else_node in else_block { + else_node.walk_tree(f); + } + } + NdRule::LoopNode { + kind: _, + ref mut cond_node, + } => { + let CondNode { cond, body } = cond_node; + cond.walk_tree(f); + for body_node in body { + body_node.walk_tree(f); + } + } + NdRule::ForNode { + vars: _, + arr: _, + ref mut body, + } => { + for body_node in body { + body_node.walk_tree(f); + } + } + NdRule::CaseNode { + pattern: _, + ref mut case_blocks, + } => { + for block in case_blocks { + let CaseNode { pattern: _, body } = block; + for body_node in body { + body_node.walk_tree(f); + } + } + } + NdRule::Command { + ref mut assignments, + argv: _, + } => { + for assign_node in assignments { + assign_node.walk_tree(f); + } + } + NdRule::Pipeline { + ref mut cmds, + pipe_err: _, + } => { + for cmd_node in cmds { + cmd_node.walk_tree(f); + } + } + NdRule::Conjunction { ref mut elements } => { + for node in elements.iter_mut() { + let ConjunctNode { cmd, operator: _ } = node; + cmd.walk_tree(f); + } + } + NdRule::Assignment { + kind: _, + var: _, + val: _, + } => (), // No nodes to check + NdRule::BraceGrp { ref mut body } => { + for body_node in body { + body_node.walk_tree(f); + } + } + NdRule::FuncDef { + name: _, + ref mut body, + } => { + body.walk_tree(f); + } + NdRule::Test { cases: _ } => (), + } + } + pub fn propagate_context(&mut self, ctx: (SpanSource, Label)) { + self.walk_tree(&|nd| nd.context.push_back(ctx.clone())); + } pub fn get_span(&self) -> Span { let Some(first_tk) = self.tokens.first() else { unreachable!() @@ -565,7 +667,7 @@ impl Debug for ParseStream { impl ParseStream { pub fn new(tokens: Vec) -> Self { - Self { tokens, context: vec![] } + Self { tokens, context: VecDeque::new() } } pub fn with_context(tokens: Vec, context: LabelCtx) -> Self { Self { tokens, context } @@ -726,7 +828,7 @@ impl ParseStream { src.rename(name_raw.clone()); let color = next_color(); // Push a placeholder context so child nodes inherit it - self.context.push(( + self.context.push_back(( src.clone(), Label::new(name_tk.span.clone().with_name(name_raw.clone())) .with_message(format!("in function '{}' defined here", name_raw.clone().fg(color))) @@ -734,7 +836,7 @@ impl ParseStream { )); let Some(brc_grp) = self.parse_brc_grp(true /* from_func_def */)? else { - self.context.pop(); + self.context.pop_back(); return Err(parse_err_full( "Expected a brace group after function name", &node_tks.get_span().unwrap(), @@ -743,14 +845,7 @@ impl ParseStream { }; body = Box::new(brc_grp); // Replace placeholder with full-span label - self.context.pop(); - let full_span = body.get_span(); - self.context.push(( - src, - Label::new(full_span.with_name(name_raw.clone())) - .with_message(format!("in function '{}' called here", name_raw.fg(color))) - .with_color(color), - )); + self.context.pop_back(); let node = Node { class: NdRule::FuncDef { name, body }, @@ -760,7 +855,7 @@ impl ParseStream { context: self.context.clone() }; - self.context.pop(); + self.context.pop_back(); Ok(Some(node)) } fn panic_mode(&mut self, node_tks: &mut Vec) { @@ -906,10 +1001,10 @@ impl ParseStream { let path_tk = self.next_tk(); if path_tk.clone().is_none_or(|tk| tk.class == TkRule::EOI) { - return Err(ShErr::full( + return Err(ShErr::at( ShErrKind::ParseErr, - "Expected a filename after this redirection", tk.span.clone(), + "Expected a filename after this redirection", )); }; @@ -1412,6 +1507,14 @@ impl ParseStream { // If we have assignments but no command word, // return the assignment-only command without parsing more tokens self.commit(node_tks.len()); + let mut context = self.context.clone(); + let assignments_span = assignments.get_span().unwrap(); + context.push_back(( + assignments_span.source().clone(), + Label::new(assignments_span) + .with_message("in variable assignment defined here".to_string()) + .with_color(next_color()) + )); return Ok(Some(Node { class: NdRule::Command { assignments, argv }, tokens: node_tks, @@ -1448,10 +1551,11 @@ impl ParseStream { let path_tk = tk_iter.next(); if path_tk.is_none_or(|tk| tk.class == TkRule::EOI) { - return Err(ShErr::full( + self.panic_mode(&mut node_tks); + return Err(ShErr::at( ShErrKind::ParseErr, - "Expected a filename after this redirection", tk.span.clone(), + "Expected a filename after this redirection", )); }; diff --git a/src/tests/error.rs b/src/tests/error.rs index 577f6e9..5ae5eaa 100644 --- a/src/tests/error.rs +++ b/src/tests/error.rs @@ -7,7 +7,7 @@ fn cmd_not_found() { .next() .unwrap() .unwrap(); - let err = ShErr::full(ShErrKind::CmdNotFound("foo".into()), "", token.span); + let err = ShErr::at(ShErrKind::CmdNotFound, token.span, ""); let err_fmt = format!("{err}"); insta::assert_snapshot!(err_fmt)