Skip to content

Commit

Permalink
implement string sigil rewrites
Browse files Browse the repository at this point in the history
  • Loading branch information
novaugust committed Mar 28, 2024
1 parent 817983e commit c29b841
Show file tree
Hide file tree
Showing 4 changed files with 62 additions and 4 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
**this is a big one!** please report any issues :) #135
* `if`/`unless`: invert if and unless with `!=` or `!==`, like we do for `!` and `not` #132
* `@derive`: move `@derive` before `defstruct|schema|embedded_schema` declarations (fixes compiler warning!) #134
* strings: rewrite double-quoted strings to use `~s` when there's 4+ escaped double-quotes
(`"\"\"\"\""` -> `~s("""")`) (`Credo.Check.Readability.StringSigils`) #146

### Fixes

Expand Down
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ Disabling the rules means updating your `.credo.exs` depending on your configura
{Credo.Check.Readability.SinglePipe, false},
# **potentially breaks compilation** - see **Troubleshooting** section below
{Credo.Check.Readability.StrictModuleLayout, false},
{Credo.Check.Readability.StringSigils, false},
{Credo.Check.Readability.UnnecessaryAliasExpansion, false},
{Credo.Check.Readability.WithSingleClause, false},
{Credo.Check.Refactor.CaseTrivialMatches, false},
Expand Down
47 changes: 43 additions & 4 deletions lib/style/single_node.ex
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ defmodule Styler.Style.SingleNode do
* Credo.Check.Readability.LargeNumbers
* Credo.Check.Readability.ParenthesesOnZeroArityDefs
* Credo.Check.Readability.PreferImplicitTry
* Credo.Check.Readability.StringSigils
* Credo.Check.Readability.WithSingleClause
* Credo.Check.Refactor.CaseTrivialMatches
* Credo.Check.Refactor.CondStatements
Expand All @@ -27,17 +28,55 @@ defmodule Styler.Style.SingleNode do

@behaviour Styler.Style

@closing_delimiters [~s|"|, ")", "}", "|", "]", "'", ">", "/"]

alias Styler.Zipper

def run({node, meta}, ctx), do: {:cont, {style(node), meta}, ctx}

# as of 1.15, elixir's formatter takes care of this for us.
if Version.match?(System.version(), "< 1.15.0-dev") do
# 'charlist' => ~c"charlist"
defp style({:__block__, meta, [chars]} = node) when is_list(chars) do
if meta[:delimiter] == "'",
do: {:sigil_c, Keyword.put(meta, :delimiter, "\""), [{:<<>>, [line: meta[:line]], [List.to_string(chars)]}, []]},
else: node
defp style({:__block__, [{:delimiter, ~s|'|} | meta], [chars]}) when is_list(chars),
do: {:sigil_c, [{:delimiter, ~s|"|} | meta], [{:<<>>, [line: meta[:line]], [List.to_string(chars)]}, []]}
end

# rewrite double-quote strings with >= 4 escaped double-quotes as sigils
defp style({:__block__, [{:delimiter, ~s|"|} | meta], [string]} = node) when is_binary(string) do
# running a regex against every double-quote delimited string literal in a codebase doesn't have too much impact
# on adobe's internal codebase, but perhaps other codebases have way more literals where this'd have an impact?
if string =~ ~r/".*".*".*"/ do
# choose whichever delimiter would require the least # of escapes,
# ties being broken by our stylish ordering of delimiters (reflected in the 1-8 values)
{closer, _} =
string
|> String.codepoints()
|> Stream.filter(&(&1 in @closing_delimiters))
|> Stream.concat(@closing_delimiters)
|> Enum.frequencies()
|> Enum.min_by(fn
{~s|"|, count} -> {count, 1}
{")", count} -> {count, 2}
{"}", count} -> {count, 3}
{"|", count} -> {count, 4}
{"]", count} -> {count, 5}
{"'", count} -> {count, 6}
{">", count} -> {count, 7}
{"/", count} -> {count, 8}
end)

delimiter =
case closer do
")" -> "("
"}" -> "{"
"]" -> "["
">" -> "<"
closer -> closer
end

{:sigil_s, [{:delimiter, delimiter} | meta], [{:<<>>, [line: meta[:line]], [string]}, []]}
else
node
end
end

Expand Down
16 changes: 16 additions & 0 deletions test/style/single_node_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,22 @@ defmodule Styler.Style.SingleNodeTest do
end
end

test "string sigil rewrites" do
assert_style ~s|""|
assert_style ~s|"\\""|
assert_style ~s|"\\"\\""|
assert_style ~s|"\\"\\"\\""|
assert_style ~s|"\\"\\"\\"\\""|, ~s|~s("""")|
# choose closing delimiter wisely, based on what has the least conflicts, in the styliest order
assert_style ~s/"\\"\\"\\"\\" )"/, ~s/~s{"""" )}/
assert_style ~s/"\\"\\"\\"\\" })"/, ~s/~s|"""" })|/
assert_style ~s/"\\"\\"\\"\\" |})"/, ~s/~s["""" |})]/
assert_style ~s/"\\"\\"\\"\\" ]|})"/, ~s/~s'"""" ]|})'/
assert_style ~s/"\\"\\"\\"\\" ']|})"/, ~s/~s<"""" ']|})>/
assert_style ~s/"\\"\\"\\"\\" >']|})"/, ~s|~s/"""" >']\|})/|
assert_style ~s/"\\"\\"\\"\\" \/>']|})"/, ~s|~s("""" />']\|}\\))|
end

test "{Map/Keyword}.merge with a single static key" do
for module <- ~w(Map Keyword) do
assert_style("#{module}.merge(foo, %{one_key: :bar})", "#{module}.put(foo, :one_key, :bar)")
Expand Down

0 comments on commit c29b841

Please sign in to comment.