From 6ac5fc386c874a78feb4fd0f8366b586a1abf0f5 Mon Sep 17 00:00:00 2001
From: Renan Vaz <renan.piresvz@gmail.com>
Date: Fri, 6 Oct 2023 17:17:15 -0300
Subject: [PATCH] Fix non-concurrent index detection (#1)

* Update ast parser to match any structure using index

* Fix false positive
---
 lib/ast_parser.ex                             | 17 +++++++---------
 test/ast_parser_test.exs                      |  4 ++++
 ..._create_index_unconventional_structure.exs |  8 ++++++++
 test/runner_test.exs                          | 20 +++++++++++++++++++
 4 files changed, 39 insertions(+), 10 deletions(-)
 create mode 100644 test/example_migrations/20230922211058_create_index_unconventional_structure.exs

diff --git a/lib/ast_parser.ex b/lib/ast_parser.ex
index 9cf4fc4..b9c3b45 100644
--- a/lib/ast_parser.ex
+++ b/lib/ast_parser.ex
@@ -2,7 +2,6 @@ defmodule ExcellentMigrations.AstParser do
   @moduledoc false
   @max_columns_for_index 3
 
-  @index_functions [:create, :create_if_not_exists, :drop, :drop_if_exists]
   @index_types [:index, :unique_index]
 
   def parse(ast) do
@@ -37,16 +36,14 @@ defmodule ExcellentMigrations.AstParser do
       detect_json_column_added(code_part)
   end
 
-  defp detect_index_not_concurrently({fun_name, location, [{operation, _, [_, _]}]})
-       when fun_name in @index_functions and operation in @index_types do
-    [{:index_not_concurrently, Keyword.get(location, :line)}]
-  end
+  defp detect_index_not_concurrently({operation, location, args})
+       when is_list(args) and operation in @index_types do
+    options = List.last(args, [])
 
-  defp detect_index_not_concurrently({fun_name, location, [{operation, _, [_, _, options]}]})
-       when fun_name in @index_functions and operation in @index_types do
-    case Keyword.get(options, :concurrently) do
-      true -> []
-      _ -> [{:index_not_concurrently, Keyword.get(location, :line)}]
+    if is_list(options) and Keyword.get(options, :concurrently) do
+      []
+    else
+      [{:index_not_concurrently, Keyword.get(location, :line)}]
     end
   end
 
diff --git a/test/ast_parser_test.exs b/test/ast_parser_test.exs
index be37639..ae38887 100644
--- a/test/ast_parser_test.exs
+++ b/test/ast_parser_test.exs
@@ -128,14 +128,18 @@ defmodule ExcellentMigrations.AstParserTest do
     ast_multi = string_to_ast("create index(:recipes, [:cuisine])")
     ast_multi_with_opts = string_to_ast("create index(:recipes, [:cuisine], unique: true)")
     ast_conc_false = string_to_ast("create index(:recipes, [:cuisine], concurrently: false)")
+    ast_pipeline = string_to_ast("index(:recipes, [:cuisine]) |> create()")
     ast_conc_true = string_to_ast("create index(:recipes, [:cuisine], concurrently: true)")
+    ast_invalid = string_to_ast("fn {index, id} -> create(index, id) end")
 
     assert [index_not_concurrently: 1] == AstParser.parse(ast_single)
     assert [index_not_concurrently: 1] == AstParser.parse(ast_single_with_opts)
     assert [index_not_concurrently: 1] == AstParser.parse(ast_multi)
     assert [index_not_concurrently: 1] == AstParser.parse(ast_multi_with_opts)
     assert [index_not_concurrently: 1] == AstParser.parse(ast_conc_false)
+    assert [index_not_concurrently: 1] == AstParser.parse(ast_pipeline)
     assert [] == AstParser.parse(ast_conc_true)
+    assert [] == AstParser.parse(ast_invalid)
   end
 
   test "detects index added not concurrently using if not exists" do
diff --git a/test/example_migrations/20230922211058_create_index_unconventional_structure.exs b/test/example_migrations/20230922211058_create_index_unconventional_structure.exs
new file mode 100644
index 0000000..d3101d9
--- /dev/null
+++ b/test/example_migrations/20230922211058_create_index_unconventional_structure.exs
@@ -0,0 +1,8 @@
+defmodule ExcellentMigrations.CreateIndexUnconventionalStructure do
+  def change do
+    dumplings_index = index(:dumplings, [:dough])
+    create(dumplings_index)
+
+    :dumplings |> index([:dough]) |> create()
+  end
+end
diff --git a/test/runner_test.exs b/test/runner_test.exs
index b67a066..d716335 100644
--- a/test/runner_test.exs
+++ b/test/runner_test.exs
@@ -138,6 +138,26 @@ defmodule ExcellentMigrations.RunnerTest do
            } == Runner.check_migrations(migrations_paths: file_paths)
   end
 
+  test "finds non-concurrent indices in unconventional structures" do
+    file_path = "test/example_migrations/20230922211058_create_index_unconventional_structure.exs"
+
+    assert {
+             :dangerous,
+             [
+               %{
+                 line: 3,
+                 path: file_path,
+                 type: :index_not_concurrently
+               },
+               %{
+                 line: 6,
+                 path: file_path,
+                 type: :index_not_concurrently
+               }
+             ]
+           } == Runner.check_migrations(migrations_paths: [file_path])
+  end
+
   test "no dangerous operations" do
     file_paths = [
       "test/example_migrations/20191026103003_create_table.exs",