From 0199583435c3b584b90769d50252d16105349671 Mon Sep 17 00:00:00 2001 From: Johannes Altmanninger Date: Thu, 21 Nov 2024 23:33:13 +0100 Subject: [PATCH] Interpret () in command position as subshell --- src/expand.rs | 63 +++++++++++++++---------- tests/checks/expansion.fish | 9 ++-- tests/checks/syntax-error-location.fish | 12 ----- 3 files changed, 40 insertions(+), 44 deletions(-) diff --git a/src/expand.rs b/src/expand.rs index c7c7a0bda..398de2e34 100644 --- a/src/expand.rs +++ b/src/expand.rs @@ -3,6 +3,8 @@ //! from using a more clever memory allocation scheme, perhaps an evil combination of talloc, //! string buffers and reference counting. +use std::sync::atomic::Ordering; + use crate::builtins::shared::{ STATUS_CMD_ERROR, STATUS_CMD_UNKNOWN, STATUS_EXPAND_ERROR, STATUS_ILLEGAL_CMD, STATUS_INVALID_ARGS, STATUS_NOT_EXECUTABLE, STATUS_READ_TOO_MUCH, STATUS_UNMATCHED_WILDCARD, @@ -17,12 +19,14 @@ use crate::exec::exec_subshell_for_expand; use crate::future_feature_flags::{feature_test, FeatureFlag}; use crate::history::{history_session_id, History}; +use crate::libc::_PATH_BSHELL; use crate::operation_context::OperationContext; use crate::parse_constants::{ParseError, ParseErrorCode, ParseErrorList, SOURCE_LOCATION_UNKNOWN}; use crate::parse_util::{ parse_util_expand_variable_error, parse_util_locate_cmdsubst_range, MaybeParentheses, }; use crate::path::path_apply_working_directory; +use crate::threads::MainThread; use crate::util::wcsfilecmp_glob; use crate::wchar::prelude::*; use crate::wcstringutil::{join_strings, trim}; @@ -30,6 +34,7 @@ use crate::wildcard::{WildcardResult, ANY_CHAR, ANY_STRING, ANY_STRING_RECURSIVE}; use crate::wutil::{normalize_path, wcstoi_partial, Options}; use bitflags::bitflags; +use once_cell::unsync::OnceCell; bitflags! { /// Set of flags controlling expansions. @@ -74,6 +79,8 @@ pub struct ExpandFlags : u16 { const SPECIAL_FOR_COMMAND = 1 << 13; /// The token has an unclosed brace, so don't add a space. const NO_SPACE_FOR_UNCLOSED_BRACE = 1 << 14; + /// TODO + const FOR_COMMAND = 1 << 15; } } @@ -226,7 +233,7 @@ pub fn expand_to_command_and_args( } let mut eflags = if ctx.has_parser() { - ExpandFlags::empty() + ExpandFlags::FOR_COMMAND } else { ExpandFlags::FAIL_ON_CMDSUBST }; @@ -370,20 +377,6 @@ macro_rules! append_syntax_error { } } -/// Append a cmdsub error to the given error list. But only do so if the error hasn't already been -/// recorded. This is needed because command substitution is a recursive process and some errors -/// could consequently be recorded more than once. -macro_rules! append_cmdsub_error { - ( - $errors:expr, $source_start:expr, $source_end:expr, - $fmt:expr $(, $arg:expr )* $(,)? - ) => { - append_cmdsub_error_formatted!( - $errors, $source_start, $source_end, - wgettext_maybe_fmt!($fmt $(, $arg)*)); - } -} - macro_rules! append_cmdsub_error_formatted { ( $errors:expr, $source_start:expr, $source_end:expr, @@ -965,6 +958,7 @@ pub fn expand_cmdsubst( ctx: &OperationContext, out: &mut CompletionReceiver, errors: &mut Option<&mut ParseErrorList>, + for_command: bool, ) -> ExpandResult { let mut cursor = 0; let mut is_quoted = false; @@ -986,7 +980,23 @@ pub fn expand_cmdsubst( } return ExpandResult::ok(); } - MaybeParentheses::CommandSubstitution(parens) => parens, + MaybeParentheses::CommandSubstitution(parens) => { + if for_command { + // TODO error on extra args + static PATH_BSHELL: MainThread> = + MainThread::new(OnceCell::new()); + let shell = PATH_BSHELL + .get() + .get_or_init(|| charptr2wcstring(_PATH_BSHELL.load(Ordering::Relaxed))); + for arg in [shell.as_utfstr(), L!("-c"), &input[parens.command()]] { + if !out.add(arg.to_owned()) { + return append_overflow_error(errors, None); + } + } + return ExpandResult::ok(); + } + parens + } }; let mut sub_res = vec![]; @@ -1092,7 +1102,7 @@ pub fn expand_cmdsubst( tail.insert(0, '"'); } - let _ = expand_cmdsubst(tail, ctx, &mut tail_expand_recv, errors); // TODO: offset error locations + let _ = expand_cmdsubst(tail, ctx, &mut tail_expand_recv, errors, false); // TODO: offset error locations let tail_expand = tail_expand_recv.take(); // Combine the result of the current command substitution with the result of the recursive tail @@ -1366,6 +1376,7 @@ fn stage_cmdsubst(&mut self, input: WString, out: &mut CompletionReceiver) -> Ex return ExpandResult::ok(); } if self.flags.contains(ExpandFlags::FAIL_ON_CMDSUBST) { + // TODO this if FOR_COMMAND let mut cursor = 0; let mut has_dollar = false; match parse_util_locate_cmdsubst_range( @@ -1384,17 +1395,11 @@ fn stage_cmdsubst(&mut self, input: WString, out: &mut CompletionReceiver) -> Ex } return ExpandResult::ok(); } - MaybeParentheses::CommandSubstitution(parens) => { + MaybeParentheses::CommandSubstitution(_parens) => { if has_dollar { return ExpandResult::ok(); } - append_cmdsub_error!( - self.errors, - parens.start(), - parens.end()-1, - "command substitutions not allowed in command position. Try var=(your-cmd) $var ..." - ); - return ExpandResult::make_error(STATUS_EXPAND_ERROR.unwrap()); + return ExpandResult::ok(); } } } else { @@ -1402,7 +1407,13 @@ fn stage_cmdsubst(&mut self, input: WString, out: &mut CompletionReceiver) -> Ex self.ctx.has_parser(), "Must have a parser to expand command substitutions" ); - expand_cmdsubst(input, self.ctx, out, self.errors) + expand_cmdsubst( + input, + self.ctx, + out, + self.errors, + self.flags.contains(ExpandFlags::FOR_COMMAND), + ) } } diff --git a/tests/checks/expansion.fish b/tests/checks/expansion.fish index 713fc509c..ad6e51f8a 100644 --- a/tests/checks/expansion.fish +++ b/tests/checks/expansion.fish @@ -324,11 +324,8 @@ $fish -c 'echo {}}' #CHECKERR: fish: Unexpected '}' for unopened brace #CHECKERR: echo {}} #CHECKERR: ^ -printf '<%s>\n' ($fish -c 'command (asd)' 2>&1) -#CHECK: -#CHECK: -#CHECK: < ^~~~^> -true +printf '<%s>\n' ($fish -c 'command (echo asd)' 2>&1) +# CHECK: printf '<%s>\n' ($fish -c 'echo "$abc["' 2>&1) #CHECK: @@ -337,7 +334,7 @@ printf '<%s>\n' ($fish -c 'echo "$abc["' 2>&1) set -l pager command less echo foo | $pager -#CHECKERR: {{.*}}checks/expansion.fish (line 339): The expanded command is a keyword. +#CHECKERR: {{.*}}checks/expansion.fish (line {{\d+}}): The expanded command is a keyword. #CHECKERR: echo foo | $pager #CHECKERR: ^~~~~^ diff --git a/tests/checks/syntax-error-location.fish b/tests/checks/syntax-error-location.fish index 14d0df8d2..300a954f8 100644 --- a/tests/checks/syntax-error-location.fish +++ b/tests/checks/syntax-error-location.fish @@ -17,18 +17,6 @@ echo 'true | time false' | $fish 2>| string replace -r '(.*)' '<$1>' # CHECK: < ^~~~~~~~~^> -echo ' - -FOO=BAR (true one) -(true two) - -# more things -' | $fish 2>| string replace -r '(.*)' '<$1>' - -# CHECK: -# CHECK: -# CHECK: < ^~~~~~~~~^> - $fish -c 'echo "unfinished "(subshell' 2>| string replace -r '.*' '<$0>' # CHECK: # CHECK: