Fix heredoc parsing in Strings

Refs #924

lib/credo/code/strings.ex

@@ -103,18 +103,19 @@ defmodule Credo.Code.Strings do
       parse_heredoc(
         t,
         acc <> unquote(sigil_start),
+        "",
         replacement,
         unquote(sigil_end)
       )
     end
   end
 
   defp parse_code(<<"\"\"\""::utf8, t::binary>>, acc, replacement) do
-    parse_heredoc(t, acc <> ~s("""), replacement, ~s("""))
+    parse_heredoc(t, acc <> ~s("""), "", replacement, ~s("""))
   end
 
   defp parse_code(<<"\'\'\'"::utf8, t::binary>>, acc, replacement) do
-    parse_heredoc(t, acc <> ~s('''), replacement, ~s('''))
+    parse_heredoc(t, acc <> ~s('''), "", replacement, ~s('''))
   end
 
   for {sigil_start, sigil_end} <- all_string_sigils do
@@ -387,57 +388,71 @@ defmodule Credo.Code.Strings do
   # Heredocs
   #
 
-  defp parse_heredoc(<<"\"\"\""::utf8, t::binary>>, acc, "" = replacement, "\"\"\"") do
-    parse_code(t, acc <> "\"\"\"", replacement)
+  defp parse_heredoc(<<"\"\"\""::utf8, t::binary>>, acc, heredoc_acc, "" = replacement, "\"\"\"") do
+    parse_code(t, acc <> heredoc_acc <> "\"\"\"", replacement)
   end
 
-  defp parse_heredoc(<<"\"\"\""::utf8, t::binary>>, acc, " " = replacement, "\"\"\"") do
-    parse_code(t, acc <> "\"\"\"", replacement)
+  defp parse_heredoc(<<"\"\"\""::utf8, t::binary>>, acc, heredoc_acc, " " = replacement, "\"\"\"") do
+    parse_code(t, acc <> heredoc_acc <> "\"\"\"", replacement)
   end
 
-  defp parse_heredoc(<<"\"\"\""::utf8, t::binary>>, acc, replacement, "\"\"\"") do
-    acc =
-      Regex.replace(~r/(\n[#{replacement}]+)(\"\"\")\z/m, acc <> "\"\"\"", fn _, x ->
-        "\n#{String.pad_trailing("", String.length(x))}\"\"\""
-      end)
+  defp parse_heredoc(<<"\"\"\""::utf8, t::binary>>, acc, heredoc_acc, replacement, "\"\"\"") do
+    heredoc_acc = heredoc_acc <> "\"\"\""
+
+    heredoc_acc =
+      case Regex.run(~r/\n([#{replacement}]+)\"\"\"\z/m, heredoc_acc) do
+        [_, indent_string] ->
+          x = String.length(indent_string)
+          Regex.replace(~r/^(.{#{x}})/m, heredoc_acc, String.pad_trailing("", x))
+
+        _ ->
+          heredoc_acc
+      end
 
-    parse_code(t, acc, replacement)
+    parse_code(t, acc <> heredoc_acc, replacement)
   end
 
-  defp parse_heredoc(<<"\'\'\'"::utf8, t::binary>>, acc, "" = replacement, "\'\'\'") do
-    parse_code(t, acc, replacement)
+  defp parse_heredoc(<<"\'\'\'"::utf8, t::binary>>, acc, heredoc_acc, "" = replacement, "\'\'\'") do
+    parse_code(t, acc <> heredoc_acc <> "\'\'\'", replacement)
   end
 
-  defp parse_heredoc(<<"\'\'\'"::utf8, t::binary>>, acc, " " = replacement, "\'\'\'") do
-    parse_code(t, acc <> "\'\'\'", replacement)
+  defp parse_heredoc(<<"\'\'\'"::utf8, t::binary>>, acc, heredoc_acc, " " = replacement, "\'\'\'") do
+    parse_code(t, acc <> heredoc_acc <> "\'\'\'", replacement)
   end
 
-  defp parse_heredoc(<<"\'\'\'"::utf8, t::binary>>, acc, replacement, "\'\'\'") do
-    acc =
-      Regex.replace(~r/(\n[#{replacement}]+)(\'\'\')\z/m, acc <> "\'\'\'", fn _, x ->
-        "\n#{String.pad_trailing("", String.length(x))}\'\'\'"
-      end)
+  defp parse_heredoc(<<"\'\'\'"::utf8, t::binary>>, acc, heredoc_acc, replacement, "\'\'\'") do
+    heredoc_acc = heredoc_acc <> "\'\'\'"
+
+    heredoc_acc =
+      case Regex.run(~r/\n([#{replacement}]+)\'\'\'\z/m, heredoc_acc) do
+        [_, indent_string] ->
+          x = String.length(indent_string)
+          Regex.replace(~r/^(.{#{x}})/m, heredoc_acc, String.pad_trailing("", x))
+
+        _ ->
+          heredoc_acc
+      end
 
-    parse_code(t, acc, replacement)
+    parse_code(t, acc <> heredoc_acc, replacement)
   end
 
-  defp parse_heredoc("", acc, _replacement, _delimiter) do
+  defp parse_heredoc("", acc, _heredoc_acc, _replacement, _delimiter) do
     acc
   end
 
-  defp parse_heredoc(<<"\\\\"::utf8, t::binary>>, acc, replacement, delimiter) do
-    parse_heredoc(t, acc, replacement, delimiter)
+  defp parse_heredoc(<<"\\\\"::utf8, t::binary>>, acc, heredoc_acc, replacement, delimiter) do
+    parse_heredoc(t, acc, heredoc_acc, replacement, delimiter)
   end
 
-  defp parse_heredoc(<<"\\\""::utf8, t::binary>>, acc, replacement, delimiter) do
-    parse_heredoc(t, acc, replacement, delimiter)
+  defp parse_heredoc(<<"\\\""::utf8, t::binary>>, acc, heredoc_acc, replacement, delimiter) do
+    parse_heredoc(t, acc, heredoc_acc, replacement, delimiter)
   end
 
-  defp parse_heredoc(<<"\n"::utf8, t::binary>>, acc, replacement, delimiter) do
-    parse_heredoc(t, acc <> "\n", replacement, delimiter)
+  defp parse_heredoc(<<"\n"::utf8, t::binary>>, acc, heredoc_acc, replacement, delimiter) do
+    parse_heredoc(t, acc, heredoc_acc <> "\n", replacement, delimiter)
   end
 
-  defp parse_heredoc(<<_::utf8, t::binary>>, acc, replacement, delimiter) do
-    parse_heredoc(t, acc <> replacement, replacement, delimiter)
+  defp parse_heredoc(<<_::utf8, t::binary>>, acc, heredoc_acc, replacement, delimiter) do
+    parse_heredoc(t, acc, heredoc_acc <> replacement, replacement, delimiter)
   end
 end

test/credo/code/strings_test.exs

@@ -246,6 +246,24 @@ defmodule Credo.Code.StringsTest do
     assert result == result2, "Strings.replace_with_spaces/2 should be idempotent"
   end
 
+  @tag slow: :disk_io
+  test "it should produce valid code /6" do
+    example_code = File.read!("test/fixtures/example_code/browser2.ex")
+
+    result =
+      example_code
+      |> to_source_file()
+      |> Strings.replace_with_spaces(".", ".")
+
+    result2 =
+      result
+      |> Strings.replace_with_spaces(".", ".")
+
+    assert match?({:ok, _}, Code.string_to_quoted(result))
+
+    assert result == result2, "Strings.replace_with_spaces/2 should be idempotent"
+  end
+
   @tag slow: :disk_io
   test "it should produce valid code with empty string as replacement /5" do
     example_code = File.read!("test/fixtures/example_code/browser.ex")
@@ -263,4 +281,22 @@ defmodule Credo.Code.StringsTest do
 
     assert result == result2, "Strings.replace_with_spaces/2 should be idempotent"
   end
+
+  @tag slow: :disk_io
+  test "it should produce valid code with empty string as replacement /6" do
+    example_code = File.read!("test/fixtures/example_code/browser2.ex")
+
+    result =
+      example_code
+      |> to_source_file()
+      |> Strings.replace_with_spaces("", "")
+
+    result2 =
+      result
+      |> Strings.replace_with_spaces("", "")
+
+    assert match?({:ok, _}, Code.string_to_quoted(result))
+
+    assert result == result2, "Strings.replace_with_spaces/2 should be idempotent"
+  end
 end

test/fixtures/example_code/browser.ex

@@ -1,5 +1,5 @@
 defmodule Wallaby.Browser do
-  @moduledoc """
+  @moduledoc ~S"""
   The Browser module is the entrypoint for interacting with a real browser.
 
   By default, action only work with elements that are visible to a real user.