Skip to content

Commit

Permalink
de-alias when sorting module directives (#155)
Browse files Browse the repository at this point in the history
closes #137

fixes what may be styler's longest standing bug/user complaint/onboarding nightmare.
horay!
  • Loading branch information
novaugust authored Apr 8, 2024
1 parent db80ca9 commit aac822c
Show file tree
Hide file tree
Showing 4 changed files with 88 additions and 56 deletions.
37 changes: 1 addition & 36 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -108,42 +108,7 @@ Once styled the first time, future styling formats shouldn't take noticeably mor

### Troubleshooting: Compilation broke due to Module Directive rearrangement

Sometimes naively moving Module Directives around can break compilation.

Here's helpers on how to manually fix that and have a happy styling for the rest of
your codebase's life.

#### Alias dependency

If you have an alias that, for example, a `@behaviour` relies on, compilation will break after your first run.
This requires one-time manual fixing to get your repo in line with Styler's standards.

For example, if your code was this:
```elixir
defmodule MyModule do
@moduledoc "Implements MyBehaviour!"
alias Deeply.Nested.MyBehaviour
@behaviour MyBehaviour
...
end
```

then Styler will style the file like this, which cannot compile due to `MyBehaviour` not being defined.

```elixir
defmodule MyModule do
@moduledoc "Implements MyBehaviour!"
@behaviour MyBehaviour # <------ compilation error, MyBehaviour is not defined!

alias Deeply.Nested.MyBehaviour

...
end
```

A simple solution is to manually expand the alias with a find-replace-all like:
`@behaviour MyBehaviour` -> `@behaviour Deeply.Nested.MyBehaviour`. It's important to specify that you only want to
find-replace with the `@behaviour` prefix or you'll unintentially expand `MyBehaviour` everywhere in the codebase.
Styler naively moves module attributes, which can break compilation. For now, the only fix is some elbow grease.

#### Module Attribute dependency

Expand Down
36 changes: 34 additions & 2 deletions lib/style/module_directives.ex
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ defmodule Styler.Style.ModuleDirectives do
**This can break your code.**
### Strict Layout: alias referents
### Strict Layout
Modules directives are sorted into the following order:
Expand Down Expand Up @@ -218,6 +218,7 @@ defmodule Styler.Style.ModuleDirectives do
requires = expand_and_sort(directives[:require] || [])

{aliases, requires, nondirectives} = lift_aliases(aliases, requires, nondirectives)
earliest_alias_line = aliases |> Stream.map(fn {_, meta, _} -> meta[:line] end) |> Enum.min(fn -> nil end)

directives =
[
Expand All @@ -229,7 +230,8 @@ defmodule Styler.Style.ModuleDirectives do
aliases,
requires
]
|> Enum.concat()
|> Stream.concat()
|> Enum.map(&dealias(&1, aliases, earliest_alias_line))
|> Style.fix_line_numbers(List.first(nondirectives))

# the # of aliases can be decreased during sorting - if there were any, we need to be sure to write the deletion
Expand Down Expand Up @@ -367,6 +369,36 @@ defmodule Styler.Style.ModuleDirectives do
|> Zipper.node()
end

# if a behaviour/use/import/whatever relied on an `alias` above it, de-aliasing
# fixes that node to have the full reference again - which is necessary since the alias will be moving below it
defp dealias(node, aliases, earliest_alias_line)
# skip alias/require nodes, as they don't need dealiasing
defp dealias({:alias, _, _} = ast, _, _), do: ast
defp dealias({:require, _, _} = ast, _, _), do: ast
# no aliases, so nothing to de-alias
defp dealias(ast, [], _), do: ast

defp dealias({_, meta, _} = ast, aliases, earliest_alias_line) do
line = meta[:line]

if line < earliest_alias_line do
ast
else
dealiases =
aliases
|> Enum.filter(fn {_, meta, _} -> meta[:line] < line end)
|> Map.new(fn {:alias, _, [{:__aliases__, _, aliases}]} -> {List.last(aliases), aliases} end)

Macro.prewalk(ast, fn
{:__aliases__, meta, [first | rest]} = ast ->
if dealias = dealiases[first], do: {:__aliases__, meta, dealias ++ rest}, else: ast

ast ->
ast
end)
end
end

defp expand_and_sort(directives) do
# sorting is done with `downcase` to match Credo
directives
Expand Down
35 changes: 19 additions & 16 deletions test/style/module_directives/alias_lifting_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -155,24 +155,27 @@ defmodule Styler.Style.ModuleDirectives.AliasLiftingTest do
end

test "lifts in modules with only-child bodies" do
assert_style """
defmodule A do
def lift_me() do
A.B.C.foo()
A.B.C.baz()
assert_style(
"""
defmodule A do
def lift_me() do
A.B.C.foo()
A.B.C.baz()
end
end
end
""","""
defmodule A do
@moduledoc false
alias A.B.C
def lift_me do
C.foo()
C.baz()
""",
"""
defmodule A do
@moduledoc false
alias A.B.C
def lift_me do
C.foo()
C.baz()
end
end
end
"""
"""
)
end

describe "comments stay put" do
Expand Down
36 changes: 34 additions & 2 deletions test/style/module_directives_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -224,9 +224,9 @@ defmodule Styler.Style.ModuleDirectivesTest do
@behaviour Lawful
use B
use A
use A.A
import A
import A.A
import C
alias A.A
Expand Down Expand Up @@ -548,4 +548,36 @@ defmodule Styler.Style.ModuleDirectivesTest do

assert_style "@derive Inspect"
end

test "de-aliases use/behaviour/import/moduledoc" do
assert_style(
"""
defmodule MyModule do
alias A.B.C
@moduledoc "Implements \#{C.foo()}!"
alias D.F.C
import C
alias G.H.C
@behaviour C
alias Z.X.C
use SomeMacro, with: C
end
""",
"""
defmodule MyModule do
@moduledoc "Implements \#{A.B.C.foo()}!"
@behaviour G.H.C
use SomeMacro, with: Z.X.C
import D.F.C
alias A.B.C
alias D.F.C
alias G.H.C
alias Z.X.C
end
"""
)
end
end

0 comments on commit aac822c

Please sign in to comment.