[WDL 1.2] add join_paths() (#874)

Author: Wid-L-Hacker

WDL/StdLib.py

@@ -2,6 +2,7 @@
 from dataclasses import dataclass, replace
 import math
 import os
+import posixpath
 import json
 import tempfile
 from typing import List, Tuple, Dict, Callable, IO, Optional, Union, Any
@@ -195,6 +196,7 @@ def sep(sep: Value.String, iterable: Value.Array) -> Value.String:
             static([Type.String(), Type.String()], Type.String(optional=True))(find)
             static([Type.String(), Type.String()], Type.Boolean())(matches)
             self._pow = _ExponentiationOperator()
+            self.join_paths = _JoinPaths(self)
             self.contains = _Contains()
             self.chunk = _Chunk()
             self.values = _Values()
@@ -244,6 +246,16 @@ def _virtualize_filename(self, filename: str) -> str:
         # TODO: add directory: bool argument when we have stdlib functions that take Directory
         raise NotImplementedError()
 
+    def _join_paths_default_directory(self) -> str:
+        """
+        Directory against which join_paths() resolves relative results. Runtime stdlib subclasses
+        should override this with the appropriate context-specific base directory.
+
+        For task evaluation, this is the task working directory. For workflow evaluation, this is
+        the WDL source directory. The abstract stdlib has no such context.
+        """
+        raise NotImplementedError("join_paths() relative path resolution requires runtime context")
+
     def _override_static(self, name: str, f: Callable) -> None:
         # replace the implementation lambda of a StaticFunction (keeping its
         # types etc. the same)
@@ -1427,6 +1439,73 @@ def _call_eager(self, expr: "Expr.Apply", arguments: List[Value.Base]) -> Value.
         )
 
 
+class _JoinPaths(EagerFunction):
+    # String join_paths(Directory, String)
+    # String join_paths(Directory, Array[String]+)
+    # String join_paths(Array[String]+)
+
+    stdlib: Base
+
+    def __init__(self, stdlib: Base) -> None:
+        self.stdlib = stdlib
+
+    def infer_type(self, expr: "Expr.Apply") -> Type.Base:
+        if len(expr.arguments) not in (1, 2):
+            raise Error.WrongArity(expr, 2)
+        if len(expr.arguments) == 1:
+            expr.arguments[0].typecheck(Type.Array(Type.String(), nonempty=True))
+        else:
+            expr.arguments[0].typecheck(Type.Directory())
+            try:
+                expr.arguments[1].typecheck(Type.String())
+            except Error.StaticTypeMismatch:
+                try:
+                    expr.arguments[1].typecheck(Type.Array(Type.String(), nonempty=True))
+                except Error.StaticTypeMismatch:
+                    raise Error.StaticTypeMismatch(
+                        expr.arguments[1],
+                        Type.Any(),
+                        expr.arguments[1].type,
+                        "join_paths() argument #2 expects String or Array[String]+",
+                    ) from None
+        return Type.String()
+
+    def _absolute_path(self, parts: List[str]) -> str:
+        assert parts
+        ans = posixpath.normpath(posixpath.join(*parts))
+        if not ans.startswith("/"):
+            ans = posixpath.normpath(
+                posixpath.join(self.stdlib._join_paths_default_directory(), ans)
+            )
+        return ans
+
+    def _call_eager(self, expr: "Expr.Apply", arguments: List[Value.Base]) -> Value.Base:
+        if len(arguments) == 1:
+            paths = arguments[0].coerce(Type.Array(Type.String(), nonempty=True))
+            assert isinstance(paths, Value.Array)
+            parts = [path.coerce(Type.String()).value for path in paths.value]
+            relative_from = 1
+        else:
+            directory = arguments[0].coerce(Type.Directory())
+            assert isinstance(directory, Value.Directory)
+            if isinstance(arguments[1], Value.Array):
+                paths = arguments[1].coerce(Type.Array(Type.String(), nonempty=True))
+                assert isinstance(paths, Value.Array)
+                parts = [directory.value] + [
+                    path.coerce(Type.String()).value for path in paths.value
+                ]
+            else:
+                path = arguments[1].coerce(Type.String())
+                assert isinstance(path, Value.String)
+                parts = [directory.value, path.value]
+            relative_from = 1
+        for path in parts[relative_from:]:
+            assert isinstance(path, str)
+            if path.startswith("/"):
+                raise Error.EvalError(expr, "join_paths(): only the first path may be absolute")
+        return Value.String(self._absolute_path(parts))
+
+
 class _Quote(EagerFunction):
     # t array -> string array
     # if input array is nonempty then so is output

WDL/runtime/task.py

@@ -1152,6 +1152,9 @@ def _virtualize_filename(self, filename: str) -> str:
         self.logger.info(_("wrote", file=self.container.input_path_map[filename]))
         return self.container.input_path_map[filename]
 
+    def _join_paths_default_directory(self) -> str:
+        return os.path.join(self.container.container_dir, "work")
+
 
 class InputStdLib(_StdLib):
     # StdLib for evaluation of task inputs and command

WDL/runtime/workflow.py

@@ -707,6 +707,12 @@ def _devirtualize_filename(self, filename: str) -> str:
             self.cfg, self.state.fspath_allowlist, "read_*() argument", Value.File(filename)
         )
 
+    def _join_paths_default_directory(self) -> str:
+        source = self.state.workflow.pos.abspath
+        if not source or source == "(buffer)":
+            raise NotImplementedError("join_paths() relative path resolution requires WDL source")
+        return os.path.dirname(source)
+
     def _virtualize_filename(self, filename: str) -> str:
         # After write_* generates a file at the workflow level, query CallCache for an existing
         # identical file; if available, then return that copy. This improves cacheability of

tests/spec_tests/config.yaml

@@ -20,7 +20,6 @@ wdl-1.1:
 wdl-1.2:
   xfail:
     # FIXME:
-    - join_paths_task.wdl
     - relative_paths_context.wdl  # issue #869
     # Known spec deviations:
     - file_sizes_task.wdl       # expected output is malformed

tests/test_5stdlib.py

@@ -327,6 +327,97 @@ def test_basename_empty_suffix(self):
         self.assertEqual(str(self._eval_expr('basename("file.txt","")')), '"file.txt"')
         self.assertEqual(str(self._eval_expr('basename("/path/to/file.txt",sfx)', env=env)), '"file.txt"')
 
+    def test_join_paths(self):
+        class LocalStdLib(WDL.StdLib.Base):
+            def _join_paths_default_directory(self) -> str:
+                return "/work"
+
+        def eval_join(expr: str, env=None, type_env=None):
+            env = env or WDL.Env.Bindings()
+            if type_env is None:
+                type_env = WDL.Env.Bindings()
+                for binding in env:
+                    type_env = type_env.bind(binding.name, binding.value.type)
+            stdlib = LocalStdLib("1.2")
+            ex = WDL.parse_expr(expr, version="1.2").infer_type(type_env, stdlib)
+            self.assertEqual(str(ex.type), "String")
+            return ex.eval(env, stdlib)
+
+        self.assertEqual(str(eval_join('join_paths("/usr", "bin")')), '"/usr/bin"')
+        self.assertEqual(str(eval_join('join_paths("/usr", ["bin", "echo"])')), '"/usr/bin/echo"')
+        self.assertEqual(str(eval_join('join_paths(["/usr", "bin", "echo"])')), '"/usr/bin/echo"')
+        self.assertEqual(str(eval_join('join_paths("usr", "bin")')), '"/work/usr/bin"')
+        self.assertEqual(str(eval_join('join_paths(["usr", "bin", "echo"])')), '"/work/usr/bin/echo"')
+        self.assertEqual(str(eval_join('join_paths("/usr/", "./bin/../bin")')), '"/usr/bin"')
+
+        env = WDL.Env.Bindings().bind("paths", WDL.Value.Array(WDL.Type.String(), []))
+        type_env = WDL.Env.Bindings().bind(
+            "paths", WDL.Type.Array(WDL.Type.String(), nonempty=True)
+        )
+        with self.assertRaises(WDL.Error.EmptyArray):
+            eval_join("join_paths(paths)", env=env, type_env=type_env)
+
+        for expr in (
+            'join_paths("/usr", "/bin")',
+            'join_paths("/usr", ["bin", "/echo"])',
+            'join_paths(["/usr", "/bin"])',
+        ):
+            with self.subTest(expr=expr):
+                with self.assertRaisesRegex(
+                    WDL.Error.EvalError, "only the first path may be absolute"
+                ):
+                    eval_join(expr)
+
+    def test_join_paths_typecheck(self):
+        with self.assertRaises(WDL.Error.NoSuchFunction):
+            self._infer_expr_type('join_paths("/usr", "bin")', version="1.1")
+        with self.assertRaises(WDL.Error.WrongArity):
+            self._infer_expr_type("join_paths()", version="1.2")
+        with self.assertRaises(WDL.Error.WrongArity):
+            self._infer_expr_type('join_paths("/usr", "bin", "echo")', version="1.2")
+        with self.assertRaises(WDL.Error.StaticTypeMismatch):
+            self._infer_expr_type("join_paths(1)", version="1.2")
+        with self.assertRaisesRegex(
+            WDL.Error.StaticTypeMismatch, "expects String or Array\\[String\\]\\+"
+        ):
+            self._infer_expr_type('join_paths("/usr", {"x": 1})', version="1.2")
+        with self.assertRaises(WDL.Error.StaticTypeMismatch):
+            self._infer_expr_type("join_paths(None)", version="1.2")
+        optional_dir = WDL.Env.Bindings().bind("d", WDL.Type.Directory(optional=True))
+        with self.assertRaises(WDL.Error.StaticTypeMismatch):
+            self._infer_expr_type('join_paths(d, "x")', type_env=optional_dir, version="1.2")
+
+    def test_join_paths_runtime_context_required(self):
+        stdlib = WDL.StdLib.Base("1.2")
+        expr = WDL.parse_expr('join_paths(["relative", "path"])', version="1.2").infer_type(
+            WDL.Env.Bindings(), stdlib
+        )
+        with self.assertRaisesRegex(
+            WDL.Error.EvalError, "relative path resolution requires runtime context"
+        ):
+            expr.eval(WDL.Env.Bindings(), stdlib)
+
+        doc = WDL.parse_document(
+            R"""
+            version 1.2
+            workflow w {
+                output {
+                    String path = join_paths(["relative", "path"])
+                }
+            }
+            """
+        )
+        doc.typecheck()
+        cfg = WDL.runtime.config.Loader(logging.getLogger(self.id()), [])
+        state = WDL.runtime.workflow.StateMachine(
+            self.id(), self._dir, doc.workflow, WDL.Env.Bindings()
+        )
+        stdlib = WDL.runtime.workflow._StdLib("1.2", cfg, state, None)
+        with self.assertRaisesRegex(
+            NotImplementedError, "relative path resolution requires WDL source"
+        ):
+            stdlib._join_paths_default_directory()
+
     def test_parse_tsv_row_type(self):
         rows = WDL.StdLib._parse_tsv("alpha\tbeta\n")
         self.assertEqual(rows.json, [["alpha", "beta"]])

tests/test_7runner.py

@@ -68,6 +68,39 @@ def test_coercion(self):
         assert isinstance(d, WDL.Value.Directory)
         assert d.value == "foo"
 
+    @unittest.expectedFailure
+    def test_workflow_join_paths_child_of_input_directory(self):
+        # TODO: permit workflow-level File/Directory paths derived from an allowlisted Directory
+        # input, provided the derived path is really within that Directory.
+        wdl = R"""
+        version 1.2
+        workflow w {
+            input {
+                Directory d
+            }
+            output {
+                String contents = read_string(join_paths(d, "alice.txt"))
+            }
+        }
+        """
+        os.makedirs(os.path.join(self._dir, "d"))
+        with open(os.path.join(self._dir, "d/alice.txt"), mode="w") as outfile:
+            print("Alice", file=outfile)
+        outp = self._run(wdl, {"d": os.path.join(self._dir, "d")})
+        assert outp["contents"] == "Alice"
+
+    def test_workflow_join_paths_relative_to_source_directory(self):
+        wdl = R"""
+        version 1.2
+        workflow w {
+            output {
+                String path = join_paths(["subdir", "alice.txt"])
+            }
+        }
+        """
+        outp = self._run(wdl)
+        assert outp["path"] == os.path.join(self._dir, "subdir", "alice.txt")
+
     def test_basic_directory(self):
         wdl = R"""
         version development