From 344cf957dbd30020f0526ad386764ca1dbfded4d Mon Sep 17 00:00:00 2001 From: Tyler Gregg Date: Fri, 11 Oct 2024 14:19:21 -0700 Subject: [PATCH] Adds support for reading text Ion 1.1 system macros and implicit rest parameters. --- .../impl/IonReaderContinuableCoreBinary.java | 6 ++-- .../amazon/ion/impl/IonReaderTextSystemX.java | 21 ++++++----- .../ion/impl/macro/EExpressionArgsReader.java | 35 ++++++++++++++++--- .../com/amazon/ion/impl/macro/SystemMacro.kt | 8 +++++ .../ion/impl/IonRawTextReaderTest_1_1.java | 27 ++++++++++++++ ...onReaderContinuableTopLevelBinaryTest.java | 2 +- 6 files changed, 81 insertions(+), 18 deletions(-) diff --git a/src/main/java/com/amazon/ion/impl/IonReaderContinuableCoreBinary.java b/src/main/java/com/amazon/ion/impl/IonReaderContinuableCoreBinary.java index 6d6606338..2ac8f2ab1 100644 --- a/src/main/java/com/amazon/ion/impl/IonReaderContinuableCoreBinary.java +++ b/src/main/java/com/amazon/ion/impl/IonReaderContinuableCoreBinary.java @@ -1428,7 +1428,7 @@ private void readSingleExpression(Macro.Parameter parameter, List expres } @Override - protected void readParameter(Macro.Parameter parameter, long parameterPresence, List expressions) { + protected void readParameter(Macro.Parameter parameter, long parameterPresence, List expressions, boolean isTrailing) { switch (parameter.getCardinality()) { case ZeroOrOne: if (parameterPresence == PresenceBitmap.EXPRESSION) { diff --git a/src/main/java/com/amazon/ion/impl/IonReaderTextSystemX.java b/src/main/java/com/amazon/ion/impl/IonReaderTextSystemX.java index f396ea059..6a3de4adc 100644 --- a/src/main/java/com/amazon/ion/impl/IonReaderTextSystemX.java +++ b/src/main/java/com/amazon/ion/impl/IonReaderTextSystemX.java @@ -35,6 +35,7 @@ import com.amazon.ion.impl.macro.MacroRef; import com.amazon.ion.impl.macro.ReaderAdapter; import com.amazon.ion.impl.macro.ReaderAdapterIonReader; +import com.amazon.ion.impl.macro.SystemMacro; import java.io.IOException; import java.math.BigDecimal; @@ -62,11 +63,11 @@ class IonReaderTextSystemX SymbolTable _system_symtab; - // The IonReader-like MacroEvaluator that this core reader delegates to when evaluating a macro invocation. - protected MacroEvaluatorAsIonReader macroEvaluatorIonReader = null; - // The core MacroEvaluator that this core reader delegates to when evaluating a macro invocation. - private MacroEvaluator macroEvaluator = null; + private final MacroEvaluator macroEvaluator = new MacroEvaluator(); + + // The IonReader-like MacroEvaluator that this core reader delegates to when evaluating a macro invocation. + protected final MacroEvaluatorAsIonReader macroEvaluatorIonReader = new MacroEvaluatorAsIonReader(macroEvaluator); // The encoding context (macro table) that is currently active. private EncodingContext encodingContext = null; @@ -1184,8 +1185,6 @@ private void readEncodingDirective() { )); } encodingContext = new EncodingContext(encodingDirectiveReader.getNewMacros()); - macroEvaluator = new MacroEvaluator(); - macroEvaluatorIonReader = new MacroEvaluatorAsIonReader(macroEvaluator); } @@ -1199,11 +1198,11 @@ private class TextEExpressionArgsReader extends EExpressionArgsReader { } @Override - protected void readParameter(Macro.Parameter parameter, long parameterPresence, List expressions) { + protected void readParameter(Macro.Parameter parameter, long parameterPresence, List expressions, boolean isTrailing) { if (IonReaderTextSystemX.this.nextRaw() == null) { return; } - readValueAsExpression(expressions); + readValueAsExpression(isTrailing && parameter.getCardinality().canBeMulti, expressions); } @Override @@ -1229,7 +1228,11 @@ protected Macro loadMacro() { throw new IonException("E-expressions must begin with an address."); } Macro macro; - if (encodingContext == null || ((macro = encodingContext.getMacroTable().get(address)) == null)) { + if (encodingContext == null) { + if ((macro = SystemMacro.get(address)) == null) { + throw new IonException(String.format("Encountered an unknown macro address: %s.", address)); + } + } else if ((macro = encodingContext.getMacroTable().get(address)) == null) { throw new IonException(String.format("Encountered an unknown macro address: %s.", address)); } return macro; diff --git a/src/main/java/com/amazon/ion/impl/macro/EExpressionArgsReader.java b/src/main/java/com/amazon/ion/impl/macro/EExpressionArgsReader.java index 38ead25bd..24b2ac419 100644 --- a/src/main/java/com/amazon/ion/impl/macro/EExpressionArgsReader.java +++ b/src/main/java/com/amazon/ion/impl/macro/EExpressionArgsReader.java @@ -83,8 +83,9 @@ public EExpressionArgsReader(ReaderAdapter reader) { * @param parameter information about the parameter from the macro signature. * @param parameterPresence the presence bits dedicated to this parameter (unused in text). * @param expressions receives the expressions as they are materialized. + * @param isTrailing true if this parameter is the last one in the signature; otherwise, false. */ - protected abstract void readParameter(Macro.Parameter parameter, long parameterPresence, List expressions); + protected abstract void readParameter(Macro.Parameter parameter, long parameterPresence, List expressions, boolean isTrailing); /** * Reads the macro's address and attempts to resolve that address to a Macro from the macro table. @@ -180,7 +181,7 @@ private void readContainerValueAsExpression( if (type == IonType.STRUCT) { expressions.add(new Expression.FieldName(reader.getFieldNameSymbol())); } - readValueAsExpression(expressions); // TODO avoid recursion + readValueAsExpression(false, expressions); // TODO avoid recursion } stepOutRaw(); // Overwrite the placeholder with an expression representing the actual type of the container and the @@ -207,19 +208,38 @@ private void readContainerValueAsExpression( expressions.set(startIndex, expression); } + /** + * Reads the rest of the stream into a single expression group. + * @param expressions receives the expressions as they are materialized. + */ + private void readStreamAsExpressionGroup( + List expressions + ) { + int startIndex = expressions.size(); + expressions.add(Expression.Placeholder.INSTANCE); + do { + readValueAsExpression(false, expressions); // TODO avoid recursion + } while (nextRaw()); + expressions.set(startIndex, new Expression.ExpressionGroup(startIndex, expressions.size())); + } + /** * Reads a value from the stream into expression(s) that will eventually be passed to the MacroEvaluator * responsible for evaluating the e-expression to which this value belongs. + * @param isImplicitRest true if this is the final parameter in the signature, it is variadic, and the format + * supports implicit rest parameters (text only); otherwise, false. * @param expressions receives the expressions as they are materialized. */ - protected void readValueAsExpression(List expressions) { + protected void readValueAsExpression(boolean isImplicitRest, List expressions) { if (isMacroInvocation()) { collectEExpressionArgs(expressions); // TODO avoid recursion return; } IonType type = reader.encodingType(); List annotations = getAnnotations(); - if (IonType.isContainer(type)) { + if (isImplicitRest) { + readStreamAsExpressionGroup(expressions); + } else if (IonType.isContainer(type)) { readContainerValueAsExpression(type, annotations, expressions); } else { readScalarValueAsExpression(type, annotations, expressions); @@ -242,7 +262,12 @@ private void collectEExpressionArgs(List e int numberOfParameters = signature.size(); stepIntoEExpression(); for (int i = 0; i < numberOfParameters; i++) { - readParameter(signature.get(i), presenceBitmap == null ? 0 : presenceBitmap.get(i), expressions); + readParameter( + signature.get(i), + presenceBitmap == null ? 0 : presenceBitmap.get(i), + expressions, + i == (numberOfParameters - 1) + ); } stepOutOfEExpression(); expressions.set(invocationStartIndex, new Expression.EExpression(macro, invocationStartIndex, expressions.size())); 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 e24a977e0..f131711a4 100644 --- a/src/main/java/com/amazon/ion/impl/macro/SystemMacro.kt +++ b/src/main/java/com/amazon/ion/impl/macro/SystemMacro.kt @@ -50,6 +50,14 @@ enum class SystemMacro(val id: Byte, val macroName: String, override val signatu @JvmStatic operator fun get(name: String): SystemMacro? = MACROS_BY_NAME[name]?.takeUnless { it.id < 0 } + @JvmStatic + operator fun get(address: MacroRef): SystemMacro? { + return when (address) { + is MacroRef.ById -> get(address.id) + is MacroRef.ByName -> get(address.name) + } + } + /** 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] diff --git a/src/test/java/com/amazon/ion/impl/IonRawTextReaderTest_1_1.java b/src/test/java/com/amazon/ion/impl/IonRawTextReaderTest_1_1.java index 5f03b6c52..5da12b07c 100644 --- a/src/test/java/com/amazon/ion/impl/IonRawTextReaderTest_1_1.java +++ b/src/test/java/com/amazon/ion/impl/IonRawTextReaderTest_1_1.java @@ -2,10 +2,12 @@ // SPDX-License-Identifier: Apache-2.0 package com.amazon.ion.impl; +import com.amazon.ion.IonReader; import com.amazon.ion.IonType; import com.amazon.ion.system.SimpleCatalog; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.CsvSource; import org.junit.jupiter.params.provider.MethodSource; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -126,4 +128,29 @@ public void invalidExpressionSyntax(int minorVersion, String input, String first assertThrows(IonReaderTextRawX.IonReaderTextParsingException.class, reader::nextRaw); } } + + @ParameterizedTest + @CsvSource({ + "(:values 0) (:values 1)", + "(:values (: values 0)) 1", + "(:values) 0 (:values) 1", + "(:values) (:values 0) 1", + "(:values) (:values) (:values 0) 1", + "(:values (:: ) ) 0 1", + "(:values (:: 0 1))", + "(:values 0 1)", + "(:values (:: (:: 0) (:values (:: 1))))", + "(:values (:: 0) (:values (:: 1)))", + "(:values (:values (:: 0 1)))", + "(:values (:values 0 1))" + }) + public void validValuesInvocations(String text) throws Exception { + try (IonReader reader = newTextReader("$ion_1_1 " + text)) { + assertEquals(IonType.INT, reader.next()); + assertEquals(0, reader.intValue()); + assertEquals(IonType.INT, reader.next()); + assertEquals(1, reader.intValue()); + assertNull(reader.next()); + } + } } diff --git a/src/test/java/com/amazon/ion/impl/IonReaderContinuableTopLevelBinaryTest.java b/src/test/java/com/amazon/ion/impl/IonReaderContinuableTopLevelBinaryTest.java index fd43ee1f5..fab606f4d 100644 --- a/src/test/java/com/amazon/ion/impl/IonReaderContinuableTopLevelBinaryTest.java +++ b/src/test/java/com/amazon/ion/impl/IonReaderContinuableTopLevelBinaryTest.java @@ -5961,7 +5961,7 @@ public void invokeValuesUsingSystemMacroOpcode(String bytes) throws Exception { "EF 01 02 07 60 61 01", // (:values 0 1) // using delimited expression group "EF 01 02 01 60 61 01 F0", - // (:values (:: 0) (:values (:: 1)) + // (:values (:: 0) (:values (:: 1))) "EF 01 02 03 60 EF 01 02 05 61 01", // (:values (:values 0 1)) "EF 01 01 EF 01 02 07 60 61 01",