From b8fade23de7a1b987910f59456b27743d905414a Mon Sep 17 00:00:00 2001 From: meisl Date: Sun, 1 Aug 2021 15:15:54 +0200 Subject: [PATCH] * (first quick) fix: ModuleImporter should look in given "libdirs" (or better "srcdirs"?) for module file --- compilerAst/src/prog8/parser/ModuleParsing.kt | 35 +- compilerAst/src/prog8/parser/SourceCode.kt | 13 +- compilerAst/test/TestModuleImporter.kt | 363 +++++++++++------- compilerAst/test/helpers/paths.kt | 2 + compilerAst/test/helpers_pathsTests.kt | 52 +++ 5 files changed, 312 insertions(+), 153 deletions(-) diff --git a/compilerAst/src/prog8/parser/ModuleParsing.kt b/compilerAst/src/prog8/parser/ModuleParsing.kt index 40a1a81f4..c66d95e10 100644 --- a/compilerAst/src/prog8/parser/ModuleParsing.kt +++ b/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) { + libdirs: List) { + + private val libpaths: List = 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 diff --git a/compilerAst/src/prog8/parser/SourceCode.kt b/compilerAst/src/prog8/parser/SourceCode.kt index 44f4fdb25..57d1a7724 100644 --- a/compilerAst/src/prog8/parser/SourceCode.kt +++ b/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() diff --git a/compilerAst/test/TestModuleImporter.kt b/compilerAst/test/TestModuleImporter.kt index e3c1105ba..16edb94c4 100644 --- a/compilerAst/test/TestModuleImporter.kt +++ b/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,172 +18,265 @@ import prog8.parser.ModuleImporter @TestInstance(TestInstance.Lifecycle.PER_CLASS) class TestModuleImporter { - private val count = listOf("1st", "2nd", "3rd", "4th", "5th") - @Test - fun testImportModuleWithExistingPath_absolute() { - val program = Program("foo", mutableListOf(), DummyFunctions, DummyMemsizer) - val searchIn = listOf( - Path(".").div(workingDir.relativize(fixturesDir)), // we do want a dot "." in front - ).map { it.invariantSeparatorsPathString } - val importer = ModuleImporter(program, "blah", searchIn) - val fileName = "simple_main.p8" - val path = assumeReadableFile(searchIn[0], fileName) + @Nested + inner class Constructor { - val module = importer.importModule(path.absolute()) - assertThat(module.program, `is`(program)) + @Test + @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() {} } - @Test - fun testImportModuleWithExistingPath_relativeToWorkingDir() { - val program = Program("foo", mutableListOf(), DummyFunctions, DummyMemsizer) - val searchIn = listOf( - Path(".").div(workingDir.relativize(fixturesDir)), // we do want a dot "." in front - ).map { it.invariantSeparatorsPathString } - val importer = ModuleImporter(program, "blah", searchIn) - val fileName = "simple_main.p8" - val path = assumeReadableFile(searchIn[0], fileName) - assertThat("sanity check: path should NOT be absolute", path.isAbsolute, `is`(false)) + @Nested + inner class ImportModule { - val module = importer.importModule(path) - assertThat(module.program, `is`(program)) - } + @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() - @Test - fun testImportModuleWithExistingPath_relativeTo1stDirInSearchList() { - val program = Program("foo", mutableListOf(), DummyFunctions, DummyMemsizer) - val searchIn = listOf( - Path(".").div(workingDir.relativize(fixturesDir)), // we do want a dot "." in front - ).map { it.invariantSeparatorsPathString } - val importer = ModuleImporter(program, "blah", searchIn) - val fileName = "simple_main.p8" - val path = Path(".", fileName) - assumeReadableFile(searchIn[0], path) + assertThrows { 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()}") + ) + } - val module = importer.importModule(path) - assertThat(module.program, `is`(program)) - } - - @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 { importer.importModule(srcPath) } - } - - @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 { importer.importModule(srcPath) } - .let { - assertThat(it.message!!, containsString("$srcPath")) - assertThat(it.file, `is`(srcPath.toFile())) + assertThrows { 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 testImportModuleWithSyntaxError() { - 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") + @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() - val act = { importer.importModule(srcPath) } + assertThrows { 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()}") + ) + } - repeat (2) { n -> - assertThrows(count[n] + " call") { act() }.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)) - assertThat("endCol; should be 0-based", it.position.endCol, `is`(6)) + assertThrows { 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 testImportModuleWithImportingModuleWithSyntaxError() { - 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") + @Nested + inner class WithValidPath { - val act = { importer.importModule(importing) } + @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 + ).map { it.invariantSeparatorsPathString } + val importer = ModuleImporter(program, "blah", searchIn) + val fileName = "simple_main.p8" + val path = assumeReadableFile(searchIn[0], fileName) - repeat (2) { n -> - assertThrows(count[n] + " call") { act() }.let { - assertThat(it.position.file, `is`(imported.absolutePathString())) - assertThat("line; should be 1-based", it.position.line, `is`(2)) - assertThat("startCol; should be 0-based", it.position.startCol, `is`(6)) - assertThat("endCol; should be 0-based", it.position.endCol, `is`(6)) + val module = importer.importModule(path.absolute()) + assertThat(module.program, `is`(program)) } - } - } - @Test - fun testImportLibraryModuleWithNonExistingName() { - 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 + @Test + fun testRelativeToWorkingDir() { + val program = Program("foo", mutableListOf(), DummyFunctions, DummyMemsizer) + val searchIn = listOf( + Path(".").div(workingDir.relativize(fixturesDir)), // we do want a dot "." in front + ).map { it.invariantSeparatorsPathString } + val importer = ModuleImporter(program, "blah", searchIn) + val fileName = "simple_main.p8" + val path = assumeReadableFile(searchIn[0], fileName) + assertThat("sanity check: path should NOT be absolute", path.isAbsolute, `is`(false)) - repeat (2) { n -> - assertThrows(count[n] + " call / NO .p8 extension") - { importer.importLibraryModule(filenameNoExt) }.let { - assertThat(it.message!!, containsString(filenameWithExt)) + val module = importer.importModule(path) + assertThat(module.program, `is`(program)) + } + + @Test + fun testRelativeTo1stDirInSearchList() { + val program = Program("foo", mutableListOf(), DummyFunctions, DummyMemsizer) + val searchIn = listOf( + Path(".").div(workingDir.relativize(fixturesDir)), // we do want a dot "." in front + ).map { it.invariantSeparatorsPathString } + val importer = ModuleImporter(program, "blah", searchIn) + val fileName = "simple_main.p8" + val path = Path(".", fileName) + assumeReadableFile(searchIn[0], path) + + val module = importer.importModule(path) + assertThat(module.program, `is`(program)) + } + + @Test + @Disabled("TODO: relative to 2nd in search list") + fun testRelativeTo2ndDirInSearchList() {} + + @Test + @Disabled("TODO: ambiguous - 2 or more really different candidates") + fun testAmbiguousCandidates() {} + + @Nested + inner class WithBadFile { + @Test + 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 act = { importer.importModule(srcPath) } + + repeat(2) { n -> + assertThrows(count[n] + " call") { act() }.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)) + assertThat("endCol; should be 0-based", it.position.endCol, `is`(6)) + } + } } - assertThrows(count[n] + " call / with .p8 extension") - { importer.importLibraryModule(filenameWithExt) }.let { - assertThat(it.message!!, containsString(filenameWithExt)) + + @Test + 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 act = { importer.importModule(importing) } + + repeat(2) { n -> + assertThrows(count[n] + " call") { act() }.let { + assertThat(it.position.file, `is`(imported.absolutePathString())) + assertThat("line; should be 1-based", it.position.line, `is`(2)) + assertThat("startCol; should be 0-based", it.position.startCol, `is`(6)) + assertThat("endCol; should be 0-based", it.position.endCol, `is`(6)) + } + } } + } } + } - @Test - fun testImportLibraryModuleWithSyntaxError() { - 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") + @Nested + inner class ImportLibraryModule { + @Nested + inner class WithInvalidName { + @Test + 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 -> - assertThrows (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)) - assertThat("endCol; should be 0-based", it.position.endCol, `is`(6)) + repeat(2) { n -> + assertThrows(count[n] + " call / NO .p8 extension") + { importer.importLibraryModule(filenameNoExt) }.let { + assertThat(it.message!!, containsString(filenameWithExt)) + } + assertThrows(count[n] + " call / with .p8 extension") + { importer.importLibraryModule(filenameWithExt) }.let { + assertThat(it.message!!, containsString(filenameWithExt)) + } } + } } - } - @Test - fun testImportLibraryModuleWithImportingBadModule() { - 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") + @Nested + inner class WithValidName { + @Nested + inner class WithBadFile { + @Test + 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 act = { importer.importLibraryModule(importing.nameWithoutExtension) } + repeat(2) { n -> + assertThrows(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)) + assertThat("endCol; should be 0-based", it.position.endCol, `is`(6)) + } + } + } - repeat(2) { n -> - assertThrows(count[n] + " call") { act() }.let { - assertThat(it.position.file, `is`(imported.normalize().absolutePathString())) - assertThat("line; should be 1-based", it.position.line, `is`(2)) - assertThat("startCol; should be 0-based", it.position.startCol, `is`(6)) - assertThat("endCol; should be 0-based", it.position.endCol, `is`(6)) + @Test + 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 act = { importer.importLibraryModule(importing.nameWithoutExtension) } + + repeat(2) { n -> + assertThrows(count[n] + " call") { act() }.let { + assertThat(it.position.file, `is`(imported.normalize().absolutePathString())) + assertThat("line; should be 1-based", it.position.line, `is`(2)) + assertThat("startCol; should be 0-based", it.position.startCol, `is`(6)) + assertThat("endCol; should be 0-based", it.position.endCol, `is`(6)) + } + } + } } } } - } diff --git a/compilerAst/test/helpers/paths.kt b/compilerAst/test/helpers/paths.kt index 85b29fb26..4e99518b0 100644 --- a/compilerAst/test/helpers/paths.kt +++ b/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.", diff --git a/compilerAst/test/helpers_pathsTests.kt b/compilerAst/test/helpers_pathsTests.kt index 418946b97..f85fb8363 100644 --- a/compilerAst/test/helpers_pathsTests.kt +++ b/compilerAst/test/helpers_pathsTests.kt @@ -178,6 +178,58 @@ class PathsHelpersTests { ) } } + + @Nested + inner class WithStringAndStringArgs { + @Test + fun testOnNonExistingPath() { + assertThrows { + assumeDirectory("$fixturesDir", "i_do_not_exist") + } + } + + @Test + fun testOnExistingFile() { + assertThrows { + 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 { + assumeDirectory("$fixturesDir", Path("i_do_not_exist")) + } + } + + @Test + fun testOnExistingFile() { + assertThrows { + assumeDirectory("$fixturesDir", Path("simple_main.p8")) + } + } + + @Test + fun testOnExistingDirectory() { + val path = workingDir.div("..") + assertThat( + "should return resulting path", + assumeDirectory("$workingDir", Path("..")), `is`(path) + ) + } + } } @Nested