From b804b11e38758177d761ec9c1034a10325df49dd Mon Sep 17 00:00:00 2001 From: Andrew Lechowicz Date: Tue, 11 Oct 2022 10:39:55 -0400 Subject: [PATCH 1/4] Add case-sensitive ordering option for AliasOrder --- lib/credo/check/readability/alias_order.ex | 36 +++++++++++++------ .../check/readability/alias_order_test.exs | 24 +++++++++++++ 2 files changed, 50 insertions(+), 10 deletions(-) diff --git a/lib/credo/check/readability/alias_order.ex b/lib/credo/check/readability/alias_order.ex index e267156a3..4e246d3cc 100644 --- a/lib/credo/check/readability/alias_order.ex +++ b/lib/credo/check/readability/alias_order.ex @@ -1,6 +1,9 @@ defmodule Credo.Check.Readability.AliasOrder do use Credo.Check, base_priority: :low, + param_defaults: [ + sort_method: :alpha + ], explanations: [ check: """ Alphabetically ordered lists are more easily scannable by the read. @@ -38,7 +41,16 @@ 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 + - `:aplha` - Alphabetical case-insensitive sorting. + - `:ascii` - Case-sensitive sorting where capitals are ordered earier than their lower case equivilants. + """ + ] ] alias Credo.Code.Name @@ -46,26 +58,27 @@ defmodule Credo.Check.Readability.AliasOrder do @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 @@ -75,7 +88,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 @@ -88,7 +104,7 @@ 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(_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) -> @@ -108,7 +124,7 @@ defmodule Credo.Check.Readability.AliasOrder do end end - defp process_group([{line_no1, mod_list_first, _}], _) do + defp process_group(_sort_method, [{line_no1, mod_list_first, _}]) do if issue_opts = inner_group_order_issue(line_no1, mod_list_first) do {:halt, issue_opts} else diff --git a/test/credo/check/readability/alias_order_test.exs b/test/credo/check/readability/alias_order_test.exs index 62793ae41..ad3643059 100644 --- a/test/credo/check/readability/alias_order_test.exs +++ b/test/credo/check/readability/alias_order_test.exs @@ -118,6 +118,18 @@ 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 + # # cases raising issues # @@ -228,4 +240,16 @@ defmodule Credo.Check.Readability.AliasOrderTest do assert issue.trigger == "Sorter" end) end + + test "it should repot 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) + |> refute_issues() + end end From 569daf90e2d9619c12c4239082f614964088ad3b Mon Sep 17 00:00:00 2001 From: Ky Bishop Date: Tue, 8 Aug 2023 15:30:18 -0400 Subject: [PATCH 2/4] Fix case-sensitive ordering for AliasOrder --- lib/credo/check/readability/alias_order.ex | 12 ++++++++++-- .../check/readability/alias_order_test.exs | 19 +++++++++++++++++-- 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/lib/credo/check/readability/alias_order.ex b/lib/credo/check/readability/alias_order.ex index 4e246d3cc..a0262849c 100644 --- a/lib/credo/check/readability/alias_order.ex +++ b/lib/credo/check/readability/alias_order.ex @@ -47,8 +47,8 @@ defmodule Credo.Check.Readability.AliasOrder do The ordering method to use. Options - - `:aplha` - Alphabetical case-insensitive sorting. - - `:ascii` - Case-sensitive sorting where capitals are ordered earier than their lower case equivilants. + - `:alpha` - Alphabetical case-insensitive sorting. + - `:ascii` - Case-sensitive sorting where upper case characters are ordered before their lower case equivalent. (Does not support alias groups) """ ] ] @@ -104,6 +104,14 @@ defmodule Credo.Check.Readability.AliasOrder do {:halt, issue_opts} end + 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 diff --git a/test/credo/check/readability/alias_order_test.exs b/test/credo/check/readability/alias_order_test.exs index ad3643059..36330beab 100644 --- a/test/credo/check/readability/alias_order_test.exs +++ b/test/credo/check/readability/alias_order_test.exs @@ -241,7 +241,7 @@ defmodule Credo.Check.Readability.AliasOrderTest do end) end - test "it should repot violation with case-sensitive sorting" do + test "it should report a violation with case-sensitive sorting" do """ defmodule Test do alias MyApp.AlphaBravoalpha @@ -250,6 +250,21 @@ defmodule Credo.Check.Readability.AliasOrderTest do """ |> to_source_file |> run_check(@described_check, sort_method: :ascii) - |> refute_issues() + |> 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 From 8af12ed39a0d3be89d02a1fb84e0222ca098ff46 Mon Sep 17 00:00:00 2001 From: Ky Bishop Date: Tue, 8 Aug 2023 16:13:53 -0400 Subject: [PATCH 3/4] Alias group support for ascii sorting --- lib/credo/check/readability/alias_order.ex | 26 +++++++++----- .../check/readability/alias_order_test.exs | 35 ++++++++++++------- 2 files changed, 40 insertions(+), 21 deletions(-) diff --git a/lib/credo/check/readability/alias_order.ex b/lib/credo/check/readability/alias_order.ex index a0262849c..bc5871386 100644 --- a/lib/credo/check/readability/alias_order.ex +++ b/lib/credo/check/readability/alias_order.ex @@ -112,13 +112,13 @@ defmodule Credo.Check.Readability.AliasOrder 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 + 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 -> @@ -132,8 +132,8 @@ defmodule Credo.Check.Readability.AliasOrder do end end - defp process_group(_sort_method, [{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} @@ -142,9 +142,9 @@ 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(line_no, {base, mod_list}) do + defp inner_group_order_issue(:alpha = _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) @@ -153,9 +153,17 @@ 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 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 issue_opts(line_no, base, mod_list, trigger_source_mod_list, sorted_downcased_mod_list) do trigger = - downcased_mod_list + trigger_source_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 diff --git a/test/credo/check/readability/alias_order_test.exs b/test/credo/check/readability/alias_order_test.exs index 36330beab..7128cd8a2 100644 --- a/test/credo/check/readability/alias_order_test.exs +++ b/test/credo/check/readability/alias_order_test.exs @@ -130,6 +130,17 @@ defmodule Credo.Check.Readability.AliasOrderTest do |> 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 # @@ -255,16 +266,16 @@ defmodule Credo.Check.Readability.AliasOrderTest do 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 + 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 From 907bb3bd4dabf869c793d405b615aceca3e78cf7 Mon Sep 17 00:00:00 2001 From: Ky Bishop Date: Tue, 8 Aug 2023 16:15:20 -0400 Subject: [PATCH 4/4] Fix comment --- lib/credo/check/readability/alias_order.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/credo/check/readability/alias_order.ex b/lib/credo/check/readability/alias_order.ex index bc5871386..8e344eeaf 100644 --- a/lib/credo/check/readability/alias_order.ex +++ b/lib/credo/check/readability/alias_order.ex @@ -48,7 +48,7 @@ defmodule Credo.Check.Readability.AliasOrder do Options - `:alpha` - Alphabetical case-insensitive sorting. - - `:ascii` - Case-sensitive sorting where upper case characters are ordered before their lower case equivalent. (Does not support alias groups) + - `:ascii` - Case-sensitive sorting where upper case characters are ordered before their lower case equivalent. """ ] ]