Skip to content

Commit e781dbb

Browse files
committed
Merge branch 'comparison-against-1' of github.com:hauleth/credo into 1226-empty-enum
2 parents c20779e + d876c73 commit e781dbb

File tree

2 files changed

+108
-14
lines changed

2 files changed

+108
-14
lines changed

lib/credo/check/warning/expensive_empty_enum_check.ex

Lines changed: 40 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -40,17 +40,43 @@ defmodule Credo.Check.Warning.ExpensiveEmptyEnumCheck do
4040
}
4141
@length_pattern quote do: {:length, _, [_]}
4242
@comparisons [
43-
{@enum_count_pattern, 0, "Enum.count"},
44-
{0, @enum_count_pattern, "Enum.count"},
45-
{@length_pattern, 0, "length"},
46-
{0, @length_pattern, "length"}
43+
{@enum_count_pattern, "Enum.count"},
44+
{@length_pattern, "length"}
4745
]
48-
@operators [:==, :!=, :===, :!==, :>, :<]
46+
@eq_operators [:==, :!=, :===, :!==, :>, :<]
4947

50-
for {lhs, rhs, trigger} <- @comparisons,
51-
operator <- @operators do
48+
# Comparisons against 0
49+
for {pattern, trigger} <- @comparisons,
50+
operator <- @eq_operators do
5251
defp traverse(
53-
{unquote(operator), _meta, [unquote(lhs), unquote(rhs)]} = ast,
52+
{unquote(operator), _meta, [unquote(pattern), 0]} = ast,
53+
issues,
54+
issue_meta
55+
) do
56+
{ast, issues_for_call(unquote(trigger), issues, issue_meta, ast)}
57+
end
58+
59+
defp traverse(
60+
{unquote(operator), _meta, [0, unquote(pattern)]} = ast,
61+
issues,
62+
issue_meta
63+
) do
64+
{ast, issues_for_call(unquote(trigger), issues, issue_meta, ast)}
65+
end
66+
end
67+
68+
# Comparisons against 1
69+
for {pattern, trigger} <- @comparisons do
70+
defp traverse(
71+
{:>=, _meta, [unquote(pattern), 1]} = ast,
72+
issues,
73+
issue_meta
74+
) do
75+
{ast, issues_for_call(unquote(trigger), issues, issue_meta, ast)}
76+
end
77+
78+
defp traverse(
79+
{:<=, _meta, [1, unquote(pattern)]} = ast,
5480
issues,
5581
issue_meta
5682
) do
@@ -67,13 +93,13 @@ defmodule Credo.Check.Warning.ExpensiveEmptyEnumCheck do
6793
[issue_for(issue_meta, meta, trigger, suggest(ast)) | issues]
6894
end
6995

70-
defp suggest({_op, _, [0, {_pattern, _, args}]}), do: suggest_for_arity(Enum.count(args))
71-
defp suggest({_op, _, [{_pattern, _, args}, 0]}), do: suggest_for_arity(Enum.count(args))
96+
defp suggest({_op, _, [_, {_pattern, _, args}]}), do: suggest_for_arity(Enum.count(args))
97+
defp suggest({_op, _, [{_pattern, _, args}, _]}), do: suggest_for_arity(Enum.count(args))
7298

73-
defp get_meta({_op, _, [0, {{:., _, [{:__aliases__, meta, _}, _]}, _, _}]}), do: meta
74-
defp get_meta({_op, _, [0, {_, meta, _}]}), do: meta
75-
defp get_meta({_op, _, [{{:., _, [{:__aliases__, meta, _}, _]}, _, _}, 0]}), do: meta
76-
defp get_meta({_op, _, [{_, meta, _}, 0]}), do: meta
99+
defp get_meta({_op, _, [_, {{:., _, [{:__aliases__, meta, _}, _]}, _, _}]}), do: meta
100+
defp get_meta({_op, _, [_, {_, meta, _}]}), do: meta
101+
defp get_meta({_op, _, [{{:., _, [{:__aliases__, meta, _}, _]}, _, _}, _]}), do: meta
102+
defp get_meta({_op, _, [{_, meta, _}, _]}), do: meta
77103

78104
defp suggest_for_arity(2), do: "`not Enum.any?/2`"
79105
defp suggest_for_arity(1), do: "`Enum.empty?/1` or `list == []`"

test/credo/check/warning/expensive_empty_enum_check_test.exs

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -438,6 +438,74 @@ defmodule Credo.Check.Warning.ExpensiveEmptyEnumCheckTest do
438438
|> assert_issue()
439439
end
440440

441+
test "it should report when checking if length is greater or equal to 1" do
442+
"""
443+
defmodule CredoSampleModule do
444+
def some_function(some_list) do
445+
if length(some_list) >= 1 do
446+
"not empty"
447+
else
448+
"empty"
449+
end
450+
end
451+
end
452+
"""
453+
|> to_source_file
454+
|> run_check(@described_check)
455+
|> assert_issue()
456+
end
457+
458+
test "it should report when checking if length is greater or equal to 1 backwards" do
459+
"""
460+
defmodule CredoSampleModule do
461+
def some_function(some_list) do
462+
if 1 <= length(some_list) do
463+
"not empty"
464+
else
465+
"empty"
466+
end
467+
end
468+
end
469+
"""
470+
|> to_source_file
471+
|> run_check(@described_check)
472+
|> assert_issue()
473+
end
474+
475+
test "it should report when checking if Enum.count is greater or equal to 1" do
476+
"""
477+
defmodule CredoSampleModule do
478+
def some_function(some_list) do
479+
if Enum.count(some_list) >= 1 do
480+
"not empty"
481+
else
482+
"empty"
483+
end
484+
end
485+
end
486+
"""
487+
|> to_source_file
488+
|> run_check(@described_check)
489+
|> assert_issue()
490+
end
491+
492+
test "it should report when checking if Enum.count is greater or equal to 1 backwards" do
493+
"""
494+
defmodule CredoSampleModule do
495+
def some_function(some_list) do
496+
if 1 <= Enum.count(some_list) do
497+
"not empty"
498+
else
499+
"empty"
500+
end
501+
end
502+
end
503+
"""
504+
|> to_source_file
505+
|> run_check(@described_check)
506+
|> assert_issue()
507+
end
508+
441509
test "it should report when multiple issues (see #1184)" do
442510
"""
443511
defmodule Test do

0 commit comments

Comments
 (0)