Allow newlines after && and ||

We do the same for pipes (#1285). This matches POSIX sh behavior.

CHANGELOG.rst

@@ -28,7 +28,8 @@ Notable improvements and fixes
 Syntax changes and new commands
 -------------------------------
 - A new ``fish_is_root_user`` simplifies checking for superuser privilege, largely for use in prompts (#7031).
--  Range limits in index range expansions like ``$x[$start..$end]`` may be omitted: ``$start`` and ``$end`` default to 1 and -1 (the last item) respectively.
+- Range limits in index range expansions like ``$x[$start..$end]`` may be omitted: ``$start`` and ``$end`` default to 1 and -1 (the last item) respectively.
+- Logical operators ``&&`` and ``||`` can be followed by newlines before their right operand, matching POSIX shells.
 
 Scripting improvements
 ----------------------

src/ast.h

@@ -730,11 +730,12 @@ struct job_conjunction_continuation_t final
     : public branch_t<type_t::job_conjunction_continuation> {
     // The && or || token.
     token_t<parse_token_type_t::andand, parse_token_type_t::oror> conjunction;
+    maybe_newlines_t newlines;
 
     // The job itself.
     job_t job;
 
-    FIELDS(conjunction, job)
+    FIELDS(conjunction, newlines, job)
 };
 
 // An andor_job just wraps a job, but requires that the job have an 'and' or 'or' job_decorator.

src/fish_tests.cpp

@@ -1190,6 +1190,18 @@ static void test_parser() {
         err(L"bogus boolean statement error not detected on line %d", __LINE__);
     }
 
+    if (detect_errors(L"true && ") != PARSER_TEST_INCOMPLETE) {
+        err(L"unterminated conjunction not reported properly");
+    }
+
+    if (detect_errors(L"true && \n true")) {
+        err(L"newline after && reported as error");
+    }
+
+    if (detect_errors(L"true || \n") != PARSER_TEST_INCOMPLETE) {
+        err(L"unterminated conjunction not reported properly");
+    }
+
     say(L"Testing basic evaluation");
 
     // Ensure that we don't crash on infinite self recursion and mutual recursion. These must use
@@ -4344,7 +4356,7 @@ static void test_new_parser_correctness() {
         {L"true || false; and true", true},
         {L"true || ||", false},
         {L"|| true", false},
-        {L"true || \n\n false", false},
+        {L"true || \n\n false", true},
     };
 
     for (const auto &test : parser_tests) {

src/parse_util.cpp

@@ -1215,6 +1215,10 @@ parser_test_error_bits_t parse_util_detect_errors(const ast::ast_t &ast, const w
     // detecting job_continuations that have source for pipes but not the statement.
     bool has_unclosed_pipe = false;
 
+    // Whether we encounter a missing job, i.e. a newline after && or ||. This is found by
+    // detecting job_conjunction_continuations that have source for && or || but not the job.
+    bool has_unclosed_conjunction = false;
+
     // Expand all commands.
     // Verify 'or' and 'and' not used inside pipelines.
     // Verify pipes via parser_is_pipe_forbidden.
@@ -1229,6 +1233,12 @@ parser_test_error_bits_t parse_util_detect_errors(const ast::ast_t &ast, const w
             if (!jc->pipe.unsourced && !jc->statement.try_source_range().has_value()) {
                 has_unclosed_pipe = true;
             }
+        } else if (const auto *jcc = node.try_as<job_conjunction_continuation_t>()) {
+            // Somewhat clumsy way of checking for a job without source in a conjunction.
+            // See if our conjunction operator (&& or ||) has source but our job does not.
+            if (!jcc->conjunction.unsourced && !jcc->job.try_source_range().has_value()) {
+                has_unclosed_conjunction = true;
+            }
         } else if (const argument_t *arg = node.try_as<argument_t>()) {
             const wcstring &arg_src = arg->source(buff_src, &storage);
             res |= parse_util_detect_errors_in_argument(*arg, arg_src, out_errors);
@@ -1262,7 +1272,8 @@ parser_test_error_bits_t parse_util_detect_errors(const ast::ast_t &ast, const w
 
     if (errored) res |= PARSER_TEST_ERROR;
 
-    if (has_unclosed_block || has_unclosed_pipe) res |= PARSER_TEST_INCOMPLETE;
+    if (has_unclosed_block || has_unclosed_pipe || has_unclosed_conjunction)
+        res |= PARSER_TEST_INCOMPLETE;
 
     return res;
 }

tests/checks/andandoror.fish

@@ -93,6 +93,24 @@ end
 # CHECK: 3
 # CHECK: 4
 
+# Newlines
+true &&
+echo newline after conjunction
+# CHECK: newline after conjunction
+false ||
+echo newline after disjunction
+# CHECK: newline after disjunction
+
+true &&
+
+echo empty lines after conjunction
+# CHECK: empty lines after conjunction
+
+true &&
+# can have comments here!
+echo comment after conjunction
+# CHECK: comment after conjunction
+
 # --help works
 builtin and --help >/dev/null
 echo $status

tests/checks/indent.fish

@@ -298,9 +298,23 @@ printf '%s\n' "a; and b" | $fish_indent
 printf '%s\n' "a;" "and b" | $fish_indent
 #CHECK: a
 #CHECK: and b
-printf '%s\n' "a" "; and b" | $fish_indent
+printf '%s\n' a "; and b" | $fish_indent
 #CHECK: a
 #CHECK: and b
 printf '%s\n' "a; b" | $fish_indent
 #CHECK: a
 #CHECK: b
+
+echo 'foo &&
+  #
+bar' | $fish_indent
+#CHECK: {{^}}foo &&
+#CHECK: {{^}}#
+#CHECK: {{^}}bar
+
+echo 'command 1 |
+command 1 cont ||
+command 2' | $fish_indent
+#CHECK: {{^}}command 1 |
+#CHECK: {{^    }}command 1 cont ||
+#CHECK: {{^}}command 2