diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c233861a..a2391e35 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -9,8 +9,13 @@ jobs: name: Ex${{matrix.elixir}}/OTP${{matrix.otp}} strategy: matrix: - elixir: ['1.15.7', '1.16.0', '1.17.0-rc.0'] - otp: ['25.1.2'] + elixir: ["1.15.8", "1.16.3", "1.17.3", "1.18.2"] + otp: ["25.3.2", "26.2.5", "27.2.4"] + exclude: + - elixir: "1.15.8" + otp: "27.2.4" + - elixir: "1.16.3" + otp: "27.2.4" steps: - uses: actions/checkout@v4 - uses: erlef/setup-beam@v1 diff --git a/.tool-versions b/.tool-versions index 16f4970a..0b6770c6 100644 --- a/.tool-versions +++ b/.tool-versions @@ -1,2 +1,2 @@ -erlang 26.1.2 -elixir 1.16.0-otp-26 +elixir 1.18.2-otp-27 +erlang 27.2.3 diff --git a/CHANGELOG.md b/CHANGELOG.md index 528daf90..62da55e2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,231 @@ they can and will change without that change being reflected in Styler's semantic version. ## main +## 1.4.2 + +### Fixes + +- fix comment misplacement for large comment blocks in config files and `# styler:sort` (#230, h/t @cschmatzler) + +## 1.4.1 + +### Improvements + +- `to_timeout/1` rewrites to use the next largest unit in some simple instances + + ```elixir + # before + to_timeout(second: 60 * m) + to_timeout(day: 7) + # after + to_timeout(minute: m) + to_timeout(week: 1) + ``` + +### Fixes + +- fixed styler raising when encountering invalid function definition ast + +## 1.4.0 + +- A very nice change in alias lifting means Styler will make sure that your code is _using_ the aliases that it's specified. +- Shoutout to the smartrent folks for finding pipifying recursion issues +- Elixir 1.17 improvements and fixes +- Elixir 1.19-dev: delete struct updates + +Read on for details. + +### Improvements + +#### Alias Lifting + +This release taught Styler to try just that little bit harder when doing alias lifting. + +- general improvements around conflict detection, lifting in more correct places and fewer incorrect places (#193, h/t @jsw800) +- use knowledge of existing aliases to shorten invocations (#201, h/t me) + + example: + + alias A.B.C + + A.B.C.foo() + A.B.C.bar() + A.B.C.baz() + + becomes: + + alias A.B.C + + C.foo() + C.bar() + C.baz() + +#### Struct Updates => Map Updates + +1.19 deprecates struct update syntax in favor of map update syntax. + +```elixir +# This +%Struct{x | y} +# Styles to this +%{x | y} +``` + +**WARNING** Double check your diffs to make sure your variable is pattern matching against the same struct if you want to harness 1.19's type checking features. Apologies to folks who hoped Styler would do this step for you <3 (#199, h/t @SteffenDE) + +#### Ex1.17+ + +- Replace `:timer.units(x)` with the new `to_timeout(unit: x)` for `hours|minutes|seconds` (This style is only applied if you're on 1.17+) + +### Fixes + +- `pipes`: handle pipifying when the first arg is itself a pipe: `c(a |> b, d)` => `a |> b() |> c(d)` (#214, h/t @kybishop) +- `pipes`: handle pipifying nested functions `d(c(a |> b))` => `a |> b |> c() |> d` (#216, h/t @emkguts) +- `with`: fix a stabby `with` `, else: (_ -> :ok)` being rewritten to a case (#219, h/t @iamhassangm) + +## 1.3.3 + +### Improvements + +- `with do: body` and variations with no arrows in the head will be rewritten to just `body` +- `# styler:sort` will sort arbitrary ast nodes within a `do end` block: + + Given: + + # styler:sort + my_macro "some arg" do + another_macro :q + another_macro :w + another_macro :e + another_macro :r + another_macro :t + another_macro :y + end + + We get + + # styler:sort + my_macro "some arg" do + another_macro :e + another_macro :q + another_macro :r + another_macro :t + another_macro :w + another_macro :y + end + +### Fixes + +- fix a bug in comment-movement when multiple `# styler:sort` directives are added to a file at the same time + +## 1.3.2 + +### Improvements + +- `# styler:sort` can be used to sort values of key-value pairs. eg, sort the value of a single key in a map (Closes #208, h/t @ypconstante) + +## 1.3.1 + +### Improvements + +- `# styler:sort` now works with maps and the `defstruct` macro + +### Fixes + +- `# styler:sort` no longer blows up on keyword lists :X + +## 1.3.0 + +### Improvements + +#### `# styler:sort` Styler's first comment directive + +Styler will now keep a user-designated list or wordlist (`~w` sigil) sorted as part of formatting via the use of comments. Elements of the list are sorted by their string representation. + +The intention is to remove comments to humans, like `# Please keep this list sorted!`, in favor of comments to robots: `# styler:sort`. Personally speaking, Styler is much better at alphabetical-order than I ever will be. + +To use the new directive, put it on the line before a list or wordlist. + +This example: + +```elixir +# styler:sort +[:c, :a, :b] + +# styler:sort +~w(a list of words) + +# styler:sort +@country_codes ~w( + en_US + po_PO + fr_CA + ja_JP +) + +# styler:sort +a_var = + [ + Modules, + In, + A, + List + ] +``` + +Would yield: + +```elixir +# styler:sort +[:a, :b, :c] + +# styler:sort +~w(a list of words) + +# styler:sort +@country_codes ~w( + en_US + fr_CA + ja_JP + po_PO +) + +# styler:sort +a_var = + [ + A, + In, + List, + Modules + ] +``` + +## 1.2.1 + +### Fixes + +* `|>` don't pipify when the call is itself in a pipe (aka don't touch `a |> b(c |> d() |>e()) |> f()`) (Closes #204, h/t @paulswartz) + +## 1.2.0 + +### Improvements + +* `pipes`: pipe-ifies when first arg to a function is a pipe. reach out if this happens in unstylish places in your code (Closes #133) +* `pipes`: unpiping assignments will make the assignment one-line when possible (Closes #181) +* `deprecations`: 1.18 deprecations + * `List.zip` => `Enum.zip` + * `first..last = range` => `first..last//_ = range` + +### Fixes + +* `pipes`: optimizations are less likely to move comments (Closes #176) + +## 1.1.2 + +### Improvements + +* Config Sorting: improve comment handling when only sorting a few nodes (Closes #187) + ## 1.1.1 ### Improvements diff --git a/README.md b/README.md index 1a76af66..a9c2ec74 100644 --- a/README.md +++ b/README.md @@ -11,17 +11,18 @@ You can learn more about the history, purpose and implementation of Styler from ## Features -- auto-fixes [many credo rules](docs/credo.md), meaning you can turn them off to speed credo up -- [keeps a strict module layout](docs/module_directives.md#directive-organization) - - alphabetizes module directives -- [extracts repeated aliases](docs/module_directives.md#alias-lifting) -- [makes your pipe chains pretty as can be](docs/pipes.md) - - pipes and unpipes function calls based on the number of calls - - optimizes standard library calls (`a |> Enum.map(m) |> Enum.into(Map.new)` => `Map.new(a, m)`) -- replaces strings with sigils when the string has many escaped quotes -- ... and so much more - -[See our Rewrites documentation on hexdocs](https://hexdocs.pm/styler/styles.html) +Styler fixes a plethora of elixir style and optimization issues automatically as part of mix format. + +[See Styler's documentation on Hex](https://hexdocs.pm/styler/styles.html) for the comprehensive list of its features. + +The fastest way to see what all it can do you for you is to just try it out in your codebase... but here's a list of a few features to help you decide if you're interested in Styler: + +- sorts and organizes `import`/`alias`/`require` and other [module directives](docs/module_directives.md) +- keeps lists, sigils, and even arbitrary code sorted with the `# styler:sort` [comment directive](docs/comment_directives.md) +- automatically creates aliases for repeatedly referenced modules names ([_"alias lifting"_](docs/module_directives.md#alias-lifting)) +- optimizes pipe chains for [readability and performance](docs/pipes.md) +- rewrites strings as sigils when it results in fewer escapes +- auto-fixes [many credo rules](docs/credo.md), meaning you can spend less time fighting with CI ## Who is Styler for? @@ -41,7 +42,7 @@ Add `:styler` as a dependency to your project's `mix.exs`: ```elixir def deps do [ - {:styler, "~> 1.1", only: [:dev, :test], runtime: false}, + {:styler, "~> 1.4", only: [:dev, :test], runtime: false}, ] end ``` @@ -75,10 +76,54 @@ Styler's only current configuration option is `:alias_lifting_exclude`, which ac #### No Credo-Style Enable/Disable -Styler [will not add configuration](https://github.com/adobe/elixir-styler/pull/127#issuecomment-1912242143) for ad-hoc enabling/disabling of rewrites. Sorry! Its implementation simply does not support that approach. There are however many forks out there that have attempted this; please explore the [Github forks tab](https://github.com/adobe/elixir-styler/forks) to see if there's a project that suits your needs or that you can draw inspiration from. +Styler [will not add configuration](https://github.com/adobe/elixir-styler/pull/127#issuecomment-1912242143) for ad-hoc enabling/disabling of rewrites. Sorry! + +However, Smartrent has a fork of Styler named [Quokka](https://github.com/smartrent/quokka) that allows for finegrained control of Styler. Maybe it's what you're looking for. If not, you can always fork it or Styler as a starting point for your own tool! Ultimately Styler is @adobe's internal tool that we're happy to share with the world. We're delighted if you like it as is, and just as excited if it's a starting point for you to make something even better for yourself. +## WARNING: Styler can change the behaviour of your program! + +In some cases, this can introduce bugs. It goes without saying, but look over your changes before committing to main :) + +A simple example of a way Styler changes the behaviour of code is the following rewrite: + +```elixir +# Before: this case statement... +case foo do + true -> :ok + false -> :error +end + +# After: ... is rewritten by Styler to be an if statement!. +if foo do + :ok +else + :error +end +``` + +These programs are not semantically equivalent. The former would raise if `foo` returned any value other than `true` or `false`, while the latter blissfully completes. + +However, Styler is about _style_, and the `if` statement is (in our opinion) of much better style. If the exception behaviour was intentional on the code author's part, they should have written the program like this: + +```elixir +case foo do + true -> :ok + false -> :error + other -> raise "expected `true` or `false`, got: #{inspect other}" +end +``` + +Also good style! But Styler assumes that most of the time people just meant the `if` equivalent of the code, and so makes that change. If issues like this bother you, Styler probably isn't the tool you're looking for. + +Other ways Styler can change your program: + +- [`with` statement rewrites](https://github.com/adobe/elixir-styler/issues/186) +- [config file sorting](https://hexdocs.pm/styler/mix_configs.html#this-can-break-your-program) +- and likely other ways. stay safe out there! + +>>>>>>> upstream/main ## Thanks & Inspiration ### [Sourceror](https://github.com/doorgan/sourceror/) diff --git a/docs/comment_directives.md b/docs/comment_directives.md new file mode 100644 index 00000000..41dc0a63 --- /dev/null +++ b/docs/comment_directives.md @@ -0,0 +1,101 @@ +## Comment Directives + +Comment Directives are a Styler feature that let you instruct Styler to do maintain additional formatting via comments. + +The plural in the name is optimistic as there's currently only one, but who knows + +### `# styler:sort` + +Styler can keep static values sorted for your team as part of its formatting pass. To instruct it to do so, replace any `# Please keep this list sorted!` notes you wrote to your teammates with `# styler:sort` + +Sorting is done via string comparison of the code. + +Styler knows how to sort the following things: + +- lists of elements +- arbitrary code within `do end` blocks (helpful for schema-like macros) +- `~w` sigils elements +- keyword shapes (structs, maps, and keywords) + +Since you can't have comments in arbitrary places when using Elixir's formatter, +Styler will apply those sorts when they're on the righthand side fo the following operators: + +- module directives (eg `@my_dir ~w(a list of things)`) +- assignments (eg `x = ~w(a list again)`) +- `defstruct` + +#### Examples + +**Before** + +```elixir +# styler:sort +[:c, :a, :b] + +# styler:sort +~w(a list of words) + +# styler:sort +@country_codes ~w( + en_US + po_PO + fr_CA + ja_JP +) + +# styler:sort +a_var = + [ + Modules, + In, + A, + List + ] + +# styler:sort +my_macro "some arg" do + another_macro :q + another_macro :w + another_macro :e + another_macro :r + another_macro :t + another_macro :y +end +``` + +**After** + +```elixir +# styler:sort +[:a, :b, :c] + +# styler:sort +~w(a list of words) + +# styler:sort +@country_codes ~w( + en_US + fr_CA + ja_JP + po_PO +) + +# styler:sort +a_var = + [ + A, + In, + List, + Modules + ] + +# styler:sort +my_macro "some arg" do + another_macro :e + another_macro :q + another_macro :r + another_macro :t + another_macro :w + another_macro :y +end +``` diff --git a/docs/deprecations.md b/docs/deprecations.md new file mode 100644 index 00000000..45bdbe24 --- /dev/null +++ b/docs/deprecations.md @@ -0,0 +1,104 @@ +## Elixir Deprecation Rewrites + +Elixir's built-in formatter now does its own rewrites via the `--migrate` flag, but doesn't quite cover every possible automated rewrite on the hard deprecations list. Styler tries to cover the rest! + +Styler will rewrite deprecations so long as their alternative is available on the given elixir version. In other words, Styler doesn't care what version of Elixir you're using when it applies the ex-1.18 rewrites - all it cares about is that the alternative is valid in your version of elixir. + +### elixir `main` + +https://github.com/elixir-lang/elixir/blob/main/CHANGELOG.md#4-hard-deprecations + +These deprecations will be released with Elixir 1.18 + +#### `List.zip/1` + +```elixir +# Before +List.zip(list) +# Styled +Enum.zip(list) +``` + +#### `unless` + +This is covered by the Elixir Formatter with the `--migrate` flag, but Styler brings the same transformation to codebases on earlier versions of Elixir. + +Rewrite `unless x` to `if !x` + +### Change Struct Updates to Map Updates + +1.19 deprecates struct update syntax in favor of map update syntax. + +```elixir +# This +%Struct{x | y} +# Styles to this +%{x | y} +``` + +**WARNING** Double check your diffs to make sure your variable is pattern matching against the same struct if you want to harness 1.19's type checking features. + +### 1.18 + +None? + +### 1.17 + +[1.17 Deprecations](https://hexdocs.pm/elixir/1.17.0/changelog.html#4-hard-deprecations) + +- Replace `:timer.units(x)` with the new `to_timeout(unit: x)` for `hours|minutes|seconds` + +#### Range Matching Without Step + +```elixir +# Before +first..last = range +# Styled +first..last//_ = range + +# Before +def foo(x..y), do: :ok +# Styled +def foo(x..y//_), do: :ok +``` + +### 1.16 + +[1.16 Deprecations](https://hexdocs.pm/elixir/1.16.0/changelog.html#4-hard-deprecations) + +#### `File.stream!/3` `:line` and `:bytes` deprecation + +```elixir +# Before +File.stream!(path, [encoding: :utf8, trim_bom: true], :line) +# Styled +File.stream!(path, :line, encoding: :utf8, trim_bom: true) +``` + +### Explicit decreasing ranges + +In all these cases, the rewrite will only be applied when literals are being passed to the function. In other words, variables will not be traced back to their assignment, and so it is still possible to receive deprecation warnings on this issue. + +```elixir +# Before +Enum.slice(x, 1..-2) +# Styled +Enum.slice(x, 1..-2//1) + +# Before +Date.range(~D[2000-01-01], ~D[1999-01-01]) +# Styled +Date.range(~D[2000-01-01], ~D[1999-01-01], -1) +``` + +### 1.15 + +[1.15 Deprecations](https://hexdocs.pm/elixir/1.15.0/changelog.html#4-hard-deprecations) + +| Before | After | +|--------|-------| +| `Logger.warn` | `Logger.warning`| +| `Path.safe_relative_to/2` | `Path.safe_relative/2`| +| `~R/my_regex/` | `~r/my_regex/`| +| `Date.range/2` with decreasing range | `Date.range/3` *| +| `IO.read/bin_read` with `:all` option | replace `:all` with `:eof`| diff --git a/docs/module_directives.md b/docs/module_directives.md index d3a334af..c385d0b3 100644 --- a/docs/module_directives.md +++ b/docs/module_directives.md @@ -167,6 +167,20 @@ C.foo() C.bar() ``` +Styler also notices when you have a module aliased and aren't employing that alias and will do the updates for you. + +```elixir +# Given +alias My.Apps.Widget + +x = Repo.get(My.Apps.Widget, id) + +# Styled +alias My.Apps.Widget + +x = Repo.get(Widget, id) +``` + ### Collisions Styler won't lift aliases that will collide with existing aliases, and likewise won't lift any module whose name would collide with a standard library name. diff --git a/docs/pipes.md b/docs/pipes.md index 08673c94..8d800984 100644 --- a/docs/pipes.md +++ b/docs/pipes.md @@ -1,6 +1,6 @@ -# Pipe Chains +## Pipe Chains -## Pipe Start +### Pipe Start Styler will ensure that the start of a pipechain is a 0-arity function, a raw value, or a variable. @@ -120,3 +120,13 @@ map = a |> Enum.map(mapper) |> Map.new() # Styled: map = Map.new(a, mapper) ``` + +### Pipe-ify + +If the first argument to a function call is a pipe, Styler makes the function call the final pipe of the chain. + +```elixir +d(a |> b |> c) +# Styled +a |> b() |> c() |> d() +``` diff --git a/docs/styles.md b/docs/styles.md index afa1abe5..a7e89b7e 100644 --- a/docs/styles.md +++ b/docs/styles.md @@ -47,7 +47,7 @@ Note that all of the examples below also apply to pipes (`enum |> Enum.into(...) | `Enum.into(enum, %{})` | `Map.new(enum)`| | `Enum.into(enum, Map.new())` | `Map.new(enum)`| | `Enum.into(enum, Keyword.new())` | `Keyword.new(enum)`| -| `Enum.into(enum, MapSet.new())` | `Keyword.new(enum)`| +| `Enum.into(enum, MapSet.new())` | `MapSet.new(enum)`| | `Enum.into(enum, %{}, fn x -> {x, x} end)` | `Map.new(enum, fn x -> {x, x} end)`| | `Enum.into(enum, [])` | `Enum.to_list(enum)` | | `Enum.into(enum, [], mapper)` | `Enum.map(enum, mapper)`| @@ -109,7 +109,13 @@ baz |> Enum.reverse() |> Enum.concat(bop) Enum.reverse(baz, bop) ``` -## `Timex.now/0` ->` DateTime.utc_now/0` +# Before +baz |> Enum.reverse() |> Enum.concat(bop) +# Styled +Enum.reverse(baz, bop) +``` + +## `Timex.now/0` -> `DateTime.utc_now/0` Timex certainly has its uses, but knowing what stdlib date/time struct is returned by `now/0` is a bit difficult! @@ -137,51 +143,47 @@ DateTime.compare(start, end_date) == :lt DateTime.before?(start, end_date) ``` +## Implicit `try` -## Remove parenthesis from 0-arity function & macro definitions - -The author of the library disagrees with this style convention :) BUT, the wonderful thing about Styler is it lets you write code how _you_ want to, while normalizing it for reading for your entire team. The most important thing is not having to think about the style, and instead focus on what you're trying to achieve. +Styler will rewrite functions whose entire body is a try/do to instead use the implicit try syntax, per Credo's `Credo.Check.Readability.PreferImplicitTry` ```elixir -# Before -def foo() -defp foo() -defmacro foo() -defmacrop foo() +# before +def foo do + try do + throw_ball() + catch + :ball -> :caught + end +end -# Styled -def foo -defp foo -defmacro foo -defmacrop foo +# Styled: +def foo do + throw_ball() +catch + :ball -> :caught +end ``` -## Elixir Deprecation Rewrites - -### 1.15+ - -| Before | After | -|--------|-------| -| `Logger.warn` | `Logger.warning`| -| `Path.safe_relative_to/2` | `Path.safe_relative/2`| -| `~R/my_regex/` | `~r/my_regex/`| -| `Enum/String.slice/2` with decreasing ranges | add explicit steps to the range * | -| `Date.range/2` with decreasing range | `Date.range/3` *| -| `IO.read/bin_read` with `:all` option | replace `:all` with `:eof`| - -\* For both of the "decreasing range" changes, the rewrite can only be applied if the range is being passed as an argument to the function. +## Remove parenthesis from 0-arity function & macro definitions -### 1.16+ -File.stream! `:line` and `:bytes` deprecation +The author of the library disagrees with this style convention :) BUT, the wonderful thing about Styler is it lets you write code how _you_ want to, while normalizing it for reading for your entire team. The most important thing is not having to think about the style, and instead focus on what you're trying to achieve. ```elixir # Before -File.stream!(path, [encoding: :utf8, trim_bom: true], :line) +def foo() do +defp foo() do +defmacro foo() do +defmacrop foo() do + # Styled -File.stream!(path, :line, encoding: :utf8, trim_bom: true) +def foo do +defp foo do +defmacro foo do +defmacrop foo do ``` -## Putting variable matching on the right +## Variable matching on the right ```elixir # Before @@ -224,26 +226,6 @@ case foo do end ``` -## Use Implicit Try - -```elixir -# before -def foo d - try do - throw_ball() - catch - :ball -> :caught - end -end - -# Styled: -def foo d - throw_ball() -catch - :ball -> :caught -end -``` - ## Shrink Function Definitions to One Line When Possible ```elixir diff --git a/lib/style.ex b/lib/style.ex index 1553a290..184a902c 100644 --- a/lib/style.ex +++ b/lib/style.ex @@ -59,7 +59,7 @@ defmodule Styler.Style do @doc "Traverses an ast node, updating all nodes' meta with `meta_fun`" def update_all_meta(node, meta_fun), do: Macro.prewalk(node, &Macro.update_meta(&1, meta_fun)) - # useful for comparing AST without meta (line numbers, etc) interfering + @doc "prewalks ast and sets all meta to `nil`. useful for comparing AST without meta (line numbers, etc) interfering" def without_meta(ast), do: update_all_meta(ast, fn _ -> nil end) @doc """ @@ -83,6 +83,9 @@ defmodule Styler.Style do end end + def do_block?([{{:__block__, _, [:do]}, _body} | _]), do: true + def do_block?(_), do: false + @doc """ Returns a zipper focused on the nearest node where additional nodes can be inserted (a "block"). @@ -144,7 +147,7 @@ defmodule Styler.Style do comments |> Enum.map(fn comment -> if delta = Enum.find_value(shifts, fn {range, delta} -> comment.line in range && delta end) do - %{comment | line: comment.line + delta} + %{comment | line: max(comment.line + delta, 1)} else comment end @@ -167,77 +170,93 @@ defmodule Styler.Style do {directive, updated_meta, children} end - @doc """ - "Fixes" the line numbers of nodes who have had their orders changed via sorting or other methods. - This "fix" simply ensures that comments don't get wrecked as part of us moving AST nodes willy-nilly. + def max_line([_ | _] = list), do: list |> List.last() |> max_line() + + def max_line(ast) do + meta = meta(ast) - The fix is rather naive, and simply enforces the following property on the code: - A given node must have a line number less than the following node. - Et voila! Comments behave much better. + cond do + line = meta[:end_of_expression][:line] -> + line - ## In Detail + line = meta[:closing][:line] -> + line - For example, given document + true -> + {_, max_line} = + Macro.prewalk(ast, 0, fn + {_, meta, _} = ast, max -> {ast, max(meta[:line] || max, max)} + ast, max -> {ast, max} + end) - 1: defmodule ... - 2: alias B - 3: # this is foo - 4: def foo ... - 5: alias A + max_line + end + end - Sorting aliases the ast node for would put `alias A` (line 5) before `alias B` (line 2). + @doc "Sets the nodes' meta line and comments' line numbers to fit the ordering of the nodes list." + # TODO this doesn't grab comments which are floating as their own paragrpah, unconnected to a node + # they'll just be left floating where they were, then mangled with the re-ordered comments.. + def order_line_meta_and_comments(nodes, comments, first_line) do + {nodes, shifted_comments, comments, _line} = + Enum.reduce(nodes, {[], [], comments, first_line}, fn node, {n_acc, c_acc, comments, move_to_line} -> + meta = meta(node) + line = meta[:line] + last_line = max_line(node) + {mine, comments} = comments_for_lines(comments, line, last_line) - 1: defmodule ... - 5: alias A - 2: alias B - 3: # this is foo - 4: def foo ... + shift = move_to_line - (List.first(mine)[:line] || line) + 1 + shifted_node = shift_line(node, shift) + shifted_comments = Enum.map(mine, &%{&1 | line: &1.line + shift}) - Elixir's document algebra would then encounter `line: 5` and immediately dump all comments with `line <= 5`, - meaning after running through the formatter we'd end up with + move_to_line = last_line + shift + (meta[:end_of_expression][:newlines] || 0) - 1: defmodule - 2: # hi - 3: # this is foo - 4: alias A - 5: alias B - 6: - 7: def foo ... + {[shifted_node | n_acc], shifted_comments ++ c_acc, comments, move_to_line} + end) - This function fixes that by seeing that `alias A` has a higher line number than its following sibling `alias B` and so - updates `alias A`'s line to be preceding `alias B`'s line. + {Enum.reverse(nodes), Enum.sort_by(comments ++ shifted_comments, & &1.line)} + end - Running the results of this function through the formatter now no longer dumps the comments prematurely + # typical node + def meta({_, meta, _}), do: meta + # kwl tuple ala a: :b + def meta({{_, meta, _}, _}), do: meta + def meta(_), do: nil - 1: defmodule ... - 2: alias A - 3: alias B - 4: # this is foo - 5: def foo ... + @doc """ + Returns all comments "for" a node, including on the line before it. see `comments_for_lines` for more """ - def fix_line_numbers(nodes, nil), do: fix_line_numbers(nodes, 999_999) - def fix_line_numbers(nodes, {_, meta, _}), do: fix_line_numbers(nodes, meta[:line]) - def fix_line_numbers(nodes, max), do: nodes |> Enum.reverse() |> do_fix_lines(max, []) + def comments_for_node({_, m, _} = node, comments), do: comments_for_lines(comments, m[:line], max_line(node)) - defp do_fix_lines([], _, acc), do: acc + @doc """ + Gets all comments in range start_line..last_line, and any comments immediately before start_line.s - defp do_fix_lines([{_, meta, _} = node | nodes], max, acc) do - line = meta[:line] + 1. code + 2. # a + 3. # b + 4. code # c + 5. # d + 6. code + 7. # e - # the -2 is just an ugly hack to leave room for one-liner comments and not hijack them. - if line > max, - do: do_fix_lines(nodes, max, [shift_line(node, max - line - 2) | acc]), - else: do_fix_lines(nodes, line, [node | acc]) + here, comments_for_lines(comments, 4, 6) is "a", "b", "c", "d" + """ + def comments_for_lines(comments, start_line, last_line) do + comments |> Enum.reverse() |> comments_for_lines(start_line, last_line, [], []) end - # @TODO can i shortcut and just return end_of_expression[:line] when it's available? - def max_line(ast) do - {_, max_line} = - Macro.prewalk(ast, 0, fn - {_, meta, _} = ast, max -> {ast, max(meta[:line] || max, max)} - ast, max -> {ast, max} - end) - - max_line + defp comments_for_lines([%{line: line} = comment | rev_comments], start, last, match, acc) do + cond do + # after our block - no match + line > last -> comments_for_lines(rev_comments, start, last, match, [comment | acc]) + # after start, before last -- it's a match! + line >= start -> comments_for_lines(rev_comments, start, last, [comment | match], acc) + # this is a comment immediately before start, which means it's modifying this block... + # we count that as a match, and look above it to see if it's a multiline comment + line == start - 1 -> comments_for_lines(rev_comments, start - 1, last, [comment | match], acc) + # comment before start - we've thus iterated through all comments which could be in our range + true -> {match, Enum.reverse(rev_comments, [comment | acc])} + end end + + defp comments_for_lines([], _, _, match, acc), do: {match, acc} end diff --git a/lib/style/blocks.ex b/lib/style/blocks.ex index 06051d6b..9c308ba6 100644 --- a/lib/style/blocks.ex +++ b/lib/style/blocks.ex @@ -17,17 +17,19 @@ defmodule Styler.Style.Blocks do * Credo.Check.Consistency.ParameterPatternMatching * Credo.Check.Readability.LargeNumbers * Credo.Check.Readability.ParenthesesOnZeroArityDefs + * Credo.Check.Readability.PreferImplicitTry * Credo.Check.Readability.WithSingleClause + * Credo.Check.Refactor.CaseTrivialMatches * Credo.Check.Refactor.CondStatements * Credo.Check.Refactor.RedundantWithClauseResult * Credo.Check.Refactor.WithClauses """ + @behaviour Styler.Style + alias Styler.Style alias Styler.Zipper - @behaviour Styler.Style - defguardp is_negator(n) when elem(n, 0) in [:!, :not, :!=, :!==] # case statement with exactly 2 `->` cases @@ -51,10 +53,31 @@ defmodule Styler.Style.Blocks do # to `case single_statement do success -> body; ...elses end` def run({{:with, m, [{:<-, am, [success, single_statement]}, [body, elses]]}, zm}, ctx) do {{:__block__, do_meta, [:do]}, body} = body - {{:__block__, _else_meta, [:else]}, elses} = elses + {{:__block__, _, [:else]}, elses} = elses + + elses = + case elses do + # unwrap a stab ala `, else: (_ -> :ok)`. these became literals in 1.17 + {:__block__, _, [[{:->, _, _}] = stab]} -> stab + elses -> elses + end + + # drops keyword formatting etc + do_meta = [line: do_meta[:line]] clauses = [{{:__block__, am, [:do]}, [{:->, do_meta, [[success], body]} | elses]}] + end_line = Style.max_line(elses) + 1 + + # fun fact: i added the detailed meta just because i noticed it was missing while debugging something ... + # ... and it fixed the bug 🤷 + case_meta = [ + end_of_expression: [newlines: 1, line: end_line], + do: do_meta, + end: [line: end_line], + line: m[:line] + ] + # recurse in case this new case should be rewritten to a `if`, etc - run({{:case, m, [single_statement, clauses]}, zm}, ctx) + run({{:case, case_meta, [single_statement, clauses]}, zm}, ctx) end # `with true <- x, do: bar` =>`if x, do: bar` @@ -74,103 +97,27 @@ defmodule Styler.Style.Blocks do end # Credo.Check.Refactor.WithClauses - def run({{:with, with_meta, children}, _} = zipper, ctx) when is_list(children) do - # a std lib `with` block will have at least one left arrow and a `do` body. anything else we skip ¯\_(ツ)_/¯ - arrow_or_match? = &(left_arrow?(&1) || match?({:=, _, _}, &1)) - do_block? = &match?([{{:__block__, _, [:do]}, _body} | _], &1) - - if Enum.any?(children, arrow_or_match?) and Enum.any?(children, do_block?) do - {preroll, children} = - children - |> Enum.map(fn - # `_ <- rhs` => `rhs` - {:<-, _, [{:_, _, _}, rhs]} -> rhs - # `lhs <- rhs` => `lhs = rhs` - {:<-, m, [{atom, _, nil} = lhs, rhs]} when is_atom(atom) -> {:=, m, [lhs, rhs]} - child -> child - end) - |> Enum.split_while(&(not left_arrow?(&1))) - - # after rewriting `x <- y()` to `x = y()` there are no more arrows. - # this never should've been a with statement at all! we can just replace it with assignments - if Enum.empty?(children) do - {:cont, replace_with_statement(zipper, preroll), ctx} - else - [[{{_, do_meta, _} = do_block, do_body} | elses] | reversed_clauses] = Enum.reverse(children) - {postroll, reversed_clauses} = Enum.split_while(reversed_clauses, &(not left_arrow?(&1))) - [{:<-, final_clause_meta, [lhs, rhs]} = _final_clause | rest] = reversed_clauses - - # drop singleton identity else clauses like `else foo -> foo end` - elses = - case elses do - [{{_, _, [:else]}, [{:->, _, [[left], right]}]}] -> if nodes_equivalent?(left, right), do: [], else: elses - _ -> elses - end - - {reversed_clauses, do_body} = - cond do - # Put the postroll into the body - Enum.any?(postroll) -> - {node, do_body_meta, do_children} = do_body - do_children = if node == :__block__, do: do_children, else: [do_body] - do_body = {:__block__, Keyword.take(do_body_meta, [:line]), Enum.reverse(postroll, do_children)} - {reversed_clauses, do_body} - - # Credo.Check.Refactor.RedundantWithClauseResult - Enum.empty?(elses) and nodes_equivalent?(lhs, do_body) -> - {rest, rhs} - - # no change - true -> - {reversed_clauses, do_body} - end - - do_line = do_meta[:line] - final_clause_line = final_clause_meta[:line] - - do_line = - cond do - do_meta[:format] == :keyword && final_clause_line + 1 >= do_line -> do_line - do_meta[:format] == :keyword -> final_clause_line + 1 - true -> final_clause_line - end - - do_block = Macro.update_meta(do_block, &Keyword.put(&1, :line, do_line)) - # disable keyword `, do:` since there will be multiple statements in the body - with_meta = - if Enum.any?(postroll), - do: Keyword.merge(with_meta, do: [line: with_meta[:line]], end: [line: Style.max_line(children) + 1]), - else: with_meta - - with_children = Enum.reverse(reversed_clauses, [[{do_block, do_body} | elses]]) - zipper = Zipper.replace(zipper, {:with, with_meta, with_children}) + def run({{:with, _, children}, _} = zipper, ctx) when is_list(children) do + do_block? = Enum.any?(children, &Style.do_block?/1) + arrow_or_match? = Enum.any?(children, &(left_arrow?(&1) || match?({:=, _, _}, &1))) + + cond do + # we can style this! + do_block? and arrow_or_match? -> + style_with_statement(zipper, ctx) + + # `with (head_statements) do: x (else ...)` + do_block? -> + # head statements can be the empty list, if it matters + {head_statements, [[{{:__block__, _, [:do]}, body} | _]]} = Enum.split_while(children, &(not Style.do_block?(&1))) + [first | rest] = head_statements ++ [body] + # replace this `with` statement with its headers + body + zipper = zipper |> Zipper.replace(first) |> Zipper.insert_siblings(rest) + {:cont, zipper, ctx} - cond do - # oops! RedundantWithClauseResult removed the final arrow in this. no more need for a with statement! - Enum.empty?(reversed_clauses) -> - {:cont, replace_with_statement(zipper, preroll ++ with_children), ctx} - - # recurse if the # of `<-` have changed (this `with` could now be eligible for a `case` rewrite) - Enum.any?(preroll) -> - # put the preroll before the with statement in either a block we create or the existing parent block - zipper - |> Style.find_nearest_block() - |> Zipper.prepend_siblings(preroll) - |> run(ctx) - - # the # of `<-` canged, so we should have another look at this with statement - Enum.any?(postroll) -> - run(zipper, ctx) - - true -> - # of clauess didn't change, so don't reecurse or we'll loop FOREEEVEERR - {:cont, zipper, ctx} - end - end - else - # maybe this isn't a with statement - could be a function named `with` - # or it's just a with statement with no arrows, but that's too saddening to imagine - {:cont, zipper, ctx} + # maybe this isn't a with statement - could be a function named `with` or something. + true -> + {:cont, zipper, ctx} end end @@ -196,6 +143,10 @@ defmodule Styler.Style.Blocks do [head, [do_block, {_, {:__block__, _, []}}]] -> {:cont, Zipper.replace(zipper, {:if, m, [head, [do_block]]}), ctx} + # drop `else: nil` + [head, [do_block, {_, {:__block__, _, [nil]}}]] -> + {:cont, Zipper.replace(zipper, {:if, m, [head, [do_block]]}), ctx} + [head, [do_, else_]] -> if Style.max_line(do_) > Style.max_line(else_) do # we inverted the if/else blocks of this `if` statement in a previous pass (due to negators or unless) @@ -212,6 +163,99 @@ defmodule Styler.Style.Blocks do def run(zipper, ctx), do: {:cont, zipper, ctx} + # with statements can do _a lot_, so this beast of a function likewise does a lot. + defp style_with_statement({{:with, with_meta, children}, _} = zipper, ctx) do + {preroll, children} = + children + |> Enum.map(fn + # `_ <- rhs` => `rhs` + {:<-, _, [{:_, _, _}, rhs]} -> rhs + # `lhs <- rhs` => `lhs = rhs` + {:<-, m, [{atom, _, nil} = lhs, rhs]} when is_atom(atom) -> {:=, m, [lhs, rhs]} + child -> child + end) + |> Enum.split_while(&(not left_arrow?(&1))) + + # after rewriting `x <- y()` to `x = y()` there are no more arrows. + # this never should've been a with statement at all! we can just replace it with assignments + if Enum.empty?(children) do + {:cont, replace_with_statement(zipper, preroll), ctx} + else + [[{{_, do_meta, _} = do_block, do_body} | elses] | reversed_clauses] = Enum.reverse(children) + {postroll, reversed_clauses} = Enum.split_while(reversed_clauses, &(not left_arrow?(&1))) + [{:<-, final_clause_meta, [lhs, rhs]} = _final_clause | rest] = reversed_clauses + + # drop singleton identity else clauses like `else foo -> foo end` + elses = + with [{{_, _, [:else]}, [{:->, _, [[left], right]}]}] <- elses, + true <- nodes_equivalent?(left, right), + do: [], + else: (_ -> elses) + + # Remove Redundant body + {postroll, reversed_clauses, do_body} = + if Enum.empty?(postroll) and Enum.empty?(elses) and nodes_equivalent?(lhs, do_body) do + # removing redundant RHS can expose more non-arrows behind it, so repeat our postroll process + {postroll, reversed_clauses} = Enum.split_while(rest, &(not left_arrow?(&1))) + {postroll, reversed_clauses, rhs} + else + {postroll, reversed_clauses, do_body} + end + + # Put the postroll into the body + {reversed_clauses, do_body} = + if Enum.any?(postroll) do + {node, do_body_meta, do_children} = do_body + do_children = if node == :__block__, do: do_children, else: [do_body] + do_body = {:__block__, Keyword.take(do_body_meta, [:line]), Enum.reverse(postroll, do_children)} + {reversed_clauses, do_body} + else + {reversed_clauses, do_body} + end + + final_clause_line = final_clause_meta[:line] + + do_line = + cond do + do_meta[:format] == :keyword && final_clause_line + 1 >= do_meta[:line] -> do_meta[:line] + do_meta[:format] == :keyword -> final_clause_line + 1 + true -> final_clause_line + end + + do_block = Macro.update_meta(do_block, &Keyword.put(&1, :line, do_line)) + # disable keyword `, do:` since there will be multiple statements in the body + with_meta = + if Enum.any?(postroll), + do: Keyword.merge(with_meta, do: [line: with_meta[:line]], end: [line: Style.max_line(children) + 1]), + else: with_meta + + with_children = Enum.reverse(reversed_clauses, [[{do_block, do_body} | elses]]) + zipper = Zipper.replace(zipper, {:with, with_meta, with_children}) + + cond do + # oops! RedundantWithClauseResult removed the final arrow in this. no more need for a with statement! + Enum.empty?(reversed_clauses) -> + {:cont, replace_with_statement(zipper, preroll ++ with_children), ctx} + + # recurse if the # of `<-` have changed (this `with` could now be eligible for a `case` rewrite) + Enum.any?(preroll) -> + # put the preroll before the with statement in either a block we create or the existing parent block + zipper + |> Style.find_nearest_block() + |> Zipper.prepend_siblings(preroll) + |> run(ctx) + + # the # of `<-` changed, so we should have another look at this with statement + Enum.any?(postroll) -> + run(zipper, ctx) + + true -> + # of clauses didn't change, so don't reecurse or we'll loop FOREEEVEERR + {:cont, zipper, ctx} + end + end + end + # `with a <- b(), c <- d(), do: :ok, else: (_ -> :error)` # => # `a = b(); c = d(); :ok` diff --git a/lib/style/comment_directives.ex b/lib/style/comment_directives.ex new file mode 100644 index 00000000..95edec58 --- /dev/null +++ b/lib/style/comment_directives.ex @@ -0,0 +1,128 @@ +# Copyright 2024 Adobe. All rights reserved. +# This file is licensed to you under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. You may obtain a copy +# of the License at http://www.apache.org/licenses/LICENSE-2.0 + +# Unless required by applicable law or agreed to in writing, software distributed under +# the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS +# OF ANY KIND, either express or implied. See the License for the specific language +# governing permissions and limitations under the License. + +defmodule Styler.Style.CommentDirectives do + @moduledoc """ + Leave a comment for Styler asking it to maintain code in a certain way. + + `# styler:sort` maintains sorting of wordlists (by string comparison) and lists (string comparison of code representation) + """ + + @behaviour Styler.Style + + alias Styler.Style + alias Styler.Zipper + + def run(zipper, ctx) do + {zipper, comments} = + ctx.comments + |> Enum.filter(&(&1.text == "# styler:sort")) + |> Enum.map(& &1.line) + |> Enum.reduce({zipper, ctx.comments}, fn line, {zipper, comments} -> + found = + Zipper.find(zipper, fn node -> + node_line = Style.meta(node)[:line] || -1 + node_line >= line + end) + + if found do + {sorted, comments} = found |> Zipper.node() |> sort(comments) + {Zipper.replace(found, sorted), comments} + else + {zipper, comments} + end + end) + + {:halt, zipper, %{ctx | comments: comments}} + end + + # defstruct with a syntax-sugared keyword list hits here + defp sort({parent, meta, [list]} = node, comments) when parent in ~w(defstruct __block__)a and is_list(list) do + list = Enum.sort_by(list, &Macro.to_string/1) + line = meta[:line] + # no need to fix line numbers if it's a single line structure + {list, comments} = + if line == Style.max_line(node), + do: {list, comments}, + else: Style.order_line_meta_and_comments(list, comments, line) + + {{parent, meta, [list]}, comments} + end + + # defstruct with a literal list + defp sort({:defstruct, meta, [{:__block__, _, [_]} = list]}, comments) do + {list, comments} = sort(list, comments) + {{:defstruct, meta, [list]}, comments} + end + + defp sort({:%{}, meta, list}, comments) when is_list(list) do + {{:__block__, meta, [list]}, comments} = sort({:__block__, meta, [list]}, comments) + {{:%{}, meta, list}, comments} + end + + defp sort({:%, m, [struct, map]}, comments) do + {map, comments} = sort(map, comments) + {{:%, m, [struct, map]}, comments} + end + + defp sort({:sigil_w, sm, [{:<<>>, bm, [string]}, modifiers]}, comments) do + # ew. gotta be a better way. + # this keeps indentation for the sigil via joiner, while prepend and append are the bookending whitespace + {prepend, joiner, append} = + case Regex.run(~r|^\s+|, string) do + # oneliner like `~w|c a b|` + nil -> {"", " ", ""} + # multiline like + # `"\n a\n list\n long\n of\n static\n values\n"` + # ^^^^ `prepend` ^^^^ `joiner` ^^ `append` + # note that joiner and prepend are the same in a multiline (unsure if this is always true) + # @TODO: get all 3 in one pass of a regex. probably have to turn off greedy or something... + [joiner] -> {joiner, joiner, ~r|\s+$| |> Regex.run(string) |> hd()} + end + + string = string |> String.split() |> Enum.sort() |> Enum.join(joiner) + {{:sigil_w, sm, [{:<<>>, bm, [prepend, string, append]}, modifiers]}, comments} + end + + defp sort({:=, m, [lhs, rhs]}, comments) do + {rhs, comments} = sort(rhs, comments) + {{:=, m, [lhs, rhs]}, comments} + end + + defp sort({:@, m, [{a, am, [assignment]}]}, comments) do + {assignment, comments} = sort(assignment, comments) + {{:@, m, [{a, am, [assignment]}]}, comments} + end + + defp sort({key, value}, comments) do + {value, comments} = sort(value, comments) + {{key, value}, comments} + end + + # sorts arbitrary ast nodes within a `do end` list + defp sort({f, m, args} = node, comments) do + if m[:do] && m[:end] && match?([{{:__block__, _, [:do]}, {:__block__, _, _}}], List.last(args)) do + {[{{:__block__, m1, [:do]}, {:__block__, m2, nodes}}], args} = List.pop_at(args, -1) + + {nodes, comments} = + nodes + |> Enum.sort_by(&Macro.to_string/1) + |> Style.order_line_meta_and_comments(comments, m[:line]) + + args = List.insert_at(args, -1, [{{:__block__, m1, [:do]}, {:__block__, m2, nodes}}]) + + {{f, m, args}, comments} + else + {node, comments} + end + end + + defp sort(x, comments), do: {x, comments} +end diff --git a/lib/style/configs.ex b/lib/style/configs.ex index 9725bcf4..a5379897 100644 --- a/lib/style/configs.ex +++ b/lib/style/configs.ex @@ -48,8 +48,9 @@ defmodule Styler.Style.Configs do end def run({{:config, cfm, [_, _ | _]} = config, zm}, %{mix_config?: true, comments: comments} = ctx) do + {l, p, r} = zm # all of these list are reversed due to the reduce - {configs, assignments, rest} = accumulate(zm.r, [], []) + {configs, assignments, rest} = accumulate(r, [], []) # @TODO # okay so comments between nodes that we moved....... # lets just push them out of the way (???). so @@ -80,31 +81,21 @@ defmodule Styler.Style.Configs do |> Style.reset_newlines() |> Enum.concat(configs) - # `set_lines` performs better than `fix_line_numbers` for a large number of nodes moving, as it moves their comments with them - # however, it will also move any comments not associated with a node, causing wildly unpredictable sad times! - # so i'm trying to guess which change will be less damaging. - # moving >=3 nodes hints that this is an initial run, where `set_lines` definitely outperforms. {nodes, comments} = - case change_count(nodes) do - 0 -> - {nodes, comments} - - n when n < 3 -> - {Style.fix_line_numbers(nodes, List.last(rest)), comments} - - _ -> - # after running, this block should take up the same # of lines that it did before - # the first node of `rest` is greater than the highest line in configs, assignments - # config line is the first line to be used as part of this block - # that will change when we consider preceding comments - {node_comments, _} = comments_for_node(config, comments) - first_line = min(List.last(node_comments)[:line] || cfm[:line], cfm[:line]) - set_lines(nodes, comments, first_line) + if changed?(nodes) do + # after running, this block should take up the same # of lines that it did before + # the first node of `rest` is greater than the highest line in configs, assignments + # config line is the first line to be used as part of this block + {node_comments, _} = Style.comments_for_node(config, comments) + first_line = min(List.first(node_comments)[:line] || cfm[:line], cfm[:line]) + Style.order_line_meta_and_comments(nodes, comments, first_line) + else + {nodes, comments} end - [config | left_siblings] = Enum.reverse(nodes, zm.l) + [config | left_siblings] = Enum.reverse(nodes, l) - {:skip, {config, %{zm | l: left_siblings, r: rest}}, %{ctx | comments: comments}} + {:skip, {config, {left_siblings, p, rest}}, %{ctx | comments: comments}} end def run(zipper, %{config?: true} = ctx) do @@ -120,81 +111,11 @@ defmodule Styler.Style.Configs do end end - defp change_count(nodes, n \\ 0) - - defp change_count([{_, am, _}, {_, bm, _} = b | tail], n) do - n = if am[:line] > bm[:line], do: n + 1, else: n - change_count([b | tail], n) - end - - defp change_count(_, n), do: n - - defp set_lines(nodes, comments, first_line) do - {nodes, comments, node_comments} = set_lines(nodes, comments, first_line, [], []) - # @TODO if there are dangling comments between the nodes min/max, push them somewhere? - # likewise deal with conflicting line comments? - {nodes, Enum.sort_by(comments ++ node_comments, & &1.line)} - end - - def set_lines([], comments, _, node_acc, c_acc), do: {Enum.reverse(node_acc), comments, c_acc} - - def set_lines([{_, meta, _} = node | nodes], comments, start_line, n_acc, c_acc) do - line = meta[:line] - last_line = meta[:end_of_expression][:line] || Style.max_line(node) - - {node, node_comments, comments} = - if start_line == line do - {node, [], comments} - else - {mine, comments} = comments_for_lines(comments, line, last_line) - line_with_comments = (List.first(mine)[:line] || line) - (List.first(mine)[:previous_eol_count] || 1) + 1 - - if line_with_comments == start_line do - {node, mine, comments} - else - shift = start_line - line_with_comments - node = Style.shift_line(node, shift) - - mine = Enum.map(mine, &%{&1 | line: &1.line + shift}) - {node, mine, comments} - end - end - - {_, meta, _} = node - # @TODO what about comments that were free floating between blocks? i'm just ignoring them and maybe always will... - # kind of just want to shove them to the end though, so that they don't interrupt existing stanzas. - # i think that's accomplishable by doing a final call above that finds all comments in the comments list that weren't moved - # and which are in the range of start..finish and sets their lines to finish! - last_line = meta[:end_of_expression][:line] || Style.max_line(node) - last_line = (meta[:end_of_expression][:newlines] || 1) + last_line - set_lines(nodes, comments, last_line, [node | n_acc], node_comments ++ c_acc) - end - - defp comments_for_node({_, m, _} = node, comments) do - last_line = m[:end_of_expression][:line] || Style.max_line(node) - comments_for_lines(comments, m[:line], last_line) - end - - defp comments_for_lines(comments, start_line, last_line) do - comments - |> Enum.reverse() - |> comments_for_lines(start_line, last_line, [], []) + defp changed?([{_, am, _}, {_, bm, _} = b | tail]) do + if am[:line] > bm[:line], do: true, else: changed?([b | tail]) end - defp comments_for_lines(reversed_comments, start, last, match, acc) - - defp comments_for_lines([], _, _, match, acc), do: {Enum.reverse(match), acc} - - defp comments_for_lines([%{line: line} = comment | rev_comments], start, last, match, acc) do - cond do - line > last -> comments_for_lines(rev_comments, start, last, match, [comment | acc]) - line >= start -> comments_for_lines(rev_comments, start, last, [comment | match], acc) - # @TODO bug: match line looks like `x = :foo # comment for x` - # could account for that by pre-running the formatter on config files :/ - line == start - 1 -> comments_for_lines(rev_comments, start - 1, last, [comment | match], acc) - true -> {match, Enum.reverse(rev_comments, [comment | acc])} - end - end + defp changed?(_), do: false defp accumulate([{:config, _, [_, _ | _]} = c | siblings], cs, as), do: accumulate(siblings, [c | cs], as) defp accumulate([{:=, _, [_lhs, _rhs]} = a | siblings], cs, as), do: accumulate(siblings, cs, [a | as]) diff --git a/lib/style/defs.ex b/lib/style/defs.ex index eaedbaca..59d81f74 100644 --- a/lib/style/defs.ex +++ b/lib/style/defs.ex @@ -62,19 +62,18 @@ defmodule Styler.Style.Defs do end end - # all the other kinds of defs! - # @TODO all paths here skip, which means that `def a .. quote do def b ...` won't style `def b` - def run({{def, def_meta, [head, body]}, _} = zipper, ctx) when def in [:def, :defp] do + def run({{def, def_meta, [head, [{{:__block__, dm, [:do]}, {_, bm, _}} | _] = body]}, _} = zipper, ctx) + when def in [:def, :defp] do def_line = def_meta[:line] + end_line = def_meta[:end][:line] || bm[:closing][:line] || dm[:line] - if do_meta = def_meta[:do] do - # This is a def with a do end block - end_line = def_meta[:end][:line] - - if def_line == end_line do + cond do + def_line == end_line -> {:skip, zipper, ctx} - else - do_line = do_meta[:line] + + # def do end + Keyword.has_key?(def_meta, :do) -> + do_line = dm[:line] delta = def_line - do_line def_meta = @@ -92,19 +91,12 @@ defmodule Styler.Style.Defs do |> Style.shift_comments(do_line..end_line, delta) {:skip, Zipper.replace(zipper, node), %{ctx | comments: comments}} - end - else - # This is a def with a keyword do - [{{:__block__, do_meta, [:do]}, {_, body_meta, _}}] = body - end_line = body_meta[:closing][:line] || do_meta[:line] - if def_line == end_line do - {:skip, zipper, ctx} - else + # def , do: + true -> node = Style.set_line({def, def_meta, [head, body]}, def_line) comments = Style.displace_comments(ctx.comments, def_line..end_line) {:skip, Zipper.replace(zipper, node), %{ctx | comments: comments}} - end end end diff --git a/lib/style/deprecations.ex b/lib/style/deprecations.ex index 68050083..824a88e6 100644 --- a/lib/style/deprecations.ex +++ b/lib/style/deprecations.ex @@ -17,6 +17,20 @@ defmodule Styler.Style.Deprecations do def run({node, meta}, ctx), do: {:cont, {style(node), meta}, ctx} + # Deprecated in 1.18 + # rewrite patterns of `first..last = ...` to `first..last//_ = ...` + defp style({:=, m, [{:.., _, [_first, _last]} = range, rhs]}), do: {:=, m, [rewrite_range_match(range), rhs]} + defp style({:->, m, [[{:.., _, [_first, _last]} = range], rhs]}), do: {:->, m, [[rewrite_range_match(range)], rhs]} + defp style({:<-, m, [{:.., _, [_first, _last]} = range, rhs]}), do: {:<-, m, [rewrite_range_match(range), rhs]} + + defp style({def, dm, [{x, xm, params} | rest]}) when def in ~w(def defp)a and is_list(params), + do: {def, dm, [{x, xm, Enum.map(params, &rewrite_range_match/1)} | rest]} + + # Deprecated in 1.18 + # List.zip => Enum.zip + defp style({{:., dm_, [{:__aliases__, am, [:List]}, :zip]}, fm, arg}), + do: {{:., dm_, [{:__aliases__, am, [:Enum]}, :zip]}, fm, arg} + # Logger.warn => Logger.warning # Started to emit warning after Elixir 1.15.0 defp style({{:., dm, [{:__aliases__, am, [:Logger]}, :warn]}, funm, args}), @@ -44,6 +58,17 @@ defmodule Styler.Style.Deprecations do do: {:|>, m, [lhs, {f, fm, [lob, opts]}]} end + if Version.match?(System.version(), ">= 1.17.0-dev") do + for {erl, ex} <- [hours: :hour, minutes: :minute, seconds: :second] do + defp style({{:., _, [{:__block__, _, [:timer]}, unquote(erl)]}, fm, [x]}), + do: {:to_timeout, fm, [[{{:__block__, [format: :keyword, line: fm[:line]], [unquote(ex)]}, x}]]} + end + end + + # Struct update syntax is deprecated in 1.19 + # `%Foo{x | y} => %{x | y}` + defp style({:%, _, [_struct, {:%{}, _, [{:|, _, _}]} = update]}), do: update + # For ranges where `start > stop`, you need to explicitly include the step # Enum.slice(enumerable, 1..-2) => Enum.slice(enumerable, 1..-2//1) # String.slice("elixir", 2..-1) => String.slice("elixir", 2..-1//1) @@ -84,6 +109,9 @@ defmodule Styler.Style.Deprecations do defp style(node), do: node + defp rewrite_range_match({:.., dm, [first, {_, m, _} = last]}), do: {:..//, dm, [first, last, {:_, m, nil}]} + defp rewrite_range_match(x), do: x + defp add_step_to_date_range?(first, last) do with {:ok, f} <- extract_date_value(first), {:ok, l} <- extract_date_value(last), @@ -100,7 +128,7 @@ defmodule Styler.Style.Deprecations do {:ok, stop} <- extract_value_from_range(last), true <- start > stop do step = {:__block__, [token: "1", line: lm[:line]], [1]} - {:"..//", rm, [first, last, step]} + {:..//, rm, [first, last, step]} else _ -> range end diff --git a/lib/style/module_directives.ex b/lib/style/module_directives.ex index ffee62d1..59dd153a 100644 --- a/lib/style/module_directives.ex +++ b/lib/style/module_directives.ex @@ -21,7 +21,6 @@ defmodule Styler.Style.ModuleDirectives do * `Credo.Check.Consistency.MultiAliasImportRequireUse` (force expansion) * `Credo.Check.Readability.AliasOrder` (we sort `__MODULE__`, which credo doesn't) - * `Credo.Check.Readability.ModuleDoc` (adds `@moduledoc false` if missing. includes `*.exs` files) * `Credo.Check.Readability.MultiAlias` * `Credo.Check.Readability.StrictModuleLayout` (see section below for details) * `Credo.Check.Readability.UnnecessaryAliasExpansion` @@ -49,6 +48,7 @@ defmodule Styler.Style.ModuleDirectives do @directives ~w(alias import require use)a @attr_directives ~w(moduledoc shortdoc behaviour)a + @defstruct ~w(schema embedded_schema defstruct)a def run({{:defmodule, _, children}, _} = zipper, ctx) do [name, [{{:__block__, do_meta, [:do]}, _body}]] = children @@ -102,6 +102,32 @@ defmodule Styler.Style.ModuleDirectives do end end + # puts `@derive` before `defstruct` etc, fixing compiler warnings + def run({{:@, _, [{:derive, _, _}]}, _} = zipper, ctx) do + case Style.ensure_block_parent(zipper) do + {:ok, {derive, {l, p, r}}} -> + previous_defstruct = + l + |> Stream.with_index() + |> Enum.find_value(fn + {{struct_def, meta, _}, index} when struct_def in @defstruct -> {meta[:line], index} + _ -> nil + end) + + if previous_defstruct do + {defstruct_line, defstruct_index} = previous_defstruct + derive = Style.set_line(derive, defstruct_line - 1) + left_siblings = List.insert_at(l, defstruct_index + 1, derive) + {:skip, Zipper.remove({derive, {left_siblings, p, r}}), ctx} + else + {:cont, zipper, ctx} + end + + :error -> + {:cont, zipper, ctx} + end + end + def run(zipper, ctx), do: {:cont, zipper, ctx} # a dynamic module name, like `defmodule my_variable do ... end` @@ -221,7 +247,7 @@ defmodule Styler.Style.ModuleDirectives do acc.behaviour ] |> Stream.concat() - |> Style.fix_line_numbers(List.first(nondirectives)) + |> 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 if Enum.empty?(directives) do @@ -240,8 +266,7 @@ defmodule Styler.Style.ModuleDirectives do # we can't use the dealias map built into state as that's what things look like before sorting # now that we've sorted, it could be different! dealiases = AliasEnv.define(aliases) - excluded = dealiases |> Map.keys() |> Enum.into(Styler.Config.get(:lifting_excludes)) - liftable = find_liftable_aliases(requires ++ nondirectives, excluded) + liftable = find_liftable_aliases(requires ++ nondirectives, dealiases) if Enum.any?(liftable) do # This is a silly hack that helps comments stay put. @@ -264,9 +289,15 @@ defmodule Styler.Style.ModuleDirectives do end end - defp find_liftable_aliases(ast, excluded) do + defp find_liftable_aliases(ast, dealiases) do + excluded = dealiases |> Map.keys() |> Enum.into(Styler.Config.get(:lifting_excludes)) + + firsts = MapSet.new(dealiases, fn {_last, [first | _]} -> first end) + ast |> Zipper.zip() + # we're reducing a datastructure that looks like + # %{last => {aliases, seen_before?} | :some_collision_probelm} |> Zipper.reduce_while(%{}, fn # we don't want to rewrite alias name `defx Aliases ... do` of these three keywords {{defx, _, args}, _} = zipper, lifts when defx in ~w(defmodule defimpl defprotocol)a -> @@ -287,19 +318,83 @@ defmodule Styler.Style.ModuleDirectives do {{:quote, _, _}, _} = zipper, lifts -> {:skip, zipper, lifts} - {{:__aliases__, _, [_, _, _ | _] = aliases}, _} = zipper, lifts -> + {{:__aliases__, _, [first, _, _ | _] = aliases}, _} = zipper, lifts -> last = List.last(aliases) lifts = - if last in excluded or not Enum.all?(aliases, &is_atom/1) do - lifts - else - Map.update(lifts, last, {aliases, false}, fn - {^aliases, _} -> {aliases, true} - # if we have `Foo.Bar.Baz` and `Foo.Bar.Bop.Baz` both not aliased, we'll create a collision by lifting both - # grouping by last alias lets us detect these collisions - _ -> :collision_with_last - end) + cond do + # this alias already exists, they just wrote it out fully and are leaving it up to us to shorten it down! + dealiases[last] == aliases -> + Map.put(lifts, last, {aliases, true}) + + last in excluded or Enum.any?(aliases, &(not is_atom(&1))) -> + lifts + + # aliasing this would change the meaning of an existing alias + last > first and last in firsts -> + lifts + + # We've seen this once before, time to mark it for lifting and do some bookkeeping for first-collisions + lifts[last] == {aliases, false} -> + lifts = Map.put(lifts, last, {aliases, true}) + + # Here's the bookkeeping for collisions with this alias's first module name... + case lifts[first] do + {:collision_with_first, claimants, colliders} -> + # release our claim on this collision + claimants = MapSet.delete(claimants, aliases) + empty? = Enum.empty?(claimants) + + cond do + empty? and Enum.any?(colliders) -> + # no more claimants, try to promote a collider to be lifted + colliders = Enum.to_list(colliders) + # There's no longer a collision because the only claimant is being lifted. + # So, promote a claimant with these criteria + # - required: its first comes _after_ last, so we aren't promoting an alias that changes the meaning of the other alias we're doing + # - preferred: take a collider we know we want to lift (we've seen it multiple times) + lift = + Enum.reduce_while(colliders, :collision_with_first, fn + {[first | _], true} = liftable, _ when first > last -> + {:halt, liftable} + + {[first | _], _false} = promotable, :collision_with_first when first > last -> + {:cont, promotable} + + _, result -> + {:cont, result} + end) + + Map.put(lifts, first, lift) + + empty? -> + Map.delete(lifts, first) + + true -> + Map.put(lifts, first, {:collision_with_first, claimants, colliders}) + end + + _ -> + lifts + end + + true -> + lifts + |> Map.update(last, {aliases, false}, fn + # if something is claiming the atom we want, add ourselves to the list of colliders + {:collision_with_first, claimers, colliders} -> + {:collision_with_first, claimers, Map.update(colliders, aliases, false, fn _ -> true end)} + + other -> + other + end) + |> Map.update(first, {:collision_with_first, MapSet.new([aliases]), %{}}, fn + {:collision_with_first, claimers, colliders} -> + {:collision_with_first, MapSet.put(claimers, aliases), colliders} + + other -> + other + end) end {:skip, zipper, lifts} @@ -312,6 +407,7 @@ defmodule Styler.Style.ModuleDirectives do # C.foo() # # lifting A.B.C would create a collision with C. + # unlike the collision_with_first tuple book-keeping, there's no recovery here because we won't lift a < 3 length alias {:skip, zipper, Map.put(lifts, first, :collision_with_first)} zipper, lifts -> @@ -377,4 +473,66 @@ defmodule Styler.Style.ModuleDirectives do |> Enum.map(&elem(&1, 0)) |> Style.reset_newlines() end + + # TODO investigate removing this in favor of the Style.post_sort_cleanup(node, comments) + # "Fixes" the line numbers of nodes who have had their orders changed via sorting or other methods. + # This "fix" simply ensures that comments don't get wrecked as part of us moving AST nodes willy-nilly. + # + # The fix is rather naive, and simply enforces the following property on the code: + # A given node must have a line number less than the following node. + # Et voila! Comments behave much better. + # + # ## In Detail + # + # For example, given document + # + # 1: defmodule ... + # 2: alias B + # 3: # this is foo + # 4: def foo ... + # 5: alias A + # + # Sorting aliases the ast node for would put `alias A` (line 5) before `alias B` (line 2). + # + # 1: defmodule ... + # 5: alias A + # 2: alias B + # 3: # this is foo + # 4: def foo ... + # + # Elixir's document algebra would then encounter `line: 5` and immediately dump all comments with `line <= 5`, + # meaning after running through the formatter we'd end up with + # + # 1: defmodule + # 2: # hi + # 3: # this is foo + # 4: alias A + # 5: alias B + # 6: + # 7: def foo ... + # + # This function fixes that by seeing that `alias A` has a higher line number than its following sibling `alias B` and so + # updates `alias A`'s line to be preceding `alias B`'s line. + # + # Running the results of this function through the formatter now no longer dumps the comments prematurely + # + # 1: defmodule ... + # 2: alias A + # 3: alias B + # 4: # this is foo + # 5: def foo ... + defp fix_line_numbers(nodes, nil), do: fix_line_numbers(nodes, 999_999) + defp fix_line_numbers(nodes, {_, meta, _}), do: fix_line_numbers(nodes, meta[:line]) + defp fix_line_numbers(nodes, max), do: nodes |> Enum.reverse() |> do_fix_lines(max, []) + + defp do_fix_lines([], _, acc), do: acc + + defp do_fix_lines([{_, meta, _} = node | nodes], max, acc) do + line = meta[:line] + + # the -2 is just an ugly hack to leave room for one-liner comments and not hijack them. + if line > max, + do: do_fix_lines(nodes, max, [Style.shift_line(node, max - line - 2) | acc]), + else: do_fix_lines(nodes, line, [node | acc]) + end end diff --git a/lib/style/pipes.ex b/lib/style/pipes.ex index ab5765cd..0d9b5e48 100644 --- a/lib/style/pipes.ex +++ b/lib/style/pipes.ex @@ -50,25 +50,79 @@ defmodule Styler.Style.Pipes do {{:|>, _, [_, {:unquote, _, [_]}]}, _} = single_pipe_unquote_zipper -> {:cont, single_pipe_unquote_zipper, ctx} + # unpipe a single pipe zipper {{:|>, _, [lhs, rhs]}, _} = single_pipe_zipper -> - {_, meta, _} = lhs - # try to get everything on one line if we can - line = meta[:line] - {fun, meta, args} = rhs + {fun, rhs_meta, args} = rhs + {_, lhs_meta, _} = lhs + lhs_line = lhs_meta[:line] args = args || [] - - # no way multi-headed fn fits on one line; everything else (?) is just a matter of line length - args = - if Enum.any?(args, &match?({:fn, _, [{:->, _, _}, {:->, _, _} | _]}, &1)) do - Style.shift_line(args, -1) - else - Style.set_line(args, line) + # Every branch ends with the zipper being replaced with a function call + # `lhs |> rhs(...args)` => `rhs(lhs, ...args)` + # The differences are just figuring out what line number updates to make + # in order to get the following properties: + # + # 1. write the function call on one line if reasonable + # 2. keep comments well behaved (by doing meta line-number gymnastics) + + # if we see multiple `->`, there's no way we can online this + # future heuristics would include finding multiple lines + definitively_multiline? = + Enum.any?(args, fn + {:fn, _, [{:->, _, _}, {:->, _, _} | _]} -> true + {:fn, _, [{:->, _, [_, _]}]} -> true + _ -> false + end) + + if definitively_multiline? do + # shift rhs up to hang out with lhs + # 1 lhs + # 2 |> fun( + # 3 ...args... + # n ) + # => + # 1 fun(lhs + # 2 ... args... + # n-1 ) + + # because there could be comments between lhs and rhs, or the dev may have a bunch of empty lines, + # we need to calculate the distance between the two ("shift") + rhs_line = rhs_meta[:line] + shift = lhs_line - rhs_line + {fun, meta, args} = Style.shift_line(rhs, shift) + + # Not going to lie, no idea why the `shift + 1` is correct but it makes tests pass ¯\_(ツ)_/¯ + rhs_max_line = Style.max_line(rhs) + + comments = + ctx.comments + |> Style.displace_comments(lhs_line..(rhs_line - 1)//1) + |> Style.shift_comments(rhs_line..rhs_max_line, shift + 1) + + {:cont, Zipper.replace(single_pipe_zipper, {fun, meta, [lhs | args]}), %{ctx | comments: comments}} + else + # try to get everything on one line. + # formatter will kick it back to multiple if line-length doesn't accommodate + case Zipper.up(single_pipe_zipper) do + # if the parent is an assignment, put it on the same line as the `=` + {{:=, am, [{_, vm, _} = var, _single_pipe]}, _} = assignment_parent -> + # 1 var = + # 2 lhs + # 3 |> rhs(...args) + # => + # 1 var = rhs(lhs, ...args) + oneline_assignment = Style.set_line({:=, am, [var, {fun, rhs_meta, [lhs | args]}]}, vm[:line]) + # skip so we don't re-traverse + {:cont, Zipper.replace(assignment_parent, oneline_assignment), ctx} + + _ -> + # lhs + # |> rhs(...args) + # => + # rhs(lhs, ...) + oneline_function_call = Style.set_line({fun, rhs_meta, [lhs | args]}, lhs_line) + {:cont, Zipper.replace(single_pipe_zipper, oneline_function_call), ctx} end - - lhs = Style.set_line(lhs, line) - {_, meta, _} = Style.set_line({:ignore, meta, []}, line) - function_call_zipper = Zipper.replace(single_pipe_zipper, {fun, meta, [lhs | args]}) - {:cont, function_call_zipper, ctx} + end end non_pipe -> @@ -76,6 +130,52 @@ defmodule Styler.Style.Pipes do end end + # a(b |> c[, ...args]) + # The first argument to a function-looking node is a pipe. + # Maybe pipify the whole thing? + def run({{f, m, [{:|>, _, _} = pipe | args]}, _} = zipper, ctx) do + parent = + case Zipper.up(zipper) do + {{parent, _, _}, _} -> parent + _ -> nil + end + + stringified = is_atom(f) && to_string(f) + + cond do + # this is likely a macro + # assert a |> b() |> c() + !m[:closing] -> + {:cont, zipper, ctx} + + # leave bools alone as they often read better coming first, like when prepended with `not` + # [not ]is_nil(a |> b() |> c()) + stringified && (String.starts_with?(stringified, "is_") or String.ends_with?(stringified, "?")) -> + {:cont, zipper, ctx} + + # string interpolation, module attribute assignment, or prettier bools with not + parent in [:"::", :@, :not, :|>] -> + {:cont, zipper, ctx} + + # double down on being good to exunit macros, and any other special ops + # ..., do: assert(a |> b |> c) + # not (a |> b() |> c()) + f in [:assert, :refute | @special_ops] -> + {:cont, zipper, ctx} + + # if a |> b() |> c(), do: ... + Enum.any?(args, &Style.do_block?/1) -> + {:cont, zipper, ctx} + + true -> + zipper = Zipper.replace(zipper, {:|>, m, [pipe, {f, m, args}]}) + # it's possible this is a nested function call `c(b(a |> b))`, so we should walk up the tree for de-nesting + zipper = Zipper.up(zipper) || zipper + # recursion ensures we get those nested function calls and any additional pipes + run(zipper, ctx) + end + end + def run(zipper, ctx), do: {:cont, zipper, ctx} defp fix_pipe_start({pipe, zmeta} = zipper) do @@ -162,7 +262,7 @@ defmodule Styler.Style.Pipes do # `pipe_chain(a, b, c)` generates the ast for `a |> b |> c` # the intention is to make it a little easier to see what the fix_pipe functions are matching on =) defmacrop pipe_chain(pm, a, b, c) do - quote do: {:|>, unquote(pm), [{:|>, _, [unquote(a), unquote(b)]}, unquote(c)]} + quote do: {:|>, _, [{:|>, unquote(pm), [unquote(a), unquote(b)]}, unquote(c)]} end # a |> fun => a |> fun() @@ -261,7 +361,7 @@ defmodule Styler.Style.Pipes do ) ) when mod in @enum do - rhs = Style.set_line({{:., dm, [enum, :map_join]}, em, join_args ++ map_args}, dm[:line]) + rhs = {{:., dm, [enum, :map_join]}, em, Style.set_line(join_args, dm[:line]) ++ map_args} {:|>, pm, [lhs, rhs]} end @@ -272,7 +372,7 @@ defmodule Styler.Style.Pipes do pipe_chain( pm, lhs, - {{:., dm, [{_, _, [mod]}, :map]}, _, [mapper]}, + {{:., dm, [{_, _, [mod]}, :map]}, em, [mapper]}, {{:., _, [{_, _, [:Enum]}, :into]} = into, _, [collectable]} ) ) @@ -280,16 +380,17 @@ defmodule Styler.Style.Pipes do rhs = case collectable do {{:., _, [{_, _, [mod]}, :new]}, _, []} when mod in @collectable -> - {{:., dm, [{:__aliases__, dm, [mod]}, :new]}, dm, [mapper]} + {{:., dm, [{:__aliases__, dm, [mod]}, :new]}, em, [mapper]} {:%{}, _, []} -> - {{:., dm, [{:__aliases__, dm, [:Map]}, :new]}, dm, [mapper]} + {{:., dm, [{:__aliases__, dm, [:Map]}, :new]}, em, [mapper]} _ -> - {into, dm, [collectable, mapper]} + {into, m, [collectable]} = Style.set_line({into, em, [collectable]}, dm[:line]) + {into, m, [collectable, mapper]} end - Style.set_line({:|>, pm, [lhs, rhs]}, dm[:line]) + {:|>, pm, [lhs, rhs]} end # `lhs |> Enum.map(mapper) |> Map.new()` => `lhs |> Map.new(mapper)` @@ -297,12 +398,12 @@ defmodule Styler.Style.Pipes do pipe_chain( pm, lhs, - {{:., _, [{_, _, [enum]}, :map]}, _, [mapper]}, - {{:., _, [{_, _, [mod]}, :new]} = new, nm, []} + {{:., _, [{_, _, [enum]}, :map]}, em, [mapper]}, + {{:., _, [{_, _, [mod]}, :new]} = new, _, []} ) ) when mod in @collectable and enum in @enum do - Style.set_line({:|>, pm, [lhs, {new, nm, [mapper]}]}, nm[:line]) + {:|>, pm, [lhs, {Style.set_line(new, em[:line]), em, [mapper]}]} end defp fix_pipe(node), do: node diff --git a/lib/style/single_node.ex b/lib/style/single_node.ex index 832a3724..4176909f 100644 --- a/lib/style/single_node.ex +++ b/lib/style/single_node.ex @@ -183,6 +183,49 @@ defmodule Styler.Style.SingleNode do defp style({:case, cm, [head, [{do_, arrows}]]}), do: {:case, cm, [head, [{do_, rewrite_arrows(arrows)}]]} defp style({:fn, m, arrows}), do: {:fn, m, rewrite_arrows(arrows)} + defp style({:to_timeout, meta, [[{{:__block__, um, [unit]}, {:*, _, [left, right]}}]]} = node) + when unit in ~w(day hour minute second millisecond)a do + [l, r] = + Enum.map([left, right], fn + {_, _, [x]} -> x + _ -> nil + end) + + {step, next_unit} = + case unit do + :day -> {7, :week} + :hour -> {24, :day} + :minute -> {60, :hour} + :second -> {60, :minute} + :millisecond -> {1000, :second} + end + + if step in [l, r] do + n = if l == step, do: right, else: left + style({:to_timeout, meta, [[{{:__block__, um, [next_unit]}, n}]]}) + else + node + end + end + + defp style({:to_timeout, meta, [[{{:__block__, um, [unit]}, {:__block__, tm, [n]}}]]} = node) do + step_up = + case {unit, n} do + {:day, 7} -> :week + {:hour, 24} -> :day + {:minute, 60} -> :hour + {:second, 60} -> :minute + {:millisecond, 1000} -> :second + _ -> nil + end + + if step_up do + {:to_timeout, meta, [[{{:__block__, um, [step_up]}, {:__block__, [token: "1", line: tm[:line]], [1]}}]]} + else + node + end + end + defp style(node), do: node defp replace_into({:., dm, [{_, am, _} = enum, _]}, collectable, rest) do diff --git a/lib/styler.ex b/lib/styler.ex index af790069..9ca3ded9 100644 --- a/lib/styler.ex +++ b/lib/styler.ex @@ -22,11 +22,12 @@ defmodule Styler do @styles [ Styler.Style.ModuleDirectives, Styler.Style.Pipes, + Styler.Style.Deprecations, Styler.Style.SingleNode, Styler.Style.Defs, Styler.Style.Blocks, - Styler.Style.Deprecations, - Styler.Style.Configs + Styler.Style.Configs, + Styler.Style.CommentDirectives ] @doc false @@ -63,21 +64,21 @@ defmodule Styler do def features(_opts), do: [sigils: [], extensions: [".ex", ".exs"]] @impl Format - def format(input, formatter_opts) do + def format(input, formatter_opts \\ []) do file = formatter_opts[:file] styler_opts = formatter_opts[:styler] || [] {ast, comments} = input - |> string_to_quoted_with_comments(to_string(file)) + |> string_to_ast(to_string(file)) |> style(file, styler_opts) - quoted_to_string(ast, comments, formatter_opts) + ast_to_string(ast, comments, formatter_opts) end @doc false # Wrap `Code.string_to_quoted_with_comments` with our desired options - def string_to_quoted_with_comments(code, file \\ "nofile") when is_binary(code) do + def string_to_ast(code, file \\ "nofile") when is_binary(code) do Code.string_to_quoted_with_comments!(code, literal_encoder: &__MODULE__.literal_encoder/2, token_metadata: true, @@ -89,9 +90,8 @@ defmodule Styler do @doc false def literal_encoder(literal, meta), do: {:ok, {:__block__, meta, [literal]}} - @doc false - # Turns an ast and comments back into code, formatting it along the way. - def quoted_to_string(ast, comments, formatter_opts \\ []) do + @doc "Turns an ast and comments back into code, formatting it along the way." + def ast_to_string(ast, comments \\ [], formatter_opts \\ []) do opts = [{:comments, comments}, {:escape, false} | formatter_opts] {line_length, opts} = Keyword.pop(opts, :line_length, 122) diff --git a/lib/zipper.ex b/lib/zipper.ex index c3114aff..e0b88d05 100644 --- a/lib/zipper.ex +++ b/lib/zipper.ex @@ -27,14 +27,8 @@ defmodule Styler.Zipper do import Kernel, except: [node: 1] @type tree :: Macro.t() - - @opaque path :: %{ - l: [tree], - ptree: zipper, - r: [tree] - } - @type zipper :: {tree, path | nil} + @type path :: {left :: [tree], parent :: zipper, right :: [tree]} @type t :: zipper @type command :: :cont | :skip | :halt @@ -92,7 +86,7 @@ defmodule Styler.Zipper do def down(zipper) do case children(zipper) do [] -> nil - [first | rest] -> {first, %{ptree: zipper, l: [], r: rest}} + [first | rest] -> {first, {[], zipper, rest}} end end @@ -103,9 +97,8 @@ defmodule Styler.Zipper do @spec up(zipper) :: zipper | nil def up({_, nil}), do: nil - def up({tree, meta}) do - children = Enum.reverse(meta.l, [tree | meta.r]) - {parent, parent_meta} = meta.ptree + def up({tree, {l, {parent, parent_meta}, r}}) do + children = Enum.reverse(l, [tree | r]) {do_replace_children(parent, children), parent_meta} end @@ -113,16 +106,16 @@ defmodule Styler.Zipper do Returns the zipper of the left sibling of the node at this zipper, or nil. """ @spec left(zipper) :: zipper | nil - def left({tree, %{l: [ltree | l], r: r} = meta}), do: {ltree, %{meta | l: l, r: [tree | r]}} + def left({tree, {[ltree | l], p, r}}), do: {ltree, {l, p, [tree | r]}} def left(_), do: nil @doc """ Returns the leftmost sibling of the node at this zipper, or itself. """ @spec leftmost(zipper) :: zipper - def leftmost({tree, %{l: [_ | _] = l} = meta}) do - [leftmost | r] = Enum.reverse(l, [tree | meta.r]) - {leftmost, %{meta | l: [], r: r}} + def leftmost({tree, {[_ | _] = l, p, r}}) do + [leftmost | r] = Enum.reverse(l, [tree | r]) + {leftmost, {[], p, r}} end def leftmost({_, _} = zipper), do: zipper @@ -131,16 +124,16 @@ defmodule Styler.Zipper do Returns the zipper of the right sibling of the node at this zipper, or nil. """ @spec right(zipper) :: zipper | nil - def right({tree, %{r: [rtree | r]} = meta}), do: {rtree, %{meta | r: r, l: [tree | meta.l]}} + def right({tree, {l, p, [rtree | r]}}), do: {rtree, {[tree | l], p, r}} def right(_), do: nil @doc """ Returns the rightmost sibling of the node at this zipper, or itself. """ @spec rightmost(zipper) :: zipper - def rightmost({tree, %{r: [_ | _] = r} = meta}) do - [rightmost | l] = Enum.reverse(r, [tree | meta.l]) - {rightmost, %{meta | l: l, r: []}} + def rightmost({tree, {l, p, [_ | _] = r}}) do + [rightmost | l] = Enum.reverse(r, [tree | l]) + {rightmost, {l, p, []}} end def rightmost({_, _} = zipper), do: zipper @@ -164,8 +157,8 @@ defmodule Styler.Zipper do """ @spec remove(zipper) :: zipper def remove({_, nil}), do: raise(ArgumentError, message: "Cannot remove the top level node.") - def remove({_, %{l: [left | rest]} = meta}), do: prev_down({left, %{meta | l: rest}}) - def remove({_, %{ptree: {parent, parent_meta}, r: children}}), do: {do_replace_children(parent, children), parent_meta} + def remove({_, {[left | rest], p, r}}), do: prev_down({left, {rest, p, r}}) + def remove({_, {_, {parent, parent_meta}, children}}), do: {do_replace_children(parent, children), parent_meta} @doc """ Inserts the item as the left sibling of the node at this zipper, without @@ -173,19 +166,19 @@ defmodule Styler.Zipper do top level. """ @spec insert_left(zipper, tree) :: zipper - def insert_left({_, nil}, _), do: raise(ArgumentError, message: "Can't insert siblings at the top level.") - def insert_left({tree, meta}, child), do: {tree, %{meta | l: [child | meta.l]}} + def insert_left(zipper, child), do: prepend_siblings(zipper, [child]) @doc """ Inserts many siblings to the left. + If the node is at the top of the tree, builds a new root `:__block__` while maintaining focus on the current node. Equivalent to Enum.reduce(siblings, zipper, &Zipper.insert_left(&2, &1)) """ @spec prepend_siblings(zipper, [tree]) :: zipper - def prepend_siblings({_, nil}, _), do: raise(ArgumentError, message: "Can't insert siblings at the top level.") - def prepend_siblings({tree, meta}, siblings), do: {tree, %{meta | l: Enum.reverse(siblings, meta.l)}} + def prepend_siblings({node, nil}, siblings), do: {:__block__, [], siblings ++ [node]} |> zip() |> down() |> rightmost() + def prepend_siblings({tree, {l, p, r}}, siblings), do: {tree, {Enum.reverse(siblings, l), p , r}} @doc """ Inserts the item as the right sibling of the node at this zipper, without @@ -193,19 +186,19 @@ defmodule Styler.Zipper do top level. """ @spec insert_right(zipper, tree) :: zipper - def insert_right({_, nil}, _), do: raise(ArgumentError, message: "Can't insert siblings at the top level.") - def insert_right({tree, meta}, child), do: {tree, %{meta | r: [child | meta.r]}} + def insert_right(zipper, child), do: insert_siblings(zipper, [child]) @doc """ Inserts many siblings to the right. + If the node is at the top of the tree, builds a new root `:__block__` while maintaining focus on the current node. Equivalent to Enum.reduce(siblings, zipper, &Zipper.insert_right(&2, &1)) """ @spec insert_siblings(zipper, [tree]) :: zipper - def insert_siblings({_, nil}, _), do: raise(ArgumentError, message: "Can't insert siblings at the top level.") - def insert_siblings({tree, meta}, siblings), do: {tree, %{meta | r: siblings ++ meta.r}} + def insert_siblings({node, nil}, siblings), do: {:__block__, [], [node | siblings]} |> zip() |> down() + def insert_siblings({tree, {l, p, r}}, siblings), do: {tree, {l, p, siblings ++ r}} @doc """ Inserts the item as the leftmost child of the node at this zipper, @@ -318,23 +311,6 @@ defmodule Styler.Zipper do if next = next(zipper), do: do_traverse(next, acc, fun), else: {top(zipper), acc} end - # Same as `traverse/3`, but doesn't waste cycles going back to the top of the tree when traversal is finished - @doc false - @spec reduce(zipper, term, (zipper, term -> {zipper, term})) :: term - def reduce({_, nil} = zipper, acc, fun) do - do_reduce(zipper, acc, fun) - end - - def reduce({tree, meta}, acc, fun) do - {{updated, _meta}, acc} = do_reduce({tree, nil}, acc, fun) - {{updated, meta}, acc} - end - - defp do_reduce(zipper, acc, fun) do - {zipper, acc} = fun.(zipper, acc) - if next = next(zipper), do: do_reduce(next, acc, fun), else: acc - end - @doc """ Traverses the tree in depth-first pre-order calling the given function for each node. @@ -391,17 +367,14 @@ defmodule Styler.Zipper do end end - @doc false - # Similar to traverse_while/3, but returns the `acc` directly, skipping the return to the top of the zipper. - # For that reason the :halt tuple is instead just a 2-ple of `{:halt, acc}` - @spec reduce_while(zipper, term, (zipper, term -> {command, zipper, term})) :: {zipper, term} - def reduce_while({_tree, nil} = zipper, acc, fun) do - do_reduce_while(zipper, acc, fun) - end + @doc """ + Same as `traverse_while/3` except it only returns the acc, saving the work of returning to the top of the zipper. - def reduce_while({tree, meta}, acc, fun) do - {{updated, _meta}, acc} = do_reduce_while({tree, nil}, acc, fun) - {{updated, meta}, acc} + For that reason the `:halt` tuple is instead just a 2-ple of `{:halt, acc}` + """ + @spec reduce_while(zipper, term, (zipper, term -> {:cont | :skip, zipper, term} | {:halt, term})) :: term + def reduce_while({tree, _meta}, acc, fun) do + do_reduce_while({tree, nil}, acc, fun) end defp do_reduce_while(zipper, acc, fun) do diff --git a/mix.exs b/mix.exs index edf369cb..305656f2 100644 --- a/mix.exs +++ b/mix.exs @@ -12,7 +12,7 @@ defmodule Styler.MixProject do use Mix.Project # Don't forget to bump the README when doing non-patch version changes - @version "1.1.1" + @version "1.4.2" @url "https://github.com/adobe/elixir-styler" def project do @@ -65,10 +65,12 @@ defmodule Styler.MixProject do extras: [ "CHANGELOG.md": [title: "Changelog"], "docs/styles.md": [title: "Basic Styles"], + "docs/deprecations.md": [title: "Deprecated Elixirisms"], "docs/pipes.md": [title: "Pipe Chains"], "docs/control_flow_macros.md": [title: "Control Flow Macros (if, case, ...)"], - "docs/mix_configs.md": [title: "Mix Configs (config/config.exs, ...)"], + "docs/mix_configs.md": [title: "Mix Configs (config/*.exs)"], "docs/module_directives.md": [title: "Module Directives (use, alias, ...)"], + "docs/comment_directives.md": [title: "Comment Directives (# styler:sort)"], "docs/credo.md": [title: "Styler & Credo"], "README.md": [title: "Styler"] ] diff --git a/test/style/blocks_test.exs b/test/style/blocks_test.exs index 7e16ea6f..a4e97c4d 100644 --- a/test/style/blocks_test.exs +++ b/test/style/blocks_test.exs @@ -156,6 +156,11 @@ defmodule Styler.Style.BlocksTest do z """ ) + + assert_style "with do: x", "x" + assert_style "with do x end", "x" + assert_style "with do x else foo -> bar end", "x" + assert_style "with foo() do bar() else _ -> baz() end", "foo()\nbar()" end test "doesn't false positive with vars" do @@ -202,7 +207,7 @@ defmodule Styler.Style.BlocksTest do """ ) - for nontrivial_head <- ["foo", ":ok <- foo, :ok <- bar"] do + for nontrivial_head <- [":ok <- foo, :ok <- bar"] do assert_style(""" with #{nontrivial_head} do :success @@ -494,6 +499,7 @@ defmodule Styler.Style.BlocksTest do assert_style( """ with {:ok, a} <- foo(), + x = y, {:ok, b} <- bar(a) do {:ok, b} else @@ -502,6 +508,7 @@ defmodule Styler.Style.BlocksTest do """, """ with {:ok, a} <- foo() do + x = y bar(a) end """ @@ -554,6 +561,20 @@ defmodule Styler.Style.BlocksTest do end """ end + + test "elixir1.17+ stab regressions" do + assert_style( + """ + with :ok <- foo, do: :bar, else: (_ -> :baz) + """, + """ + case foo do + :ok -> :bar + _ -> :baz + end + """ + ) + end end test "Credo.Check.Refactor.CondStatements" do @@ -713,6 +734,32 @@ defmodule Styler.Style.BlocksTest do end describe "if" do + test "drops else nil" do + assert_style("if a, do: b, else: nil", "if a, do: b") + + assert_style("if a do b else nil end", """ + if a do + b + end + """) + + assert_style( + """ + if a != b do + # comment + else + :ok + end + """, + """ + if a == b do + # comment + :ok + end + """ + ) + end + test "double negator rewrites" do for a <- ~w(not !), block <- ["do: z", "do: z, else: zz"] do assert_style "if #{a} (x != y), #{block}", "if x == y, #{block}" diff --git a/test/style/comment_directives_test.exs b/test/style/comment_directives_test.exs new file mode 100644 index 00000000..68b6f4f9 --- /dev/null +++ b/test/style/comment_directives_test.exs @@ -0,0 +1,420 @@ +# Copyright 2024 Adobe. All rights reserved. +# This file is licensed to you under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. You may obtain a copy +# of the License at http://www.apache.org/licenses/LICENSE-2.0 + +# Unless required by applicable law or agreed to in writing, software distributed under +# the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS +# OF ANY KIND, either express or implied. See the License for the specific language +# governing permissions and limitations under the License. + +defmodule Styler.Style.CommentDirectivesTest do + @moduledoc false + use Styler.StyleCase, async: true + + describe "sort" do + test "we dont just sort by accident" do + assert_style "[:c, :b, :a]" + end + + test "sorts lists of atoms" do + assert_style( + """ + # styler:sort + [ + :c, + :b, + :c, + :a + ] + """, + """ + # styler:sort + [ + :a, + :b, + :c, + :c + ] + """ + ) + end + + test "sort keywordy things" do + assert_style( + """ + # styler:sort + [ + c: 2, + b: 3, + a: 4, + d: 1 + ] + """, + """ + # styler:sort + [ + a: 4, + b: 3, + c: 2, + d: 1 + ] + """ + ) + + assert_style( + """ + # styler:sort + %{ + c: 2, + b: 3, + a: 4, + d: 1 + } + """, + """ + # styler:sort + %{ + a: 4, + b: 3, + c: 2, + d: 1 + } + """ + ) + + assert_style( + """ + # styler:sort + %Struct{ + c: 2, + b: 3, + a: 4, + d: 1 + } + """, + """ + # styler:sort + %Struct{ + a: 4, + b: 3, + c: 2, + d: 1 + } + """ + ) + + assert_style( + """ + # styler:sort + defstruct c: 2, b: 3, a: 4, d: 1 + """, + """ + # styler:sort + defstruct a: 4, b: 3, c: 2, d: 1 + """ + ) + + assert_style( + """ + # styler:sort + defstruct [ + :repo, + :query, + :order, + :chunk_size, + :timeout, + :cursor + ] + """, + """ + # styler:sort + defstruct [ + :chunk_size, + :cursor, + :order, + :query, + :repo, + :timeout + ] + """ + ) + end + + test "inside keywords" do + assert_style( + """ + %{ + key: + # styler:sort + [ + 3, + 2, + 1 + ] + } + """, + """ + %{ + # styler:sort + key: [ + 1, + 2, + 3 + ] + } + """ + ) + + assert_style( + """ + %{ + # styler:sort + key: [ + 3, + 2, + 1 + ] + } + """, + """ + %{ + # styler:sort + key: [ + 1, + 2, + 3 + ] + } + """ + ) + end + + test "sorts sigils" do + assert_style("# styler:sort\n~w|c a b|", "# styler:sort\n~w|a b c|") + + assert_style( + """ + # styler:sort + ~w( + a + long + list + of + static + values + ) + """, + """ + # styler:sort + ~w( + a + list + long + of + static + values + ) + """ + ) + end + + test "assignments" do + assert_style( + """ + # styler:sort + my_var = + ~w( + a + long + list + of + static + values + ) + """, + """ + # styler:sort + my_var = + ~w( + a + list + long + of + static + values + ) + """ + ) + + assert_style( + """ + defmodule M do + @moduledoc false + # styler:sort + @attr ~w( + a + long + list + of + static + values + ) + end + """, + """ + defmodule M do + @moduledoc false + # styler:sort + @attr ~w( + a + list + long + of + static + values + ) + end + """ + ) + end + + test "doesnt affect downstream nodes" do + assert_style( + """ + # styler:sort + [:c, :a, :b] + + @country_codes ~w( + po_PO + en_US + fr_CA + ja_JP + ) + """, + """ + # styler:sort + [:a, :b, :c] + + @country_codes ~w( + po_PO + en_US + fr_CA + ja_JP + ) + """ + ) + end + + test "list of tuples" do + # 2ples are represented as block literals while >2ples are created via `:{}` + # decided the easiest way to handle this is to just use string representation for meow + assert_style( + """ + # styler:sort + [ + {:styler, github: "adobe/elixir-styler"}, + {:ash, "~> 3.0"}, + {:fluxon, "~> 1.0.0", repo: :fluxon}, + {:phoenix_live_reload, "~> 1.2", only: :dev}, + {:tailwind, "~> 0.2", runtime: Mix.env() == :dev} + ] + """, + """ + # styler:sort + [ + {:ash, "~> 3.0"}, + {:fluxon, "~> 1.0.0", repo: :fluxon}, + {:phoenix_live_reload, "~> 1.2", only: :dev}, + {:styler, github: "adobe/elixir-styler"}, + {:tailwind, "~> 0.2", runtime: Mix.env() == :dev} + ] + """ + ) + end + + test "nodes within a do end block" do + assert_style( + """ + # styler:sort + my_macro "some arg" do + another_macro :q + # w + another_macro :w + another_macro :e + # r comment 1 + # r comment 2 + another_macro :r + another_macro :t + another_macro :y + end + """, + """ + # styler:sort + my_macro "some arg" do + another_macro(:e) + another_macro(:q) + # r comment 1 + # r comment 2 + another_macro(:r) + another_macro(:t) + # w + another_macro(:w) + another_macro(:y) + end + """ + ) + end + + test "treats comments nicely" do + assert_style( + """ + # pre-amble comment + # styler:sort + [ + {:phoenix, "~> 1.7"}, + # hackney comment + {:hackney, "1.18.1", override: true}, + {:styler, "~> 1.2", only: [:dev, :test], runtime: false}, + {:credo, "~> 1.7", only: [:dev, :test], runtime: false}, + {:excellent_migrations, "~> 0.1", only: [:dev, :test], runtime: false}, + # ecto + {:ecto, "~> 3.12"}, + {:ecto_sql, "~> 3.12"}, + # genstage comment 1 + # genstage comment 2 + {:gen_stage, "~> 1.0", override: true}, + # telemetry + {:telemetry, "~> 1.0", override: true}, + # dangling comment + ] + + # some other comment + """, + """ + # pre-amble comment + # styler:sort + [ + {:credo, "~> 1.7", only: [:dev, :test], runtime: false}, + # ecto + {:ecto, "~> 3.12"}, + {:ecto_sql, "~> 3.12"}, + {:excellent_migrations, "~> 0.1", only: [:dev, :test], runtime: false}, + # genstage comment 1 + # genstage comment 2 + {:gen_stage, "~> 1.0", override: true}, + # hackney comment + {:hackney, "1.18.1", override: true}, + {:phoenix, "~> 1.7"}, + {:styler, "~> 1.2", only: [:dev, :test], runtime: false}, + # telemetry + {:telemetry, "~> 1.0", override: true} + # dangling comment + ] + + # some other comment + """ + ) + end + end +end diff --git a/test/style/configs_test.exs b/test/style/configs_test.exs index 7d232ab0..71e7cf81 100644 --- a/test/style/configs_test.exs +++ b/test/style/configs_test.exs @@ -16,7 +16,7 @@ defmodule Styler.Style.ConfigsTest do alias Styler.Style.Configs test "only runs on exs files in config folders" do - {ast, _} = Styler.string_to_quoted_with_comments("import Config\n\nconfig :bar, boop: :baz") + {ast, _} = Styler.string_to_ast("import Config\n\nconfig :bar, boop: :baz") zipper = Styler.Zipper.zip(ast) for file <- ~w(dev.exs my_app.exs config.exs) do @@ -167,6 +167,7 @@ defmodule Styler.Style.ConfigsTest do config :a, 2 config :a, 3 config :a, 4 + # comment # b comment config :b, 1 @@ -335,9 +336,9 @@ defmodule Styler.Style.ConfigsTest do c: :d, e: :f - config :c, - # some junk after b, idk + # some junk after b, idk + config :c, # ca ca: :ca, # cb 1 @@ -351,5 +352,43 @@ defmodule Styler.Style.ConfigsTest do """ ) end + + test "big block regression #230" do + # The nodes are in reverse order + assert_style( + """ + import Config + + # z-a + # z-b + # z-c + # z-d + # z-e + config :z, z + + # y + config :y, y + + # x + config :x, x + """, + """ + import Config + + # x + config :x, x + + # y + config :y, y + + # z-a + # z-b + # z-c + # z-d + # z-e + config :z, z + """ + ) + end end end diff --git a/test/style/defs_test.exs b/test/style/defs_test.exs index d80f5871..f7a9fab7 100644 --- a/test/style/defs_test.exs +++ b/test/style/defs_test.exs @@ -11,232 +11,238 @@ defmodule Styler.Style.DefsTest do use Styler.StyleCase, async: true - describe "run" do - test "comments stay put when we can't shrink the head, even with blocks" do - assert_style(""" - def my_function( - so_long_that_this_head_will_not_fit_on_one_lineso_long_that_this_head_will_not_fit_on_one_line, - so_long_that_this_head_will_not_fit_on_one_line - ) do - result = - case foo do - :bar -> :baz - :baz -> :bong - end - - # My comment - Context.process(result) - end - """) - end + test "comments stay put when we can't shrink the head, even with blocks" do + assert_style(""" + def my_function( + so_long_that_this_head_will_not_fit_on_one_lineso_long_that_this_head_will_not_fit_on_one_line, + so_long_that_this_head_will_not_fit_on_one_line + ) do + result = + case foo do + :bar -> :baz + :baz -> :bong + end - test "function with do keyword" do - assert_style( - """ - # Top comment - def save( - # Socket comment - %Socket{assigns: %{user: user, live_action: :new}} = initial_socket, - # Params comment - params - ), - do: :ok - """, - """ - # Top comment - # Socket comment - # Params comment - def save(%Socket{assigns: %{user: user, live_action: :new}} = initial_socket, params), do: :ok - """ - ) + # My comment + Context.process(result) end + """) + end - test "bodyless function with spec" do - assert_style(""" - @spec original_object(atom()) :: atom() - def original_object(object) - """) - end + test "function with do keyword" do + assert_style( + """ + # Top comment + def save( + # Socket comment + %Socket{assigns: %{user: user, live_action: :new}} = initial_socket, + # Params comment + params + ), + do: :ok + """, + """ + # Top comment + # Socket comment + # Params comment + def save(%Socket{assigns: %{user: user, live_action: :new}} = initial_socket, params), do: :ok + """ + ) + end - test "block function body doesn't get newlined" do - assert_style(""" - # Here's a comment - def some_function(%{id: id, type: type, processed_at: processed_at} = file, params, _) - when type == :file and is_nil(processed_at) do - with {:ok, results} <- FileProcessor.process(file) do - # This comment could make sense - {:ok, post_process_the_results_somehow(results)} - end + test "bodyless function with spec" do + assert_style(""" + @spec original_object(atom()) :: atom() + def original_object(object) + """) + end + + test "block function body doesn't get newlined" do + assert_style(""" + # Here's a comment + def some_function(%{id: id, type: type, processed_at: processed_at} = file, params, _) + when type == :file and is_nil(processed_at) do + with {:ok, results} <- FileProcessor.process(file) do + # This comment could make sense + {:ok, post_process_the_results_somehow(results)} end - """) end + """) + end - test "kwl function body doesn't get newlined" do - assert_style(""" - def is_expired_timestamp?(timestamp) when is_integer(timestamp), - do: Timex.from_unix(timestamp, :second) <= Timex.shift(DateTime.utc_now(), minutes: 1) - """) - end + test "kwl function body doesn't get newlined" do + assert_style(""" + def is_expired_timestamp?(timestamp) when is_integer(timestamp), + do: Timex.from_unix(timestamp, :second) <= Timex.shift(DateTime.utc_now(), minutes: 1) + """) + end - test "function with do block" do - assert_style( - """ - def save( - %Socket{assigns: %{user: user, live_action: :new}} = initial_socket, - params # Comments in the darndest places - ) do - :ok - end - """, - """ - # Comments in the darndest places - def save(%Socket{assigns: %{user: user, live_action: :new}} = initial_socket, params) do - :ok - end - """ - ) - end + test "function with do block" do + assert_style( + """ + def save( + %Socket{assigns: %{user: user, live_action: :new}} = initial_socket, + params # Comments in the darndest places + ) do + :ok + end + """, + """ + # Comments in the darndest places + def save(%Socket{assigns: %{user: user, live_action: :new}} = initial_socket, params) do + :ok + end + """ + ) + end - test "no body" do - assert_style "def no_body_nor_parens_yikes!" - - assert_style( - """ - # Top comment - def no_body( - foo, # This is a foo - bar # This is a bar - ) - - # Another comment for this head - def no_body(nil, _), do: nil - """, - """ - # Top comment - # This is a foo - # This is a bar - def no_body(foo, bar) - - # Another comment for this head - def no_body(nil, _), do: nil - """ - ) - end + test "no body" do + assert_style "def no_body_nor_parens_yikes!" - test "when clause w kwl do" do - assert_style( - """ - def foo(%{ - bar: baz - }) - # Self-documenting code! - when baz in [ - :a, # Obviously, this is a - :b # ... and this is b - ], - do: :never_write_code_like_this - """, - """ - # Self-documenting code! - # Obviously, this is a - # ... and this is b - def foo(%{bar: baz}) when baz in [:a, :b], do: :never_write_code_like_this - """ + assert_style( + """ + # Top comment + def no_body( + foo, # This is a foo + bar # This is a bar ) - end - test "keyword do with a list" do - assert_style( - """ - def foo, - do: [ - # Weirdo comment - :never_write_code_like_this - ] - """, - """ - # Weirdo comment - def foo, do: [:never_write_code_like_this] - """ - ) - end + # Another comment for this head + def no_body(nil, _), do: nil + """, + """ + # Top comment + # This is a foo + # This is a bar + def no_body(foo, bar) + + # Another comment for this head + def no_body(nil, _), do: nil + """ + ) + end - test "rewrites subsequent definitions" do - assert_style( - """ - def foo(), do: :ok + test "when clause w kwl do" do + assert_style( + """ + def foo(%{ + bar: baz + }) + # Self-documenting code! + when baz in [ + :a, # Obviously, this is a + :b # ... and this is b + ], + do: :never_write_code_like_this + """, + """ + # Self-documenting code! + # Obviously, this is a + # ... and this is b + def foo(%{bar: baz}) when baz in [:a, :b], do: :never_write_code_like_this + """ + ) + end + + test "keyword do with a list" do + assert_style( + """ + def foo, + do: [ + # Weirdo comment + :never_write_code_like_this + ] + """, + """ + # Weirdo comment + def foo, do: [:never_write_code_like_this] + """ + ) + end - def foo( - too, - # Long long is too long - long - ), do: :ok - """, - """ - def foo, do: :ok + test "rewrites subsequent definitions" do + assert_style( + """ + def foo(), do: :ok + def foo( + too, # Long long is too long - def foo(too, long), do: :ok - """ - ) - end + long + ), do: :ok + """, + """ + def foo, do: :ok + + # Long long is too long + def foo(too, long), do: :ok + """ + ) + end - test "when clause with block do" do - assert_style( - """ - # Foo takes a bar - def foo(%{ - bar: baz - }) - # Baz should be either :a or :b - when baz in [ - :a, - :b - ] - do # Weird place for a comment - # Above the body - :never_write_code_like_this - # Below the body - end - """, - """ - # Foo takes a bar + test "when clause with block do" do + assert_style( + """ + # Foo takes a bar + def foo(%{ + bar: baz + }) # Baz should be either :a or :b - # Weird place for a comment - def foo(%{bar: baz}) when baz in [:a, :b] do - # Above the body - :never_write_code_like_this - # Below the body - end - """ - ) - end + when baz in [ + :a, + :b + ] + do # Weird place for a comment + # Above the body + :never_write_code_like_this + # Below the body + end + """, + """ + # Foo takes a bar + # Baz should be either :a or :b + # Weird place for a comment + def foo(%{bar: baz}) when baz in [:a, :b] do + # Above the body + :never_write_code_like_this + # Below the body + end + """ + ) + end - test "Doesn't move stuff around if it would make the line too long" do - assert_style(""" - @doc "this is a doc" - # And also a comment - def wow_this_function_name_is_super_long(it_also, has_a, ton_of, arguments), - do: "this is going to end up making the line too long if we inline it" - - @doc "this is another function" - # And it also has a comment - def this_one_fits_on_one_line, do: :ok - """) - end + test "Doesn't move stuff around if it would make the line too long" do + assert_style(""" + @doc "this is a doc" + # And also a comment + def wow_this_function_name_is_super_long(it_also, has_a, ton_of, arguments), + do: "this is going to end up making the line too long if we inline it" + + @doc "this is another function" + # And it also has a comment + def this_one_fits_on_one_line, do: :ok + """) + end - test "Doesn't collapse pipe chains in a def do ... end" do - assert_style(""" - def foo(some_list) do - some_list - |> Enum.reject(&is_nil/1) - |> Enum.map(&transform/1) - end - """) + test "Doesn't collapse pipe chains in a def do ... end" do + assert_style(""" + def foo(some_list) do + some_list + |> Enum.reject(&is_nil/1) + |> Enum.map(&transform/1) end + """) + end + describe "no ops" do test "regression: @def module attribute" do assert_style("@def ~s(this should be okay)") end + + test "no explode on invalid def syntax" do + assert_style("def foo, true") + assert_style("def foo(a), true") + assert_raise SyntaxError, fn -> assert_style("def foo(a) true") end + end end end diff --git a/test/style/deprecations_test.exs b/test/style/deprecations_test.exs index 3d5e8ed9..0fbd13b7 100644 --- a/test/style/deprecations_test.exs +++ b/test/style/deprecations_test.exs @@ -33,20 +33,38 @@ defmodule Styler.Style.DeprecationsTest do ) end - describe "1.16 deprecations" do - @describetag skip: Version.match?(System.version(), "< 1.16.0-dev") + test "matching ranges" do + assert_style "first..last = range", "first..last//_ = range" + assert_style "^first..^last = range", "^first..^last//_ = range" + assert_style "first..last = x = y", "first..last//_ = x = y" + assert_style "y = first..last = x", "y = first..last//_ = x" - test "File.stream!(path, modes, line_or_bytes) to File.stream!(path, line_or_bytes, modes)" do - assert_style( - "File.stream!(path, [encoding: :utf8, trim_bom: true], :line)", - "File.stream!(path, :line, encoding: :utf8, trim_bom: true)" - ) + assert_style "def foo(x..y), do: :ok", "def foo(x..y//_), do: :ok" + assert_style "def foo(a, x..y = z), do: :ok", "def foo(a, x..y//_ = z), do: :ok" + assert_style "def foo(%{a: x..y = z}), do: :ok", "def foo(%{a: x..y//_ = z}), do: :ok" - assert_style( - "f |> File.stream!([encoding: :utf8, trim_bom: true], :line) |> Enum.take(2)", - "f |> File.stream!(:line, encoding: :utf8, trim_bom: true) |> Enum.take(2)" - ) - end + assert_style "with a..b = c <- :ok, d..e <- :better, do: :ok", "with a..b//_ = c <- :ok, d..e//_ <- :better, do: :ok" + + assert_style( + """ + case x do + a..b = c -> :ok + d..e -> :better + end + """, + """ + case x do + a..b//_ = c -> :ok + d..e//_ -> :better + end + """ + ) + end + + test "List.zip/1" do + assert_style "List.zip(foo)", "Enum.zip(foo)" + assert_style "foo |> List.zip |> bar", "foo |> Enum.zip() |> bar()" + assert_style "foo |> List.zip", "Enum.zip(foo)" end test "~R is deprecated in favor of ~r" do @@ -97,4 +115,42 @@ defmodule Styler.Style.DeprecationsTest do assert_style("foo |> bar() |> #{mod}.slice(x..y)") end end + + test "struct update, deprecated in 1.19" do + assert_style "%Foo{widget | bar: :baz}", "%{widget | bar: :baz}" + end + + describe "1.16+" do + @describetag skip: Version.match?(System.version(), "< 1.16.0-dev") + + test "File.stream!(path, modes, line_or_bytes) to File.stream!(path, line_or_bytes, modes)" do + assert_style( + "File.stream!(path, [encoding: :utf8, trim_bom: true], :line)", + "File.stream!(path, :line, encoding: :utf8, trim_bom: true)" + ) + + assert_style( + "f |> File.stream!([encoding: :utf8, trim_bom: true], :line) |> Enum.take(2)", + "f |> File.stream!(:line, encoding: :utf8, trim_bom: true) |> Enum.take(2)" + ) + end + end + + describe "1.17+" do + @describetag skip: Version.match?(System.version(), "< 1.17.0-dev") + + test "to_timeout/1 vs :timer.units(x)" do + assert_style ":timer.hours(x)", "to_timeout(hour: x)" + assert_style ":timer.minutes(x)", "to_timeout(minute: x)" + assert_style ":timer.seconds(x)", "to_timeout(second: x)" + + assert_style "a |> x() |> :timer.hours()" + assert_style "a |> x() |> :timer.minutes()" + assert_style "a |> x() |> :timer.seconds()" + end + + test "combined with to_timeout improvements" do + assert_style ":timer.minutes(60 * 4)", "to_timeout(hour: 4)" + end + end end diff --git a/test/style/module_directives/alias_lifting_test.exs b/test/style/module_directives/alias_lifting_test.exs index d1208c4f..c3ea9cdb 100644 --- a/test/style/module_directives/alias_lifting_test.exs +++ b/test/style/module_directives/alias_lifting_test.exs @@ -175,6 +175,128 @@ defmodule Styler.Style.ModuleDirectives.AliasLiftingTest do ) end + test "replaces known aliases" do + assert_style( + """ + alias A.B.C + + A.B.C.foo() + A.B.C.foo() + A.B.C.foo() + """, + """ + alias A.B.C + + C.foo() + C.foo() + C.foo() + """ + ) + end + + test "two modules that seem to conflict but don't!" do + assert_style( + """ + defmodule Foo do + @moduledoc false + + A.B.C.foo(X.Y.A) + A.B.C.bar() + + X.Y.A + end + """, + """ + defmodule Foo do + @moduledoc false + + alias A.B.C + alias X.Y.A + + C.foo(A) + C.bar() + + A + end + """ + ) + end + + test "if multiple lifts collide, lifts only one" do + assert_style( + """ + defmodule Foo do + @moduledoc false + + A.B.C.f() + A.B.C.f() + X.Y.C.f() + end + """, + """ + defmodule Foo do + @moduledoc false + + alias A.B.C + + C.f() + C.f() + X.Y.C.f() + end + """ + ) + + assert_style( + """ + defmodule Foo do + @moduledoc false + + A.B.C.f() + X.Y.C.f() + X.Y.C.f() + A.B.C.f() + end + """, + """ + defmodule Foo do + @moduledoc false + + alias A.B.C + + C.f() + X.Y.C.f() + X.Y.C.f() + C.f() + end + """ + ) + + assert_style( + """ + defmodule Foo do + @moduledoc false + + X.Y.C.f() + A.B.C.f() + X.Y.C.f() + A.B.C.f() + end + """, + """ + defmodule Foo do + @moduledoc false + + alias X.Y.C + + C.f() + A.B.C.f() + C.f() + A.B.C.f() + end + """ + ) + end + describe "comments stay put" do test "comments before alias stanza" do assert_style( @@ -261,45 +383,61 @@ defmodule Styler.Style.ModuleDirectives.AliasLiftingTest do end end - test "collisions with other lifts" do + test "collisions with submodules" do assert_style """ - defmodule NuhUh do + defmodule A do @moduledoc false A.B.C.f() + + defmodule C do + @moduledoc false + + A.B.C.f() + end + A.B.C.f() - X.Y.C.f() end """ + end + test "collisions with 3-deep one-off" do assert_style """ - defmodule NuhUh do + defmodule Foo do @moduledoc false - A.B.C.f() - A.B.C.f() - X.Y.C.f() - X.Y.C.f() + X.Y.Z.foo(A.B.X) + + A.B.X end """ end - test "collisions with submodules" do - assert_style """ - defmodule A do - @moduledoc false + test "when new alias being sorted in would change an existing alias" do + assert_style( + """ + defmodule Foo do + @moduledoc false - A.B.C.f() + X.Y.Z.foo(A.B.X) + X.Y.Z.bar() - defmodule C do + A.B.X + end + """, + """ + defmodule Foo do @moduledoc false - A.B.C.f() - end + alias X.Y.Z - A.B.C.f() - end - """ + Z.foo(A.B.X) + Z.bar() + + A.B.X + end + """ + ) end test "defprotocol, defmodule, or defimpl" do diff --git a/test/style/pipes_test.exs b/test/style/pipes_test.exs index 49763ea5..50284537 100644 --- a/test/style/pipes_test.exs +++ b/test/style/pipes_test.exs @@ -90,7 +90,7 @@ defmodule Styler.Style.PipesTest do y end - a(foo(if_result), b) + if_result |> foo() |> a(b) """ ) end @@ -402,13 +402,13 @@ defmodule Styler.Style.PipesTest do assert_style( """ def halt(exec, halt_message) do - %__MODULE__{exec | halted: true} + %{exec | halted: true} |> put_halt_message(halt_message) end """, """ def halt(exec, halt_message) do - put_halt_message(%__MODULE__{exec | halted: true}, halt_message) + put_halt_message(%{exec | halted: true}, halt_message) end """ ) @@ -432,6 +432,18 @@ defmodule Styler.Style.PipesTest do """ ) end + + test "onelines assignments" do + assert_style( + """ + x = + y + |> Enum.map(&f/1) + |> Enum.join() + """, + "x = Enum.map_join(y, &f/1)" + ) + end end describe "valid pipe starts & unpiping" do @@ -677,16 +689,24 @@ defmodule Styler.Style.PipesTest do assert_style( """ + # a + # b a_multiline_mapper |> #{enum}.map(fn %{gets: shrunk, down: to_a_more_reasonable} -> + # c IO.puts "woo!" + # d {shrunk, to_a_more_reasonable} end) |> Enum.into(size) """, """ + # a + # b Enum.into(a_multiline_mapper, size, fn %{gets: shrunk, down: to_a_more_reasonable} -> + # c IO.puts("woo!") + # d {shrunk, to_a_more_reasonable} end) """ @@ -747,31 +767,207 @@ defmodule Styler.Style.PipesTest do end end - describe "comments" do - test "unpiping doesn't move comment in anonymous function" do + describe "comments and..." do + test "unpiping" do + assert_style( + """ + aliased = + aliases + |> MapSet.new(fn + {:alias, _, [{:__aliases__, _, aliases}]} -> List.last(aliases) + {:alias, _, [{:__aliases__, _, _}, [{_as, {:__aliases__, _, [as]}}]]} -> as + # alias __MODULE__ or other oddities + {:alias, _, _} -> nil + end) + + excluded_first = MapSet.union(aliased, @excluded_namespaces) + """, + """ + aliased = + MapSet.new(aliases, fn + {:alias, _, [{:__aliases__, _, aliases}]} -> List.last(aliases) + {:alias, _, [{:__aliases__, _, _}, [{_as, {:__aliases__, _, [as]}}]]} -> as + # alias __MODULE__ or other oddities + {:alias, _, _} -> nil + end) + + excluded_first = MapSet.union(aliased, @excluded_namespaces) + """ + ) + + assert_style( + """ + foo = + # bar + bar + # baz + |> baz(fn -> + # a + a + # b + b + end) + """, + """ + # bar + # baz + foo = + baz(bar, fn -> + # a + a + # b + b + end) + """ + ) + + assert_style( + """ + foo = + # bar + bar + # baz + + + + |> baz(fn -> + # a + a + # b + b + end) + """, + """ + # bar + # baz + foo = + baz(bar, fn -> + # a + a + # b + b + end) + """ + ) + end + + test "optimizing" do + assert_style( + """ + a + |> Enum.map(fn b -> + c + # a comment + d + end) + |> Enum.join(x) + |> Enum.each(...) + """, + """ + a + |> Enum.map_join(x, fn b -> + c + # a comment + d + end) + |> Enum.each(...) + """ + ) + + assert_style( + """ + a + |> Enum.map(fn b -> + c + # a comment + d + end) + |> Enum.into(x) + |> Enum.each(...) + """, + """ + a + |> Enum.into(x, fn b -> + c + # a comment + d + end) + |> Enum.each(...) + """ + ) + + assert_style( + """ + a + |> Enum.map(fn b -> + c + # a comment + d + end) + |> Keyword.new() + |> Enum.each(...) + """, + """ + a + |> Keyword.new(fn b -> + c + # a comment + d + end) + |> Enum.each(...) + """ + ) + end + end + + describe "pipifying" do + test "no false positives" do + pipe = "a() |> b() |> c()" + assert_style pipe + assert_style String.replace(pipe, " |>", "\n|>") + assert_style "fn -> #{pipe} end" + assert_style "if #{pipe}, do: ..." + assert_style "x\n\n#{pipe}" + assert_style "@moduledoc #{pipe}" + assert_style "!(#{pipe})" + assert_style "not foo(#{pipe})" + assert_style ~s<"\#{#{pipe}}"> + end + + test "when it's not actually the first argument!" do assert_style """ - aliased = - aliases - |> MapSet.new(fn - {:alias, _, [{:__aliases__, _, aliases}]} -> List.last(aliases) - {:alias, _, [{:__aliases__, _, _}, [{_as, {:__aliases__, _, [as]}}]]} -> as - # alias __MODULE__ or other oddities - {:alias, _, _} -> nil - end) - - excluded_first = MapSet.union(aliased, @excluded_namespaces) - """, - """ - aliased = - MapSet.new(aliases, fn - {:alias, _, [{:__aliases__, _, aliases}]} -> List.last(aliases) - {:alias, _, [{:__aliases__, _, _}, [{_as, {:__aliases__, _, [as]}}]]} -> as - # alias __MODULE__ or other oddities - {:alias, _, _} -> nil - end) - - excluded_first = MapSet.union(aliased, @excluded_namespaces) - """ + a + |> M.f0(b |> M.f1() |> M.f2()) + |> M.f3() + """ + end + + test "pipifying" do + assert_style("e(d(a |> b |> c), f)", "a |> b() |> c() |> d() |> e(f)") + + assert_style( + """ + # d + d( + # a + a + # b + |> b + # c + |> c + ) + """, + """ + # d + # a + a + # b + |> b() + # c + |> c() + |> d() + """ + ) end end end diff --git a/test/style/single_node_test.exs b/test/style/single_node_test.exs index c3dbd0ab..00ec66ec 100644 --- a/test/style/single_node_test.exs +++ b/test/style/single_node_test.exs @@ -314,4 +314,30 @@ defmodule Styler.Style.SingleNodeTest do assert_style("Enum.reverse(foo, bar) ++ bar") end end + + describe "to_timeout" do + test "to next unit" do + facts = [ + {1000, :millisecond, :second}, + {60, :second, :minute}, + {60, :minute, :hour}, + {24, :hour, :day}, + {7, :day, :week} + ] + + for {n, unit, next} <- facts do + assert_style "to_timeout(#{unit}: #{n} * m)", "to_timeout(#{next}: m)" + assert_style "to_timeout(#{unit}: m * #{n})", "to_timeout(#{next}: m)" + assert_style "to_timeout(#{unit}: #{n})", "to_timeout(#{next}: 1)" + end + + assert_style "to_timeout(second: 60 * 60)", "to_timeout(hour: 1)" + end + + test "doesnt mess with" do + assert_style "to_timeout(hour: n * m)" + assert_style "to_timeout(whatever)" + assert_style "to_timeout(hour: 24 * 1, second: 60 * 4)" + end + end end diff --git a/test/style_test.exs b/test/style_test.exs index fbe7a009..43bc8b27 100644 --- a/test/style_test.exs +++ b/test/style_test.exs @@ -3,8 +3,6 @@ defmodule Styler.StyleTest do import Styler.Style, only: [displace_comments: 2, shift_comments: 3] - alias Styler.Style - @code """ # Above module defmodule Foo do @@ -35,7 +33,7 @@ defmodule Styler.StyleTest do # After module """ - @comments @code |> Styler.string_to_quoted_with_comments() |> elem(1) + @comments @code |> Styler.string_to_ast() |> elem(1) describe "displace_comments/2" do test "Doesn't lose any comments" do @@ -117,16 +115,4 @@ defmodule Styler.StyleTest do end end end - - describe "fix_line_numbers" do - test "returns ast list with increasing line numbers" do - nodes = for n <- [1, 2, 999, 1000, 5, 6], do: {:node, [line: n], [n]} - fixed = Style.fix_line_numbers(nodes, 7) - - Enum.scan(fixed, fn {_, [line: this_line], _} = this_node, {_, [line: previous_line], _} -> - assert this_line >= previous_line - this_node - end) - end - end end diff --git a/test/support/style_case.ex b/test/support/style_case.ex index ad213bdb..20c57038 100644 --- a/test/support/style_case.ex +++ b/test/support/style_case.ex @@ -41,11 +41,11 @@ defmodule Styler.StyleCase do if styled != expected and ExUnit.configuration()[:trace] do IO.puts("\n======Given=============\n") IO.puts(before) - {before_ast, before_comments} = Styler.string_to_quoted_with_comments(before) + {before_ast, before_comments} = Styler.string_to_ast(before) dbg(before_ast) dbg(before_comments) IO.puts("======Expected AST==========\n") - {expected_ast, expected_comments} = Styler.string_to_quoted_with_comments(expected) + {expected_ast, expected_comments} = Styler.string_to_ast(expected) dbg(expected_ast) dbg(expected_comments) IO.puts("======Got AST===============\n") @@ -83,6 +83,7 @@ defmodule Styler.StyleCase do _ -> false end + # This isn't enabled in any test, but can be a useful audit if @ordered_siblings do case Zipper.left(zipper) do {{_, prev_meta, _} = prev, _} -> @@ -108,7 +109,7 @@ defmodule Styler.StyleCase do dbg(styled_ast) IO.puts("expected:") - dbg(elem(Styler.string_to_quoted_with_comments(expected), 0)) + dbg(elem(Styler.string_to_ast(expected), 0)) IO.puts("code:\n#{styled}") flunk("") @@ -134,11 +135,11 @@ defmodule Styler.StyleCase do end def style(code, filename \\ "testfile") do - {ast, comments} = Styler.string_to_quoted_with_comments(code) + {ast, comments} = Styler.string_to_ast(code) {styled_ast, comments} = Styler.style({ast, comments}, filename, on_error: :raise) try do - styled_code = styled_ast |> Styler.quoted_to_string(comments) |> String.trim_trailing("\n") + styled_code = styled_ast |> Styler.ast_to_string(comments) |> String.trim_trailing("\n") {styled_ast, styled_code, comments} rescue exception -> diff --git a/test/zipper_test.exs b/test/zipper_test.exs index fc03cd28..81dcf6e3 100644 --- a/test/zipper_test.exs +++ b/test/zipper_test.exs @@ -63,14 +63,14 @@ defmodule StylerTest.ZipperTest do describe "down/1" do test "rips and tears the parent node" do - assert [1, 2] |> Zipper.zip() |> Zipper.down() == {1, %{l: [], r: [2], ptree: {[1, 2], nil}}} - assert {1, 2} |> Zipper.zip() |> Zipper.down() == {1, %{l: [], r: [2], ptree: {{1, 2}, nil}}} + assert [1, 2] |> Zipper.zip() |> Zipper.down() == {1, {[], {[1, 2], nil}, [2]}} + assert {1, 2} |> Zipper.zip() |> Zipper.down() == {1, {[], {{1, 2}, nil}, [2]}} assert {:foo, [], [1, 2]} |> Zipper.zip() |> Zipper.down() == - {1, %{l: [], r: [2], ptree: {{:foo, [], [1, 2]}, nil}}} + {1, {[], {{:foo, [], [1, 2]}, nil}, [2]}} assert {{:., [], [:a, :b]}, [], [1, 2]} |> Zipper.zip() |> Zipper.down() == - {{:., [], [:a, :b]}, %{l: [], r: [1, 2], ptree: {{{:., [], [:a, :b]}, [], [1, 2]}, nil}}} + {{:., [], [:a, :b]}, {[],{{{:., [], [:a, :b]}, [], [1, 2]}, nil}, [1, 2]}} end end @@ -470,9 +470,9 @@ defmodule StylerTest.ZipperTest do |> Zipper.root() == [1, :left, 2, :right, 3] end - test "raise when attempting to insert a sibling at the root" do - assert_raise ArgumentError, fn -> 42 |> Zipper.zip() |> Zipper.insert_left(:nope) end - assert_raise ArgumentError, fn -> 42 |> Zipper.zip() |> Zipper.insert_right(:nope) end + test "builds a new root node made of a block" do + assert {42, {[:nope], {{:__block__, _, _}, nil}, []}} = 42 |> Zipper.zip() |> Zipper.insert_left(:nope) + assert {42, {[], {{:__block__, _, _}, nil}, [:nope]}} = 42 |> Zipper.zip() |> Zipper.insert_right(:nope) end end