Skip to content

Commit

Permalink
Fix non-concurrent index detection (#1)
Browse files Browse the repository at this point in the history
* Update ast parser to match any structure using index

* Fix false positive
  • Loading branch information
renanpvaz authored Oct 6, 2023
1 parent e9077f2 commit 6ac5fc3
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 10 deletions.
17 changes: 7 additions & 10 deletions lib/ast_parser.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand Down
4 changes: 4 additions & 0 deletions test/ast_parser_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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
20 changes: 20 additions & 0 deletions test/runner_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down

0 comments on commit 6ac5fc3

Please sign in to comment.