[completion] Implement complete/compgen -W properly.

It now does word evaluation and splitting.

- Added a spec test for a related case where where complete/compgen -F
  swallows errors.
- Add test cases for runtime errors in the interactive 'complete' case.

Addresses issue #208.

core/completion.py

@@ -37,6 +37,7 @@
 import pwd
 import time
 
+from core import ui
 from core import util
 from core.meta import (
     Id, REDIR_ARG_TYPES, syntax_asdl, runtime_asdl, types_asdl)
@@ -270,17 +271,26 @@ def Matches(self, comp):
 
 
 class DynamicWordsAction(CompletionAction):
-  # NOTE: Have to split the words passed to -W.  Using IFS or something else?
-  def __init__(self, words, delay=None):
-    self.words = words
-    self.delay = delay
+  """ compgen -W '$(echo one two three)' """
+
+  def __init__(self, word_ev, splitter, arg_word, arena):
+    self.word_ev = word_ev
+    self.splitter = splitter
+    self.arg_word = arg_word
+    self.arena = arena
 
   def Matches(self, comp):
-    for w in self.words:
-      if w.startswith(comp.to_complete):
-        if self.delay:
-          time.sleep(self.delay)
-        yield w
+    try:
+      val = self.word_ev.EvalWordToString(self.arg_word)
+    except util.FatalRuntimeError as e:
+      ui.PrettyPrintError(e, self.arena)
+      raise
+
+    # SplitForWordEval() Allows \ escapes
+    candidates = self.splitter.SplitForWordEval(val.s)
+    for c in candidates:
+      if c.startswith(comp.to_complete):
+        yield c
 
 
 class FileSystemAction(CompletionAction):
@@ -946,6 +956,13 @@ def __call__(self, unused_word, state):
     """Return a single match."""
     try:
       return self._GetNextCompletion(state)
+    except util.FatalRuntimeError as e:
+      # From -W.  TODO: -F is swallowed now.
+      # We should have a nicer UI for displaying errors.  Maybe they shouldn't
+      # print it to stderr.  That messes up the completion display.  We could
+      # print what WOULD have been COMPREPLY here.
+      log('Runtime error while completing: %s', e)
+      self.debug_f.log('Runtime error while completing: %s', e)
     except Exception as e:  # ESSENTIAL because readline swallows exceptions.
       import traceback
       traceback.print_exc()

osh/builtin_comp.py

@@ -6,6 +6,7 @@
 import pwd
 
 from core import completion
+from core import ui
 from core import util
 from core.meta import runtime_asdl
 from frontend import args
@@ -198,9 +199,26 @@ def Build(self, argv, arg, comp_opts):
 
     # e.g. -W comes after -A directory
     if arg.W:
-      # TODO: Split with IFS.  Is that done at registration time or completion
-      # time?
-      actions.append(completion.DynamicWordsAction(arg.W.split()))
+      # NOTES:
+      # - Parsing is done at REGISTRATION time, but execution and splitting is
+      #   done at COMPLETION time (when the user hits tab).  So parse errors
+      #   happen early.
+      # - It's OK to reuse the same arena because this doesn't happen during
+      #   translation.  TODO: But we need PushSource().  We should
+      #   create SideArena instances that track back to the current location of
+      #   the 'complete' builtin.
+      arena = self.parse_ctx.arena
+      w_parser = self.parse_ctx.MakeWordParserForPlugin(arg.W, arena)
+
+      try:
+        arg_word = w_parser.ReadForPlugin()
+      except util.ParseError as e:
+        ui.PrettyPrintError(e, self.parse_ctx.arena)
+        raise  # Let 'complete' or 'compgen' return 2
+
+      a = completion.DynamicWordsAction(
+          self.word_ev, self.splitter, arg_word, arena)
+      actions.append(a)
 
     extra_actions = []
     if comp_opts.Get('plusdirs'):
@@ -268,7 +286,11 @@ def __call__(self, argv):
       return 0
 
     comp_opts = completion.Options(arg.opt_changes)
-    user_spec = self.spec_builder.Build(argv, arg, comp_opts)
+    try:
+      user_spec = self.spec_builder.Build(argv, arg, comp_opts)
+    except util.ParseError as e:
+      # error printed above
+      return 2
     for command in commands:
       self.comp_lookup.RegisterName(command, comp_opts, user_spec)
 
@@ -309,17 +331,29 @@ def __call__(self, argv):
     matched = False
 
     comp_opts = completion.Options(arg.opt_changes)
-    user_spec = self.spec_builder.Build(argv, arg, comp_opts)
+    try:
+      user_spec = self.spec_builder.Build(argv, arg, comp_opts)
+    except util.ParseError as e:
+      # error printed above
+      return 2
 
     # NOTE: Matching bash in passing dummy values for COMP_WORDS and COMP_CWORD,
     # and also showing ALL COMPREPLY reuslts, not just the ones that start with
     # the word to complete.
     matched = False 
     comp = completion.Api()
     comp.Update(first='compgen', to_complete=to_complete, prev='', index=-1)
-    for m, _ in user_spec.Matches(comp):
-      matched = True
-      print(m)
+    try:
+      for m, _ in user_spec.Matches(comp):
+        matched = True
+        print(m)
+    except util.FatalRuntimeError:
+      # - DynamicWordsAction: We already printed an error, so return failure.
+      return 1
+
+    # - ShellFuncAction: We do NOT get FatalRuntimeError.  We printed an error
+    # in the executor, but RunFuncForCompletion swallows failures.  See test
+    # case in builtin-completion.test.sh.
 
     # TODO:
     # - need to dedupe results.

osh/cmd_exec.py

@@ -1451,6 +1451,8 @@ def __init__(self, parse_ctx, exec_opts, mem, word_ev, f):
     self.word_ev = word_ev
     self.f = f  # can be the --debug-file as well
 
+    # NOTE: We could use the same arena, since this doesn't happen during
+    # translation.
     self.arena = alloc.SideArena('<$PS4>')
     self.parse_cache = {}  # PS4 value -> CompoundWord.  PS4 is scoped.
 

osh/cmd_parse.py

@@ -155,6 +155,7 @@ def _MakeAssignPair(parse_ctx, preparsed):
     code_str = ''.join(pieces)
 
     # NOTE: It's possible that an alias expansion underlies this, not a real file!
+    # We have to use a SideArena since this will happen during translation.
     line_num = 99
     source_name = 'TODO'
     arena = alloc.SideArena('<LHS array index at line %d of %s>' %

spec/builtin-completion.test.sh

@@ -339,3 +339,36 @@ spam
 eggs
 ham cheese
 ## END
+
+#### Parse errors for compgen -W and complete -W
+# bash doesn't detect as many errors because it lacks static parsing.
+compgen -W '${'
+echo status=$?
+complete -W '${' foo
+echo status=$?
+## STDOUT:
+status=2
+status=2
+## END
+## BUG bash STDOUT:
+status=1
+status=0
+## END
+
+#### Runtime errors for compgen -W 
+compgen -W 'foo $(( 1 / 0 )) bar'
+echo status=$?
+## STDOUT:
+status=1
+## END
+
+#### Runtime errors for compgen -F func
+_foo() {
+  COMPREPLY=( foo bar )
+  COMPREPLY+=( $(( 1 / 0 )) )  # FATAL, but we still have candidates
+}
+compgen -F _foo foo
+echo status=$?
+## STDOUT:
+status=1
+## END

test/spec.sh

@@ -360,7 +360,7 @@ builtin-bash() {
 
 # This is bash/OSH only
 builtin-completion() {
-  sh-spec spec/builtin-completion.test.sh --osh-failures-allowed 2 \
+  sh-spec spec/builtin-completion.test.sh --osh-failures-allowed 1 \
     $BASH $OSH_LIST "$@"
 }
 

testdata/completion/osh-unit.bash

@@ -168,6 +168,16 @@ complete -F complete_op_chars -o filenames op_chars_filenames
 W_runtime() { argv "$@"; }
 complete -W 'foo $x $(echo --$x)' W_runtime
 
+W_runtime_error() { argv "$@"; }
+complete -W 'foo $(( 1 / 0 )) bar' W_runtime_error
+
+F_runtime_error() { argv "$@"; }
+complete_F_runtime_error() {
+  COMPREPLY=( foo bar )
+  COMPREPLY+=( $(( 1 / 0 )) )  # FATAL, but we still have candidates
+}
+complete -F complete_F_runtime_error F_runtime_error
+
 #
 # Unit tests use this
 #