miniwdl check: present validation/typechecking errors with source exc…

…erpt

Makefile

@@ -1,7 +1,7 @@
 SHELL := /bin/bash
 
 test: check
-	coverage run --include "WDL/*" -m unittest -v
+	coverage run --include "WDL/*" --omit WDL/CLI.py -m unittest -v
 	coverage report
 	prove -v tests/cli.t
 

WDL/CLI.py

@@ -35,6 +35,7 @@ def main(args=None):
         action="store_false",
         help="relax static typechecking of optional (?) and nonempty (+) type quantifiers (discouraged; for backwards compatibility with older WDL)",
     )
+    check_parser.add_argument("--debug", action="store_true", help="show full exception traceback")
 
     args = parser.parse_args(args if args is not None else sys.argv[1:])
 
@@ -57,12 +58,30 @@ def check(args):
             # Print an outline
             print(os.path.basename(uri))
             outline(doc, 0)
-    except WDL.Error.ParseError as exn:
+    except (WDL.Error.ParseError, WDL.Error.ImportError, WDL.Error.Base) as exn:
         print(str(exn), file=sys.stderr)
-        sys.exit(1)
-    except WDL.Error.ImportError as exn:
-        print(str(exn), file=sys.stderr)
-        sys.exit(1)
+        if isinstance(exn, WDL.Error.ImportError) and hasattr(exn, "__cause__"):
+            print(str(exn.__cause__), file=sys.stderr)
+        if isinstance(exn, WDL.Error.Base) and exn.source_text:
+            # show source excerpt
+            lines = exn.source_text.split("\n")
+            print("    " + lines[exn.pos.line - 1], file=sys.stderr)
+            end_line = exn.pos.end_line
+            end_column = exn.pos.end_column
+            if end_line == exn.pos.line + 1 and end_column == 1:
+                # strip newline off the SourcePosition
+                end_line = exn.pos.line
+                end_column = len(lines[exn.pos.line - 1]) + 1
+            print(
+                "    "
+                + " " * (exn.pos.column - 1)
+                + "^" * (max(end_column - exn.pos.column, 1) if end_line == exn.pos.line else 1),
+                file=sys.stderr,
+            )
+        if args.debug:
+            raise exn
+        else:
+            sys.exit(1)
 
 
 # recursively pretty-print a brief outline of the workflow

WDL/Error.py

@@ -5,13 +5,19 @@
 
 
 class ParseError(Exception):
+    """Failure to lex/parse a WDL document"""
+
     def __init__(self, filename: str, msg: str) -> None:
         super().__init__("({}) {}".format(filename, msg))
 
 
 class ImportError(Exception):
+    """Failure to open/retrieve an imported WDL document
+
+    The ``__cause__`` attribute may hold the inner error object."""
+
     def __init__(self, document: str, import_uri: str, message: Optional[str] = None) -> None:
-        msg =  "({}) Failed to import {}".format(document, import_uri)
+        msg = "({}) Failed to import {}".format(document, import_uri)
         if message:
             msg = msg + ", " + message
         super().__init__(msg)
@@ -67,7 +73,18 @@ def children(self: TVSourceNode) -> Iterable[TVSourceNode]:
 
 
 class Base(Exception):
-    node: Optional[SourceNode]
+    """Base class for a WDL validation error (when the document loads and parses, but fails typechecking or other static validity tests)"""
+
+    pos: SourcePosition
+    """:type: SourcePosition"""
+
+    node: Optional[SourceNode] = None
+    """:type: Optional[SourceNode]"""
+
+    source_text: Optional[str] = None
+    """:type: Optional[str]
+
+    The complete source text of the WDL document (if available)"""
 
     def __init__(self, node: Union[SourceNode, SourcePosition], message: str) -> None:
         if isinstance(node, SourceNode):

WDL/Tree.py

@@ -722,7 +722,8 @@ def load(
         if os.path.exists(fn):
             with open(fn, "r") as infile:
                 # read and parse the document
-                doc = WDL._parser.parse_document(infile.read(), uri=uri, imported=imported)
+                source_text = infile.read()
+                doc = WDL._parser.parse_document(source_text, uri=uri, imported=imported)
                 assert isinstance(doc, Document)
                 # recursively descend into document's imports, and store the imported
                 # documents into doc.imports
@@ -734,9 +735,13 @@ def load(
                             doc.imports[i][0], subpath, check_quant=check_quant, imported=True
                         )
                     except Exception as exn:
-                        raise Err.ImportError(uri, doc.imports[i][0], str(exn)) from exn
+                        raise Err.ImportError(uri, doc.imports[i][0]) from exn
                     doc.imports[i] = (doc.imports[i][0], doc.imports[i][1], subdoc)
-                doc.typecheck(check_quant=check_quant)
+                try:
+                    doc.typecheck(check_quant=check_quant)
+                except WDL.Error.Base as exn:
+                    exn.source_text = source_text
+                    raise exn
                 return doc
     raise FileNotFoundError(errno.ENOENT, os.strerror(errno.ENOENT), uri)
 

tests/cli.t

@@ -11,7 +11,7 @@ source tests/bash-tap/bash-tap-bootstrap
 export PYTHONPATH="$SOURCE_DIR:$PYTHONPATH"
 miniwdl="python3 -m WDL"
 
-plan tests 18
+plan tests 30
 
 DN=$(mktemp -d --tmpdir miniwdl_tests_XXXXXX)
 cd $DN
@@ -45,6 +45,9 @@ is "$(cat lex_error.out | wc -c)" "0" "lex_error.wdl stdout"
 is "$(grep Traceback lex_error.err | wc -l)" "0" "lex_error.wdl stderr, no traceback"
 is "$(grep 'line 2 col 10' lex_error.err | wc -l)" "1" "lex_error.wdl stderr, position"
 
+$miniwdl check --debug lex_error.wdl > lex_error_debug.out 2> lex_error_debug.err
+is "$(grep Traceback lex_error_debug.err | wc -l)" "1" "lex_error.wdl stderr, traceback"
+
 cat << EOF > parse_error.wdl
 # comment 1
 # comment 2
@@ -64,4 +67,27 @@ is "$?" "1" "import_error.wdl exit code"
 is "$(cat import_error.out | wc -c)" "0" "import_error.wdl stdout"
 is "$(grep Traceback import_error.err | wc -l)" "0" "import_error.wdl stderr, no traceback"
 
+cat << EOF > imports_parse_error.wdl
+import "parse_error.wdl"
+EOF
+$miniwdl check imports_parse_error.wdl > imports_parse_error.out 2> imports_parse_error.err
+is "$?" "1" "imports_parse_error.wdl exit code"
+is "$(cat imports_parse_error.out | wc -c)" "0" "imports_parse_error.wdl stdout"
+is "$(grep Traceback imports_parse_error.err | wc -l)" "0" "imports_parse_error.wdl stderr, no traceback"
+is "$(grep 'Failed to import parse_error.wdl' imports_parse_error.err | wc -l)" "1" "imports_parse_error.wdl stderr, outer error"
+is "$(grep 'line 3, column 12' imports_parse_error.err | wc -l)" "1" "imports_parse_error.wdl stderr, inner position"
+
+cat << EOF > trivial_type_error.wdl
+workflow x {
+    Int x = "42"
+}
+EOF
+$miniwdl check trivial_type_error.wdl > trivial_type_error.out 2> trivial_type_error.err
+is "$?" "1" "trivial_type_error.wdl exit code"
+is "$(cat trivial_type_error.out | wc -c)" "0" "trivial_type_error.wdl stdout"
+is "$(grep Traceback trivial_type_error.err | wc -l)" "0" "trivial_type_error.wdl stderr, no traceback"
+is "$(grep '(trivial_type_error.wdl Ln 2, Col 13) Expected Int instead of String' trivial_type_error.err | wc -l)" "1" "trivial_type_error.wdl error message line 1"
+is "$(grep '        Int x = \"42\"' trivial_type_error.err | wc -l)" "1" "trivial_type_error.wdl error message line 2"
+is "$(grep '                ^^^^' trivial_type_error.err | wc -l)" "1" "trivial_type_error.wdl error message line 3"
+
 rm -rf $DN