From b72fb94bc51b22eaf19a36456894fdd801152be0 Mon Sep 17 00:00:00 2001 From: Matt Enlow Date: Thu, 16 May 2024 09:22:44 -0600 Subject: [PATCH] step --- docs/styles.cheatmd | 172 ++++++++++++++++++++++++++++++++++++++++---- lib/style/pipes.ex | 8 +-- 2 files changed, 162 insertions(+), 18 deletions(-) diff --git a/docs/styles.cheatmd b/docs/styles.cheatmd index febc4bf..4017997 100644 --- a/docs/styles.cheatmd +++ b/docs/styles.cheatmd @@ -2,27 +2,172 @@ ## Simple (Single Node) Styles -These styles typically modify a single function call or similarly minor one-line(-ish) changes. In nerd terms, they are styles that can be executed without regard to sibling or parent nodes in the AST -- "Single Node Styles". -### Literal Rewrites +Function Performance & Readability Optimizations -- string sigils -- large base 10 numbers +Optimizing for either performance or readability, probably both! +These apply to the piped versions as well -### Function Optimizations -These apply to the piped versions as well +### Strings to Sigils + +Rewrites strings with 4 or more escaped quotes to string sigils with an alternative delimiter. +The delimiter will be one of `" ( { | [ ' < /`, chosen by which would require the fewest escapes, and otherwise preferred in the order listed. + +#### Before + +```elixir +conn +|> put_resp_content_type("application/json") +|> send_resp(403, "{\"errors\":[\"Not Authorized\"]}") +|> halt() +``` + +#### After + +```elixir +conn +|> put_resp_content_type("application/json") +|> send_resp(403, ~s({"errors":["Not Authorized"]}))) +|> halt() +``` + +### Large Base 10 Numbers + +Style base 10 numbers with 5 or more digits to have a `_` every three digits. +Formatter already does this except it doesn't rewrite "typos" like `100_000_0`. + +If you're concerned that this breaks your team's formatting for things like "cents" (like "$100" being written as `100_00`), +consider using a library made for denoting currencies rather than raw elixir integers. + +#### Before + +```elixir +10000 +1_0_0_0_0 # Elixir's formatter is fine with this +-543213 +123456789 +55333.22 +-123456728.0001 +``` + +#### After + +```elixir +10_000 +10_000 +-543_213 +123_456_789 +55_333.22 +-123_456_728.0001 +``` + +### `Enum.into(%{}/Map/Keyword/MapSet.new)` -> `X.new` + +While these examples use `%{}`, the same behaviour occurs for `Keyword.new()`, `MapSet.new()` and the empty map `%{}`. + +This is an improvement for the reader, who gets a more natural language expression: "make a new map from a" vs "take a and enumerate it into a new map" + +#### Before + +```elixir +Enum.into(a, %{}) +Enum.into(a, %{}, mapping_function) +``` + +#### After + +```elixir +Map.new(a) +Map.new(a, mapping_function) +``` - Enum.into(%{}/Map/Keyword/MapSet.new) -> X.new -- Map/Keyword.merge w/ single key literal -> X.put -- Enum.reverse(foo) ++ bar -> Enum.reverse(foo, bar) -### Function Readability +### Map/Keyword.merge w/ single key literal -> X.put -- Timex.now/0 -> DateTime.utc_now/0 -- DateModule.compare(x, y) == :lt/:gt -> DateModule.before?/after? +`Keyword.merge` and `Map.merge` called with a literal map or keyword argument with a single key are rewritten to the +equivalent `put`, a cognitively simpler function. + +#### Before +```elixir +foo |> Keyword.merge(%{just_one_key: the_value}) |> bar() +``` + +#### After +```elixir +foo |> Keyword.put(:just_one_key, the_value) |> bar() +``` + +### Map/Keyword.drop w/ single key -> X.delete + +In the same vein as the `merge` style above, `[Map|Keyword].drop/2` with a single key to drop are rewritten to use `delete/2` + +#### Before +```elixir +Map.drop(foo, [key]) +``` +#### After +```elixir +Map.delete(foo, key) +``` + +### `Enum.reverse(foo) ++ bar -> Enum.reverse(foo, bar)` + +`Enum.reverse/2` optimizes a two-step reverse and concatenation into a single step. + +#### Before +```elixir +Enum.reverse(foo) ++ bar + +baz +|> Enum.reverse() +|> Enum.concat(bop) + +``` +#### After +```elixir +Enum.reverse(foo, bar) + +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! +We prefer calling the actual function rather than its rename in Timex, helping the reader by being more explicit. + +#### Before +```elixir +Timex.now() +``` +#### After +```elixir +DateTime.utc_now() +``` + + +### DateModule.compare(x, y) == :lt/:gt -> DateModule.before?/after? + +Again, the goal is readability and maintainability. `before?/2` and `after?/2` were implemented long after `compare/2`, +so it's not unusual that a codebase needs a lot of refactoring to be brought up to date with these new functions. +That's where Styler comes in! + +#### Before +```elixir +if DateTime.compare(start, end) == :gt, + do: :error, + else: :ok +``` +#### After +```elixir +if DateTime.after?(start, end), + do: :error, + else: :ok +``` ### Code Readability + - put matches on right - `Credo.Check.Readability.PreferImplicitTry` @@ -112,15 +257,14 @@ if/unless often looks to see if the root of the statement is a "negator", define ### Piped function optimizations -- tries to fit everything on one line +Two function calls into one! Tries to fit everything on one line when shrinking. + - `lhs |> Enum.reverse() |> Enum.concat(enum)` => `lhs |> Enum.reverse(enum)` (also Kernel.++) - `lhs |> Enum.filter(filterer) |> Enum.count()` => `lhs |> Enum.count(count)` - `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) |> Enum.into(collectable)` => `lhs |> Enum.into(collectable, mapper)` -- `lhs |> Enum.into(%{}, ...) => lhs |> Map.new(...)` - `lhs |> Enum.map(mapper) |> Map.new()` => `lhs |> Map.new(mapper)` mapset & keyword also -- `lhs |> Map.merge(%{key: value}) => lhs |> Map.put(key, value)` (keyword literal also) ### Unpiping Single Pipes diff --git a/lib/style/pipes.ex b/lib/style/pipes.ex index 4e0b252..3d10d54 100644 --- a/lib/style/pipes.ex +++ b/lib/style/pipes.ex @@ -209,7 +209,7 @@ defmodule Styler.Style.Pipes do {:|>, [line: meta[:line]], [lhs, {reverse, [line: meta[:line]], [enum]}]} end - # `lhs |> Enum.reverse() |> Enum.concat(enum)` => `lhs |> Enum.reverse(enum)` + # `lhs |> Enum.reverse() |> Kernel.++(enum)` => `lhs |> Enum.reverse(enum)` defp fix_pipe( pipe_chain( lhs, @@ -245,7 +245,7 @@ defmodule Styler.Style.Pipes do {:|>, [line: dm[:line]], [lhs, rhs]} end - # `lhs |> Enum.map(mapper) |> Enum.into(empty_map)` => `lhs |> Map.new(mapper) + # `lhs |> Enum.map(mapper) |> Enum.into(empty_map)` => `lhs |> Map.new(mapper)` # or # `lhs |> Enum.map(mapper) |> Enum.into(collectable)` => `lhs |> Enum.into(collectable, mapper) defp fix_pipe( @@ -271,7 +271,7 @@ defmodule Styler.Style.Pipes do Style.set_line({:|>, [], [lhs, rhs]}, dm[:line]) end - # `lhs |> Enum.into(%{}, ...)`` => `lhs |> Map.new(...)`` + # lhs |> Enum.into(%{}, ...) => lhs |> Map.new(...) defp fix_pipe({:|>, meta, [lhs, {{:., dm, [{_, _, [:Enum]}, :into]}, _, [collectable | rest]}]} = node) do replacement = case collectable do @@ -288,7 +288,7 @@ defmodule Styler.Style.Pipes do if replacement, do: {:|>, meta, [lhs, {replacement, dm, rest}]}, else: node end - # `lhs |> Enum.map(mapper) |> Map.new()` => `lhs |> Map.new(mapper)`` + # `lhs |> Enum.map(mapper) |> Map.new()` => `lhs |> Map.new(mapper)` defp fix_pipe( pipe_chain(lhs, {{:., _, [{_, _, [enum]}, :map]}, _, [mapper]}, {{:., _, [{_, _, [mod]}, :new]} = new, nm, []}) )