source: Return error instead of implicitly reading from tty

For things like

    source $undefined

or
    source (nooutput)

it was quite annoying that it read from tty.

Instead we now require a "-" as the filename to read from the tty.

This does not apply to reading from stdin if it's redirected, so

    something | source

still works.

Fixes #2633.

CHANGELOG.md

@@ -23,6 +23,7 @@ fish 3.0 is a major release which brings with it both improvements in functional
 - Background jobs not first `disown`'d will be reaped upon `exec`, bringing the behavior in line with that of `exit`.
 - `read` now uses `-s` as short for `--silent` (à la `bash`); `--shell`'s abbreviation (formerly `-s`) is now `-S` instead (#4490).
 - `cd` no longer resolves symlinks. fish now maintains a virtual path, matching other shells. (#3350).
+- `source` now requires an explicit `-` as the filename to read from the terminal (#2633).
 
 ## Notable fixes and improvements
 ### Syntax/semantic changes and new builtins

doc_src/source.txt

@@ -3,13 +3,14 @@
 \subsection source-synopsis Synopsis
 \fish{synopsis}
 source FILENAME [ARGUMENTS...]
+somecommand | source
 \endfish
 
 \subsection source-description Description
 
 `source` evaluates the commands of the specified file in the current shell. This is different from starting a new process to perform the commands (i.e. `fish < FILENAME`) since the commands will be evaluated by the current shell, which means that changes in shell variables will affect the current shell. If additional arguments are specified after the file name, they will be inserted into the `$argv` variable. The `$argv` variable will not include the name of the sourced file.
 
-If no file is specified, or if the file name '`-`' is used, stdin will be read.
+If no file is specified and stdin is not the terminal, or if the file name '`-`' is used, stdin will be read.
 
 The return status of `source` is the return status of the last job to execute. If something goes wrong while opening or reading the file, `source` exits with a non-zero status.
 

src/builtin_source.cpp

@@ -41,6 +41,10 @@ int builtin_source(parser_t &parser, io_streams_t &streams, wchar_t **argv) {
 
     if (argc == optind || wcscmp(argv[optind], L"-") == 0) {
         // Either a bare `source` which means to implicitly read from stdin or an explicit `-`.
+        if (argc == optind && !streams.stdin_is_directly_redirected) {
+            // Don't implicitly read from the terminal.
+            return STATUS_CMD_ERROR;
+        }
         fn = L"-";
         fn_intern = fn;
         fd = dup(streams.stdin_fd);

tests/test1.in

@@ -213,6 +213,9 @@ echo 'echo "source argv {$argv}"' | source
 echo 'echo "source argv {$argv}"' | source -
 echo 'echo "source argv {$argv}"' | source - abc
 echo 'echo "source argv {$argv}"' | source - abc def
+# This hangs if it fails!
+source
+echo $status
 
 always_fails
 echo $status

tests/test1.out

@@ -140,3 +140,4 @@ source argv {}
 source argv {abc}
 source argv {abc def}
 1
+1