Improve handling of evaling dependency notation URIs (#1595)

pkl-cli/src/main/kotlin/org/pkl/cli/CliCommandRunner.kt

@@ -67,7 +67,7 @@ constructor(
     val evaluator = builder.build()
     evaluator.use {
       evaluator.evaluateCommand(
-        uri(resolvedSourceModules.first()),
+        uri(options.normalizedSourceModules.first()),
         reservedFlagNames,
         reservedFlagShortNames,
       ) { spec ->

pkl-cli/src/main/kotlin/org/pkl/cli/CliEvaluator.kt

@@ -115,7 +115,7 @@ constructor(
     // used just to resolve the `%{moduleName}` placeholder
     val moduleResolver = ModuleResolver(moduleKeyFactories(ModulePathResolver.empty()))
 
-    return resolvedSourceModules.associateWith { uri ->
+    return options.base.normalizedSourceModules.associateWith { uri ->
       val moduleDir: String? =
         IoUtils.toPath(uri)?.let {
           IoUtils.relativize(it.parent, workingDir).toString().ifEmpty { "." }
@@ -191,7 +191,7 @@ constructor(
         }
       } else {
         var outputWritten = false
-        for (moduleUri in resolvedSourceModules) {
+        for (moduleUri in options.base.normalizedSourceModules) {
           val moduleSource = toModuleSource(moduleUri, inputStream)
           if (options.expression != null) {
             val output = evaluator.evaluateExpressionString(moduleSource, options.expression)

pkl-cli/src/main/kotlin/org/pkl/cli/CliImportAnalyzer.kt

@@ -66,7 +66,7 @@ constructor(
     try {
       return builder
         .apply {
-          for ((idx, sourceModule) in resolvedSourceModules.withIndex()) {
+          for ((idx, sourceModule) in options.base.normalizedSourceModules.withIndex()) {
             addExternalProperty("pkl.analyzeImports.$idx", sourceModule.toString())
           }
         }

pkl-cli/src/main/kotlin/org/pkl/cli/CliTestRunner.kt

@@ -47,7 +47,7 @@ constructor(
 
   private fun evalTest(builder: EvaluatorBuilder) {
     val sources =
-      resolvedSourceModules.ifEmpty { project?.tests?.map { it.toUri() } }
+      options.normalizedSourceModules.ifEmpty { project?.tests?.map { it.toUri() } }
         ?:
         // keep in sync with error message thrown by clikt
         throw CliException(

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

@@ -1754,6 +1754,32 @@ result = someLib.x
     )
   }
 
+  @Test
+  fun `eval dependency notation source`(@TempDir tempDir: Path) {
+    PackageServer.populateCacheDir(tempDir)
+    val projectPath =
+      FileTestUtils.rootProjectDir.resolve(
+        "pkl-commons-cli/src/main/resources/org/pkl/commons/cli/project1/"
+      )
+    val options =
+      CliEvaluatorOptions(
+        CliBaseOptions(
+          sourceModules = listOf(URI("@fruit/catalog/apple.pkl")),
+          projectDir = projectPath,
+          moduleCacheDir = tempDir,
+        )
+      )
+    val output = evalToConsole(options)
+    assertThat(output)
+      .isEqualTo(
+        """
+        name = "Apple 🍎"
+
+        """
+          .trimIndent()
+      )
+  }
+
   private fun evalModuleThatImportsPackage(certsFile: Path?, testPort: Int = -1) {
     val moduleUri =
       writePklFile(

pkl-codegen-java/src/main/kotlin/org/pkl/codegen/java/CliJavaCodeGenerator.kt

@@ -31,7 +31,7 @@ class CliJavaCodeGenerator(private val options: CliJavaCodeGeneratorOptions) :
     val builder = evaluatorBuilder()
     try {
       builder.build().use { evaluator ->
-        for (moduleUri in resolvedSourceModules) {
+        for (moduleUri in options.base.normalizedSourceModules) {
           val schema = evaluator.evaluateSchema(ModuleSource.uri(moduleUri))
           val codeGenerator = JavaCodeGenerator(schema, options.toJavaCodeGeneratorOptions())
           try {

pkl-codegen-kotlin/src/main/kotlin/org/pkl/codegen/kotlin/CliKotlinCodeGenerator.kt

@@ -32,7 +32,7 @@ class CliKotlinCodeGenerator(private val options: CliKotlinCodeGeneratorOptions)
 
     try {
       builder.build().use { evaluator ->
-        for (moduleUri in resolvedSourceModules) {
+        for (moduleUri in options.base.normalizedSourceModules) {
           val schema = evaluator.evaluateSchema(ModuleSource.uri(moduleUri))
           val codeGenerator = KotlinCodeGenerator(schema, options.toKotlinCodeGeneratorOptions())
           try {

pkl-commons-cli/src/main/kotlin/org/pkl/commons/cli/CliBaseOptions.kt

@@ -190,7 +190,7 @@ data class CliBaseOptions(
     sourceModules
       .map { uri ->
         if (uri.isAbsolute) uri
-        else if (uri.path.startsWith("@") && !noProject && normalizedProjectFile != null) uri
+        else if (uri.path.startsWith("@")) uri
         else IoUtils.resolve(normalizedWorkingDir.toUri(), uri)
       }
       // sort modules to make cli output independent of source module order

pkl-commons-cli/src/main/kotlin/org/pkl/commons/cli/CliCommand.kt

@@ -86,36 +86,6 @@ abstract class CliCommand(protected val cliOptions: CliBaseOptions) {
     }
   }
 
-  protected fun resolveModuleUri(uri: URI): URI =
-    if (uri.isAbsolute) uri
-    else { // must be @dep/mod.pkl notation!!
-      if (!uri.path.startsWith('@'))
-        throw CliBugException(
-          RuntimeException("tried to resolve project URI `$uri` with no @ prefix")
-        )
-      if (project == null)
-        throw CliBugException(
-          RuntimeException("tried to resolve project URI `$uri` with no project present")
-        )
-      val dep = uri.path.substringBefore('/').drop(1)
-      val path = uri.path.dropWhile { it != '/' }
-      if (path.isEmpty()) throw CliException("Invalid project dependency URI `$uri`.")
-
-      val remoteDep =
-        project!!.dependencies.remoteDependencies()[dep]
-          ?: if (project!!.dependencies.localDependencies().containsKey(dep))
-            throw CliException(
-              "Only remote project dependencies may be referenced using @-notation. Dependency `@$dep` is a local dependency."
-            )
-          else throw CliException("Project does not contain dependency `@$dep`.")
-      remoteDep.packageUri.toPackageAssetUri(path).uri
-    }
-
-  protected val resolvedSourceModules: List<URI> by lazy {
-    if (project == null) cliOptions.normalizedSourceModules
-    else cliOptions.normalizedSourceModules.map(::resolveModuleUri)
-  }
-
   protected fun loadProject(projectFile: Path): Project {
     val securityManager =
       SecurityManagers.standard(

pkl-commons-cli/src/main/resources/org/pkl/commons/cli/project1/PklProject

@@ -0,0 +1,5 @@
+amends "pkl:Project"
+
+dependencies {
+  ["fruit"] { uri = "package://localhost:0/fruit@1.1.0" }
+}

pkl-commons-cli/src/main/resources/org/pkl/commons/cli/project1/PklProject.deps.json

@@ -0,0 +1,12 @@
+{
+  "schemaVersion": 1,
+  "resolvedDependencies": {
+    "package://localhost:0/fruit@1": {
+      "type": "remote",
+      "uri": "projectpackage://localhost:0/fruit@1.1.0",
+      "checksums": {
+        "sha256": "$skipChecksumVerification"
+      }
+    }
+  }
+}

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

@@ -17,13 +17,10 @@ package org.pkl.commons.cli
 
 import com.github.ajalt.clikt.core.parse
 import com.github.ajalt.clikt.parameters.groups.provideDelegate
-import java.net.URI
 import java.nio.file.Path
 import kotlin.io.path.ExperimentalPathApi
-import kotlin.io.path.writeText
 import org.assertj.core.api.Assertions.assertThat
 import org.junit.jupiter.api.Test
-import org.junit.jupiter.api.assertThrows
 import org.junit.jupiter.api.condition.DisabledOnJre
 import org.junit.jupiter.api.condition.JRE
 import org.junit.jupiter.api.io.TempDir
@@ -49,7 +46,6 @@ class CliCommandTest {
   class CliTest(options: CliBaseOptions) : CliCommand(options) {
     override fun doRun() = Unit
 
-    val myResolvedSourceModules = resolvedSourceModules
     val myAllowedModules = allowedModules
     val myAllowedResources = allowedResources
     val myRootDir = rootDir
@@ -98,114 +94,6 @@ class CliCommandTest {
       )
   }
 
-  @Test
-  fun `@-notation package URIs - treated as relative paths when no project present`(
-    @TempDir tempDir: Path
-  ) {
-    cmd.parse(arrayOf("--working-dir=$tempDir"))
-    val opts = cmd.baseOptions.baseOptions(listOf(URI("@foo/bar.pkl")), testMode = true)
-    val cliTest = CliTest(opts)
-    assertThat(cliTest.myResolvedSourceModules)
-      .isEqualTo(listOf(tempDir.toUri().resolve("@foo/bar.pkl")))
-  }
-
-  @Test
-  fun `@-notation package URIs - empty paths are rejected`(@TempDir tempDir: Path) {
-    tempDir
-      .resolve("PklProject")
-      .writeText(
-        """
-        amends "pkl:Project"
-        """
-          .trimIndent()
-      )
-    cmd.parse(arrayOf("--working-dir=$tempDir"))
-    val opts = cmd.baseOptions.baseOptions(listOf(URI("@no.slash")), testMode = true)
-    val exc = assertThrows<CliException> { CliTest(opts) }
-    assertThat(exc.message).isEqualTo("Invalid project dependency URI `@no.slash`.")
-  }
-
-  @Test
-  fun `@-notation package URIs - missing dependencies are rejected`(@TempDir tempDir: Path) {
-    tempDir
-      .resolve("PklProject")
-      .writeText(
-        """
-        amends "pkl:Project"
-        """
-          .trimIndent()
-      )
-    cmd.parse(arrayOf("--working-dir=$tempDir"))
-    val opts = cmd.baseOptions.baseOptions(listOf(URI("@foo/bar.pkl")), testMode = true)
-    val exc = assertThrows<CliException> { CliTest(opts) }
-    assertThat(exc.message).isEqualTo("Project does not contain dependency `@foo`.")
-  }
-
-  @Test
-  fun `@-notation package URIs - local dependencies are rejected`(
-    @TempDir tempDir: Path,
-    @TempDir depDir: Path,
-  ) {
-    depDir
-      .resolve("PklProject")
-      .writeText(
-        """
-        amends "pkl:Project"
-
-        package {
-          name = "foo"
-          baseUri = "package://example.com/foo"
-          version = "0.0.1"
-          packageZipUrl = "https://example.com/foo@\(version).zip"
-        }
-        """
-          .trimIndent()
-      )
-
-    tempDir
-      .resolve("PklProject")
-      .writeText(
-        """
-      amends "pkl:Project"
-
-      dependencies {
-        ["foo"] = import("${depDir.toUri().resolve("PklProject")}")
-      }
-    """
-          .trimIndent()
-      )
-    cmd.parse(arrayOf("--working-dir=$tempDir"))
-    val opts = cmd.baseOptions.baseOptions(listOf(URI("@foo/bar.pkl")), testMode = true)
-    val exc = assertThrows<CliException> { CliTest(opts) }
-    assertThat(exc.message)
-      .isEqualTo(
-        "Only remote project dependencies may be referenced using @-notation. Dependency `@foo` is a local dependency."
-      )
-  }
-
-  @Test
-  fun `@-notation package URIs - remote dependencies are resolved`(@TempDir tempDir: Path) {
-    tempDir
-      .resolve("PklProject")
-      .writeText(
-        """
-        amends "pkl:Project"
-
-        dependencies {
-          ["foo"] {
-            uri = "package://example.com/foo@1.2.3"
-          }
-        }
-        """
-          .trimIndent()
-      )
-    cmd.parse(arrayOf("--working-dir=$tempDir"))
-    val opts = cmd.baseOptions.baseOptions(listOf(URI("@foo/bar.pkl")), testMode = true)
-    val cliTest = CliTest(opts)
-    assertThat(cliTest.myResolvedSourceModules)
-      .isEqualTo(listOf(tempDir.toUri().resolve("package://example.com/foo@1.2.3#/bar.pkl")))
-  }
-
   val projectWithAllEvaluatorSettings =
     """
     amends "pkl:Project"

pkl-core/src/main/java/org/pkl/core/EvaluatorImpl.java

@@ -16,6 +16,8 @@
 package org.pkl.core;
 
 import java.io.IOException;
+import java.net.URI;
+import java.net.URISyntaxException;
 import java.nio.file.Path;
 import java.time.Duration;
 import java.util.Collection;
@@ -33,9 +35,11 @@ import org.msgpack.core.MessagePack;
 import org.pkl.core.ast.ConstantValueNode;
 import org.pkl.core.ast.internal.ToStringNodeGen;
 import org.pkl.core.evaluatorSettings.TraceMode;
+import org.pkl.core.externalreader.ExternalReaderProcessException;
 import org.pkl.core.http.HttpClient;
 import org.pkl.core.module.ModuleKeyFactory;
 import org.pkl.core.module.ProjectDependenciesManager;
+import org.pkl.core.packages.PackageLoadError;
 import org.pkl.core.packages.PackageResolver;
 import org.pkl.core.project.DeclaredDependencies;
 import org.pkl.core.resource.ResourceReader;
@@ -56,6 +60,7 @@ import org.pkl.core.runtime.VmUtils;
 import org.pkl.core.runtime.VmValue;
 import org.pkl.core.runtime.VmValueRenderer;
 import org.pkl.core.util.ErrorMessages;
+import org.pkl.core.util.IoUtils;
 import org.pkl.core.util.Nullable;
 
 public final class EvaluatorImpl implements Evaluator {
@@ -69,6 +74,7 @@ public final class EvaluatorImpl implements Evaluator {
   private final BufferedLogger logger;
   private final PackageResolver packageResolver;
   private final VmValueRenderer vmValueRenderer = VmValueRenderer.singleLine(1000);
+  private final @Nullable URI projectFileUri;
   private @Nullable MessageBufferPacker messagePacker;
 
   public EvaluatorImpl(
@@ -94,6 +100,11 @@ public final class EvaluatorImpl implements Evaluator {
     moduleResolver = new ModuleResolver(factories);
     this.logger = new BufferedLogger(logger);
     packageResolver = PackageResolver.getInstance(securityManager, httpClient, moduleCacheDir);
+    if (projectDependencies != null) {
+      this.projectFileUri = projectDependencies.projectFileUri();
+    } else {
+      this.projectFileUri = null;
+    }
     polyglotContext =
         VmUtils.createContext(
             () -> {
@@ -424,10 +435,37 @@ public final class EvaluatorImpl implements Evaluator {
     return evalResult;
   }
 
+  /** Resolve dependency notation URIs (e.g. `@foo/bar.pkl`) to its resolved absolute URI. */
+  private ModuleSource normalizeModuleSource(ModuleSource moduleSource) {
+    if (moduleSource.getContents() != null
+        || moduleSource.getUri().isAbsolute()
+        || !moduleSource.getUri().getPath().startsWith("@")) {
+      return moduleSource;
+    }
+    try {
+      if (projectFileUri != null) {
+        var moduleKey = moduleResolver.resolve(projectFileUri);
+        var uri = IoUtils.resolve(securityManager, moduleKey, moduleSource.getUri());
+        return ModuleSource.uri(uri);
+      } else {
+        throw new PackageLoadError("cannotResolveDependencyNoProject");
+      }
+    } catch (URISyntaxException e) {
+      // impossible condition
+      throw PklBugException.unreachableCode();
+    } catch (IOException e) {
+      throw new VmExceptionBuilder()
+          .evalError("ioErrorLoadingModule", moduleSource.getUri())
+          .build();
+    } catch (ExternalReaderProcessException | SecurityManagerException | PackageLoadError e) {
+      throw new VmExceptionBuilder().withCause(e).build();
+    }
+  }
+
   private <T> T doEvaluate(ModuleSource moduleSource, Function<VmTyped, T> doEvaluate) {
     return doEvaluate(
         () -> {
-          var moduleKey = moduleResolver.resolve(moduleSource);
+          var moduleKey = moduleResolver.resolve(normalizeModuleSource(moduleSource));
           var module = VmLanguage.get(null).loadModule(moduleKey);
           return doEvaluate.apply(module);
         });

pkl-core/src/main/resources/org/pkl/core/errorMessages.properties

@@ -817,10 +817,10 @@ invalidModuleOutput=\
 Expected `{0}` of module `{3}` to be of type `{1}`, but got type `{2}`.
 
 cannotResolveDependencyWithoutHierarchicalUris=\
-Cannot import dependency because project URI `{0}` does not have a hierarchical path.
+Cannot resolve dependency because project URI `{0}` does not have a hierarchical path.
 
 cannotResolveDependencyNoProject=\
-Cannot import dependency because there is no project found.\n\
+Cannot resolve dependency because there is no project found.\n\
 \n\
 If you meant to import a path that starts with `@`, prefix the path with `./` (e.g. `import "./@myPath").\n\
 If you meant to import a dependency, ensure that this file is within a directory that contains a PklProject module.

pkl-core/src/test/files/LanguageSnippetTests/output/projects/notAProject/badImport.err

@@ -1,5 +1,5 @@
 –– Pkl Error ––
-Cannot import dependency because there is no project found.
+Cannot resolve dependency because there is no project found.
 
 If you meant to import a path that starts with `@`, prefix the path with `./` (e.g. `import "./@myPath").
 If you meant to import a dependency, ensure that this file is within a directory that contains a PklProject module.

pkl-core/src/test/kotlin/org/pkl/core/EvaluatorTest.kt

@@ -507,7 +507,7 @@ class EvaluatorTest {
     val evaluator = evaluatorBuilder.setProjectDependencies(project.dependencies).build()
     assertThatCode { evaluator.use { it.evaluateOutputText(uri("foobar:baz")) } }
       .hasMessageContaining(
-        "Cannot import dependency because project URI `foobar:foo/PklProject` does not have a hierarchical path."
+        "Cannot resolve dependency because project URI `foobar:foo/PklProject` does not have a hierarchical path."
       )
   }
 
@@ -722,6 +722,34 @@ class EvaluatorTest {
     }
   }
 
+  @Test
+  fun `eval dependency notation as a module source`(@TempDir tempDir: Path) {
+    PackageServer.populateCacheDir(tempDir)
+    val project = Project.load(modulePath("org/pkl/core/project/project5/PklProject"))
+    val evaluator =
+      with(EvaluatorBuilder.preconfigured()) {
+        moduleCacheDir = tempDir
+        applyFromProject(project)
+        build()
+      }
+    val outputText = evaluator.evaluateOutputText(uri("@fruit/catalog/apple.pkl"))
+    assertThat(outputText)
+      .isEqualTo(
+        """
+        name = "Apple"
+
+        """
+          .trimIndent()
+      )
+  }
+
+  @Test
+  fun `eval dependency notation -- no project configured`() {
+    val evaluator = Evaluator.preconfigured()
+    assertThatCode { evaluator.evaluateOutputText(uri("@fruit/catalog/apple.pkl")) }
+      .hasMessageContaining("Cannot resolve dependency because there is no project found.")
+  }
+
   private fun checkModule(module: PModule) {
     assertThat(module.properties.size).isEqualTo(2)
     assertThat(module.getProperty("name")).isEqualTo("pigeon")

pkl-doc/src/main/kotlin/org/pkl/doc/CliDocGenerator.kt

@@ -159,7 +159,7 @@ class CliDocGenerator(
     val regularModuleUris = mutableListOf<URI>()
     val pklProjectPaths = mutableSetOf<Path>()
     val packageUris = mutableListOf<PackageUri>()
-    for (moduleUri in resolvedSourceModules) {
+    for (moduleUri in options.base.normalizedSourceModules) {
       if (moduleUri.scheme == "file") {
         val dir = moduleUri.toPath().parent
         val projectFile = dir.getProjectFile(options.base.normalizedRootDir)