Introduce test-require-arg feature flag

This removes builtin test's zero and one argument special modes.

That means:

- `test -n` returns false
- `test -z` returns true
- `test -x` with any other option errors out with "missing argument"
- `test foo` errors out as expecting an option

src/builtins/test.rs

@@ -1,5 +1,6 @@
 use super::prelude::*;
 use crate::common;
+use crate::future_feature_flags::{feature_test, FeatureFlag};
 
 mod test_expressions {
     use super::*;
@@ -552,6 +553,10 @@ mod test_expressions {
                 );
             }
 
+            if feature_test(FeatureFlag::test_require_arg) {
+                return self.error(start, sprintf!("Unknown option at index %u", start));
+            }
+
             return JustAString {
                 arg: self.arg(start).to_owned(),
                 range: start..start + 1,
@@ -718,9 +723,6 @@ mod test_expressions {
             err: &mut WString,
             program_name: &wstr,
         ) -> Option<Box<dyn Expression>> {
-            // Empty list and one-arg list should be handled by caller.
-            assert!(args.len() > 1);
-
             let mut parser = TestParser {
                 strings: args,
                 errors: Vec::new(),
@@ -1019,15 +1021,32 @@ pub fn test(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> Opt
         .collect();
     let args: &[WString] = &args;
 
-    if argc == 0 {
-        return STATUS_INVALID_ARGS; // Per 1003.1, exit false.
-    } else if argc == 1 {
-        // Per 1003.1, exit true if the arg is non-empty.
-        return if args[0].is_empty() {
-            STATUS_CMD_ERROR
-        } else {
-            STATUS_CMD_OK
-        };
+    if feature_test(FeatureFlag::test_require_arg) {
+        if argc == 0 {
+            streams.err.appendln(wgettext_fmt!(
+                "%ls: Expected at least one argument",
+                program_name
+            ));
+            builtin_print_error_trailer(parser, streams.err, program_name);
+            return STATUS_INVALID_ARGS;
+        } else if argc == 1 {
+            if args[0] == "-n" {
+                return STATUS_CMD_ERROR;
+            } else if args[0] == "-z" {
+                return STATUS_CMD_OK;
+            }
+        }
+    } else {
+        if argc == 0 {
+            return STATUS_INVALID_ARGS; // Per 1003.1, exit false.
+        } else if argc == 1 {
+            // Per 1003.1, exit true if the arg is non-empty.
+            return if args[0].is_empty() {
+                STATUS_CMD_ERROR
+            } else {
+                STATUS_CMD_OK
+            };
+        }
     }
 
     // Try parsing

src/future_feature_flags.rs

@@ -24,6 +24,9 @@ pub enum FeatureFlag {
     ampersand_nobg_in_token,
     /// Whether "%self" is expanded to fish's pid
     remove_percent_self,
+
+    /// Remove `test`'s one and zero arg mode (make `test -n` return false etc)
+    test_require_arg,
 }
 
 struct Features {
@@ -96,6 +99,14 @@ pub const METADATA: &[FeatureMetadata] = &[
         default_value: false,
         read_only: false,
     },
+    FeatureMetadata {
+        flag: FeatureFlag::test_require_arg,
+        name: L!("test-require-arg"),
+        groups: L!("3.8"),
+        description: L!("builtin test requires an argument"),
+        default_value: false,
+        read_only: false,
+    },
 ];
 
 thread_local!(
@@ -156,6 +167,7 @@ impl Features {
                 AtomicBool::new(METADATA[2].default_value),
                 AtomicBool::new(METADATA[3].default_value),
                 AtomicBool::new(METADATA[4].default_value),
+                AtomicBool::new(METADATA[5].default_value),
             ],
         }
     }

tests/checks/status.fish

@@ -58,6 +58,7 @@ status features
 #CHECK: regex-easyesc           on  3.1 string replace -r needs fewer \'s
 #CHECK: ampersand-nobg-in-token on  3.4 & only backgrounds if followed by a separator
 #CHECK: remove-percent-self     off 3.8 %self is no longer expanded (use $fish_pid)
+#CHECK: test-require-arg        off 3.8 builtin test requires an argument
 status test-feature stderr-nocaret
 echo $status
 #CHECK: 0

tests/checks/test-posix.fish

@@ -0,0 +1,27 @@
+# RUN: %fish %s
+#
+# Tests for the posix-mandated zero and one argument modes for the `test` builtin, aka `[`.
+
+test -n
+echo $status
+#CHECK: 0
+
+test -z
+echo $status
+#CHECK: 0
+
+test -d
+echo $status
+#CHECK: 0
+
+test "foo"
+echo $status
+#CHECK: 0
+
+test ""
+echo $status
+#CHECK: 1
+
+test -z "" -a foo
+echo $status
+#CHECK: 0

tests/checks/test.fish

@@ -1,4 +1,4 @@
-# RUN: %fish %s
+# RUN: %fish --features test-require-arg %s
 #
 # Tests for the `test` builtin, aka `[`.
 test inf -gt 0
@@ -94,3 +94,51 @@ test epoch -ef old && echo bad ef || echo good ef
 #CHECK: good ef
 
 rm -f epoch old newest epochlink
+
+test -n
+echo -- -n $status
+#CHECK: -n 1
+test -z
+echo -- -z $status
+#CHECK: -z 0
+
+test -d
+#CHECKERR: test: Missing argument at index 2
+#CHECKERR: -d
+#CHECKERR: ^
+#CHECKERR: {{.*}}test.fish (line {{\d+}}): 
+#CHECKERR: test -d
+#CHECKERR: ^
+
+test "foo"
+#CHECKERR: test: Missing argument at index 2
+#CHECKERR: foo
+#CHECKERR: ^
+#CHECKERR: {{.*}}test.fish (line {{\d+}}): 
+#CHECKERR: test "foo"
+#CHECKERR: ^
+
+test ""
+#CHECKERR: test: Missing argument at index 2
+#CHECKERR: ^
+#CHECKERR: {{.*}}test.fish (line {{\d+}}): 
+#CHECKERR: test ""
+#CHECKERR: ^
+
+test -z "" -a foo
+#CHECKERR: test: Missing argument at index 5
+#CHECKERR: -z  -a foo
+#CHECKERR: ^
+#CHECKERR: {{.*}}test.fish (line {{\d+}}): 
+#CHECKERR: test -z "" -a foo
+#CHECKERR: ^
+
+echo $status
+#CHECK: 1
+
+test
+#CHECKERR: test: Expected at least one argument
+#CHECKERR: {{.*}}test.fish (line {{\d+}}): 
+#CHECKERR: test
+#CHECKERR: ^
+#CHECKERR: (Type 'help test' for related documentation)