Fixes a bug that prevented nested struct macro invocations from being…

… evaluated properly.
amazon-ion · Dec 19, 2024 · 5ea6d73 · 5ea6d73
1 parent 620228b
commit 5ea6d73
Show file tree

Hide file tree

Showing 3 changed files with 66 additions and 4 deletions.
diff --git a/src/main/java/com/amazon/ion/impl/macro/EExpressionArgsReader.java b/src/main/java/com/amazon/ion/impl/macro/EExpressionArgsReader.java
@@ -251,9 +251,6 @@ protected void readValueAsExpression(boolean isImplicitRest, List<Expression.EEx
      * @param expressions receives the expressions as they are materialized.
      */
     private void collectEExpressionArgs(List<Expression.EExpressionBodyExpression> expressions) {
-        if (reader.isInStruct()) {
-            expressions.add(new Expression.FieldName(reader.getFieldNameSymbol()));
-        }
         Macro macro = loadMacro();
         List<Macro.Parameter> signature = macro.getSignature();
         PresenceBitmap presenceBitmap = loadPresenceBitmapIfNecessary(signature);
@@ -281,6 +278,9 @@ public void beginEvaluatingMacroInvocation(MacroEvaluator macroEvaluator) {
         // TODO performance: use a pool of expression lists to avoid repetitive allocations.
         List<Expression.EExpressionBodyExpression> expressions = new ArrayList<>();
         // TODO performance: avoid fully materializing all expressions up-front.
+        if (reader.isInStruct()) {
+            expressions.add(new Expression.FieldName(reader.getFieldNameSymbol()));
+        }
         collectEExpressionArgs(expressions);
         macroEvaluator.initExpansion(expressions);
     }

diff --git a/src/main/java/com/amazon/ion/impl/macro/MacroEvaluatorAsIonReader.kt b/src/main/java/com/amazon/ion/impl/macro/MacroEvaluatorAsIonReader.kt
@@ -70,6 +70,8 @@ class MacroEvaluatorAsIonReader(
         val arguments: List<Expression> = evaluator.getArguments()
         val numberOfContainerEndsAtExpressionIndex = IntArray(arguments.size + 1)
 
+        currentFieldName = null // Field names are written only via FieldName expressions
+
         while (index < arguments.size) {
             for (i in 0 until numberOfContainerEndsAtExpressionIndex[index]) {
                 writer.stepOut()
@@ -87,7 +89,6 @@ class MacroEvaluatorAsIonReader(
                     writer.writeValue(this)
                 }
                 is Expression.FieldName -> {
-                    queuedFieldName = argument
                     writer.setFieldNameSymbol(argument.value)
                 }
                 is Expression.EExpression -> {

diff --git a/src/test/java/com/amazon/ion/impl/EncodingDirectiveCompilationTest.java b/src/test/java/com/amazon/ion/impl/EncodingDirectiveCompilationTest.java
@@ -1010,6 +1010,67 @@ public void structAsParameter(InputType inputType, StreamType streamType) throws
         }
     }
 
+    private byte[] macroInvocationAsParameterToItself(StreamType outputFormat) {
+        ByteArrayOutputStream out = new ByteArrayOutputStream();
+        IonRawWriter_1_1 writer = outputFormat.newWriter(out);
+        SortedMap<String, Macro> expectedMacroTable = new TreeMap<String, Macro>() {{
+            put("SimonSays", writeSimonSaysMacro(writer));
+        }};
+
+        outputFormat.startMacroInvocationByName(writer, "SimonSays", expectedMacroTable);
+        writer.stepInStruct(true);
+        writer.writeFieldName(FIRST_LOCAL_SYMBOL_ID);
+        outputFormat.startMacroInvocationByName(writer, "SimonSays", expectedMacroTable);
+        writer.stepInStruct(true);
+        writer.writeFieldName(FIRST_LOCAL_SYMBOL_ID);
+        writer.writeInt(123);
+        writer.stepOut();
+        writer.stepOut();
+
+        writer.stepOut();
+        writer.stepOut();
+
+        return getBytes(writer, out);
+    }
+
+    @ParameterizedTest(name = "{0},{1}")
+    @MethodSource("allCombinations")
+    public void macroInvocationAsParameterToItself(InputType inputType, StreamType streamType) throws Exception {
+        byte[] data = macroInvocationAsParameterToItself(streamType);
+
+        try (IonReader reader = inputType.newReader(data)) {
+            assertEquals(IonType.STRUCT, reader.next());
+            reader.stepIn();
+            assertEquals(IonType.STRUCT, reader.next());
+            assertEquals("foo", reader.getFieldName());
+            reader.stepIn();
+            assertEquals(IonType.INT, reader.next());
+            assertEquals("foo", reader.getFieldName());
+            assertEquals(123, reader.intValue());
+            assertNull(reader.next());
+            reader.stepOut();
+            reader.stepOut();
+            assertNull(reader.next());
+        }
+    }
+
+    @ParameterizedTest(name = "{0},{1},{2}")
+    @MethodSource("allInputFormatsInputTypesAndOutputFormats")
+    public void macroInvocationAsParameterToItselfMacroAwareTranscode(
+        InputType inputType,
+        StreamType inputFormat,
+        StreamType outputFormat
+    ) throws Exception {
+        byte[] data = macroInvocationAsParameterToItself(outputFormat);
+
+        verifyMacroAwareTranscode(
+            data, inputType, inputFormat,
+            substringCount("(:SimonSays", 2),
+            substringCount("foo:", 2),
+            substringCount("123", 1)
+        );
+    }
+
     @ParameterizedTest(name = "{0},{1}")
     @MethodSource("allCombinations")
     public void macroInvocationAsParameter(InputType inputType, StreamType streamType) throws Exception {