From 79ef1e95fb4ecc3ed34ba6bd8f346643bf06c32b Mon Sep 17 00:00:00 2001 From: David Lurton Date: Fri, 14 May 2021 01:02:05 -0700 Subject: [PATCH 01/22] Add import statement (#57) --- README.md | 18 +- pig/src/org/partiql/pig/cmdline/Command.kt | 2 +- .../partiql/pig/cmdline/CommandLineParser.kt | 2 +- .../pig/domain/model/SemanticErrorContext.kt | 6 +- .../pig/domain/parser/ImportSourceOpener.kt | 34 ++ .../partiql/pig/domain/parser/InputSource.kt | 45 ++ .../pig/domain/parser/ParserErrorContext.kt | 11 +- .../pig/domain/parser/SourceLocation.kt | 36 ++ .../pig/domain/parser/TypeDomainParser.kt | 512 ++++++++++-------- pig/src/org/partiql/pig/errors/PigError.kt | 7 +- pig/src/org/partiql/pig/main.kt | 6 +- .../domains/circular_universe_c.ion | 4 + .../domains/circular_universe_d.ion | 4 + pig/test-domains/domains/import_b.ion | 4 + pig/test-domains/domains/universe_a.ion | 3 + pig/test-domains/main.ion | 4 + pig/test-domains/other/universe_b.ion | 3 + .../pig/cmdline/CommandLineParserTests.kt | 12 +- .../partiql/pig/domain/PermuteDomainTests.kt | 5 +- .../pig/domain/TypeDomainParserErrorsTest.kt | 4 +- .../pig/domain/TypeDomainParserImportTests.kt | 40 ++ .../pig/domain/TypeDomainParserTests.kt | 11 +- .../domain/TypeDomainSemanticCheckerTests.kt | 4 +- pig/test/org/partiql/pig/domain/Util.kt | 3 +- pig/test/org/partiql/pig/util/ParseHelpers.kt | 50 ++ 25 files changed, 573 insertions(+), 257 deletions(-) create mode 100644 pig/src/org/partiql/pig/domain/parser/ImportSourceOpener.kt create mode 100644 pig/src/org/partiql/pig/domain/parser/InputSource.kt create mode 100644 pig/src/org/partiql/pig/domain/parser/SourceLocation.kt create mode 100644 pig/test-domains/domains/circular_universe_c.ion create mode 100644 pig/test-domains/domains/circular_universe_d.ion create mode 100644 pig/test-domains/domains/import_b.ion create mode 100644 pig/test-domains/domains/universe_a.ion create mode 100644 pig/test-domains/main.ion create mode 100644 pig/test-domains/other/universe_b.ion create mode 100644 pig/test/org/partiql/pig/domain/TypeDomainParserImportTests.kt create mode 100644 pig/test/org/partiql/pig/util/ParseHelpers.kt diff --git a/README.md b/README.md index 1c017b8..db42771 100644 --- a/README.md +++ b/README.md @@ -174,7 +174,9 @@ assertEquals(onePlusOne, anotherOnePlusOne) // Top level type_universe ::= ... definition ::= '(' 'define' symbol ')' -stmt ::= | +stmt ::= | | + +import ::= `(import | @@ -400,6 +402,20 @@ Unlike record elements, product element defintions must include identifiers. (product int_pair first::int second::int) ``` +#### Imports + +It is possible to split type universe definitions among multiple files: + +``` +// root.ion: +(import "a.ion") +(import "b.ion") +``` + +The resulting type universe will contain all type domains from both `a.ion` and `b.ion`, `root.ion` may also +define additional type domains. The primary purpose of this is to be able to permute domains defined in another +file. + #### Using PIG In Your Project diff --git a/pig/src/org/partiql/pig/cmdline/Command.kt b/pig/src/org/partiql/pig/cmdline/Command.kt index fa5ede4..c565d7e 100644 --- a/pig/src/org/partiql/pig/cmdline/Command.kt +++ b/pig/src/org/partiql/pig/cmdline/Command.kt @@ -20,5 +20,5 @@ import java.io.File sealed class Command { object ShowHelp : Command() data class InvalidCommandLineArguments(val message: String) : Command() - data class Generate(val typeUniverseFile: File, val outputFile: File, val target: TargetLanguage) : Command() + data class Generate(val typeUniverseFile: String, val outputFile: String, val target: TargetLanguage) : Command() } \ No newline at end of file diff --git a/pig/src/org/partiql/pig/cmdline/CommandLineParser.kt b/pig/src/org/partiql/pig/cmdline/CommandLineParser.kt index e8f11c7..232305f 100644 --- a/pig/src/org/partiql/pig/cmdline/CommandLineParser.kt +++ b/pig/src/org/partiql/pig/cmdline/CommandLineParser.kt @@ -145,7 +145,7 @@ class CommandLineParser { LanguageTargetType.CUSTOM -> TargetLanguage.Custom(optSet.valueOf(templateOpt)) } - Command.Generate(typeUniverseFile, outputFile, target) + Command.Generate(typeUniverseFile.toString(), outputFile.toString(), target) } } } catch(ex: OptionException) { diff --git a/pig/src/org/partiql/pig/domain/model/SemanticErrorContext.kt b/pig/src/org/partiql/pig/domain/model/SemanticErrorContext.kt index 5d7b20e..61c9c1e 100644 --- a/pig/src/org/partiql/pig/domain/model/SemanticErrorContext.kt +++ b/pig/src/org/partiql/pig/domain/model/SemanticErrorContext.kt @@ -18,6 +18,8 @@ package org.partiql.pig.domain.model import com.amazon.ionelement.api.IonLocation import com.amazon.ionelement.api.MetaContainer import com.amazon.ionelement.api.location +import org.partiql.pig.domain.parser.SourceLocation +import org.partiql.pig.domain.parser.sourceLocation import org.partiql.pig.errors.PigException import org.partiql.pig.errors.ErrorContext import org.partiql.pig.errors.PigError @@ -109,9 +111,9 @@ sealed class SemanticErrorContext(val msgFormatter: () -> String): ErrorContext * Shortcut for throwing [PigException] with the specified metas and [PigError]. */ fun semanticError(blame: MetaContainer, context: ErrorContext): Nothing = - semanticError(blame.location, context) + semanticError(blame.sourceLocation, context) /** * Shortcut for throwing [PigException] with the specified metas and [PigError]. */ -fun semanticError(blame: IonLocation?, context: ErrorContext): Nothing = +fun semanticError(blame: SourceLocation?, context: ErrorContext): Nothing = throw PigException(PigError(blame, context)) diff --git a/pig/src/org/partiql/pig/domain/parser/ImportSourceOpener.kt b/pig/src/org/partiql/pig/domain/parser/ImportSourceOpener.kt new file mode 100644 index 0000000..6f6e434 --- /dev/null +++ b/pig/src/org/partiql/pig/domain/parser/ImportSourceOpener.kt @@ -0,0 +1,34 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + */ + +package org.partiql.pig.domain.parser + +import com.amazon.ion.IonReader +import com.amazon.ion.system.IonReaderBuilder +import java.io.Closeable +import java.io.File +import java.io.FileInputStream +import java.nio.file.Path + +//class ImportSource( +// val fullyQualifiedName: String, +// val reader: IonReader +//) : Closeable { +// override fun close() { +// reader.close() +// } +//} + +// TODO: names diff --git a/pig/src/org/partiql/pig/domain/parser/InputSource.kt b/pig/src/org/partiql/pig/domain/parser/InputSource.kt new file mode 100644 index 0000000..01055fd --- /dev/null +++ b/pig/src/org/partiql/pig/domain/parser/InputSource.kt @@ -0,0 +1,45 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + */ + +package org.partiql.pig.domain.parser + +import java.io.File +import java.io.FileInputStream +import java.io.InputStream + +/** + * Provides an abstraction for file-system related functions used when importing files. + */ +internal interface InputSource { + /** + * Opens an input stream for the given source name. + * + * The [sourceName] is implementation-defined. In the case of a file system implementaiton it is the path to a + * file, either relative to the working directory or absolute. + */ + fun openStream(sourceName: String): InputStream + + /** + * Returns the "canonical name" of the given source. In the case of a file system, this converts the relative + * path to an absolute path. + */ + fun getCanonicalName(sourceName: String): String +} + +internal val FILE_SYSTEM_SOURCE = object : InputSource { + override fun openStream(qualifiedSource: String) = FileInputStream(qualifiedSource) + + override fun getCanonicalName(sourceName: String): String = File(sourceName).canonicalFile.toString() +} \ No newline at end of file diff --git a/pig/src/org/partiql/pig/domain/parser/ParserErrorContext.kt b/pig/src/org/partiql/pig/domain/parser/ParserErrorContext.kt index 7c4ddba..495c736 100644 --- a/pig/src/org/partiql/pig/domain/parser/ParserErrorContext.kt +++ b/pig/src/org/partiql/pig/domain/parser/ParserErrorContext.kt @@ -42,6 +42,9 @@ sealed class ParserErrorContext(val msgFormatter: () -> String): ErrorContext { override fun hashCode(): Int = 0 } + data class CouldNotFindImportedTypeUniverse(val tag: String) + : ParserErrorContext({ "Could not find imported type universe: $tag" }) + data class UnknownConstructor(val tag: String) : ParserErrorContext({ "Unknown constructor: '$tag' (expected constructors are 'domain' or 'permute_domain')" }) @@ -79,8 +82,7 @@ sealed class ParserErrorContext(val msgFormatter: () -> String): ErrorContext { : ParserErrorContext({ "Element has multiple name annotations"}) } - -fun parseError(blame: IonLocation?, context: ErrorContext): Nothing = +fun parseError(blame: SourceLocation?, context: ErrorContext): Nothing = PigError(blame, context).let { throw when (context) { is ParserErrorContext.IonElementError -> { @@ -91,8 +93,3 @@ fun parseError(blame: IonLocation?, context: ErrorContext): Nothing = } } -fun parseError(blame: IonElement, context: ErrorContext): Nothing { - val loc = blame.metas.location - parseError(loc, context) -} - diff --git a/pig/src/org/partiql/pig/domain/parser/SourceLocation.kt b/pig/src/org/partiql/pig/domain/parser/SourceLocation.kt new file mode 100644 index 0000000..fdb307c --- /dev/null +++ b/pig/src/org/partiql/pig/domain/parser/SourceLocation.kt @@ -0,0 +1,36 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + */ + +package org.partiql.pig.domain.parser + +import com.amazon.ionelement.api.IonLocation +import com.amazon.ionelement.api.MetaContainer + +/** + * Includes path to a source file and a position within it. + * + * Used to construct helpful error messages for the end-user, who will be able to know the file, line & column of + * a given error. + */ +data class SourceLocation(val path: String, val location: IonLocation) { + override fun toString(): String { + return "$path:$location" + } +} + +internal const val SOURCE_LOCATION_META_TAG = "\$pig_source_location" + +internal val MetaContainer.sourceLocation + get() = this[SOURCE_LOCATION_META_TAG] as? SourceLocation diff --git a/pig/src/org/partiql/pig/domain/parser/TypeDomainParser.kt b/pig/src/org/partiql/pig/domain/parser/TypeDomainParser.kt index fb23328..b4c78da 100644 --- a/pig/src/org/partiql/pig/domain/parser/TypeDomainParser.kt +++ b/pig/src/org/partiql/pig/domain/parser/TypeDomainParser.kt @@ -15,275 +15,351 @@ package org.partiql.pig.domain.parser -import com.amazon.ionelement.api.* import com.amazon.ion.IonReader import com.amazon.ion.system.IonReaderBuilder -import org.partiql.pig.domain.model.* +import com.amazon.ionelement.api.AnyElement +import com.amazon.ionelement.api.IonElement +import com.amazon.ionelement.api.IonElementException +import com.amazon.ionelement.api.IonElementLoaderOptions +import com.amazon.ionelement.api.IonLocation +import com.amazon.ionelement.api.MetaContainer +import com.amazon.ionelement.api.SexpElement +import com.amazon.ionelement.api.SymbolElement +import com.amazon.ionelement.api.createIonElementLoader +import com.amazon.ionelement.api.emptyMetaContainer +import com.amazon.ionelement.api.head +import com.amazon.ionelement.api.location +import com.amazon.ionelement.api.metaContainerOf +import com.amazon.ionelement.api.tag +import com.amazon.ionelement.api.tail +import org.partiql.pig.domain.model.Arity +import org.partiql.pig.domain.model.DataType +import org.partiql.pig.domain.model.NamedElement +import org.partiql.pig.domain.model.PermutedDomain +import org.partiql.pig.domain.model.PermutedSum +import org.partiql.pig.domain.model.Statement +import org.partiql.pig.domain.model.Transform +import org.partiql.pig.domain.model.TupleType +import org.partiql.pig.domain.model.TypeDomain +import org.partiql.pig.domain.model.TypeRef +import org.partiql.pig.domain.model.TypeUniverse +import org.partiql.pig.errors.ErrorContext +import java.io.File +import java.io.FileNotFoundException +import java.io.InputStream +import java.util.Stack + + +fun parseTypeUniverseFile(path: String): TypeUniverse { + val parser = Parser(FILE_SYSTEM_SOURCE) + return parser.parseTypeUniverse(path) +} + +private class Parser( + val inputSource: InputSource +) { + /** This contains every file the parser has "seen" and is used detect and prevent import cycles. */ + private val parseHistory = HashSet() + private val qualifedSourceStack = Stack().apply { push(".") } + + /** Parses a type universe in the specified [IonReader]. */ + fun parseTypeUniverse(source: String): TypeUniverse { + val elementLoader = createIonElementLoader(IonElementLoaderOptions(includeLocationMeta = true)) + val qualifiedSource = inputSource.getCanonicalName(source) + return inputSource.openStream(qualifiedSource).use { inputStream: InputStream -> + IonReaderBuilder.standard().build(inputStream).use { reader: IonReader -> + this.parseHistory.add(qualifiedSource) + this.qualifedSourceStack.push(qualifiedSource) + val domains = try { + val topLevelElements = elementLoader.loadAllElements(reader) + topLevelElements.flatMap { topLevelValue -> + val topLevelSexp = topLevelValue.asSexp() + when (topLevelSexp.tag) { + "define" -> listOf(parseDefine(topLevelSexp)) + "transform" -> listOf(parseTransform(topLevelSexp)) + "import" -> parseImport(topLevelSexp) + else -> parseError( + topLevelSexp.head, + ParserErrorContext.InvalidTopLevelTag(topLevelSexp.tag)) + } + } + } catch (iee: IonElementException) { + parseError(iee.location?.toSourceLocation(), ParserErrorContext.IonElementError(iee)) + } + this.qualifedSourceStack.pop() + TypeUniverse(domains) + } + } + + } -/** Parses a type universe contained in [universeText]. */ -fun parseTypeUniverse(universeText: String) = - IonReaderBuilder.standard().build(universeText).use { - parseTypeUniverse(it) + fun parseError(blame: IonElement, context: ErrorContext): Nothing { + val loc = blame.metas.location?.toSourceLocation() + parseError(loc, context) } -/** Parses a type universe in the specified [IonReader]. */ -fun parseTypeUniverse(reader: IonReader): TypeUniverse { - val elementLoader = createIonElementLoader(IonElementLoaderOptions(includeLocationMeta = true)) - - val domains = try { - val topLevelElements = elementLoader.loadAllElements(reader) - topLevelElements.map { topLevelValue -> - val topLevelSexp = topLevelValue.asSexp() - when (topLevelSexp.tag) { - "define" -> parseDefine(topLevelSexp) - "transform" -> parseTransform(topLevelSexp) - else -> parseError( - topLevelSexp.head, - ParserErrorContext.InvalidTopLevelTag(topLevelSexp.tag)) + private fun IonLocation.toSourceLocation() = SourceLocation(qualifedSourceStack.peek(), this) + + private fun MetaContainer.toSourceLocationMetas(): MetaContainer = this.location?.let { + metaContainerOf(SOURCE_LOCATION_META_TAG to it.toSourceLocation()) + } ?: emptyMetaContainer() + + private fun parseImport(sexp: SexpElement): List { + requireArityForTag(sexp, 1) + val relativePath = sexp.tail.single().asString().textValue + // TODO: need to report proper error to user if file does not exist. + + val workingDirectory = File(this.qualifedSourceStack.peek()).parentFile + val qualifiedSourcePath = File(workingDirectory, relativePath).canonicalPath + return if(!parseHistory.contains(qualifiedSourcePath)) { + try { + parseTypeUniverse(qualifiedSourcePath).statements + } catch (e: FileNotFoundException) { + parseError( + sexp.metas.location?.toSourceLocation(), + ParserErrorContext.CouldNotFindImportedTypeUniverse(qualifiedSourcePath)) } + } else { + listOf() } } - catch(iee: IonElementException) { - parseError(iee.location, ParserErrorContext.IonElementError(iee)) + private fun parseDefine(sexp: SexpElement): Statement { + requireArityForTag(sexp, 2) + val args = sexp.tail // Skip tag + val name = args.head.symbolValue + val valueSexp = args.tail.head.asSexp() + + return when (valueSexp.tag) { + "domain" -> parseTypeDomain(name, valueSexp) + "permute_domain" -> parsePermuteDomain(name, valueSexp) + else -> parseError( + valueSexp.head, + ParserErrorContext.UnknownConstructor(valueSexp.tag)) + } } - return TypeUniverse(domains) -} - -private fun parseDefine(sexp: SexpElement): Statement { - requireArityForTag(sexp, 2) - val args = sexp.tail // Skip tag - val name = args.head.symbolValue - val valueSexp = args.tail.head.asSexp() - - return when (valueSexp.tag) { - "domain" -> parseTypeDomain(name, valueSexp) - "permute_domain" -> parsePermuteDomain(name, valueSexp) - else -> parseError( - valueSexp.head, - ParserErrorContext.UnknownConstructor(valueSexp.tag)) + fun parseTransform(sexp: SexpElement): Statement { + requireArityForTag(sexp, 2) + return Transform( + sourceDomainTag = sexp.values[1].symbolValue, + destinationDomainTag = sexp.values[2].symbolValue, + metas = sexp.metas.toSourceLocationMetas() + ) } -} -fun parseTransform(sexp: SexpElement): Statement { - requireArityForTag(sexp, 2) - return Transform( - sourceDomainTag = sexp.values[1].symbolValue, - destinationDomainTag = sexp.values[2].symbolValue, - metas = sexp.metas - ) -} + private fun parseTypeDomain(domainName: String, sexp: SexpElement): TypeDomain { + val args = sexp.tail // Skip tag + //val typesSexps = args.tail -private fun parseTypeDomain(domainName: String, sexp: SexpElement): TypeDomain { - val args = sexp.tail // Skip tag - //val typesSexps = args.tail + val userTypes = args.map { tlv -> + val tlvs = tlv.asSexp() + parseDomainLevelStatement(tlvs) + }.toList() - val userTypes = args.map { tlv -> - val tlvs = tlv.asSexp() - parseDomainLevelStatement(tlvs) - }.toList() - - return TypeDomain( - tag = domainName, - userTypes = userTypes, - metas = sexp.metas) -} + return TypeDomain( + tag = domainName, + userTypes = userTypes, + metas = sexp.metas.toSourceLocationMetas()) + } -private fun parseDomainLevelStatement(tlvs: SexpElement): DataType.UserType { - return when (tlvs.tag) { - "product" -> parseProductBody(tlvs.tail, tlvs.metas) - "record" -> parseRecordBody(tlvs.tail, tlvs.metas) - "sum" -> parseSum(tlvs) - else -> parseError(tlvs.head, ParserErrorContext.InvalidDomainLevelTag(tlvs.tag)) + private fun parseDomainLevelStatement(tlvs: SexpElement): DataType.UserType { + return when (tlvs.tag) { + "product" -> parseProductBody(tlvs.tail, tlvs.metas.toSourceLocationMetas()) + "record" -> parseRecordBody(tlvs.tail, tlvs.metas.toSourceLocationMetas()) + "sum" -> parseSum(tlvs) + else -> parseError(tlvs.head, ParserErrorContext.InvalidDomainLevelTag(tlvs.tag)) + } } -} -private fun parseTypeRefs(values: List): List = - values.map { parseSingleTypeRef(it) } - -// Parses a sum-variant product or record (depending on the syntax used) -private fun parseVariant( - bodyArguments: List, - metas: MetaContainer -): DataType.UserType.Tuple { - val elements = bodyArguments.tail - - // If there are no elements, definitely not a record. - val isRecord = if(elements.none()) { - false - } else { - // if the head element is an s-exp that does not start with `?` or `*` then we're parsing a record - when (val headElem = elements.head) { - is SexpElement -> { - when (headElem.values.head.symbolValue) { - "?", "*" -> false - else -> true + private fun parseTypeRefs(values: List): List = + values.map { parseSingleTypeRef(it) } + + // Parses a sum-variant product or record (depending on the syntax used) + private fun parseVariant( + bodyArguments: List, + metas: MetaContainer + ): DataType.UserType.Tuple { + val elements = bodyArguments.tail + + // If there are no elements, definitely not a record. + val isRecord = if(elements.none()) { + false + } else { + // if the head element is an s-exp that does not start with `?` or `*` then we're parsing a record + when (val headElem = elements.head) { + is SexpElement -> { + when (headElem.values.head.symbolValue) { + "?", "*" -> false + else -> true + } } + is SymbolElement -> false + else -> parseError(elements.head, ParserErrorContext.ExpectedSymbolOrSexp(elements.head.type)) } - is SymbolElement -> false - else -> parseError(elements.head, ParserErrorContext.ExpectedSymbolOrSexp(elements.head.type)) } - } - return when { - isRecord -> { - parseRecordBody(bodyArguments, metas) - } else -> { - parseProductBody(bodyArguments, metas) + return when { + isRecord -> { + parseRecordBody(bodyArguments, metas) + } else -> { + parseProductBody(bodyArguments, metas) + } } } -} - -private fun parseProductBody(bodyArguments: List, metas: MetaContainer): DataType.UserType.Tuple { - val typeName = bodyArguments.head.symbolValue - val namedElements = parseProductElements(bodyArguments.tail) + private fun parseProductBody(bodyArguments: List, metas: MetaContainer): DataType.UserType.Tuple { + val typeName = bodyArguments.head.symbolValue - return DataType.UserType.Tuple(typeName, TupleType.PRODUCT, namedElements, metas) -} - -private fun parseProductElements(values: List): List = - values.map { - val identifier = when(it.annotations.size) { - // TODO: add tests for these errrors - 0 -> parseError(it, ParserErrorContext.MissingElementIdentifierAnnotation) - 1 -> it.annotations.single() - else -> parseError(it, ParserErrorContext.MultipleElementIdentifierAnnotations) - } + val namedElements = parseProductElements(bodyArguments.tail) - NamedElement( - tag = "", // NOTE: tag is not used in the s-expression representation of products! - identifier = identifier, - typeReference = parseSingleTypeRef(it), - metas = it.metas) + return DataType.UserType.Tuple(typeName, TupleType.PRODUCT, namedElements, metas) } -private fun parseRecordBody(bodyArguments: List, metas: MetaContainer): DataType.UserType.Tuple { - val typeName = bodyArguments.head.symbolValue - val namedElements = parseRecordElements(bodyArguments.tail) - return DataType.UserType.Tuple(typeName, TupleType.RECORD, namedElements, metas) -} - -fun parseRecordElements(elementSexps: List): List = - elementSexps.asSequence() - .map { it.asSexp() } - .map { elementSexp -> - if(elementSexp.values.size != 2) { - parseError(elementSexp, ParserErrorContext.InvalidArity(2, elementSexp.size)) - } - val tag = elementSexp.values[0].symbolValue - val identifier = when(elementSexp.annotations.size) { - 0 -> tag - 1 -> elementSexp.annotations.single() - else -> parseError(elementSexp, ParserErrorContext.MultipleElementIdentifierAnnotations) + private fun parseProductElements(values: List): List = + values.map { + val identifier = when(it.annotations.size) { + // TODO: add tests for these errrors + 0 -> parseError(it, ParserErrorContext.MissingElementIdentifierAnnotation) + 1 -> it.annotations.single() + else -> parseError(it, ParserErrorContext.MultipleElementIdentifierAnnotations) } - val typeRef = parseSingleTypeRef(elementSexp.values[1]) + NamedElement( + tag = "", // NOTE: tag is not used in the s-expression representation of products! identifier = identifier, - tag = tag, - typeReference = typeRef, - metas = elementSexp.metas) + typeReference = parseSingleTypeRef(it), + metas = it.metas.toSourceLocationMetas()) } - .toList() -private fun parseSum(sexp: SexpElement): DataType.UserType.Sum { - val args = sexp.tail // Skip tag - val typeName = args.head.symbolValue - - val variants = args.tail.map { - parseSumVariant(it.asSexp()) + private fun parseRecordBody(bodyArguments: List, metas: MetaContainer): DataType.UserType.Tuple { + val typeName = bodyArguments.head.symbolValue + val namedElements = parseRecordElements(bodyArguments.tail) + return DataType.UserType.Tuple(typeName, TupleType.RECORD, namedElements, metas) } - return DataType.UserType.Sum(typeName, variants.toList(), sexp.metas) -} - -private fun parseSumVariant(sexp: SexpElement): DataType.UserType.Tuple { - return parseVariant(sexp.values, sexp.metas) -} - -private fun parseSingleTypeRef(typeRefExp: IonElement): TypeRef { - val metas = typeRefExp.metas - return when (typeRefExp) { - is SymbolElement -> TypeRef(typeRefExp.textValue, Arity.Required, metas) - is SexpElement -> { - when (typeRefExp.tag) { - "?" -> { - requireArityForTag(typeRefExp, 1) - val typeName = typeRefExp.tail.head.symbolValue - TypeRef(typeName, Arity.Optional, metas) + fun parseRecordElements(elementSexps: List): List = + elementSexps.asSequence() + .map { it.asSexp() } + .map { elementSexp -> + if(elementSexp.values.size != 2) { + parseError(elementSexp, ParserErrorContext.InvalidArity(2, elementSexp.size)) } - "*" -> { - requireArityForTag(typeRefExp, IntRange(2, 3)) - val typeName = typeRefExp.tail.head.symbolValue - val arityRange = typeRefExp.tail.tail - val minArity = arityRange.head.longValue - TypeRef(typeName, Arity.Variadic(minArity.toInt()), metas) + val tag = elementSexp.values[0].symbolValue + val identifier = when(elementSexp.annotations.size) { + 0 -> tag + 1 -> elementSexp.annotations.single() + else -> parseError(elementSexp, ParserErrorContext.MultipleElementIdentifierAnnotations) } - else -> parseError(typeRefExp.head, ParserErrorContext.ExpectedTypeReferenceArityTag(typeRefExp.tag)) + val typeRef = parseSingleTypeRef(elementSexp.values[1]) + NamedElement( + identifier = identifier, + tag = tag, + typeReference = typeRef, + metas = elementSexp.metas.toSourceLocationMetas()) } + .toList() + + private fun parseSum(sexp: SexpElement): DataType.UserType.Sum { + val args = sexp.tail // Skip tag + val typeName = args.head.symbolValue + + val variants = args.tail.map { + parseSumVariant(it.asSexp()) } - else -> parseError(typeRefExp, ParserErrorContext.ExpectedSymbolOrSexp(typeRefExp.type)) + + return DataType.UserType.Sum(typeName, variants.toList(), sexp.metas.toSourceLocationMetas()) + } + + private fun parseSumVariant(sexp: SexpElement): DataType.UserType.Tuple { + return parseVariant(sexp.values, sexp.metas.toSourceLocationMetas()) } -} -private fun parsePermuteDomain(domainName: String, sexp: SexpElement): PermutedDomain { - val args = sexp.tail // Skip tag - - val permutingDomain = args.head.symbolValue - val removedTypes = mutableListOf() - val newTypes = mutableListOf() - val permutedSums = mutableListOf() - - val alterSexps = args.tail - alterSexps.map { it.asSexp() }.forEach { alterSexp -> - when(alterSexp.head.symbolValue) { - "with" -> permutedSums.add(parseWithSum(alterSexp)) - "exclude" -> alterSexp.tail.mapTo(removedTypes) { it.symbolValue } - "include" -> alterSexp.tail.mapTo(newTypes) { parseDomainLevelStatement(it.asSexp()) } - else -> parseError(alterSexp, ParserErrorContext.InvalidPermutedDomainTag(alterSexp.head.symbolValue)) + private fun parseSingleTypeRef(typeRefExp: IonElement): TypeRef { + val metas = typeRefExp.metas.toSourceLocationMetas() + return when (typeRefExp) { + is SymbolElement -> TypeRef(typeRefExp.textValue, Arity.Required, metas) + is SexpElement -> { + when (typeRefExp.tag) { + "?" -> { + requireArityForTag(typeRefExp, 1) + val typeName = typeRefExp.tail.head.symbolValue + TypeRef(typeName, Arity.Optional, metas) + } + "*" -> { + requireArityForTag(typeRefExp, IntRange(2, 3)) + val typeName = typeRefExp.tail.head.symbolValue + val arityRange = typeRefExp.tail.tail + val minArity = arityRange.head.longValue + TypeRef(typeName, Arity.Variadic(minArity.toInt()), metas) + } + else -> parseError(typeRefExp.head, ParserErrorContext.ExpectedTypeReferenceArityTag(typeRefExp.tag)) + } + } + else -> parseError(typeRefExp, ParserErrorContext.ExpectedSymbolOrSexp(typeRefExp.type)) } } - return PermutedDomain( - tag = domainName, - permutesDomain = permutingDomain, - excludedTypes = removedTypes, - includedTypes = newTypes, - permutedSums = permutedSums, - metas = sexp.metas) -} + private fun parsePermuteDomain(domainName: String, sexp: SexpElement): PermutedDomain { + val args = sexp.tail // Skip tag + + val permutingDomain = args.head.symbolValue + val removedTypes = mutableListOf() + val newTypes = mutableListOf() + val permutedSums = mutableListOf() + + val alterSexps = args.tail + alterSexps.map { it.asSexp() }.forEach { alterSexp -> + when(alterSexp.head.symbolValue) { + "with" -> permutedSums.add(parseWithSum(alterSexp)) + "exclude" -> alterSexp.tail.mapTo(removedTypes) { it.symbolValue } + "include" -> alterSexp.tail.mapTo(newTypes) { parseDomainLevelStatement(it.asSexp()) } + else -> parseError(alterSexp, ParserErrorContext.InvalidPermutedDomainTag(alterSexp.head.symbolValue)) + } + } -private fun parseWithSum(sexp: SexpElement): PermutedSum { - val args = sexp.tail // Skip tag + return PermutedDomain( + tag = domainName, + permutesDomain = permutingDomain, + excludedTypes = removedTypes, + includedTypes = newTypes, + permutedSums = permutedSums, + metas = sexp.metas.toSourceLocationMetas()) + } - val nameOfAlteredSum = args.head.symbolValue - val removedVariants = mutableListOf() - val addedVariants = mutableListOf() + private fun parseWithSum(sexp: SexpElement): PermutedSum { + val args = sexp.tail // Skip tag - args.tail.forEach { alterationValue -> - val alterationSexp = alterationValue.asSexp() - when (val alterationTag = alterationSexp.tag) { - "exclude" -> alterationSexp.tail.mapTo(removedVariants) { it.symbolValue } - "include" -> alterationSexp.tail.mapTo(addedVariants) { parseSumVariant(it.asSexp()) } - else -> parseError(alterationSexp, ParserErrorContext.InvalidWithSumTag(alterationTag)) + val nameOfAlteredSum = args.head.symbolValue + val removedVariants = mutableListOf() + val addedVariants = mutableListOf() + + args.tail.forEach { alterationValue -> + val alterationSexp = alterationValue.asSexp() + when (val alterationTag = alterationSexp.tag) { + "exclude" -> alterationSexp.tail.mapTo(removedVariants) { it.symbolValue } + "include" -> alterationSexp.tail.mapTo(addedVariants) { parseSumVariant(it.asSexp()) } + else -> parseError(alterationSexp, ParserErrorContext.InvalidWithSumTag(alterationTag)) + } } - } - return PermutedSum(nameOfAlteredSum, removedVariants, addedVariants, sexp.metas) -} + return PermutedSum(nameOfAlteredSum, removedVariants, addedVariants, sexp.metas.toSourceLocationMetas()) + } -private fun requireArityForTag(sexp: SexpElement, arity: Int) { - // Note: arity does not include the tag! - val argCount = sexp.values.size - 1 - if(argCount != arity) { - parseError(sexp, ParserErrorContext.InvalidArityForTag(IntRange(arity, arity), sexp.head.symbolValue, argCount)) + private fun requireArityForTag(sexp: SexpElement, arity: Int) { + // Note: arity does not include the tag! + val argCount = sexp.values.size - 1 + if(argCount != arity) { + parseError(sexp, ParserErrorContext.InvalidArityForTag(IntRange(arity, arity), sexp.head.symbolValue, argCount)) + } } -} -private fun requireArityForTag(sexp: SexpElement, arityRange: IntRange) { - // Note: arity does not include the tag! - val argCount = sexp.values.size - 1 - if(argCount !in arityRange) { - parseError(sexp, ParserErrorContext.InvalidArityForTag(arityRange, sexp.head.symbolValue, argCount)) + private fun requireArityForTag(sexp: SexpElement, arityRange: IntRange) { + // Note: arity does not include the tag! + val argCount = sexp.values.size - 1 + if(argCount !in arityRange) { + parseError(sexp, ParserErrorContext.InvalidArityForTag(arityRange, sexp.head.symbolValue, argCount)) + } } + } + diff --git a/pig/src/org/partiql/pig/errors/PigError.kt b/pig/src/org/partiql/pig/errors/PigError.kt index 8a96c1d..fdc775c 100644 --- a/pig/src/org/partiql/pig/errors/PigError.kt +++ b/pig/src/org/partiql/pig/errors/PigError.kt @@ -15,8 +15,7 @@ package org.partiql.pig.errors -import com.amazon.ionelement.api.IonLocation -import com.amazon.ionelement.api.locationToString +import org.partiql.pig.domain.parser.SourceLocation /** * [ErrorContext] instances provide information about an error message which can later be used @@ -29,7 +28,7 @@ interface ErrorContext { val message: String } -data class PigError(val location: IonLocation?, val context: ErrorContext) { - override fun toString(): String = "${locationToString(location)}: ${context.message}" +data class PigError(val location: SourceLocation?, val context: ErrorContext) { + override fun toString(): String = "${location?.toString() ?: ""}: ${context.message}" } diff --git a/pig/src/org/partiql/pig/main.kt b/pig/src/org/partiql/pig/main.kt index b0f6c9a..0a68584 100644 --- a/pig/src/org/partiql/pig/main.kt +++ b/pig/src/org/partiql/pig/main.kt @@ -21,7 +21,7 @@ import org.partiql.pig.cmdline.CommandLineParser import org.partiql.pig.cmdline.TargetLanguage import org.partiql.pig.errors.PigException import org.partiql.pig.domain.model.TypeUniverse -import org.partiql.pig.domain.parser.parseTypeUniverse +import org.partiql.pig.domain.parser.parseTypeUniverseFile import org.partiql.pig.generator.custom.applyCustomTemplate import org.partiql.pig.generator.html.applyHtmlTemplate import org.partiql.pig.generator.kotlin.applyKotlinTemplate @@ -65,9 +65,7 @@ fun generateCode(command: Command.Generate) { progress("output file : ${command.outputFile}") progress("parsing the universe...") - val typeUniverse: TypeUniverse = FileInputStream(command.typeUniverseFile).use { inputStream -> - IonReaderBuilder.standard().build(inputStream).use { ionReader -> parseTypeUniverse(ionReader) } - } + val typeUniverse: TypeUniverse = parseTypeUniverseFile(command.typeUniverseFile) progress("permuting domains...") diff --git a/pig/test-domains/domains/circular_universe_c.ion b/pig/test-domains/domains/circular_universe_c.ion new file mode 100644 index 0000000..3a3ec73 --- /dev/null +++ b/pig/test-domains/domains/circular_universe_c.ion @@ -0,0 +1,4 @@ + +(define domain_c (domain (product foo))) +// make a circular reference +(import "circular_universe_d.ion") diff --git a/pig/test-domains/domains/circular_universe_d.ion b/pig/test-domains/domains/circular_universe_d.ion new file mode 100644 index 0000000..298d7c0 --- /dev/null +++ b/pig/test-domains/domains/circular_universe_d.ion @@ -0,0 +1,4 @@ + +(define domain_d (domain (product foo))) +// make a circular reference +(import "circular_universe_c.ion") diff --git a/pig/test-domains/domains/import_b.ion b/pig/test-domains/domains/import_b.ion new file mode 100644 index 0000000..84d5782 --- /dev/null +++ b/pig/test-domains/domains/import_b.ion @@ -0,0 +1,4 @@ + +// import file with a relative path +(import "../other/universe_b.ion") + diff --git a/pig/test-domains/domains/universe_a.ion b/pig/test-domains/domains/universe_a.ion new file mode 100644 index 0000000..847e08b --- /dev/null +++ b/pig/test-domains/domains/universe_a.ion @@ -0,0 +1,3 @@ + +(define domain_a (domain (product foo))) +(import "import_b.ion") diff --git a/pig/test-domains/main.ion b/pig/test-domains/main.ion new file mode 100644 index 0000000..e1fea21 --- /dev/null +++ b/pig/test-domains/main.ion @@ -0,0 +1,4 @@ + +(import "domains/universe_a.ion") +(import "domains/circular_universe_c.ion") + diff --git a/pig/test-domains/other/universe_b.ion b/pig/test-domains/other/universe_b.ion new file mode 100644 index 0000000..a8487bd --- /dev/null +++ b/pig/test-domains/other/universe_b.ion @@ -0,0 +1,3 @@ + +(define domain_b (domain (product foo))) + diff --git a/pig/test/org/partiql/pig/cmdline/CommandLineParserTests.kt b/pig/test/org/partiql/pig/cmdline/CommandLineParserTests.kt index 1bb6362..142f032 100644 --- a/pig/test/org/partiql/pig/cmdline/CommandLineParserTests.kt +++ b/pig/test/org/partiql/pig/cmdline/CommandLineParserTests.kt @@ -67,12 +67,12 @@ class CommandLineParserTests { //////////////////////////////////////////////////////// // long parameter names TestCase( - Command.Generate(File("input.ion"), File("output.kt"), TargetLanguage.Kotlin("some.package")), + Command.Generate("input.ion", "output.kt", TargetLanguage.Kotlin("some.package")), "--universe=input.ion", "--target=kotlin", "--output=output.kt", "--namespace=some.package"), // short parameter names TestCase( - Command.Generate(File("input.ion"), File("output.kt"), TargetLanguage.Kotlin("some.package")), + Command.Generate("input.ion", "output.kt", TargetLanguage.Kotlin("some.package")), "-u=input.ion", "-t=kotlin", "-o=output.kt", "-n=some.package"), // missing the --namespace argument @@ -85,12 +85,12 @@ class CommandLineParserTests { //////////////////////////////////////////////////////// // long parameter names TestCase( - Command.Generate(File("input.ion"), File("output.html"), TargetLanguage.Html), + Command.Generate("input.ion", "output.html", TargetLanguage.Html), "--universe=input.ion", "--target=html", "--output=output.html"), // short parameter names TestCase( - Command.Generate(File("input.ion"), File("output.html"), TargetLanguage.Html), + Command.Generate("input.ion", "output.html", TargetLanguage.Html), "-u=input.ion", "-target=html", "--output=output.html"), //////////////////////////////////////////////////////// @@ -98,12 +98,12 @@ class CommandLineParserTests { //////////////////////////////////////////////////////// // long parameter names TestCase( - Command.Generate(File("input.ion"), File("output.txt"), TargetLanguage.Custom(File("template.ftl"))), + Command.Generate("input.ion", "output.txt", TargetLanguage.Custom(File("template.ftl"))), "--universe=input.ion", "--target=custom", "--output=output.txt", "--template=template.ftl"), // short parameter names TestCase( - Command.Generate(File("input.ion"), File("output.txt"), TargetLanguage.Custom(File("template.ftl"))), + Command.Generate("input.ion", "output.txt", TargetLanguage.Custom(File("template.ftl"))), "-u=input.ion", "-t=custom", "-o=output.txt", "-e=template.ftl"), // missing the --template argument diff --git a/pig/test/org/partiql/pig/domain/PermuteDomainTests.kt b/pig/test/org/partiql/pig/domain/PermuteDomainTests.kt index e4c34ac..8c46729 100644 --- a/pig/test/org/partiql/pig/domain/PermuteDomainTests.kt +++ b/pig/test/org/partiql/pig/domain/PermuteDomainTests.kt @@ -15,14 +15,13 @@ package org.partiql.pig.domain -import com.amazon.ion.system.IonReaderBuilder import org.junit.jupiter.api.Assertions.assertEquals import org.junit.jupiter.api.Assertions.assertTrue import org.junit.jupiter.api.Test import org.partiql.pig.domain.model.Arity import org.partiql.pig.domain.model.DataType import org.partiql.pig.domain.model.TypeUniverse -import org.partiql.pig.domain.parser.parseTypeUniverse +import org.partiql.pig.util.parseTypeUniverseInString class PermuteDomainTests { /** @@ -59,7 +58,7 @@ class PermuteDomainTests { (e b::symbol))))) """ - val td: TypeUniverse = IonReaderBuilder.standard().build(typeUniverseWithExtensions).use { parseTypeUniverse(it) } + val td: TypeUniverse = parseTypeUniverseInString(typeUniverseWithExtensions) val concretes = td.computeTypeDomains() diff --git a/pig/test/org/partiql/pig/domain/TypeDomainParserErrorsTest.kt b/pig/test/org/partiql/pig/domain/TypeDomainParserErrorsTest.kt index 0bb3135..9bcc5c2 100644 --- a/pig/test/org/partiql/pig/domain/TypeDomainParserErrorsTest.kt +++ b/pig/test/org/partiql/pig/domain/TypeDomainParserErrorsTest.kt @@ -23,9 +23,9 @@ import org.junit.jupiter.api.assertThrows import org.junit.jupiter.params.ParameterizedTest import org.junit.jupiter.params.provider.MethodSource import org.partiql.pig.domain.parser.ParserErrorContext -import org.partiql.pig.domain.parser.parseTypeUniverse import org.partiql.pig.errors.PigError import org.partiql.pig.errors.PigException +import org.partiql.pig.util.parseTypeUniverseInString class TypeDomainParserErrorsTest { @@ -35,7 +35,7 @@ class TypeDomainParserErrorsTest { @MethodSource("parametersForErrorsTest") fun errorsTest(tc: TestCase) { val ex = assertThrows { - val oops = parseTypeUniverse(tc.typeUniverseText) + val oops = parseTypeUniverseInString(tc.typeUniverseText) println("this was erroneously parsed: ${oops.toIonElement()}") } assertEquals(tc.expectedError, ex.error) diff --git a/pig/test/org/partiql/pig/domain/TypeDomainParserImportTests.kt b/pig/test/org/partiql/pig/domain/TypeDomainParserImportTests.kt new file mode 100644 index 0000000..308fb20 --- /dev/null +++ b/pig/test/org/partiql/pig/domain/TypeDomainParserImportTests.kt @@ -0,0 +1,40 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + */ + +package org.partiql.pig.domain + +import org.junit.jupiter.api.Assertions +import org.junit.jupiter.api.Test +import org.partiql.pig.domain.model.TypeDomain +import org.partiql.pig.domain.parser.parseTypeUniverseFile +import kotlin.test.assertTrue + +class TypeDomainParserImportTests { + + @Test + fun testImport() { + val universe = parseTypeUniverseFile("test-domains/main.ion") + println(universe.toIonElement()) + + val allDomains = universe.statements.filterIsInstance() + + // If 4 domains are loaded correctly, then we deal with relative paths and circular references correctly. + Assertions.assertEquals(4, allDomains.size) + assertTrue(allDomains.any { it.tag == "domain_a" }) + assertTrue(allDomains.any { it.tag == "domain_b" }) + assertTrue(allDomains.any { it.tag == "domain_c" }) + assertTrue(allDomains.any { it.tag == "domain_d" }) + } +} diff --git a/pig/test/org/partiql/pig/domain/TypeDomainParserTests.kt b/pig/test/org/partiql/pig/domain/TypeDomainParserTests.kt index 3ae4863..1b79e4a 100644 --- a/pig/test/org/partiql/pig/domain/TypeDomainParserTests.kt +++ b/pig/test/org/partiql/pig/domain/TypeDomainParserTests.kt @@ -15,7 +15,6 @@ package org.partiql.pig.domain -import com.amazon.ion.system.IonReaderBuilder import com.amazon.ionelement.api.IonElementLoaderOptions import com.amazon.ionelement.api.createIonElementLoader import com.amazon.ionelement.api.ionSexpOf @@ -23,7 +22,9 @@ import com.amazon.ionelement.api.ionSymbol import org.junit.jupiter.api.Assertions.assertEquals import org.junit.jupiter.api.Test import org.junit.jupiter.api.assertDoesNotThrow -import org.partiql.pig.domain.parser.parseTypeUniverse +import org.partiql.pig.domain.model.TypeDomain +import org.partiql.pig.util.parseTypeUniverseInString +import kotlin.test.assertTrue class TypeDomainParserTests { private val loader = createIonElementLoader(IonElementLoaderOptions(includeLocationMeta = true)) @@ -36,6 +37,7 @@ class TypeDomainParserTests { (product foo a::string b::(* int 2)) (product bar a::bat b::(? baz) c::(* blargh 10)))) """) + @Test fun testTransform() = runTestCase("(transform domain_a domain_b)") @@ -88,10 +90,9 @@ class TypeDomainParserTests { val expected = assertDoesNotThrow("loading the expected type universe") { loader.loadSingleElement(tc) } + val parsed = assertDoesNotThrow("parsing type universe") { - IonReaderBuilder.standard().build(tc).use { - parseTypeUniverse(it) - } + parseTypeUniverseInString(tc) } assertEquals( diff --git a/pig/test/org/partiql/pig/domain/TypeDomainSemanticCheckerTests.kt b/pig/test/org/partiql/pig/domain/TypeDomainSemanticCheckerTests.kt index 2aeb03b..99d98ae 100644 --- a/pig/test/org/partiql/pig/domain/TypeDomainSemanticCheckerTests.kt +++ b/pig/test/org/partiql/pig/domain/TypeDomainSemanticCheckerTests.kt @@ -20,9 +20,9 @@ import org.junit.jupiter.api.assertThrows import org.junit.jupiter.params.ParameterizedTest import org.junit.jupiter.params.provider.MethodSource import org.partiql.pig.domain.model.SemanticErrorContext -import org.partiql.pig.domain.parser.parseTypeUniverse import org.partiql.pig.errors.PigError import org.partiql.pig.errors.PigException +import org.partiql.pig.util.parseTypeUniverseInString class TypeDomainSemanticCheckerTests { @@ -43,7 +43,7 @@ class TypeDomainSemanticCheckerTests { fun nameErrorsTest2(tc: TestCase) = runTest(tc) private fun runTest(tc: TestCase) { - val u = parseTypeUniverse(tc.typeUniverseText) + val u = parseTypeUniverseInString(tc.typeUniverseText) val ex = assertThrows { u.computeTypeDomains() } assertEquals(tc.expectedError, ex.error) } diff --git a/pig/test/org/partiql/pig/domain/Util.kt b/pig/test/org/partiql/pig/domain/Util.kt index 6bff621..711d04e 100644 --- a/pig/test/org/partiql/pig/domain/Util.kt +++ b/pig/test/org/partiql/pig/domain/Util.kt @@ -29,11 +29,12 @@ import org.partiql.pig.domain.model.Transform import org.partiql.pig.domain.model.TupleType import org.partiql.pig.domain.model.TypeDomain import org.partiql.pig.domain.model.TypeUniverse +import org.partiql.pig.domain.parser.SourceLocation import org.partiql.pig.errors.ErrorContext import org.partiql.pig.errors.PigError fun makeErr(line: Int, col: Int, errorContext: ErrorContext) = - PigError(IonTextLocation(line.toLong(), col.toLong()), errorContext) + PigError(SourceLocation("root.ion", IonTextLocation(line.toLong(), col.toLong())), errorContext) fun makeErr(errorContext: ErrorContext) = PigError(null, errorContext) diff --git a/pig/test/org/partiql/pig/util/ParseHelpers.kt b/pig/test/org/partiql/pig/util/ParseHelpers.kt new file mode 100644 index 0000000..55a0dbe --- /dev/null +++ b/pig/test/org/partiql/pig/util/ParseHelpers.kt @@ -0,0 +1,50 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + */ + +package org.partiql.pig.util + +import org.partiql.pig.domain.model.TypeUniverse +import org.partiql.pig.domain.parser.InputSource +import org.partiql.pig.domain.parser.Parser +import java.io.ByteArrayInputStream +import java.io.FileNotFoundException +import java.io.InputStream + +/** + * For testing purposes, parses the type universe specified in [topUnvierseText]. + * + * [includes] is a map keyed by "fake" filename that will be used instead of a real-file system for looking + * up the content of imported files. [includes] must not contain any filename by the name of "root.ion", which + * is the name given to the type universe specified in [topUnvierseText]. + */ +fun parseTypeUniverseInString(topUnvierseText: String, includes: Map = emptyMap()): TypeUniverse { + assert(!includes.containsKey("root.ion")) + val allIncludes = mapOf("root.ion" to topUnvierseText) + includes + val parser = Parser(StringSource(allIncludes)) + return parser.parseTypeUniverse("root.ion") +} + +class StringSource(val sources: Map) : InputSource { + override fun openStream(sourceName: String): InputStream { + val text: String = sources[sourceName] ?: throw FileNotFoundException("$sourceName does not exist") + + return ByteArrayInputStream(text.toByteArray(Charsets.UTF_8)) + } + + override fun getCanonicalName(sourceName: String): String { + TODO("not implemented") + } +} + From 214aa8aaa561cd3de95aaabb758c44edf17ef7af Mon Sep 17 00:00:00 2001 From: David Lurton Date: Fri, 14 May 2021 01:08:56 -0700 Subject: [PATCH 02/22] Remove stale TODO --- README.md | 2 +- pig/src/org/partiql/pig/domain/parser/TypeDomainParser.kt | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index db42771..f1cf688 100644 --- a/README.md +++ b/README.md @@ -412,7 +412,7 @@ It is possible to split type universe definitions among multiple files: (import "b.ion") ``` -The resulting type universe will contain all type domains from both `a.ion` and `b.ion`, `root.ion` may also +The resulting type universe will contain all type domains from both `a.ion` and `b.ion`. `root.ion` may also define additional type domains. The primary purpose of this is to be able to permute domains defined in another file. diff --git a/pig/src/org/partiql/pig/domain/parser/TypeDomainParser.kt b/pig/src/org/partiql/pig/domain/parser/TypeDomainParser.kt index b4c78da..5dca718 100644 --- a/pig/src/org/partiql/pig/domain/parser/TypeDomainParser.kt +++ b/pig/src/org/partiql/pig/domain/parser/TypeDomainParser.kt @@ -107,8 +107,7 @@ private class Parser( private fun parseImport(sexp: SexpElement): List { requireArityForTag(sexp, 1) val relativePath = sexp.tail.single().asString().textValue - // TODO: need to report proper error to user if file does not exist. - + val workingDirectory = File(this.qualifedSourceStack.peek()).parentFile val qualifiedSourcePath = File(workingDirectory, relativePath).canonicalPath return if(!parseHistory.contains(qualifiedSourcePath)) { From e3f773e0952924b250e509439f7cdcb90cc5c276 Mon Sep 17 00:00:00 2001 From: David Lurton Date: Fri, 14 May 2021 01:26:54 -0700 Subject: [PATCH 03/22] Fix a few build errors --- pig/src/org/partiql/pig/domain/parser/InputSource.kt | 2 +- pig/src/org/partiql/pig/domain/parser/TypeDomainParser.kt | 4 ++-- pig/test/org/partiql/pig/domain/TypeDomainParserTests.kt | 2 -- pig/test/org/partiql/pig/util/ParseHelpers.kt | 5 ++--- 4 files changed, 5 insertions(+), 8 deletions(-) diff --git a/pig/src/org/partiql/pig/domain/parser/InputSource.kt b/pig/src/org/partiql/pig/domain/parser/InputSource.kt index 01055fd..f6963c4 100644 --- a/pig/src/org/partiql/pig/domain/parser/InputSource.kt +++ b/pig/src/org/partiql/pig/domain/parser/InputSource.kt @@ -39,7 +39,7 @@ internal interface InputSource { } internal val FILE_SYSTEM_SOURCE = object : InputSource { - override fun openStream(qualifiedSource: String) = FileInputStream(qualifiedSource) + override fun openStream(sourceName: String) = FileInputStream(sourceName) override fun getCanonicalName(sourceName: String): String = File(sourceName).canonicalFile.toString() } \ No newline at end of file diff --git a/pig/src/org/partiql/pig/domain/parser/TypeDomainParser.kt b/pig/src/org/partiql/pig/domain/parser/TypeDomainParser.kt index 5dca718..dfcd308 100644 --- a/pig/src/org/partiql/pig/domain/parser/TypeDomainParser.kt +++ b/pig/src/org/partiql/pig/domain/parser/TypeDomainParser.kt @@ -55,7 +55,7 @@ fun parseTypeUniverseFile(path: String): TypeUniverse { return parser.parseTypeUniverse(path) } -private class Parser( +internal class Parser( val inputSource: InputSource ) { /** This contains every file the parser has "seen" and is used detect and prevent import cycles. */ @@ -107,7 +107,7 @@ private class Parser( private fun parseImport(sexp: SexpElement): List { requireArityForTag(sexp, 1) val relativePath = sexp.tail.single().asString().textValue - + val workingDirectory = File(this.qualifedSourceStack.peek()).parentFile val qualifiedSourcePath = File(workingDirectory, relativePath).canonicalPath return if(!parseHistory.contains(qualifiedSourcePath)) { diff --git a/pig/test/org/partiql/pig/domain/TypeDomainParserTests.kt b/pig/test/org/partiql/pig/domain/TypeDomainParserTests.kt index 1b79e4a..5756103 100644 --- a/pig/test/org/partiql/pig/domain/TypeDomainParserTests.kt +++ b/pig/test/org/partiql/pig/domain/TypeDomainParserTests.kt @@ -22,9 +22,7 @@ import com.amazon.ionelement.api.ionSymbol import org.junit.jupiter.api.Assertions.assertEquals import org.junit.jupiter.api.Test import org.junit.jupiter.api.assertDoesNotThrow -import org.partiql.pig.domain.model.TypeDomain import org.partiql.pig.util.parseTypeUniverseInString -import kotlin.test.assertTrue class TypeDomainParserTests { private val loader = createIonElementLoader(IonElementLoaderOptions(includeLocationMeta = true)) diff --git a/pig/test/org/partiql/pig/util/ParseHelpers.kt b/pig/test/org/partiql/pig/util/ParseHelpers.kt index 55a0dbe..aa994ea 100644 --- a/pig/test/org/partiql/pig/util/ParseHelpers.kt +++ b/pig/test/org/partiql/pig/util/ParseHelpers.kt @@ -36,6 +36,7 @@ fun parseTypeUniverseInString(topUnvierseText: String, includes: Map. Used only for testing. */ class StringSource(val sources: Map) : InputSource { override fun openStream(sourceName: String): InputStream { val text: String = sources[sourceName] ?: throw FileNotFoundException("$sourceName does not exist") @@ -43,8 +44,6 @@ class StringSource(val sources: Map) : InputSource { return ByteArrayInputStream(text.toByteArray(Charsets.UTF_8)) } - override fun getCanonicalName(sourceName: String): String { - TODO("not implemented") - } + override fun getCanonicalName(sourceName: String): String = sourceName } From 2a15e486af8d4c16c9df711c30b3c8603801d357 Mon Sep 17 00:00:00 2001 From: David Lurton Date: Tue, 1 Jun 2021 15:35:12 -0700 Subject: [PATCH 04/22] import -> include_file --- pig/src/org/partiql/pig/domain/parser/TypeDomainParser.kt | 4 ++-- pig/test-domains/domains/circular_universe_c.ion | 2 +- pig/test-domains/domains/circular_universe_d.ion | 2 +- pig/test-domains/domains/import_b.ion | 4 ---- pig/test-domains/domains/include_b.ion | 3 +++ pig/test-domains/domains/universe_a.ion | 2 +- pig/test-domains/main.ion | 5 ++--- .../org/partiql/pig/domain/TypeDomainParserErrorsTest.kt | 7 ++++++- 8 files changed, 16 insertions(+), 13 deletions(-) delete mode 100644 pig/test-domains/domains/import_b.ion create mode 100644 pig/test-domains/domains/include_b.ion diff --git a/pig/src/org/partiql/pig/domain/parser/TypeDomainParser.kt b/pig/src/org/partiql/pig/domain/parser/TypeDomainParser.kt index dfcd308..e10bb21 100644 --- a/pig/src/org/partiql/pig/domain/parser/TypeDomainParser.kt +++ b/pig/src/org/partiql/pig/domain/parser/TypeDomainParser.kt @@ -77,7 +77,7 @@ internal class Parser( when (topLevelSexp.tag) { "define" -> listOf(parseDefine(topLevelSexp)) "transform" -> listOf(parseTransform(topLevelSexp)) - "import" -> parseImport(topLevelSexp) + "include_file" -> parseIncludeFile(topLevelSexp) else -> parseError( topLevelSexp.head, ParserErrorContext.InvalidTopLevelTag(topLevelSexp.tag)) @@ -104,7 +104,7 @@ internal class Parser( metaContainerOf(SOURCE_LOCATION_META_TAG to it.toSourceLocation()) } ?: emptyMetaContainer() - private fun parseImport(sexp: SexpElement): List { + private fun parseIncludeFile(sexp: SexpElement): List { requireArityForTag(sexp, 1) val relativePath = sexp.tail.single().asString().textValue diff --git a/pig/test-domains/domains/circular_universe_c.ion b/pig/test-domains/domains/circular_universe_c.ion index 3a3ec73..9e797ba 100644 --- a/pig/test-domains/domains/circular_universe_c.ion +++ b/pig/test-domains/domains/circular_universe_c.ion @@ -1,4 +1,4 @@ (define domain_c (domain (product foo))) // make a circular reference -(import "circular_universe_d.ion") +(include_file "circular_universe_d.ion") diff --git a/pig/test-domains/domains/circular_universe_d.ion b/pig/test-domains/domains/circular_universe_d.ion index 298d7c0..faa4f73 100644 --- a/pig/test-domains/domains/circular_universe_d.ion +++ b/pig/test-domains/domains/circular_universe_d.ion @@ -1,4 +1,4 @@ (define domain_d (domain (product foo))) // make a circular reference -(import "circular_universe_c.ion") +(include_file "circular_universe_c.ion") diff --git a/pig/test-domains/domains/import_b.ion b/pig/test-domains/domains/import_b.ion deleted file mode 100644 index 84d5782..0000000 --- a/pig/test-domains/domains/import_b.ion +++ /dev/null @@ -1,4 +0,0 @@ - -// import file with a relative path -(import "../other/universe_b.ion") - diff --git a/pig/test-domains/domains/include_b.ion b/pig/test-domains/domains/include_b.ion new file mode 100644 index 0000000..e461e47 --- /dev/null +++ b/pig/test-domains/domains/include_b.ion @@ -0,0 +1,3 @@ + +// include file with a relative path +(include_file "../other/universe_b.ion") diff --git a/pig/test-domains/domains/universe_a.ion b/pig/test-domains/domains/universe_a.ion index 847e08b..922ce36 100644 --- a/pig/test-domains/domains/universe_a.ion +++ b/pig/test-domains/domains/universe_a.ion @@ -1,3 +1,3 @@ (define domain_a (domain (product foo))) -(import "import_b.ion") +(include_file "include_b.ion") diff --git a/pig/test-domains/main.ion b/pig/test-domains/main.ion index e1fea21..3bf6d0f 100644 --- a/pig/test-domains/main.ion +++ b/pig/test-domains/main.ion @@ -1,4 +1,3 @@ -(import "domains/universe_a.ion") -(import "domains/circular_universe_c.ion") - +(include_file "domains/universe_a.ion") +(include_file "domains/circular_universe_c.ion") diff --git a/pig/test/org/partiql/pig/domain/TypeDomainParserErrorsTest.kt b/pig/test/org/partiql/pig/domain/TypeDomainParserErrorsTest.kt index 9bcc5c2..d993041 100644 --- a/pig/test/org/partiql/pig/domain/TypeDomainParserErrorsTest.kt +++ b/pig/test/org/partiql/pig/domain/TypeDomainParserErrorsTest.kt @@ -105,7 +105,12 @@ class TypeDomainParserErrorsTest { TestCase( // Covers second place in parser this can be thrown "(define huh (domain (product huh x::int y::42)))", - makeErr(1, 41, ParserErrorContext.ExpectedSymbolOrSexp(ElementType.INT))) + makeErr(1, 41, ParserErrorContext.ExpectedSymbolOrSexp(ElementType.INT))), + + TestCase( + "(include_file \"some-non-existing-file.ion\")", + makeErr(1, 1, ParserErrorContext.CouldNotFindImportedTypeUniverse( + File("some-non-existing-file.ion").canonicalPath))) ) } } From dd483e47ee54124ba261f2eb5dffc2fb04cd0b4f Mon Sep 17 00:00:00 2001 From: David Lurton Date: Sun, 30 May 2021 12:13:45 -0700 Subject: [PATCH 05/22] apply feedback wip --- README.md | 11 ++++++-- .../test/org/partiql/pig/tests/Aye.kt | 19 ++----------- pig-tests/test/org/partiql/pig/tests/Bee.kt | 18 +++++++++++++ .../pig/domain/model/SemanticErrorContext.kt | 4 +-- .../pig/domain/parser/ParserErrorContext.kt | 7 ++--- .../domains/circular_universe_d.ion | 1 + pig/test-domains/main.ion | 27 +++++++++++++++++++ .../pig/domain/TypeDomainParserErrorsTest.kt | 1 + .../pig/domain/TypeDomainParserImportTests.kt | 3 +-- 9 files changed, 62 insertions(+), 29 deletions(-) rename pig/src/org/partiql/pig/domain/parser/ImportSourceOpener.kt => pig-tests/test/org/partiql/pig/tests/Aye.kt (59%) create mode 100644 pig-tests/test/org/partiql/pig/tests/Bee.kt diff --git a/README.md b/README.md index f1cf688..d72211a 100644 --- a/README.md +++ b/README.md @@ -174,9 +174,9 @@ assertEquals(onePlusOne, anotherOnePlusOne) // Top level type_universe ::= ... definition ::= '(' 'define' symbol ')' -stmt ::= | | +stmt ::= | | -import ::= `(import | @@ -416,6 +416,13 @@ The resulting type universe will contain all type domains from both `a.ion` and define additional type domains. The primary purpose of this is to be able to permute domains defined in another file. +Paths are relative to the directory containing the current type universe: + +``` +(import "subdir/partiql.ion") +(import "../other/some-universe.ion") +``` + #### Using PIG In Your Project diff --git a/pig/src/org/partiql/pig/domain/parser/ImportSourceOpener.kt b/pig-tests/test/org/partiql/pig/tests/Aye.kt similarity index 59% rename from pig/src/org/partiql/pig/domain/parser/ImportSourceOpener.kt rename to pig-tests/test/org/partiql/pig/tests/Aye.kt index 6f6e434..b5edcdb 100644 --- a/pig/src/org/partiql/pig/domain/parser/ImportSourceOpener.kt +++ b/pig-tests/test/org/partiql/pig/tests/Aye.kt @@ -13,22 +13,7 @@ * permissions and limitations under the License. */ -package org.partiql.pig.domain.parser +package org.partiql.pig.tests -import com.amazon.ion.IonReader -import com.amazon.ion.system.IonReaderBuilder -import java.io.Closeable -import java.io.File -import java.io.FileInputStream -import java.nio.file.Path +class Aye(val b: Bee) -//class ImportSource( -// val fullyQualifiedName: String, -// val reader: IonReader -//) : Closeable { -// override fun close() { -// reader.close() -// } -//} - -// TODO: names diff --git a/pig-tests/test/org/partiql/pig/tests/Bee.kt b/pig-tests/test/org/partiql/pig/tests/Bee.kt new file mode 100644 index 0000000..0dd6a47 --- /dev/null +++ b/pig-tests/test/org/partiql/pig/tests/Bee.kt @@ -0,0 +1,18 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + */ + +package org.partiql.pig.tests + +class Bee(val a: Aye) \ No newline at end of file diff --git a/pig/src/org/partiql/pig/domain/model/SemanticErrorContext.kt b/pig/src/org/partiql/pig/domain/model/SemanticErrorContext.kt index 61c9c1e..5cc855a 100644 --- a/pig/src/org/partiql/pig/domain/model/SemanticErrorContext.kt +++ b/pig/src/org/partiql/pig/domain/model/SemanticErrorContext.kt @@ -15,14 +15,12 @@ package org.partiql.pig.domain.model -import com.amazon.ionelement.api.IonLocation import com.amazon.ionelement.api.MetaContainer -import com.amazon.ionelement.api.location import org.partiql.pig.domain.parser.SourceLocation import org.partiql.pig.domain.parser.sourceLocation -import org.partiql.pig.errors.PigException import org.partiql.pig.errors.ErrorContext import org.partiql.pig.errors.PigError +import org.partiql.pig.errors.PigException /** * Encapsulates all error context information in an easily testable way. diff --git a/pig/src/org/partiql/pig/domain/parser/ParserErrorContext.kt b/pig/src/org/partiql/pig/domain/parser/ParserErrorContext.kt index 495c736..9ca0277 100644 --- a/pig/src/org/partiql/pig/domain/parser/ParserErrorContext.kt +++ b/pig/src/org/partiql/pig/domain/parser/ParserErrorContext.kt @@ -15,14 +15,11 @@ package org.partiql.pig.domain.parser -import com.amazon.ionelement.api.IonElement -import com.amazon.ionelement.api.IonLocation -import com.amazon.ionelement.api.location import com.amazon.ionelement.api.ElementType import com.amazon.ionelement.api.IonElementException -import org.partiql.pig.errors.PigException import org.partiql.pig.errors.ErrorContext import org.partiql.pig.errors.PigError +import org.partiql.pig.errors.PigException /** * Variants of [ParserErrorContext] contain details about various parse errors that can be encountered @@ -33,7 +30,7 @@ import org.partiql.pig.errors.PigError sealed class ParserErrorContext(val msgFormatter: () -> String): ErrorContext { override val message: String get() = msgFormatter() - /** Indicates that an []IonElectrolyteException] was thrown during parsing of a type universe. */ + /** Indicates that an [IonElectrolyteException] was thrown during parsing of a type universe. */ data class IonElementError(val ex: IonElementException) : ParserErrorContext({ ex.message!! }) { // This is for unit tests... we don't include IonElectrolyteException here since it doesn't implement diff --git a/pig/test-domains/domains/circular_universe_d.ion b/pig/test-domains/domains/circular_universe_d.ion index faa4f73..f0da743 100644 --- a/pig/test-domains/domains/circular_universe_d.ion +++ b/pig/test-domains/domains/circular_universe_d.ion @@ -1,4 +1,5 @@ (define domain_d (domain (product foo))) + // make a circular reference (include_file "circular_universe_c.ion") diff --git a/pig/test-domains/main.ion b/pig/test-domains/main.ion index 3bf6d0f..0c6944a 100644 --- a/pig/test-domains/main.ion +++ b/pig/test-domains/main.ion @@ -1,3 +1,30 @@ +/* + +This file is used to test the `(import )` statement. + +Two primary things are tested: + +- Cyclic imports are tolerated without exception. +- Relative paths, which are always relative to the directory containing the type universe currently being parsed. + +. +├── domains +│ ├── circular_universe_c.ion imports circular_universe_d.ion +│ ├── circular_universe_d.ion imports circular_universe_c.ion +│ ├── import_b.ion imports ../other/universe_b +│ └── universe_a.ion imports import_b.ion +├── main.ion imports domains/universe_a.ion and domains/circular_universe_c.ion +└── other + └── universe_b.ion imports nothing + + +By parsing, main.ion (this file), we test that each of the other files is imported (directly or indirectly). Each +.ion file (except for "import_b.ion") defines a type domain, for a total of 4 type domains. The test is successful +if main.ion can be imported without error and results in a single universe with all 4 type domains. + +*/ + (include_file "domains/universe_a.ion") (include_file "domains/circular_universe_c.ion") + diff --git a/pig/test/org/partiql/pig/domain/TypeDomainParserErrorsTest.kt b/pig/test/org/partiql/pig/domain/TypeDomainParserErrorsTest.kt index d993041..9b56bd2 100644 --- a/pig/test/org/partiql/pig/domain/TypeDomainParserErrorsTest.kt +++ b/pig/test/org/partiql/pig/domain/TypeDomainParserErrorsTest.kt @@ -26,6 +26,7 @@ import org.partiql.pig.domain.parser.ParserErrorContext import org.partiql.pig.errors.PigError import org.partiql.pig.errors.PigException import org.partiql.pig.util.parseTypeUniverseInString +import java.io.File class TypeDomainParserErrorsTest { diff --git a/pig/test/org/partiql/pig/domain/TypeDomainParserImportTests.kt b/pig/test/org/partiql/pig/domain/TypeDomainParserImportTests.kt index 308fb20..268e15f 100644 --- a/pig/test/org/partiql/pig/domain/TypeDomainParserImportTests.kt +++ b/pig/test/org/partiql/pig/domain/TypeDomainParserImportTests.kt @@ -26,11 +26,10 @@ class TypeDomainParserImportTests { @Test fun testImport() { val universe = parseTypeUniverseFile("test-domains/main.ion") - println(universe.toIonElement()) - val allDomains = universe.statements.filterIsInstance() // If 4 domains are loaded correctly, then we deal with relative paths and circular references correctly. + // see documentation at top of test-domains/main.ion Assertions.assertEquals(4, allDomains.size) assertTrue(allDomains.any { it.tag == "domain_a" }) assertTrue(allDomains.any { it.tag == "domain_b" }) From 497328edaaefa2316b8c189c405da135c939ddc2 Mon Sep 17 00:00:00 2001 From: David Lurton Date: Sun, 13 Jun 2021 15:49:46 -0700 Subject: [PATCH 06/22] Apply feedback from Almann, Abhilash and Alan --- README.md | 27 ++++++---- pig/src/org/partiql/pig/cmdline/Command.kt | 22 +++++++- .../partiql/pig/cmdline/CommandLineParser.kt | 2 +- .../partiql/pig/domain/parser/ImportSource.kt | 47 +++++++++++++++++ .../partiql/pig/domain/parser/InputSource.kt | 45 ----------------- .../pig/domain/parser/TypeDomainParser.kt | 50 ++++++++++++------- pig/src/org/partiql/pig/main.kt | 10 ++-- pig/test-domains/include-missing-file.ion | 4 ++ .../pig/cmdline/CommandLineParserTests.kt | 12 ++--- .../partiql/pig/domain/PermuteDomainTests.kt | 4 +- .../pig/domain/TypeDomainParserErrorsTest.kt | 4 +- ...kt => TypeDomainParserIncludeFileTests.kt} | 31 ++++++++++-- .../pig/domain/TypeDomainParserTests.kt | 4 +- .../domain/TypeDomainSemanticCheckerTests.kt | 4 +- pig/test/org/partiql/pig/domain/Util.kt | 6 +-- pig/test/org/partiql/pig/util/ParseHelpers.kt | 37 ++++++++------ 16 files changed, 191 insertions(+), 118 deletions(-) create mode 100644 pig/src/org/partiql/pig/domain/parser/ImportSource.kt delete mode 100644 pig/src/org/partiql/pig/domain/parser/InputSource.kt create mode 100644 pig/test-domains/include-missing-file.ion rename pig/test/org/partiql/pig/domain/{TypeDomainParserImportTests.kt => TypeDomainParserIncludeFileTests.kt} (57%) diff --git a/README.md b/README.md index d72211a..6927b26 100644 --- a/README.md +++ b/README.md @@ -174,9 +174,9 @@ assertEquals(onePlusOne, anotherOnePlusOne) // Top level type_universe ::= ... definition ::= '(' 'define' symbol ')' -stmt ::= | | +stmt ::= | | -import ::= `(import )` // Domain domain_definition ::= | @@ -402,27 +402,32 @@ Unlike record elements, product element defintions must include identifiers. (product int_pair first::int second::int) ``` -#### Imports +#### Type Domain Includes It is possible to split type universe definitions among multiple files: ``` // root.ion: -(import "a.ion") -(import "b.ion") +(include_file "a.ion") +(include_file "b.ion") ``` -The resulting type universe will contain all type domains from both `a.ion` and `b.ion`. `root.ion` may also -define additional type domains. The primary purpose of this is to be able to permute domains defined in another -file. +The resulting type universe will contain all type domains from both `a.ion` and `b.ion`. `root.ion` may also define +additional type domains. The primary purpose of this is to be able to permute domains defined in another file. -Paths are relative to the directory containing the current type universe: +`include_file` is recursive, so any type domains in any files included by `a.ion` and `b.ion` will also be present. Any +attempt to include a file that has already been seen will be ignored. + +Paths are always relative to the directory containing the current file. The working directory at the time the `pig` is +command is irrelevant. ``` -(import "subdir/partiql.ion") -(import "../other/some-universe.ion") +(include_file "subdir/partiql.ion") +(include_file "../other/some-universe.ion") ``` +Lastly, `include_file` can only be used at the top-level within a `.ion` file. It is not allowed within a +`(domain ...)`clause. #### Using PIG In Your Project diff --git a/pig/src/org/partiql/pig/cmdline/Command.kt b/pig/src/org/partiql/pig/cmdline/Command.kt index c565d7e..28ab799 100644 --- a/pig/src/org/partiql/pig/cmdline/Command.kt +++ b/pig/src/org/partiql/pig/cmdline/Command.kt @@ -17,8 +17,26 @@ package org.partiql.pig.cmdline import java.io.File +/** Represents command line options specified by the user. */ sealed class Command { + + /** The `--help` command. */ object ShowHelp : Command() + + /** + * Returned by [CommandLineParser] when the user has specified invalid command-line arguments + * + * - [message]: an error message to be displayed to the user. + */ data class InvalidCommandLineArguments(val message: String) : Command() - data class Generate(val typeUniverseFile: String, val outputFile: String, val target: TargetLanguage) : Command() -} \ No newline at end of file + + /** + * Contains the details of a *valid* command-line specified by the user. + * + * - [typeUniverseFilePath]: the path to the type universe file. + * - [outputFilePath]: the path to the output file. (This makes the assumption that there is only one output file.) + * - [target]: specifies the target language and any other parameters unique to the target language. + */ + data class Generate(val typeUniverseFilePath: File, val outputFilePath: File, val target: TargetLanguage) : Command() +} + diff --git a/pig/src/org/partiql/pig/cmdline/CommandLineParser.kt b/pig/src/org/partiql/pig/cmdline/CommandLineParser.kt index 232305f..e8f11c7 100644 --- a/pig/src/org/partiql/pig/cmdline/CommandLineParser.kt +++ b/pig/src/org/partiql/pig/cmdline/CommandLineParser.kt @@ -145,7 +145,7 @@ class CommandLineParser { LanguageTargetType.CUSTOM -> TargetLanguage.Custom(optSet.valueOf(templateOpt)) } - Command.Generate(typeUniverseFile.toString(), outputFile.toString(), target) + Command.Generate(typeUniverseFile, outputFile, target) } } } catch(ex: OptionException) { diff --git a/pig/src/org/partiql/pig/domain/parser/ImportSource.kt b/pig/src/org/partiql/pig/domain/parser/ImportSource.kt new file mode 100644 index 0000000..2664ada --- /dev/null +++ b/pig/src/org/partiql/pig/domain/parser/ImportSource.kt @@ -0,0 +1,47 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + */ + +package org.partiql.pig.domain.parser + +import java.io.File +import java.io.FileInputStream +import java.io.InputStream + + +/** + * Provides an abstraction for file-system related functions used when importing files. + * + * This exists to allow code requiring these functions to be tested with a fake implementation. Many unit tests for + * example put test type universes in strings within the test itself. Those tests can't create `FileInputStream` + * instances or convert relative paths to absolute. Hence, a "fakeable" interface to these two operations is needed. + */ +internal interface ImportSource { + /** + * Opens an input stream for the given source name. + * + * The exact meaning of [resolvedSourceName] is implementation-defined. In the case of a file system + * implementation it is the path to a file, either relative to the working directory or absolute. + * + * The caller must be sure to close the [InputStream]. + */ + fun openInputStream(resolvedSourceName: String): InputStream +} + + +/** A "real" file system used by production code. */ +internal val FILE_IMPORT_SOURCE = object : ImportSource { + override fun openInputStream(resolvedName: String) = FileInputStream(resolvedName) +} + diff --git a/pig/src/org/partiql/pig/domain/parser/InputSource.kt b/pig/src/org/partiql/pig/domain/parser/InputSource.kt deleted file mode 100644 index f6963c4..0000000 --- a/pig/src/org/partiql/pig/domain/parser/InputSource.kt +++ /dev/null @@ -1,45 +0,0 @@ -/* - * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. - * - * Licensed under the Apache License, Version 2.0 (the "License"). - * You may not use this file except in compliance with the License. - * A copy of the License is located at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * or in the "license" file accompanying this file. This file is distributed - * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either - * express or implied. See the License for the specific language governing - * permissions and limitations under the License. - */ - -package org.partiql.pig.domain.parser - -import java.io.File -import java.io.FileInputStream -import java.io.InputStream - -/** - * Provides an abstraction for file-system related functions used when importing files. - */ -internal interface InputSource { - /** - * Opens an input stream for the given source name. - * - * The [sourceName] is implementation-defined. In the case of a file system implementaiton it is the path to a - * file, either relative to the working directory or absolute. - */ - fun openStream(sourceName: String): InputStream - - /** - * Returns the "canonical name" of the given source. In the case of a file system, this converts the relative - * path to an absolute path. - */ - fun getCanonicalName(sourceName: String): String -} - -internal val FILE_SYSTEM_SOURCE = object : InputSource { - override fun openStream(sourceName: String) = FileInputStream(sourceName) - - override fun getCanonicalName(sourceName: String): String = File(sourceName).canonicalFile.toString() -} \ No newline at end of file diff --git a/pig/src/org/partiql/pig/domain/parser/TypeDomainParser.kt b/pig/src/org/partiql/pig/domain/parser/TypeDomainParser.kt index e10bb21..c5b918a 100644 --- a/pig/src/org/partiql/pig/domain/parser/TypeDomainParser.kt +++ b/pig/src/org/partiql/pig/domain/parser/TypeDomainParser.kt @@ -49,27 +49,42 @@ import java.io.FileNotFoundException import java.io.InputStream import java.util.Stack - -fun parseTypeUniverseFile(path: String): TypeUniverse { - val parser = Parser(FILE_SYSTEM_SOURCE) - return parser.parseTypeUniverse(path) +/** + * Parses the type universe specified in [universeFilePath]. + * + * Any files included with `include_file` are relative to the directory containing [universeFilePath]. + */ +internal fun parseTypeUniverseFile(universeFilePath: String): TypeUniverse { + val parser = TypeUniverseParser(FILE_IMPORT_SOURCE) + return parser.parseTypeUniverse(universeFilePath) } -internal class Parser( - val inputSource: InputSource +internal class TypeUniverseParser( + private val imporSource: ImportSource + //private val imoprtResolver: ImportResolver ) { - /** This contains every file the parser has "seen" and is used detect and prevent import cycles. */ + /** + * This contains every file the parser has "seen" and is used to detect and ignore any `(include_file ...)` + * statement for a file that the parser has previously seen. Serves two purposes: 1) sidesteps issues arising from + * files being included more than once and 2) prevents problems with cyclic includes. + */ private val parseHistory = HashSet() - private val qualifedSourceStack = Stack().apply { push(".") } + + /** + * The top of this stack is always the path to the input file currently being parsed. It's pushed and popped + * before and after files included with `(include_file ...)` are parsed. + */ + private val inputFilePathStack = Stack() /** Parses a type universe in the specified [IonReader]. */ fun parseTypeUniverse(source: String): TypeUniverse { val elementLoader = createIonElementLoader(IonElementLoaderOptions(includeLocationMeta = true)) - val qualifiedSource = inputSource.getCanonicalName(source) - return inputSource.openStream(qualifiedSource).use { inputStream: InputStream -> + //val qualifiedSource = imoprtResolver.resolve(source) + val qualifiedSource = File(source).absolutePath + return imporSource.openInputStream(qualifiedSource).use { inputStream: InputStream -> IonReaderBuilder.standard().build(inputStream).use { reader: IonReader -> this.parseHistory.add(qualifiedSource) - this.qualifedSourceStack.push(qualifiedSource) + this.inputFilePathStack.push(qualifiedSource) val domains = try { val topLevelElements = elementLoader.loadAllElements(reader) topLevelElements.flatMap { topLevelValue -> @@ -86,19 +101,19 @@ internal class Parser( } catch (iee: IonElementException) { parseError(iee.location?.toSourceLocation(), ParserErrorContext.IonElementError(iee)) } - this.qualifedSourceStack.pop() + this.inputFilePathStack.pop() TypeUniverse(domains) } } } - fun parseError(blame: IonElement, context: ErrorContext): Nothing { + private fun parseError(blame: IonElement, context: ErrorContext): Nothing { val loc = blame.metas.location?.toSourceLocation() parseError(loc, context) } - private fun IonLocation.toSourceLocation() = SourceLocation(qualifedSourceStack.peek(), this) + private fun IonLocation.toSourceLocation() = SourceLocation(inputFilePathStack.peek(), this) private fun MetaContainer.toSourceLocationMetas(): MetaContainer = this.location?.let { metaContainerOf(SOURCE_LOCATION_META_TAG to it.toSourceLocation()) @@ -108,7 +123,7 @@ internal class Parser( requireArityForTag(sexp, 1) val relativePath = sexp.tail.single().asString().textValue - val workingDirectory = File(this.qualifedSourceStack.peek()).parentFile + val workingDirectory = File(this.inputFilePathStack.peek()).parentFile val qualifiedSourcePath = File(workingDirectory, relativePath).canonicalPath return if(!parseHistory.contains(qualifiedSourcePath)) { try { @@ -137,7 +152,7 @@ internal class Parser( } } - fun parseTransform(sexp: SexpElement): Statement { + private fun parseTransform(sexp: SexpElement): Statement { requireArityForTag(sexp, 2) return Transform( sourceDomainTag = sexp.values[1].symbolValue, @@ -170,9 +185,6 @@ internal class Parser( } } - private fun parseTypeRefs(values: List): List = - values.map { parseSingleTypeRef(it) } - // Parses a sum-variant product or record (depending on the syntax used) private fun parseVariant( bodyArguments: List, diff --git a/pig/src/org/partiql/pig/main.kt b/pig/src/org/partiql/pig/main.kt index 0a68584..f8fe1ed 100644 --- a/pig/src/org/partiql/pig/main.kt +++ b/pig/src/org/partiql/pig/main.kt @@ -15,7 +15,6 @@ package org.partiql.pig -import com.amazon.ion.system.IonReaderBuilder import org.partiql.pig.cmdline.Command import org.partiql.pig.cmdline.CommandLineParser import org.partiql.pig.cmdline.TargetLanguage @@ -26,7 +25,6 @@ import org.partiql.pig.generator.custom.applyCustomTemplate import org.partiql.pig.generator.html.applyHtmlTemplate import org.partiql.pig.generator.kotlin.applyKotlinTemplate import org.partiql.pig.generator.kotlin.convertToKTypeUniverse -import java.io.FileInputStream import java.io.PrintWriter import kotlin.system.exitProcess @@ -61,15 +59,15 @@ fun main(args: Array) { * having to `exec` pig as a separate process. */ fun generateCode(command: Command.Generate) { - progress("universe file: ${command.typeUniverseFile}") - progress("output file : ${command.outputFile}") + progress("universe file: ${command.typeUniverseFilePath}") + progress("output file : ${command.outputFilePath}") progress("parsing the universe...") - val typeUniverse: TypeUniverse = parseTypeUniverseFile(command.typeUniverseFile) + val typeUniverse: TypeUniverse = parseTypeUniverseFile(command.typeUniverseFilePath.canonicalPath) progress("permuting domains...") - PrintWriter(command.outputFile).use { printWriter -> + PrintWriter(command.outputFilePath).use { printWriter -> when (command.target) { is TargetLanguage.Kotlin -> { progress("applying Kotlin pre-processing") diff --git a/pig/test-domains/include-missing-file.ion b/pig/test-domains/include-missing-file.ion new file mode 100644 index 0000000..d8ce906 --- /dev/null +++ b/pig/test-domains/include-missing-file.ion @@ -0,0 +1,4 @@ + +// This file attempts to include a missing file. + +(include_file "domains/doesnotexist.ion") diff --git a/pig/test/org/partiql/pig/cmdline/CommandLineParserTests.kt b/pig/test/org/partiql/pig/cmdline/CommandLineParserTests.kt index 142f032..1bb6362 100644 --- a/pig/test/org/partiql/pig/cmdline/CommandLineParserTests.kt +++ b/pig/test/org/partiql/pig/cmdline/CommandLineParserTests.kt @@ -67,12 +67,12 @@ class CommandLineParserTests { //////////////////////////////////////////////////////// // long parameter names TestCase( - Command.Generate("input.ion", "output.kt", TargetLanguage.Kotlin("some.package")), + Command.Generate(File("input.ion"), File("output.kt"), TargetLanguage.Kotlin("some.package")), "--universe=input.ion", "--target=kotlin", "--output=output.kt", "--namespace=some.package"), // short parameter names TestCase( - Command.Generate("input.ion", "output.kt", TargetLanguage.Kotlin("some.package")), + Command.Generate(File("input.ion"), File("output.kt"), TargetLanguage.Kotlin("some.package")), "-u=input.ion", "-t=kotlin", "-o=output.kt", "-n=some.package"), // missing the --namespace argument @@ -85,12 +85,12 @@ class CommandLineParserTests { //////////////////////////////////////////////////////// // long parameter names TestCase( - Command.Generate("input.ion", "output.html", TargetLanguage.Html), + Command.Generate(File("input.ion"), File("output.html"), TargetLanguage.Html), "--universe=input.ion", "--target=html", "--output=output.html"), // short parameter names TestCase( - Command.Generate("input.ion", "output.html", TargetLanguage.Html), + Command.Generate(File("input.ion"), File("output.html"), TargetLanguage.Html), "-u=input.ion", "-target=html", "--output=output.html"), //////////////////////////////////////////////////////// @@ -98,12 +98,12 @@ class CommandLineParserTests { //////////////////////////////////////////////////////// // long parameter names TestCase( - Command.Generate("input.ion", "output.txt", TargetLanguage.Custom(File("template.ftl"))), + Command.Generate(File("input.ion"), File("output.txt"), TargetLanguage.Custom(File("template.ftl"))), "--universe=input.ion", "--target=custom", "--output=output.txt", "--template=template.ftl"), // short parameter names TestCase( - Command.Generate("input.ion", "output.txt", TargetLanguage.Custom(File("template.ftl"))), + Command.Generate(File("input.ion"), File("output.txt"), TargetLanguage.Custom(File("template.ftl"))), "-u=input.ion", "-t=custom", "-o=output.txt", "-e=template.ftl"), // missing the --template argument diff --git a/pig/test/org/partiql/pig/domain/PermuteDomainTests.kt b/pig/test/org/partiql/pig/domain/PermuteDomainTests.kt index 8c46729..a3570f6 100644 --- a/pig/test/org/partiql/pig/domain/PermuteDomainTests.kt +++ b/pig/test/org/partiql/pig/domain/PermuteDomainTests.kt @@ -21,7 +21,7 @@ import org.junit.jupiter.api.Test import org.partiql.pig.domain.model.Arity import org.partiql.pig.domain.model.DataType import org.partiql.pig.domain.model.TypeUniverse -import org.partiql.pig.util.parseTypeUniverseInString +import org.partiql.pig.util.parseTypeUniverseString class PermuteDomainTests { /** @@ -58,7 +58,7 @@ class PermuteDomainTests { (e b::symbol))))) """ - val td: TypeUniverse = parseTypeUniverseInString(typeUniverseWithExtensions) + val td: TypeUniverse = parseTypeUniverseString(typeUniverseWithExtensions) val concretes = td.computeTypeDomains() diff --git a/pig/test/org/partiql/pig/domain/TypeDomainParserErrorsTest.kt b/pig/test/org/partiql/pig/domain/TypeDomainParserErrorsTest.kt index 9b56bd2..965440e 100644 --- a/pig/test/org/partiql/pig/domain/TypeDomainParserErrorsTest.kt +++ b/pig/test/org/partiql/pig/domain/TypeDomainParserErrorsTest.kt @@ -25,7 +25,7 @@ import org.junit.jupiter.params.provider.MethodSource import org.partiql.pig.domain.parser.ParserErrorContext import org.partiql.pig.errors.PigError import org.partiql.pig.errors.PigException -import org.partiql.pig.util.parseTypeUniverseInString +import org.partiql.pig.util.parseTypeUniverseString import java.io.File class TypeDomainParserErrorsTest { @@ -36,7 +36,7 @@ class TypeDomainParserErrorsTest { @MethodSource("parametersForErrorsTest") fun errorsTest(tc: TestCase) { val ex = assertThrows { - val oops = parseTypeUniverseInString(tc.typeUniverseText) + val oops = parseTypeUniverseString(tc.typeUniverseText) println("this was erroneously parsed: ${oops.toIonElement()}") } assertEquals(tc.expectedError, ex.error) diff --git a/pig/test/org/partiql/pig/domain/TypeDomainParserImportTests.kt b/pig/test/org/partiql/pig/domain/TypeDomainParserIncludeFileTests.kt similarity index 57% rename from pig/test/org/partiql/pig/domain/TypeDomainParserImportTests.kt rename to pig/test/org/partiql/pig/domain/TypeDomainParserIncludeFileTests.kt index 268e15f..9b1f517 100644 --- a/pig/test/org/partiql/pig/domain/TypeDomainParserImportTests.kt +++ b/pig/test/org/partiql/pig/domain/TypeDomainParserIncludeFileTests.kt @@ -15,25 +15,50 @@ package org.partiql.pig.domain +import com.amazon.ionelement.api.IonTextLocation import org.junit.jupiter.api.Assertions +import org.junit.jupiter.api.Assertions.assertEquals import org.junit.jupiter.api.Test +import org.junit.jupiter.api.assertThrows +import org.partiql.pig.domain.model.SemanticErrorContext import org.partiql.pig.domain.model.TypeDomain +import org.partiql.pig.domain.parser.ParserErrorContext +import org.partiql.pig.domain.parser.SourceLocation import org.partiql.pig.domain.parser.parseTypeUniverseFile +import org.partiql.pig.errors.PigError +import org.partiql.pig.errors.PigException +import java.io.File import kotlin.test.assertTrue -class TypeDomainParserImportTests { +class TypeDomainParserIncludeFileTests { @Test - fun testImport() { + fun `include happy path`() { val universe = parseTypeUniverseFile("test-domains/main.ion") val allDomains = universe.statements.filterIsInstance() // If 4 domains are loaded correctly, then we deal with relative paths and circular references correctly. // see documentation at top of test-domains/main.ion - Assertions.assertEquals(4, allDomains.size) + assertEquals(4, allDomains.size) assertTrue(allDomains.any { it.tag == "domain_a" }) assertTrue(allDomains.any { it.tag == "domain_b" }) assertTrue(allDomains.any { it.tag == "domain_c" }) assertTrue(allDomains.any { it.tag == "domain_d" }) } + + @Test + fun `missing file`() { + val universeFilePath = "test-domains/include-missing-file.ion" + val ex = assertThrows { + parseTypeUniverseFile(universeFilePath) + } + + assertEquals( + PigError( + SourceLocation(File(universeFilePath).canonicalPath, IonTextLocation(4L, 1L)), + ParserErrorContext.CouldNotFindImportedTypeUniverse(File("test-domains/domains/doesnotexist.ion").canonicalPath) + ), + ex.error + ) + } } diff --git a/pig/test/org/partiql/pig/domain/TypeDomainParserTests.kt b/pig/test/org/partiql/pig/domain/TypeDomainParserTests.kt index 5756103..3a5842f 100644 --- a/pig/test/org/partiql/pig/domain/TypeDomainParserTests.kt +++ b/pig/test/org/partiql/pig/domain/TypeDomainParserTests.kt @@ -22,7 +22,7 @@ import com.amazon.ionelement.api.ionSymbol import org.junit.jupiter.api.Assertions.assertEquals import org.junit.jupiter.api.Test import org.junit.jupiter.api.assertDoesNotThrow -import org.partiql.pig.util.parseTypeUniverseInString +import org.partiql.pig.util.parseTypeUniverseString class TypeDomainParserTests { private val loader = createIonElementLoader(IonElementLoaderOptions(includeLocationMeta = true)) @@ -90,7 +90,7 @@ class TypeDomainParserTests { } val parsed = assertDoesNotThrow("parsing type universe") { - parseTypeUniverseInString(tc) + parseTypeUniverseString(tc) } assertEquals( diff --git a/pig/test/org/partiql/pig/domain/TypeDomainSemanticCheckerTests.kt b/pig/test/org/partiql/pig/domain/TypeDomainSemanticCheckerTests.kt index 99d98ae..a939887 100644 --- a/pig/test/org/partiql/pig/domain/TypeDomainSemanticCheckerTests.kt +++ b/pig/test/org/partiql/pig/domain/TypeDomainSemanticCheckerTests.kt @@ -22,7 +22,7 @@ import org.junit.jupiter.params.provider.MethodSource import org.partiql.pig.domain.model.SemanticErrorContext import org.partiql.pig.errors.PigError import org.partiql.pig.errors.PigException -import org.partiql.pig.util.parseTypeUniverseInString +import org.partiql.pig.util.parseTypeUniverseString class TypeDomainSemanticCheckerTests { @@ -43,7 +43,7 @@ class TypeDomainSemanticCheckerTests { fun nameErrorsTest2(tc: TestCase) = runTest(tc) private fun runTest(tc: TestCase) { - val u = parseTypeUniverseInString(tc.typeUniverseText) + val u = parseTypeUniverseString(tc.typeUniverseText) val ex = assertThrows { u.computeTypeDomains() } assertEquals(tc.expectedError, ex.error) } diff --git a/pig/test/org/partiql/pig/domain/Util.kt b/pig/test/org/partiql/pig/domain/Util.kt index 711d04e..de92f10 100644 --- a/pig/test/org/partiql/pig/domain/Util.kt +++ b/pig/test/org/partiql/pig/domain/Util.kt @@ -32,19 +32,19 @@ import org.partiql.pig.domain.model.TypeUniverse import org.partiql.pig.domain.parser.SourceLocation import org.partiql.pig.errors.ErrorContext import org.partiql.pig.errors.PigError +import org.partiql.pig.util.FAKE_ROOT_FILE +import java.io.File fun makeErr(line: Int, col: Int, errorContext: ErrorContext) = - PigError(SourceLocation("root.ion", IonTextLocation(line.toLong(), col.toLong())), errorContext) + PigError(SourceLocation(File(FAKE_ROOT_FILE).canonicalPath, IonTextLocation(line.toLong(), col.toLong())), errorContext) fun makeErr(errorContext: ErrorContext) = PigError(null, errorContext) - /* * The [toIonElement] functions below generate an s-expression representation of a [TypeUniverse]. */ - fun TypeUniverse.toIonElement(): IonElement = ionSexpOf( ionSymbol("universe"), diff --git a/pig/test/org/partiql/pig/util/ParseHelpers.kt b/pig/test/org/partiql/pig/util/ParseHelpers.kt index aa994ea..901091f 100644 --- a/pig/test/org/partiql/pig/util/ParseHelpers.kt +++ b/pig/test/org/partiql/pig/util/ParseHelpers.kt @@ -16,34 +16,43 @@ package org.partiql.pig.util import org.partiql.pig.domain.model.TypeUniverse -import org.partiql.pig.domain.parser.InputSource -import org.partiql.pig.domain.parser.Parser +import org.partiql.pig.domain.parser.ImportSource +import org.partiql.pig.domain.parser.TypeUniverseParser import java.io.ByteArrayInputStream +import java.io.File import java.io.FileNotFoundException import java.io.InputStream /** - * For testing purposes, parses the type universe specified in [topUnvierseText]. + * The name of the "fake" root file used by unit tests. + */ +internal const val FAKE_ROOT_FILE = "root.ion" + +/** + * For unit tests only. Parses the type universe specified in [topUnvierseText]. + * + * * * [includes] is a map keyed by "fake" filename that will be used instead of a real-file system for looking * up the content of imported files. [includes] must not contain any filename by the name of "root.ion", which * is the name given to the type universe specified in [topUnvierseText]. */ -fun parseTypeUniverseInString(topUnvierseText: String, includes: Map = emptyMap()): TypeUniverse { - assert(!includes.containsKey("root.ion")) - val allIncludes = mapOf("root.ion" to topUnvierseText) + includes - val parser = Parser(StringSource(allIncludes)) - return parser.parseTypeUniverse("root.ion") +fun parseTypeUniverseString(topUnvierseText: String, includes: Map = emptyMap()): TypeUniverse { + assert(!includes.containsKey(FAKE_ROOT_FILE)) + + val allIncludes = (mapOf(FAKE_ROOT_FILE to topUnvierseText) + includes).map { + File(it.key).canonicalPath to it.value + }.toMap() + + val parser = TypeUniverseParser(FakeImportSource(allIncludes)) + return parser.parseTypeUniverse(FAKE_ROOT_FILE) } /** A minimal faux file system backed by a Map. Used only for testing. */ -class StringSource(val sources: Map) : InputSource { - override fun openStream(sourceName: String): InputStream { - val text: String = sources[sourceName] ?: throw FileNotFoundException("$sourceName does not exist") +class FakeImportSource(val sources: Map) : ImportSource { + override fun openInputStream(resolvedName: String): InputStream { + val text: String = sources[resolvedName] ?: throw FileNotFoundException("$resolvedName does not exist") return ByteArrayInputStream(text.toByteArray(Charsets.UTF_8)) } - - override fun getCanonicalName(sourceName: String): String = sourceName } - From 650dedf51e29cb856ead390e209aca4581240a20 Mon Sep 17 00:00:00 2001 From: David Lurton Date: Sun, 13 Jun 2021 16:06:18 -0700 Subject: [PATCH 07/22] More changes I should have included earlier. --- README.md | 2 +- pig-tests/test/org/partiql/pig/tests/Aye.kt | 19 -------- pig-tests/test/org/partiql/pig/tests/Bee.kt | 18 ------- .../partiql/pig/domain/parser/ImportSource.kt | 47 ------------------- .../pig/domain/parser/InputStreamSource.kt | 35 ++++++++++++++ .../pig/domain/parser/ParserErrorContext.kt | 2 +- .../pig/domain/parser/SourceLocation.kt | 2 - .../pig/domain/parser/TypeDomainParser.kt | 14 +++--- pig/test/org/partiql/pig/util/ParseHelpers.kt | 6 +-- 9 files changed, 47 insertions(+), 98 deletions(-) delete mode 100644 pig-tests/test/org/partiql/pig/tests/Aye.kt delete mode 100644 pig-tests/test/org/partiql/pig/tests/Bee.kt delete mode 100644 pig/src/org/partiql/pig/domain/parser/ImportSource.kt create mode 100644 pig/src/org/partiql/pig/domain/parser/InputStreamSource.kt diff --git a/README.md b/README.md index 6927b26..efce88c 100644 --- a/README.md +++ b/README.md @@ -404,7 +404,7 @@ Unlike record elements, product element defintions must include identifiers. #### Type Domain Includes -It is possible to split type universe definitions among multiple files: +It is possible to split type universes among multiple files: ``` // root.ion: diff --git a/pig-tests/test/org/partiql/pig/tests/Aye.kt b/pig-tests/test/org/partiql/pig/tests/Aye.kt deleted file mode 100644 index b5edcdb..0000000 --- a/pig-tests/test/org/partiql/pig/tests/Aye.kt +++ /dev/null @@ -1,19 +0,0 @@ -/* - * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. - * - * Licensed under the Apache License, Version 2.0 (the "License"). - * You may not use this file except in compliance with the License. - * A copy of the License is located at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * or in the "license" file accompanying this file. This file is distributed - * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either - * express or implied. See the License for the specific language governing - * permissions and limitations under the License. - */ - -package org.partiql.pig.tests - -class Aye(val b: Bee) - diff --git a/pig-tests/test/org/partiql/pig/tests/Bee.kt b/pig-tests/test/org/partiql/pig/tests/Bee.kt deleted file mode 100644 index 0dd6a47..0000000 --- a/pig-tests/test/org/partiql/pig/tests/Bee.kt +++ /dev/null @@ -1,18 +0,0 @@ -/* - * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. - * - * Licensed under the Apache License, Version 2.0 (the "License"). - * You may not use this file except in compliance with the License. - * A copy of the License is located at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * or in the "license" file accompanying this file. This file is distributed - * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either - * express or implied. See the License for the specific language governing - * permissions and limitations under the License. - */ - -package org.partiql.pig.tests - -class Bee(val a: Aye) \ No newline at end of file diff --git a/pig/src/org/partiql/pig/domain/parser/ImportSource.kt b/pig/src/org/partiql/pig/domain/parser/ImportSource.kt deleted file mode 100644 index 2664ada..0000000 --- a/pig/src/org/partiql/pig/domain/parser/ImportSource.kt +++ /dev/null @@ -1,47 +0,0 @@ -/* - * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. - * - * Licensed under the Apache License, Version 2.0 (the "License"). - * You may not use this file except in compliance with the License. - * A copy of the License is located at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * or in the "license" file accompanying this file. This file is distributed - * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either - * express or implied. See the License for the specific language governing - * permissions and limitations under the License. - */ - -package org.partiql.pig.domain.parser - -import java.io.File -import java.io.FileInputStream -import java.io.InputStream - - -/** - * Provides an abstraction for file-system related functions used when importing files. - * - * This exists to allow code requiring these functions to be tested with a fake implementation. Many unit tests for - * example put test type universes in strings within the test itself. Those tests can't create `FileInputStream` - * instances or convert relative paths to absolute. Hence, a "fakeable" interface to these two operations is needed. - */ -internal interface ImportSource { - /** - * Opens an input stream for the given source name. - * - * The exact meaning of [resolvedSourceName] is implementation-defined. In the case of a file system - * implementation it is the path to a file, either relative to the working directory or absolute. - * - * The caller must be sure to close the [InputStream]. - */ - fun openInputStream(resolvedSourceName: String): InputStream -} - - -/** A "real" file system used by production code. */ -internal val FILE_IMPORT_SOURCE = object : ImportSource { - override fun openInputStream(resolvedName: String) = FileInputStream(resolvedName) -} - diff --git a/pig/src/org/partiql/pig/domain/parser/InputStreamSource.kt b/pig/src/org/partiql/pig/domain/parser/InputStreamSource.kt new file mode 100644 index 0000000..0299e14 --- /dev/null +++ b/pig/src/org/partiql/pig/domain/parser/InputStreamSource.kt @@ -0,0 +1,35 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + */ + +package org.partiql.pig.domain.parser + +import java.io.FileInputStream +import java.io.InputStream + + +/** + * A simple abstraction for opening [InputStream], allows for type domains to be loaded from a file system or an + * in-memory data structure. The latter is useful for unit testing. + */ +internal interface InputStreamSource { + /** Opens an input stream for the given filename name. */ + fun openInputStream(fileName: String): InputStream +} + +/** An [InputStreamSource] that constructs [FileInputStream] instances. */ +internal val FILE_SYSTEM_INPUT_STREAM_SOURCE = object : InputStreamSource { + override fun openInputStream(fileName: String) = FileInputStream(fileName) +} + diff --git a/pig/src/org/partiql/pig/domain/parser/ParserErrorContext.kt b/pig/src/org/partiql/pig/domain/parser/ParserErrorContext.kt index 9ca0277..92a6595 100644 --- a/pig/src/org/partiql/pig/domain/parser/ParserErrorContext.kt +++ b/pig/src/org/partiql/pig/domain/parser/ParserErrorContext.kt @@ -30,7 +30,7 @@ import org.partiql.pig.errors.PigException sealed class ParserErrorContext(val msgFormatter: () -> String): ErrorContext { override val message: String get() = msgFormatter() - /** Indicates that an [IonElectrolyteException] was thrown during parsing of a type universe. */ + /** Indicates that an [IonElementException] was thrown during parsing of a type universe. */ data class IonElementError(val ex: IonElementException) : ParserErrorContext({ ex.message!! }) { // This is for unit tests... we don't include IonElectrolyteException here since it doesn't implement diff --git a/pig/src/org/partiql/pig/domain/parser/SourceLocation.kt b/pig/src/org/partiql/pig/domain/parser/SourceLocation.kt index fdb307c..45ef8a0 100644 --- a/pig/src/org/partiql/pig/domain/parser/SourceLocation.kt +++ b/pig/src/org/partiql/pig/domain/parser/SourceLocation.kt @@ -19,8 +19,6 @@ import com.amazon.ionelement.api.IonLocation import com.amazon.ionelement.api.MetaContainer /** - * Includes path to a source file and a position within it. - * * Used to construct helpful error messages for the end-user, who will be able to know the file, line & column of * a given error. */ diff --git a/pig/src/org/partiql/pig/domain/parser/TypeDomainParser.kt b/pig/src/org/partiql/pig/domain/parser/TypeDomainParser.kt index c5b918a..330e0f2 100644 --- a/pig/src/org/partiql/pig/domain/parser/TypeDomainParser.kt +++ b/pig/src/org/partiql/pig/domain/parser/TypeDomainParser.kt @@ -55,24 +55,25 @@ import java.util.Stack * Any files included with `include_file` are relative to the directory containing [universeFilePath]. */ internal fun parseTypeUniverseFile(universeFilePath: String): TypeUniverse { - val parser = TypeUniverseParser(FILE_IMPORT_SOURCE) + val parser = TypeUniverseParser(FILE_SYSTEM_INPUT_STREAM_SOURCE) return parser.parseTypeUniverse(universeFilePath) } internal class TypeUniverseParser( - private val imporSource: ImportSource + private val imporSource: InputStreamSource //private val imoprtResolver: ImportResolver ) { /** * This contains every file the parser has "seen" and is used to detect and ignore any `(include_file ...)` - * statement for a file that the parser has previously seen. Serves two purposes: 1) sidesteps issues arising from - * files being included more than once and 2) prevents problems with cyclic includes. + * statement for a file that the parser has previously seen. This sidesteps issues arising from files being + * included more than once, including infinite recursion in the case of cyclic includes. */ private val parseHistory = HashSet() /** - * The top of this stack is always the path to the input file currently being parsed. It's pushed and popped - * before and after files included with `(include_file ...)` are parsed. + * The top of this stack is always the path to the input file currently being parsed. + * + * Kept up to date by the [parseTypeUniverse] function. */ private val inputFilePathStack = Stack() @@ -105,7 +106,6 @@ internal class TypeUniverseParser( TypeUniverse(domains) } } - } private fun parseError(blame: IonElement, context: ErrorContext): Nothing { diff --git a/pig/test/org/partiql/pig/util/ParseHelpers.kt b/pig/test/org/partiql/pig/util/ParseHelpers.kt index 901091f..e57aaba 100644 --- a/pig/test/org/partiql/pig/util/ParseHelpers.kt +++ b/pig/test/org/partiql/pig/util/ParseHelpers.kt @@ -16,7 +16,7 @@ package org.partiql.pig.util import org.partiql.pig.domain.model.TypeUniverse -import org.partiql.pig.domain.parser.ImportSource +import org.partiql.pig.domain.parser.InputStreamSource import org.partiql.pig.domain.parser.TypeUniverseParser import java.io.ByteArrayInputStream import java.io.File @@ -44,12 +44,12 @@ fun parseTypeUniverseString(topUnvierseText: String, includes: Map. Used only for testing. */ -class FakeImportSource(val sources: Map) : ImportSource { +class FakeInputStreamSource(val sources: Map) : InputStreamSource { override fun openInputStream(resolvedName: String): InputStream { val text: String = sources[resolvedName] ?: throw FileNotFoundException("$resolvedName does not exist") From f9d1e6e4fd12ddbe8a2ac8b906e2f8e6d89b97ff Mon Sep 17 00:00:00 2001 From: David Lurton Date: Tue, 15 Jun 2021 16:43:03 -0700 Subject: [PATCH 08/22] Apply Alan's feedback --- README.md | 6 ++--- .../pig/domain/parser/ParserErrorContext.kt | 4 ++-- .../pig/domain/parser/TypeDomainParser.kt | 8 +++---- pig/test-domains/main.ion | 23 ++++++++++--------- .../pig/domain/TypeDomainParserErrorsTest.kt | 2 +- .../TypeDomainParserIncludeFileTests.kt | 4 +--- pig/test/org/partiql/pig/util/ParseHelpers.kt | 10 ++++---- 7 files changed, 27 insertions(+), 30 deletions(-) diff --git a/README.md b/README.md index efce88c..491ad1d 100644 --- a/README.md +++ b/README.md @@ -418,8 +418,8 @@ additional type domains. The primary purpose of this is to be able to permute d `include_file` is recursive, so any type domains in any files included by `a.ion` and `b.ion` will also be present. Any attempt to include a file that has already been seen will be ignored. -Paths are always relative to the directory containing the current file. The working directory at the time the `pig` is -command is irrelevant. +Paths are always relative to the directory containing the current file. The working directory at the time `pig` +is executed is irrelevant. ``` (include_file "subdir/partiql.ion") @@ -427,7 +427,7 @@ command is irrelevant. ``` Lastly, `include_file` can only be used at the top-level within a `.ion` file. It is not allowed within a -`(domain ...)`clause. +`(domain ...)` clause. #### Using PIG In Your Project diff --git a/pig/src/org/partiql/pig/domain/parser/ParserErrorContext.kt b/pig/src/org/partiql/pig/domain/parser/ParserErrorContext.kt index 92a6595..58d9dc8 100644 --- a/pig/src/org/partiql/pig/domain/parser/ParserErrorContext.kt +++ b/pig/src/org/partiql/pig/domain/parser/ParserErrorContext.kt @@ -39,8 +39,8 @@ sealed class ParserErrorContext(val msgFormatter: () -> String): ErrorContext { override fun hashCode(): Int = 0 } - data class CouldNotFindImportedTypeUniverse(val tag: String) - : ParserErrorContext({ "Could not find imported type universe: $tag" }) + data class CouldNotFindIncludedFile(val tag: String) + : ParserErrorContext({ "Included file missing: $tag" }) data class UnknownConstructor(val tag: String) : ParserErrorContext({ "Unknown constructor: '$tag' (expected constructors are 'domain' or 'permute_domain')" }) diff --git a/pig/src/org/partiql/pig/domain/parser/TypeDomainParser.kt b/pig/src/org/partiql/pig/domain/parser/TypeDomainParser.kt index 330e0f2..e9efc60 100644 --- a/pig/src/org/partiql/pig/domain/parser/TypeDomainParser.kt +++ b/pig/src/org/partiql/pig/domain/parser/TypeDomainParser.kt @@ -60,8 +60,7 @@ internal fun parseTypeUniverseFile(universeFilePath: String): TypeUniverse { } internal class TypeUniverseParser( - private val imporSource: InputStreamSource - //private val imoprtResolver: ImportResolver + private val inputStreamSource: InputStreamSource ) { /** * This contains every file the parser has "seen" and is used to detect and ignore any `(include_file ...)` @@ -80,9 +79,8 @@ internal class TypeUniverseParser( /** Parses a type universe in the specified [IonReader]. */ fun parseTypeUniverse(source: String): TypeUniverse { val elementLoader = createIonElementLoader(IonElementLoaderOptions(includeLocationMeta = true)) - //val qualifiedSource = imoprtResolver.resolve(source) val qualifiedSource = File(source).absolutePath - return imporSource.openInputStream(qualifiedSource).use { inputStream: InputStream -> + return inputStreamSource.openInputStream(qualifiedSource).use { inputStream: InputStream -> IonReaderBuilder.standard().build(inputStream).use { reader: IonReader -> this.parseHistory.add(qualifiedSource) this.inputFilePathStack.push(qualifiedSource) @@ -131,7 +129,7 @@ internal class TypeUniverseParser( } catch (e: FileNotFoundException) { parseError( sexp.metas.location?.toSourceLocation(), - ParserErrorContext.CouldNotFindImportedTypeUniverse(qualifiedSourcePath)) + ParserErrorContext.CouldNotFindIncludedFile(qualifiedSourcePath)) } } else { listOf() diff --git a/pig/test-domains/main.ion b/pig/test-domains/main.ion index 0c6944a..ef4df78 100644 --- a/pig/test-domains/main.ion +++ b/pig/test-domains/main.ion @@ -1,26 +1,27 @@ /* -This file is used to test the `(import )` statement. +This file is used to test the `(include_file )` statement. Two primary things are tested: -- Cyclic imports are tolerated without exception. +- Cyclic includes are tolerated without exception. - Relative paths, which are always relative to the directory containing the type universe currently being parsed. . ├── domains -│ ├── circular_universe_c.ion imports circular_universe_d.ion -│ ├── circular_universe_d.ion imports circular_universe_c.ion -│ ├── import_b.ion imports ../other/universe_b -│ └── universe_a.ion imports import_b.ion -├── main.ion imports domains/universe_a.ion and domains/circular_universe_c.ion +│ ├── circular_universe_c.ion includes circular_universe_d.ion +│ ├── circular_universe_d.ion includes circular_universe_c.ion +│ ├── include_b.ion includes ../other/universe_b +│ └── universe_a.ion includes include_b.ion +├── main.ion includes domains/universe_a.ion and domains/circular_universe_c.ion +├── include-missing-file.ion used by error handling tests and not included by main.ion └── other - └── universe_b.ion imports nothing + └── universe_b.ion includes nothing -By parsing, main.ion (this file), we test that each of the other files is imported (directly or indirectly). Each -.ion file (except for "import_b.ion") defines a type domain, for a total of 4 type domains. The test is successful -if main.ion can be imported without error and results in a single universe with all 4 type domains. +By parsing, main.ion (this file), we test that each of the other files is includeed (directly or indirectly). Each +.ion file (except for "include_b.ion") defines a type domain, for a total of 4 type domains. The test is successful +if main.ion can be included without error and results in a single universe with all 4 type domains. */ diff --git a/pig/test/org/partiql/pig/domain/TypeDomainParserErrorsTest.kt b/pig/test/org/partiql/pig/domain/TypeDomainParserErrorsTest.kt index 965440e..15b0f46 100644 --- a/pig/test/org/partiql/pig/domain/TypeDomainParserErrorsTest.kt +++ b/pig/test/org/partiql/pig/domain/TypeDomainParserErrorsTest.kt @@ -110,7 +110,7 @@ class TypeDomainParserErrorsTest { TestCase( "(include_file \"some-non-existing-file.ion\")", - makeErr(1, 1, ParserErrorContext.CouldNotFindImportedTypeUniverse( + makeErr(1, 1, ParserErrorContext.CouldNotFindIncludedFile( File("some-non-existing-file.ion").canonicalPath))) ) } diff --git a/pig/test/org/partiql/pig/domain/TypeDomainParserIncludeFileTests.kt b/pig/test/org/partiql/pig/domain/TypeDomainParserIncludeFileTests.kt index 9b1f517..008a174 100644 --- a/pig/test/org/partiql/pig/domain/TypeDomainParserIncludeFileTests.kt +++ b/pig/test/org/partiql/pig/domain/TypeDomainParserIncludeFileTests.kt @@ -16,11 +16,9 @@ package org.partiql.pig.domain import com.amazon.ionelement.api.IonTextLocation -import org.junit.jupiter.api.Assertions import org.junit.jupiter.api.Assertions.assertEquals import org.junit.jupiter.api.Test import org.junit.jupiter.api.assertThrows -import org.partiql.pig.domain.model.SemanticErrorContext import org.partiql.pig.domain.model.TypeDomain import org.partiql.pig.domain.parser.ParserErrorContext import org.partiql.pig.domain.parser.SourceLocation @@ -56,7 +54,7 @@ class TypeDomainParserIncludeFileTests { assertEquals( PigError( SourceLocation(File(universeFilePath).canonicalPath, IonTextLocation(4L, 1L)), - ParserErrorContext.CouldNotFindImportedTypeUniverse(File("test-domains/domains/doesnotexist.ion").canonicalPath) + ParserErrorContext.CouldNotFindIncludedFile(File("test-domains/domains/doesnotexist.ion").canonicalPath) ), ex.error ) diff --git a/pig/test/org/partiql/pig/util/ParseHelpers.kt b/pig/test/org/partiql/pig/util/ParseHelpers.kt index e57aaba..e527aa6 100644 --- a/pig/test/org/partiql/pig/util/ParseHelpers.kt +++ b/pig/test/org/partiql/pig/util/ParseHelpers.kt @@ -34,10 +34,10 @@ internal const val FAKE_ROOT_FILE = "root.ion" * * * [includes] is a map keyed by "fake" filename that will be used instead of a real-file system for looking - * up the content of imported files. [includes] must not contain any filename by the name of "root.ion", which + * up the content of included files. [includes] must not contain any filename by the name of "root.ion", which * is the name given to the type universe specified in [topUnvierseText]. */ -fun parseTypeUniverseString(topUnvierseText: String, includes: Map = emptyMap()): TypeUniverse { +internal fun parseTypeUniverseString(topUnvierseText: String, includes: Map = emptyMap()): TypeUniverse { assert(!includes.containsKey(FAKE_ROOT_FILE)) val allIncludes = (mapOf(FAKE_ROOT_FILE to topUnvierseText) + includes).map { @@ -49,9 +49,9 @@ fun parseTypeUniverseString(topUnvierseText: String, includes: Map. Used only for testing. */ -class FakeInputStreamSource(val sources: Map) : InputStreamSource { - override fun openInputStream(resolvedName: String): InputStream { - val text: String = sources[resolvedName] ?: throw FileNotFoundException("$resolvedName does not exist") +internal class FakeInputStreamSource(private val sources: Map) : InputStreamSource { + override fun openInputStream(fileName: String): InputStream { + val text: String = sources[fileName] ?: throw FileNotFoundException("$fileName does not exist") return ByteArrayInputStream(text.toByteArray(Charsets.UTF_8)) } From 53af53cb4c7a0ae1a8301e987930ac6c8e876eac Mon Sep 17 00:00:00 2001 From: David Lurton Date: Thu, 22 Jul 2021 20:02:27 -0700 Subject: [PATCH 09/22] Almost completely rewrite this thing... separate concerns, block .., test better, etc. --- README.md | 49 +++-- pig/build.gradle | 1 + pig/src/org/partiql/pig/cmdline/Command.kt | 12 +- .../partiql/pig/cmdline/CommandLineParser.kt | 47 +++-- .../org/partiql/pig/cmdline/TargetLanguage.kt | 3 +- .../pig/domain/include/IncludeCycleHandler.kt | 71 +++++++ .../include/IncludeResolutionException.kt | 25 +++ .../pig/domain/include/IncludeResolver.kt | 100 ++++++++++ .../partiql/pig/domain/include/InputSource.kt | 22 +++ .../include/InvalidIncludePathException.kt | 25 +++ .../pig/domain/model/SemanticErrorContext.kt | 8 +- .../pig/domain/parser/InputStreamSource.kt | 35 ---- .../pig/domain/parser/ParserErrorContext.kt | 21 +- .../pig/domain/parser/TypeDomainParser.kt | 129 ++++++++----- .../partiql/pig/generator/custom/generator.kt | 7 +- pig/src/org/partiql/pig/main.kt | 32 +++- pig/test-domains/domains/include_b.ion | 3 - pig/test-domains/include-missing-absolute.ion | 4 + pig/test-domains/include-missing-file.ion | 4 - pig/test-domains/include-missing-relative.ion | 4 + pig/test-domains/main.ion | 43 ++--- .../dir_x}/circular_universe_c.ion | 0 .../dir_x}/circular_universe_d.ion | 0 pig/test-domains/root_a/dir_x/include_b.ion | 3 + .../{domains => root_a/dir_x}/universe_a.ion | 0 .../{other => root_b/dir_z}/universe_b.ion | 0 pig/test-domains/sibling-of-main.ion | 5 + .../pig/cmdline/CommandLineParserTests.kt | 180 +++++++++++------- .../pig/domain/TypeDomainParserErrorsTest.kt | 29 ++- .../TypeDomainParserIncludeFileTests.kt | 67 +++++-- pig/test/org/partiql/pig/domain/Util.kt | 8 +- .../pig/include/IncludeResolverTests.kt | 104 ++++++++++ pig/test/org/partiql/pig/util/ParseHelpers.kt | 55 +++--- 33 files changed, 805 insertions(+), 291 deletions(-) create mode 100644 pig/src/org/partiql/pig/domain/include/IncludeCycleHandler.kt create mode 100644 pig/src/org/partiql/pig/domain/include/IncludeResolutionException.kt create mode 100644 pig/src/org/partiql/pig/domain/include/IncludeResolver.kt create mode 100644 pig/src/org/partiql/pig/domain/include/InputSource.kt create mode 100644 pig/src/org/partiql/pig/domain/include/InvalidIncludePathException.kt delete mode 100644 pig/src/org/partiql/pig/domain/parser/InputStreamSource.kt delete mode 100644 pig/test-domains/domains/include_b.ion create mode 100644 pig/test-domains/include-missing-absolute.ion delete mode 100644 pig/test-domains/include-missing-file.ion create mode 100644 pig/test-domains/include-missing-relative.ion rename pig/test-domains/{domains => root_a/dir_x}/circular_universe_c.ion (100%) rename pig/test-domains/{domains => root_a/dir_x}/circular_universe_d.ion (100%) create mode 100644 pig/test-domains/root_a/dir_x/include_b.ion rename pig/test-domains/{domains => root_a/dir_x}/universe_a.ion (100%) rename pig/test-domains/{other => root_b/dir_z}/universe_b.ion (100%) create mode 100644 pig/test-domains/sibling-of-main.ion create mode 100644 pig/test/org/partiql/pig/include/IncludeResolverTests.kt diff --git a/README.md b/README.md index 491ad1d..f915e2c 100644 --- a/README.md +++ b/README.md @@ -404,29 +404,40 @@ Unlike record elements, product element defintions must include identifiers. #### Type Domain Includes -It is possible to split type universes among multiple files: +It is possible to split type universes among multiple files, which allows type domains defined in another project to be +permuted. For example: ``` // root.ion: -(include_file "a.ion") -(include_file "b.ion") +(include_file "sibling.ion") +(include_file "sub-dir/thing.ion") +(include_file "/other-project/toy-ast.ion") ``` -The resulting type universe will contain all type domains from both `a.ion` and `b.ion`. `root.ion` may also define -additional type domains. The primary purpose of this is to be able to permute domains defined in another file. +While discussing further details, it is helpful to introduce two terms: an "includer" is a file which includes another +using `include_file`, and the "includee" is a file which is included. -`include_file` is recursive, so any type domains in any files included by `a.ion` and `b.ion` will also be present. Any -attempt to include a file that has already been seen will be ignored. +The `root.ion` universe will contain all type domains from all includees and may still define additional type domains +of its own. -Paths are always relative to the directory containing the current file. The working directory at the time `pig` -is executed is irrelevant. +`include_file` statements are allowed in includees. Any attempt to include a file that has already been seen will be +ignored. -``` -(include_file "subdir/partiql.ion") -(include_file "../other/some-universe.ion") -``` +When resolving the file to include, the following locations are searched: + +- The directory containing the includer (if the path to the includee does not start with `/`) +- The directory containing the "main" type universe that was passed to `pig` on the command-line. +- Any directories specified with the `--include` or `-I` arguments, in the order they were specified. + +The first matching file found wins and any other matching files ignored. + +Note that paths starting with `/` do not actually refer to the root of any file system, but instead are treated as +relative to the include directories. + +Paths specified with `include_file` may only contain alphanumeric or one of the following characters: +`-`, `_`, `.` or `/`. -Lastly, `include_file` can only be used at the top-level within a `.ion` file. It is not allowed within a +Lastly, `include_file` can only be used at the top-level within a `.ion` file. It is not allowed anywhere within a `(domain ...)` clause. #### Using PIG In Your Project @@ -436,16 +447,14 @@ Lastly, `include_file` can only be used at the top-level within a `.ion` file. At build time and before compilation of your application or library, the following should be executed: ``` -pig \ - -u \ - -t kotlin \ - -n \ - -o path/to/package/ +pig -u -t kotlin -n -o [ -I ] ``` - ``: path to the Ion text file containing the type universe -- ``: path to the file for the generated code +- ``: path to the file for the generated code - ``: the name used in the `package` statement at the top of the output file +- ``: search path to external include directory (optional). Can be specified more than once, +i.e. `pig ... -I -I -I ` Execute: `pig --help` for all command-line options. diff --git a/pig/build.gradle b/pig/build.gradle index 02b0347..3e6d91b 100644 --- a/pig/build.gradle +++ b/pig/build.gradle @@ -43,6 +43,7 @@ dependencies { testImplementation 'org.junit.jupiter:junit-jupiter-api:5.6.2' testImplementation 'org.junit.jupiter:junit-jupiter-params:5.6.2' testRuntimeOnly 'org.junit.jupiter:junit-jupiter-engine:5.6.2' + testImplementation 'com.google.jimfs:jimfs:1.2' } application { diff --git a/pig/src/org/partiql/pig/cmdline/Command.kt b/pig/src/org/partiql/pig/cmdline/Command.kt index 28ab799..1042b97 100644 --- a/pig/src/org/partiql/pig/cmdline/Command.kt +++ b/pig/src/org/partiql/pig/cmdline/Command.kt @@ -15,7 +15,7 @@ package org.partiql.pig.cmdline -import java.io.File +import java.nio.file.Path /** Represents command line options specified by the user. */ sealed class Command { @@ -35,8 +35,16 @@ sealed class Command { * * - [typeUniverseFilePath]: the path to the type universe file. * - [outputFilePath]: the path to the output file. (This makes the assumption that there is only one output file.) + * - [includePaths]: directories to be searched when looking for files included with `include_file`. * - [target]: specifies the target language and any other parameters unique to the target language. */ - data class Generate(val typeUniverseFilePath: File, val outputFilePath: File, val target: TargetLanguage) : Command() + data class Generate( + val typeUniverseFilePath: Path, + val outputFilePath: Path, + val includePaths: List, + val target: TargetLanguage + ) : Command() } + + diff --git a/pig/src/org/partiql/pig/cmdline/CommandLineParser.kt b/pig/src/org/partiql/pig/cmdline/CommandLineParser.kt index e8f11c7..d801b70 100644 --- a/pig/src/org/partiql/pig/cmdline/CommandLineParser.kt +++ b/pig/src/org/partiql/pig/cmdline/CommandLineParser.kt @@ -16,8 +16,9 @@ package org.partiql.pig.cmdline import joptsimple.* -import java.io.File import java.io.PrintStream +import java.nio.file.Path +import java.nio.file.Paths class CommandLineParser { @@ -30,7 +31,7 @@ class CommandLineParser { HTML } - private object languageTargetTypeValueConverter : ValueConverter { + private object LanguageTargetTypeValueConverter : ValueConverter { private val lookup = LanguageTargetType.values().associateBy { it.name.toLowerCase() } override fun convert(value: String?): LanguageTargetType { @@ -47,6 +48,12 @@ class CommandLineParser { } } + private object PathValueConverter : ValueConverter { + override fun convert(value: String?): Path = Paths.get(value).toAbsolutePath().normalize() + override fun valueType(): Class = Path::class.java + override fun valuePattern(): String? = null + } + private val formatter = object : BuiltinHelpFormatter(120, 2) { override fun format(options: MutableMap?): String { return """PartiQL I.R. Generator @@ -56,6 +63,8 @@ class CommandLineParser { | | --target=kotlin requires --namespace= | --target=custom requires --template= + | All paths specified in these command-line options are relative to the current working + | directory by default. | |Examples: | @@ -65,7 +74,9 @@ class CommandLineParser { """.trimMargin() } } - private val optParser = OptionParser().also { it.formatHelpWith(formatter) } + private val optParser = OptionParser().apply { + formatHelpWith(formatter) + } private val helpOpt = optParser.acceptsAll(listOf("help", "h", "?"), "prints this help") @@ -73,18 +84,22 @@ class CommandLineParser { private val universeOpt = optParser.acceptsAll(listOf("universe", "u"), "Type universe input file") .withRequiredArg() - .ofType(File::class.java) + .withValuesConvertedBy(PathValueConverter) .required() + private val includeSearchRootOpt = optParser.acceptsAll(listOf("include", "I"), "Include search path") + .withRequiredArg() + .withValuesConvertedBy(PathValueConverter) + .describedAs("Search path for files included with include_file. May be specified multiple times.") + private val outputOpt = optParser.acceptsAll(listOf("output", "o"), "Generated output file") .withRequiredArg() - .ofType(File::class.java) + .withValuesConvertedBy(PathValueConverter) .required() private val targetTypeOpt = optParser.acceptsAll(listOf("target", "t"), "Target language") .withRequiredArg() - //.ofType(LanguageTargetType::class.java) - .withValuesConvertedBy(languageTargetTypeValueConverter) + .withValuesConvertedBy(LanguageTargetTypeValueConverter) .required() private val namespaceOpt = optParser.acceptsAll(listOf("namespace", "n"), "Namespace for generated code") @@ -93,7 +108,7 @@ class CommandLineParser { private val templateOpt = optParser.acceptsAll(listOf("template", "e"), "Path to an Apache FreeMarker template") .withOptionalArg() - .ofType(File::class.java) + .withValuesConvertedBy(PathValueConverter) /** @@ -116,9 +131,15 @@ class CommandLineParser { optSet.has(helpOpt) -> Command.ShowHelp else -> { // !! is fine in this case since we define these options as .required() above. - val typeUniverseFile: File = optSet.valueOf(universeOpt)!! + val typeUniverseFile: Path = optSet.valueOf(universeOpt)!! val targetType = optSet.valueOf(targetTypeOpt)!! - val outputFile: File = optSet.valueOf(outputOpt)!! + val outputFile: Path = optSet.valueOf(outputOpt)!! + + // Always add the parent of the file containing the main type universe as an include root. + val includeSearchRoots = listOf( + typeUniverseFile.parent, + *optSet.valuesOf(includeSearchRootOpt)!!.toTypedArray() + ) if (targetType.requireNamespace) { if (!optSet.has(namespaceOpt)) { @@ -145,13 +166,11 @@ class CommandLineParser { LanguageTargetType.CUSTOM -> TargetLanguage.Custom(optSet.valueOf(templateOpt)) } - Command.Generate(typeUniverseFile, outputFile, target) + Command.Generate(typeUniverseFile, outputFile, includeSearchRoots, target) } } } catch(ex: OptionException) { Command.InvalidCommandLineArguments(ex.message!!) } - } - -} \ No newline at end of file +} diff --git a/pig/src/org/partiql/pig/cmdline/TargetLanguage.kt b/pig/src/org/partiql/pig/cmdline/TargetLanguage.kt index 3e7dc5c..1be15e4 100644 --- a/pig/src/org/partiql/pig/cmdline/TargetLanguage.kt +++ b/pig/src/org/partiql/pig/cmdline/TargetLanguage.kt @@ -16,9 +16,10 @@ package org.partiql.pig.cmdline import java.io.File +import java.nio.file.Path sealed class TargetLanguage { data class Kotlin(val namespace: String) : TargetLanguage() - data class Custom(val templateFile: File) : TargetLanguage() + data class Custom(val templateFile: Path) : TargetLanguage() object Html : TargetLanguage() } \ No newline at end of file diff --git a/pig/src/org/partiql/pig/domain/include/IncludeCycleHandler.kt b/pig/src/org/partiql/pig/domain/include/IncludeCycleHandler.kt new file mode 100644 index 0000000..4e5babb --- /dev/null +++ b/pig/src/org/partiql/pig/domain/include/IncludeCycleHandler.kt @@ -0,0 +1,71 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + */ + +package org.partiql.pig.domain.include + +import org.partiql.pig.domain.model.Statement +import org.partiql.pig.domain.parser.TypeUniverseParser +import java.nio.file.FileSystem +import java.nio.file.Files +import java.nio.file.Path + +/** + * Prevents cycles in files included with the `include_file` statement from becoming a problem. + * + * This is accomplished by keeping track of all files "seen" by the parser and then making any attempt to + * include a file that was previously seen a no-op. + * + * @param mainTypeUniversePath The path to the main type universe that was passed on the command-line. This is the + * first "seen" file and does not require resolution because the user gave an explicit path to its location. + * + * @param resolver For identifying the full path to the file to be included. + * + * @see IncludeResolver + * @see FileSystem + */ +internal class IncludeCycleHandler( + mainTypeUniversePath: Path, + private val resolver: IncludeResolver +) { + private val seenFiles = HashSet().apply { add(mainTypeUniversePath.toAbsolutePath().normalize()) } + + /** + * Parses a universe file included with `include_file`. + * + * The return value is a [List] of [Statement]s that make up the type universe file. + * + * This function becomes a no-op in the event that the [includee] has been seen previously: an + * empty [List] is is returned instead of the file being parsed again. + * + * @param includeePath The file requested to be included. + * @param includerPath The file in which the includee is to be included. + */ + fun parseIncludedTypeUniverse(includeePath: String, includerPath: Path): List { + + val resolvedIncludeFile = resolver.resolve(resolver.fileSystem.getPath(includeePath), includerPath) + + return if(!seenFiles.contains(resolvedIncludeFile)) { + seenFiles.add(resolvedIncludeFile) + Files.newInputStream(resolvedIncludeFile).use { + val source = InputSource(resolvedIncludeFile, it) + val parser = TypeUniverseParser(source, this) + parser.parse() + }.statements + } else { + listOf() + } + } +} + diff --git a/pig/src/org/partiql/pig/domain/include/IncludeResolutionException.kt b/pig/src/org/partiql/pig/domain/include/IncludeResolutionException.kt new file mode 100644 index 0000000..38defb6 --- /dev/null +++ b/pig/src/org/partiql/pig/domain/include/IncludeResolutionException.kt @@ -0,0 +1,25 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + */ + +package org.partiql.pig.domain.include + +import java.lang.Exception + +/** Thrown by [IncludeResolver] to indicate that it cannot locate an include file. */ +class IncludeResolutionException( + val inputFilePathString: String, + val consideredFilePaths: List +) : Exception("Could not locate include file '$inputFilePathString' in any considered path") + diff --git a/pig/src/org/partiql/pig/domain/include/IncludeResolver.kt b/pig/src/org/partiql/pig/domain/include/IncludeResolver.kt new file mode 100644 index 0000000..e991fdb --- /dev/null +++ b/pig/src/org/partiql/pig/domain/include/IncludeResolver.kt @@ -0,0 +1,100 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + */ + +package org.partiql.pig.domain.include + +import java.nio.file.FileSystem +import java.nio.file.FileSystems +import java.nio.file.Files +import java.nio.file.Path + +/** + * Manages the task of locating a file that was specified with the `include_file` statement. + * + * Terminology: + * + * - "includer": is a file containing an `include_file` statement. i.e. it is the file doing the including. + * - "includee": is a file being included by the includer. + * + * @param searchDirs A list of directories to search after searching the directory containing the includer, normally + * specified on the command-line. + * @param fileSystem The instance of [FileSystem] to be used. Allowing this to be specified instead of always using + * the default [FileSystem] allows an in-memory file system to be used during tests. For production uses, use the + * [FileSystem] returned from [FileSystems.getDefault] here. + * + * @throws InvalidIncludePathException if one of [searchDirs] does not exist or is not a directory. + */ +internal class IncludeResolver( + searchDirs: List, + internal val fileSystem: FileSystem +) { + + private val searchDirectories = searchDirs + .map { it.toAbsolutePath().normalize() } + .onEach { + validatePath(it) + if (!Files.isDirectory(it)) { + throw InvalidIncludePathException(it.toString()) + } + }.toTypedArray() + + private fun validatePath(it: Path) { + require(it.fileSystem === fileSystem) { "Path $it must not belong to a different FileSystem instance." } + } + + /** + * Searches for the absolute path of an included file, returning the first file found. + * + * The parent directory of the [includerFile] is searched first, followed by each of the source roots in turn. If + * [fileToInclude] starts with `/`, the parent directory of [includerFile] is skipped. + * + * @return The absolute [Path] of the first file located. + */ + fun resolve(includeeFile: Path, includerFile: Path): Path { + val normalizedIncluder = includerFile.normalize().toAbsolutePath() + + // Determine list of all possible search roots, excluding the includer's parent directory if + // the includee an absolute path. T + val includerParentDir = normalizedIncluder.parent + val searchPaths = listOfNotNull( + includerParentDir.takeIf { !includeeFile.isAbsolute }, + *searchDirectories + ).distinct() + // distinct is needed because duplicate entries can arise in this list, for instance if the + // includer's parent directory is the same as a include directory. Primarily this is needed to + // prevent the directory from appearing twice in the error messages we display when we can't + // find an include file. + + val possibleIncludeeFiles = searchPaths + .map { + // Truncates / at the beginning of an absolute path This forces absolute paths absolute of includees + // to be relative the include paths specified on the command-line and prevents them from being relative + // to the includer. + val appendPath = when { + includeeFile.isAbsolute -> includeeFile.toString().substring(1) + else -> includeeFile.toString() + } + it.resolve(appendPath) + } + + + // The resolved file is the first one that exists. + return possibleIncludeeFiles.firstOrNull { Files.exists(it) } + // TODO: can we used a algebraic type here instead of an exception? + ?: throw IncludeResolutionException( + inputFilePathString = includeeFile.toString(), + consideredFilePaths = possibleIncludeeFiles.map { "${it}" }) + } +} \ No newline at end of file diff --git a/pig/src/org/partiql/pig/domain/include/InputSource.kt b/pig/src/org/partiql/pig/domain/include/InputSource.kt new file mode 100644 index 0000000..da27a32 --- /dev/null +++ b/pig/src/org/partiql/pig/domain/include/InputSource.kt @@ -0,0 +1,22 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + */ + +package org.partiql.pig.domain.include + +import java.io.InputStream +import java.nio.file.Path + +/** Provides a convenient way to associate an inputStream with it's path. */ +class InputSource(val absolutePath: Path, val inputStream: InputStream) diff --git a/pig/src/org/partiql/pig/domain/include/InvalidIncludePathException.kt b/pig/src/org/partiql/pig/domain/include/InvalidIncludePathException.kt new file mode 100644 index 0000000..81f10e6 --- /dev/null +++ b/pig/src/org/partiql/pig/domain/include/InvalidIncludePathException.kt @@ -0,0 +1,25 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + */ + +package org.partiql.pig.domain.include + +/** + * Thrown by [IncludeResolver] to indicate that one of the search roots passed to it is invalid. + * + * Search roots are normally specified on the command-line. + */ +class InvalidIncludePathException(invalidIncludePath: String) + : Exception("Specified include path '$invalidIncludePath' does not exist or is not a directory.") + diff --git a/pig/src/org/partiql/pig/domain/model/SemanticErrorContext.kt b/pig/src/org/partiql/pig/domain/model/SemanticErrorContext.kt index 5cc855a..556651e 100644 --- a/pig/src/org/partiql/pig/domain/model/SemanticErrorContext.kt +++ b/pig/src/org/partiql/pig/domain/model/SemanticErrorContext.kt @@ -66,16 +66,16 @@ sealed class SemanticErrorContext(val msgFormatter: () -> String): ErrorContext : SemanticErrorContext({ "Cannot remove built-in type '$typeName'" }) data class DuplicateTypeDomainName(val domainName: String) - : SemanticErrorContext({ "Duplicate type domain tag: '${domainName} "}) + : SemanticErrorContext({ "Duplicate type domain tag: '${domainName}' "}) data class DuplicateRecordElementTag(val elementName: String) - : SemanticErrorContext({ "Duplicate record element tag: '${elementName} "}) + : SemanticErrorContext({ "Duplicate record element tag: '${elementName}' "}) data class DuplicateElementIdentifier(val elementName: String) - : SemanticErrorContext({ "Duplicate element identifier: '${elementName} "}) + : SemanticErrorContext({ "Duplicate element identifier: '${elementName}' "}) data class NameAlreadyUsed(val name: String, val domainName: String) - : SemanticErrorContext({ "Name '$name' was previously used in the `$domainName` type domain" }) + : SemanticErrorContext({ "Name '$name' was previously used in the '$domainName' type domain" }) data class CannotRemoveNonExistentSumVariant(val sumTypeName: String, val variantName: String) : SemanticErrorContext({ "Permuted sum type '${sumTypeName}' tries to remove variant '${variantName}' which " + diff --git a/pig/src/org/partiql/pig/domain/parser/InputStreamSource.kt b/pig/src/org/partiql/pig/domain/parser/InputStreamSource.kt deleted file mode 100644 index 0299e14..0000000 --- a/pig/src/org/partiql/pig/domain/parser/InputStreamSource.kt +++ /dev/null @@ -1,35 +0,0 @@ -/* - * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. - * - * Licensed under the Apache License, Version 2.0 (the "License"). - * You may not use this file except in compliance with the License. - * A copy of the License is located at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * or in the "license" file accompanying this file. This file is distributed - * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either - * express or implied. See the License for the specific language governing - * permissions and limitations under the License. - */ - -package org.partiql.pig.domain.parser - -import java.io.FileInputStream -import java.io.InputStream - - -/** - * A simple abstraction for opening [InputStream], allows for type domains to be loaded from a file system or an - * in-memory data structure. The latter is useful for unit testing. - */ -internal interface InputStreamSource { - /** Opens an input stream for the given filename name. */ - fun openInputStream(fileName: String): InputStream -} - -/** An [InputStreamSource] that constructs [FileInputStream] instances. */ -internal val FILE_SYSTEM_INPUT_STREAM_SOURCE = object : InputStreamSource { - override fun openInputStream(fileName: String) = FileInputStream(fileName) -} - diff --git a/pig/src/org/partiql/pig/domain/parser/ParserErrorContext.kt b/pig/src/org/partiql/pig/domain/parser/ParserErrorContext.kt index 58d9dc8..bd65d68 100644 --- a/pig/src/org/partiql/pig/domain/parser/ParserErrorContext.kt +++ b/pig/src/org/partiql/pig/domain/parser/ParserErrorContext.kt @@ -20,6 +20,7 @@ import com.amazon.ionelement.api.IonElementException import org.partiql.pig.errors.ErrorContext import org.partiql.pig.errors.PigError import org.partiql.pig.errors.PigException +import javax.swing.text.html.parser.Parser /** * Variants of [ParserErrorContext] contain details about various parse errors that can be encountered @@ -39,9 +40,6 @@ sealed class ParserErrorContext(val msgFormatter: () -> String): ErrorContext { override fun hashCode(): Int = 0 } - data class CouldNotFindIncludedFile(val tag: String) - : ParserErrorContext({ "Included file missing: $tag" }) - data class UnknownConstructor(val tag: String) : ParserErrorContext({ "Unknown constructor: '$tag' (expected constructors are 'domain' or 'permute_domain')" }) @@ -51,15 +49,26 @@ sealed class ParserErrorContext(val msgFormatter: () -> String): ErrorContext { data class InvalidTopLevelTag(val tag: String) : ParserErrorContext({ "Invalid top-level tag: '$tag'"}) - data class InvalidSumLevelTag(val tag: String) - : ParserErrorContext({ "Invalid tag for sum variant: '$tag'"}) - data class InvalidPermutedDomainTag(val tag: String) : ParserErrorContext({ "Invalid tag for permute_domain body: '$tag'"}) data class InvalidWithSumTag(val tag: String) : ParserErrorContext({ "Invalid tag for with body: '$tag'"}) + data class IncludeFileNotFound(val includeFilePath: String, val searchedPaths: List ) + : ParserErrorContext( + { + "Could not locate include file '$includeFilePath' at any of the following locations:\n" + + searchedPaths.joinToString("\n") + } + ) + + data class IncludeFilePathContainsIllegalCharacter(val c: Char) + : ParserErrorContext({ "Illegal character '$c' in include_file path" }) + + object IncludeFilePathContainsParentDirectory + : ParserErrorContext({ "include_file path contained parent directory, i.e. \"..\"" }) + data class ExpectedTypeReferenceArityTag(val tag: String) : ParserErrorContext({ "Expected '*' or '?' but found '$tag'"}) diff --git a/pig/src/org/partiql/pig/domain/parser/TypeDomainParser.kt b/pig/src/org/partiql/pig/domain/parser/TypeDomainParser.kt index e9efc60..18ef222 100644 --- a/pig/src/org/partiql/pig/domain/parser/TypeDomainParser.kt +++ b/pig/src/org/partiql/pig/domain/parser/TypeDomainParser.kt @@ -32,6 +32,10 @@ import com.amazon.ionelement.api.location import com.amazon.ionelement.api.metaContainerOf import com.amazon.ionelement.api.tag import com.amazon.ionelement.api.tail +import org.partiql.pig.domain.include.IncludeCycleHandler +import org.partiql.pig.domain.include.IncludeResolutionException +import org.partiql.pig.domain.include.IncludeResolver +import org.partiql.pig.domain.include.InputSource import org.partiql.pig.domain.model.Arity import org.partiql.pig.domain.model.DataType import org.partiql.pig.domain.model.NamedElement @@ -44,46 +48,61 @@ import org.partiql.pig.domain.model.TypeDomain import org.partiql.pig.domain.model.TypeRef import org.partiql.pig.domain.model.TypeUniverse import org.partiql.pig.errors.ErrorContext -import java.io.File -import java.io.FileNotFoundException -import java.io.InputStream -import java.util.Stack +import org.partiql.pig.errors.PigError +import org.partiql.pig.errors.PigException +import java.nio.file.FileSystem +import java.nio.file.Files +import java.nio.file.Path /** - * Parses the type universe specified in [universeFilePath]. + * Parses the type universe specified in [mainTypeUniversePath]. * - * Any files included with `include_file` are relative to the directory containing [universeFilePath]. + * @param mainTypeUnversePath Specifies the "main" type universe file where parsing will begin. The [FileSystem] + * of this path (as returned by [Path.getFileSystem]) will be utilized when searching for and reading files referenced + * by any `include_file` statement. Thus, it is possible to read type domains from a real file system for + * production or from an in-memory file system for tests. In the future this may also form the basis for reading type + * universes from `.jar` files. + * + * To read the type universe from a real file system, obtain the [Path] from the [java.nio.file.FileSystem.getPath] + * method on the default file system ([java.nio.file.FileSystems.getDefault]) or [java.nio.file.Paths.get]. + * + * @param includePaths specifies a list of directories to be searched (in order) when looking for files specified + * with `include_file`. The parent directory of [mainTypeUniversePath] is always added as a source root. */ -internal fun parseTypeUniverseFile(universeFilePath: String): TypeUniverse { - val parser = TypeUniverseParser(FILE_SYSTEM_INPUT_STREAM_SOURCE) - return parser.parseTypeUniverse(universeFilePath) +internal fun parseMainTypeUniverse( + mainTypeUniversePath: Path, + includePaths: List +): TypeUniverse { + require(includePaths.all { it.fileSystem === mainTypeUniversePath.fileSystem}) { + "The mainTypeUniversePath and all includePaths must be from the same FileSystem" + } + + val resolver = IncludeResolver( + searchDirs = listOf(mainTypeUniversePath.parent, *includePaths.toTypedArray()), + fileSystem = mainTypeUniversePath.fileSystem + ) + val includeFileManager = IncludeCycleHandler(mainTypeUniversePath, resolver) + val source = InputSource(mainTypeUniversePath.toAbsolutePath().normalize(), Files.newInputStream(mainTypeUniversePath)) + return TypeUniverseParser(source, includeFileManager).parse() } +/** + * Parses a type universe file. + * + * There must exist only one instance of this class per type universe file. (i.e. every `include_file` statement + * results in new instance of this class. + * + * When an `include_file` statement is encountered, [includeCycleHandler] will be utilized to deal with include + * search paths and include cycles. + */ internal class TypeUniverseParser( - private val inputStreamSource: InputStreamSource + private val source: InputSource, + private val includeCycleHandler: IncludeCycleHandler ) { - /** - * This contains every file the parser has "seen" and is used to detect and ignore any `(include_file ...)` - * statement for a file that the parser has previously seen. This sidesteps issues arising from files being - * included more than once, including infinite recursion in the case of cyclic includes. - */ - private val parseHistory = HashSet() - - /** - * The top of this stack is always the path to the input file currently being parsed. - * - * Kept up to date by the [parseTypeUniverse] function. - */ - private val inputFilePathStack = Stack() - /** Parses a type universe in the specified [IonReader]. */ - fun parseTypeUniverse(source: String): TypeUniverse { + fun parse(): TypeUniverse { val elementLoader = createIonElementLoader(IonElementLoaderOptions(includeLocationMeta = true)) - val qualifiedSource = File(source).absolutePath - return inputStreamSource.openInputStream(qualifiedSource).use { inputStream: InputStream -> - IonReaderBuilder.standard().build(inputStream).use { reader: IonReader -> - this.parseHistory.add(qualifiedSource) - this.inputFilePathStack.push(qualifiedSource) + return IonReaderBuilder.standard().build(source.inputStream).use { reader: IonReader -> val domains = try { val topLevelElements = elementLoader.loadAllElements(reader) topLevelElements.flatMap { topLevelValue -> @@ -100,10 +119,8 @@ internal class TypeUniverseParser( } catch (iee: IonElementException) { parseError(iee.location?.toSourceLocation(), ParserErrorContext.IonElementError(iee)) } - this.inputFilePathStack.pop() TypeUniverse(domains) } - } } private fun parseError(blame: IonElement, context: ErrorContext): Nothing { @@ -111,7 +128,7 @@ internal class TypeUniverseParser( parseError(loc, context) } - private fun IonLocation.toSourceLocation() = SourceLocation(inputFilePathStack.peek(), this) + private fun IonLocation.toSourceLocation() = SourceLocation(source.absolutePath.toString(), this) private fun MetaContainer.toSourceLocationMetas(): MetaContainer = this.location?.let { metaContainerOf(SOURCE_LOCATION_META_TAG to it.toSourceLocation()) @@ -119,22 +136,31 @@ internal class TypeUniverseParser( private fun parseIncludeFile(sexp: SexpElement): List { requireArityForTag(sexp, 1) - val relativePath = sexp.tail.single().asString().textValue - - val workingDirectory = File(this.inputFilePathStack.peek()).parentFile - val qualifiedSourcePath = File(workingDirectory, relativePath).canonicalPath - return if(!parseHistory.contains(qualifiedSourcePath)) { - try { - parseTypeUniverse(qualifiedSourcePath).statements - } catch (e: FileNotFoundException) { - parseError( - sexp.metas.location?.toSourceLocation(), - ParserErrorContext.CouldNotFindIncludedFile(qualifiedSourcePath)) + val includeePath = sexp.tail.single().asString() + + includeePath.textValue.forEach { + if (!isValidPathChar(it)) { + parseError(includeePath, ParserErrorContext.IncludeFilePathContainsIllegalCharacter(it)) } - } else { - listOf() + } + + if(includeePath.textValue.contains("..")) { + parseError(includeePath, ParserErrorContext.IncludeFilePathContainsParentDirectory) + } + + return try { + includeCycleHandler.parseIncludedTypeUniverse(includeePath.textValue, source.absolutePath) + } catch(ex: IncludeResolutionException) { + throw PigException( + PigError( + sexp.metas.location?.toSourceLocation(), + ParserErrorContext.IncludeFileNotFound(ex.inputFilePathString, ex.consideredFilePaths) + ), + ex + ) } } + private fun parseDefine(sexp: SexpElement): Statement { requireArityForTag(sexp, 2) val args = sexp.tail // Skip tag @@ -246,7 +272,7 @@ internal class TypeUniverseParser( return DataType.UserType.Tuple(typeName, TupleType.RECORD, namedElements, metas) } - fun parseRecordElements(elementSexps: List): List = + private fun parseRecordElements(elementSexps: List): List = elementSexps.asSequence() .map { it.asSexp() } .map { elementSexp -> @@ -369,6 +395,13 @@ internal class TypeUniverseParser( parseError(sexp, ParserErrorContext.InvalidArityForTag(arityRange, sexp.head.symbolValue, argCount)) } } - } +private val OTHER_VALID_PATH_CHARS = setOf('_', '-', '.', '/') + +/** + * Legal characters are: any letter or digit, plus any character in [OTHER_VALID_PATH_CHARS]. + * + * All other characters are illegal. + */ +private fun isValidPathChar(c: Char) = c.isLetterOrDigit() || OTHER_VALID_PATH_CHARS.contains(c) diff --git a/pig/src/org/partiql/pig/generator/custom/generator.kt b/pig/src/org/partiql/pig/generator/custom/generator.kt index 205f1de..a7cddf6 100644 --- a/pig/src/org/partiql/pig/generator/custom/generator.kt +++ b/pig/src/org/partiql/pig/generator/custom/generator.kt @@ -19,10 +19,11 @@ import org.partiql.pig.domain.model.TypeDomain import org.partiql.pig.generator.createDefaultFreeMarkerConfiguration import java.io.File import java.io.PrintWriter +import java.nio.file.Path import java.time.OffsetDateTime fun applyCustomTemplate( - templateFile: File, + templatePath: Path, domains: List, output: PrintWriter ) { @@ -30,8 +31,8 @@ fun applyCustomTemplate( val cfg = createDefaultFreeMarkerConfiguration() - cfg.setDirectoryForTemplateLoading(templateFile.parentFile) - val template = cfg.getTemplate(templateFile.name)!! + cfg.setDirectoryForTemplateLoading(templatePath.parent.toFile()) + val template = cfg.getTemplate(templatePath.fileName.toString())!! template.process(renderModel, output) } diff --git a/pig/src/org/partiql/pig/main.kt b/pig/src/org/partiql/pig/main.kt index f8fe1ed..5133f17 100644 --- a/pig/src/org/partiql/pig/main.kt +++ b/pig/src/org/partiql/pig/main.kt @@ -18,9 +18,10 @@ package org.partiql.pig import org.partiql.pig.cmdline.Command import org.partiql.pig.cmdline.CommandLineParser import org.partiql.pig.cmdline.TargetLanguage +import org.partiql.pig.domain.include.InvalidIncludePathException import org.partiql.pig.errors.PigException import org.partiql.pig.domain.model.TypeUniverse -import org.partiql.pig.domain.parser.parseTypeUniverseFile +import org.partiql.pig.domain.parser.parseMainTypeUniverse import org.partiql.pig.generator.custom.applyCustomTemplate import org.partiql.pig.generator.html.applyHtmlTemplate import org.partiql.pig.generator.kotlin.applyKotlinTemplate @@ -28,8 +29,13 @@ import org.partiql.pig.generator.kotlin.convertToKTypeUniverse import java.io.PrintWriter import kotlin.system.exitProcess - fun progress(msg: String) = - println("pig: ${msg}") +fun progress(msg: String) = + println("pig: $msg") + +fun fatal(msg: String) { + System.err.print("pig: $msg") + exitProcess(-1) +} /** * Entry point for when pig is being invoked from the command-line. @@ -45,9 +51,12 @@ fun main(args: Array) { is Command.Generate -> { try { generateCode(command) - } catch (e: PigException) { - System.err.println("pig: ${e.error.location}: ${e.error.context.message}\n${e.stackTrace}") - exitProcess(-1) + } + catch(e: InvalidIncludePathException) { + fatal(e.message!!) + } + catch (e: PigException) { + fatal("${e.error.location}: ${e.error.context.message}\n${e.stackTrace}") } } } @@ -61,13 +70,18 @@ fun main(args: Array) { fun generateCode(command: Command.Generate) { progress("universe file: ${command.typeUniverseFilePath}") progress("output file : ${command.outputFilePath}") - + command.includePaths.forEach { + progress("include dir : $it") + } progress("parsing the universe...") - val typeUniverse: TypeUniverse = parseTypeUniverseFile(command.typeUniverseFilePath.canonicalPath) + val typeUniverse: TypeUniverse = parseMainTypeUniverse( + mainTypeUniversePath = command.typeUniverseFilePath, + includePaths = command.includePaths + ) progress("permuting domains...") - PrintWriter(command.outputFilePath).use { printWriter -> + PrintWriter(command.outputFilePath.toFile()).use { printWriter -> when (command.target) { is TargetLanguage.Kotlin -> { progress("applying Kotlin pre-processing") diff --git a/pig/test-domains/domains/include_b.ion b/pig/test-domains/domains/include_b.ion deleted file mode 100644 index e461e47..0000000 --- a/pig/test-domains/domains/include_b.ion +++ /dev/null @@ -1,3 +0,0 @@ - -// include file with a relative path -(include_file "../other/universe_b.ion") diff --git a/pig/test-domains/include-missing-absolute.ion b/pig/test-domains/include-missing-absolute.ion new file mode 100644 index 0000000..5598a82 --- /dev/null +++ b/pig/test-domains/include-missing-absolute.ion @@ -0,0 +1,4 @@ + +// This file attempts to include a missing file using an absolute path. + +(include_file "/dir_x/does-not-exist.ion") diff --git a/pig/test-domains/include-missing-file.ion b/pig/test-domains/include-missing-file.ion deleted file mode 100644 index d8ce906..0000000 --- a/pig/test-domains/include-missing-file.ion +++ /dev/null @@ -1,4 +0,0 @@ - -// This file attempts to include a missing file. - -(include_file "domains/doesnotexist.ion") diff --git a/pig/test-domains/include-missing-relative.ion b/pig/test-domains/include-missing-relative.ion new file mode 100644 index 0000000..f02fa17 --- /dev/null +++ b/pig/test-domains/include-missing-relative.ion @@ -0,0 +1,4 @@ + +// This file attempts to include a missing file specified with a relative (not prefixed with '/') path + +(include_file "does-not-exist.ion") diff --git a/pig/test-domains/main.ion b/pig/test-domains/main.ion index ef4df78..b1ec9df 100644 --- a/pig/test-domains/main.ion +++ b/pig/test-domains/main.ion @@ -1,31 +1,30 @@ /* -This file is used to test the `(include_file )` statement. - -Two primary things are tested: +This file is used to test the `include_file` statement. Parsing this file and the files it includes is challenging - + if we can do it correctly then we should be able to handle any `include_file` statement the user throws at us. - Cyclic includes are tolerated without exception. -- Relative paths, which are always relative to the directory containing the type universe currently being parsed. - -. -├── domains -│ ├── circular_universe_c.ion includes circular_universe_d.ion -│ ├── circular_universe_d.ion includes circular_universe_c.ion -│ ├── include_b.ion includes ../other/universe_b -│ └── universe_a.ion includes include_b.ion -├── main.ion includes domains/universe_a.ion and domains/circular_universe_c.ion -├── include-missing-file.ion used by error handling tests and not included by main.ion -└── other - └── universe_b.ion includes nothing +- Aboslute and relative paths are respected during include resolution. +- Searching across multiple search roots works. + +├── include-missing-absolute.ion used by error handling tests and not included by main.ion +├── include-missing-relative.ion used by error handling tests and not included by main.ion +├── main.ion includes siblin-of-main.ion root_a/dir_x/universe_a.ion, root_b/dir_z/circular_universe_c.ion +├── root_a +│ └── dir_x +│ ├── circular_universe_c.ion includes circular_universe_d.ion +│ ├── circular_universe_d.ion includes circular_universe_c.ion +│ ├── include_b.ion includes ../dir_a/universe_b +│ └── universe_a.ion includes include_b.ion +├── root_b +│ └── dir_z +│ └── universe_b.ion includes nothing +└── sibling-of-main.ion -By parsing, main.ion (this file), we test that each of the other files is includeed (directly or indirectly). Each -.ion file (except for "include_b.ion") defines a type domain, for a total of 4 type domains. The test is successful -if main.ion can be included without error and results in a single universe with all 4 type domains. - */ - -(include_file "domains/universe_a.ion") -(include_file "domains/circular_universe_c.ion") +(include_file "/sibling-of-main.ion") +(include_file "/dir_x/universe_a.ion") +(include_file "/dir_x/circular_universe_c.ion") diff --git a/pig/test-domains/domains/circular_universe_c.ion b/pig/test-domains/root_a/dir_x/circular_universe_c.ion similarity index 100% rename from pig/test-domains/domains/circular_universe_c.ion rename to pig/test-domains/root_a/dir_x/circular_universe_c.ion diff --git a/pig/test-domains/domains/circular_universe_d.ion b/pig/test-domains/root_a/dir_x/circular_universe_d.ion similarity index 100% rename from pig/test-domains/domains/circular_universe_d.ion rename to pig/test-domains/root_a/dir_x/circular_universe_d.ion diff --git a/pig/test-domains/root_a/dir_x/include_b.ion b/pig/test-domains/root_a/dir_x/include_b.ion new file mode 100644 index 0000000..5fc6f65 --- /dev/null +++ b/pig/test-domains/root_a/dir_x/include_b.ion @@ -0,0 +1,3 @@ + +// include file with an aboslute path +(include_file "/dir_z/universe_b.ion") diff --git a/pig/test-domains/domains/universe_a.ion b/pig/test-domains/root_a/dir_x/universe_a.ion similarity index 100% rename from pig/test-domains/domains/universe_a.ion rename to pig/test-domains/root_a/dir_x/universe_a.ion diff --git a/pig/test-domains/other/universe_b.ion b/pig/test-domains/root_b/dir_z/universe_b.ion similarity index 100% rename from pig/test-domains/other/universe_b.ion rename to pig/test-domains/root_b/dir_z/universe_b.ion diff --git a/pig/test-domains/sibling-of-main.ion b/pig/test-domains/sibling-of-main.ion new file mode 100644 index 0000000..d01e632 --- /dev/null +++ b/pig/test-domains/sibling-of-main.ion @@ -0,0 +1,5 @@ + + +(define domain_s (domain (product foo))) + + diff --git a/pig/test/org/partiql/pig/cmdline/CommandLineParserTests.kt b/pig/test/org/partiql/pig/cmdline/CommandLineParserTests.kt index 1bb6362..70d39d5 100644 --- a/pig/test/org/partiql/pig/cmdline/CommandLineParserTests.kt +++ b/pig/test/org/partiql/pig/cmdline/CommandLineParserTests.kt @@ -18,7 +18,7 @@ package org.partiql.pig.cmdline import org.junit.jupiter.api.Assertions.assertEquals import org.junit.jupiter.params.ParameterizedTest import org.junit.jupiter.params.provider.MethodSource -import java.io.File +import java.nio.file.Paths class CommandLineParserTests { @@ -39,77 +39,113 @@ class CommandLineParserTests { @JvmStatic @Suppress("unused") - fun parametersForTests() = listOf( - // Help - TestCase(Command.ShowHelp, "-h"), - TestCase(Command.ShowHelp, "--help"), - - //////////////////////////////////////////////////////// - // Missing parameters required for all language targets - //////////////////////////////////////////////////////// - // No --universe - TestCase( - Command.InvalidCommandLineArguments("Missing required option(s) [u/universe]"), - "--target=kotlin", "--output=output.kt", "--namespace=some.package"), - - // No --target - TestCase( - Command.InvalidCommandLineArguments("Missing required option(s) [t/target]"), - "--universe=input.ion", "--output=output.kt", "--namespace=some.package"), - - // No the --output argument - TestCase( - Command.InvalidCommandLineArguments("Missing required option(s) [o/output]"), - "--universe=input.ion", "--target=kotlin", "--namespace=some.package"), - - //////////////////////////////////////////////////////// - // Kotlin target - //////////////////////////////////////////////////////// - // long parameter names - TestCase( - Command.Generate(File("input.ion"), File("output.kt"), TargetLanguage.Kotlin("some.package")), - "--universe=input.ion", "--target=kotlin", "--output=output.kt", "--namespace=some.package"), - - // short parameter names - TestCase( - Command.Generate(File("input.ion"), File("output.kt"), TargetLanguage.Kotlin("some.package")), - "-u=input.ion", "-t=kotlin", "-o=output.kt", "-n=some.package"), - - // missing the --namespace argument - TestCase( - Command.InvalidCommandLineArguments("The selected language target requires the --namespace argument"), - "-u=input.ion", "-t=kotlin", "-o=output.kt"), - - //////////////////////////////////////////////////////// - // Html target - //////////////////////////////////////////////////////// - // long parameter names - TestCase( - Command.Generate(File("input.ion"), File("output.html"), TargetLanguage.Html), - "--universe=input.ion", "--target=html", "--output=output.html"), - - // short parameter names - TestCase( - Command.Generate(File("input.ion"), File("output.html"), TargetLanguage.Html), - "-u=input.ion", "-target=html", "--output=output.html"), - - //////////////////////////////////////////////////////// - // Custom target - //////////////////////////////////////////////////////// - // long parameter names - TestCase( - Command.Generate(File("input.ion"), File("output.txt"), TargetLanguage.Custom(File("template.ftl"))), - "--universe=input.ion", "--target=custom", "--output=output.txt", "--template=template.ftl"), - - // short parameter names - TestCase( - Command.Generate(File("input.ion"), File("output.txt"), TargetLanguage.Custom(File("template.ftl"))), - "-u=input.ion", "-t=custom", "-o=output.txt", "-e=template.ftl"), - - // missing the --template argument - TestCase( - Command.InvalidCommandLineArguments("The selected language target requires the --template argument"), - "-u=input.ion", "-t=custom", "-o=output.kt") + fun parametersForTests(): List { + + return listOf( + // Help + TestCase(Command.ShowHelp, "-h"), + TestCase(Command.ShowHelp, "--help"), + + //////////////////////////////////////////////////////// + // Missing parameters required for all language targets + //////////////////////////////////////////////////////// + // No --universe + TestCase( + Command.InvalidCommandLineArguments("Missing required option(s) [u/universe]"), + "--target=kotlin", "--output=output.kt", "--namespace=some.package"), + + // No --target + TestCase( + Command.InvalidCommandLineArguments("Missing required option(s) [t/target]"), + "--universe=input.ion", "--output=output.kt", "--namespace=some.package"), + + // No --output argument + TestCase( + Command.InvalidCommandLineArguments("Missing required option(s) [o/output]"), + "--universe=input.ion", "--target=kotlin", "--namespace=some.package"), + + //////////////////////////////////////////////////////// + // Kotlin target + //////////////////////////////////////////////////////// + // long parameter names + TestCase( + createExpectedGenerateCommand(TargetLanguage.Kotlin("some.package"), "dir_a", "dir_b"), + "--universe=input.ion", + "--target=kotlin", + "--output=output.any", + "--include=dir_a", + "--include=dir_b", + "--namespace=some.package"), + + // short parameter names + TestCase( + createExpectedGenerateCommand(TargetLanguage.Kotlin("some.package"), "dir_a", "dir_b"), + "-u=input.ion", + "-t=kotlin", + "-o=output.any", + "-I=dir_a", + "-I=dir_b", + "-n=some.package" + ), + + // no include directories + TestCase( + createExpectedGenerateCommand(TargetLanguage.Kotlin("some.package")), + "-u=input.ion", + "-t=kotlin", + "-o=output.any", + "-n=some.package" + ), + + // missing the --namespace argument + TestCase( + Command.InvalidCommandLineArguments("The selected language target requires the --namespace argument"), + "-u=input.ion", "-t=kotlin", "-o=output.any"), + + //////////////////////////////////////////////////////// + // Html target + //////////////////////////////////////////////////////// + // long parameter names + TestCase( + createExpectedGenerateCommand(TargetLanguage.Html), + "--universe=input.ion", "--target=html", "--output=output.any"), + + // short parameter names + TestCase( + createExpectedGenerateCommand(TargetLanguage.Html), + "-u=input.ion", "-target=html", "--output=output.any"), + + //////////////////////////////////////////////////////// + // Custom target + //////////////////////////////////////////////////////// + // long parameter names + TestCase( + createExpectedGenerateCommand(TargetLanguage.Custom(Paths.get("template.ftl").toAbsolutePath())), + "--universe=input.ion", "--target=custom", "--output=output.any", "--template=template.ftl"), + + // short parameter names + TestCase( + createExpectedGenerateCommand(TargetLanguage.Custom(Paths.get("template.ftl").toAbsolutePath())), + "-u=input.ion", "-t=custom", "-o=output.any", "-e=template.ftl"), + + // missing the --template argument + TestCase( + Command.InvalidCommandLineArguments("The selected language target requires the --template argument"), + "-u=input.ion", "-t=custom", "-o=output.any") + ) + } + + private fun createExpectedGenerateCommand( + target: TargetLanguage, + vararg addlIncludePaths: String + ) = Command.Generate( + typeUniverseFilePath = Paths.get("input.ion").toAbsolutePath(), + outputFilePath = Paths.get("output.any").toAbsolutePath(), + includePaths = listOf( + Paths.get(".").toAbsolutePath().normalize(), + *addlIncludePaths.map { Paths.get(it).toAbsolutePath() }.toTypedArray() + ), + target = target ) } } \ No newline at end of file diff --git a/pig/test/org/partiql/pig/domain/TypeDomainParserErrorsTest.kt b/pig/test/org/partiql/pig/domain/TypeDomainParserErrorsTest.kt index 15b0f46..8fc5d80 100644 --- a/pig/test/org/partiql/pig/domain/TypeDomainParserErrorsTest.kt +++ b/pig/test/org/partiql/pig/domain/TypeDomainParserErrorsTest.kt @@ -25,8 +25,8 @@ import org.junit.jupiter.params.provider.MethodSource import org.partiql.pig.domain.parser.ParserErrorContext import org.partiql.pig.errors.PigError import org.partiql.pig.errors.PigException +import org.partiql.pig.util.makeFakePath import org.partiql.pig.util.parseTypeUniverseString -import java.io.File class TypeDomainParserErrorsTest { @@ -110,10 +110,33 @@ class TypeDomainParserErrorsTest { TestCase( "(include_file \"some-non-existing-file.ion\")", - makeErr(1, 1, ParserErrorContext.CouldNotFindIncludedFile( - File("some-non-existing-file.ion").canonicalPath))) + makeErr(1, 1, + ParserErrorContext.IncludeFileNotFound( + "some-non-existing-file.ion", + listOf(makeFakePath("some-non-existing-file.ion")) + ))), + TestCase( + "(include_file \"/some-sub-dir/some-non-existing-file.ion\")", + makeErr(1, 1, + ParserErrorContext.IncludeFileNotFound( + "/some-sub-dir/some-non-existing-file.ion", + listOf(makeFakePath("some-sub-dir/some-non-existing-file.ion")) + ))), + TestCase( + "(include_file \"../doesntmatter.ion\")", + makeErr(1, 15, ParserErrorContext.IncludeFilePathContainsParentDirectory)), + TestCase( + "(include_file \"c:/windows/drive/letter/is/bad.ion\")", + makeErr(1, 15, ParserErrorContext.IncludeFilePathContainsIllegalCharacter(':'))), + TestCase( + """(include_file "\\windows\\path\\separator")""", + makeErr(1, 15, ParserErrorContext.IncludeFilePathContainsIllegalCharacter('\\'))), + TestCase( + "(include_file \"space in name\")", + makeErr(1, 15, ParserErrorContext.IncludeFilePathContainsIllegalCharacter(' '))) ) } } + diff --git a/pig/test/org/partiql/pig/domain/TypeDomainParserIncludeFileTests.kt b/pig/test/org/partiql/pig/domain/TypeDomainParserIncludeFileTests.kt index 008a174..bf80908 100644 --- a/pig/test/org/partiql/pig/domain/TypeDomainParserIncludeFileTests.kt +++ b/pig/test/org/partiql/pig/domain/TypeDomainParserIncludeFileTests.kt @@ -20,41 +20,82 @@ import org.junit.jupiter.api.Assertions.assertEquals import org.junit.jupiter.api.Test import org.junit.jupiter.api.assertThrows import org.partiql.pig.domain.model.TypeDomain +import org.partiql.pig.domain.model.TypeUniverse import org.partiql.pig.domain.parser.ParserErrorContext import org.partiql.pig.domain.parser.SourceLocation -import org.partiql.pig.domain.parser.parseTypeUniverseFile +import org.partiql.pig.domain.parser.parseMainTypeUniverse import org.partiql.pig.errors.PigError import org.partiql.pig.errors.PigException -import java.io.File +import java.nio.file.Path +import java.nio.file.Paths import kotlin.test.assertTrue class TypeDomainParserIncludeFileTests { + private val mainDir = Paths.get("test-domains").toAbsolutePath() + private val rootA = mainDir.resolve("root_a").toAbsolutePath() + private val rootB = mainDir.resolve("root_b").toAbsolutePath() @Test - fun `include happy path`() { - val universe = parseTypeUniverseFile("test-domains/main.ion") + fun `include happy case`() { + val universe = parseWithTestRoots("test-domains/main.ion") val allDomains = universe.statements.filterIsInstance() - // If 4 domains are loaded correctly, then we deal with relative paths and circular references correctly. + // If 5 domains are loaded correctly, then we deal with relative paths and circular references correctly. // see documentation at top of test-domains/main.ion - assertEquals(4, allDomains.size) + assertEquals(5, allDomains.size) assertTrue(allDomains.any { it.tag == "domain_a" }) assertTrue(allDomains.any { it.tag == "domain_b" }) assertTrue(allDomains.any { it.tag == "domain_c" }) assertTrue(allDomains.any { it.tag == "domain_d" }) + assertTrue(allDomains.any { it.tag == "domain_s" }) + } + + private fun parseWithTestRoots(universeFile: String): TypeUniverse { + val includeSearchRoots = listOf(rootA, rootB) + return parseMainTypeUniverse(Paths.get(universeFile), includeSearchRoots) + } + + @Test + fun `include sad case - missing include - relative path`() { + val includeeFile = "does-not-exist.ion" + testMissingInclude( + mainUniverseFile = "test-domains/include-missing-relative.ion", + includeeFile = includeeFile, + pathsSearched = listOf( + "${mainDir}/$includeeFile", + "${rootA.resolve("does-not-exist.ion").toAbsolutePath()}", + "${rootB.resolve("does-not-exist.ion").toAbsolutePath()}" + ) + ) } @Test - fun `missing file`() { - val universeFilePath = "test-domains/include-missing-file.ion" - val ex = assertThrows { - parseTypeUniverseFile(universeFilePath) - } + fun `include sad case - missing include - absolute path`() { + val includeeFile = "/dir_x/does-not-exist.ion" + testMissingInclude( + mainUniverseFile = "test-domains/include-missing-absolute.ion", + includeeFile = includeeFile, + pathsSearched = listOf( + "${mainDir}$includeeFile", + "${rootA.resolve("dir_x/does-not-exist.ion").toAbsolutePath()}", + "${rootB.resolve("dir_x/does-not-exist.ion").toAbsolutePath()}" + ) + ) + } + private fun testMissingInclude( + mainUniverseFile: String, + includeeFile: String, + pathsSearched: List + ) { + val ex = assertThrows { parseWithTestRoots(mainUniverseFile) } assertEquals( PigError( - SourceLocation(File(universeFilePath).canonicalPath, IonTextLocation(4L, 1L)), - ParserErrorContext.CouldNotFindIncludedFile(File("test-domains/domains/doesnotexist.ion").canonicalPath) + SourceLocation(Paths.get(mainUniverseFile).toAbsolutePath().toString(), IonTextLocation(4L, 1L)), + ParserErrorContext.IncludeFileNotFound( + includeeFile, + pathsSearched + ) ), ex.error ) diff --git a/pig/test/org/partiql/pig/domain/Util.kt b/pig/test/org/partiql/pig/domain/Util.kt index de92f10..bd49301 100644 --- a/pig/test/org/partiql/pig/domain/Util.kt +++ b/pig/test/org/partiql/pig/domain/Util.kt @@ -36,7 +36,13 @@ import org.partiql.pig.util.FAKE_ROOT_FILE import java.io.File fun makeErr(line: Int, col: Int, errorContext: ErrorContext) = - PigError(SourceLocation(File(FAKE_ROOT_FILE).canonicalPath, IonTextLocation(line.toLong(), col.toLong())), errorContext) + PigError( + SourceLocation( + File(FAKE_ROOT_FILE).canonicalPath, + IonTextLocation(line.toLong(), col.toLong()) + ), + errorContext + ) fun makeErr(errorContext: ErrorContext) = PigError(null, errorContext) diff --git a/pig/test/org/partiql/pig/include/IncludeResolverTests.kt b/pig/test/org/partiql/pig/include/IncludeResolverTests.kt new file mode 100644 index 0000000..351384e --- /dev/null +++ b/pig/test/org/partiql/pig/include/IncludeResolverTests.kt @@ -0,0 +1,104 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + */ + +package org.partiql.pig.include + +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.assertThrows +import org.junit.jupiter.params.ParameterizedTest +import org.junit.jupiter.params.provider.MethodSource +import org.partiql.pig.domain.include.IncludeResolver +import org.partiql.pig.domain.include.IncludeResolutionException +import java.nio.file.FileSystems +import java.nio.file.Path +import java.nio.file.Paths + +class IncludeResolverTests { + companion object { + private val PATH_TO_MAIN = Paths.get("./test-domains/main.ion") + + class HappyCase(val includeePath: String, expectedResolved: String) { + val includerPath: Path = PATH_TO_MAIN + val expectedResolvedPath: Path = Paths.get(expectedResolved).toAbsolutePath().normalize() + } + + @JvmStatic @Suppress("UNUSED") + fun happyCases() = listOf( + HappyCase( + includeePath = "sibling-of-main.ion", + expectedResolved = "./test-domains/sibling-of-main.ion" + ), + HappyCase( + includeePath = "/dir_x/universe_a.ion", + expectedResolved = "./test-domains/root_a/dir_x/universe_a.ion" + ), + HappyCase( + includeePath = "/dir_z/universe_b.ion", + expectedResolved = "./test-domains/root_b/dir_z/universe_b.ion" + ) + ) + + class NotFoundCase(includee: String, val expectedConsdieredPaths: List) { + val includee: Path = Paths.get(includee) + } + + @JvmStatic @Suppress("UNUSED") + fun notFoundCases() = listOf( + NotFoundCase( + includee = "does-not-exist.ion", + expectedConsdieredPaths = listOf( + // no / at start of includePath, therefore we include the parent directory of the includer + Paths.get("test-domains/does-not-exist.ion").toAbsolutePath().toString(), + Paths.get("test-domains/root_a/does-not-exist.ion").toAbsolutePath().toString(), + Paths.get("test-domains/root_b/does-not-exist.ion").toAbsolutePath().toString() + ) + ), + NotFoundCase( + includee = "/does-not-exist.ion", + expectedConsdieredPaths = listOf( + // / at start of includePath, therefore we exclude the parent directory of the includer. + Paths.get("test-domains/root_a/does-not-exist.ion").toAbsolutePath().toString(), + Paths.get("test-domains/root_b/does-not-exist.ion").toAbsolutePath().toString()) + ) + ) + } + + private val resolver = IncludeResolver( + listOf( + Paths.get("./test-domains/root_a"), + Paths.get("./test-domains/root_b") + ), + // These tests refer to actual files in the source tree--here we should use the default FileSystem. + FileSystems.getDefault() + ) + + @ParameterizedTest + @MethodSource("happyCases") + fun `test happy resolution cases`(tc: HappyCase) { + val actualResolvedPath = resolver.resolve(Paths.get(tc.includeePath), tc.includerPath) + assertEquals(tc.expectedResolvedPath, actualResolvedPath) + } + + @ParameterizedTest + @MethodSource("notFoundCases") + fun `test not found resolution cases`(tc: NotFoundCase) { + val ex = assertThrows { + resolver.resolve(tc.includee, PATH_TO_MAIN) + } + assertEquals(tc.expectedConsdieredPaths, ex.consideredFilePaths) + } + + // TODO: include tests for InvalidIncludePathException. +} diff --git a/pig/test/org/partiql/pig/util/ParseHelpers.kt b/pig/test/org/partiql/pig/util/ParseHelpers.kt index e527aa6..6deaba7 100644 --- a/pig/test/org/partiql/pig/util/ParseHelpers.kt +++ b/pig/test/org/partiql/pig/util/ParseHelpers.kt @@ -15,44 +15,37 @@ package org.partiql.pig.util +import com.google.common.collect.ImmutableList +import com.google.common.jimfs.Configuration +import com.google.common.jimfs.Jimfs import org.partiql.pig.domain.model.TypeUniverse -import org.partiql.pig.domain.parser.InputStreamSource -import org.partiql.pig.domain.parser.TypeUniverseParser -import java.io.ByteArrayInputStream -import java.io.File -import java.io.FileNotFoundException -import java.io.InputStream +import org.partiql.pig.domain.parser.parseMainTypeUniverse +import java.nio.charset.StandardCharsets +import java.nio.file.FileSystem +import java.nio.file.Files +import java.nio.file.Path + + +internal const val FAKE_ROOT_DIR = "/fake-directory" +internal fun makeFakePath(fileName: String) = "$FAKE_ROOT_DIR/$fileName" + +/** The name of the "fake" root file used by unit tests. */ +internal val FAKE_ROOT_FILE = makeFakePath("root.ion") -/** - * The name of the "fake" root file used by unit tests. - */ -internal const val FAKE_ROOT_FILE = "root.ion" /** * For unit tests only. Parses the type universe specified in [topUnvierseText]. * - * - * - * [includes] is a map keyed by "fake" filename that will be used instead of a real-file system for looking - * up the content of included files. [includes] must not contain any filename by the name of "root.ion", which - * is the name given to the type universe specified in [topUnvierseText]. + * Accomplishes this by using Jimfs to create an in-memory file system, and then + * writing [topUnvierseText] to [FAKE_ROOT_FILE] within it. */ -internal fun parseTypeUniverseString(topUnvierseText: String, includes: Map = emptyMap()): TypeUniverse { - assert(!includes.containsKey(FAKE_ROOT_FILE)) - - val allIncludes = (mapOf(FAKE_ROOT_FILE to topUnvierseText) + includes).map { - File(it.key).canonicalPath to it.value - }.toMap() - - val parser = TypeUniverseParser(FakeInputStreamSource(allIncludes)) - return parser.parseTypeUniverse(FAKE_ROOT_FILE) -} +internal fun parseTypeUniverseString(topUnvierseText: String): TypeUniverse { + val build = Configuration.unix().toBuilder().setWorkingDirectory("/").build() + val fs: FileSystem = Jimfs.newFileSystem(build) + Files.createDirectory(fs.getPath(FAKE_ROOT_DIR)) + val rootPath: Path = fs.getPath(FAKE_ROOT_FILE) -/** A minimal faux file system backed by a Map. Used only for testing. */ -internal class FakeInputStreamSource(private val sources: Map) : InputStreamSource { - override fun openInputStream(fileName: String): InputStream { - val text: String = sources[fileName] ?: throw FileNotFoundException("$fileName does not exist") + Files.write(rootPath, ImmutableList.of(topUnvierseText), StandardCharsets.UTF_8) - return ByteArrayInputStream(text.toByteArray(Charsets.UTF_8)) - } + return parseMainTypeUniverse(rootPath, listOf()) } From ed06f64b86dda2a01b32a91e57201176b6dc684f Mon Sep 17 00:00:00 2001 From: David Lurton Date: Thu, 22 Jul 2021 20:06:44 -0700 Subject: [PATCH 10/22] Revise README.md --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index f915e2c..09f2e3e 100644 --- a/README.md +++ b/README.md @@ -435,7 +435,7 @@ Note that paths starting with `/` do not actually refer to the root of any file relative to the include directories. Paths specified with `include_file` may only contain alphanumeric or one of the following characters: -`-`, `_`, `.` or `/`. +`-`, `_`, `.` or `/`. Additionally, two consecutive ".." (i.e. a parent directory) are not allowed. Lastly, `include_file` can only be used at the top-level within a `.ion` file. It is not allowed anywhere within a `(domain ...)` clause. From 8ce7cf6985a8ecebaa2c4d18512ae899538afc96 Mon Sep 17 00:00:00 2001 From: David Lurton Date: Thu, 22 Jul 2021 20:06:44 -0700 Subject: [PATCH 11/22] Revise README.md --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index f915e2c..d164cfe 100644 --- a/README.md +++ b/README.md @@ -435,7 +435,7 @@ Note that paths starting with `/` do not actually refer to the root of any file relative to the include directories. Paths specified with `include_file` may only contain alphanumeric or one of the following characters: -`-`, `_`, `.` or `/`. +`-`, `_`, `.` or `/`. Additionally, two consecutive periods ".." (i.e. a parent directory) are not allowed. Lastly, `include_file` can only be used at the top-level within a `.ion` file. It is not allowed anywhere within a `(domain ...)` clause. From da4fbee3ebd35b583f9fbfcbbdc004911b46f1c0 Mon Sep 17 00:00:00 2001 From: David Lurton Date: Thu, 22 Jul 2021 20:08:52 -0700 Subject: [PATCH 12/22] Update README.md --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 09f2e3e..d164cfe 100644 --- a/README.md +++ b/README.md @@ -435,7 +435,7 @@ Note that paths starting with `/` do not actually refer to the root of any file relative to the include directories. Paths specified with `include_file` may only contain alphanumeric or one of the following characters: -`-`, `_`, `.` or `/`. Additionally, two consecutive ".." (i.e. a parent directory) are not allowed. +`-`, `_`, `.` or `/`. Additionally, two consecutive periods ".." (i.e. a parent directory) are not allowed. Lastly, `include_file` can only be used at the top-level within a `.ion` file. It is not allowed anywhere within a `(domain ...)` clause. From fe63a24ba80b292314b5b2d5aa961801299ea5b4 Mon Sep 17 00:00:00 2001 From: David Lurton Date: Fri, 30 Jul 2021 16:08:02 -0700 Subject: [PATCH 13/22] Apply suggestions from code review Co-authored-by: Alan Cai --- pig/src/org/partiql/pig/domain/include/IncludeCycleHandler.kt | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pig/src/org/partiql/pig/domain/include/IncludeCycleHandler.kt b/pig/src/org/partiql/pig/domain/include/IncludeCycleHandler.kt index 4e5babb..35af71e 100644 --- a/pig/src/org/partiql/pig/domain/include/IncludeCycleHandler.kt +++ b/pig/src/org/partiql/pig/domain/include/IncludeCycleHandler.kt @@ -47,7 +47,7 @@ internal class IncludeCycleHandler( * The return value is a [List] of [Statement]s that make up the type universe file. * * This function becomes a no-op in the event that the [includee] has been seen previously: an - * empty [List] is is returned instead of the file being parsed again. + * empty [List] is returned instead of the file being parsed again. * * @param includeePath The file requested to be included. * @param includerPath The file in which the includee is to be included. @@ -68,4 +68,3 @@ internal class IncludeCycleHandler( } } } - From 5bd18559baed157ebe5b10961ecbabc4d3ed0993 Mon Sep 17 00:00:00 2001 From: David Lurton Date: Fri, 30 Jul 2021 16:10:17 -0700 Subject: [PATCH 14/22] Apply suggestions from code review Co-authored-by: Alan Cai Co-authored-by: Abhilash Kuhikar --- .../org/partiql/pig/domain/include/IncludeResolver.kt | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/pig/src/org/partiql/pig/domain/include/IncludeResolver.kt b/pig/src/org/partiql/pig/domain/include/IncludeResolver.kt index e991fdb..c346129 100644 --- a/pig/src/org/partiql/pig/domain/include/IncludeResolver.kt +++ b/pig/src/org/partiql/pig/domain/include/IncludeResolver.kt @@ -58,7 +58,7 @@ internal class IncludeResolver( * Searches for the absolute path of an included file, returning the first file found. * * The parent directory of the [includerFile] is searched first, followed by each of the source roots in turn. If - * [fileToInclude] starts with `/`, the parent directory of [includerFile] is skipped. + * [includeeFile] starts with `/`, the parent directory of [includerFile] is skipped. * * @return The absolute [Path] of the first file located. */ @@ -66,7 +66,7 @@ internal class IncludeResolver( val normalizedIncluder = includerFile.normalize().toAbsolutePath() // Determine list of all possible search roots, excluding the includer's parent directory if - // the includee an absolute path. T + // the includee is an absolute path. val includerParentDir = normalizedIncluder.parent val searchPaths = listOfNotNull( includerParentDir.takeIf { !includeeFile.isAbsolute }, @@ -79,8 +79,8 @@ internal class IncludeResolver( val possibleIncludeeFiles = searchPaths .map { - // Truncates / at the beginning of an absolute path This forces absolute paths absolute of includees - // to be relative the include paths specified on the command-line and prevents them from being relative + // Truncates / at the beginning of an absolute path. This forces absolute paths of includees + // to be relative to the include paths specified on the command-line and prevents them from being relative // to the includer. val appendPath = when { includeeFile.isAbsolute -> includeeFile.toString().substring(1) @@ -97,4 +97,4 @@ internal class IncludeResolver( inputFilePathString = includeeFile.toString(), consideredFilePaths = possibleIncludeeFiles.map { "${it}" }) } -} \ No newline at end of file +} From 393213a565c2697c32306d2b75f41cff03f086ec Mon Sep 17 00:00:00 2001 From: David Lurton Date: Fri, 30 Jul 2021 16:11:02 -0700 Subject: [PATCH 15/22] Update pig/src/org/partiql/pig/domain/include/IncludeResolver.kt Co-authored-by: Alan Cai --- pig/src/org/partiql/pig/domain/include/IncludeResolver.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pig/src/org/partiql/pig/domain/include/IncludeResolver.kt b/pig/src/org/partiql/pig/domain/include/IncludeResolver.kt index c346129..5dc8592 100644 --- a/pig/src/org/partiql/pig/domain/include/IncludeResolver.kt +++ b/pig/src/org/partiql/pig/domain/include/IncludeResolver.kt @@ -95,6 +95,6 @@ internal class IncludeResolver( // TODO: can we used a algebraic type here instead of an exception? ?: throw IncludeResolutionException( inputFilePathString = includeeFile.toString(), - consideredFilePaths = possibleIncludeeFiles.map { "${it}" }) + consideredFilePaths = possibleIncludeeFiles.map { "$it" }) } } From 39471ee93c9d609d966637275b4b33c0fcbdc680 Mon Sep 17 00:00:00 2001 From: David Lurton Date: Fri, 30 Jul 2021 16:17:05 -0700 Subject: [PATCH 16/22] rephrase kdoc --- pig/src/org/partiql/pig/domain/parser/SourceLocation.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pig/src/org/partiql/pig/domain/parser/SourceLocation.kt b/pig/src/org/partiql/pig/domain/parser/SourceLocation.kt index 45ef8a0..787d205 100644 --- a/pig/src/org/partiql/pig/domain/parser/SourceLocation.kt +++ b/pig/src/org/partiql/pig/domain/parser/SourceLocation.kt @@ -19,8 +19,8 @@ import com.amazon.ionelement.api.IonLocation import com.amazon.ionelement.api.MetaContainer /** - * Used to construct helpful error messages for the end-user, who will be able to know the file, line & column of - * a given error. + * Used to construct helpful error messages for the end-user, who will be able to the location of a given + * error which includes the file name, line & column. */ data class SourceLocation(val path: String, val location: IonLocation) { override fun toString(): String { From b9d959ee8f3f78524a863925f3c50191410b21d4 Mon Sep 17 00:00:00 2001 From: David Lurton Date: Fri, 30 Jul 2021 16:18:28 -0700 Subject: [PATCH 17/22] Apply suggestions from code review Co-authored-by: Alan Cai --- pig/src/org/partiql/pig/domain/parser/TypeDomainParser.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pig/src/org/partiql/pig/domain/parser/TypeDomainParser.kt b/pig/src/org/partiql/pig/domain/parser/TypeDomainParser.kt index 18ef222..e6ec7a3 100644 --- a/pig/src/org/partiql/pig/domain/parser/TypeDomainParser.kt +++ b/pig/src/org/partiql/pig/domain/parser/TypeDomainParser.kt @@ -57,7 +57,7 @@ import java.nio.file.Path /** * Parses the type universe specified in [mainTypeUniversePath]. * - * @param mainTypeUnversePath Specifies the "main" type universe file where parsing will begin. The [FileSystem] + * @param mainTypeUniversePath Specifies the "main" type universe file where parsing will begin. The [FileSystem] * of this path (as returned by [Path.getFileSystem]) will be utilized when searching for and reading files referenced * by any `include_file` statement. Thus, it is possible to read type domains from a real file system for * production or from an in-memory file system for tests. In the future this may also form the basis for reading type From 47352597ad020042762e3de6b20992b5eeeeb8a5 Mon Sep 17 00:00:00 2001 From: David Lurton Date: Fri, 30 Jul 2021 16:30:27 -0700 Subject: [PATCH 18/22] Apply CR feedback --- .../domain/include/InvalidIncludePathException.kt | 2 +- .../org/partiql/pig/generator/custom/generator.kt | 1 - pig/test-domains/main.ion | 4 +++- .../partiql/pig/cmdline/CommandLineParserTests.kt | 4 ++-- .../pig/domain/TypeDomainParserIncludeFileTests.kt | 1 - .../org/partiql/pig/include/IncludeResolverTests.kt | 13 ++++++++++++- 6 files changed, 18 insertions(+), 7 deletions(-) diff --git a/pig/src/org/partiql/pig/domain/include/InvalidIncludePathException.kt b/pig/src/org/partiql/pig/domain/include/InvalidIncludePathException.kt index 81f10e6..c3740b5 100644 --- a/pig/src/org/partiql/pig/domain/include/InvalidIncludePathException.kt +++ b/pig/src/org/partiql/pig/domain/include/InvalidIncludePathException.kt @@ -20,6 +20,6 @@ package org.partiql.pig.domain.include * * Search roots are normally specified on the command-line. */ -class InvalidIncludePathException(invalidIncludePath: String) +class InvalidIncludePathException(val invalidIncludePath: String) : Exception("Specified include path '$invalidIncludePath' does not exist or is not a directory.") diff --git a/pig/src/org/partiql/pig/generator/custom/generator.kt b/pig/src/org/partiql/pig/generator/custom/generator.kt index a7cddf6..0f84b7c 100644 --- a/pig/src/org/partiql/pig/generator/custom/generator.kt +++ b/pig/src/org/partiql/pig/generator/custom/generator.kt @@ -17,7 +17,6 @@ package org.partiql.pig.generator.custom import org.partiql.pig.domain.model.TypeDomain import org.partiql.pig.generator.createDefaultFreeMarkerConfiguration -import java.io.File import java.io.PrintWriter import java.nio.file.Path import java.time.OffsetDateTime diff --git a/pig/test-domains/main.ion b/pig/test-domains/main.ion index b1ec9df..71d7ab0 100644 --- a/pig/test-domains/main.ion +++ b/pig/test-domains/main.ion @@ -3,13 +3,15 @@ This file is used to test the `include_file` statement. Parsing this file and the files it includes is challenging - if we can do it correctly then we should be able to handle any `include_file` statement the user throws at us. +The following aspects are being tested here: + - Cyclic includes are tolerated without exception. - Aboslute and relative paths are respected during include resolution. - Searching across multiple search roots works. ├── include-missing-absolute.ion used by error handling tests and not included by main.ion ├── include-missing-relative.ion used by error handling tests and not included by main.ion -├── main.ion includes siblin-of-main.ion root_a/dir_x/universe_a.ion, root_b/dir_z/circular_universe_c.ion +├── main.ion includes sibling-of-main.ion, root_a/dir_x/universe_a.ion, root_b/dir_z/circular_universe_c.ion ├── root_a │ └── dir_x │ ├── circular_universe_c.ion includes circular_universe_d.ion diff --git a/pig/test/org/partiql/pig/cmdline/CommandLineParserTests.kt b/pig/test/org/partiql/pig/cmdline/CommandLineParserTests.kt index 70d39d5..d2f8d43 100644 --- a/pig/test/org/partiql/pig/cmdline/CommandLineParserTests.kt +++ b/pig/test/org/partiql/pig/cmdline/CommandLineParserTests.kt @@ -137,13 +137,13 @@ class CommandLineParserTests { private fun createExpectedGenerateCommand( target: TargetLanguage, - vararg addlIncludePaths: String + vararg additionalIncludePaths: String ) = Command.Generate( typeUniverseFilePath = Paths.get("input.ion").toAbsolutePath(), outputFilePath = Paths.get("output.any").toAbsolutePath(), includePaths = listOf( Paths.get(".").toAbsolutePath().normalize(), - *addlIncludePaths.map { Paths.get(it).toAbsolutePath() }.toTypedArray() + *additionalIncludePaths.map { Paths.get(it).toAbsolutePath() }.toTypedArray() ), target = target ) diff --git a/pig/test/org/partiql/pig/domain/TypeDomainParserIncludeFileTests.kt b/pig/test/org/partiql/pig/domain/TypeDomainParserIncludeFileTests.kt index bf80908..20d0f14 100644 --- a/pig/test/org/partiql/pig/domain/TypeDomainParserIncludeFileTests.kt +++ b/pig/test/org/partiql/pig/domain/TypeDomainParserIncludeFileTests.kt @@ -26,7 +26,6 @@ import org.partiql.pig.domain.parser.SourceLocation import org.partiql.pig.domain.parser.parseMainTypeUniverse import org.partiql.pig.errors.PigError import org.partiql.pig.errors.PigException -import java.nio.file.Path import java.nio.file.Paths import kotlin.test.assertTrue diff --git a/pig/test/org/partiql/pig/include/IncludeResolverTests.kt b/pig/test/org/partiql/pig/include/IncludeResolverTests.kt index 351384e..89b1e93 100644 --- a/pig/test/org/partiql/pig/include/IncludeResolverTests.kt +++ b/pig/test/org/partiql/pig/include/IncludeResolverTests.kt @@ -16,11 +16,14 @@ package org.partiql.pig.include import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Test import org.junit.jupiter.api.assertThrows import org.junit.jupiter.params.ParameterizedTest import org.junit.jupiter.params.provider.MethodSource import org.partiql.pig.domain.include.IncludeResolver import org.partiql.pig.domain.include.IncludeResolutionException +import org.partiql.pig.domain.include.InvalidIncludePathException +import java.nio.file.FileSystem import java.nio.file.FileSystems import java.nio.file.Path import java.nio.file.Paths @@ -100,5 +103,13 @@ class IncludeResolverTests { assertEquals(tc.expectedConsdieredPaths, ex.consideredFilePaths) } - // TODO: include tests for InvalidIncludePathException. + @Test + fun `test invalid include search path`() { + assertThrows { + IncludeResolver( + listOf(Paths.get("/this/dir/does/not/exist")), + FileSystems.getDefault() + ) + } + } } From e30663247e7349f0206373620b03b8700fe772b4 Mon Sep 17 00:00:00 2001 From: David Lurton Date: Wed, 4 Aug 2021 16:11:43 -0700 Subject: [PATCH 19/22] Add test for duplicate domains across multiple files --- pig/test-domains/README.md | 33 ++++++++++++++++++ pig/test-domains/duplicate_domains.ion | 4 +++ pig/test-domains/main.ion | 27 --------------- .../dir_x/first_duplicated_domain_name.ion | 4 +++ .../dir_z/second_duplicated_domain_name.ion | 4 +++ .../TypeDomainParserIncludeFileTests.kt | 24 ++++++------- .../domain/TypeDomainSemanticCheckerTests.kt | 34 +++++++++++++++++++ pig/test/org/partiql/pig/util/ParseHelpers.kt | 11 ++++++ 8 files changed, 100 insertions(+), 41 deletions(-) create mode 100644 pig/test-domains/README.md create mode 100644 pig/test-domains/duplicate_domains.ion create mode 100644 pig/test-domains/root_a/dir_x/first_duplicated_domain_name.ion create mode 100644 pig/test-domains/root_b/dir_z/second_duplicated_domain_name.ion diff --git a/pig/test-domains/README.md b/pig/test-domains/README.md new file mode 100644 index 0000000..62e7eda --- /dev/null +++ b/pig/test-domains/README.md @@ -0,0 +1,33 @@ +The `.ion` files in this directory are used to test various aspects of the `include_file` statement. Parsing `main.ion` +and the files it includes is challenging if we can do it correctly then we should be able to handle any +`include_file` statement the user throws at us. + +The following concerns of `IncludeCycleHandler`, `IncludeResolver`, and `TypeDomainParser` are being tested here: + +- Cyclic includes are tolerated without exception. +- Aboslute and relative paths are respected during include resolution. +- Searching across multiple search roots works. + +Also, the concern of the `TypeDomainSemanticChecker` is tested as well: + +- Duplicate domains names are detected even if they are defined in different files. + NOTE: this is a concern of the semantic checker! + + +├── duplicate_domains.ion used by `TypeDomainSemanticCheckerTests` +├── include-missing-absolute.ion used by parser error handling tests and not included by main.ion +├── include-missing-relative.ion used by parser error handling tests and not included by main.ion +├── main.ion includes sibling-of-main.ion, root_a/dir_x/universe_a.ion, root_b/dir_z/circular_universe_c.ion +├── root_a +│ └── dir_x +│ ├── circular_universe_c.ion includes circular_universe_d.ion +│ ├── circular_universe_d.ion includes circular_universe_c.ion +│ ├── first_duplicated_domain_name.ion defines domain_dup +│ ├── include_b.ion includes . ./dir_a/universe_b +│ └── universe_a.ion includes include_b.ion +├── root_b +│ └── dir_z +│ ├── second_duplicated_domain_name.ion defines domain_dup +│ └── universe_b.ion includes nothing +└── sibling-of-main.ion + diff --git a/pig/test-domains/duplicate_domains.ion b/pig/test-domains/duplicate_domains.ion new file mode 100644 index 0000000..db45680 --- /dev/null +++ b/pig/test-domains/duplicate_domains.ion @@ -0,0 +1,4 @@ + +(include_file "dir_x/first_duplicated_domain_name.ion") +(include_file "dir_z/second_duplicated_domain_name.ion") + diff --git a/pig/test-domains/main.ion b/pig/test-domains/main.ion index 71d7ab0..b37ee26 100644 --- a/pig/test-domains/main.ion +++ b/pig/test-domains/main.ion @@ -1,32 +1,5 @@ -/* -This file is used to test the `include_file` statement. Parsing this file and the files it includes is challenging - - if we can do it correctly then we should be able to handle any `include_file` statement the user throws at us. - -The following aspects are being tested here: - -- Cyclic includes are tolerated without exception. -- Aboslute and relative paths are respected during include resolution. -- Searching across multiple search roots works. - -├── include-missing-absolute.ion used by error handling tests and not included by main.ion -├── include-missing-relative.ion used by error handling tests and not included by main.ion -├── main.ion includes sibling-of-main.ion, root_a/dir_x/universe_a.ion, root_b/dir_z/circular_universe_c.ion -├── root_a -│ └── dir_x -│ ├── circular_universe_c.ion includes circular_universe_d.ion -│ ├── circular_universe_d.ion includes circular_universe_c.ion -│ ├── include_b.ion includes ../dir_a/universe_b -│ └── universe_a.ion includes include_b.ion -├── root_b -│ └── dir_z -│ └── universe_b.ion includes nothing -└── sibling-of-main.ion - - -*/ (include_file "/sibling-of-main.ion") (include_file "/dir_x/universe_a.ion") (include_file "/dir_x/circular_universe_c.ion") - diff --git a/pig/test-domains/root_a/dir_x/first_duplicated_domain_name.ion b/pig/test-domains/root_a/dir_x/first_duplicated_domain_name.ion new file mode 100644 index 0000000..827b0f2 --- /dev/null +++ b/pig/test-domains/root_a/dir_x/first_duplicated_domain_name.ion @@ -0,0 +1,4 @@ + +// The first instance of domain_dup should not result in an error +(define domain_dup (domain (product foo))) + diff --git a/pig/test-domains/root_b/dir_z/second_duplicated_domain_name.ion b/pig/test-domains/root_b/dir_z/second_duplicated_domain_name.ion new file mode 100644 index 0000000..bf8cc3a --- /dev/null +++ b/pig/test-domains/root_b/dir_z/second_duplicated_domain_name.ion @@ -0,0 +1,4 @@ + +// the second instance of domain_dup should result in an error +(define domain_dup (domain (product foo))) + diff --git a/pig/test/org/partiql/pig/domain/TypeDomainParserIncludeFileTests.kt b/pig/test/org/partiql/pig/domain/TypeDomainParserIncludeFileTests.kt index 20d0f14..067babb 100644 --- a/pig/test/org/partiql/pig/domain/TypeDomainParserIncludeFileTests.kt +++ b/pig/test/org/partiql/pig/domain/TypeDomainParserIncludeFileTests.kt @@ -26,14 +26,14 @@ import org.partiql.pig.domain.parser.SourceLocation import org.partiql.pig.domain.parser.parseMainTypeUniverse import org.partiql.pig.errors.PigError import org.partiql.pig.errors.PigException +import org.partiql.pig.util.MAIN_DOMAINS_DIR +import org.partiql.pig.util.ROOT_A +import org.partiql.pig.util.ROOT_B +import org.partiql.pig.util.parseWithTestRoots import java.nio.file.Paths import kotlin.test.assertTrue class TypeDomainParserIncludeFileTests { - private val mainDir = Paths.get("test-domains").toAbsolutePath() - private val rootA = mainDir.resolve("root_a").toAbsolutePath() - private val rootB = mainDir.resolve("root_b").toAbsolutePath() - @Test fun `include happy case`() { val universe = parseWithTestRoots("test-domains/main.ion") @@ -49,10 +49,6 @@ class TypeDomainParserIncludeFileTests { assertTrue(allDomains.any { it.tag == "domain_s" }) } - private fun parseWithTestRoots(universeFile: String): TypeUniverse { - val includeSearchRoots = listOf(rootA, rootB) - return parseMainTypeUniverse(Paths.get(universeFile), includeSearchRoots) - } @Test fun `include sad case - missing include - relative path`() { @@ -61,9 +57,9 @@ class TypeDomainParserIncludeFileTests { mainUniverseFile = "test-domains/include-missing-relative.ion", includeeFile = includeeFile, pathsSearched = listOf( - "${mainDir}/$includeeFile", - "${rootA.resolve("does-not-exist.ion").toAbsolutePath()}", - "${rootB.resolve("does-not-exist.ion").toAbsolutePath()}" + "${MAIN_DOMAINS_DIR}/$includeeFile", + "${ROOT_A.resolve("does-not-exist.ion").toAbsolutePath()}", + "${ROOT_B.resolve("does-not-exist.ion").toAbsolutePath()}" ) ) } @@ -75,9 +71,9 @@ class TypeDomainParserIncludeFileTests { mainUniverseFile = "test-domains/include-missing-absolute.ion", includeeFile = includeeFile, pathsSearched = listOf( - "${mainDir}$includeeFile", - "${rootA.resolve("dir_x/does-not-exist.ion").toAbsolutePath()}", - "${rootB.resolve("dir_x/does-not-exist.ion").toAbsolutePath()}" + "${MAIN_DOMAINS_DIR}$includeeFile", + "${ROOT_A.resolve("dir_x/does-not-exist.ion").toAbsolutePath()}", + "${ROOT_B.resolve("dir_x/does-not-exist.ion").toAbsolutePath()}" ) ) } diff --git a/pig/test/org/partiql/pig/domain/TypeDomainSemanticCheckerTests.kt b/pig/test/org/partiql/pig/domain/TypeDomainSemanticCheckerTests.kt index a939887..3d7ab37 100644 --- a/pig/test/org/partiql/pig/domain/TypeDomainSemanticCheckerTests.kt +++ b/pig/test/org/partiql/pig/domain/TypeDomainSemanticCheckerTests.kt @@ -15,14 +15,24 @@ package org.partiql.pig.domain +import com.amazon.ionelement.api.IonTextLocation import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Test import org.junit.jupiter.api.assertThrows import org.junit.jupiter.params.ParameterizedTest import org.junit.jupiter.params.provider.MethodSource import org.partiql.pig.domain.model.SemanticErrorContext +import org.partiql.pig.domain.parser.SourceLocation +import org.partiql.pig.errors.ErrorContext import org.partiql.pig.errors.PigError import org.partiql.pig.errors.PigException +import org.partiql.pig.util.FAKE_ROOT_FILE +import org.partiql.pig.util.MAIN_DOMAINS_DIR +import org.partiql.pig.util.ROOT_B import org.partiql.pig.util.parseTypeUniverseString +import org.partiql.pig.util.parseWithTestRoots +import java.io.File +import java.nio.file.Paths class TypeDomainSemanticCheckerTests { @@ -210,4 +220,28 @@ class TypeDomainSemanticCheckerTests { TestCase("(define some_domain (domain (record some_record)))", makeErr(1, 29, SemanticErrorContext.EmptyRecord))) } + + /** + * This ensures that we can still detect duplicate domains even if they reside in different files. + * + * To be totally clear here, [org.partiql.pig.domain.model.TypeDomainSemanticChecker] knows absolutely nothing + * of the `include_file` statement: all `TypeDomainSemanticChecker` sees is a list of domains and does not care one + * whit what file the domain was defined in: if a duplicate name was in the list of domains, it will issue an + * error. This test only exists to cover the unlikely scenario that this assumption changes in the future. + */ + @Test + fun `duplicate domain detection works across include_file`() { + val u = parseWithTestRoots("test-domains/duplicate_domains.ion") + val ex = assertThrows { u.computeTypeDomains() } + val expectedError = + PigError( + SourceLocation( + ROOT_B.resolve("dir_z/second_duplicated_domain_name.ion").toString(), + IonTextLocation(3, 20) + ), + SemanticErrorContext.DuplicateTypeDomainName("domain_dup") + ) + assertEquals(expectedError, ex.error) + } + } diff --git a/pig/test/org/partiql/pig/util/ParseHelpers.kt b/pig/test/org/partiql/pig/util/ParseHelpers.kt index 6deaba7..adf3b3b 100644 --- a/pig/test/org/partiql/pig/util/ParseHelpers.kt +++ b/pig/test/org/partiql/pig/util/ParseHelpers.kt @@ -24,6 +24,7 @@ import java.nio.charset.StandardCharsets import java.nio.file.FileSystem import java.nio.file.Files import java.nio.file.Path +import java.nio.file.Paths internal const val FAKE_ROOT_DIR = "/fake-directory" @@ -49,3 +50,13 @@ internal fun parseTypeUniverseString(topUnvierseText: String): TypeUniverse { return parseMainTypeUniverse(rootPath, listOf()) } + + +internal val MAIN_DOMAINS_DIR = Paths.get("test-domains").toAbsolutePath() +internal val ROOT_A = MAIN_DOMAINS_DIR.resolve("root_a").toAbsolutePath() +internal val ROOT_B = MAIN_DOMAINS_DIR.resolve("root_b").toAbsolutePath() + +internal fun parseWithTestRoots(universeFile: String): TypeUniverse { + val includeSearchRoots = listOf(ROOT_A, ROOT_B) + return parseMainTypeUniverse(Paths.get(universeFile), includeSearchRoots) +} From 94cdb51c4ea487d37705866a058057cbccaa32e7 Mon Sep 17 00:00:00 2001 From: David Lurton Date: Wed, 4 Aug 2021 16:41:58 -0700 Subject: [PATCH 20/22] add test for when files of the same exist in multiple -I search roots. --- pig/test-domains/README.md | 28 +++++++++++-------- pig/test-domains/main.ion | 1 + pig/test-domains/root_a/same_file_name.ion | 4 +++ pig/test-domains/root_b/same_file_name.ion | 4 +++ .../TypeDomainParserIncludeFileTests.kt | 23 +++++++-------- 5 files changed, 37 insertions(+), 23 deletions(-) create mode 100644 pig/test-domains/root_a/same_file_name.ion create mode 100644 pig/test-domains/root_b/same_file_name.ion diff --git a/pig/test-domains/README.md b/pig/test-domains/README.md index 62e7eda..69fcc0d 100644 --- a/pig/test-domains/README.md +++ b/pig/test-domains/README.md @@ -8,26 +8,30 @@ The following concerns of `IncludeCycleHandler`, `IncludeResolver`, and `TypeDom - Aboslute and relative paths are respected during include resolution. - Searching across multiple search roots works. -Also, the concern of the `TypeDomainSemanticChecker` is tested as well: +Also, a concern of the `TypeDomainSemanticChecker` is tested as well: - Duplicate domains names are detected even if they are defined in different files. - NOTE: this is a concern of the semantic checker! +The file structure here is: +``` ├── duplicate_domains.ion used by `TypeDomainSemanticCheckerTests` ├── include-missing-absolute.ion used by parser error handling tests and not included by main.ion ├── include-missing-relative.ion used by parser error handling tests and not included by main.ion -├── main.ion includes sibling-of-main.ion, root_a/dir_x/universe_a.ion, root_b/dir_z/circular_universe_c.ion +├── main.ion includes directly or indirectly all other files (unless noted otherwise) ├── root_a -│ └── dir_x -│ ├── circular_universe_c.ion includes circular_universe_d.ion -│ ├── circular_universe_d.ion includes circular_universe_c.ion -│ ├── first_duplicated_domain_name.ion defines domain_dup -│ ├── include_b.ion includes . ./dir_a/universe_b -│ └── universe_a.ion includes include_b.ion +│ ├── dir_x +│ │ ├── circular_universe_c.ion includes circular_universe_d.ion +│ │ ├── circular_universe_d.ion includes circular_universe_c.ion +│ │ ├── first_duplicated_domain_name.ion defines domain_dup +│ │ ├── include_b.ion includes . ./dir_a/universe_b +│ │ └── universe_a.ion includes include_b.ion +│ └── same-file-name.ion has same file name as root_b/same-file-name.ion (search stops here) ├── root_b │ └── dir_z -│ ├── second_duplicated_domain_name.ion defines domain_dup -│ └── universe_b.ion includes nothing +│ │ ├── second_duplicated_domain_name.ion defines domain_dup +│ │ └── universe_b.ion includes nothing│ +│ └── same-file-name.ion has same file name as root_a/same-file-name.ion (but this one is ignored) └── sibling-of-main.ion - +``` + \ No newline at end of file diff --git a/pig/test-domains/main.ion b/pig/test-domains/main.ion index b37ee26..f96ad34 100644 --- a/pig/test-domains/main.ion +++ b/pig/test-domains/main.ion @@ -3,3 +3,4 @@ (include_file "/sibling-of-main.ion") (include_file "/dir_x/universe_a.ion") (include_file "/dir_x/circular_universe_c.ion") +(include_file "/same_file_name.ion") diff --git a/pig/test-domains/root_a/same_file_name.ion b/pig/test-domains/root_a/same_file_name.ion new file mode 100644 index 0000000..058937e --- /dev/null +++ b/pig/test-domains/root_a/same_file_name.ion @@ -0,0 +1,4 @@ +// Another file of the same name resides in `root_b`, which comes later in the `-I` include search order. That file +// should be ignored and this file should be included. +(define domain_f (domain (product a))) + diff --git a/pig/test-domains/root_b/same_file_name.ion b/pig/test-domains/root_b/same_file_name.ion new file mode 100644 index 0000000..16d8df1 --- /dev/null +++ b/pig/test-domains/root_b/same_file_name.ion @@ -0,0 +1,4 @@ +// Another file of the same name resides in `root_a`, which comes earlier in the `-I` include search order. That file +// should be included and this one should be ignored. +(define this_domain_should_not_be_included (domain (product a))) + diff --git a/pig/test/org/partiql/pig/domain/TypeDomainParserIncludeFileTests.kt b/pig/test/org/partiql/pig/domain/TypeDomainParserIncludeFileTests.kt index 067babb..927092f 100644 --- a/pig/test/org/partiql/pig/domain/TypeDomainParserIncludeFileTests.kt +++ b/pig/test/org/partiql/pig/domain/TypeDomainParserIncludeFileTests.kt @@ -20,10 +20,8 @@ import org.junit.jupiter.api.Assertions.assertEquals import org.junit.jupiter.api.Test import org.junit.jupiter.api.assertThrows import org.partiql.pig.domain.model.TypeDomain -import org.partiql.pig.domain.model.TypeUniverse import org.partiql.pig.domain.parser.ParserErrorContext import org.partiql.pig.domain.parser.SourceLocation -import org.partiql.pig.domain.parser.parseMainTypeUniverse import org.partiql.pig.errors.PigError import org.partiql.pig.errors.PigException import org.partiql.pig.util.MAIN_DOMAINS_DIR @@ -31,7 +29,6 @@ import org.partiql.pig.util.ROOT_A import org.partiql.pig.util.ROOT_B import org.partiql.pig.util.parseWithTestRoots import java.nio.file.Paths -import kotlin.test.assertTrue class TypeDomainParserIncludeFileTests { @Test @@ -39,17 +36,21 @@ class TypeDomainParserIncludeFileTests { val universe = parseWithTestRoots("test-domains/main.ion") val allDomains = universe.statements.filterIsInstance() - // If 5 domains are loaded correctly, then we deal with relative paths and circular references correctly. + // If the following 6 domains are loaded, then we deal with relative paths and circular references correctly. // see documentation at top of test-domains/main.ion - assertEquals(5, allDomains.size) - assertTrue(allDomains.any { it.tag == "domain_a" }) - assertTrue(allDomains.any { it.tag == "domain_b" }) - assertTrue(allDomains.any { it.tag == "domain_c" }) - assertTrue(allDomains.any { it.tag == "domain_d" }) - assertTrue(allDomains.any { it.tag == "domain_s" }) + assertEquals( + setOf( + "domain_a", + "domain_b", + "domain_c", + "domain_d", + "domain_f", + "domain_s" + ), + allDomains.map { it.tag }.toSet() + ) } - @Test fun `include sad case - missing include - relative path`() { val includeeFile = "does-not-exist.ion" From 2bef7be4b1b05c20efe804c7425d6be147a9363e Mon Sep 17 00:00:00 2001 From: David Lurton Date: Wed, 4 Aug 2021 16:42:03 -0700 Subject: [PATCH 21/22] Remove unused property --- .../partiql/pig/domain/include/InvalidIncludePathException.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pig/src/org/partiql/pig/domain/include/InvalidIncludePathException.kt b/pig/src/org/partiql/pig/domain/include/InvalidIncludePathException.kt index c3740b5..81f10e6 100644 --- a/pig/src/org/partiql/pig/domain/include/InvalidIncludePathException.kt +++ b/pig/src/org/partiql/pig/domain/include/InvalidIncludePathException.kt @@ -20,6 +20,6 @@ package org.partiql.pig.domain.include * * Search roots are normally specified on the command-line. */ -class InvalidIncludePathException(val invalidIncludePath: String) +class InvalidIncludePathException(invalidIncludePath: String) : Exception("Specified include path '$invalidIncludePath' does not exist or is not a directory.") From 6b460e9d1c4213a015dc920dd80ca1551361082f Mon Sep 17 00:00:00 2001 From: David Lurton Date: Thu, 12 Aug 2021 20:19:49 -0700 Subject: [PATCH 22/22] Disallow include paths starting with '/' --- .../pig/domain/parser/ParserErrorContext.kt | 3 +++ .../pig/domain/parser/TypeDomainParser.kt | 17 +++++++----- .../include/IncludeCycleHandler.kt | 2 +- .../include/IncludeResolutionException.kt | 2 +- .../{ => parser}/include/IncludeResolver.kt | 17 ++++-------- .../{ => parser}/include/InputSource.kt | 4 +-- .../include/InvalidIncludePathException.kt | 2 +- pig/src/org/partiql/pig/main.kt | 2 +- pig/test-domains/include-missing-relative.ion | 4 --- ...ssing-absolute.ion => include-missing.ion} | 2 +- pig/test-domains/main.ion | 8 +++--- pig/test-domains/root_a/dir_x/include_b.ion | 2 +- .../TypeDomainParserIncludeFileTests.kt | 24 ++++------------- .../{ => parser}/TypeDomainParserTests.kt | 3 ++- .../parser}/include/IncludeResolverTests.kt | 27 ++++++++----------- .../include}/TypeDomainParserErrorsTest.kt | 8 +++--- 16 files changed, 54 insertions(+), 73 deletions(-) rename pig/src/org/partiql/pig/domain/{ => parser}/include/IncludeCycleHandler.kt (98%) rename pig/src/org/partiql/pig/domain/{ => parser}/include/IncludeResolutionException.kt (95%) rename pig/src/org/partiql/pig/domain/{ => parser}/include/IncludeResolver.kt (81%) rename pig/src/org/partiql/pig/domain/{ => parser}/include/InputSource.kt (93%) rename pig/src/org/partiql/pig/domain/{ => parser}/include/InvalidIncludePathException.kt (95%) delete mode 100644 pig/test-domains/include-missing-relative.ion rename pig/test-domains/{include-missing-absolute.ion => include-missing.ion} (63%) rename pig/test/org/partiql/pig/domain/{ => parser}/TypeDomainParserIncludeFileTests.kt (77%) rename pig/test/org/partiql/pig/domain/{ => parser}/TypeDomainParserTests.kt (97%) rename pig/test/org/partiql/pig/{ => domain/parser}/include/IncludeResolverTests.kt (75%) rename pig/test/org/partiql/pig/domain/{ => parser/include}/TypeDomainParserErrorsTest.kt (95%) diff --git a/pig/src/org/partiql/pig/domain/parser/ParserErrorContext.kt b/pig/src/org/partiql/pig/domain/parser/ParserErrorContext.kt index bd65d68..8dcbe10 100644 --- a/pig/src/org/partiql/pig/domain/parser/ParserErrorContext.kt +++ b/pig/src/org/partiql/pig/domain/parser/ParserErrorContext.kt @@ -69,6 +69,9 @@ sealed class ParserErrorContext(val msgFormatter: () -> String): ErrorContext { object IncludeFilePathContainsParentDirectory : ParserErrorContext({ "include_file path contained parent directory, i.e. \"..\"" }) + object IncludeFilePathMustNotStartWithRoot + : ParserErrorContext({ "include_file path must not start with '/'" }) + data class ExpectedTypeReferenceArityTag(val tag: String) : ParserErrorContext({ "Expected '*' or '?' but found '$tag'"}) diff --git a/pig/src/org/partiql/pig/domain/parser/TypeDomainParser.kt b/pig/src/org/partiql/pig/domain/parser/TypeDomainParser.kt index e6ec7a3..6598f3c 100644 --- a/pig/src/org/partiql/pig/domain/parser/TypeDomainParser.kt +++ b/pig/src/org/partiql/pig/domain/parser/TypeDomainParser.kt @@ -32,10 +32,10 @@ import com.amazon.ionelement.api.location import com.amazon.ionelement.api.metaContainerOf import com.amazon.ionelement.api.tag import com.amazon.ionelement.api.tail -import org.partiql.pig.domain.include.IncludeCycleHandler -import org.partiql.pig.domain.include.IncludeResolutionException -import org.partiql.pig.domain.include.IncludeResolver -import org.partiql.pig.domain.include.InputSource +import org.partiql.pig.domain.parser.include.IncludeCycleHandler +import org.partiql.pig.domain.parser.include.IncludeResolutionException +import org.partiql.pig.domain.parser.include.IncludeResolver +import org.partiql.pig.domain.parser.include.InputSource import org.partiql.pig.domain.model.Arity import org.partiql.pig.domain.model.DataType import org.partiql.pig.domain.model.NamedElement @@ -137,17 +137,22 @@ internal class TypeUniverseParser( private fun parseIncludeFile(sexp: SexpElement): List { requireArityForTag(sexp, 1) val includeePath = sexp.tail.single().asString() + val includeePathText = includeePath.textValue.trim() - includeePath.textValue.forEach { + includeePathText.forEach { if (!isValidPathChar(it)) { parseError(includeePath, ParserErrorContext.IncludeFilePathContainsIllegalCharacter(it)) } } - if(includeePath.textValue.contains("..")) { + if(includeePathText.contains("..")) { parseError(includeePath, ParserErrorContext.IncludeFilePathContainsParentDirectory) } + if(includeePathText.startsWith('/')) { + parseError(includeePath, ParserErrorContext.IncludeFilePathMustNotStartWithRoot) + } + return try { includeCycleHandler.parseIncludedTypeUniverse(includeePath.textValue, source.absolutePath) } catch(ex: IncludeResolutionException) { diff --git a/pig/src/org/partiql/pig/domain/include/IncludeCycleHandler.kt b/pig/src/org/partiql/pig/domain/parser/include/IncludeCycleHandler.kt similarity index 98% rename from pig/src/org/partiql/pig/domain/include/IncludeCycleHandler.kt rename to pig/src/org/partiql/pig/domain/parser/include/IncludeCycleHandler.kt index 35af71e..a0fb3c6 100644 --- a/pig/src/org/partiql/pig/domain/include/IncludeCycleHandler.kt +++ b/pig/src/org/partiql/pig/domain/parser/include/IncludeCycleHandler.kt @@ -13,7 +13,7 @@ * permissions and limitations under the License. */ -package org.partiql.pig.domain.include +package org.partiql.pig.domain.parser.include import org.partiql.pig.domain.model.Statement import org.partiql.pig.domain.parser.TypeUniverseParser diff --git a/pig/src/org/partiql/pig/domain/include/IncludeResolutionException.kt b/pig/src/org/partiql/pig/domain/parser/include/IncludeResolutionException.kt similarity index 95% rename from pig/src/org/partiql/pig/domain/include/IncludeResolutionException.kt rename to pig/src/org/partiql/pig/domain/parser/include/IncludeResolutionException.kt index 38defb6..b25239b 100644 --- a/pig/src/org/partiql/pig/domain/include/IncludeResolutionException.kt +++ b/pig/src/org/partiql/pig/domain/parser/include/IncludeResolutionException.kt @@ -13,7 +13,7 @@ * permissions and limitations under the License. */ -package org.partiql.pig.domain.include +package org.partiql.pig.domain.parser.include import java.lang.Exception diff --git a/pig/src/org/partiql/pig/domain/include/IncludeResolver.kt b/pig/src/org/partiql/pig/domain/parser/include/IncludeResolver.kt similarity index 81% rename from pig/src/org/partiql/pig/domain/include/IncludeResolver.kt rename to pig/src/org/partiql/pig/domain/parser/include/IncludeResolver.kt index 5dc8592..856d9a0 100644 --- a/pig/src/org/partiql/pig/domain/include/IncludeResolver.kt +++ b/pig/src/org/partiql/pig/domain/parser/include/IncludeResolver.kt @@ -13,7 +13,7 @@ * permissions and limitations under the License. */ -package org.partiql.pig.domain.include +package org.partiql.pig.domain.parser.include import java.nio.file.FileSystem import java.nio.file.FileSystems @@ -65,23 +65,17 @@ internal class IncludeResolver( fun resolve(includeeFile: Path, includerFile: Path): Path { val normalizedIncluder = includerFile.normalize().toAbsolutePath() - // Determine list of all possible search roots, excluding the includer's parent directory if - // the includee is an absolute path. + // Determine list of all possible search roots val includerParentDir = normalizedIncluder.parent - val searchPaths = listOfNotNull( - includerParentDir.takeIf { !includeeFile.isAbsolute }, - *searchDirectories - ).distinct() + + val searchPaths = listOf(includerParentDir, *searchDirectories).distinct() // distinct is needed because duplicate entries can arise in this list, for instance if the - // includer's parent directory is the same as a include directory. Primarily this is needed to + // includer's parent directory is the same as an include directory. Primarily this is needed to // prevent the directory from appearing twice in the error messages we display when we can't // find an include file. val possibleIncludeeFiles = searchPaths .map { - // Truncates / at the beginning of an absolute path. This forces absolute paths of includees - // to be relative to the include paths specified on the command-line and prevents them from being relative - // to the includer. val appendPath = when { includeeFile.isAbsolute -> includeeFile.toString().substring(1) else -> includeeFile.toString() @@ -92,7 +86,6 @@ internal class IncludeResolver( // The resolved file is the first one that exists. return possibleIncludeeFiles.firstOrNull { Files.exists(it) } - // TODO: can we used a algebraic type here instead of an exception? ?: throw IncludeResolutionException( inputFilePathString = includeeFile.toString(), consideredFilePaths = possibleIncludeeFiles.map { "$it" }) diff --git a/pig/src/org/partiql/pig/domain/include/InputSource.kt b/pig/src/org/partiql/pig/domain/parser/include/InputSource.kt similarity index 93% rename from pig/src/org/partiql/pig/domain/include/InputSource.kt rename to pig/src/org/partiql/pig/domain/parser/include/InputSource.kt index da27a32..b29da7f 100644 --- a/pig/src/org/partiql/pig/domain/include/InputSource.kt +++ b/pig/src/org/partiql/pig/domain/parser/include/InputSource.kt @@ -13,10 +13,10 @@ * permissions and limitations under the License. */ -package org.partiql.pig.domain.include +package org.partiql.pig.domain.parser.include import java.io.InputStream import java.nio.file.Path -/** Provides a convenient way to associate an inputStream with it's path. */ +/** Provides a convenient way to associate an inputStream with its path. */ class InputSource(val absolutePath: Path, val inputStream: InputStream) diff --git a/pig/src/org/partiql/pig/domain/include/InvalidIncludePathException.kt b/pig/src/org/partiql/pig/domain/parser/include/InvalidIncludePathException.kt similarity index 95% rename from pig/src/org/partiql/pig/domain/include/InvalidIncludePathException.kt rename to pig/src/org/partiql/pig/domain/parser/include/InvalidIncludePathException.kt index 81f10e6..d191050 100644 --- a/pig/src/org/partiql/pig/domain/include/InvalidIncludePathException.kt +++ b/pig/src/org/partiql/pig/domain/parser/include/InvalidIncludePathException.kt @@ -13,7 +13,7 @@ * permissions and limitations under the License. */ -package org.partiql.pig.domain.include +package org.partiql.pig.domain.parser.include /** * Thrown by [IncludeResolver] to indicate that one of the search roots passed to it is invalid. diff --git a/pig/src/org/partiql/pig/main.kt b/pig/src/org/partiql/pig/main.kt index 5133f17..577561d 100644 --- a/pig/src/org/partiql/pig/main.kt +++ b/pig/src/org/partiql/pig/main.kt @@ -18,7 +18,7 @@ package org.partiql.pig import org.partiql.pig.cmdline.Command import org.partiql.pig.cmdline.CommandLineParser import org.partiql.pig.cmdline.TargetLanguage -import org.partiql.pig.domain.include.InvalidIncludePathException +import org.partiql.pig.domain.parser.include.InvalidIncludePathException import org.partiql.pig.errors.PigException import org.partiql.pig.domain.model.TypeUniverse import org.partiql.pig.domain.parser.parseMainTypeUniverse diff --git a/pig/test-domains/include-missing-relative.ion b/pig/test-domains/include-missing-relative.ion deleted file mode 100644 index f02fa17..0000000 --- a/pig/test-domains/include-missing-relative.ion +++ /dev/null @@ -1,4 +0,0 @@ - -// This file attempts to include a missing file specified with a relative (not prefixed with '/') path - -(include_file "does-not-exist.ion") diff --git a/pig/test-domains/include-missing-absolute.ion b/pig/test-domains/include-missing.ion similarity index 63% rename from pig/test-domains/include-missing-absolute.ion rename to pig/test-domains/include-missing.ion index 5598a82..e980333 100644 --- a/pig/test-domains/include-missing-absolute.ion +++ b/pig/test-domains/include-missing.ion @@ -1,4 +1,4 @@ // This file attempts to include a missing file using an absolute path. -(include_file "/dir_x/does-not-exist.ion") +(include_file "dir_x/does-not-exist.ion") diff --git a/pig/test-domains/main.ion b/pig/test-domains/main.ion index f96ad34..7f7438f 100644 --- a/pig/test-domains/main.ion +++ b/pig/test-domains/main.ion @@ -1,6 +1,6 @@ -(include_file "/sibling-of-main.ion") -(include_file "/dir_x/universe_a.ion") -(include_file "/dir_x/circular_universe_c.ion") -(include_file "/same_file_name.ion") +(include_file "sibling-of-main.ion") +(include_file "dir_x/universe_a.ion") +(include_file "dir_x/circular_universe_c.ion") +(include_file "same_file_name.ion") diff --git a/pig/test-domains/root_a/dir_x/include_b.ion b/pig/test-domains/root_a/dir_x/include_b.ion index 5fc6f65..11553e2 100644 --- a/pig/test-domains/root_a/dir_x/include_b.ion +++ b/pig/test-domains/root_a/dir_x/include_b.ion @@ -1,3 +1,3 @@ // include file with an aboslute path -(include_file "/dir_z/universe_b.ion") +(include_file "dir_z/universe_b.ion") diff --git a/pig/test/org/partiql/pig/domain/TypeDomainParserIncludeFileTests.kt b/pig/test/org/partiql/pig/domain/parser/TypeDomainParserIncludeFileTests.kt similarity index 77% rename from pig/test/org/partiql/pig/domain/TypeDomainParserIncludeFileTests.kt rename to pig/test/org/partiql/pig/domain/parser/TypeDomainParserIncludeFileTests.kt index 927092f..259b259 100644 --- a/pig/test/org/partiql/pig/domain/TypeDomainParserIncludeFileTests.kt +++ b/pig/test/org/partiql/pig/domain/parser/TypeDomainParserIncludeFileTests.kt @@ -13,15 +13,13 @@ * permissions and limitations under the License. */ -package org.partiql.pig.domain +package org.partiql.pig.domain.parser import com.amazon.ionelement.api.IonTextLocation import org.junit.jupiter.api.Assertions.assertEquals import org.junit.jupiter.api.Test import org.junit.jupiter.api.assertThrows import org.partiql.pig.domain.model.TypeDomain -import org.partiql.pig.domain.parser.ParserErrorContext -import org.partiql.pig.domain.parser.SourceLocation import org.partiql.pig.errors.PigError import org.partiql.pig.errors.PigException import org.partiql.pig.util.MAIN_DOMAINS_DIR @@ -51,28 +49,16 @@ class TypeDomainParserIncludeFileTests { ) } - @Test - fun `include sad case - missing include - relative path`() { - val includeeFile = "does-not-exist.ion" - testMissingInclude( - mainUniverseFile = "test-domains/include-missing-relative.ion", - includeeFile = includeeFile, - pathsSearched = listOf( - "${MAIN_DOMAINS_DIR}/$includeeFile", - "${ROOT_A.resolve("does-not-exist.ion").toAbsolutePath()}", - "${ROOT_B.resolve("does-not-exist.ion").toAbsolutePath()}" - ) - ) - } + @Test fun `include sad case - missing include - absolute path`() { - val includeeFile = "/dir_x/does-not-exist.ion" + val includeeFile = "dir_x/does-not-exist.ion" testMissingInclude( - mainUniverseFile = "test-domains/include-missing-absolute.ion", + mainUniverseFile = "test-domains/include-missing.ion", includeeFile = includeeFile, pathsSearched = listOf( - "${MAIN_DOMAINS_DIR}$includeeFile", + "${MAIN_DOMAINS_DIR}/$includeeFile", "${ROOT_A.resolve("dir_x/does-not-exist.ion").toAbsolutePath()}", "${ROOT_B.resolve("dir_x/does-not-exist.ion").toAbsolutePath()}" ) diff --git a/pig/test/org/partiql/pig/domain/TypeDomainParserTests.kt b/pig/test/org/partiql/pig/domain/parser/TypeDomainParserTests.kt similarity index 97% rename from pig/test/org/partiql/pig/domain/TypeDomainParserTests.kt rename to pig/test/org/partiql/pig/domain/parser/TypeDomainParserTests.kt index 3a5842f..25ccc14 100644 --- a/pig/test/org/partiql/pig/domain/TypeDomainParserTests.kt +++ b/pig/test/org/partiql/pig/domain/parser/TypeDomainParserTests.kt @@ -13,7 +13,7 @@ * permissions and limitations under the License. */ -package org.partiql.pig.domain +package org.partiql.pig.domain.parser import com.amazon.ionelement.api.IonElementLoaderOptions import com.amazon.ionelement.api.createIonElementLoader @@ -22,6 +22,7 @@ import com.amazon.ionelement.api.ionSymbol import org.junit.jupiter.api.Assertions.assertEquals import org.junit.jupiter.api.Test import org.junit.jupiter.api.assertDoesNotThrow +import org.partiql.pig.domain.toIonElement import org.partiql.pig.util.parseTypeUniverseString class TypeDomainParserTests { diff --git a/pig/test/org/partiql/pig/include/IncludeResolverTests.kt b/pig/test/org/partiql/pig/domain/parser/include/IncludeResolverTests.kt similarity index 75% rename from pig/test/org/partiql/pig/include/IncludeResolverTests.kt rename to pig/test/org/partiql/pig/domain/parser/include/IncludeResolverTests.kt index 89b1e93..17aaa71 100644 --- a/pig/test/org/partiql/pig/include/IncludeResolverTests.kt +++ b/pig/test/org/partiql/pig/domain/parser/include/IncludeResolverTests.kt @@ -13,17 +13,13 @@ * permissions and limitations under the License. */ -package org.partiql.pig.include +package org.partiql.pig.domain.parser.include import org.junit.jupiter.api.Assertions.assertEquals import org.junit.jupiter.api.Test import org.junit.jupiter.api.assertThrows import org.junit.jupiter.params.ParameterizedTest import org.junit.jupiter.params.provider.MethodSource -import org.partiql.pig.domain.include.IncludeResolver -import org.partiql.pig.domain.include.IncludeResolutionException -import org.partiql.pig.domain.include.InvalidIncludePathException -import java.nio.file.FileSystem import java.nio.file.FileSystems import java.nio.file.Path import java.nio.file.Paths @@ -61,20 +57,19 @@ class IncludeResolverTests { fun notFoundCases() = listOf( NotFoundCase( includee = "does-not-exist.ion", - expectedConsdieredPaths = listOf( - // no / at start of includePath, therefore we include the parent directory of the includer - Paths.get("test-domains/does-not-exist.ion").toAbsolutePath().toString(), - Paths.get("test-domains/root_a/does-not-exist.ion").toAbsolutePath().toString(), - Paths.get("test-domains/root_b/does-not-exist.ion").toAbsolutePath().toString() - ) + expectedConsdieredPaths = expectedConsideredPaths ), NotFoundCase( includee = "/does-not-exist.ion", - expectedConsdieredPaths = listOf( - // / at start of includePath, therefore we exclude the parent directory of the includer. - Paths.get("test-domains/root_a/does-not-exist.ion").toAbsolutePath().toString(), - Paths.get("test-domains/root_b/does-not-exist.ion").toAbsolutePath().toString()) - ) + expectedConsdieredPaths = expectedConsideredPaths + ) + ) + + private val expectedConsideredPaths + get() = listOf( + Paths.get("test-domains/does-not-exist.ion").toAbsolutePath().toString(), + Paths.get("test-domains/root_a/does-not-exist.ion").toAbsolutePath().toString(), + Paths.get("test-domains/root_b/does-not-exist.ion").toAbsolutePath().toString() ) } diff --git a/pig/test/org/partiql/pig/domain/TypeDomainParserErrorsTest.kt b/pig/test/org/partiql/pig/domain/parser/include/TypeDomainParserErrorsTest.kt similarity index 95% rename from pig/test/org/partiql/pig/domain/TypeDomainParserErrorsTest.kt rename to pig/test/org/partiql/pig/domain/parser/include/TypeDomainParserErrorsTest.kt index 8fc5d80..615957c 100644 --- a/pig/test/org/partiql/pig/domain/TypeDomainParserErrorsTest.kt +++ b/pig/test/org/partiql/pig/domain/parser/include/TypeDomainParserErrorsTest.kt @@ -13,7 +13,7 @@ * permissions and limitations under the License. */ -package org.partiql.pig.domain +package org.partiql.pig.domain.parser.include import com.amazon.ionelement.api.ElementType import com.amazon.ionelement.api.IonElementLoaderException @@ -22,7 +22,9 @@ import org.junit.jupiter.api.Assertions.assertNotNull import org.junit.jupiter.api.assertThrows import org.junit.jupiter.params.ParameterizedTest import org.junit.jupiter.params.provider.MethodSource +import org.partiql.pig.domain.makeErr import org.partiql.pig.domain.parser.ParserErrorContext +import org.partiql.pig.domain.toIonElement import org.partiql.pig.errors.PigError import org.partiql.pig.errors.PigException import org.partiql.pig.util.makeFakePath @@ -116,10 +118,10 @@ class TypeDomainParserErrorsTest { listOf(makeFakePath("some-non-existing-file.ion")) ))), TestCase( - "(include_file \"/some-sub-dir/some-non-existing-file.ion\")", + "(include_file \"some-sub-dir/some-non-existing-file.ion\")", makeErr(1, 1, ParserErrorContext.IncludeFileNotFound( - "/some-sub-dir/some-non-existing-file.ion", + "some-sub-dir/some-non-existing-file.ion", listOf(makeFakePath("some-sub-dir/some-non-existing-file.ion")) ))), TestCase(