From 1683197bc0ae77e0cee56557085719544933b24a Mon Sep 17 00:00:00 2001
From: torque <torque@epicyclic.dev>
Date: Tue, 3 Oct 2023 23:25:30 -0700
Subject: [PATCH] state: parse whitespace in flow objects a bit differently

There were (and probably still are) some weird and ugly edge cases
here. For example, `[ 1 ]` would parse to a list of `1 `. This
implementation allows a single space to precede the closing ] and
errors out if there is more than one. Additionally, it rejects any
spaces before the item separator comma. This also applies to flow
maps, with the addition that they do not permit whitespace before `:`
now, either.

Leading spaces are still consumed with reckless abandon, so, for
example, `[   lopsided]` is valid. There is also some state sloppiness
flying around so `[   val,    ]` probably currently works as well.
Tightening up the handling of leading whitespace will be a bigger
restructuring that may involve state machine changes. I'll have to
think about it.
---
 src/parser/state.zig | 58 +++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 54 insertions(+), 4 deletions(-)

diff --git a/src/parser/state.zig b/src/parser/state.zig
index 85f4895..cbac552 100644
--- a/src/parser/state.zig
+++ b/src/parser/state.zig
@@ -455,6 +455,7 @@ pub const State = struct {
                     ' ', '\t' => continue :charloop,
                     ',' => {
                         // empty value
+                        // don't check for whitespace here: [ , ] is okay, as is [ , , ]
                         const tip = try state.getStackTip();
                         try tip.flow_list.append(Value.newScalar(arena_alloc));
                         item_start = idx + 1;
@@ -499,9 +500,18 @@ pub const State = struct {
                     },
                 },
                 .consuming_list_item => switch (char) {
+                    // consider: detecting trailing whitespace. "[ 1 ]" should
+                    // produce "1" and not "1 " as it currently does, which breaks
+                    // the principle of least astonishment. design: no trailing
+                    // whitespace before "," and only a single space is allowed before "]"
                     ',' => {
-                        const tip = try state.getStackTip();
+                        if (contents[idx - 1] == ' ' or contents[idx - 1] == '\t') {
+                            state.diagnostics.length = 1;
+                            state.diagnostics.message = "the flow list contains whitespace before ,";
+                            return error.TrailingWhitespace;
+                        }
 
+                        const tip = try state.getStackTip();
                         try tip.flow_list.append(
                             try Value.fromScalar(arena_alloc, contents[item_start..idx]),
                         );
@@ -510,13 +520,23 @@ pub const State = struct {
                         pstate = .want_list_item;
                     },
                     ']' => {
+                        var end = idx;
+                        if (contents[idx - 1] == ' ' or contents[idx - 1] == '\t') {
+                            if (idx > 1 and (contents[idx - 2] == ' ' or contents[idx - 2] == '\t')) {
+                                state.diagnostics.length = 1;
+                                state.diagnostics.message = "the flow list contains extra whitespace before ]";
+                                return error.TrailingWhitespace;
+                            }
+                            end = idx - 1;
+                        }
+
                         const finished = state.value_stack.getLastOrNull() orelse {
                             state.diagnostics.length = 1;
                             state.diagnostics.message = "the flow list was closed too many times";
                             return error.BadState;
                         };
                         try finished.flow_list.append(
-                            try Value.fromScalar(arena_alloc, contents[item_start..idx]),
+                            try Value.fromScalar(arena_alloc, contents[item_start..end]),
                         );
                         pstate = try state.popFlowStack();
                     },
@@ -558,6 +578,11 @@ pub const State = struct {
                 },
                 .consuming_map_key => switch (char) {
                     ':' => {
+                        if (contents[idx - 1] == ' ' or contents[idx - 1] == '\t') {
+                            state.diagnostics.length = 1;
+                            state.diagnostics.message = "the flow map contains whitespace before :";
+                            return error.TrailingWhitespace;
+                        }
                         dangling_key = try arena_alloc.dupe(u8, contents[item_start..idx]);
                         pstate = .want_map_value;
                     },
@@ -625,7 +650,12 @@ pub const State = struct {
                     },
                 },
                 .consuming_map_value => switch (char) {
-                    ',', '}' => |term| {
+                    ',' => {
+                        if (contents[idx - 1] == ' ' or contents[idx - 1] == '\t') {
+                            state.diagnostics.length = 1;
+                            state.diagnostics.message = "the flow map contains whitespace before ,";
+                            return error.TrailingWhitespace;
+                        }
                         const tip = try state.getStackTip();
                         try state.putMap(
                             &tip.flow_map,
@@ -635,7 +665,27 @@ pub const State = struct {
                         );
                         dangling_key = null;
                         pstate = .want_map_key;
-                        if (term == '}') pstate = try state.popFlowStack();
+                    },
+                    '}' => {
+                        var end = idx;
+                        if (contents[idx - 1] == ' ' or contents[idx - 1] == '\t') {
+                            if (idx > 1 and (contents[idx - 2] == ' ' or contents[idx - 2] == '\t')) {
+                                state.diagnostics.length = 1;
+                                state.diagnostics.message = "the flow map contains extra whitespace before }";
+                                return error.TrailingWhitespace;
+                            }
+                            end = idx - 1;
+                        }
+
+                        const tip = try state.getStackTip();
+                        try state.putMap(
+                            &tip.flow_map,
+                            dangling_key.?,
+                            try Value.fromScalar(arena_alloc, contents[item_start..end]),
+                            dkb,
+                        );
+                        dangling_key = null;
+                        pstate = try state.popFlowStack();
                     },
                     else => continue :charloop,
                 },