Skip to content

Commit

Permalink
Merge branch 'kybishop-case-sensitive-alias-order'
Browse files Browse the repository at this point in the history
  • Loading branch information
rrrene committed Jan 2, 2024
2 parents 1f3b49a + a1b6609 commit bb43348
Show file tree
Hide file tree
Showing 2 changed files with 101 additions and 19 deletions.
70 changes: 51 additions & 19 deletions lib/credo/check/readability/alias_order.ex
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@ defmodule Credo.Check.Readability.AliasOrder do
use Credo.Check,
id: "EX3002",
base_priority: :low,
param_defaults: [
sort_method: :alpha
],
explanations: [
check: """
Alphabetically ordered lists are more easily scannable by the reader.
Expand Down Expand Up @@ -39,34 +42,44 @@ defmodule Credo.Check.Readability.AliasOrder do
Like all `Readability` issues, this one is not a technical concern.
But you can improve the odds of others reading and liking your code by making
it easier to follow.
"""
""",
params: [
sort_method: """
The ordering method to use.
Options
- `:alpha` - Alphabetical case-insensitive sorting.
- `:ascii` - Case-sensitive sorting where upper case characters are ordered before their lower case equivalent.
"""
]
]

alias Credo.Code.Name

@doc false
@impl true
def run(%SourceFile{} = source_file, params) do
sort_method = Params.get(params, :sort_method, __MODULE__)
issue_meta = IssueMeta.for(source_file, params)

Credo.Code.prewalk(source_file, &traverse(&1, &2, issue_meta))
Credo.Code.prewalk(source_file, &traverse(&1, &2, issue_meta, sort_method))
end

defp traverse({:defmodule, _, _} = ast, issues, issue_meta) do
defp traverse({:defmodule, _, _} = ast, issues, issue_meta, sort_method) do
new_issues =
ast
|> extract_alias_groups()
|> Enum.reduce([], &traverse_groups(&1, &2, issue_meta))
|> Enum.reduce([], &traverse_groups(&1, &2, issue_meta, sort_method))

{ast, issues ++ new_issues}
end

defp traverse(ast, issues, _), do: {ast, issues}
defp traverse(ast, issues, _, _), do: {ast, issues}

defp traverse_groups(group, acc, issue_meta) do
defp traverse_groups(group, acc, issue_meta, sort_method) do
group
|> Enum.chunk_every(2, 1)
|> Enum.reduce_while(nil, &process_group/2)
|> Enum.reduce_while(nil, fn chunk, _ -> process_group(sort_method, chunk) end)
|> case do
nil ->
acc
Expand All @@ -76,7 +89,10 @@ defmodule Credo.Check.Readability.AliasOrder do
end
end

defp process_group([{line_no, mod_list_second, a}, {_line_no, _mod_list_second, b}], _)
defp process_group(:alpha, [
{line_no, mod_list_second, a},
{_line_no, _mod_list_second, b}
])
when a > b do
module =
case mod_list_second do
Expand All @@ -89,13 +105,21 @@ defmodule Credo.Check.Readability.AliasOrder do
{:halt, issue_opts}
end

defp process_group([{line_no1, mod_list_first, _}, {line_no2, mod_list_second, _}], _) do
defp process_group(:ascii, [
{line_no, {a, []}, _},
{_line_no, {b, []}, _}
])
when a > b do
{:halt, issue_opts(line_no, a, a)}
end

defp process_group(sort_method, [{line_no1, mod_list_first, _}, {line_no2, mod_list_second, _}]) do
issue_opts =
cond do
issue = inner_group_order_issue(line_no1, mod_list_first) ->
issue = inner_group_order_issue(sort_method, line_no1, mod_list_first) ->
issue

issue = inner_group_order_issue(line_no2, mod_list_second) ->
issue = inner_group_order_issue(sort_method, line_no2, mod_list_second) ->
issue

true ->
Expand All @@ -109,8 +133,8 @@ defmodule Credo.Check.Readability.AliasOrder do
end
end

defp process_group([{line_no1, mod_list_first, _}], _) do
if issue_opts = inner_group_order_issue(line_no1, mod_list_first) do
defp process_group(sort_method, [{line_no1, mod_list_first, _}]) do
if issue_opts = inner_group_order_issue(sort_method, line_no1, mod_list_first) do
{:halt, issue_opts}
else
{:cont, nil}
Expand All @@ -119,9 +143,17 @@ defmodule Credo.Check.Readability.AliasOrder do

defp process_group(_, _), do: {:cont, nil}

defp inner_group_order_issue(_line_no, {_base, []}), do: nil
defp inner_group_order_issue(_sort_method, _line_no, {_base, []}), do: nil

defp inner_group_order_issue(:ascii = _sort_method, line_no, {base, mod_list}) do
sorted_mod_list = Enum.sort(mod_list)

if mod_list != sorted_mod_list do
issue_opts(line_no, base, mod_list, mod_list, sorted_mod_list)
end
end

defp inner_group_order_issue(line_no, {base, mod_list}) do
defp inner_group_order_issue(_sort_method, line_no, {base, mod_list}) do
downcased_mod_list = Enum.map(mod_list, &String.downcase(to_string(&1)))
sorted_downcased_mod_list = Enum.sort(downcased_mod_list)

Expand All @@ -130,12 +162,12 @@ defmodule Credo.Check.Readability.AliasOrder do
end
end

defp issue_opts(line_no, base, mod_list, downcased_mod_list, sorted_downcased_mod_list) do
defp issue_opts(line_no, base, mod_list, comparison_mod_list, sorted_comparison_mod_list) do
trigger =
downcased_mod_list
comparison_mod_list
|> Enum.with_index()
|> Enum.find_value(fn {downcased_mod_entry, index} ->
if downcased_mod_entry != Enum.at(sorted_downcased_mod_list, index) do
|> Enum.find_value(fn {comparison_mod_entry, index} ->
if comparison_mod_entry != Enum.at(sorted_comparison_mod_list, index) do
Enum.at(mod_list, index)
end
end)
Expand Down
50 changes: 50 additions & 0 deletions test/credo/check/readability/alias_order_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,29 @@ defmodule Credo.Check.Readability.AliasOrderTest do
|> refute_issues()
end

test "it should work with case-sensitive sorting" do
"""
defmodule Test do
alias MyApp.AlphaBravoCharlie
alias MyApp.AlphaBravoalpha
end
"""
|> to_source_file
|> run_check(@described_check, sort_method: :ascii)
|> refute_issues()
end

test "it should work with case-sensitive alias grouping" do
"""
defmodule Test do
alias MyApp.{AlphaBravoCharlie, AlphaBravoalpha}
end
"""
|> to_source_file
|> run_check(@described_check, sort_method: :ascii)
|> refute_issues()
end

#
# cases raising issues
#
Expand Down Expand Up @@ -228,4 +251,31 @@ defmodule Credo.Check.Readability.AliasOrderTest do
assert issue.trigger == "Sorter"
end)
end

test "it should report a violation with case-sensitive sorting" do
"""
defmodule Test do
alias MyApp.AlphaBravoalpha
alias MyApp.AlphaBravoCharlie
end
"""
|> to_source_file
|> run_check(@described_check, sort_method: :ascii)
|> assert_issue(fn issue ->
assert issue.trigger == "MyApp.AlphaBravoalpha"
end)
end

test "it should report a violation with case-sensitive sorting in a multi-alias" do
"""
defmodule Test do
alias MyApp.{AlphaBravoalpha, AlphaBravoCharlie}
end
"""
|> to_source_file
|> run_check(@described_check, sort_method: :ascii)
|> assert_issue(fn issue ->
assert issue.trigger == "AlphaBravoalpha"
end)
end
end

0 comments on commit bb43348

Please sign in to comment.