Skip to content

Commit

Permalink
Cannot format doctest with escaped string (#15)
Browse files Browse the repository at this point in the history
* Add failing test

* Do not use private module Code.Formatter

* Document known limitation of double-escaped quotes

* Update changelog
  • Loading branch information
angelikatyborska authored Apr 1, 2024
1 parent 7bd8635 commit daa05fc
Show file tree
Hide file tree
Showing 7 changed files with 184 additions and 36 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
## Unreleased

- Support opaque types in doctest results (e.g. `#User<name: "", ...>`).
- Do not crash when doctests contain double-escaped quotes. Instead, print a warning and leave the code snippet unformatted.

## 0.2.1 (2024-03-22)

Expand Down
24 changes: 24 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,30 @@ This formatter plugin will not only format the Elixir code inside the doctest, b

## Known limitations

### Double-escaped quotes

This plugin cannot handle doctests with double-escaped quotes like this:

```elixir
@doc """
iex> "\\""
~S(")
"""
```

The above is a valid doctest, but this plugin is unable to parse it into an AST and then correctly back into a string. Such cases will produce logger warnings.

You can ignore the warnings and accept that this doctests won't be formatted, or you can try the below workaround.

The workaround is to rewrite the whole `@doc`/`@moduledoc` attribute using the `sigil_S`, which does not allow escape characters. This doctest will work exactly the same as the one above, and it will get formatted by this plugin:

```elixir
@doc ~S"""
iex> "\""
~S(")
"""
```

### Dynamic value

This plugin will only format string literals and `s`/`S` sigil literal values of `@doc`/`@moduledoc`. It will not format strings with interpolation or other dynamic values. For example:
Expand Down
127 changes: 95 additions & 32 deletions lib/doctest_formatter/formatter.ex
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ defmodule DoctestFormatter.Formatter do
@moduledoc false

alias DoctestFormatter.{Parser, Indentation, OtherContent, DoctestExpression}
require Logger

defp default_elixir_line_length, do: 98

Expand All @@ -15,28 +16,52 @@ defmodule DoctestFormatter.Formatter do
emit_warnings: false
] ++ opts

doc_metadata = %{
file: Keyword.get(opts, :file, "nofile"),
loc_start: 0,
attribute_name: nil
}

{forms, comments} = Code.string_to_quoted_with_comments!(content, to_quoted_opts)

to_algebra_opts = [comments: comments] ++ opts
to_algebra_opts = [comments: comments, escape: false] ++ opts

forms =
Macro.prewalk(forms, fn node ->
case node do
{:@, meta1, [{attribute_name, meta2, [{:__block__, meta3, [doc_content]}]}]}
when attribute_name in [:doc, :moduledoc] and is_binary(doc_content) ->
formatted_doc_content = format_doc_content(doc_content, opts)
doc_metadata = %{
doc_metadata
| loc_start: Keyword.get(meta3, :line, 0),
attribute_name: attribute_name
}

formatted_doc_content = format_doc_content(doc_content, opts, doc_metadata)
{:@, meta1, [{attribute_name, meta2, [{:__block__, meta3, [formatted_doc_content]}]}]}

{:@, meta1, [{attribute_name, meta2, [doc_content]}]}
when attribute_name in [:doc, :moduledoc] and is_binary(doc_content) ->
formatted_doc_content = format_doc_content(doc_content, opts)
doc_metadata = %{
doc_metadata
| loc_start: Keyword.get(meta2, :line, 0),
attribute_name: attribute_name
}

formatted_doc_content = format_doc_content(doc_content, opts, doc_metadata)
{:@, meta1, [{attribute_name, meta2, [formatted_doc_content]}]}

{:@, meta1,
[{attribute_name, meta2, [{sigil, meta3, [{:<<>>, meta4, [doc_content]}, []]}]}]}
when attribute_name in [:doc, :moduledoc] and is_binary(doc_content) and
sigil in [:sigil_S, :sigil_s] ->
formatted_doc_content = format_doc_content(doc_content, opts)
doc_metadata = %{
doc_metadata
| loc_start: Keyword.get(meta4, :line, 0),
attribute_name: attribute_name
}

formatted_doc_content = format_doc_content(doc_content, opts, doc_metadata)

{:@, meta1,
[
Expand All @@ -50,51 +75,57 @@ defmodule DoctestFormatter.Formatter do
end)

forms
|> Code.Formatter.to_algebra(to_algebra_opts)
|> Code.quoted_to_algebra(to_algebra_opts)
|> Inspect.Algebra.format(default_elixir_line_length())
|> Kernel.++(["\n"])
|> IO.iodata_to_binary()
end

defp format_doc_content(doc_content, opts) do
defp format_doc_content(doc_content, opts, doc_metadata) do
Parser.parse(doc_content)
|> Enum.flat_map(fn chunk ->
case chunk do
%OtherContent{} -> chunk.lines
%DoctestExpression{} -> do_format_expression(chunk, opts)
%DoctestExpression{} -> do_format_expression(chunk, opts, doc_metadata)
end
end)
|> Enum.join("\n")
end

def do_format_expression(%DoctestExpression{result: nil} = chunk, opts) do
format_lines(chunk, opts)
def do_format_expression(%DoctestExpression{result: nil} = chunk, opts, doc_metadata) do
format_lines(chunk, opts, doc_metadata)
end

def do_format_expression(%DoctestExpression{} = chunk, opts) do
format_lines(chunk, opts) ++ format_result(chunk, opts)
def do_format_expression(%DoctestExpression{} = chunk, opts, doc_metadata) do
format_lines(chunk, opts, doc_metadata) ++ format_result(chunk, opts, doc_metadata)
end

defp format_lines(chunk, opts) do
defp format_lines(chunk, opts, doc_metadata) do
desired_line_length = Keyword.get(opts, :line_length, default_elixir_line_length())

line_length =
desired_line_length - elem(chunk.indentation, 1) - String.length(get_prompt(chunk, 0))

opts = Keyword.put(opts, :line_length, line_length)

chunk.lines
|> Enum.join("\n")
|> Code.format_string!(opts)
|> IO.iodata_to_binary()
|> String.split("\n")
string = Enum.join(chunk.lines, "\n")

case try_format_string(string, opts, doc_metadata) do
{:ok, formatted} ->
formatted
|> IO.iodata_to_binary()
|> String.split("\n")

:error ->
chunk.lines
end
|> Enum.with_index()
|> Enum.map(fn {line, index} ->
Indentation.indent(get_prompt(chunk, index) <> line, chunk.indentation)
end)
end

defp format_result(chunk, opts) do
defp format_result(chunk, opts, doc_metadata) do
desired_line_length = Keyword.get(opts, :line_length, default_elixir_line_length())

line_length = desired_line_length - elem(chunk.indentation, 1)
Expand All @@ -104,21 +135,27 @@ defmodule DoctestFormatter.Formatter do
chunk.result
|> Enum.join("\n")

string_result =
if exception_result?(string_result) || opaque_type_result?(string_result) do
string_result
|> String.trim()
else
string_result
|> Code.format_string!(opts)
|> IO.iodata_to_binary()
if exception_result?(string_result) || opaque_type_result?(string_result) do
string_result
|> String.trim()
|> String.split("\n")
|> Enum.map(fn line ->
Indentation.indent(line, chunk.indentation)
end)
else
case try_format_string(string_result, opts, doc_metadata) do
{:ok, formatted} ->
formatted
|> IO.iodata_to_binary()
|> String.split("\n")
|> Enum.map(fn line ->
Indentation.indent(line, chunk.indentation)
end)

:error ->
chunk.result
end

string_result
|> String.split("\n")
|> Enum.map(fn line ->
Indentation.indent(line, chunk.indentation)
end)
end
end

def exception_result?(string) do
Expand Down Expand Up @@ -146,4 +183,30 @@ defmodule DoctestFormatter.Formatter do

"#{prompt_text}#{iex_line_number}> "
end

defp try_format_string(string, opts, doc_metadata) do
try do
{:ok, Code.format_string!(string, opts)}
rescue
error in SyntaxError ->
message =
"""
The @#{doc_metadata.attribute_name} attribute on #{Path.relative_to_cwd(doc_metadata.file)}:#{doc_metadata.loc_start} contains a doctest with some code that couldn't be formatted.
The code:
#{string}
The error:
#{inspect(error, pretty: true)}
If this doctests compiles and passes when running `mix test`, then the problem lies with the formatter plugin `doctest_formatter`. Please check the list on known limitations of the plugin (https://github.com/angelikatyborska/doctest_formatter/#known-limitations). If none of them apply to your code, please open an issue (https://github.com/angelikatyborska/doctest_formatter/issues).
"""

Logger.warning(message)

:error
end
end
end
23 changes: 23 additions & 0 deletions test/doctest_formatter/formatter_test.exs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
defmodule DoctestFormatter.FormatterTest do
use ExUnit.Case

import ExUnit.CaptureLog
import DoctestFormatter.Formatter

test "exception_result?/1" do
Expand Down Expand Up @@ -1117,4 +1118,26 @@ defmodule DoctestFormatter.FormatterTest do
assert output == desired_output
end
end

describe "format/2 on content loaded from file" do
# bring to light bugs that might be hidden because of inline-heredoc-string-code formatting, like in the above test files
test "double-escaped quotes cannot be helped, prints a warning and leaves unchanged" do
input =
File.read!(Path.join(__DIR__, "../fixtures/escaped_quotes.ex"))

desired_output =
File.read!(Path.join(__DIR__, "../fixtures/escaped_quotes_desired_output.ex"))

io =
capture_log(fn ->
output = format(input, [])
assert output == desired_output
end)

assert io =~
"[warning] The @doc attribute on nofile:3 contains a doctest with some code that couldn't be formatted."

assert io =~ "SyntaxError"
end
end
end
8 changes: 4 additions & 4 deletions test/doctest_formatter/parser_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,8 @@ defmodule DoctestFormatter.ParserTest do
%DoctestExpression{lines: ["1 + 2"], result: ["3"], indentation: {:spaces, 0}}
]

assert parse(" iex> 1 + 2\n 3") == [
%DoctestExpression{lines: ["1 + 2"], result: [" 3"], indentation: {:spaces, 2}}
assert parse(" iex> 1 + 2\n 3") == [
%DoctestExpression{lines: [" 1 + 2"], result: [" 3"], indentation: {:spaces, 2}}
]

assert parse(" iex> 1 + 2\n3") == [
Expand All @@ -73,9 +73,9 @@ defmodule DoctestFormatter.ParserTest do
}
]

assert parse(" iex> 1 +\n ...> 2 +\n ...> 4\n 7") == [
assert parse(" iex> 1 +\n ...> 2 +\n ...> 4\n 7") == [
%DoctestExpression{
lines: ["1 +", "2 +", "4"],
lines: [" 1 +", " 2 +", " 4"],
result: [" 7"],
indentation: {:spaces, 2}
}
Expand Down
19 changes: 19 additions & 0 deletions test/fixtures/escaped_quotes.ex
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
defmodule EscapedQuotes do
# this doctest will not be fully formatted
@doc """
iex> %{
iex> data: "{\\"supply\\": 100}"
iex> }
%{data:
"{\\"supply\\": 100}"}
"""

# but this one will
@doc ~S"""
iex> %{
...> data: "{\"supply\": 100}"
...> }
%{data:
"{\"supply\": 100}"}
"""
end
18 changes: 18 additions & 0 deletions test/fixtures/escaped_quotes_desired_output.ex
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
defmodule EscapedQuotes do
# this doctest will not be fully formatted
@doc """
iex> %{
...> data: "{\\"supply\\": 100}"
...> }
%{data:
"{\\"supply\\": 100}"}
"""

# but this one will
@doc ~S"""
iex> %{
...> data: "{\"supply\": 100}"
...> }
%{data: "{\"supply\": 100}"}
"""
end

0 comments on commit daa05fc

Please sign in to comment.