Skip to content

Commit

Permalink
Change to autosort config
Browse files Browse the repository at this point in the history
  • Loading branch information
emkguts committed Feb 28, 2025
1 parent 19e26a3 commit 702bd71
Show file tree
Hide file tree
Showing 7 changed files with 110 additions and 52 deletions.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -99,12 +99,12 @@ in `.formatter.exs` to fine tune your setup:

| Option | Description | Default |
| -------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------- |
| `:autosort` | Sort all maps and/or defstructs in your codebase. Quokka will skip sort maps that have comments inside them, though sorting can still be forced with `# quokka:sort`. | `[]` |
| `:files` | Quokka gets files from `.formatter.exs[:inputs]`. However, in some cases you may need to selectively exclude/include files you wish to still run in `mix format`, but have different behavior with Quokka. | `%{included: [], excluded: []}` (all files included, none excluded) |
| `:only` | Only include the given modules. The special `:line_length` option excludes all changes except line length fixups. | `[]` (all modules included) |
| `:exclude` | Exclude the given modules. This is just a convenience function that filters from the `:only` list. | `[]` (all modules included) |
| `:inefficient_function_rewrites` | Rewrite inefficient functions to more efficient form | `true` |
| `:piped_function_exclusions` | Allows you to specify certain functions that won't be rewritten into a pipe. Particularly good for things like Ecto's `subquery` macro. | `[]` |
| `:sort_all_maps` | Sort all maps in your codebase. This can be disabled by setting `sort_all_maps: false` in the config. Quokka will skip sort maps that have comments inside them, though sorting can still be forced with `# quokka:sort`. | `true` |

## Credo inspired rewrites

Expand Down
5 changes: 3 additions & 2 deletions docs/comment_directives.md
Original file line number Diff line number Diff line change
Expand Up @@ -58,12 +58,13 @@ a_var =
]
```

## Sort all maps
## Autosort

By default, Quokka will sort all maps in your codebase. This can be disabled by setting `sort_all_maps: false` in the config. Quokka will skip sort maps that have comments inside them, though sorting can still be forced with `# quokka:sort`. Finally, when `sort_all_maps` is true, a specific map can be skipped by adding `# quokka:skip-sort` on the line above the map.
Quokka can autosort maps and defstructs. To enable this feature, set `autosort: [:map, :defstruct]` in the config. Quokka will skip sort maps that have comments inside them, though sorting can still be forced with `# quokka:sort`. Finally, when `sort_all_maps` is true, a specific map can be skipped by adding `# quokka:skip-sort` on the line above the map.

#### Examples

When `autosort: [:map]` is enabled:
```elixir
# quokka:skip-sort
%{c: 3, b: 2, a: 1}
Expand Down
10 changes: 5 additions & 5 deletions lib/quokka/config.ex
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ defmodule Quokka.Config do
@key,
# quokka:sort
%{
autosort: config[:autosort] || [],
block_pipe_exclude: credo_opts[:block_pipe_exclude] || [],
block_pipe_flag: credo_opts[:block_pipe_flag] || false,
directories_excluded: Map.get(config[:files] || %{}, :excluded, []),
Expand All @@ -95,7 +96,6 @@ defmodule Quokka.Config do
piped_function_exclusions: config[:piped_function_exclusions] || [],
rewrite_multi_alias: credo_opts[:rewrite_multi_alias] || false,
single_pipe_flag: credo_opts[:single_pipe_flag] || false,
sort_all_maps: config[:sort_all_maps] || true,
sort_order: credo_opts[:sort_order] || :alpha,
strict_module_layout_order: strict_module_layout_order ++ (default_order -- strict_module_layout_order),
zero_arity_parens: credo_opts[:zero_arity_parens] || false
Expand Down Expand Up @@ -139,6 +139,10 @@ defmodule Quokka.Config do
get(:sort_order)
end

def autosort() do
get(:autosort)
end

def block_pipe_flag?() do
get(:block_pipe_flag)
end
Expand Down Expand Up @@ -199,10 +203,6 @@ defmodule Quokka.Config do
get(:single_pipe_flag)
end

def sort_all_maps?() do
get(:sort_all_maps)
end

def strict_module_layout_order() do
get(:strict_module_layout_order)
end
Expand Down
87 changes: 56 additions & 31 deletions lib/style/comment_directives.ex
Original file line number Diff line number Diff line change
Expand Up @@ -41,42 +41,64 @@ defmodule Quokka.Style.CommentDirectives do
end
end)

zipper = if Quokka.Config.sort_all_maps?(), do: sort_all_maps(zipper, ctx), else: zipper
zipper = apply_autosort(zipper, ctx)

{:halt, zipper, %{ctx | comments: comments}}
end

defp sort_all_maps(zipper, ctx) do
{zipper, skip_sort_lines} =
Enum.reduce(
Enum.filter(ctx.comments, &(&1.text == "# quokka:skip-sort")),
{zipper, MapSet.new()},
fn comment, {z, lines} ->
{z, lines |> MapSet.put(comment.line) |> MapSet.put(comment.line + 1)}
end
)

Zipper.traverse(zipper, fn z ->
node = Zipper.node(z)
node_line = Style.meta(node)[:line]

# Skip sorting if the node has a skip-sort directive on its line or the line above
should_skip = node_line && MapSet.member?(skip_sort_lines, node_line)

# Skip sorting maps with comments to avoid disrupting comment placement. Can still force sorting with # quokka:sort
case !should_skip && !has_comments_inside?(node, ctx.comments) && node do
{:%{}, _, _} ->
{sorted, _} = sort(node, [])
Zipper.replace(z, sorted)

{:%, _, [_, {:%{}, _, _}]} ->
{sorted, _} = sort(node, [])
Zipper.replace(z, sorted)
defp apply_autosort(zipper, ctx) do
autosort_types = Quokka.Config.autosort()

_ ->
z
end
end)
if Enum.empty?(autosort_types) do
zipper
else
{zipper, skip_sort_lines} =
Enum.reduce(
Enum.filter(ctx.comments, &(&1.text == "# quokka:skip-sort")),
{zipper, MapSet.new()},
fn comment, {z, lines} ->
{z, lines |> MapSet.put(comment.line) |> MapSet.put(comment.line + 1)}
end
)

Zipper.traverse(zipper, fn z ->
node = Zipper.node(z)
node_line = Style.meta(node)[:line]

# Skip sorting if the node has a skip-sort directive on its line or the line above
should_skip = node_line && MapSet.member?(skip_sort_lines, node_line)

# Skip sorting nodes with comments to avoid disrupting comment placement. Can still force sorting with # quokka:sort
case !should_skip && !has_comments_inside?(node, ctx.comments) && node do
{:%{}, _, _} ->
if :map in autosort_types do
{sorted, _} = sort(node, [])
Zipper.replace(z, sorted)
else
z
end

{:%, _, [_, {:%{}, _, _}]} ->
if :map in autosort_types do
{sorted, _} = sort(node, [])
Zipper.replace(z, sorted)
else
z
end

{:defstruct, _, _} ->
if :defstruct in autosort_types do
{sorted, _} = sort(node, [])
Zipper.replace(z, sorted)
else
z
end

_ ->
z
end
end)
end
end

# Check if there are any comments within the line range of a node
Expand Down Expand Up @@ -113,16 +135,19 @@ defmodule Quokka.Style.CommentDirectives do
{{:defstruct, meta, [list]}, comments}
end

# map update with a keyword list
defp sort({:%{}, meta, [{:|, _, [var, keyword_list]}]}, comments) do
{{:__block__, meta, [keyword_list]}, comments} = sort({:__block__, meta, [keyword_list]}, comments)
{{:%{}, meta, [{:|, meta, [var, keyword_list]}]}, comments}
end

# map
defp sort({:%{}, meta, list}, comments) when is_list(list) do
{{:__block__, meta, [list]}, comments} = sort({:__block__, meta, [list]}, comments)
{{:%{}, meta, list}, comments}
end

# struct map
defp sort({:%, m, [struct, map]}, comments) do
{map, comments} = sort(map, comments)
{{:%, m, [struct, map]}, comments}
Expand Down
56 changes: 45 additions & 11 deletions test/style/comment_directives_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@ defmodule Quokka.Style.CommentDirectivesTest do
@moduledoc false
use Quokka.StyleCase, async: true

describe "sort" do
test "test" do
Mimic.stub(Quokka.Config, :sort_all_maps?, fn -> true end)
describe "autosort" do
test "autosorts map update keys" do
Mimic.stub(Quokka.Config, :autosort, fn -> [:map] end)

assert_style(
"""
Expand All @@ -26,8 +26,8 @@ defmodule Quokka.Style.CommentDirectivesTest do
)
end

test "sorts all maps if the config is set" do
Mimic.stub(Quokka.Config, :sort_all_maps?, fn -> true end)
test "autosorts maps" do
Mimic.stub(Quokka.Config, :autosort, fn -> [:map] end)

assert_style(
"""
Expand All @@ -39,17 +39,17 @@ defmodule Quokka.Style.CommentDirectivesTest do
)
end

test "skips sorting maps when config is set and there is a skip-sort directive" do
Mimic.stub(Quokka.Config, :sort_all_maps?, fn -> true end)
test "skips autosorting maps when there is a skip-sort directive" do
Mimic.stub(Quokka.Config, :autosort, fn -> [:map] end)

assert_style("""
# quokka:skip-sort
%{c: 2, b: 3, a: 4, d: 1}
""")
end

test "skips sorting maps when config is set and there is a comment inside the map" do
Mimic.stub(Quokka.Config, :sort_all_maps?, fn -> true end)
test "skips autosorting maps when there is a comment inside the map" do
Mimic.stub(Quokka.Config, :autosort, fn -> [:map] end)

assert_style("""
%{
Expand All @@ -61,8 +61,8 @@ defmodule Quokka.Style.CommentDirectivesTest do
""")
end

test "sorts maps when config is set, map has comment, and there is a sort directive" do
Mimic.stub(Quokka.Config, :sort_all_maps?, fn -> true end)
test "autosorts maps when map contains comment and there is a sort directive" do
Mimic.stub(Quokka.Config, :autosort, fn -> [:map] end)

assert_style(
"""
Expand All @@ -86,6 +86,40 @@ defmodule Quokka.Style.CommentDirectivesTest do
)
end

test "autosorts module attributes" do
Mimic.stub(Quokka.Config, :autosort, fn -> [:map] end)

assert_style("""
@attr %{c: 1, b: 2, a: 3}
""",
"""
@attr %{a: 3, b: 2, c: 1}
"""
)
end

test "autosorts defstructs" do
Mimic.stub(Quokka.Config, :autosort, fn -> [:defstruct] end)

assert_style("""
defstruct c: 1, b: 2, a: 3
""",
"""
defstruct a: 3, b: 2, c: 1
"""
)

assert_style("""
defstruct [c, b, a]
""",
"""
defstruct [a, b, c]
"""
)
end
end

describe "sort" do
test "we dont just sort by accident" do
assert_style "[:c, :b, :a]"
end
Expand Down
1 change: 0 additions & 1 deletion test/style/defs_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ defmodule Quokka.Style.DefsTest do

setup do
stub(Quokka.Config, :zero_arity_parens?, fn -> true end)
stub(Quokka.Config, :sort_all_maps?, fn -> false end)

:ok
end
Expand Down
1 change: 0 additions & 1 deletion test/style/pipes_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ defmodule Quokka.Style.PipesTest do
setup do
stub(Quokka.Config, :single_pipe_flag?, fn -> true end)
stub(Quokka.Config, :refactor_pipe_chain_starts?, fn -> true end)
stub(Quokka.Config, :sort_all_maps?, fn -> false end)

:ok
end
Expand Down

0 comments on commit 702bd71

Please sign in to comment.