Relative imports!

This is a breaking change because existing imports in user Wren code
that assume the path is relative to the entrypoint file will now likely
fail.

Also, stack trace output and host API calls that take a module string
now need the resolved module string, not the short name that appears in
the import.

example/import_module/lovecraft.wren

@@ -1,4 +1,4 @@
-import "cthulu" for Cthulu
+import "./cthulu" for Cthulu
 
 class Lovecraft {
   construct new() {}

src/cli/vm.c

@@ -3,6 +3,7 @@
 
 #include "io.h"
 #include "modules.h"
+#include "path.h"
 #include "scheduler.h"
 #include "vm.h"
 
@@ -15,7 +16,9 @@ static WrenForeignMethodFn afterLoadFn = NULL;
 
 static uv_loop_t* loop;
 
-static char const* rootDirectory = NULL;
+// TODO: This isn't currently used, but probably will be when package imports
+// are supported. If not then, then delete this.
+static char* rootDirectory = NULL;
 
 // The exit code to use unless some other error overrides it.
 int defaultExitCode = 0;
@@ -44,7 +47,7 @@ static char* readFile(const char* path)
   }
   
   // Read the entire file.
-  size_t bytesRead = fread(buffer, sizeof(char), fileSize, file);
+  size_t bytesRead = fread(buffer, 1, fileSize, file);
   if (bytesRead < fileSize)
   {
     fprintf(stderr, "Could not read file \"%s\".\n", path);
@@ -58,25 +61,46 @@ static char* readFile(const char* path)
   return buffer;
 }
 
-// Converts the module [name] to a file path.
-static char* wrenFilePath(const char* name)
+// Applies the CLI's import resolution policy. The rules are:
+//
+// * If [name] starts with "./" or "../", it is a relative import, relative to
+//   [importer]. The resolved path is [name] concatenated onto the directory
+//   containing [importer] and then normalized.
+//
+//   For example, importing "./a/./b/../c" from "d/e/f" gives you "d/e/a/c".
+//
+// * Otherwise, it is a "package" import. This isn't implemented yet.
+//
+static const char* resolveModule(WrenVM* vm, const char* importer,
+                                 const char* name)
 {
-  // The module path is relative to the root directory and with ".wren".
-  size_t rootLength = rootDirectory == NULL ? 0 : strlen(rootDirectory);
   size_t nameLength = strlen(name);
-  size_t pathLength = rootLength + nameLength + 5;
-  char* path = (char*)malloc(pathLength + 1);
   
-  if (rootDirectory != NULL)
+  // See if it's a relative import.
+  if (nameLength > 2 &&
+      ((name[0] == '.' && name[1] == '/') ||
+       (name[0] == '.' && name[1] == '.' && name[2] == '/')))
   {
-    memcpy(path, rootDirectory, rootLength);
+    // Get the directory containing the importing module.
+    Path* relative = pathNew(importer);
+    pathDirName(relative);
+    
+    // Add the relative import path.
+    pathJoin(relative, name);
+    Path* normal = pathNormalize(relative);
+    pathFree(relative);
+
+    char* resolved = pathToString(normal);
+    pathFree(normal);
+    return resolved;
+  }
+  else
+  {
+    // TODO: Implement package imports. For now, treat any non-relative import
+    // as an import relative to the current working directory.
   }
   
-  memcpy(path + rootLength, name, nameLength);
-  memcpy(path + rootLength + nameLength, ".wren", 5);
-  path[pathLength] = '\0';
-  
-  return path;
+  return name;
 }
 
 // Attempts to read the source for [module] relative to the current root
@@ -86,32 +110,26 @@ static char* wrenFilePath(const char* name)
 // module was found but could not be read.
 static char* readModule(WrenVM* vm, const char* module)
 {
-  char* source = readBuiltInModule(module);
+  // Since the module has already been resolved, it should now be either a
+  // valid relative path, or a package-style name.
+  
+  // TODO: Implement package imports.
+  
+  // Add a ".wren" file extension.
+  Path* modulePath = pathNew(module);
+  pathAppendString(modulePath, ".wren");
+  
+  char* source = readFile(modulePath->chars);
+  pathFree(modulePath);
+  
   if (source != NULL) return source;
-  
-  // First try to load the module with a ".wren" extension.
-  char* modulePath = wrenFilePath(module);
-  char* moduleContents = readFile(modulePath);
-  free(modulePath);
-  
-  if (moduleContents != NULL) return moduleContents;
-  
-  // If no contents could be loaded treat the module name as specifying a
-  // directory and try to load the "module.wren" file in the directory.
-  size_t moduleLength = strlen(module);
-  size_t moduleDirLength = moduleLength + 7;
-  char* moduleDir = (char*)malloc(moduleDirLength + 1);
-  memcpy(moduleDir, module, moduleLength);
-  memcpy(moduleDir + moduleLength, "/module", 7);
-  moduleDir[moduleDirLength] = '\0';
-  
-  char* moduleDirPath = wrenFilePath(moduleDir);
-  free(moduleDir);
-  
-  moduleContents = readFile(moduleDirPath);
-  free(moduleDirPath);
-  
-  return moduleContents;
+
+  // TODO: This used to look for a file named "<path>/module.wren" if
+  // "<path>.wren" could not be found. Do we still want to support that with
+  // the new relative import and package stuff?
+
+  // Otherwise, see if it's a built-in module.
+  return readBuiltInModule(module);
 }
 
 // Binds foreign methods declared in either built in modules, or the injected
@@ -179,6 +197,7 @@ static void initVM()
 
   config.bindForeignMethodFn = bindForeignMethod;
   config.bindForeignClassFn = bindForeignClass;
+  config.resolveModuleFn = resolveModule;
   config.loadModuleFn = readModule;
   config.writeFn = write;
   config.errorFn = reportError;
@@ -207,18 +226,6 @@ static void freeVM()
 
 void runFile(const char* path)
 {
-  // Use the directory where the file is as the root to resolve imports
-  // relative to.
-  char* root = NULL;
-  const char* lastSlash = strrchr(path, '/');
-  if (lastSlash != NULL)
-  {
-    root = (char*)malloc(lastSlash - path + 2);
-    memcpy(root, path, lastSlash - path + 1);
-    root[lastSlash - path + 1] = '\0';
-    rootDirectory = root;
-  }
-
   char* source = readFile(path);
   if (source == NULL)
   {
@@ -226,9 +233,19 @@ void runFile(const char* path)
     exit(66);
   }
 
+  // Use the directory where the file is as the root to resolve imports
+  // relative to.
+  Path* directory = pathNew(path);
+  pathDirName(directory);
+  rootDirectory = pathToString(directory);
+  pathFree(directory);
+  
+  Path* moduleName = pathNew(path);
+  pathRemoveExtension(moduleName);
+
   initVM();
 
-  WrenInterpretResult result = wrenInterpret(vm, "main", source);
+  WrenInterpretResult result = wrenInterpret(vm, moduleName->chars, source);
 
   if (afterLoadFn != NULL) afterLoadFn(vm);
   
@@ -240,7 +257,8 @@ void runFile(const char* path)
   freeVM();
 
   free(source);
-  free(root);
+  free(rootDirectory);
+  pathFree(moduleName);
 
   // Exit with an error code if the script failed.
   if (result == WREN_RESULT_COMPILE_ERROR) exit(65); // EX_DATAERR.
@@ -256,7 +274,7 @@ int runRepl()
   printf("\\\\/\"-\n");
   printf(" \\_/   wren v%s\n", WREN_VERSION_STRING);
 
-  wrenInterpret(vm, "main", "import \"repl\"\n");
+  wrenInterpret(vm, "repl", "import \"repl\"\n");
   
   uv_run(loop, UV_RUN_DEFAULT);
 

src/include/wren.h

@@ -92,14 +92,15 @@ typedef enum
 // Reports an error to the user.
 //
 // An error detected during compile time is reported by calling this once with
-// `WREN_ERROR_COMPILE`, the name of the module and line where the error occurs,
-// and the compiler's error message.
+// [type] `WREN_ERROR_COMPILE`, the resolved name of the [module] and [line]
+// where the error occurs, and the compiler's error [message].
 //
-// A runtime error is reported by calling this once with `WREN_ERROR_RUNTIME`,
-// no module or line, and the runtime error's message. After that, a series of
-// `WREN_ERROR_STACK_TRACE` calls are made for each line in the stack trace.
-// Each of those has the module and line where the method or function is
-// defined and [message] is the name of the method or function.
+// A runtime error is reported by calling this once with [type]
+// `WREN_ERROR_RUNTIME`, no [module] or [line], and the runtime error's
+// [message]. After that, a series of [type] `WREN_ERROR_STACK_TRACE` calls are
+// made for each line in the stack trace. Each of those has the resolved
+// [module] and [line] where the method or function is defined and [message] is
+// the name of the method or function.
 typedef void (*WrenErrorFn)(
     WrenVM* vm, WrenErrorType type, const char* module, int line,
     const char* message);
@@ -120,7 +121,7 @@ typedef struct
 } WrenForeignClassMethods;
 
 // Returns a pair of pointers to the foreign methods used to allocate and
-// finalize the data for instances of [className] in [module].
+// finalize the data for instances of [className] in resolved [module].
 typedef WrenForeignClassMethods (*WrenBindForeignClassFn)(
     WrenVM* vm, const char* module, const char* className);
 
@@ -287,7 +288,8 @@ void wrenFreeVM(WrenVM* vm);
 // Immediately run the garbage collector to free unused memory.
 void wrenCollectGarbage(WrenVM* vm);
 
-// Runs [source], a string of Wren source code in a new fiber in [vm].
+// Runs [source], a string of Wren source code in a new fiber in [vm] in the
+// context of resolved [module].
 WrenInterpretResult wrenInterpret(WrenVM* vm, const char* module,
                                   const char* source);
 
@@ -468,10 +470,11 @@ void wrenGetListElement(WrenVM* vm, int listSlot, int index, int elementSlot);
 // an element, use `-1` for the index.
 void wrenInsertInList(WrenVM* vm, int listSlot, int index, int elementSlot);
 
-// Looks up the top level variable with [name] in [module] and stores it in
-// [slot].
+// Looks up the top level variable with [name] in resolved [module] and stores
+// it in [slot].
 void wrenGetVariable(WrenVM* vm, const char* module, const char* name,
                      int slot);
+
 // Sets the current fiber to be aborted, and uses the value in [slot] as the
 // runtime error object.
 void wrenAbortFiber(WrenVM* vm, int slot);

test/api/call.c

@@ -6,7 +6,7 @@
 void callRunTests(WrenVM* vm)
 {
   wrenEnsureSlots(vm, 1);
-  wrenGetVariable(vm, "main", "Call", 0);
+  wrenGetVariable(vm, "test/api/call", "Call", 0);
   WrenHandle* callClass = wrenGetSlotHandle(vm, 0);
   
   WrenHandle* noParams = wrenMakeCallHandle(vm, "noParams");

test/api/get_variable.c

@@ -4,23 +4,23 @@
 
 static void beforeDefined(WrenVM* vm)
 {
-  wrenGetVariable(vm, "main", "A", 0);
+  wrenGetVariable(vm, "test/api/get_variable", "A", 0);
 }
 
 static void afterDefined(WrenVM* vm)
 {
-  wrenGetVariable(vm, "main", "A", 0);
+  wrenGetVariable(vm, "test/api/get_variable", "A", 0);
 }
 
 static void afterAssigned(WrenVM* vm)
 {
-  wrenGetVariable(vm, "main", "A", 0);
+  wrenGetVariable(vm, "test/api/get_variable", "A", 0);
 }
 
 static void otherSlot(WrenVM* vm)
 {
   wrenEnsureSlots(vm, 3);
-  wrenGetVariable(vm, "main", "B", 2);
+  wrenGetVariable(vm, "test/api/get_variable", "B", 2);
   
   // Move it into return position.
   const char* string = wrenGetSlotString(vm, 2);
@@ -29,7 +29,7 @@ static void otherSlot(WrenVM* vm)
 
 static void otherModule(WrenVM* vm)
 {
-  wrenGetVariable(vm, "get_variable_module", "Variable", 0);
+  wrenGetVariable(vm, "test/api/get_variable_module", "Variable", 0);
 }
 
 WrenForeignMethodFn getVariableBindMethod(const char* signature)

test/api/get_variable.wren

@@ -1,4 +1,4 @@
-import "get_variable_module"
+import "./get_variable_module"
 
 class GetVariable {
   foreign static beforeDefined()

test/api/main.c

@@ -24,8 +24,8 @@ const char* testName;
 static WrenForeignMethodFn bindForeignMethod(
     WrenVM* vm, const char* module, const char* className,
     bool isStatic, const char* signature)
-{  
-  if (strcmp(module, "main") != 0) return NULL;
+{
+  if (strncmp(module, "test/", 5) != 0) return NULL;
 
   // For convenience, concatenate all of the method qualifiers into a single
   // signature string.
@@ -78,7 +78,7 @@ static WrenForeignClassMethods bindForeignClass(
     WrenVM* vm, const char* module, const char* className)
 {
   WrenForeignClassMethods methods = { NULL, NULL };
-  if (strcmp(module, "main") != 0) return methods;
+  if (strncmp(module, "test/", 5) != 0) return methods;
 
   foreignClassBindClass(className, &methods);
   if (methods.allocate != NULL) return methods;

test/api/reset_stack_after_call_abort.c

@@ -6,7 +6,7 @@
 void resetStackAfterCallAbortRunTests(WrenVM* vm)
 {
   wrenEnsureSlots(vm, 1);
-  wrenGetVariable(vm, "main", "Test", 0);
+  wrenGetVariable(vm, "test/api/reset_stack_after_call_abort", "Test", 0);
   WrenHandle* testClass = wrenGetSlotHandle(vm, 0);
   
   WrenHandle* abortFiber = wrenMakeCallHandle(vm, "abortFiber()");
@@ -25,4 +25,4 @@ void resetStackAfterCallAbortRunTests(WrenVM* vm)
   wrenReleaseHandle(vm, testClass);
   wrenReleaseHandle(vm, abortFiber);
   wrenReleaseHandle(vm, afterConstruct);
-}
+}

test/api/reset_stack_after_foreign_construct.c

@@ -22,7 +22,7 @@ void resetStackAfterForeignConstructBindClass(
 void resetStackAfterForeignConstructRunTests(WrenVM* vm)
 {
   wrenEnsureSlots(vm, 1);
-  wrenGetVariable(vm, "main", "Test", 0);
+  wrenGetVariable(vm, "test/api/reset_stack_after_foreign_construct", "Test", 0);
   WrenHandle* testClass = wrenGetSlotHandle(vm, 0);
   
   WrenHandle* callConstruct = wrenMakeCallHandle(vm, "callConstruct()");
@@ -41,4 +41,4 @@ void resetStackAfterForeignConstructRunTests(WrenVM* vm)
   wrenReleaseHandle(vm, testClass);
   wrenReleaseHandle(vm, callConstruct);
   wrenReleaseHandle(vm, afterConstruct);
-}
+}

test/core/fiber/yield_from_import.wren

@@ -1,7 +1,7 @@
 var fiber = Fiber.new {
   System.print("fiber 1")
 
-  import "yield_from_import_module"
+  import "./yield_from_import_module"
 
   System.print("fiber 2")
 }

test/language/foreign/unknown_method.wren

@@ -1,3 +1,3 @@
 class Foo {
-  foreign someUnknownMethod // expect runtime error: Could not find foreign method 'someUnknownMethod' for class Foo in module 'main'.
+  foreign someUnknownMethod // expect runtime error: Could not find foreign method 'someUnknownMethod' for class Foo in module 'test/language/foreign/unknown_method'.
 }

test/language/module/change_imported_value/change_imported_value.wren

@@ -1,4 +1,4 @@
-import "module" for Module, Other
+import "./module" for Module, Other
 
 System.print(Module) // expect: before
 

test/language/module/compile_error/compile_error.wren

@@ -1,2 +1,2 @@
 System.print("before") // expect: before
-import "module" for Module // expect runtime error: Could not compile module 'module'.
+import "./module" for Module // expect runtime error: Could not compile module 'test/language/module/compile_error/module'.

test/language/module/cyclic_import/a.wren

@@ -3,7 +3,7 @@ System.print("start a")
 
 var A = "a value"
 System.print("a defined %(A)")
-import "b" for B
+import "./b" for B
 System.print("a imported %(B)")
 
 System.print("end a")

test/language/module/cyclic_import/b.wren

@@ -3,7 +3,7 @@ System.print("start b")
 
 var B = "b value"
 System.print("b defined %(B)")
-import "a" for A
+import "./a" for A
 System.print("b imported %(A)")
 
 System.print("end b")

test/language/module/cyclic_import/cyclic_import.wren

@@ -1,4 +1,4 @@
-import "a"
+import "./a"
 
 // Shared module should only run once:
 // expect: start a

test/language/module/implicitly_imports_core/implicitly_imports_core.wren

@@ -1,4 +1,4 @@
-import "module"
+import "./module"
 // expect: Bool
 // expect: Class
 // expect: Fiber

test/language/module/inside_block/inside_block.wren

@@ -1,7 +1,7 @@
 var Module = "outer"
 
 if (true) {
-  import "module" for Module
+  import "./module" for Module
   // expect: ran module
 
   System.print(Module) // expect: from module

test/language/module/missing_for.wren

@@ -1 +1 @@
-import "module" NoString // expect error
+import "./module" NoString // expect error

test/language/module/module_dir/module_dir.wren

@@ -1,3 +1,3 @@
-import "something" for Index
+import "./something/module" for Index
 
 System.print(Index) // expect: index

test/language/module/multiple_variables/multiple_variables.wren

@@ -1,4 +1,4 @@
-import "module" for Module1, Module2, Module3, Module4, Module5
+import "./module" for Module1, Module2, Module3, Module4, Module5
 
 // Only execute module body once:
 // expect: ran module

test/language/module/name_collision.wren

@@ -1,2 +1,2 @@
 var Collides
-import "module" for Collides // expect error
+import "./module" for Collides // expect error

test/language/module/newlines/newlines.wren

@@ -1,9 +1,9 @@
 import
 
 
-"module"
+"./module"
 
-import "module" for
+import "./module" for
 
 A,
 

test/language/module/no_variable/no_variable.wren

@@ -1,2 +1,2 @@
-import "module"
+import "./module"
 // expect: ran module

test/language/module/relative_import/module_3.wren

@@ -0,0 +1,2 @@
+// nontest
+System.print("module_3")

test/language/module/relative_import/relative_import.wren

@@ -0,0 +1,8 @@
+import "./sub/module"
+import "./sub/././///dir/module"
+// expect: sub/module
+// expect: sub/module_2
+// expect: sub/dir/module
+// expect: sub/dir/module_2
+// expect: sub/module_3
+// expect: module_3

test/language/module/relative_import/sub/dir/module.wren

@@ -0,0 +1,3 @@
+// nontest
+System.print("sub/dir/module")
+import "./module_2"

test/language/module/relative_import/sub/dir/module_2.wren

@@ -0,0 +1,4 @@
+// nontest
+System.print("sub/dir/module_2")
+import "../module_3"
+import "../../module_3"

test/language/module/relative_import/sub/module.wren

@@ -0,0 +1,3 @@
+// nontest
+System.print("sub/module")
+import "./module_2"

test/language/module/relative_import/sub/module_2.wren

@@ -0,0 +1,2 @@
+// nontest
+System.print("sub/module_2")

test/language/module/relative_import/sub/module_3.wren

@@ -0,0 +1,2 @@
+// nontest
+System.print("sub/module_3")

test/language/module/shared_import/a.wren

@@ -1,5 +1,5 @@
 // nontest
 System.print("a")
-import "shared" for Shared
+import "./shared" for Shared
 var A = "a %(Shared)"
 System.print("a done")

test/language/module/shared_import/b.wren

@@ -1,5 +1,5 @@
 // nontest
 System.print("b")
-import "shared" for Shared
+import "./shared" for Shared
 var B = "b %(Shared)"
 System.print("b done")

test/language/module/shared_import/shared_import.wren

@@ -1,5 +1,5 @@
-import "a" for A
-import "b" for B
+import "./a" for A
+import "./b" for B
 
 // Shared module should only run once:
 // expect: a

test/language/module/simple_import/simple_import.wren

@@ -1,4 +1,4 @@
-import "module" for Module
+import "./module" for Module
 // expect: ran module
 
 System.print(Module) // expect: from module

test/language/module/unknown_module.wren

@@ -1 +1 @@
-import "does_not_exist" for DoesNotExist // expect runtime error: Could not load module 'does_not_exist'.
+import "./does_not_exist" for DoesNotExist // expect runtime error: Could not load module 'test/language/module/does_not_exist'.

test/language/module/unknown_variable/unknown_variable.wren

@@ -1,3 +1,3 @@
 // Should execute the module:
 // expect: ran module
-import "module" for DoesNotExist // expect runtime error: Could not find a variable named 'DoesNotExist' in module 'module'.
+import "./module" for DoesNotExist // expect runtime error: Could not find a variable named 'DoesNotExist' in module 'test/language/module/unknown_variable/module'.

test/meta/get_module_variables.wren

@@ -1,6 +1,6 @@
 import "meta" for Meta
 
-var variables = Meta.getModuleVariables("main")
+var variables = Meta.getModuleVariables("test/meta/get_module_variables")
 
 // Includes implicitly imported core stuff.
 System.print(variables.contains("Object")) // expect: true

util/benchmark.py

@@ -40,7 +40,7 @@ import sys
 
 WREN_DIR = os.path.dirname(os.path.dirname(os.path.realpath(__file__)))
 WREN_BIN = os.path.join(WREN_DIR, 'bin')
-BENCHMARK_DIR = os.path.join(WREN_DIR, 'test', 'benchmark')
+BENCHMARK_DIR = os.path.join('test', 'benchmark')
 
 # How many times to run a given benchmark.
 NUM_TRIALS = 10

util/test.py

@@ -32,7 +32,7 @@ EXPECT_ERROR_PATTERN = re.compile(r'// expect error(?! line)')
 EXPECT_ERROR_LINE_PATTERN = re.compile(r'// expect error line (\d+)')
 EXPECT_RUNTIME_ERROR_PATTERN = re.compile(r'// expect (handled )?runtime error: (.+)')
 ERROR_PATTERN = re.compile(r'\[.* line (\d+)\] Error')
-STACK_TRACE_PATTERN = re.compile(r'\[main line (\d+)\] in')
+STACK_TRACE_PATTERN = re.compile(r'\[test/.* line (\d+)\] in')
 STDIN_PATTERN = re.compile(r'// stdin: (.*)')
 SKIP_PATTERN = re.compile(r'// skip: (.*)')
 NONTEST_PATTERN = re.compile(r'// nontest')