Relax evaluation of "$@" to string, but add set -o strict-array.

This is the first pass -- there is stlil a failing test which involves "decay_array" in word_eval.py. Positive fallout: - Two more dbracket spec tests pass. (But strict-array will disallow this behavior.) - Fix to return status 1 if you redirect to "$@", '' or a string that doesn't look like a file descriptor. (Whether it aborts depends on set -e.) This also fixes a few spec test failures. Addresses issue #61. Alpine uses local _quiet="$@" (which might be a typo honestly, because elsewhere they use local _quiet="$1".)
oilshell · Jan 11, 2018 · e9e8bf44e5ed270102fe4ddba6cfcf483f67aceb · e9e8bf4
1 parent 7a34048
commit e9e8bf44e5ed270102fe4ddba6cfcf483f67aceb
Unified Split

Showing with 77 additions and 30 deletions.

+8 −8 core/cmd_exec.py

+8 −3 core/state.py

+16 −11 core/word_eval.py

+33 −4 spec/array-compat.test.sh

+10 −2 spec/dbracket.test.sh

+2 −2 test/spec.sh
diff --git a/core/cmd_exec.py b/core/cmd_exec.py
@@ -400,29 +400,29 @@ def _EvalRedirect(self, n):
        # NOTE: no globbing.  You can write to a file called '*.py'.
        val = self.word_ev.EvalWordToString(n.arg_word)
        if val.tag != value_e.Str:  # TODO: This error never fires
-          util.warn("Redirect filename must be a string, got %s", val)
+          util.error("Redirect filename must be a string, got %s", val)
          return None
        filename = val.s
        if not filename:
          # Whether this is fatal depends on errexit.
-          util.warn("Redirect filename can't be empty")
+          util.error("Redirect filename can't be empty")
          return None

        return runtime.PathRedirect(n.op_id, fd, filename)

      elif redir_type == RedirType.Desc:  # e.g. 1>&2
        val = self.word_ev.EvalWordToString(n.arg_word)
        if val.tag != value_e.Str:  # TODO: This error never fires
-          util.warn("Redirect descriptor should be a string, got %s", val)
+          util.error("Redirect descriptor should be a string, got %s", val)
          return None
        t = val.s
        if not t:
-          util.warn("Redirect descriptor can't be empty")
+          util.error("Redirect descriptor can't be empty")
          return None
        try:
          target_fd = int(t)
        except ValueError:
-          util.warn(
+          util.error(
              "Redirect descriptor should look like an integer, got %s", val)
          return None

@@ -466,9 +466,7 @@ def _EvalRedirects(self, node):
    for redir in node.redirects:
      r = self._EvalRedirect(redir)
      if r is None:
-        # Ignore it for now.  TODO: We might want to skip JUST the command.
-        # Give it status 1, and then errexit will take care of it.
-        continue
+        return None  # bad redirect
      redirects.append(r)
    return redirects

@@ -1272,6 +1270,8 @@ def RunFunc(self, func_node, argv):
    # f 2>&1

    def_redirects = self._EvalRedirects(func_node)
+    if def_redirects is None:
+      return None
    if not self.fd_state.Push(def_redirects, self.waiter):
      return 1  # error

diff --git a/core/state.py b/core/state.py
@@ -85,6 +85,7 @@ def Disable(self):

    (None, 'strict-control-flow'),
    (None, 'strict-errexit'),
+    (None, 'strict-array'),
 ]

 _SET_OPTION_NAMES = set(name for _, name in SET_OPTIONS)
@@ -123,6 +124,13 @@ def __init__(self, mem):
    # local still needs to fail.
    self.strict_errexit = False

+    # Several problems:
+    # - foo="$@" not allowed because it decays.  Should be foo=( "$@" ).
+    # - ${a} not ${a[0]}
+    # - possibly disallow $* "$*" altogether.
+    # - do not allow [[ "$@" == "${a[@]}" ]]
+    self.strict_array = False
+
    # This comes after all the 'set' options.
    shellopts = self.mem.GetVar('SHELLOPTS')
    assert shellopts.tag == value_e.Str, shellopts
@@ -137,9 +145,6 @@ def __init__(self, mem):
    #

    self.strict_arith = False  # e.g. $(( x )) where x doesn't look like integer
-    self.strict_array = False  # ${a} not ${a[0]}, require double quotes, etc.
-                               # compare array for equality?  Or just use a
-                               # different syntax.
    self.strict_word = False  # word splitting, etc.
    self.strict_scope = False  # disable dynamic scope
    # TODO: strict_bool.  Some of this is covered by arithmetic, e.g. -eq.
diff --git a/core/word_eval.py b/core/word_eval.py
@@ -720,19 +720,24 @@ def EvalWordToString(self, word, do_fnmatch=False, decay=False):
      # TODO: if decay, then allow string part.  e.g. for here word or here
      # doc with "$@".

-      if part_val.tag != part_value_e.StringPartValue:
-        # Examples: echo f > "$@"; local foo="$@"
-        e_die("Expected string, got %s", part_val, word=word)
+      if part_val.tag == part_value_e.StringPartValue:
+        # [[ foo == */"*".py ]] or case *.py) ... esac
+        if do_fnmatch and not part_val.do_split_glob:
+          s = glob_.GlobEscape(part_val.s)
+        else:
+          s = part_val.s
+      else:
+        if self.exec_opts.strict_array:
+          # Examples: echo f > "$@"; local foo="$@"
+          e_die("Expected string, got %s", part_val, word=word)

-        # TODO: Maybe add detail like this.
-        #e_die('RHS of assignment should only have strings.  '
-        #      'To assign arrays, using b=( "${a[@]}" )')
+          # TODO: Maybe add detail like this.
+          #e_die('RHS of assignment should only have strings.  '
+          #      'To assign arrays, using b=( "${a[@]}" )')
+        else:
+          # It appears to not respect IFS
+          s = ' '.join(part_val.strs)

-      # [[ foo == */"*".py ]] or case *.py) ... esac
-      if do_fnmatch and not part_val.do_split_glob:
-        s = glob_.GlobEscape(part_val.s)
-      else:
-        s = part_val.s
      strs.append(s)

    return runtime.Str(''.join(strs))
diff --git a/spec/array-compat.test.sh b/spec/array-compat.test.sh
@@ -5,7 +5,17 @@

 ### Assignment Causes Array Decay
 set -- x y z
-#argv "[$@]"  # NOT DECAYED here.
+argv.py "[$@]"
+var="[$@]"
+argv.py "$var"
+## STDOUT:
+['[x', 'y', 'z]']
+['[x y z]']
+## END
+
+### Array Decay with IFS
+IFS=x
+set -- x y z
 var="[$@]"
 argv.py "$var"
 # stdout: ['[x y z]']
@@ -16,8 +26,14 @@ a=(x y z)
 b="${a[@]}"  # this collapses to a string
 c=("${a[@]}")  # this preserves the array
 c[1]=YYY  # mutate a copy -- doesn't affect the original
-argv.py "${a[@]}" "${b[@]}" "${c[@]}"
-# stdout: ['x', 'y', 'z', 'x y z', 'x', 'YYY', 'z']
+argv.py "${a[@]}"
+argv.py "${b[@]}"
+argv.py "${c[@]}"
+## STDOUT:
+['x', 'y', 'z']
+['x y z']
+['x', 'YYY', 'z']
+## END

 ### $a gives first element of array
 a=(1 '2 3')
@@ -43,11 +59,24 @@ a=('1 2' '3 4')
 s='1 2 3 4'  # length 2, length 4
 echo ${#a[@]} ${#s}
 [[ "${a[@]}" = "$s" ]] && echo EQUAL
-# stdout-json: "2 7\nEQUAL\n"
+## STDOUT:
+2 7
+EQUAL
+## END

 ### Increment array variables
 a=(1 2)
 (( a++ ))  # doesn't make sense
 echo "${a[@]}"
 # stdout: 2 2

+### Apply vectorized operations on ${a[*]}
+a=('-x-' 'y-y' '-z-')
+
+# This does the prefix stripping FIRST, and then it joins.
+argv.py "${a[*]#-}"
+## STDOUT:
+['x- y-y z-']
+## END
+## N-I mksh status: 1
+## N-I mksh stdout-json: ""
diff --git a/spec/dbracket.test.sh b/spec/dbracket.test.sh
@@ -175,18 +175,26 @@ FOO=bar [[ foo == foo ]]
 ### Does user array equal "$@" ?
 # Oh it coerces both to a string.  Lame.
 # I think it disobeys "${a[@]}", and treats it like an UNQUOTED ${a[@]}.
+# NOTE: set -o strict-array should make this invalid
 a=(1 3 5)
 b=(1 2 3)
 set -- 1 3 5
 [[ "$@" = "${a[@]}" ]] && echo true
 [[ "$@" = "${b[@]}" ]] || echo false
-# stdout-json: "true\nfalse\n"
+## STDOUT:
+true
+false
+## END

 ### Array coerces to string
+# NOTE: set -o strict-array should make this invalid
 a=(1 3 5)
 [[ '1 3 5' = "${a[@]}" ]] && echo true
 [[ '1 3 4' = "${a[@]}" ]] || echo false
-# stdout-json: "true\nfalse\n"
+## STDOUT:
+true
+false
+## END

 ### Quotes don't matter in comparison
 [[ '3' = 3 ]] && echo true
diff --git a/test/spec.sh b/test/spec.sh
@@ -361,7 +361,7 @@ here-doc() {
 }

 redirect() {
-  sh-spec spec/redirect.test.sh --osh-failures-allowed 8 \
+  sh-spec spec/redirect.test.sh --osh-failures-allowed 6 \
    ${REF_SHELLS[@]} $OSH "$@"
 }

@@ -486,7 +486,7 @@ assoc-zsh() {
 # NOTE: zsh passes about half and fails about half.  It supports a subset of [[
 # I guess.
 dbracket() {
-  sh-spec spec/dbracket.test.sh --osh-failures-allowed 4 \
+  sh-spec spec/dbracket.test.sh --osh-failures-allowed 2 \
    $BASH $MKSH $OSH "$@"
  #sh-spec spec/dbracket.test.sh $BASH $MKSH $OSH $ZSH "$@"
 }