Fix command error and import handling (#1436)

pkl-cli/src/test/kotlin/org/pkl/cli/CliCommandRunnerTest.kt

@@ -819,14 +819,25 @@ class CliCommandRunnerTest {
     val moduleUri =
       writePklFile(
         "cmd.pkl",
-        renderOptions +
-          """
+        """
+      extends "pkl:Command"
+      
+      options: Options
+      
+      output {
+        value = (options) {
+          fromImport {
+           baz = true // assert that imported modules are not forced
+          }
+        }
+      }
+      
       class Options {
         @Argument { convert = (it) -> new Import{ uri = it } }
         fromImport: Module
       }
     """
-            .trimIndent(),
+          .trimIndent(),
       )
 
     val importUri =
@@ -835,6 +846,7 @@ class CliCommandRunnerTest {
         """
       foo = 1
       bar = "baz"
+      baz: Boolean
     """
           .trimIndent(),
       )
@@ -847,6 +859,7 @@ class CliCommandRunnerTest {
         fromImport {
           foo = 1
           bar = "baz"
+          baz = true
         }
         
         """
@@ -866,7 +879,13 @@ class CliCommandRunnerTest {
       options: Options
       
       output {
-        value = options
+        value = (options) {
+          fromGlobImport {
+            [[true]] {
+              baz = true // assert that imported modules are not forced
+            }
+          }
+        }
       }
       
       class Options {
@@ -883,6 +902,7 @@ class CliCommandRunnerTest {
         """
       foo: Int
       bar: String
+      baz: Boolean
     """
           .trimIndent(),
       )
@@ -919,10 +939,12 @@ class CliCommandRunnerTest {
           ["file:/<dir>/glob1.pkl"] {
             foo = 1
             bar = "baz"
+            baz = true
           }
           ["file:/<dir>/glob2.pkl"] {
             foo = 2
             bar = "qux"
+            baz = true
           }
         }
         

pkl-commons-cli/src/test/kotlin/org/pkl/commons/cli/CliCommandTest.kt

@@ -148,7 +148,7 @@ class CliCommandTest {
       amends "pkl:Project"
 
       dependencies {
-        ["foo"] = import("$depDir/PklProject")
+        ["foo"] = import("${depDir.toUri().resolve("PklProject")}")
       }
     """
           .trimIndent()

pkl-core/src/main/java/org/pkl/core/runtime/CommandSpecParser.java

@@ -126,7 +126,7 @@ public final class CommandSpecParser {
                     // instance of CommandSpec.State previously returned from a prior invocation
                     // of CommandSpec.apply.
                     parent == null ? null : (SubcommandState) parent.contents()),
-                (it) -> evaluateResult(command, (SubcommandState) it)));
+                (it) -> handleErrors(() -> evaluateResult(command, (SubcommandState) it))));
   }
 
   private List<CommandSpec> collectSubcommands(VmTyped commandInfo) {
@@ -488,12 +488,12 @@ public final class CommandSpecParser {
               ? null
               : VmUtils.readMember(annotation, Identifier.CONVERT) instanceof VmFunction func
                   ? (rawValue, workingDirUri) ->
-                      handleErrors(() -> handleImports(func.apply(rawValue), workingDirUri))
+                      handleBadValue(() -> handleImports(func.apply(rawValue), workingDirUri))
                   : null,
           annotation == null
               ? null
               : VmUtils.readMember(annotation, Identifier.TRANSFORM_ALL) instanceof VmFunction func
-                  ? (it) -> handleErrors(() -> func.apply(VmList.create(it)))
+                  ? (it) -> handleBadValue(() -> func.apply(VmList.create(it)))
                   : null,
           annotation == null
               ? null
@@ -957,7 +957,6 @@ public final class CommandSpecParser {
   private SubcommandState buildExecutionModule(
       VmTyped module, VmTyped options, @Nullable SubcommandState parent) {
     EconomicMap<Object, ObjectMember> properties = EconomicMaps.create(parent != null ? 2 : 1);
-    options.force(false, true);
     properties.put(
         Identifier.OPTIONS, VmUtils.createSyntheticObjectProperty(Identifier.OPTIONS, "", options));
 
@@ -1079,31 +1078,35 @@ public final class CommandSpecParser {
   }
 
   // handle errors from convert/transformAll and correctly format them for the CLI
-  private Object handleErrors(Supplier<Object> f) {
+  private Object handleBadValue(Supplier<Object> f) {
     try {
-      try {
-        return f.get();
-      } catch (VmStackOverflowException e) {
-        if (VmUtils.isPklBug(e)) {
-          throw new VmExceptionBuilder()
-              .bug("Stack overflow")
-              .withCause(e.getCause())
-              .build()
-              .toPklException(frameTransformer, color);
-        }
-        throw e.toPklException(frameTransformer, color);
-      } catch (VmException e) {
-        throw e.toPklException(frameTransformer, color);
-      } catch (Exception e) {
-        throw new PklBugException(e);
-      }
+      return handleErrors(f);
     } catch (Throwable e) {
       // add a newline so this prints nicely under "Error: invalid value for <name>:"
       throw new BadValue("\n" + e.getMessage());
     }
   }
 
-  // for convert handle imports by replace Command.Import values
+  private <T> T handleErrors(Supplier<T> f) {
+    try {
+      return f.get();
+    } catch (VmStackOverflowException e) {
+      if (VmUtils.isPklBug(e)) {
+        throw new VmExceptionBuilder()
+            .bug("Stack overflow")
+            .withCause(e.getCause())
+            .build()
+            .toPklException(frameTransformer, color);
+      }
+      throw e.toPklException(frameTransformer, color);
+    } catch (VmException e) {
+      throw e.toPklException(frameTransformer, color);
+    } catch (Exception e) {
+      throw new PklBugException(e);
+    }
+  }
+
+  // for convert, handle imports by replacing Command.Import values
   // with imported module or Mapping<String, Module> values
   // Command.Import instances in returned Pair, List, Set, or Map values are replaced as well
   // other types or nested instances of the above are not affected
@@ -1158,13 +1161,13 @@ public final class CommandSpecParser {
     try {
       // Can't just use URI constructor, because URI(null, null, "C:/foo/bar", null) turns
       // into `URI("C", null, "/foo/bar", null)`.
-      var modulePath = Path.of(moduleName);
+      @SuppressWarnings("DuplicateExpressions")
       var uri =
           IoUtils.isUriLike(moduleName)
               ? new URI(moduleName)
               : IoUtils.isWindowsAbsolutePath(moduleName)
-                  ? modulePath.toUri()
-                  : new URI(null, null, IoUtils.toNormalizedPathString(modulePath), null);
+                  ? Path.of(moduleName).toUri()
+                  : new URI(null, null, IoUtils.toNormalizedPathString(Path.of(moduleName)), null);
       uriString =
           uri.isAbsolute() ? uri.toString() : IoUtils.resolve(workingDirUri, uri).toString();
     } catch (URISyntaxException e) {