* (first quick) fix: ModuleImporter should look in given "libdirs" (or better "srcdirs"?) for module file

compilerAst/src/prog8/parser/ModuleParsing.kt

@@ -14,21 +14,31 @@ import kotlin.io.path.*
 
 class ModuleImporter(private val program: Program,
                      private val compilationTargetName: String,
-                     private val libdirs: List<String>) {
+                     libdirs: List<String>) {
+
+    private val libpaths: List<Path> = libdirs.map { Path(it) }
 
     fun importModule(filePath: Path): Module {
-        print("importing '${filePath.nameWithoutExtension}'")
-        if (filePath.parent != null) { // TODO: use Path.relativize
-            var importloc = filePath.toString()
-            val curdir = Path("").absolutePathString()
-            if(importloc.startsWith(curdir))
-                importloc = "." + importloc.substring(curdir.length)
-            println(" (from '$importloc')")
-        }
-        else
-            println("")
+        val currentDir = Path("").absolute()
+        val searchIn = listOf(currentDir) + libpaths
+        val candidates = searchIn
+            .map { it.absolute().div(filePath).normalize().absolute() }
+            .filter { it.exists() }
+            .map { currentDir.relativize(it) }
+            .map { if (it.isAbsolute) it else Path(".", "$it") }
 
-        val module = Prog8Parser.parseModule(SourceCode.fromPath(filePath))
+        val srcPath = when (candidates.size) {
+            0 -> throw NoSuchFileException(
+                    file = filePath.normalize().toFile(),
+                    reason = "searched in $searchIn")
+            1 -> candidates.first()
+            else -> candidates.first()  // TODO: report error if more than 1 candidate?
+        }
+
+        var logMsg = "importing '${filePath.nameWithoutExtension}' (from $srcPath)"
+        println(logMsg)
+
+        val module = Prog8Parser.parseModule(SourceCode.fromPath(srcPath))
 
         module.program = program
         module.linkParents(program.namespace)
@@ -121,7 +131,6 @@ class ModuleImporter(private val program: Program,
 
     private fun tryGetModuleFromFile(name: String, importingModule: Module?): SourceCode? {
         val fileName = "$name.p8"
-        val libpaths = libdirs.map { Path(it) }
         val locations =
             if (importingModule == null) { // <=> imported from library module
                 libpaths

compilerAst/src/prog8/parser/SourceCode.kt

@@ -85,13 +85,14 @@ abstract class SourceCode {
          * @throws AccessDeniedException if the given path points to a directory or the file is non-readable for some other reason
          */
         fun fromPath(path: Path): SourceCode {
-            if (!path.exists())
-                throw NoSuchFileException(path.toFile())
-            if (path.isDirectory())
-                throw AccessDeniedException(path.toFile(), reason = "Not a file but a directory")
-            if (!path.isReadable())
-                throw AccessDeniedException(path.toFile(), reason = "Is not readable")
             val normalized = path.normalize()
+            val file = normalized.toFile()
+            if (!path.exists())
+                throw NoSuchFileException(file)
+            if (path.isDirectory())
+                throw AccessDeniedException(file, reason = "Not a file but a directory")
+            if (!path.isReadable())
+                throw AccessDeniedException(file, reason = "Is not readable")
             return object : SourceCode() {
                 override val isFromResources = false
                 override val origin = normalized.absolutePathString()

compilerAst/test/TestModuleImporter.kt

@@ -5,6 +5,8 @@ import org.junit.jupiter.api.Test
 import org.junit.jupiter.api.TestInstance
 import org.hamcrest.MatcherAssert.assertThat
 import org.hamcrest.Matchers.*
+import org.junit.jupiter.api.Disabled
+import org.junit.jupiter.api.Nested
 import org.junit.jupiter.api.assertThrows
 import kotlin.io.path.*
 
@@ -16,11 +18,103 @@ import prog8.parser.ModuleImporter
 
 @TestInstance(TestInstance.Lifecycle.PER_CLASS)
 class TestModuleImporter {
-
     private val count = listOf("1st", "2nd", "3rd", "4th", "5th")
 
+    @Nested
+    inner class Constructor {
+
         @Test
-    fun testImportModuleWithExistingPath_absolute() {
+        @Disabled("TODO: invalid entries in search list")
+        fun testInvalidEntriesInSearchList() {}
+
+        @Test
+        @Disabled("TODO: literal duplicates in search list")
+        fun testLiteralDuplicatesInSearchList() {}
+
+        @Test
+        @Disabled("TODO: factual duplicates in search list")
+        fun testFactualDuplicatesInSearchList() {}
+    }
+
+    @Nested
+    inner class ImportModule {
+
+        @Nested
+        inner class WithInvalidPath {
+            @Test
+            fun testNonexisting() {
+                val program = Program("foo", mutableListOf(), DummyFunctions, DummyMemsizer)
+                val dirRel = assumeDirectory(".", workingDir.relativize(fixturesDir))
+                val searchIn = dirRel.invariantSeparatorsPathString
+                val importer = ModuleImporter(program, "blah", listOf(searchIn))
+                val srcPathRel = assumeNotExists(dirRel, "i_do_not_exist")
+                val srcPathAbs = srcPathRel.absolute()
+
+                assertThrows<NoSuchFileException> { importer.importModule(srcPathRel) }
+                    .let {
+                        assertThat(
+                            ".file should be normalized",
+                            "${it.file}", `is`("${it.file.normalize()}")
+                        )
+                        assertThat(
+                            ".file should point to specified path",
+                            it.file.absolutePath, `is`("${srcPathAbs.normalize()}")
+                        )
+                    }
+
+                assertThrows<NoSuchFileException> { importer.importModule(srcPathAbs) }
+                    .let {
+                        assertThat(
+                            ".file should be normalized",
+                            "${it.file}", `is`("${it.file.normalize()}")
+                        )
+                        assertThat(
+                            ".file should point to specified path",
+                            it.file.absolutePath, `is`("${srcPathAbs.normalize()}")
+                        )
+                    }
+            }
+
+            @Test
+            fun testDirectory() {
+                val program = Program("foo", mutableListOf(), DummyFunctions, DummyMemsizer)
+                val dirRel = assumeDirectory(workingDir.relativize(fixturesDir))
+                val searchIn = Path(".", "$dirRel").invariantSeparatorsPathString
+                val importer = ModuleImporter(program, "blah", listOf(searchIn))
+                val srcPathRel = dirRel
+                val srcPathAbs = srcPathRel.absolute()
+
+                assertThrows<AccessDeniedException> { importer.importModule(srcPathRel) }
+                    .let {
+                        assertThat(
+                            ".file should be normalized",
+                            "${it.file}", `is`("${it.file.normalize()}")
+                        )
+                        assertThat(
+                            ".file should point to specified path",
+                            it.file.absolutePath, `is`("${srcPathAbs.normalize()}")
+                        )
+                    }
+
+                assertThrows<AccessDeniedException> { importer.importModule(srcPathAbs) }
+                    .let {
+                        assertThat(
+                            ".file should be normalized",
+                            "${it.file}", `is`("${it.file.normalize()}")
+                        )
+                        assertThat(
+                            ".file should point to specified path",
+                            it.file.absolutePath, `is`("${srcPathAbs.normalize()}")
+                        )
+                    }
+            }
+        }
+
+        @Nested
+        inner class WithValidPath {
+
+            @Test
+            fun testAbsolute() {
                 val program = Program("foo", mutableListOf(), DummyFunctions, DummyMemsizer)
                 val searchIn = listOf(
                     Path(".").div(workingDir.relativize(fixturesDir)), // we do want a dot "." in front
@@ -34,7 +128,7 @@ class TestModuleImporter {
             }
 
             @Test
-    fun testImportModuleWithExistingPath_relativeToWorkingDir() {
+            fun testRelativeToWorkingDir() {
                 val program = Program("foo", mutableListOf(), DummyFunctions, DummyMemsizer)
                 val searchIn = listOf(
                     Path(".").div(workingDir.relativize(fixturesDir)), // we do want a dot "." in front
@@ -49,7 +143,7 @@ class TestModuleImporter {
             }
 
             @Test
-    fun testImportModuleWithExistingPath_relativeTo1stDirInSearchList() {
+            fun testRelativeTo1stDirInSearchList() {
                 val program = Program("foo", mutableListOf(), DummyFunctions, DummyMemsizer)
                 val searchIn = listOf(
                     Path(".").div(workingDir.relativize(fixturesDir)), // we do want a dot "." in front
@@ -64,31 +158,17 @@ class TestModuleImporter {
             }
 
             @Test
-    fun testImportModuleWithNonExistingPath() {
-        val program = Program("foo", mutableListOf(), DummyFunctions, DummyMemsizer)
-        val searchIn = "./" + workingDir.relativize(fixturesDir).toString().replace("\\", "/")
-        val importer = ModuleImporter(program, "blah", listOf(searchIn))
-        val srcPath = assumeNotExists(fixturesDir, "i_do_not_exist")
-
-        assertThrows<NoSuchFileException> { importer.importModule(srcPath) }
-    }
+            @Disabled("TODO: relative to 2nd in search list")
+            fun testRelativeTo2ndDirInSearchList() {}
 
             @Test
-    fun testImportModuleWithDirectoryPath() {
-        val program = Program("foo", mutableListOf(), DummyFunctions, DummyMemsizer)
-        val searchIn = "./" + workingDir.relativize(fixturesDir).toString().replace("\\", "/")
-        val importer = ModuleImporter(program, "blah", listOf(searchIn))
-        val srcPath = assumeDirectory(fixturesDir)
-
-        assertThrows<AccessDeniedException> { importer.importModule(srcPath) }
-            .let {
-                assertThat(it.message!!, containsString("$srcPath"))
-                assertThat(it.file, `is`(srcPath.toFile()))
-            }
-    }
+            @Disabled("TODO: ambiguous - 2 or more really different candidates")
+            fun testAmbiguousCandidates() {}
 
+            @Nested
+            inner class WithBadFile {
                 @Test
-    fun testImportModuleWithSyntaxError() {
+                fun testWithSyntaxError() {
                     val program = Program("foo", mutableListOf(), DummyFunctions, DummyMemsizer)
                     val searchIn = "./" + workingDir.relativize(fixturesDir).toString().replace("\\", "/")
                     val importer = ModuleImporter(program, "blah", listOf(searchIn))
@@ -96,7 +176,7 @@ class TestModuleImporter {
 
                     val act = { importer.importModule(srcPath) }
 
-        repeat (2) { n ->
+                    repeat(2) { n ->
                         assertThrows<ParseError>(count[n] + " call") { act() }.let {
                             assertThat(it.position.file, `is`(srcPath.absolutePathString()))
                             assertThat("line; should be 1-based", it.position.line, `is`(2))
@@ -107,7 +187,7 @@ class TestModuleImporter {
                 }
 
                 @Test
-    fun testImportModuleWithImportingModuleWithSyntaxError() {
+                fun testImportingFileWithSyntaxError() {
                     val program = Program("foo", mutableListOf(), DummyFunctions, DummyMemsizer)
                     val searchIn = "./" + workingDir.relativize(fixturesDir).toString().replace("\\", "/")
                     val importer = ModuleImporter(program, "blah", listOf(searchIn))
@@ -116,7 +196,7 @@ class TestModuleImporter {
 
                     val act = { importer.importModule(importing) }
 
-        repeat (2) { n ->
+                    repeat(2) { n ->
                         assertThrows<ParseError>(count[n] + " call") { act() }.let {
                             assertThat(it.position.file, `is`(imported.absolutePathString()))
                             assertThat("line; should be 1-based", it.position.line, `is`(2))
@@ -125,16 +205,24 @@ class TestModuleImporter {
                         }
                     }
                 }
+            }
+        }
 
+    }
+
+    @Nested
+    inner class ImportLibraryModule {
+        @Nested
+        inner class WithInvalidName {
             @Test
-    fun testImportLibraryModuleWithNonExistingName() {
+            fun testWithNonExistingName() {
                 val program = Program("foo", mutableListOf(), DummyFunctions, DummyMemsizer)
                 val searchIn = "./" + workingDir.relativize(fixturesDir).toString().replace("\\", "/")
                 val importer = ModuleImporter(program, "blah", listOf(searchIn))
                 val filenameNoExt = assumeNotExists(fixturesDir, "i_do_not_exist").name
                 val filenameWithExt = assumeNotExists(fixturesDir, "i_do_not_exist.p8").name
 
-        repeat (2) { n ->
+                repeat(2) { n ->
                     assertThrows<NoSuchFileException>(count[n] + " call / NO .p8 extension")
                     { importer.importLibraryModule(filenameNoExt) }.let {
                         assertThat(it.message!!, containsString(filenameWithExt))
@@ -145,17 +233,22 @@ class TestModuleImporter {
                     }
                 }
             }
+        }
 
+        @Nested
+        inner class WithValidName {
+            @Nested
+            inner class WithBadFile {
                 @Test
-    fun testImportLibraryModuleWithSyntaxError() {
+                fun testWithSyntaxError() {
                     val program = Program("foo", mutableListOf(), DummyFunctions, DummyMemsizer)
                     val searchIn = "./" + workingDir.relativize(fixturesDir).toString().replace("\\", "/")
                     val importer = ModuleImporter(program, "blah", listOf(searchIn))
-        val srcPath = assumeReadableFile(fixturesDir,"file_with_syntax_error.p8")
+                    val srcPath = assumeReadableFile(fixturesDir, "file_with_syntax_error.p8")
 
-        repeat (2) { n ->
-            assertThrows<ParseError> (count[n] + " call")
-                { importer.importLibraryModule(srcPath.nameWithoutExtension) } .let {
+                    repeat(2) { n ->
+                        assertThrows<ParseError>(count[n] + " call")
+                        { importer.importLibraryModule(srcPath.nameWithoutExtension) }.let {
                             assertThat(it.position.file, `is`(srcPath.absolutePathString()))
                             assertThat("line; should be 1-based", it.position.line, `is`(2))
                             assertThat("startCol; should be 0-based", it.position.startCol, `is`(6))
@@ -165,12 +258,12 @@ class TestModuleImporter {
                 }
 
                 @Test
-    fun testImportLibraryModuleWithImportingBadModule() {
+                fun testImportingFileWithSyntaxError() {
                     val program = Program("foo", mutableListOf(), DummyFunctions, DummyMemsizer)
                     val searchIn = "./" + workingDir.relativize(fixturesDir).toString().replace("\\", "/")
                     val importer = ModuleImporter(program, "blah", listOf(searchIn))
                     val importing = assumeReadableFile(fixturesDir, "import_file_with_syntax_error.p8")
-        val imported = assumeReadableFile(fixturesDir,"file_with_syntax_error.p8")
+                    val imported = assumeReadableFile(fixturesDir, "file_with_syntax_error.p8")
 
                     val act = { importer.importLibraryModule(importing.nameWithoutExtension) }
 
@@ -183,5 +276,7 @@ class TestModuleImporter {
                         }
                     }
                 }
-
+            }
+        }
+    }
 }

compilerAst/test/helpers/paths.kt

@@ -42,6 +42,8 @@ fun assumeDirectory(path: Path): Path {
 
 fun assumeDirectory(pathStr: String): Path = assumeDirectory(Path(pathStr))
 fun assumeDirectory(path: Path, other: String): Path = assumeDirectory(path.div(other))
+fun assumeDirectory(pathStr: String, other: String): Path = assumeDirectory(Path(pathStr).div(other))
+fun assumeDirectory(pathStr: String, other: Path): Path = assumeDirectory(Path(pathStr).div(other))
 
 
 @Deprecated("Directories are checked automatically at init.",

compilerAst/test/helpers_pathsTests.kt

@@ -178,6 +178,58 @@ class PathsHelpersTests {
                 )
             }
         }
+
+        @Nested
+        inner class WithStringAndStringArgs {
+            @Test
+            fun testOnNonExistingPath() {
+                assertThrows<AssertionError> {
+                    assumeDirectory("$fixturesDir", "i_do_not_exist")
+                }
+            }
+
+            @Test
+            fun testOnExistingFile() {
+                assertThrows<AssertionError> {
+                    assumeDirectory("$fixturesDir", "simple_main.p8")
+                }
+            }
+
+            @Test
+            fun testOnExistingDirectory() {
+                val path = workingDir.div("..")
+                assertThat(
+                    "should return resulting path",
+                    assumeDirectory("$workingDir", ".."), `is`(path)
+                )
+            }
+        }
+
+        @Nested
+        inner class WithStringAndPathArgs {
+            @Test
+            fun testOnNonExistingPath() {
+                assertThrows<AssertionError> {
+                    assumeDirectory("$fixturesDir", Path("i_do_not_exist"))
+                }
+            }
+
+            @Test
+            fun testOnExistingFile() {
+                assertThrows<AssertionError> {
+                    assumeDirectory("$fixturesDir", Path("simple_main.p8"))
+                }
+            }
+
+            @Test
+            fun testOnExistingDirectory() {
+                val path = workingDir.div("..")
+                assertThat(
+                    "should return resulting path",
+                    assumeDirectory("$workingDir", Path("..")), `is`(path)
+                )
+            }
+        }
     }
 
     @Nested