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 c2f0463
Show file tree
Hide file tree
Showing 4 changed files with 56 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
* rewrite double-quoted strings to use `~s` when there's a preponderance of escaped double-quotes
(`"string\"with\"four_or_more\"escapes\""` -> `~s(string"with"four_or_more"escapes")`) (`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
43 changes: 39 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 @@ -34,10 +35,44 @@ defmodule Styler.Style.SingleNode do
# 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]} = node) when is_list(chars),

Check warning on line 38 in lib/style/single_node.ex

View workflow job for this annotation

GitHub Actions / Ex1.14.2/OTP25.1.2

variable "node" is unused (if the variable is not meant to be used, prefix it with an underscore)
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 `"", (), {}, ||, [], <>` (reflected in the 0-5 values)
{closer, _} =
string
|> String.codepoints()
|> Stream.filter(&(&1 in [")", "]", "}", "|", ">", ~s|"|]))
|> Stream.concat([")", "]", "}", "|", ">", ~s|"|])
|> Enum.frequencies()
|> Enum.min_by(fn
{~s|"|, count} -> {count, 0}
{")", count} -> {count, 1}
{"}", count} -> {count, 2}
{"|", count} -> {count, 3}
{"]", count} -> {count, 4}
{">", count} -> {count, 5}
end)

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

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

Expand Down
14 changes: 14 additions & 0 deletions test/style/single_node_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,20 @@ 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("""" >]\|}\\))|
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 c2f0463

Please sign in to comment.