oilc deps: refine the algorithm for detecting functions.

Add a failing test for a case where it's still not fully correct. Also, accept oilc input from stdin.
oilshell · Feb 10, 2018 · d53ed8bab732e35a8da9b8c6619cb33d54aec3ca · d53ed8b
1 parent 537b49d
commit d53ed8bab732e35a8da9b8c6619cb33d54aec3ca
Unified Split

Showing with 139 additions and 58 deletions.

+31 −22 bin/oil.py

+2 −0 core/util.py

+47 −1 test/oilc.sh

+59 −35 tools/deps.py
diff --git a/bin/oil.py b/bin/oil.py
@@ -481,34 +481,43 @@ def OilCommandMain(argv):
  except IndexError:
    raise args.UsageError('oilc: Missing required subcommand.')

-  # NOTE: Does every oilc subcommand take a source path?  For now we assume it.
-  # TODO: fall back to stdin
+  if action not in SUBCOMMANDS:
+    raise args.UsageError('oilc: Invalid subcommand %r.' % action)
+
  try:
-    source_path = argv[1]
+    script_name = argv[1]
  except IndexError:
-    raise args.UsageError('oilc: Missing required source path.')
+    script_name = '<stdin>'
+    f = sys.stdin
+  else:
+    try:
+      f = open(script_name)
+    except IOError as e:
+      util.error("Couldn't open %r: %s", script_name, os.strerror(e.errno))
+      return 2

  pool = alloc.Pool()
  arena = pool.NewArena()
-  arena.PushSource(source_path)
+  arena.PushSource(script_name)

-  with open(source_path) as f:
-    line_reader = reader.FileLineReader(f, arena)
-    _, c_parser = parse_lib.MakeParser(line_reader, arena)
+  line_reader = reader.FileLineReader(f, arena)
+  _, c_parser = parse_lib.MakeParser(line_reader, arena)

-    try:
-      node = c_parser.ParseWholeFile()
-    except util.ParseError as e:
-      ui.PrettyPrintError(e, arena, sys.stderr)
-      print('parse error: %s' % e.UserErrorString(), file=sys.stderr)
-      return 2
-    else:
-      # TODO: Remove this older form of error handling.
-      if not node:
-        err = c_parser.Error()
-        assert err, err  # can't be empty
-        ui.PrintErrorStack(err, arena, sys.stderr)
-        return 2  # parse error is code 2
+  try:
+    node = c_parser.ParseWholeFile()
+  except util.ParseError as e:
+    ui.PrettyPrintError(e, arena, sys.stderr)
+    print('parse error: %s' % e.UserErrorString(), file=sys.stderr)
+    return 2
+  else:
+    # TODO: Remove this older form of error handling.
+    if not node:
+      err = c_parser.Error()
+      assert err, err  # can't be empty
+      ui.PrintErrorStack(err, arena, sys.stderr)
+      return 2  # parse error is code 2
+
+  f.close()

  # Columns for list-*
  # path line name
@@ -540,7 +549,7 @@ def OilCommandMain(argv):
    pass

  else:
-    raise args.UsageError('oilc: Invalid subcommand %r.' % action)
+    raise AssertionError  # Checked above

  return 0

diff --git a/core/util.py b/core/util.py
@@ -111,6 +111,8 @@ def warn(msg, *args):
  print('osh warning: ' + msg, file=sys.stderr)


+# NOTE: This should say 'oilc error' or 'oil error', instead of 'osh error' in
+# some cases.
 def error(msg, *args):
  if args:
    msg = msg % args
diff --git a/test/oilc.sh b/test/oilc.sh
@@ -16,6 +16,11 @@ fail() {
  exit 1
 }

+# Compare osh code on stdin (fd 0) and expected oil code on fd 3.
+assert-deps() {
+  bin/oilc deps | diff -u /dev/fd/3 - || fail
+}
+
 usage() {
 	set +o errexit

@@ -26,7 +31,12 @@ usage() {
  bin/oilc invalid
  test $? -eq 2 || fail

-  bin/oilc bin-deps
+	# Syntax error
+  echo '<' | bin/oilc deps 
+  test $? -eq 2 || fail
+
+	# File not found
+  bin/oilc deps nonexistent.txt
  test $? -eq 2 || fail

 	return
@@ -42,6 +52,42 @@ usage() {
 deps() {
  bin/oilc deps $0
  test $? -eq 0 || fail
+
+	# Have to go inside a condition
+	assert-deps <<EOF 3<<DEPS
+if { grep foo bar; } then
+	cat hi
+fi
+EOF
+grep
+cat
+DEPS
+
+	# g is used textually before defined, but that's OK
+	assert-deps <<EOF 3<<DEPS
+f() {
+	g
+}
+g() {
+	echo G
+}
+f
+grep foo bar
+EOF
+grep
+DEPS
+
+	# g is used before defined, NOT OK
+	assert-deps <<EOF 3<<DEPS
+g
+g() {
+	echo G
+}
+grep foo bar
+EOF
+g
+grep
+DEPS
 }

 readonly -a PASSING=(
diff --git a/tools/deps.py b/tools/deps.py
@@ -19,9 +19,9 @@

 # TODO: Move to asdl/visitor.py?
 class Visitor(object):
-  # In Python, they do introspection on method names.
+  # Python does introspection on method names:
  # method = 'visit_' + node.__class__.__name__
-  # I'm not going to bother, because I have ASDL!  I want the generic visitor.
+  # I'm using ASDL metaprogramming instead.

  def Visit(self, node):
    raise NotImplementedError
@@ -41,10 +41,10 @@ def VisitChildren(self, node):
      if isinstance(child, list):
        #log('Visiting child array %s', name)
        for item in child:
-          # We have to do this on an INSTANCE basis, not a type basis, because
-          # sums can be like:
+          # We have to check for compound objects on an INSTANCE basis, not a
+          # type basis, because sums can look liek this:
          # iterable = IterArgv | IterArray(word* words)
-          # We visit the latter but not the foramer.
+          # We visit the latter but not the former.
          if isinstance(item, py_meta.CompoundObj):
            self.Visit(item)
        continue
@@ -63,7 +63,13 @@ class DepsVisitor(Visitor):
  bin       cp            /usr/bin/cp               prog.sh     22
  lib       functions.sh  /home/andy/src/functions  prog.sh     22

-  TODO: Make this TSV2
+  TODO:
+  - Make this TSV2
+  - handle source and .
+  - flags like --path and --special exec
+  - need some knowledge of function scope.
+    f; f() { true; }  -- f is an exeternal binary!
+    g() { f; }; f() { true; }   -- f is a function!

  """
  def __init__(self, f):
@@ -72,61 +78,79 @@ def __init__(self, f):
    self.progs_used = {}
    self.f = f

-  def Visit(self, node):
+  def _Visit(self, node):
    """
    """
    #log('VISIT %s', node.__class__.__name__)

-    # PROBLEM: The tags are not unique!!!  Crap.  This is picking up some other
-    # stuff.  Need the isinstance() check.
-    if isinstance(node, ast.command) and node.tag == command_e.SimpleCommand:
+    # NOTE: The tags are not unique!!!  We would need this:
+    # if isinstance(node, ast.command) and node.tag == command_e.SimpleCommand:
+    # But it's easier to check the __class__ attribute.
+
+    cls = node.__class__
+    if cls is ast.SimpleCommand:
      #log('SimpleCommand %s', node.words)
      #log('--')
      #ast_lib.PrettyPrint(node)

      # Things to consider:
      # - source and .
-      # - builtins: get a list from builtin.py
-      # - functions: have to enter function definitions into a dictionary
+      # - DONE builtins: get a list from builtin.py
+      # - DONE functions: have to enter function definitions into a dictionary
      # - Commands that call others: sudo, su, find, xargs, etc.
-
-      # TODO: We need two passes!  test/wild.sh make-report, etc.
-
-      if node.words:
-        w = node.words[0]
-        ok, prog, _ = word.StaticEval(w)
-        if ok:
-          # TODO: Also consider builtins
-          if (prog not in self.funcs_defined and
-              builtin.ResolveSpecial(prog) == builtin_e.NONE and
-              builtin.Resolve(prog) == builtin_e.NONE):
-            self.progs_used[prog] = True
-        else:
+      # - builtins that call others: exec, command
+      #   - except not command -v!
+
+      if not node.words:
+        return
+
+      w = node.words[0]
+      ok, argv0, _ = word.StaticEval(w)
+      if not ok:
+        log("Couldn't statically evaluate %r", w)
+        return
+
+      if (builtin.ResolveSpecial(argv0) == builtin_e.NONE and
+          builtin.Resolve(argv0) == builtin_e.NONE):
+        self.progs_used[argv0] = True
+
+      # NOTE: If argv1 is $0, then we do NOT print a warning!
+      if argv0 == 'sudo':
+        if len(node.words) < 2:
+          return
+        w1 = node.words[1]
+        ok, argv1, _ = word.StaticEval(w1)
+        if not ok:
          log("Couldn't statically evaluate %r", w)
+          return

-      # There could be command sub, e.g. even in redirect:
-      self.VisitChildren(node)
-
-    elif isinstance(node, ast.command) and node.tag == command_e.FuncDef:
+        # Should we mark them behind 'sudo'?  e.g. "sudo apt install"?
+        self.progs_used[argv1] = True
+        
+    elif cls is ast.FuncDef:
      self.funcs_defined[node.name] = True
-      self.VisitChildren(node)

-    else:
-      self.VisitChildren(node)
+  def Visit(self, node):
+    self._Visit(node)
+
+    # We always need to visit children, even for SimpleCommand, etc.  There
+    # could be command sub, e.g. even in redirect.  echo hi > $(cat out)
+    self.VisitChildren(node)

  def Emit(self, row):
    # TSV-like format
    self.f.write('\t'.join(row))
    self.f.write('\n')

  def Done(self):
+    """Write a report."""
+    # TODO: Use self.Emit(), make it TSV.
    for name in self.progs_used:
-      print name
+      if name not in self.funcs_defined:
+        print name


 def Deps(node):
  v = DepsVisitor(sys.stdout)
  v.Visit(node)
  v.Done()
-
-  #print(node)