From b3ccc02f0fbaa25cadc970dda5af1434fd3cfcb3 Mon Sep 17 00:00:00 2001 From: Matt Enlow Date: Thu, 8 Aug 2024 10:42:39 -0600 Subject: [PATCH] Wrap up 1.0.0 docs overhaul --- CHANGELOG.md | 564 ++---------------------------------- README.md | 12 +- docs/control_flow_macros.md | 17 +- docs/mix_configs.md | 77 ++++- docs/pipes.md | 122 ++++++-- mix.lock | 2 +- 6 files changed, 216 insertions(+), 578 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fbb76561..6e82a4c3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,44 +3,7 @@ **Note** Styler's only public API is its usage as a formatter plugin. While you're welcome to play with its internals, they can and will change without that change being reflected in Styler's semantic version. -## main - -### Improvements - -#### `with` - -* remove `with` structure with no left arrows in its head to be normal code (#174) -* `with true <- x(), do: y` => `if x(), do: y` (#173) - -### Fixes - -* don't blow up on `def function_head_with_no_body_nor_parens` (#185, h/t @ypconstante) -* fix `with` arrow replacement + redundant body removal creating invalid statements (#184, h/t @JesseHerrick) -* allow Kernel unary `!` and `not` as valid pipe starts (#183, h/t @nherzing) - -## 1.0.0-rc.2 - -### Fixes - -* fix `Map.drop(x, [a | b])` registering as a chance to refactor to `Map.delete` - -## 1.0.0-rc.1 - -### Improvements - -* Lots of documentation added. Nearly done and ready for 1.0.0. -* `Enum.into(x, [])` => `Enum.to_list(x)` -* `Enum.into(x, [], mapper)` => `Enum.map(x, mapper)` -* `a |> Enum.map(m) |> Enum.join()` to `map_join(a, m)`. we already did this for `join/2`, but missed the case for `join/1` - -## 1.0.0-rc.0 - -At this point, 1.0.0 feels feature complete. Two things remains for a full release: - -1. feedback! -2. documentation overhaul! [monitor progress here](https://github.com/adobe/elixir-styler/pull/166) - -### Improvements +## 1.0.0 Styler's two biggest outstanding bugs have been fixed, both related to compilation breaking during module directive organization. One was references to aliases being moved above where the aliases were declared, and the other was similarly module directives being moved after their uses in module directives. @@ -48,6 +11,10 @@ In both cases, Styler is now smart enough to auto-apply the fixes we recommended Other than that, a slew of powerful new features have been added, the neatest one (in the author's opinion anyways) being Alias Lifting. +Thanks to everyone who reported bugs that contributed to all the fixes released in 1.0.0 as well. + +### Improvements + #### Alias Lifting Along the lines of `Credo.Check.Design.AliasUsage`, Styler now "lifts" deeply nested aliases (depth >= 3, ala `A.B.C....`) that are used more than once. @@ -116,6 +83,18 @@ Styler now organizes `Mix.Config.config/2,3` stanzas according to erlang term so See the moduledoc for `Styler.Style.Configs` for more. +#### Pipe Optimizations + +* `Enum.into(x, [])` => `Enum.to_list(x)` +* `Enum.into(x, [], mapper)` => `Enum.map(x, mapper)` +* `a |> Enum.map(m) |> Enum.join()` to `map_join(a, m)`. we already did this for `join/2`, but missed the case for `join/1` +* `lhs |> Enum.reverse() |> Kernel.++(enum)` => `lhs |> Enum.reverse(enum)` + +#### `with` styles + +* remove `with` structure with no left arrows in its head to be normal code (#174) +* `with true <- x(), do: y` => `if x(), do: y` (#173) + #### Everything Else * `if`/`unless`: invert if and unless with `!=` or `!==`, like we do for `!` and `not` #132 @@ -124,10 +103,13 @@ See the moduledoc for `Styler.Style.Configs` for more. (`"\"\"\"\""` -> `~s("""")`) (`Credo.Check.Readability.StringSigils`) #146 * `Map.drop(foo, [single_key])` => `Map.delete(foo, single_key)` #161 (also in pipes) * `Keyword.drop(foo, [single_key])` => `Keyword.delete(foo, single_key)` #161 (also in pipes) -* `lhs |> Enum.reverse() |> Kernel.++(enum)` => `lhs |> Enum.reverse(enum)` ### Fixes +* don't blow up on `def function_head_with_no_body_nor_parens` (#185, h/t @ypconstante) +* fix `with` arrow replacement + redundant body removal creating invalid statements (#184, h/t @JesseHerrick) +* allow Kernel unary `!` and `not` as valid pipe starts (#183, h/t @nherzing) +* fix `Map.drop(x, [a | b])` registering as a chance to refactor to `Map.delete` * `alias`: expands aliases when moving an alias after another directive that relied on it (#137) * module directives: various fixes for unreported obscure crashes * pipes: fix a comment-shifting scenario when unpiping @@ -139,506 +121,4 @@ See the moduledoc for `Styler.Style.Configs` for more. * drop support for elixir `1.14` * ModuleDirectives: group callback attributes (`before_compile after_compile after_verify`) with nondirectives (previously, were grouped with `use`, their relative order maintained). to keep the desired behaviour, you can make new `use` macros that wrap these callbacks. Apologies if this makes using Styler untenable for your codebase, but it's probably not a good tool for macro-heavy libraries. -* sorting configs for the first time can change your configuration; see `Styler.Style.Configs` moduledoc for more - -## v0.11.9 - -### Improvements - -* pipes: check for `Stream.foo` equivalents to `Enum.foo` in a few more cases - -### Fixes - -* pipes: `|> then(&(&1 op y))` rewrites with `|> Kernel.op(y)` as long as the operator is defined in `Kernel`; skips the rewrite otherwise (h/t @kerryb for the report & @saveman71 for the fix) - -## v0.11.8 - -Two releases in one day!? @koudelka made too good a point about `Map.new` not being special... - -### Improvements - -* pipes: treat `MapSet.new` and `Keyword.new` the same way we do `Map.new` (h/t @koudelka) -* pipes: treat `Stream.map` the same as `Enum.map` when piped `|> Enum.into` - -## v0.11.7 - -### Improvements - -* deprecations: `~R` -> `~r`, `Date.range/2` -> `Date.range/3` with decreasing dates (h/t @milmazz) -* if: rewrite `if not x, do: y` => `unless x, do: y` -* pipes: `|> Enum.map(foo) |> Map.new()` => `|> Map.new(foo)` -* pipes: remove unnecessary `then/2` on named function captures: `|> then(&foo/1)` => `|> foo()`, `|> then(&foo(&1, ...))` => `|> foo(...)` (thanks to @tfiedlerdejanze for the idea + impl!) - -## v0.11.6 - -### Fixes - -* directives: maintain order of module compilation callbacks (`@before_compile` etc) relative to `use` statements (Closes #120, h/t @frankdugan3) - -## v0.11.5 - -### Fixes - -* fix parsing ranges with non-trivial integer bounds like `x..y` (Closes #119, h/t @maennchen) - -## v0.11.4 - -### Improvements - -Shoutout @milmazz for all the deprecation work below =) - -* Deprecations: Rewrite 1.16 Deprecations (h/t @milmazz for all the work here) - * add `//1` step to `Enum.slice/2|String.slice/2` with decreasing ranges - * `File.stream!(file, options, line_or_bytes)` => `File.stream!(file, line_or_bytes, options)` -* Deprecations `Path.safe_relative_to/2` => `Path.safe_relative/2` - -## v0.11.3 - -### Fixes - -* directives: fix infinite loop when encountering `@spec import(...) :: ...` (Closes #115, h/t @kerryb) -* `with`: fix deletion of arrow-less `with` statements within function invocations - -## v0.11.2 - -### Fixes - -* `pipes`: fix unpiping do-blocks into variables when the parent expression is a function invocation - like `a(if x do y end |> z(), b)` (Closes #114, h/t @wkirschbaum) - -## v0.11.1 - -### Fixes - -* `with`: fix `with` replacement when it's the only child of a `do` or `->` block (Closes #107, h/t @kerryb -- turns out those edge cases _did_ exist in the wild!) - -## v0.11.0 - -### Improvements - -#### Comments - -Styler will no longer make comments jump around in any situation, and will move comments with the appropriate node in all cases but module directive rearrangement (where they'll just be left behind - sorry! we're still working on it). - -* Keep comments in logical places when rewriting if/unless/cond/with (#79, #97, #101, #103) - -#### With Statements - -This release has a slew of improvements for `with` statements. It's not surprising that there's lots of style rules for `with` given that just about any `case`, `if`, or even `cond do` could also be expressed as a `with`. They're very powerful! And with great power... - -* style trivial pattern matches ala `lhs <- rhs` to `lhs = rhs` (#86) -* style `_ <- rhs` to `rhs` -* style keyword `, do: ` to `do end` rather than wrapping multiple statements in parens -* style statements all the way to `if` statements when appropriate (#100) - -#### Other - -* Rewrite `{Map|Keyword}.merge(single_key: value)` to use `put/3` instead (#96) - -### Fixes - -* `with`: various edge cases we can only hope no one's encountered and thus never reported - -## v0.10.5 - -After being bitten by two of them in a row, Styler's test suite now makes sure that there are no -idempotency bugs as part of its tests. - -In short, we now have `assert style(x) == style(style(x))` as part of every test. Sorry for not thinking to include this before :) - -### Fixes - -* alias: fix single-module alias deletion newlines bug -* comments: ensure all generated nodes always include line meta (#101) - -## v0.10.4 - -### Improvements - -* alias: delete noop single-module aliases (`alias Foo`, #87, h/t @mgieger) - -### Fixes - -* pipes: unnest all pipe starts in one pass (`f(g(h(x))) |> j()` => `x |> h() |> g() |> f() |> j()`, #94, h/t @tomjschuster) - -## v0.10.3 - -### Improvements - -* charlists: leave charlist rewriting to elixir's formatter on elixir >= 1.15 - -### Fixes - -* charlists: rewrite empty charlist to use sigil (`''` => `~c""`) -* pipes: don't blow up extracting fully-qualified macros (`Foo.bar do end |> foo()`, #91, h/t @NikitaNaumenko) - -## v0.10.2 - -### Improvements - -* `with`: remove identity singleton else clause (eg `else {:error, e} -> {:error, e} end`, `else error -> error end`) - -## v0.10.1 - -### Fixes - -* Fix function head shrink-failures causing comments to jump into blocks (Closes #67, h/t @APB9785) - -## v0.10.0 - -### Improvements - -* hoist all block-starts to pipes to their own variables (makes styler play better with piped macros) - -### Fixes - -* fix pipes starting with a macro do-block creating invalid ast (#83, h/t @mhanberg) - -## v0.9.7 - -### Fixes - -* rewrite pipes starting with `quote` blocks like we do with `case|if|cond|with` blocks (#82, h/t @SteffenDE) - -## v0.9.6 - -### Breaking Change - -* removed `mix style` task - -## v0.9.5 - -### Fixes - -* fix mistaking `Timex.now/1` in a pipe for `Timex.now/0` (#66, h/t @sabiwara) - -### Removed style - -* stop rewriting `Timex.today/0` given that we allow `Timex.today/1` -- too inconsistent. - -## v0.9.4 - -### Improvements - -* `if` statements: drop `else` clauses whose body is simply `nil` - -## v0.9.3 - -### Fixes - -* fix `unless a do b else c end` rewrites to `if` not flopping do/else bodies! (#77, h/t @jcowgar) -* fix pipes styling ranges with steps (`a..b//c`) incorrectly (#76, h/t @cschmatzler) - -## v0.9.2 - -### Fixes - -* fix exception styling module attributes named `@def` (we confused them with real `def`s, whoops!) (#75, h/t @randycoulman) - -## v0.9.1 - -the boolean blocks edition! - -### Improvements - -* auto-fix `Credo.Check.Refactor.CondStatements` (detects any truthy atom, not just `true`) -* if/unless rewrites: - - `Credo.Check.Refactor.NegatedConditionsWithElse` - - `Credo.Check.Refactor.NegatedConditionsInUnless` - - `Credo.Check.Refactor.UnlessWithElse` - -## v0.9.0 - -the with statement edition! - -### Improvements - -* Added right-hand-pattern-matching rewrites to `for` and `with` left arrow expressions `<-` - (ex: `with map = %{} <- foo()` => `with %{} = map <- foo`) -* `with` statement rewrites, solving the following credo rules - * `Credo.Check.Readability.WithSingleClause` - * `Credo.Check.Refactor.RedundantWithClauseResult` - * `Credo.Check.Refactor.WithClauses` - -## v0.8.5 - -### Fixes - -* Fixed exception when encountering non-arrowed case statements ala `case foo, do: unquote(quoted)` (#69, h/t @brettinternet, nice) - -## v0.8.4 - -### Fixes - -* Timex related fixes (#66): - * Rewrite `Timex.now/1` to `DateTime.now!/1` instead of `DateTime.utc_now/1` - * Only rewrite `Timex.today/0`, don't change `Timex.today/1` - -## v0.8.3 - -### Improvements - -* DateTime rewrites (#62, ht @milmazz) - * `DateTime.compare` => `DateTime.{before/after}` (elixir >= 1.15) - * `Timex.now` => `DateTime.utc_now` - * `Timex.today` => `Date.utc_today` - -### Fixes - -* Pipes: add `!=`, `!==`, `===`, `and`, and `or` to list of valid infix operators (#64) - -## v0.8.2 - -### Fixes - -* Pipes always de-sugars keyword lists when unpiping them (#60) - -## v0.8.1 - -### Fixes - -* ModuleDirectives doesn't mistake variables for directives (#57, h/t @leandrocp) - -## v0.8.0 - -### Improvements (Bug Fix!?) - -* ModuleDirectives no longer throws comments around a file when hoisting directives up (#53) - -## v0.7.14 - -### Improvements - -* rewrite `Logger.warn/1,2` to `Logger.warning/1,2` due to Elixir 1.15 deprecation - -## v0.7.13 - -### Fixes - -* don't unpipe single-piped `unquote` expressions (h/t @elliottneilclark) - -## v0.7.12 - -### Fixes - -* fix 0-arity paren removal on metaprogramming creating uncompilable code (h/t @simonprev) - -## v0.7.11 - -### Fixes - -* fix crash from `mix style` running plugins as part of formatting (no longer runs formatter plugins) - -### Improvements - -* single-quote charlists are rewritten to use the `~c` sigil (`'foo'` -> `~c'foo'`) (h/t @fhunleth) -* `mix style` warns the user that Styler is primarily meant to be used as a plugin - -## v0.7.10 - -### Fixes - -* fix crash when encountering single-quote charlists (h/t @fhunleth) - -### Improvements - -* single-quote charlists are rewritten to use the `~c` sigil (`'foo'` -> `~c'foo'`) -* when encountering `_ = bar ->`, replace it with `bar ->` - -## v0.7.9 - -### Fixes - -* Fix a toggle state resulting from (ahem, nonsense) code like `_ = bar ->` encountering ParameterPatternMatching style - -## v0.7.8 - -### Fixes - -* Fix crash trying to remove 0-arity parens from metaprogramming ala `def unquote(foo)()` - -## v0.7.7 - -### Improvements - -* Rewrite `Enum.into/2,3` into `Map.new/1,2` when the collectable is `%{}` or `Map.new/0` - -## v0.7.6 - -### Fixes - -* Fix crash when single pipe had inner defs (h/t [@michallepicki](https://github.com/adobe/elixir-styler/issues/39)) - -## v0.7.5 - -### Fixes - -* Fix bug from `ParameterPatternMatching` implementation that re-ordered pattern matching in `cond do` `->` clauses - -## v0.7.4 - -### Features - -* Implement `Credo.Check.Readability.PreferImplicitTry` -* Implement `Credo.Check.Consistency.ParameterPatternMatching` for `def|defp|fn|case` - -## v0.7.3 - -### Features - -* Remove parens from 0-arity function definitions (`Credo.Check.Readability.ParenthesesOnZeroArityDefs`) - -## v0.7.2 - -### Features - -* Rewrite `case ... true -> ...; _ -> ...` to `if` statements as well - -## v0.7.1 - -### Features - -* Rewrite `case ... true / else ->` to be `if` statements - -## v0.7.0 - -### Features - -* `Styler.Style.Simple`: - * Optimize `Enum.reverse(foo) ++ bar` to `Enum.reverse(foo, bar)` -* `Styler.Style.Pipes`: - * Rewrite `|> (& ...).()` to `|> then(& ...)` (`Credo.Check.Readability.PipeIntoAnonymousFunctions`) - * Add parens to 1-arity pipe functions (`Credo.Check.Readability.OneArityFunctionInPipe`) - * Optimize `a |> Enum.reverse() |> Enum.concat(enum)` to `Enum.reverse(a, enum)` - -## v0.6.1 - -### Improvements - -* Better error handling: `mix format` will still format files if a style fails - -### Fixes - -* `mix style`: only run on `.ex` and `.exs` files -* `ModuleDirectives`: now expands `alias __MODULE__.{A, B}` (h/t [@adriankumpf](https://github.com/adriankumpf)) - -## v0.6.0 - -### Features - -* `mix style`: brought back to life for folks who want to incrementally introduce Styler - -### Fixes - -* `Styler.Style.Pipes`: - * include `x in y` and `^foo` (for ecto) as a valid pipe starts - * work even harder to keep rewrites on one line - -## v0.5.2 - -### Fixes - -* `ModuleDirectives`: handle dynamic module names -* `Pipes`: include `Ecto.Query.from` and `Query.from` as valid pipe starts - -## v0.5.1 - -### Improvements - -* Sped up styling just a little bit - -## v0.5.0 - -### Improvements - -* `Styler` now implements `Mix.Task.Format`, meaning it is now an Elixir formatter plugin. -See the README for new installation & usage instructions - -### Breaking Change! Wooo! - -* the `mix style` task has been removed - -## v0.4.1 - -### Improvements - -* `Pipes` rewrites `|> Enum.into(%{}[, mapper])` and `Enum.into(Map.new()[, mapper])` to `Map.new/1,2` calls - -## v0.4.0 - -### Improvements - -* `Pipes` rewrites some two-step processes into one, fixing these credo issues in pipe chains: - * `Credo.Check.Refactor.FilterCount` - * `Credo.Check.Refactor.MapJoin` - * `Credo.Check.Refactor.MapInto` - -### Fixes - -* `ModuleDirectives` handles even weirder places to hide your aliases (anonymous functions, in this case) -* `Pipes` tries even harder to keep single-pipe rewrites of invocations on one line - -## v0.3.1 - -### Fixes - -* `Pipes` - * fixed omission of `==` as a valid pipe start operator (h/t @peake100 for the issue) - * fixed rewrite of `a |> b`, where `b` was invoked without parenthesis - -## v0.3.0 - -### Improvements - -* Enabled `Defs` style and overhauled it to properly handles comments -* Optimized and tweaked `ModuleDirectives` style - * Now culls newlines between "groups" of the same directive - * sorts `@behaviour` directives - * orders directives within non defmodule contexts (eg, a `def do`) if there's at least one `alias|require|use|import` - -### Fixes - -* `Pipes` will try to keep single-pipe rewrites on one line - -## v0.2.0 - -### Improvements - -* Added `ModuleDirectives` style - * Note that this is potentially destructive in some rare cases. See moduledoc for more. - * This supersedes the `Aliases` style, which has been removed. -* `mix style -` reads and writes to stdin/stdout - -### Fixes - -* `Pipes` style is now aware of `unless` blocks - -## v0.1.1 - -### Improvements - -* Lots of README tweaking =) -* Optimized some Zipper operations -* Added `Simple` style, replacing the following Credo rule: - * `Credo.Check.Readability.LargeNumbers` - -### Fixes - -* Exceptions while parsing code now appropriately render filename rather than `nofile:xx` -* Fixed opaque `Zipper.path()` typespec implementation mismatches (thanks @sega-yarkin) -* Made `ex_doc` dev only, removing it as a dependency for users of Styler - -## v0.1.0 - -### Improvements - -* Initial release of Styler -* Added `Aliases` style, replacing the following Credo rules: - * `Credo.Check.Readability.AliasOrder` - * `Credo.Check.Readability.MultiAlias` - * `Credo.Check.Readability.UnnecessaryAliasExpansion` -* Added `Pipes` style, replacing the following Credo rules: - * `Credo.Check.Readability.BlockPipe` - * `Credo.Check.Readability.SinglePipe` - * `Credo.Check.Refactor.PipeChainStart` -* Added `Defs` style (currently disabled by default) +* sorting configs for the first time can change your configuration; see [Mix Configs docs](docs/mix_configs.md) for more diff --git a/README.md b/README.md index de88728c..a4a1b085 100644 --- a/README.md +++ b/README.md @@ -13,10 +13,11 @@ You can learn more about the history, purpose and implementation of Styler from - 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 + - alphabetizes module directives - [extracts repeated aliases](docs/module_directives.md#alias-lifting) -- 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)`) +- [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 @@ -24,15 +25,14 @@ You can learn more about the history, purpose and implementation of Styler from ## Who is Styler for? -Styler was designed for a large team (40+ engineers) working in a single codebase. It helps remove fiddly code review comments and removes failed linter CI slowdowns, helping teams get things done faster. Teams in similar situations might appreciate Styler. +Styler was designed for a **large team (40+ engineers) working in a single codebase. It helps remove fiddly code review comments and removes failed linter CI slowdowns, helping teams get things done faster. Teams in similar situations might appreciate Styler. Its automations are also extremely valuable for taming legacy elixir codebases or just refactoring in general. Some of its rewrites have inspired code actions in elixir language servers. Conversely, Styler probably _isn't_ a good match for: -- libraries - experimental, macro-heavy codebases -- small teams that don't want to think about code standards +- teams that don't care about code standards ## Installation diff --git a/docs/control_flow_macros.md b/docs/control_flow_macros.md index d4123a9b..39243034 100644 --- a/docs/control_flow_macros.md +++ b/docs/control_flow_macros.md @@ -97,26 +97,29 @@ if x, do: y ### "Erlang heritage" `case` true/false -> `if` -Trivial true/false `case` statements are rewritten to `if` statements. While this results in a [semantically different program](https://github.com/rrrene/credo/issues/564#issue-338349517), we argue that it results in a better program for maintainability. If the developer wants a their case statement to raise when receiving a non-boolean value as a feature of the program, they would better serve their callers by raising something more descriptive. +Trivial true/false `case` statements are rewritten to `if` statements. While this results in a [semantically different program](https://github.com/rrrene/credo/issues/564#issue-338349517), we argue that it results in a better program for maintainability. If the developer wants their case statement to raise when receiving a non-boolean value as a feature of the program, they would better serve their callers by raising something more descriptive. In other words, Styler leaves the code with better style, trumping obscure exception design :) ```elixir -# instead of this +# Styler will rewrite this even if the clause order is flipped, +# and if the `false` is replaced with a wildcard (`_`) case foo do true -> :ok false -> :error end -# do this +# styled: if foo do :ok else :error end +``` + +Per the argument above, if the `if` statement is an incorrect rewrite for your program, we recommend this manual fix rewrite: -# OR this. readers now know that the exception is an intentional design, -# rather than an accidental "feature" +```elixir case foo do true -> :ok false -> :error @@ -265,8 +268,6 @@ If the pattern of the final clause of the head is also the `with` statements `do with {:ok, a} <- foo(), {:ok, b} <- bar(a) do {:ok, b} -else - error -> error end # Styled: with {:ok, a} <- foo() do @@ -276,7 +277,7 @@ end ### Replace with `case` -A `with` statement with a single clause in the head and `else` is a really just a `case` clause putting on airs. +A `with` statement with a single clause in the head and an `else` body is really just a `case` statement putting on airs. ```elixir # Given: diff --git a/docs/mix_configs.md b/docs/mix_configs.md index c3b45e8e..70bf88fd 100644 --- a/docs/mix_configs.md +++ b/docs/mix_configs.md @@ -4,7 +4,7 @@ Mix Config files have their config stanzas sorted. Similar to the sorting of ali A file is considered a config file if -1. its path matches `config/.*\.exs` or `rel/overlays/.*\.exs` +1. its path matches `~r|config/.*\.exs|` `~r|rel/overlays/.*\.exs|` 2. the file has `import Config` Once a file is detected as a mix config, its `config/2,3` stanzas are grouped and ordered like so: @@ -13,6 +13,79 @@ Once a file is detected as a mix config, its `config/2,3` stanzas are grouped an - sort each group according to erlang term sorting - move all existing assignments between the config stanzas to above the stanzas (without changing their ordering) +## THIS CAN BREAK YOUR PROGRAM + +It's important to double check your configuration after running Styler on it for the first time. + +**First Use Advice**: To limit the size of changes Styler submits to a codebase, we recommend formatting only a few (or a single) files at a time and making pull requests for each. Only commit Styler as a new formatter plugin once each of these more dangerous changes has been safely committed to the codebase. + +Imagine your application configures the same value twice, once with an invalid or application breaking value, and then again with a correct value, like so: + +```elixir +string = "i am a string" +atom = :i_am_an_atom + +config :my_app, value_must_be_an_atom: string +... +... +config :my_app, value_must_be_an_atom: atom +``` + +When styler sorts the configuration file, this dormant mistake can become a bug if the sorting changes the order such that the invalid value takes precedence (aka comes last) + +```elixir +string = "i am a string" +atom = :i_am_an_atom + +# The value that must be an atom is now a string! +config :my_app, value_must_be_an_atom: atom +config :my_app, value_must_be_an_atom: string +``` + ## Examples -TODOs +Sorts configs by erlang term ordering: + +```elixir +# Given +import Config + +config :z, :x, :c +config :a, :b, :c +config :y, :x, :z +config :a, :c, :d + +# Styled: +import Config + +config :a, :b, :c +config :a, :c, :d + +config :y, :x, :z + +config :z, :x, :c +``` + +Non-config statements break the file up into chunks, where each chunk is sorted separately relative to itself. + +```elixir +# Given +import Config + +config :z, :x, :c +config :a, :b, :c +var = "value" +config :y, :x, var +config :a, :c, var + +# Styled: +import Config + +config :a, :b, :c +config :z, :x, :c + +var = "value" + +config :a, :c, var +config :y, :x, var +``` diff --git a/docs/pipes.md b/docs/pipes.md index 9958bcc6..5b8f065c 100644 --- a/docs/pipes.md +++ b/docs/pipes.md @@ -2,30 +2,114 @@ ## Pipe Start -- raw value -- blocks are extracted to variables -- ecto's `from` is allowed +Styler will ensure that the start of a pipechain is a 0-arity function, a raw value, or a variable. -## Piped function rewrites +```elixir +Enum.at(enum, 5) +|> IO.inspect() + +# Styled: +enum +|> Enum.at(5) +|> IO.inspect() +``` + +If the start of a pipe is a block expression, styler will create a new variable to store the result of that expression and make that variable the start of the pipe. + +```elixir +if a do + b +else + c +end +|> Enum.at(4) +|> IO.inspect() + +# Styled: +if_result = + if a do + b + else + c + end + +if_result +|> Enum.at(4) +|> IO.inspect() +``` + +### Add parenthesis to function calls in pipes + +```elixir +a |> b |> c |> d +# Styled: +a |> b() |> c() |> d() +``` + +### Remove Unnecessary `then/2` + +When the piped argument is being passed as the first argument to the inner function, there's no need for `then/2`. + +```elixir +a |> then(&f(&1, ...)) |> b() +# Styled: +a |> f(...)) |> b() +``` - add parens to function calls `|> fun |>` => `|> fun() |>` -- remove unnecessary `then/2`: `|> then(&f(&1, ...))` -> `|> f(...)` -- add `then/2` when defining anon funs in pipe `|> (& &1).() |>` => `|> then(& &1) |>` -## Piped function optimizations +### Add `then/2` when defining and calling anonymous functions in pipes + +```elixir +a |> (fn x -> x end).() |> c() +# Styled: +a |> then(fn x -> x end) |> c() +``` + +### Piped function optimizations + +Two function calls into one! Fewer steps is always nice. + +```elixir +# reverse |> concat => reverse/2 +a |> Enum.reverse() |> Enum.concat(enum) |> ... +# Styled: +a |> Enum.reverse(enum) |> ... + +# filter |> count => count(filter) +a |> Enum.filter(filterer) |> Enum.count() |> ... +# Styled: +a |> Enum.count(filterer) |> ... + +# map |> join => map_join +a |> Enum.map(mapper) |> Enum.join(joiner) |> ... +# Styled: +a |> Enum.map_join(joiner, mapper) |> ... + +# Enum.map |> X.new() => X.new(mapper) +# where X is one of: Map, MapSet, Keyword +a |> Enum.map(mapper) |> Map.new() |> ... +# Styled: +a |> Map.new(mapper) |> ... + +# Enum.map |> Enum.into(empty_collectable) => X.new(mapper) +# Where empty_collectable is one of `%{}`, `Map.new()`, `Keyword.new()`, `MapSet.new()` +# Given: +a |> Enum.map(mapper) |> Enum.into(%{}) |> ... +# Styled: +a |> Map.new(mapper) |> ... +``` -Two function calls into one! Tries to fit everything on one line when shrinking. +### Unpiping Single Pipes -| Before | After | -|--------|-------| -| `lhs \|> Enum.reverse() \|> Enum.concat(enum)` | `lhs \|> Enum.reverse(enum)` (also Kernel.++) | -| `lhs \|> Enum.filter(filterer) \|> Enum.count()` | `lhs \|> Enum.count(filterer)` | -| `lhs \|> Enum.map(mapper) \|> Enum.join(joiner)` | `lhs \|> Enum.map_join(joiner, mapper)` | -| `lhs \|> Enum.map(mapper) \|> Enum.into(empty_map)` | `lhs \|> Map.new(mapper)` | -| `lhs \|> Enum.map(mapper) \|> Map.new()` | `lhs \|> Map.new(mapper)` mapset & keyword also | +Styler rewrites pipechains with a single pipe to be function calls. Notably, this rule combined with the optimizations rewrites above means some chains with more than one pipe will also become function calls. -## Unpiping Single Pipes +```elixir +foo = bar |> baz() +# Styled: +foo = baz(bar) -- notably, optimizations might turn a 2 pipe into a single pipe -- doesn't unpipe when we're starting w/ quote -- pretty straight forward i daresay +map = a |> Enum.map(mapper) |> Map.new() +# Styled: +map = Map.new(a, mapper) +``` diff --git a/mix.lock b/mix.lock index ad250b75..3339de94 100644 --- a/mix.lock +++ b/mix.lock @@ -1,6 +1,6 @@ %{ "earmark_parser": {:hex, :earmark_parser, "1.4.39", "424642f8335b05bb9eb611aa1564c148a8ee35c9c8a8bba6e129d51a3e3c6769", [:mix], [], "hexpm", "06553a88d1f1846da9ef066b87b57c6f605552cfbe40d20bd8d59cc6bde41944"}, - "ex_doc": {:hex, :ex_doc, "0.31.1", "8a2355ac42b1cc7b2379da9e40243f2670143721dd50748bf6c3b1184dae2089", [:mix], [{:earmark_parser, "~> 1.4.39", [hex: :earmark_parser, repo: "hexpm", optional: false]}, {:makeup_c, ">= 0.1.1", [hex: :makeup_c, repo: "hexpm", optional: true]}, {:makeup_elixir, "~> 0.14", [hex: :makeup_elixir, repo: "hexpm", optional: false]}, {:makeup_erlang, "~> 0.1", [hex: :makeup_erlang, repo: "hexpm", optional: false]}], "hexpm", "3178c3a407c557d8343479e1ff117a96fd31bafe52a039079593fb0524ef61b0"}, + "ex_doc": {:hex, :ex_doc, "0.34.2", "13eedf3844ccdce25cfd837b99bea9ad92c4e511233199440488d217c92571e8", [:mix], [{:earmark_parser, "~> 1.4.39", [hex: :earmark_parser, repo: "hexpm", optional: false]}, {:makeup_c, ">= 0.1.0", [hex: :makeup_c, repo: "hexpm", optional: true]}, {:makeup_elixir, "~> 0.14 or ~> 1.0", [hex: :makeup_elixir, repo: "hexpm", optional: false]}, {:makeup_erlang, "~> 0.1 or ~> 1.0", [hex: :makeup_erlang, repo: "hexpm", optional: false]}, {:makeup_html, ">= 0.1.0", [hex: :makeup_html, repo: "hexpm", optional: true]}], "hexpm", "5ce5f16b41208a50106afed3de6a2ed34f4acfd65715b82a0b84b49d995f95c1"}, "makeup": {:hex, :makeup, "1.1.1", "fa0bc768698053b2b3869fa8a62616501ff9d11a562f3ce39580d60860c3a55e", [:mix], [{:nimble_parsec, "~> 1.2.2 or ~> 1.3", [hex: :nimble_parsec, repo: "hexpm", optional: false]}], "hexpm", "5dc62fbdd0de44de194898b6710692490be74baa02d9d108bc29f007783b0b48"}, "makeup_elixir": {:hex, :makeup_elixir, "0.16.1", "cc9e3ca312f1cfeccc572b37a09980287e243648108384b97ff2b76e505c3555", [:mix], [{:makeup, "~> 1.0", [hex: :makeup, repo: "hexpm", optional: false]}, {:nimble_parsec, "~> 1.2.3 or ~> 1.3", [hex: :nimble_parsec, repo: "hexpm", optional: false]}], "hexpm", "e127a341ad1b209bd80f7bd1620a15693a9908ed780c3b763bccf7d200c767c6"}, "makeup_erlang": {:hex, :makeup_erlang, "0.1.3", "d684f4bac8690e70b06eb52dad65d26de2eefa44cd19d64a8095e1417df7c8fd", [:mix], [{:makeup, "~> 1.0", [hex: :makeup, repo: "hexpm", optional: false]}], "hexpm", "b78dc853d2e670ff6390b605d807263bf606da3c82be37f9d7f68635bd886fc9"},