Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions runtime/src/main/java/dev/ionfusion/fusion/BaseValue.java
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,35 @@ abstract void write(Evaluator eval, Appendable out)
throws IOException, FusionException;


/**
* Variant of {@link #write(Evaluator, Appendable)} that carries a write
* context from the caller.
* <p>
* Most code shouldn't call this method, and should prefer
* {@link FusionIo#dispatchWrite(Evaluator, Appendable, Object, boolean)}.
*
* @param eval may be null!
* @param out the output stream; not null.
* @param quoteOperators if false, this value is a direct child of a sexp
* and operator symbols should be written without quoting. Subclasses
* that are not themselves sexps must revert to
* {@code quoteOperators=true} when recursing into their own children.
*
* <p>The default implementation ignores the flag, which is correct for
* all value types that contain no operator symbols (numbers, strings,
* bools, etc.). Override only when the type itself may be a symbol, or
* when it directly contains symbols as children.
*
* @throws IOException Propagated from the output stream.
* @throws FusionException
*/
void write(Evaluator eval, Appendable out, boolean quoteOperators)
throws IOException, FusionException
{
write(eval, out);
}


/**
* Builder for temporary IonWriters needed for {@link #write}ing
* lazily injected lists and structs.
Expand Down
27 changes: 26 additions & 1 deletion runtime/src/main/java/dev/ionfusion/fusion/FusionIo.java
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,31 @@ static void dispatchWrite(Evaluator eval, Appendable out, Object value)
}


/**
* Variant of {@link #dispatchWrite(Evaluator, Appendable, Object)} that
* passes a write context to the value.
*
* @param quoteOperators if false, operator symbols that are direct
* children of a sexp should be written without quoting. Receivers
* that are not themselves sexps must revert to
* {@code quoteOperators=true} for their own children.
*/
static void dispatchWrite(Evaluator eval, Appendable out, Object value,
boolean quoteOperators)
throws IOException, FusionException
{
if (value instanceof BaseValue)
{
((BaseValue) value).write(eval, out, quoteOperators);
}
else
{
// quoteOperators defaults to true for non-BaseValue objects.
dispatchWrite(eval, out, value);
}
}


static void dispatchDisplay(Evaluator eval, Appendable out, Object value)
throws IOException, FusionException
{
Expand Down Expand Up @@ -850,7 +875,7 @@ static final class DisplayToStringProc
{
@Override
Object doApply(Evaluator eval, Object[] args)
throws FusionException
throws FusionException
{
String output = displayManyToString(eval, args, 0);
return makeString(eval, output);
Expand Down
29 changes: 26 additions & 3 deletions runtime/src/main/java/dev/ionfusion/fusion/FusionSexp.java
Original file line number Diff line number Diff line change
Expand Up @@ -1003,6 +1003,21 @@ else if (throwOnConversionFailure)
}
}

/**
* Writes this sexp using Fusion's write format.
* <p>
* Operator symbols that are direct children of a sexp are written
* without quoting (e.g. {@code +} rather than {@code '+'}), matching
* the behavior of {@link #ionize} which delegates to {@link IonWriter}
* for this purpose. This applies equally to proper sexps
* {@code (a b c)} and improper sexps {@code (a {.} b)}: properness
* affects only structure, not symbol quoting.
* <p>
* Unlike {@link #ionize}, this method tolerates non-ionizable values
* (e.g. void, closures) by falling back to their own {@code write}
* output, so expressions like {@code (write (sexp (quote +) (void)))}
* produce {@code (+ {{{void}}})} rather than raising an exception.
*/
@Override
void write(Evaluator eval, Appendable out)
throws IOException, FusionException
Expand All @@ -1011,11 +1026,17 @@ void write(Evaluator eval, Appendable out)
out.append('(');

ImmutablePair pair = this;
boolean first = true;
while (true)
{
if (pair != this) out.append(' ');
if (!first) out.append(' ');
first = false;

dispatchWrite(eval, out, pair.myHead);
// Pass quoteOperators=false: direct children of a sexp render
// operator symbols unquoted. Each child's write() reverts to
// quoteOperators=true for its own children, so only the
// immediate children of this sexp are affected.
dispatchWrite(eval, out, pair.myHead, false);

Object tail = pair.myTail;
if (tail instanceof ImmutablePair)
Expand All @@ -1028,8 +1049,10 @@ else if (tail instanceof EmptySexp)
}
else
{
// Improper sexp: same quoting rules apply to the tail
// element, since it is still a direct child of this sexp.
out.append(" {.} ");
dispatchWrite(eval, out, tail);
dispatchWrite(eval, out, tail, false);
break;
}
}
Expand Down
33 changes: 33 additions & 0 deletions runtime/src/main/java/dev/ionfusion/fusion/FusionSymbol.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@

package dev.ionfusion.fusion;

import static com.amazon.ion.util.IonTextUtils.symbolVariant;
import static com.amazon.ion.util.IonTextUtils.SymbolVariant.OPERATOR;

import static dev.ionfusion.fusion.FusionBool.falseBool;
import static dev.ionfusion.fusion.FusionBool.makeBool;
import static dev.ionfusion.fusion.FusionBool.trueBool;
Expand Down Expand Up @@ -304,6 +307,26 @@ void write(Evaluator eval, Appendable out)
IonTextUtils.printSymbol(out, myContent);
}

@Override
void write(Evaluator eval, Appendable out, boolean quoteOperators)
throws IOException
{
if (!quoteOperators && symbolVariant(myContent) == OPERATOR)
{
// Inside a sexp, operator symbols like + and = are valid Ion
// without quoting. Operator content is guaranteed to be
// ASCII non-whitespace, so raw emission is safe.
out.append(myContent);
}
else
{
// All other symbols (identifiers, symbols requiring quotes due
// to spaces or other characters, etc.) always use printSymbol
// regardless of sexp context.
IonTextUtils.printSymbol(out, myContent);
}
}

@Override
void display(Evaluator eval, Appendable out)
throws IOException
Expand Down Expand Up @@ -421,6 +444,16 @@ void write(Evaluator eval, Appendable out)
writeAnnotations(out, myAnnotations);
myValue.write(eval, out);
}

@Override
void write(Evaluator eval, Appendable out, boolean quoteOperators)
throws IOException, FusionException
{
// Annotations are always quoted per Ion syntax regardless of
// context; only the symbol value itself observes quoteOperators.
writeAnnotations(out, myAnnotations);
myValue.write(eval, out, quoteOperators);
}
}


Expand Down
174 changes: 134 additions & 40 deletions runtime/src/test/fusion/scripts/io.test.fusion
Original file line number Diff line number Diff line change
Expand Up @@ -29,34 +29,35 @@


(with_ion_from_string "0 1 2 3"
(lambda ()
(check === 0 (read))
(check === 1 (read))
(with_ion_from_string " \"hello\" "
(lambda ()
(check = "hello" (read))
(check_true (is_eof (read)))))
(check === 2 (read))
(check === 3 (read))
(check_true (is_eof (read)))))
(lambda ()
(check === 0 (read))
(check === 1 (read))
(with_ion_from_string " \"hello\" "
(lambda ()
(check = "hello" (read))
(check_true (is_eof (read)))))
(check === 2 (read))
(check === 3 (read))
(check_true (is_eof (read)))))


// This assumes that current_directory is at the project root.
(with_ion_from_file (test_data_file "ints.ion")
(lambda ()
(check === 0 (read))
(check === 1 (read))
(with_ion_from_file (test_data_file "hello.ion")
(lambda ()
(check = "hello" (read))
(check_true (is_eof (read)))))
(check === 2 (read))
(check === 3 (read))
(check_true (is_eof (read)))))
(lambda ()
(check === 0 (read))
(check === 1 (read))
(with_ion_from_file (test_data_file "hello.ion")
(lambda ()
(check = "hello" (read))
(check_true (is_eof (read)))))
(check === 2 (read))
(check === 3 (read))
(check_true (is_eof (read)))))


// A nice shortcut for reading a single value.
(check = "hello"
(with_ion_from_file (test_data_file "hello.ion") read))
(with_ion_from_file (test_data_file "hello.ion") read))

(check_true (is_eof eof))

Expand All @@ -65,22 +66,22 @@
// Lob I/O

(define_check (check_lob_io value)
(let [(lob (ionize_to_blob value))]
(check_pred is_blob lob)
(check_pred (negate is_null) lob)
(let [(v (with_ion_from_lob lob
(thunk
(let [(v (read))]
(check_pred (negate is_eof) v)
(check_pred is_eof (read))
v))))]
(check === value v))))
(let [(lob (ionize_to_blob value))]
(check_pred is_blob lob)
(check_pred (negate is_null) lob)
(let [(v (with_ion_from_lob lob
(thunk
(let [(v (read))]
(check_pred (negate is_eof) v)
(check_pred is_eof (read))
v))))]
(check === value v))))

(map (lambda (v) (check_lob_io v)) representative_ion_data)

// Make sure we can read Ion text
(check === (quote [only_me])
(with_ion_from_lob {{"[only_me]"}} read))
(with_ion_from_lob {{"[only_me]"}} read))

(expect_arity_error (ionize_to_blob))
(expect_arity_error (ionize_to_blob 1 2))
Expand Down Expand Up @@ -133,8 +134,8 @@
(check === "{f:\"a\"}" (display_to_string { f: "a" }))

(check_pred (|r| (or (=== "{f:a,g:b}" r)
(=== "{g:b,f:a}" r)))
(display_to_string (quote { f: a, g: b })))
(=== "{g:b,f:a}" r)))
(display_to_string (quote { f: a, g: b })))


//=============================================================================
Expand All @@ -145,11 +146,104 @@
(check === "" (with_output_to_string))

(check === "true12\n\"write\"display"
(with_output_to_string
(display "true")
(displayln 12)
(write "write")
(display "display")))
(with_output_to_string
(display "true")
(displayln 12)
(write "write")
(display "display")))


"SUCCESS (io.test)"
//=============================================================================
// write: symbols

// Plain identifier: written as-is, no quotes needed.
(check === "quoted_sym"
(with_output_to_string
(write (quote quoted_sym))))

Comment on lines +159 to +163
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And in this new code the indentation has all been lost. Oops!

// Symbol requiring quotes (contains spaces): always quoted regardless of context.
(check === "'with spaces'"
(with_output_to_string
(write (quote 'with spaces'))))

// Annotated symbol: annotation and value both written correctly.
(check === "tag::value"
(with_output_to_string
(write (quote tag::value))))

// Operator symbol outside a sexp: must be quoted.
(check === "'+'"
(with_output_to_string
(write (quote '+'))))

(check === "'+='"
(with_output_to_string
(write (quote +=))))

// null symbol
(check === "null.symbol"
(with_output_to_string
(write (quote null.symbol))))



// write: sexps

// Context-switching: operator symbols in lists, structs, and sexps containing non-Ion values.

// List: operator symbols must be quoted (list is not a sexp context).
(check === "['+', ['+'], {'+':'+'}, (+ {{{void}}})]"
(with_output_to_string
(write (list (quote +) (list (quote +)) (struct "+" (quote +)) (sexp (quote +) (void))))))

// Struct: operator symbols in field names and values must be quoted.
(check === "{f:'+'}"
(with_output_to_string
(write (quote {f: '+'}))))

// Sexp: operator symbols must NOT be quoted; void verified to stay in write mode.
(check === "(+ ['+', {{{void}}}] {'+':\"+\",f:{{{void}}}} (+ {{{void}}}) {{{void}}})"
(with_output_to_string
(write (sexp (quote +) (list (quote +) (void)) (struct "+" "+" "f" (void)) (sexp (quote +) (void)) (void)))))
Comment on lines +205 to +207
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd expect this test to fail 50% of the time due to struct-ordering differences. Ion/Fusion doesn't guarantee that struct fields are output in any particular order, and the runtime intentionally shuffles structs to dissuade code from relying on that.

There's no easy work-around when looking at serialized forms, and this is one of the rare cases where that's appropriate. I think the best solution today is to stick to single-field structs so the tests are robust.

That said... I pulled the PR and after many runs it never failed. That's... surprising, and perhaps something broke in the shuffler. Regardless, we can leave this as-is for now, I guess!


// Empty and null sexps.
(check === "()"
(with_output_to_string
(write (sexp))))

(check === "null.sexp"
(with_output_to_string
(write (quote null.sexp))))

// Operator symbols inside a sexp: must NOT be quoted.
(check === "(+ 1 2)"
(with_output_to_string
(write (quote (+ 1 2)))))

(check === "(+ =)"
(with_output_to_string
(write (quote (+ =)))))

// Symbol requiring quotes inside a sexp: must still be quoted.
(check === "('with spaces')"
(with_output_to_string
(write (quote ('with spaces')))))

// Annotated value inside a sexp: annotation written correctly.
(check === "(note::\"Hello\")"
(with_output_to_string
(write (quote (note::"Hello")))))

// Improper sexp (pair): operator symbols in both head and tail unquoted.
(check === "(a {.} b)"
(with_output_to_string
(write (pair (quote a) (quote b)))))

(check === "(+ {.} =)"
(with_output_to_string
(write (pair (quote +) (quote =)))))

// Symbol requiring quotes in improper sexp tail: must still be quoted.
(check === "(a {.} 'with spaces')"
(with_output_to_string
(write (pair (quote a) (quote 'with spaces')))))
Loading
Loading