From e32572e4e68f93bc4f418ccf601c396e36bf4131 Mon Sep 17 00:00:00 2001 From: Johannes Altmanninger Date: Sun, 17 Nov 2024 09:02:20 +0100 Subject: [PATCH] Allow if/then/fi, while/do/done and for/do/done --- share/completions/do.fish | 1 + share/completions/then.fish | 1 + src/ast.rs | 99 +++++++++++++++++++++++++++++++++---- src/builtins/shared.rs | 20 ++++++++ src/highlight/highlight.rs | 4 ++ src/parse_constants.rs | 12 +++++ src/parse_tree.rs | 2 +- src/parser_keywords.rs | 4 ++ tests/checks/indent.fish | 9 ++++ 9 files changed, 141 insertions(+), 11 deletions(-) create mode 100644 share/completions/do.fish create mode 100644 share/completions/then.fish diff --git a/share/completions/do.fish b/share/completions/do.fish new file mode 100644 index 000000000..8b4769aab --- /dev/null +++ b/share/completions/do.fish @@ -0,0 +1 @@ +complete -c do -xa '(__fish_complete_subcommand)' diff --git a/share/completions/then.fish b/share/completions/then.fish new file mode 100644 index 000000000..4a39a2fc5 --- /dev/null +++ b/share/completions/then.fish @@ -0,0 +1 @@ +complete -c then -xa '(__fish_complete_subcommand)' diff --git a/src/ast.rs b/src/ast.rs index 9b4825940..0136f3762 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -79,8 +79,10 @@ fn visit_decorated_statement_decorator( _node: &mut Option, ); fn visit_job_conjunction_decorator(&mut self, _node: &mut Option); + fn visit_do(&mut self, _node: &mut Option); fn visit_else_clause(&mut self, _node: &mut Option); fn visit_semi_nl(&mut self, _node: &mut Option); + fn visit_then(&mut self, _node: &mut Option); fn visit_time(&mut self, _node: &mut Option); fn visit_token_background(&mut self, _node: &mut Option); } @@ -977,6 +979,12 @@ macro_rules! visit_optional_field_mut { (KeywordTime, $field:expr, $visitor:ident) => { $visitor.visit_time(&mut $field); }; + (KeywordDo, $field:expr, $visitor:ident) => { + $visitor.visit_do(&mut $field); + }; + (KeywordThen, $field:expr, $visitor:ident) => { + $visitor.visit_then(&mut $field); + }; (TokenBackground, $field:expr, $visitor:ident) => { $visitor.visit_token_background(&mut $field); }; @@ -1315,7 +1323,11 @@ fn can_be_parsed(pop: &mut Populator<'_>) -> bool { || (token.typ == ParseTokenType::string && !matches!( token.keyword, - ParseKeyword::kw_case | ParseKeyword::kw_end | ParseKeyword::kw_else + ParseKeyword::kw_case + | ParseKeyword::kw_done + | ParseKeyword::kw_else + | ParseKeyword::kw_end + | ParseKeyword::kw_fi )) } } @@ -1333,6 +1345,8 @@ pub struct ForHeader { pub args: ArgumentList, /// newline or semicolon pub semi_nl: SemiNl, + // Optional "do" for compatibility with POSIX. + pub kw_do: Option, } implement_node!(ForHeader, branch, for_header); implement_acceptor_for_branch!( @@ -1342,6 +1356,7 @@ pub struct ForHeader { (kw_in: (KeywordIn)), (args: (ArgumentList)), (semi_nl: (SemiNl)), + (kw_do: (Option)), ); impl ConcreteNode for ForHeader { fn as_for_header(&self) -> Option<&ForHeader> { @@ -1361,6 +1376,8 @@ pub struct WhileHeader { pub kw_while: KeywordWhile, pub condition: JobConjunction, pub andor_tail: AndorJobList, + // Optional "do" for compatibility with POSIX. + pub kw_do: Option, } implement_node!(WhileHeader, branch, while_header); implement_acceptor_for_branch!( @@ -1368,6 +1385,7 @@ pub struct WhileHeader { (kw_while: (KeywordWhile)), (condition: (JobConjunction)), (andor_tail: (AndorJobList)), + (kw_do: (Option)), ); impl ConcreteNode for WhileHeader { fn as_while_header(&self) -> Option<&WhileHeader> { @@ -1441,7 +1459,7 @@ pub struct BlockStatement { /// List of jobs in this block. pub jobs: JobList, /// The 'end' node. - pub end: KeywordEnd, + pub end: KeywordEndOrDone, /// Arguments and redirections associated with the block. pub args_or_redirs: ArgumentOrRedirectionList, } @@ -1450,7 +1468,7 @@ pub struct BlockStatement { BlockStatement, (header: (variant)), (jobs: (JobList)), - (end: (KeywordEnd)), + (end: (KeywordEndOrDone)), (args_or_redirs: (ArgumentOrRedirectionList)), ); impl ConcreteNode for BlockStatement { @@ -1504,6 +1522,10 @@ pub struct IfClause { pub condition: JobConjunction, /// 'and/or' tail. pub andor_tail: AndorJobList, + // Semi + pub semi_nl: SemiNl, + // Optional "then" for compatibility with POSIX. + pub kw_then: Option, /// The body to execute if the condition is true. pub body: JobList, } @@ -1513,6 +1535,7 @@ pub struct IfClause { (kw_if: (KeywordIf)), (condition: (JobConjunction)), (andor_tail: (AndorJobList)), + (kw_then: (Option)), (body: (JobList)), ); impl ConcreteNode for IfClause { @@ -1610,7 +1633,7 @@ pub struct IfStatement { /// else part pub else_clause: Option, /// literal end - pub end: KeywordEnd, + pub end: KeywordEndOrFi, /// block args / redirs pub args_or_redirs: ArgumentOrRedirectionList, } @@ -1620,7 +1643,7 @@ pub struct IfStatement { (if_clause: (IfClause)), (elseif_clauses: (ElseifClauseList)), (else_clause: (Option)), - (end: (KeywordEnd)), + (end: (KeywordEndOrFi)), (args_or_redirs: (ArgumentOrRedirectionList)), ); impl ConcreteNode for IfStatement { @@ -2073,14 +2096,18 @@ fn can_be_parsed(pop: &mut Populator<'_>) -> bool { define_keyword_node!(JobConjunctionDecorator, kw_and, kw_or); define_keyword_node!(KeywordBegin, kw_begin); define_keyword_node!(KeywordCase, kw_case); +define_keyword_node!(KeywordDo, kw_do); define_keyword_node!(KeywordElse, kw_else); define_keyword_node!(KeywordEnd, kw_end); +define_keyword_node!(KeywordEndOrDone, kw_end, kw_done); +define_keyword_node!(KeywordEndOrFi, kw_end, kw_fi); define_keyword_node!(KeywordFor, kw_for); define_keyword_node!(KeywordFunction, kw_function); define_keyword_node!(KeywordIf, kw_if); define_keyword_node!(KeywordIn, kw_in); define_keyword_node!(KeywordNot, kw_not, kw_exclam); define_keyword_node!(KeywordSwitch, kw_switch); +define_keyword_node!(KeywordThen, kw_then); define_keyword_node!(KeywordTime, kw_time); define_keyword_node!(KeywordWhile, kw_while); @@ -2125,6 +2152,26 @@ fn can_be_parsed(pop: &mut Populator<'_>) -> bool { } } +impl CheckParse for KeywordDo { + fn can_be_parsed(pop: &mut Populator<'_>) -> bool { + let keyword = pop.peek_token(0).keyword; + if !matches!(keyword, ParseKeyword::kw_do) { + return false; + } + !pop.peek_token(1).is_dash_prefix_string() + } +} + +impl CheckParse for KeywordThen { + fn can_be_parsed(pop: &mut Populator<'_>) -> bool { + let keyword = pop.peek_token(0).keyword; + if !matches!(keyword, ParseKeyword::kw_then) { + return false; + } + !pop.peek_token(1).is_dash_prefix_string() + } +} + impl DecoratedStatement { /// Return the decoration for this statement. pub fn decoration(&self) -> StatementDecoration { @@ -3161,7 +3208,10 @@ fn did_visit_fields_of<'a>(&'a mut self, node: &'a dyn NodeMut, flow: VisitResul if next_token.typ == ParseTokenType::string && matches!( next_token.keyword, - ParseKeyword::kw_case | ParseKeyword::kw_else | ParseKeyword::kw_end + ParseKeyword::kw_case + | ParseKeyword::kw_else + | ParseKeyword::kw_end + | ParseKeyword::kw_fi ) { self.consume_excess_token_generating_error(); @@ -3245,6 +3295,12 @@ fn visit_semi_nl(&mut self, node: &mut Option) { fn visit_time(&mut self, node: &mut Option) { *node = self.try_parse::().map(|b| *b); } + fn visit_do(&mut self, node: &mut Option) { + *node = self.try_parse::().map(|b| *b); + } + fn visit_then(&mut self, node: &mut Option) { + *node = self.try_parse::().map(|b| *b); + } fn visit_token_background(&mut self, node: &mut Option) { *node = self.try_parse::().map(|b| *b); } @@ -3546,6 +3602,14 @@ fn consume_excess_token_generating_error(&mut self) { "'case' builtin not inside of switch block" ); } + ParseKeyword::kw_done => { + parse_error!( + self, + tok, + ParseErrorCode::unbalancing_case, + "'done' outside of a block" + ); + } ParseKeyword::kw_end => { parse_error!( self, @@ -3562,6 +3626,14 @@ fn consume_excess_token_generating_error(&mut self) { "'else' builtin not inside of if block" ); } + ParseKeyword::kw_fi => { + parse_error!( + self, + tok, + ParseErrorCode::unbalancing_end, + "'fi' outside of a block" + ); + } _ => { internal_error!( self, @@ -3819,9 +3891,11 @@ fn new_decorated_statement(slf: &mut Populator<'_>) -> StatementVariant { // looks like an option (starts with a dash), then parse it as a decorated statement. let help_only_kws = [ ParseKeyword::kw_begin, + ParseKeyword::kw_do, ParseKeyword::kw_function, ParseKeyword::kw_if, ParseKeyword::kw_switch, + ParseKeyword::kw_then, ParseKeyword::kw_while, ]; if if help_only_kws.contains(&self.peek_token(0).keyword) { @@ -3834,8 +3908,13 @@ fn new_decorated_statement(slf: &mut Populator<'_>) -> StatementVariant { // Likewise if the next token doesn't look like an argument at all. This corresponds to // e.g. a "naked if". - let naked_invocation_invokes_help = ![ParseKeyword::kw_begin, ParseKeyword::kw_end] - .contains(&self.peek_token(0).keyword); + let naked_invocation_invokes_help = ![ + ParseKeyword::kw_begin, + ParseKeyword::kw_done, + ParseKeyword::kw_end, + ParseKeyword::kw_fi, + ] + .contains(&self.peek_token(0).keyword); if naked_invocation_invokes_help && [ParseTokenType::end, ParseTokenType::terminate] .contains(&self.peek_token(1).typ) @@ -3864,7 +3943,7 @@ fn new_decorated_statement(slf: &mut Populator<'_>) -> StatementVariant { let embedded = self.allocate_visit::(); StatementVariant::SwitchStatement(embedded) } - ParseKeyword::kw_end => { + ParseKeyword::kw_done | ParseKeyword::kw_end | ParseKeyword::kw_fi => { // 'end' is forbidden as a command. // For example, `if end` or `while end` will produce this error. // We still have to descend into the decorated statement because @@ -4036,7 +4115,7 @@ fn visit_keyword(&mut self, keyword: &mut dyn Keyword) -> VisitResult { // Special error reporting for keyword_t. let allowed_keywords = keyword.allowed_keywords(); - if keyword.allowed_keywords() == [ParseKeyword::kw_end] { + if allowed_keywords.contains(&ParseKeyword::kw_end) { return VisitResult::Break(MissingEndError { allowed_keywords, token: *self.peek_token(0), diff --git a/src/builtins/shared.rs b/src/builtins/shared.rs index 400939772..54628c799 100644 --- a/src/builtins/shared.rs +++ b/src/builtins/shared.rs @@ -209,6 +209,14 @@ struct BuiltinData { name: L!("disown"), func: disown::disown, }, + BuiltinData { + name: L!("do"), + func: builtin_generic, + }, + BuiltinData { + name: L!("done"), + func: builtin_generic, + }, BuiltinData { name: L!("echo"), func: echo::echo, @@ -245,6 +253,10 @@ struct BuiltinData { name: L!("fg"), func: fg::fg, }, + BuiltinData { + name: L!("fi"), + func: builtin_generic, + }, BuiltinData { name: L!("fish_indent"), func: fish_indent::fish_indent, @@ -345,6 +357,10 @@ struct BuiltinData { name: L!("test"), func: test::test, }, + BuiltinData { + name: L!("then"), + func: builtin_generic, + }, BuiltinData { name: L!("time"), func: builtin_generic, @@ -504,6 +520,8 @@ pub fn builtin_get_desc(name: &wstr) -> Option<&'static wstr> { _ if name == "continue" => wgettext!("Skip over remaining innermost loop"), _ if name == "count" => wgettext!("Count the number of arguments"), _ if name == "disown" => wgettext!("Remove job from job list"), + _ if name == "do" => wgettext!("Optional after a loop header"), + _ if name == "done" => wgettext!("Alternative to 'end', to end a block of commands"), _ if name == "echo" => wgettext!("Print arguments"), _ if name == "else" => wgettext!("Evaluate block if condition is false"), _ if name == "emit" => wgettext!("Emit an event"), @@ -513,6 +531,7 @@ pub fn builtin_get_desc(name: &wstr) -> Option<&'static wstr> { _ if name == "exit" => wgettext!("Exit the shell"), _ if name == "false" => wgettext!("Return an unsuccessful result"), _ if name == "fg" => wgettext!("Send job to foreground"), + _ if name == "fi" => wgettext!("Alternative to 'end', to end a block of commands"), _ if name == "fish_key_reader" => wgettext!("explore what characters keyboard keys send"), _ if name == "for" => wgettext!("Perform a set of commands multiple times"), _ if name == "function" => wgettext!("Define a new function"), @@ -531,6 +550,7 @@ pub fn builtin_get_desc(name: &wstr) -> Option<&'static wstr> { _ if name == "realpath" => wgettext!("Show absolute path sans symlinks"), _ if name == "return" => wgettext!("Stop the currently evaluated function"), _ if name == "set" => wgettext!("Handle environment variables"), + _ if name == "then" => wgettext!("Optional after the condition of an if-statement"), _ if name == "set_color" => wgettext!("Set the terminal color"), _ if name == "source" => wgettext!("Evaluate contents of file"), _ if name == "status" => wgettext!("Return status information about fish"), diff --git a/src/highlight/highlight.rs b/src/highlight/highlight.rs index 8356f245b..16a738b00 100644 --- a/src/highlight/highlight.rs +++ b/src/highlight/highlight.rs @@ -846,14 +846,18 @@ fn visit_keyword(&mut self, node: &dyn Keyword) { | ParseKeyword::kw_builtin | ParseKeyword::kw_case | ParseKeyword::kw_command + | ParseKeyword::kw_do + | ParseKeyword::kw_done | ParseKeyword::kw_else | ParseKeyword::kw_end | ParseKeyword::kw_exec + | ParseKeyword::kw_fi | ParseKeyword::kw_for | ParseKeyword::kw_function | ParseKeyword::kw_if | ParseKeyword::kw_in | ParseKeyword::kw_switch + | ParseKeyword::kw_then | ParseKeyword::kw_while => role = HighlightRole::keyword, ParseKeyword::kw_and | ParseKeyword::kw_or diff --git a/src/parse_constants.rs b/src/parse_constants.rs index 4ad84fe9c..7e5888d57 100644 --- a/src/parse_constants.rs +++ b/src/parse_constants.rs @@ -92,10 +92,13 @@ pub enum ParseKeyword { kw_builtin, kw_case, kw_command, + kw_do, + kw_done, kw_else, kw_end, kw_exclam, kw_exec, + kw_fi, kw_for, kw_function, kw_if, @@ -103,6 +106,7 @@ pub enum ParseKeyword { kw_not, kw_or, kw_switch, + kw_then, kw_time, kw_while, } @@ -237,10 +241,13 @@ pub fn to_wstr(self) -> &'static wstr { ParseKeyword::kw_builtin => L!("builtin"), ParseKeyword::kw_case => L!("case"), ParseKeyword::kw_command => L!("command"), + ParseKeyword::kw_do => L!("do"), + ParseKeyword::kw_done => L!("done"), ParseKeyword::kw_else => L!("else"), ParseKeyword::kw_end => L!("end"), ParseKeyword::kw_exclam => L!("!"), ParseKeyword::kw_exec => L!("exec"), + ParseKeyword::kw_fi => L!("fi"), ParseKeyword::kw_for => L!("for"), ParseKeyword::kw_function => L!("function"), ParseKeyword::kw_if => L!("if"), @@ -248,6 +255,7 @@ pub fn to_wstr(self) -> &'static wstr { ParseKeyword::kw_not => L!("not"), ParseKeyword::kw_or => L!("or"), ParseKeyword::kw_switch => L!("switch"), + ParseKeyword::kw_then => L!("then"), ParseKeyword::kw_time => L!("time"), ParseKeyword::kw_while => L!("while"), _ => L!("unknown_keyword"), @@ -270,10 +278,13 @@ fn from(s: &wstr) -> Self { _ if s == "builtin" => ParseKeyword::kw_builtin, _ if s == "case" => ParseKeyword::kw_case, _ if s == "command" => ParseKeyword::kw_command, + _ if s == "do" => ParseKeyword::kw_do, + _ if s == "done" => ParseKeyword::kw_done, _ if s == "else" => ParseKeyword::kw_else, _ if s == "end" => ParseKeyword::kw_end, _ if s == "exec" => ParseKeyword::kw_exec, _ if s == "for" => ParseKeyword::kw_for, + _ if s == "fi" => ParseKeyword::kw_fi, _ if s == "function" => ParseKeyword::kw_function, _ if s == "if" => ParseKeyword::kw_if, _ if s == "in" => ParseKeyword::kw_in, @@ -281,6 +292,7 @@ fn from(s: &wstr) -> Self { _ if s == "or" => ParseKeyword::kw_or, _ if s == "switch" => ParseKeyword::kw_switch, _ if s == "time" => ParseKeyword::kw_time, + _ if s == "then" => ParseKeyword::kw_then, _ if s == "while" => ParseKeyword::kw_while, _ => ParseKeyword::none, } diff --git a/src/parse_tree.rs b/src/parse_tree.rs index eb940eba6..6922b5402 100644 --- a/src/parse_tree.rs +++ b/src/parse_tree.rs @@ -15,7 +15,7 @@ use crate::wcstringutil::count_newlines; /// A struct representing the token type that we use internally. -#[derive(Clone, Copy)] +#[derive(Clone, Copy, Debug)] pub struct ParseToken { /// The type of the token as represented by the parser pub typ: ParseTokenType, diff --git a/src/parser_keywords.rs b/src/parser_keywords.rs index 3cb110b2f..f56e36fc1 100644 --- a/src/parser_keywords.rs +++ b/src/parser_keywords.rs @@ -50,10 +50,13 @@ macro_rules! reserved_words { ("case"), ("command", [subcommand]), ("continue"), + ("do", [subcommand]), + ("done"), ("else", [subcommand]), ("end"), ("eval"), ("exec", [subcommand]), + ("fi"), ("for"), ("function"), ("if", [subcommand]), @@ -65,6 +68,7 @@ macro_rules! reserved_words { ("status"), ("string"), ("switch"), + ("then"), ("test"), ("time", [subcommand]), ("while", [subcommand]), diff --git a/tests/checks/indent.fish b/tests/checks/indent.fish index 911d92187..89ca9d4f9 100644 --- a/tests/checks/indent.fish +++ b/tests/checks/indent.fish @@ -614,3 +614,12 @@ echo 'echo "foo" "bar"' > $tmpdir/indent_test.fish $fish_indent --write $tmpdir/indent_test.fish cat $tmpdir/indent_test.fish # CHECK: echo foo bar + +# TODO: We should not force the line break. +echo 'if true; then + echo body +fi' | $fish_indent +# CHECK: if true +# CHECK: then +# CHECK: {{ }}echo body +# CHECK: fi