Refactor formatter, extract string to separate function, limit state

Author: michalmuskala

formatter_test_suite/simple-object.min.json~

@@ -52,7 +52,8 @@ defmodule Jason.Formatter do
   """
   @spec pretty_print(iodata, opts) :: binary
   def pretty_print(iodata, opts \\ []) do
-    pretty_print_to_iodata(iodata, opts)
+    iodata
+    |> pretty_print_to_iodata(opts)
     |> IO.iodata_to_binary()
   end
 
@@ -67,13 +68,10 @@ defmodule Jason.Formatter do
     opts = parse_opts(opts, opts(indent: "  ", line: "\n", record: nil, colon: " "))
     opts = opts(opts, record: opts(opts, :record) || opts(opts, :line))
 
-    depth = 0
-    in_str = false
-    in_bs = false
+    depth = :first
     empty = false
-    first = true
 
-    {output, _state} = pp_iodata(iodata, [], depth, in_str, in_bs, empty, first, opts)
+    {output, _state} = pp_iodata(iodata, [], depth, empty, opts)
 
     output
   end
@@ -96,7 +94,8 @@ defmodule Jason.Formatter do
   """
   @spec minimize(iodata, opts) :: binary
   def minimize(iodata, opts \\ []) do
-    minimize_to_iodata(iodata, opts)
+    iodata
+    |> minimize_to_iodata(opts)
     |> IO.iodata_to_binary()
   end
 
@@ -108,212 +107,166 @@ defmodule Jason.Formatter do
   """
   @spec minimize_to_iodata(iodata, opts) :: iodata
   def minimize_to_iodata(iodata, opts) do
-    opts = parse_opts(opts, opts(indent: [], line: [], record: "\n", colon: []))
+    opts = parse_opts(opts, opts(indent: "", line: "", record: "\n", colon: ""))
 
-    depth = 0
-    in_str = false
-    in_bs = false
+    depth = :first
     empty = false
-    first = true
 
-    {output, _state} = pp_iodata(iodata, [], depth, in_str, in_bs, empty, first, opts)
+    {output, _state} = pp_iodata(iodata, [], depth, empty, opts)
 
     output
   end
 
   defp parse_opts(opts, defaults) do
     Enum.reduce(opts, defaults, fn
-      {:indent, indent}, opts -> opts(opts, indent: indent)
-      {:line_separator, line}, opts -> opts(opts, line: line, record: opts(opts, :record) || line)
-      {:record_separator, record}, opts -> opts(opts, record: record)
-      {:after_colon, colon}, opts -> opts(opts, colon: colon)
+      {:indent, indent}, opts ->
+        opts(opts, indent: IO.iodata_to_binary(indent))
+
+      {:line_separator, line}, opts ->
+        line = IO.iodata_to_binary(line)
+        opts(opts, line: line, record: opts(opts, :record) || line)
+
+      {:record_separator, record}, opts ->
+        opts(opts, record: IO.iodata_to_binary(record))
+
+      {:after_colon, colon}, opts ->
+        opts(opts, colon: IO.iodata_to_binary(colon))
     end)
   end
 
+  @spec tab(String.t(), non_neg_integer) :: iodata()
   ## Returns an iolist containing `depth` instances of `opts[:indent]`
   for depth <- 1..16 do
     defp tab("  ", unquote(depth)), do: unquote(String.duplicate("  ", depth))
   end
 
-  defp tab([], _), do: ""
+  defp tab("", _), do: ""
   defp tab(indent, depth), do: List.duplicate(indent, depth)
 
-  @typep pp_state :: {
-           ## depth -- current nesting depth
-           non_neg_integer,
-           ## in_str -- is the current byte in a string?
-           boolean,
-           ## in_bs -- does the current byte follow a backslash in a string?
-           boolean,
-           ## empty -- is the current object or array empty?
-           boolean,
-           ## first -- is this the first object or array in the input?
-           boolean
-         }
-
-  @spec pp_iodata(
-          ## input -- input data
-          iodata,
-          ## output_acc -- output iolist (built in reverse order)
-          iodata,
-          ## depth -- current nesting depth
-          non_neg_integer,
-          ## in_str -- is the current byte in a string?
-          boolean,
-          ## in_bs -- does the current byte follow a backslash in a string?
-          boolean,
-          ## empty -- is the current object or array empty?
-          boolean,
-          ## first -- is this the first object or array in the input?
-          boolean,
-          opts
-        ) :: {iodata, pp_state}
-  defp pp_iodata(input, output_acc, depth, in_str, in_bs, empty, first, opts)
-
-  defp pp_iodata("", output_acc, depth, in_str, in_bs, empty, first, opts) do
-    {output_acc, {depth, in_str, in_bs, empty, first, opts}}
-  end
-
-  defp pp_iodata([], output_acc, depth, in_str, in_bs, empty, first, opts) do
-    {output_acc, {depth, in_str, in_bs, empty, first, opts}}
+  defp pp_iodata(<<>>, output_acc, depth, empty, opts) do
+    {output_acc, &pp_iodata(&1, &2, depth, empty, opts)}
   end
 
-  defp pp_iodata(
-         <<byte::size(8), rest::binary>>,
-         output_acc,
-         depth,
-         in_str,
-         in_bs,
-         empty,
-         first,
-         opts
-       ) do
-    pp_byte(byte, rest, output_acc, depth, in_str, in_bs, empty, first, opts)
+  defp pp_iodata(<<byte, rest::binary>>, output_acc, depth, empty, opts) do
+    pp_byte(byte, rest, output_acc, depth, empty, opts)
   end
 
-  defp pp_iodata(byte, output_acc, depth, in_str, in_bs, empty, first, opts)
-       when is_integer(byte) do
-    pp_byte(byte, [], output_acc, depth, in_str, in_bs, empty, first, opts)
+  defp pp_iodata([], output_acc, depth, empty, opts) do
+    {output_acc, &pp_iodata(&1, &2, depth, empty, opts)}
   end
 
-  defp pp_iodata(list, output_acc, depth, in_str, in_bs, empty, first, opts) when is_list(list) do
-    starting_state = {depth, in_str, in_bs, empty, first, opts}
-
-    {list_output, end_state} =
-      Enum.reduce(list, {[], starting_state}, fn item, {output_acc, state} ->
-        {depth, in_str, in_bs, empty, first, opts} = state
-        {item_output, new_state} = pp_iodata(item, [], depth, in_str, in_bs, empty, first, opts)
-        {[output_acc, item_output], new_state}
-      end)
-
-    {[output_acc, list_output], end_state}
+  defp pp_iodata([byte | rest], output_acc, depth, empty, opts) when is_integer(byte) do
+    pp_byte(byte, rest, output_acc, depth, empty, opts)
   end
 
-  @spec pp_byte(
-          ## byte -- current byte
-          byte,
-          ## rest -- rest of input data
-          iodata,
-          ## output -- output iolist (built in reverse order)
-          iodata,
-          ## depth -- current nesting depth
-          non_neg_integer,
-          ## in_str -- is the current byte in a string?
-          boolean,
-          ## in_bs -- does the current byte follow a backslash in a string?
-          boolean,
-          ## empty -- is the current object or array empty?
-          boolean,
-          ## first -- is this the first object or array in the input?
-          boolean,
-          opts
-        ) :: {iodata, pp_state}
-  defp pp_byte(byte, rest, output, depth, in_str, in_bs, empty, first, opts)
-
-  ## in string, following backslash
-  defp pp_byte(byte, rest, output, depth, true = in_str, true = _in_bs, empty, first, opts) do
-    in_bs = false
-    pp_iodata(rest, [output, byte], depth, in_str, in_bs, empty, first, opts)
+  defp pp_iodata([head | tail], output_acc, depth, empty, opts) do
+    {output_acc, cont} = pp_iodata(head, output_acc, depth, empty, opts)
+    cont.(tail, output_acc)
   end
 
-  ## in string, backslash
-  defp pp_byte(byte, rest, output, depth, true = in_str, _in_bs, empty, first, opts)
-       when byte in '\\' do
-    in_bs = true
-    pp_iodata(rest, [output, byte], depth, in_str, in_bs, empty, first, opts)
+  defp pp_byte(byte, rest, output, depth, empty, opts) when byte in ' \n\r\t' do
+    pp_iodata(rest, output, depth, empty, opts)
   end
 
-  ## in string, end quote
-  defp pp_byte(byte, rest, output, depth, true = _in_str, in_bs, empty, first, opts)
-       when byte in '"' do
-    in_str = false
-    pp_iodata(rest, [output, byte], depth, in_str, in_bs, empty, first, opts)
-  end
-
-  ## in string, other character
-  defp pp_byte(byte, rest, output, depth, true = in_str, in_bs, empty, first, opts) do
-    pp_iodata(rest, [output, byte], depth, in_str, in_bs, empty, first, opts)
-  end
-
-  ## out of string, whitespace
-  defp pp_byte(byte, rest, output, depth, in_str, in_bs, empty, first, opts)
-       when byte in ' \n\r\t' do
-    pp_iodata(rest, output, depth, in_str, in_bs, empty, first, opts)
-  end
-
-  ## out of string, start block
-  defp pp_byte(byte, rest, output, depth, in_str, in_bs, empty, first, opts)
-       when byte in '{[' do
-    out =
+  defp pp_byte(byte, rest, output, depth, empty, opts) when byte in '{[' do
+    {out, depth} =
       cond do
-        first -> byte
-        empty -> [opts(opts, :line), tab(opts(opts, :indent), depth), byte]
-        depth == 0 -> [opts(opts, :record), byte]
-        true -> byte
+        depth == :first -> {byte, 1}
+        depth == 0 -> {[opts(opts, :record), byte], 1}
+        empty -> {[opts(opts, :line), tab(opts(opts, :indent), depth), byte], depth + 1}
+        true -> {byte, depth + 1}
       end
 
-    first = false
     empty = true
-    depth = depth + 1
-    pp_iodata(rest, [output, out], depth, in_str, in_bs, empty, first, opts)
+    pp_iodata(rest, [output, out], depth, empty, opts)
   end
 
-  ## out of string, end empty block
-  defp pp_byte(byte, rest, output, depth, in_str, in_bs, true = _empty, first, opts)
-       when byte in '}]' do
+  defp pp_byte(byte, rest, output, depth, true = _empty, opts) when byte in '}]' do
     empty = false
     depth = depth - 1
-    pp_iodata(rest, [output, byte], depth, in_str, in_bs, empty, first, opts)
+    pp_iodata(rest, [output, byte], depth, empty, opts)
   end
 
-  ## out of string, end non-empty block
-  defp pp_byte(byte, rest, output, depth, in_str, in_bs, false = empty, first, opts)
-       when byte in '}]' do
+  defp pp_byte(byte, rest, output, depth, false = empty, opts) when byte in '}]' do
     depth = depth - 1
     out = [opts(opts, :line), tab(opts(opts, :indent), depth), byte]
-    pp_iodata(rest, [output, out], depth, in_str, in_bs, empty, first, opts)
+    pp_iodata(rest, [output, out], depth, empty, opts)
   end
 
-  ## out of string, comma
-  defp pp_byte(byte, rest, output, depth, in_str, in_bs, _empty, first, opts)
-       when byte in ',' do
+  defp pp_byte(byte, rest, output, depth, _empty, opts) when byte in ',' do
     empty = false
     out = [byte, opts(opts, :line), tab(opts(opts, :indent), depth)]
-    pp_iodata(rest, [output, out], depth, in_str, in_bs, empty, first, opts)
+    pp_iodata(rest, [output, out], depth, empty, opts)
   end
 
-  ## out of string, colon
-  defp pp_byte(byte, rest, output, depth, in_str, in_bs, empty, first, opts)
-       when byte in ':' do
+  defp pp_byte(byte, rest, output, depth, empty, opts) when byte in ':' do
     out = [byte, opts(opts, :colon)]
-    pp_iodata(rest, [output, out], depth, in_str, in_bs, empty, first, opts)
+    pp_iodata(rest, [output, out], depth, empty, opts)
   end
 
-  ## out of string, other character (maybe start quote)
-  defp pp_byte(byte, rest, output, depth, _in_str, in_bs, empty, first, opts) do
+  defp pp_byte(byte, rest, output, depth, empty, opts) do
     out = if empty, do: [opts(opts, :line), tab(opts(opts, :indent), depth), byte], else: byte
-    in_str = byte in '"'
     empty = false
-    pp_iodata(rest, [output, out], depth, in_str, in_bs, empty, first, opts)
+
+    if byte == ?" do
+      pp_string(rest, [output, out], _in_bs = false, &pp_iodata(&1, &2, depth, empty, opts))
+    else
+      pp_iodata(rest, [output, out], depth, empty, opts)
+    end
+  end
+
+  defp pp_string(<<>>, output_acc, in_bs, cont) do
+    {output_acc, &pp_string(&1, &2, in_bs, cont)}
+  end
+
+  defp pp_string(<<?", rest::binary>>, output_acc, true = _in_bs, cont) do
+    pp_string(rest, [output_acc, ?"], false, cont)
+  end
+
+  defp pp_string(<<?", rest::binary>>, output_acc, false = _in_bs, cont) do
+    cont.(rest, [output_acc, ?"])
+  end
+
+  defp pp_string(<<byte>>, output_acc, in_bs, cont) do
+    in_bs = not in_bs and byte == ?\\
+    {[output_acc, byte], &pp_string(&1, &2, in_bs, cont)}
+  end
+
+  defp pp_string(binary, output_acc, _in_bs, cont) when is_binary(binary) do
+    size = byte_size(binary)
+
+    case :binary.match(binary, "\"") do
+      :nomatch ->
+        skip = size - 2
+        <<_::binary-size(skip), prev, last>> = binary
+        in_bs = not (prev == ?\\ and last == ?\\) or last == ?\\
+        {[output_acc | binary], &pp_string(&1, &2, in_bs, cont)}
+
+      {pos, 1} ->
+        {leading, tail} = :erlang.split_binary(binary, pos + 1)
+        output = [output_acc | leading]
+
+        case :binary.at(binary, pos - 1) do
+          ?\\ -> pp_string(tail, output, false, cont)
+          _ -> cont.(tail, output)
+        end
+    end
+  end
+
+  defp pp_string([], output_acc, in_bs, cont) do
+    {output_acc, &pp_string(&1, &2, in_bs, cont)}
+  end
+
+  defp pp_string([byte | rest], output_acc, in_bs, cont) when is_integer(byte) do
+    cond do
+      in_bs -> pp_string(rest, [output_acc, byte], false, cont)
+      byte == ?" -> cont.(rest, [output_acc, byte])
+      true -> pp_string(rest, [output_acc, byte], byte == ?\\, cont)
+    end
+  end
+
+  defp pp_string([head | tail], output_acc, in_bs, cont) do
+    {output_acc, cont} = pp_string(head, output_acc, in_bs, cont)
+    cont.(tail, output_acc)
   end
 end

lib/formatter.ex

@@ -64,4 +64,38 @@ defmodule Jason.FormatterTest do
     output = ~s|{\n\t"a": {\n\t\t"b": [\n\t\t\ttrue,\n\t\t\tfalse\n\t\t]\n\t}\n}|
     assert(pretty_print(input, indent: "\t") == output)
   end
+
+  test "proper string escaping" do
+    input = ["\"abc\\\\", "\""]
+    output = ~S|"abc\\"|
+    assert(minimize(input) == output)
+
+    input = ["\"abc\\\\", ?"]
+    output = ~S|"abc\\"|
+    assert(minimize(input) == output)
+
+    input = ["\"abc\\\"", "\""]
+    output = ~S|"abc\""|
+    assert(minimize(input) == output)
+
+    input = ["\"abc\\\"", ?"]
+    output = ~S|"abc\""|
+    assert(minimize(input) == output)
+
+    input = ["\"abc\\", "\"\""]
+    output = ~S|"abc\""|
+    assert(minimize(input) == output)
+
+    input = ["\"abc\\", ?", ?"]
+    output = ~S|"abc\""|
+    assert(minimize(input) == output)
+
+    input = ["\"abc", "\\", ?", ?"]
+    output = ~S|"abc\""|
+    assert(minimize(input) == output)
+
+    input = ["\"abc\\", "\\", ?"]
+    output = ~S|"abc\\"|
+    assert(minimize(input) == output)
+  end
 end