diff --git a/ion-tests b/ion-tests index fd84d44d1..c83270f08 160000 --- a/ion-tests +++ b/ion-tests @@ -1 +1 @@ -Subproject commit fd84d44d1e2879be4800115f7b5dfa663d1d532c +Subproject commit c83270f0842ce67e2eb4237998c5f52e9926cca2 diff --git a/src/main/java/com/amazon/ion/impl/IonCursorBinary.java b/src/main/java/com/amazon/ion/impl/IonCursorBinary.java index 09f76c519..96753cb2c 100644 --- a/src/main/java/com/amazon/ion/impl/IonCursorBinary.java +++ b/src/main/java/com/amazon/ion/impl/IonCursorBinary.java @@ -2348,7 +2348,7 @@ private void uncheckedReadMacroInvocationHeader(IonTypeID valueTid, Marker marke private boolean uncheckedReadHeader(final int typeIdByte, final boolean isAnnotated, final Marker markerToSet) { IonTypeID valueTid = typeIds[typeIdByte]; if (!valueTid.isValid) { - throw new IonException("Invalid type ID."); + throw new IonException("Invalid type ID: " + valueTid.theByte); } else if (valueTid.type == IonTypeID.ION_TYPE_ANNOTATION_WRAPPER) { if (isAnnotated) { throw new IonException("Nested annotation wrappers are invalid."); diff --git a/src/main/java/com/amazon/ion/impl/IonReaderContinuableCoreBinary.java b/src/main/java/com/amazon/ion/impl/IonReaderContinuableCoreBinary.java index 58bc5abd1..f9172daad 100644 --- a/src/main/java/com/amazon/ion/impl/IonReaderContinuableCoreBinary.java +++ b/src/main/java/com/amazon/ion/impl/IonReaderContinuableCoreBinary.java @@ -1638,8 +1638,7 @@ protected void readParameter(Macro.Parameter parameter, long parameterPresence, case OneOrMore: if (parameterPresence == PresenceBitmap.EXPRESSION) { readSingleExpression(parameter, expressions); - } - if (parameterPresence == PresenceBitmap.GROUP) { + } else if (parameterPresence == PresenceBitmap.GROUP) { readGroupExpression(parameter, expressions, false); } else { throw new IonException(String.format( diff --git a/src/main/java/com/amazon/ion/impl/IonReaderTextSystemX.java b/src/main/java/com/amazon/ion/impl/IonReaderTextSystemX.java index efe4221a2..d5115cfc6 100644 --- a/src/main/java/com/amazon/ion/impl/IonReaderTextSystemX.java +++ b/src/main/java/com/amazon/ion/impl/IonReaderTextSystemX.java @@ -1237,6 +1237,9 @@ private class TextEExpressionArgsReader extends EExpressionArgsReader { @Override protected void readParameter(Macro.Parameter parameter, long parameterPresence, List expressions, boolean isTrailing) { if (IonReaderTextSystemX.this.nextRaw() == null) { + // Add an empty expression group if nothing present. + int index = expressions.size() + 1; + expressions.add(new Expression.ExpressionGroup(index, index)); return; } readValueAsExpression(isTrailing && parameter.getCardinality().canBeMulti, expressions); diff --git a/src/main/java/com/amazon/ion/impl/bin/IonManagedWriter_1_1.kt b/src/main/java/com/amazon/ion/impl/bin/IonManagedWriter_1_1.kt index 8b4596a0e..c3caf1d2e 100644 --- a/src/main/java/com/amazon/ion/impl/bin/IonManagedWriter_1_1.kt +++ b/src/main/java/com/amazon/ion/impl/bin/IonManagedWriter_1_1.kt @@ -34,6 +34,8 @@ internal class IonManagedWriter_1_1( private val onClose: () -> Unit, ) : _Private_IonWriter, MacroAwareIonWriter { + internal fun getRawUserWriter(): IonRawWriter_1_1 = userData + companion object { private val ION_VERSION_MARKER_REGEX = Regex("^\\\$ion_\\d+_\\d+$") diff --git a/src/main/java/com/amazon/ion/impl/macro/Expression.kt b/src/main/java/com/amazon/ion/impl/macro/Expression.kt index 79263eff0..555214f7b 100644 --- a/src/main/java/com/amazon/ion/impl/macro/Expression.kt +++ b/src/main/java/com/amazon/ion/impl/macro/Expression.kt @@ -95,18 +95,21 @@ sealed interface Expression { sealed interface IntValue : DataModelValue { val bigIntegerValue: BigInteger + val longValue: Long } data class LongIntValue(override val annotations: List = emptyList(), val value: Long) : IntValue { override val type: IonType get() = IonType.INT override fun withAnnotations(annotations: List) = copy(annotations = annotations) override val bigIntegerValue: BigInteger get() = BigInteger.valueOf(value) + override val longValue: Long get() = value } data class BigIntValue(override val annotations: List = emptyList(), val value: BigInteger) : IntValue { override val type: IonType get() = IonType.INT override fun withAnnotations(annotations: List) = copy(annotations = annotations) override val bigIntegerValue: BigInteger get() = value + override val longValue: Long get() = value.longValueExact() } data class FloatValue(override val annotations: List = emptyList(), val value: Double) : DataModelValue { diff --git a/src/main/java/com/amazon/ion/impl/macro/MacroCompiler.kt b/src/main/java/com/amazon/ion/impl/macro/MacroCompiler.kt index 9e2bca321..f64da15a6 100644 --- a/src/main/java/com/amazon/ion/impl/macro/MacroCompiler.kt +++ b/src/main/java/com/amazon/ion/impl/macro/MacroCompiler.kt @@ -258,7 +258,7 @@ internal class MacroCompiler( } else -> throw IonException("macro invocation must start with an id (int) or identifier (symbol); found ${encodingType() ?: "nothing"}\"") } - val m = if (isQualifiedSystemMacro) SystemMacro.get(macroRef) else getMacro(macroRef) + val m = if (isQualifiedSystemMacro) SystemMacro.getMacroOrSpecialForm(macroRef) else getMacro(macroRef) return m ?: throw IonException("Unrecognized macro: $macroRef") } diff --git a/src/main/java/com/amazon/ion/impl/macro/MacroEvaluator.kt b/src/main/java/com/amazon/ion/impl/macro/MacroEvaluator.kt index b0e167645..46ca5e275 100644 --- a/src/main/java/com/amazon/ion/impl/macro/MacroEvaluator.kt +++ b/src/main/java/com/amazon/ion/impl/macro/MacroEvaluator.kt @@ -2,13 +2,13 @@ // SPDX-License-Identifier: Apache-2.0 package com.amazon.ion.impl.macro -import com.amazon.ion.IonException -import com.amazon.ion.SymbolToken +import com.amazon.ion.* import com.amazon.ion.impl._Private_RecyclingStack import com.amazon.ion.impl._Private_Utils.newSymbolToken import com.amazon.ion.impl.macro.Expression.* import java.io.ByteArrayOutputStream import java.math.BigDecimal +import java.math.BigInteger /** * Evaluates an EExpression from a List of [EExpressionBodyExpression] and the [TemplateBodyExpression]s @@ -209,6 +209,119 @@ class MacroEvaluator { } } + private object MakeTimestampExpander : Expander { + private fun readOptionalIntArg( + signatureIndex: Int, + expansionInfo: ExpansionInfo, + macroEvaluator: MacroEvaluator + ): Int? { + if (expansionInfo.i == expansionInfo.endExclusive) return null + val parameterName = SystemMacro.MakeTimestamp.signature[signatureIndex].variableName + val arg = readZeroOrOneExpandedArgumentValues(expansionInfo, macroEvaluator, parameterName) + return arg?.let { + it as? IntValue ?: throw IonException("$parameterName must be an integer") + it.longValue.toInt() + } + } + + override fun nextExpression(expansionInfo: ExpansionInfo, macroEvaluator: MacroEvaluator): Expression { + val year = readExactlyOneExpandedArgumentValue(expansionInfo, macroEvaluator, SystemMacro.MakeTimestamp.signature[0].variableName) + .let { it as? IntValue ?: throw IonException("year must be an integer") } + .longValue.toInt() + val month = readOptionalIntArg(1, expansionInfo, macroEvaluator) + val day = readOptionalIntArg(2, expansionInfo, macroEvaluator) + val hour = readOptionalIntArg(3, expansionInfo, macroEvaluator) + val minute = readOptionalIntArg(4, expansionInfo, macroEvaluator) + val second = if (expansionInfo.i == expansionInfo.endExclusive) { + null + } else when (val arg = readZeroOrOneExpandedArgumentValues(expansionInfo, macroEvaluator, SystemMacro.MakeTimestamp.signature[5].variableName)) { + null -> null + is DecimalValue -> arg.value + is IntValue -> arg.longValue.toBigDecimal() + else -> throw IonException("second must be a decimal") + } + val offsetMinutes = readOptionalIntArg(6, expansionInfo, macroEvaluator) + + try { + val ts = if (second != null) { + month ?: throw IonException("make_timestamp: month is required when second is present") + day ?: throw IonException("make_timestamp: day is required when second is present") + hour ?: throw IonException("make_timestamp: hour is required when second is present") + minute ?: throw IonException("make_timestamp: minute is required when second is present") + Timestamp.forSecond(year, month, day, hour, minute, second, offsetMinutes) + } else if (minute != null) { + month ?: throw IonException("make_timestamp: month is required when minute is present") + day ?: throw IonException("make_timestamp: day is required when minute is present") + hour ?: throw IonException("make_timestamp: hour is required when minute is present") + Timestamp.forMinute(year, month, day, hour, minute, offsetMinutes) + } else if (hour != null) { + throw IonException("make_timestamp: minute is required when hour is present") + } else { + if (offsetMinutes != null) throw IonException("make_timestamp: offset_minutes is prohibited when hours and minute are not present") + if (day != null) { + month ?: throw IonException("make_timestamp: month is required when day is present") + Timestamp.forDay(year, month, day) + } else if (month != null) { + Timestamp.forMonth(year, month) + } else { + Timestamp.forYear(year) + } + } + return TimestampValue(value = ts) + } catch (e: IllegalArgumentException) { + throw IonException(e.message) + } + } + } + + private object SumExpander : Expander { + override fun nextExpression(expansionInfo: ExpansionInfo, macroEvaluator: MacroEvaluator): Expression { + val a = readExactlyOneExpandedArgumentValue(expansionInfo, macroEvaluator, "a") + val b = readExactlyOneExpandedArgumentValue(expansionInfo, macroEvaluator, "b") + if (a !is IntValue || b !is IntValue) throw IonException("operands of sum must be integers") + // TODO: Use LongIntValue when possible. + return BigIntValue(value = a.bigIntegerValue + b.bigIntegerValue) + } + } + + private object DeltaExpander : Expander { + override fun nextExpression(expansionInfo: ExpansionInfo, macroEvaluator: MacroEvaluator): Expression { + // TODO: Optimize to use Long and only fallback to BigInteger if needed. + // TODO: Optimize for lazy evaluation + if (expansionInfo.additionalState == null) { + val position = expansionInfo.i + var runningTotal = BigInteger.ZERO + val values = ArrayDeque() + readExpandedArgumentValues(expansionInfo, macroEvaluator) { + when (it) { + is IntValue -> { + runningTotal += it.bigIntegerValue + values += runningTotal + } + is DataModelValue -> throw IonException("Invalid argument type for 'delta': ${it.type}") + is FieldName -> TODO("Unreachable. We shouldn't be able to get here without first encountering a StructValue.") + } + true // continue expansion + } + + if (values.isEmpty()) { + // Return fake, empty expression group + return ExpressionGroup(position, position) + } + + expansionInfo.additionalState = values + expansionInfo.i = position + } + + val valueQueue = expansionInfo.additionalState as ArrayDeque + val nextValue = valueQueue.removeFirst() + if (valueQueue.isEmpty()) { + expansionInfo.i = expansionInfo.endExclusive + } + return BigIntValue(value = nextValue) + } + } + private enum class IfExpander(private val minInclusive: Int, private val maxExclusive: Int) : Expander { IF_NONE(0, 1), IF_SOME(1, -1), @@ -254,11 +367,10 @@ class MacroEvaluator { } nExpression.value.intValueExact() } - else -> throw IonException("The first argument of repeat must be a positive integer") + else -> throw IonException("The first argument of repeat must be a non-negative integer") } - if (iterationsRemaining <= 0) { - // TODO: Confirm https://github.com/amazon-ion/ion-docs/issues/350 - throw IonException("The first argument of repeat must be a positive integer") + if (iterationsRemaining < 0) { + throw IonException("The first argument of repeat must be a non-negative integer") } // Decrement because we're starting the first iteration right away. iterationsRemaining-- @@ -267,14 +379,19 @@ class MacroEvaluator { } override fun nextExpression(expansionInfo: ExpansionInfo, macroEvaluator: MacroEvaluator): Expression { - val repeatsRemaining = expansionInfo.additionalState as? Int + val repeatsRemainingAfterTheCurrentOne = expansionInfo.additionalState as? Int ?: init(expansionInfo, macroEvaluator) + if (repeatsRemainingAfterTheCurrentOne < 0) { + expansionInfo.nextSourceExpression() + return ExpressionGroup(0, 0) + } + val repeatedExpressionIndex = expansionInfo.i val next = readFirstExpandedArgumentValue(expansionInfo, macroEvaluator) - next ?: throw IonException("repeat macro requires at least one value for value parameter") - if (repeatsRemaining > 0) { - expansionInfo.additionalState = repeatsRemaining - 1 + next ?: return ExpressionGroup(0, 0) + if (repeatsRemainingAfterTheCurrentOne > 0) { + expansionInfo.additionalState = repeatsRemainingAfterTheCurrentOne - 1 expansionInfo.i = repeatedExpressionIndex } return next @@ -282,6 +399,7 @@ class MacroEvaluator { } private object MakeFieldExpander : Expander { + // This is wrong! override fun nextExpression(expansionInfo: ExpansionInfo, macroEvaluator: MacroEvaluator): Expression { /** * Uses [ExpansionInfo.additionalState] to track whether the expansion is on the field name or value. @@ -317,7 +435,10 @@ class MacroEvaluator { MakeSymbol(MakeSymbolExpander), MakeBlob(MakeBlobExpander), MakeDecimal(MakeDecimalExpander), + MakeTimestamp(MakeTimestampExpander), MakeField(MakeFieldExpander), + Sum(SumExpander), + Delta(DeltaExpander), IfNone(IfExpander.IF_NONE), IfSome(IfExpander.IF_SOME), IfSingle(IfExpander.IF_SINGLE), @@ -328,9 +449,7 @@ class MacroEvaluator { companion object { @JvmStatic fun forSystemMacro(macro: SystemMacro): ExpansionKind { - return if (macro.body != null) { - TemplateBody - } else when (macro) { + return when (macro) { SystemMacro.None -> Values // "none" takes no args, so we can treat it as an empty "values" expansion SystemMacro.Values -> Values SystemMacro.Annotate -> Annotate @@ -338,13 +457,20 @@ class MacroEvaluator { SystemMacro.MakeSymbol -> MakeSymbol SystemMacro.MakeBlob -> MakeBlob SystemMacro.MakeDecimal -> MakeDecimal + SystemMacro.MakeTimestamp -> MakeTimestamp SystemMacro.IfNone -> IfNone SystemMacro.IfSome -> IfSome SystemMacro.IfSingle -> IfSingle SystemMacro.IfMulti -> IfMulti SystemMacro.Repeat -> Repeat SystemMacro.MakeField -> MakeField - else -> throw IllegalStateException("Unreachable. All other macros have a template body.") + SystemMacro.Sum -> Sum + SystemMacro.Delta -> Delta + else -> if (macro.body != null) { + TemplateBody + } else { + TODO("System macro ${macro.macroName} needs either a template body or a hard-coded expander.") + } } } } diff --git a/src/main/java/com/amazon/ion/impl/macro/ParameterFactory.kt b/src/main/java/com/amazon/ion/impl/macro/ParameterFactory.kt index 33a9ede02..3678ddc52 100644 --- a/src/main/java/com/amazon/ion/impl/macro/ParameterFactory.kt +++ b/src/main/java/com/amazon/ion/impl/macro/ParameterFactory.kt @@ -16,6 +16,4 @@ object ParameterFactory { fun oneToManyTagged(name: String) = Parameter(name, ParameterEncoding.Tagged, ParameterCardinality.OneOrMore) @JvmStatic fun exactlyOneTagged(name: String) = Parameter(name, ParameterEncoding.Tagged, ParameterCardinality.ExactlyOne) - @JvmStatic - fun exactlyOneFlexInt(name: String) = Parameter(name, ParameterEncoding.FlexInt, ParameterCardinality.ExactlyOne) } diff --git a/src/main/java/com/amazon/ion/impl/macro/SystemMacro.kt b/src/main/java/com/amazon/ion/impl/macro/SystemMacro.kt index c73fa4f50..44c7b64e1 100644 --- a/src/main/java/com/amazon/ion/impl/macro/SystemMacro.kt +++ b/src/main/java/com/amazon/ion/impl/macro/SystemMacro.kt @@ -5,9 +5,7 @@ package com.amazon.ion.impl.macro import com.amazon.ion.impl.* import com.amazon.ion.impl.SystemSymbols_1_1.* import com.amazon.ion.impl.macro.ExpressionBuilderDsl.Companion.templateBody -import com.amazon.ion.impl.macro.ParameterFactory.exactlyOneFlexInt import com.amazon.ion.impl.macro.ParameterFactory.exactlyOneTagged -import com.amazon.ion.impl.macro.ParameterFactory.oneToManyTagged import com.amazon.ion.impl.macro.ParameterFactory.zeroOrOneTagged import com.amazon.ion.impl.macro.ParameterFactory.zeroToManyTagged @@ -34,7 +32,22 @@ enum class SystemMacro( MakeString(3, MAKE_STRING, listOf(zeroToManyTagged("text"))), MakeSymbol(4, MAKE_SYMBOL, listOf(zeroToManyTagged("text"))), MakeBlob(5, MAKE_BLOB, listOf(zeroToManyTagged("bytes"))), - MakeDecimal(6, MAKE_DECIMAL, listOf(exactlyOneFlexInt("coefficient"), exactlyOneFlexInt("exponent"))), + MakeDecimal(6, MAKE_DECIMAL, listOf(exactlyOneTagged("coefficient"), exactlyOneTagged("exponent"))), + MakeTimestamp( + 7, MAKE_TIMESTAMP, + listOf( + exactlyOneTagged("year"), + zeroOrOneTagged("month"), + zeroOrOneTagged("day"), + zeroOrOneTagged("hour"), + zeroOrOneTagged("minute"), + zeroOrOneTagged("second"), + zeroOrOneTagged("offset_minutes"), + ) + ), + // TODO: make_list + // TODO: make_sexp + // TODO: make_struct /** * ```ion @@ -176,8 +189,11 @@ enum class SystemMacro( } } ), - - Repeat(17, REPEAT, listOf(exactlyOneTagged("n"), oneToManyTagged("value"))), + // TODO: parse_ion + Repeat(17, REPEAT, listOf(exactlyOneTagged("n"), zeroToManyTagged("value"))), + Delta(18, DELTA, listOf(zeroToManyTagged("deltas"))), + // TODO: flatten + Sum(20, SUM, listOf(exactlyOneTagged("a"), exactlyOneTagged("b"))), Comment(21, META, listOf(zeroToManyTagged("values")), templateBody { macro(None) {} }), MakeField( 22, MAKE_FIELD, @@ -196,8 +212,6 @@ enum class SystemMacro( } } ), - - // TODO: Other system macros ; val macroName: String get() = this.systemSymbol.text @@ -239,7 +253,12 @@ enum class SystemMacro( /** Gets a [SystemMacro] by name, including those which are not in the system table (i.e. special forms) */ @JvmStatic - fun getMacroOrSpecialForm(name: String): SystemMacro? = MACROS_BY_NAME[name] + fun getMacroOrSpecialForm(ref: MacroRef): SystemMacro? { + return when (ref) { + is MacroRef.ById -> get(ref.id) + is MacroRef.ByName -> MACROS_BY_NAME[ref.name] + } + } @JvmStatic val SYSTEM_MACRO_TABLE = this diff --git a/src/test/java/com/amazon/ion/conformance/ConformanceTestRunner.kt b/src/test/java/com/amazon/ion/conformance/ConformanceTestRunner.kt index 15304a22d..a96f8f8f0 100644 --- a/src/test/java/com/amazon/ion/conformance/ConformanceTestRunner.kt +++ b/src/test/java/com/amazon/ion/conformance/ConformanceTestRunner.kt @@ -15,17 +15,19 @@ object DefaultReaderConformanceTests : ConformanceTestRunner( object IncrementalReaderConformanceTests : ConformanceTestRunner( IonReaderBuilder.standard() .withCatalog(ION_CONFORMANCE_TEST_CATALOG) - .withIncrementalReadingEnabled(true) + .withIncrementalReadingEnabled(true), + additionalSkipFilter = { _, testName -> "Incomplete floats signal an error for unexpected EOF" in testName } ) abstract class ConformanceTestRunner( readerBuilder: IonReaderBuilder, - additionalSkipFilter: (File, String) -> Boolean = { _, _ -> true } + /** A predicate that returns `true` iff the test case should be skipped. */ + additionalSkipFilter: (File, String) -> Boolean = { _, _ -> false } ) { private val DEFAULT_SKIP_FILTER: (File, String) -> Boolean = { file, completeTestName -> // `completeTestName` is the complete name of the test — that is all the test descriptions in a particular - // execution path, joined by ", ". (This is how it appears in the JUnit report.) + // execution path, joined by " ". (This is how it appears in the JUnit report.) when { // IonElement can't load $0. TODO: Use IonValue for `produces`, I guess. "$0" in completeTestName -> false @@ -34,8 +36,7 @@ abstract class ConformanceTestRunner( // IonWriter is making it difficult to write invalid data "If no max_id, lack of exact-match must raise an error «then»" in completeTestName -> false // IonCatalog's "best choice" logic is not spec compliant - // TODO—current test name has a typo. Update to correct spelling once ion-tests is fixed. - "When max_id is valid, pad/truncade mismatched or absent SSTs" in completeTestName -> false + "When max_id is valid, pad/truncate mismatched or absent SSTs" in completeTestName -> false // No support for reading `$ion_encoding` directives yet. "conformance/ion_encoding/" in file.absolutePath -> false file.endsWith("local_symtab_imports.ion") -> when { @@ -54,6 +55,59 @@ abstract class ConformanceTestRunner( "If max_id not non-negative int, lack of exact-match must raise an error" in completeTestName -> false else -> true } + + // FIXME: Contains test cases that are out of date, lack descriptions to have more specific exclusions + "eexp/basic_system_macros.ion" in file.absolutePath -> false + "eexp/arg_inlining.ion" in file.absolutePath -> false + + // FIXME: + // 1. Test cases expect a zero-or-one-valued expression group to be valid for ? parameters, implementation disagrees + // 2. One-to-many parameters are not raising an error for an empty expression group. This may need to be + // fixed in the macro evaluator and/or in the reader. + // 3. All other failures for tagless type cases are due to "Encountered an unknown macro address: N" where + // N is the first byte of the macro argument (after any AEB and/or expression group prefixes). + "eexp/binary/argument_encoding.ion" in file.absolutePath -> false + + // FIXME: All failing for reason #3 for argument_encoding.ion + "eexp/binary/tagless_types.ion" in file.absolutePath -> false + + // FIXME: Fails because the encoding context isn't properly populated with the system module/macros + "conformance/system_macros/" in file.absolutePath && + "in binary with a user macro address" in completeTestName -> false + + // FIXME: Timestamp should not allow an offset of +/-1440 + "the offset argument must be less than 1440" in completeTestName -> false + "the offset argument must be greater than -1440" in completeTestName -> false + + // FIXME: Ensure Ion 1.1 symbol tables are properly validated + "add_symbols does not accept null.symbol" in completeTestName -> false + "add_symbols does not accept null.string" in completeTestName -> false + "add_symbols does not accept annotated arguments" in completeTestName -> false + "set_symbols does not accept null.symbol" in completeTestName -> false + "set_symbols does not accept null.string" in completeTestName -> false + "set_symbols does not accept annotated arguments" in completeTestName -> false + + // FIXME: Ensure that the text reader throws if unexpected extra args are encountered + "sum arguments may not be more than two integers" in completeTestName -> false + "none signals an error when argument is" in completeTestName -> false + + // TODO: support continuable parsing of macro arguments + "make_decimal can be invoked in binary using system macro address 6" in completeTestName -> false + + // TODO: Macro-shaped parameters not implemented yet + "macro-shape" in completeTestName -> false + + // TODO: Not implemented yet + "subnormal f16" in completeTestName -> false + "conformance/system_macros/" in file.absolutePath -> when { + file.endsWith("parse_ion.ion") || + file.endsWith("make_list.ion") || + file.endsWith("make_sexp.ion") || + file.endsWith("make_field.ion") || + file.endsWith("flatten.ion") || + file.endsWith("make_struct.ion") -> false + else -> true + } // Some of these are failing because // - Ion Java doesn't support the Ion 1.1 system symbol table yet // - The tokens `$ion_1_0` and `'$ion_1_0'` are never user values. @@ -69,7 +123,7 @@ abstract class ConformanceTestRunner( debugEnabled = true, failUnimplemented = false, readerBuilder = readerBuilder, - testFilter = { file, name -> DEFAULT_SKIP_FILTER(file, name) && additionalSkipFilter(file, name) }, + testFilter = { file, name -> DEFAULT_SKIP_FILTER(file, name) && !additionalSkipFilter(file, name) }, ) @TestFactory diff --git a/src/test/java/com/amazon/ion/conformance/expectations.kt b/src/test/java/com/amazon/ion/conformance/expectations.kt index 069fae324..a9ecae7a7 100644 --- a/src/test/java/com/amazon/ion/conformance/expectations.kt +++ b/src/test/java/com/amazon/ion/conformance/expectations.kt @@ -122,7 +122,7 @@ fun TestCaseSupport.assertDenotes(modelValues: List, reader: IonRead */ private fun TestCaseSupport.denotesModelValue(expectation: AnyElement, reader: IonReader) { if (reader.type == null) fail(expectation, "no more values; expected $expectation") - if (expectation is SexpElement && expectation.head == "Annot") { + if (expectation is SexpElement && expectation.head == "annot") { val actualAnnotations = reader.typeAnnotationSymbols expectation.tailFrom(2) .forEachIndexed { i, it -> denotesSymtok(it, actualAnnotations[i]) } @@ -356,7 +356,7 @@ private fun TestCaseSupport.denotesStruct(expectation: SeqElement, reader: IonRe private fun TestCaseSupport.denotesSymtok(expectation: AnyElement, actual: SymbolToken) { when (expectation) { is TextElement -> assertEquals(expectation.textValue, actual.text, createFailureMessage(expectation)) - is IntElement -> assertEquals(expectation.longValue, actual.sid, createFailureMessage(expectation)) + is IntElement -> assertEquals(expectation.longValue.toInt(), actual.sid, createFailureMessage(expectation)) is SeqElement -> when (expectation.head) { "absent" -> { if (actual.text != null) fail(expectation, "Expected unknown text; was '${actual.text}'") diff --git a/src/test/java/com/amazon/ion/conformance/fragments.kt b/src/test/java/com/amazon/ion/conformance/fragments.kt index 38f101344..ae6dc7994 100644 --- a/src/test/java/com/amazon/ion/conformance/fragments.kt +++ b/src/test/java/com/amazon/ion/conformance/fragments.kt @@ -8,7 +8,8 @@ import com.amazon.ion.TestUtils.* import com.amazon.ion.conformance.ConformanceTestBuilder.* import com.amazon.ion.conformance.Encoding.* import com.amazon.ion.impl.* -import com.amazon.ion.impl.bin.SymbolInliningStrategy +import com.amazon.ion.impl.bin.* +import com.amazon.ion.impl.macro.* import com.amazon.ion.system.* import com.amazon.ion.util.* import com.amazon.ionelement.api.AnyElement @@ -21,6 +22,7 @@ import com.amazon.ionelement.api.TextElement import com.amazon.ionelement.api.ionInt import com.amazon.ionelement.api.ionSexpOf import com.amazon.ionelement.api.ionSymbol +import com.amazon.ionelement.api.loadSingleElement import java.io.ByteArrayOutputStream import kotlin.contracts.ExperimentalContracts import kotlin.contracts.contract @@ -153,7 +155,7 @@ private fun TestCaseSupport.readFragment(fragment: SeqElement, encoding: Encodin // we need to remove "bytes" from this when expression "bytes" -> readBytesFragment(fragment, encoding) "toplevel" -> readTopLevelFragment(fragment, encoding) - "mactab" -> TODO("mactab") + "mactab" -> readMactabFragment(fragment, encoding) "encoding" -> TODO("encoding") else -> reportSyntaxError(fragment, "not a valid fragment") } @@ -240,6 +242,31 @@ private fun TestCaseSupport.readTopLevelFragment(fragment: SeqElement, encoding: return bytes to currentEncoding } +private fun TestCaseSupport.readMactabFragment(fragment: SeqElement, encoding: Encoding): Pair { + val baos = ByteArrayOutputStream() + var currentEncoding = encoding + var currentWriter = encoding.writerBuilder.build(baos) + + // TODO: Consider replacing this to use literal values instead of the `set_macros` macro to + // minimize dependencies in tests. + + // Can't have a mactab for an Ion 1.0 segment, so this should be safe + currentWriter as MacroAwareIonWriter + currentWriter.startMacro(SystemMacro.SetMacros) + currentWriter.startExpressionGroup() + fragment.tail.forEach { + it.writeTo(currentWriter) + } + currentWriter.endExpressionGroup() + currentWriter.endMacro() + currentWriter.close() + val bytes = baos.toByteArray() + // Drop the initial IVM + .let { if (encoding is Binary) it.drop(4).toByteArray() else it } + .let { if (encoding is Text11) it.drop("\$ion_1_1".length).toByteArray() else it } + return bytes to currentEncoding +} + /** * Writes this [AnyElement] to an [IonWriter], applying the de-mangling logic described at * [Conformance – Abstract Syntax Forms](https://github.com/amazon-ion/ion-tests/tree/master/conformance#abstract-syntax-forms). @@ -264,12 +291,25 @@ private fun AnyElement.demangledWriteTo(writer: IonWriter) { writer.stepOut() } ElementType.SEXP -> { - if (sexpValues.firstOrNull().let { it is TextElement && it.textValue.startsWith("#$:") }) { - TODO("demangled e-expressions") + val head = sexpValues.firstOrNull() + if (head is TextElement && head.textValue.startsWith("#$:")) { + val tail = sexpValues.drop(1) + if (head.textValue == "#$::") { + // Write an expression group + writer as IonManagedWriter_1_1 + val rawWriter = writer.getRawUserWriter() + rawWriter.stepInExpressionGroup(usingLengthPrefix = true) + tail.forEach { it.demangledWriteTo(writer) } + rawWriter.stepOut() + } else { + // Write an e-expression + writer.writeDemangledEExpression(head, tail) + } + } else { + writer.stepIn(IonType.SEXP) + sexpValues.forEach { it.demangledWriteTo(writer) } + writer.stepOut() } - writer.stepIn(IonType.SEXP) - sexpValues.forEach { it.demangledWriteTo(writer) } - writer.stepOut() } ElementType.STRUCT -> { writer.stepIn(IonType.STRUCT) @@ -283,13 +323,56 @@ private fun AnyElement.demangledWriteTo(writer: IonWriter) { } } +private fun IonWriter.writeDemangledEExpression(head: TextElement, tail: List) { + this as IonManagedWriter_1_1 + val rawWriter = this.getRawUserWriter() + + // Drop the first 3 characters (the `#$:`) and then parse as Ion + val macroReference = loadSingleElement(head.textValue.drop(3)) + val annotations = macroReference.annotations + if (annotations.isNotEmpty()) { + if (annotations.singleOrNull() == "\$ion") { + val systemMacro = when (macroReference) { + is SymbolElement -> SystemMacro[macroReference.textValue]!! + is IntElement -> SystemMacro[macroReference.longValue.toInt()]!! + else -> throw IllegalArgumentException("Not a valid macro reference: $head") + } + rawWriter.stepInEExp(systemMacro) + tail.forEach { it.demangledWriteTo(this) } + rawWriter.stepOut() + } else { + TODO("demangled, non-system, qualified e-expressions") + } + } else if (macroReference is IntElement) { + val macro = if (rawWriter is IonRawBinaryWriter_1_1) { + // For this to work in binary, we need to look up the signature. + TODO("For Ion binary, we need to look up the macro definition") + } else { + // For Ion Text, we can cheat and use a placeholder because the macro arg isn't used. + SystemMacro.None + } + rawWriter.stepInEExp(macroReference.longValue.toInt(), usingLengthPrefix = false, macro) + tail.forEach { it.demangledWriteTo(this) } + rawWriter.stepOut() + } else if (macroReference is SymbolElement) { + if (rawWriter is IonRawBinaryWriter_1_1) { + TODO("For Ion binary, we need to look up the address for the macro and invoke by ID") + } + rawWriter.stepInEExp(macroReference.textValue) + tail.forEach { it.demangledWriteTo(this) } + rawWriter.stepOut() + } else { + throw IllegalArgumentException("Not a valid macro reference: $head") + } +} + private fun demangleSymbolToken(text: String): SymbolToken { return if (text.startsWith("#\$ion_")) { // Escaped IVM or system symbol FakeSymbolToken(text.drop(1), -1) } else if (text.startsWith("#$:")) { - // E-Expression macro id - TODO("demangled e-expressions - $text") + // E-Expression macro id -- Should be unreachable; handled elsewhere + TODO("Should be unreachable! demangled e-expressions - $text") } else if (text.startsWith("#$")) { // Escaped SID val id = text.drop(2).toInt() diff --git a/src/test/java/com/amazon/ion/impl/macro/MacroEvaluatorTest.kt b/src/test/java/com/amazon/ion/impl/macro/MacroEvaluatorTest.kt index d449a995c..906991013 100644 --- a/src/test/java/com/amazon/ion/impl/macro/MacroEvaluatorTest.kt +++ b/src/test/java/com/amazon/ion/impl/macro/MacroEvaluatorTest.kt @@ -1070,15 +1070,15 @@ class MacroEvaluatorTest { @Test fun `invoke repeat with invalid 'n' argument`() { // Given: - // (macro (x) (.make_string (.repeat (%x) "na ") "batman!")) + // // When: - // (:repeat 0 "a") + // (:repeat -1 "a") // Then: // evaluator.initExpansion { eexp(Repeat) { - int(0) + int(-1) string("a") } } @@ -1087,13 +1087,13 @@ class MacroEvaluatorTest { } @Test - fun `invoke repeat with invalid 'value' argument`() { + fun `invoke repeat with empty 'value' argument`() { // Given: - // (macro (x) (.make_string (.repeat (%x) "na ") "batman!")) + // // When: // (:repeat 3 (:values)) // Then: - // + // evaluator.initExpansion { eexp(Repeat) { @@ -1102,7 +1102,7 @@ class MacroEvaluatorTest { } } - assertThrows { evaluator.expandNext() } + assertNull(evaluator.expandNext()) } @Test