Fix uncaught exceptions in OSH parser uncovered by wild tests.

The underlying bugs still remain. - Fix uncaught exception in [[ regex parsing, and make note of a bug. - Fix uncaught exception for 'A= (1 2)' i.e. extra space. That fixes the first level of the problem sandstorm/install.sh. Add a test for the second problem: backtick comments. - Another fix uncaught exception for error locations on empty arrays. Now it looks like: Line 25 of '/home/andy/src/bashdb-4.4-0.92/lib/help.sh' typeset -a _Dbg_command_names_sorted=() ^~~~~~~~~~~~~~~~~~~~~~~~~~ Also: - De-emphasize the osh2oil translation in the wild report.
oilshell · Oct 18, 2017 · 982f3b1b73cf7dff84f862be283bdd2d5b7131a6 · 982f3b1
1 parent 8d24526
commit 982f3b1b73cf7dff84f862be283bdd2d5b7131a6
Unified Split

Showing with 59 additions and 21 deletions.

+4 −1 core/word.py

+2 −0 native/libc.c

+4 −4 native/libc_test.py

+7 −0 osh/bool_parse.py

+1 −1 osh/cmd_parse.py

+18 −0 osh/cmd_parse_test.py

+3 −1 osh/word_parse.py

+20 −14 test/wild_report.py
diff --git a/core/word.py b/core/word.py
@@ -102,7 +102,10 @@ def LeftMostSpanForPart(part):
  #print(IdName(part.id))

  if part.tag == word_part_e.ArrayLiteralPart:
-    return LeftMostSpanForWord(part.words[0])  # Hm this is a=(1 2 3)
+    if not part.words:
+      return -1
+    else:
+      return LeftMostSpanForWord(part.words[0])  # Hm this is a=(1 2 3)

  elif part.tag == word_part_e.LiteralPart:
    # Just use the token
diff --git a/native/libc.c b/native/libc.c
@@ -146,6 +146,8 @@ func_regex_parse(PyObject *self, PyObject *args) {
    return NULL;
  }
  regex_t pat;
+  // This is an extended regular expression rather than a basic one, i.e. we
+  // use 'a*' instaed of 'a\*'.
  int ret = regcomp(&pat, pattern, REG_EXTENDED);
  regfree(&pat);

diff --git a/native/libc_test.py b/native/libc_test.py
@@ -49,10 +49,10 @@ def testGlob(self):

  def testRegex(self):
    #print(libc.regcomp(r'.*\.py'))
-    print(libc.regex_parse(r'.*\.py'))
-    print(libc.regex_parse(r'*'))
-    print(libc.regex_parse('\\'))
-
+    self.assertEqual(True, libc.regex_parse(r'.*\.py'))
+    self.assertEqual(False, libc.regex_parse(r'*'))
+    self.assertEqual(False, libc.regex_parse('\\'))
+    self.assertEqual(False, libc.regex_parse('{'))

    cases = [
        (r'.*\.py', 'foo.py', True),
diff --git a/osh/bool_parse.py b/osh/bool_parse.py
@@ -189,6 +189,7 @@ def ParseNegatedFactor(self):
    if self.op_id == Id.KW_Bang:
      if not self._Next(): return None
      child = self.ParseFactor()
+      if not child: return None
      #return UnaryExprNode(Id.KW_Bang, child)
      return ast.LogicalNot(child)
    else:
@@ -234,7 +235,13 @@ def ParseFactor(self):

        right = self.cur_word
        if is_regex:
+          # TODO: Quoted parts need to be regex-escaped, e.g. [[ $a =~ "{" ]].
+          # I don't think libc has a function to do this.  Escape these
+          # characters:
+          # https://www.gnu.org/software/sed/manual/html_node/ERE-syntax.html0
+
          ok, regex_str, unused_quoted = word.StaticEval(right)
+
          # doesn't contain $foo, etc.
          if ok and not libc.regex_parse(regex_str):
            self.AddErrorContext("Invalid regex: %r" % regex_str, word=right)
diff --git a/osh/cmd_parse.py b/osh/cmd_parse.py
@@ -418,7 +418,7 @@ def _MakeSimpleCommand(self, prefix_bindings, suffix_words, redirects):
      if kov:
        _, _, v = kov
        if word.HasArrayPart(v):
-          self.AddErrorContext('Unexpected array literal: %s', v, word=v)
+          self.AddErrorContext('Unexpected array literal: %s', v, word=w)
          return None

    # NOTE: # In bash, {~bob,~jane}/src works, even though ~ isn't the leading
diff --git a/osh/cmd_parse_test.py b/osh/cmd_parse_test.py
@@ -1159,6 +1159,20 @@ def testArithConstants(self):
 echo $(( 0x$foo ))
 """)

+  def testBacktickCommentHack(self):
+    # Found in sandstorm.
+    # The problem here is that the comment goes to the end of the line, which
+    # eats up the closing backtick!  We could change the mode of the lexer
+    # inside a command sub, or possibly just ignore this use case.
+    return
+
+    node = assertParseCommandList(self, r"""\
+openssl \
+    -newkey rsa:4096 `# Create a new RSA key of length 4096 bits.` \
+    `# Sandcats just needs the CN= (common name) in the request.` \
+    -subj "/CN=*.${SS_HOSTNAME}/"
+""")
+

 class ErrorLocationsTest(unittest.TestCase):

@@ -1235,6 +1249,10 @@ def testArith(self):
    """Enumerating errors in arith_parse.py."""
    err = _assertParseCommandListError(self, '(( 1 + ))')

+  def testArraySyntax(self):
+    """Enumerating errors in arith_parse.py."""
+    err = _assertParseCommandListError(self, 'A= (1 2)')
+

 if __name__ == '__main__':
  unittest.main()
diff --git a/osh/word_parse.py b/osh/word_parse.py
@@ -858,7 +858,9 @@ def ReadForExpression(self):
  def _ReadArrayLiteralPart(self):
    self._Next(LexMode.OUTER)  # advance past (
    self._Peek()
-    assert self.cur_token.id == Id.Op_LParen, self.cur_token
+    if self.cur_token.id != Id.Op_LParen:
+      self.AddErrorContext('Expected ( after =', token=self.cur_token)
+      return None

    # MUST use a new word parser (with same lexer).
    w_parser = WordParser(self.lexer, self.line_reader)
diff --git a/test/wild_report.py b/test/wild_report.py
@@ -179,9 +179,11 @@ def MakeHtmlGroup(title_str, body_str):
      <td>{lines_per_sec}</td>

      {.osh2oil_failed?}
-        <td class="fail">{osh2oil_failed|commas}</td>
+        <!-- <td class="fail">{osh2oil_failed|commas}</td> -->
+        <td>{osh2oil_failed|commas}</td>
      {.or}
-        <td class="ok">{osh2oil_failed|commas}</td>
+        <!-- <td class="ok">{osh2oil_failed|commas}</td> -->
+        <td>{osh2oil_failed|commas}</td>
      {.end}

      <td class="name">
@@ -228,7 +230,7 @@ def MakeHtmlGroup(title_str, body_str):
     </td>
      <td>{num_lines|commas}</td>
      <td>
-        {.section parse_failed}
+        {.parse_failed?}
          <a class="fail" href="#stderr_parse_{name}">FAIL</a>
          <td>{parse_proc_secs}</td>
          <td>-</td>
@@ -242,11 +244,12 @@ def MakeHtmlGroup(title_str, body_str):
      </td>

      <td>
-        {# not sure how to use if? }
-        {.section osh2oil_failed}
-          <a class="fail" href="#stderr_osh2oil_{name}">FAIL</a>
+        {.osh2oil_failed?}
+          <!-- <a class="fail" href="#stderr_osh2oil_{name}">FAIL</a> -->
+          FAIL
        {.or}
-          <a class="ok" href="{name}__oil.txt">OK</a>
+          <!-- <a class="ok" href="{name}__oil.txt">OK</a> -->
+          OK
        {.end}
      </td>
      <td class="name">
@@ -392,13 +395,16 @@ def UpdateNodes(node, path_parts, file_stats):
          'contents': parse_stderr,
      })
    osh2oil_stderr = file_stats.pop('osh2oil_stderr')
-    if osh2oil_stderr or file_stats['osh2oil_failed']:
-      node.stderr.append({
-          'parsing': False,
-          'action': 'osh2oil',
-          'name': first,
-          'contents': osh2oil_stderr,
-      })
+
+    # Concentrating on parsing failures for now.
+
+    #if osh2oil_stderr or file_stats['osh2oil_failed']:
+    #  node.stderr.append({
+    #      'parsing': False,
+    #      'action': 'osh2oil',
+    #      'name': first,
+    #      'contents': osh2oil_stderr,
+    #  })

    # Attach to this dir
    node.files[first] = file_stats