From 1a28a237e6545610ce56e20dd8996464c393b83a Mon Sep 17 00:00:00 2001 From: Frerich Raabe Date: Wed, 6 Sep 2023 21:55:38 +0200 Subject: [PATCH 1/8] Add refactoring check suggesting to pass time unit to {Naive}DateTime.utc_now Instead of calling DateTime.utc_now and then truncating the result with DateTime.truncate/2, it's more efficient (and less code) to just pass a time unit indicating the desired precision to DateTime.utc_now directly. Dito for NaiveDateTime. --- lib/credo/check/refactor/utc_now_truncate.ex | 186 ++++++++++++ .../check/refactor/utc_now_truncate_test.exs | 265 ++++++++++++++++++ 2 files changed, 451 insertions(+) create mode 100644 lib/credo/check/refactor/utc_now_truncate.ex create mode 100644 test/credo/check/refactor/utc_now_truncate_test.exs diff --git a/lib/credo/check/refactor/utc_now_truncate.ex b/lib/credo/check/refactor/utc_now_truncate.ex new file mode 100644 index 000000000..175fc97a6 --- /dev/null +++ b/lib/credo/check/refactor/utc_now_truncate.ex @@ -0,0 +1,186 @@ +defmodule Credo.Check.Refactor.UtcNowTruncate do + use Credo.Check, + id: "EX4032", + base_priority: :high, + explanations: [ + check: """ + Pass time unit to `{Naive}DateTime.utc_now` instead of composing with `{Naive}DateTime.truncate/2`. + + This should be refactored: + + DateTime.utc_now() |> DateTime.truncate(:second) + + to look like this: + + DateTime.utc_now(:second) + + Dito for `NaiveDateTime`. + + The reason for this is not just performance, because no separate function + call is required, but also brevity of the resulting code. + """ + ] + + @doc false + def run(source_file, params \\ []) do + issue_meta = IssueMeta.for(source_file, params) + + Credo.Code.prewalk(source_file, &traverse(&1, &2, issue_meta)) + end + + # DateTime.truncate(DateTime.utc_now(), _) + # DateTime.truncate(DateTime.utc_now(_), _) + # DateTime.truncate(DateTime.utc_now(_, _), _) + defp traverse( + {{:., meta, [{:__aliases__, _, [:DateTime]}, :truncate]}, _, + [ + {{:., _, [{:__aliases__, _, [:DateTime]}, :utc_now]}, _, _}, + _ + ]} = + ast, + issues, + issue_meta + ) do + new_issue = issue_for(issue_meta, meta[:line], "utc_now_truncate") + {ast, issues ++ List.wrap(new_issue)} + end + + # DateTime.utc_now() |> DateTime.truncate(_) + # DateTime.utc_now(_) |> DateTime.truncate(_) + # DateTime.utc_now(_, _) |> DateTime.truncate(_) + defp traverse( + {:|>, meta, + [ + {{:., _, [{:__aliases__, _, [:DateTime]}, :utc_now]}, _, _}, + {{:., _, [{:__aliases__, _, [:DateTime]}, :truncate]}, _, [_]} + ]} = ast, + issues, + issue_meta + ) do + new_issue = issue_for(issue_meta, meta[:line], "utc_now_truncate") + {ast, issues ++ List.wrap(new_issue)} + end + + # DateTime.truncate(_ |> DateTime.utc_now(), _) + # DateTime.truncate(_ |> DateTime.utc_now(_), _) + defp traverse( + {{:., meta, [{:__aliases__, _, [:DateTime]}, :truncate]}, _, + [ + {:|>, _, + [ + _, + {{:., _, [{:__aliases__, _, [:DateTime]}, :utc_now]}, _, _} + ]}, + _ + ]} = ast, + issues, + issue_meta + ) do + new_issue = issue_for(issue_meta, meta[:line], "utc_now_truncate") + {ast, issues ++ List.wrap(new_issue)} + end + + # _ |> DateTime.utc_now() |> DateTime.truncate(_) + # _ |> DateTime.utc_now(_) |> DateTime.truncate(_) + defp traverse( + {:|>, meta, + [ + {:|>, _, + [ + _, + {{:., _, [{:__aliases__, _, [:DateTime]}, :utc_now]}, _, _} + ]}, + {{:., _, [{:__aliases__, _, [:DateTime]}, :truncate]}, _, [_]} + ]} = ast, + issues, + issue_meta + ) do + new_issue = issue_for(issue_meta, meta[:line], "utc_now_truncate") + {ast, issues ++ List.wrap(new_issue)} + end + + # NaiveDateTime.truncate(NaiveDateTime.utc_now(), _) + # NaiveDateTime.truncate(NaiveDateTime.utc_now(_), _) + # NaiveDateTime.truncate(NaiveDateTime.utc_now(_, _), _) + defp traverse( + {{:., meta, [{:__aliases__, _, [:NaiveDateTime]}, :truncate]}, _, + [ + {{:., _, [{:__aliases__, _, [:NaiveDateTime]}, :utc_now]}, _, _}, + _ + ]} = + ast, + issues, + issue_meta + ) do + new_issue = issue_for(issue_meta, meta[:line], "utc_now_truncate") + {ast, issues ++ List.wrap(new_issue)} + end + + # NaiveDateTime.utc_now() |> NaiveDateTime.truncate(_) + # NaiveDateTime.utc_now(_) |> NaiveDateTime.truncate(_) + # NaiveDateTime.utc_now(_, _) |> NaiveDateTime.truncate(_) + defp traverse( + {:|>, meta, + [ + {{:., _, [{:__aliases__, _, [:NaiveDateTime]}, :utc_now]}, _, _}, + {{:., _, [{:__aliases__, _, [:NaiveDateTime]}, :truncate]}, _, [_]} + ]} = ast, + issues, + issue_meta + ) do + new_issue = issue_for(issue_meta, meta[:line], "utc_now_truncate") + {ast, issues ++ List.wrap(new_issue)} + end + + # NaiveDateTime.truncate(_ |> NaiveDateTime.utc_now(), _) + # NaiveDateTime.truncate(_ |> NaiveDateTime.utc_now(_), _) + defp traverse( + {{:., meta, [{:__aliases__, _, [:NaiveDateTime]}, :truncate]}, _, + [ + {:|>, _, + [ + _, + {{:., _, [{:__aliases__, _, [:NaiveDateTime]}, :utc_now]}, _, _} + ]}, + _ + ]} = ast, + issues, + issue_meta + ) do + new_issue = issue_for(issue_meta, meta[:line], "utc_now_truncate") + {ast, issues ++ List.wrap(new_issue)} + end + + # _ |> NaiveDateTime.utc_now() |> NaiveDateTime.truncate(_) + # _ |> NaiveDateTime.utc_now(_) |> NaiveDateTime.truncate(_) + defp traverse( + {:|>, meta, + [ + {:|>, _, + [ + _, + {{:., _, [{:__aliases__, _, [:NaiveDateTime]}, :utc_now]}, _, _} + ]}, + {{:., _, [{:__aliases__, _, [:NaiveDateTime]}, :truncate]}, _, [_]} + ]} = ast, + issues, + issue_meta + ) do + new_issue = issue_for(issue_meta, meta[:line], "utc_now_truncate") + {ast, issues ++ List.wrap(new_issue)} + end + + defp traverse(ast, issues, _issue_meta) do + {ast, issues} + end + + defp issue_for(issue_meta, line_no, trigger) do + format_issue( + issue_meta, + message: + "Pass time unit to `{Naive}DateTime.utc_now` instead of composing with `{Naive}DateTime.truncate/2`.", + trigger: trigger, + line_no: line_no + ) + end +end diff --git a/test/credo/check/refactor/utc_now_truncate_test.exs b/test/credo/check/refactor/utc_now_truncate_test.exs new file mode 100644 index 000000000..c1c7055cb --- /dev/null +++ b/test/credo/check/refactor/utc_now_truncate_test.exs @@ -0,0 +1,265 @@ +defmodule Credo.Check.Refactor.UtcNowTruncateTest do + use Credo.Test.Case + + @described_check Credo.Check.Refactor.UtcNowTruncate + + test "triggers when applying DateTime.truncate/2 to DateTime.utc_now/0" do + """ + defmodule M do + def f do + DateTime.truncate(DateTime.utc_now(), :second) + end + end + """ + |> to_source_file + |> run_check(@described_check) + |> assert_issue() + end + + test "triggers when applying DateTime.truncate/2 to DateTime.utc_now/1" do + """ + defmodule M do + def f do + DateTime.truncate(DateTime.utc_now(:second), :second) + end + end + """ + |> to_source_file + |> run_check(@described_check) + |> assert_issue() + end + + test "triggers when applying DateTime.truncate/2 to DateTime.utc_now/2" do + """ + defmodule M do + def f do + DateTime.truncate(DateTime.utc_now(Calendar.ISO, :second), :second) + end + end + """ + |> to_source_file + |> run_check(@described_check) + |> assert_issue() + end + + test "triggers when piping result of DateTime.utc_now/0 into DateTime.truncate/2" do + """ + defmodule M do + def f do + DateTime.utc_now() |> DateTime.truncate(:second) + end + end + """ + |> to_source_file + |> run_check(@described_check) + |> assert_issue() + end + + test "triggers when piping result of DateTime.utc_now/1 into DateTime.truncate/2" do + """ + defmodule M do + def f do + DateTime.utc_now(:second) |> DateTime.truncate(:second) + end + end + """ + |> to_source_file + |> run_check(@described_check) + |> assert_issue() + end + + test "triggers when piping result of DateTime.utc_now/2 into DateTime.truncate/2" do + """ + defmodule M do + def f do + DateTime.utc_now(Calendar.ISO, :second) |> DateTime.truncate(:second) + end + end + """ + |> to_source_file + |> run_check(@described_check) + |> assert_issue() + end + + test "triggers when piping argument to DateTime.utc_now/1 and applying DateTime.truncate/2 to that" do + """ + defmodule M do + def f do + DateTime.truncate(:second |> DateTime.utc_now(), :second) + end + end + """ + |> to_source_file + |> run_check(@described_check) + |> assert_issue() + end + + test "triggers when piping argument to DateTime.utc_now/2 and applying DateTime.truncate/2 to that" do + """ + defmodule M do + def f do + DateTime.truncate(Calendar.ISO |> DateTime.utc_now(:second), :second) + end + end + """ + |> to_source_file + |> run_check(@described_check) + |> assert_issue() + end + + test "triggers when piping argument to DateTime.utc_now/1 and piping result to DateTime.truncate/2" do + """ + defmodule M do + def f do + :second |> DateTime.utc_now() |> DateTime.truncate(:second) + end + end + """ + |> to_source_file + |> run_check(@described_check) + |> assert_issue() + end + + test "triggers when piping argument to DateTime.utc_now/2 and piping result to DateTime.truncate/2" do + """ + defmodule M do + def f do + Calendar.ISO |> DateTime.utc_now(:second) |> DateTime.truncate(:second) + end + end + """ + |> to_source_file + |> run_check(@described_check) + |> assert_issue() + end + + test "triggers when applying NaiveDateTime.truncate/2 to NaiveDateTime.utc_now/0" do + """ + defmodule M do + def f do + NaiveDateTime.truncate(NaiveDateTime.utc_now(), :second) + end + end + """ + |> to_source_file + |> run_check(@described_check) + |> assert_issue() + end + + test "triggers when applying NaiveDateTime.truncate/2 to NaiveDateTime.utc_now/1" do + """ + defmodule M do + def f do + NaiveDateTime.truncate(NaiveDateTime.utc_now(:second), :second) + end + end + """ + |> to_source_file + |> run_check(@described_check) + |> assert_issue() + end + + test "triggers when applying NaiveDateTime.truncate/2 to NaiveDateTime.utc_now/2" do + """ + defmodule M do + def f do + NaiveDateTime.truncate(NaiveDateTime.utc_now(Calendar.ISO, :second), :second) + end + end + """ + |> to_source_file + |> run_check(@described_check) + |> assert_issue() + end + + test "triggers when piping result of NaiveDateTime.utc_now/0 into NaiveDateTime.truncate/2" do + """ + defmodule M do + def f do + NaiveDateTime.utc_now() |> NaiveDateTime.truncate(:second) + end + end + """ + |> to_source_file + |> run_check(@described_check) + |> assert_issue() + end + + test "triggers when piping result of NaiveDateTime.utc_now/1 into NaiveDateTime.truncate/2" do + """ + defmodule M do + def f do + NaiveDateTime.utc_now(:second) |> NaiveDateTime.truncate(:second) + end + end + """ + |> to_source_file + |> run_check(@described_check) + |> assert_issue() + end + + test "triggers when piping result of NaiveDateTime.utc_now/2 into NaiveDateTime.truncate/2" do + """ + defmodule M do + def f do + NaiveDateTime.utc_now(Calendar.ISO, :second) |> NaiveDateTime.truncate(:second) + end + end + """ + |> to_source_file + |> run_check(@described_check) + |> assert_issue() + end + + test "triggers when piping argument to NaiveDateTime.utc_now/1 and applying NaiveDateTime.truncate/2 to that" do + """ + defmodule M do + def f do + NaiveDateTime.truncate(:second |> NaiveDateTime.utc_now(), :second) + end + end + """ + |> to_source_file + |> run_check(@described_check) + |> assert_issue() + end + + test "triggers when piping argument to NaiveDateTime.utc_now/2 and applying NaiveDateTime.truncate/2 to that" do + """ + defmodule M do + def f do + NaiveDateTime.truncate(Calendar.ISO |> NaiveDateTime.utc_now(:second), :second) + end + end + """ + |> to_source_file + |> run_check(@described_check) + |> assert_issue() + end + + test "triggers when piping argument to NaiveDateTime.utc_now/1 and piping result to NaiveDateTime.truncate/2" do + """ + defmodule M do + def f do + :second |> NaiveDateTime.utc_now() |> NaiveDateTime.truncate(:second) + end + end + """ + |> to_source_file + |> run_check(@described_check) + |> assert_issue() + end + + test "triggers when piping argument to NaiveDateTime.utc_now/2 and piping result to NaiveDateTime.truncate/2" do + """ + defmodule M do + def f do + Calendar.ISO |> NaiveDateTime.utc_now(:second) |> NaiveDateTime.truncate(:second) + end + end + """ + |> to_source_file + |> run_check(@described_check) + |> assert_issue() + end +end From bff16685750fbb2d50c3f5d69493efd457193247 Mon Sep 17 00:00:00 2001 From: Frerich Raabe Date: Wed, 6 Sep 2023 21:57:16 +0200 Subject: [PATCH 2/8] Enable Credo.Check.Refactor.UtcNowTruncate check by default I believe this refactoring is universally plausible, so let's enable it by default. --- .credo.exs | 1 + 1 file changed, 1 insertion(+) diff --git a/.credo.exs b/.credo.exs index 539d213ba..381b6b4ae 100644 --- a/.credo.exs +++ b/.credo.exs @@ -134,6 +134,7 @@ {Credo.Check.Refactor.RedundantWithClauseResult, []}, {Credo.Check.Refactor.RejectReject, []}, {Credo.Check.Refactor.UnlessWithElse, []}, + {Credo.Check.Refactor.UtcNowTruncate, []}, {Credo.Check.Refactor.WithClauses, []}, # From ef897030dff16c4421198c89fa5d599543784cd1 Mon Sep 17 00:00:00 2001 From: Frerich Raabe Date: Sat, 16 Dec 2023 12:46:25 +0100 Subject: [PATCH 3/8] Update lib/credo/check/refactor/utc_now_truncate.ex MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: René Föhring --- lib/credo/check/refactor/utc_now_truncate.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/credo/check/refactor/utc_now_truncate.ex b/lib/credo/check/refactor/utc_now_truncate.ex index 175fc97a6..d677ed4b0 100644 --- a/lib/credo/check/refactor/utc_now_truncate.ex +++ b/lib/credo/check/refactor/utc_now_truncate.ex @@ -4,7 +4,7 @@ defmodule Credo.Check.Refactor.UtcNowTruncate do base_priority: :high, explanations: [ check: """ - Pass time unit to `{Naive}DateTime.utc_now` instead of composing with `{Naive}DateTime.truncate/2`. + `DateTime.utc_now/1` is more efficient than `DateTime.utc_now/0 |> DateTime.truncate/1`. This should be refactored: From 46e9ca6796363abc7f94bfc56db6aae20418d319 Mon Sep 17 00:00:00 2001 From: Frerich Raabe Date: Sat, 16 Dec 2023 12:47:02 +0100 Subject: [PATCH 4/8] Update lib/credo/check/refactor/utc_now_truncate.ex MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: René Föhring --- lib/credo/check/refactor/utc_now_truncate.ex | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/credo/check/refactor/utc_now_truncate.ex b/lib/credo/check/refactor/utc_now_truncate.ex index d677ed4b0..1a7e0e045 100644 --- a/lib/credo/check/refactor/utc_now_truncate.ex +++ b/lib/credo/check/refactor/utc_now_truncate.ex @@ -6,15 +6,15 @@ defmodule Credo.Check.Refactor.UtcNowTruncate do check: """ `DateTime.utc_now/1` is more efficient than `DateTime.utc_now/0 |> DateTime.truncate/1`. - This should be refactored: + For example, the code here ... DateTime.utc_now() |> DateTime.truncate(:second) + NaiveDateTime.utc_now() |> NaiveDateTime.truncate(:second) - to look like this: + ... can be refactored to look like this: DateTime.utc_now(:second) - - Dito for `NaiveDateTime`. + NaiveDateTime.utc_now(:second) The reason for this is not just performance, because no separate function call is required, but also brevity of the resulting code. From 155b9d5f85a33872a9e5e817c50bbc7020008489 Mon Sep 17 00:00:00 2001 From: Frerich Raabe Date: Sat, 16 Dec 2023 12:51:19 +0100 Subject: [PATCH 5/8] More specific error messages --- lib/credo/check/refactor/utc_now_truncate.ex | 22 ++++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/lib/credo/check/refactor/utc_now_truncate.ex b/lib/credo/check/refactor/utc_now_truncate.ex index 1a7e0e045..0a79b1a78 100644 --- a/lib/credo/check/refactor/utc_now_truncate.ex +++ b/lib/credo/check/refactor/utc_now_truncate.ex @@ -41,7 +41,7 @@ defmodule Credo.Check.Refactor.UtcNowTruncate do issues, issue_meta ) do - new_issue = issue_for(issue_meta, meta[:line], "utc_now_truncate") + new_issue = issue_for(issue_meta, meta[:line], "DateTime") {ast, issues ++ List.wrap(new_issue)} end @@ -57,7 +57,7 @@ defmodule Credo.Check.Refactor.UtcNowTruncate do issues, issue_meta ) do - new_issue = issue_for(issue_meta, meta[:line], "utc_now_truncate") + new_issue = issue_for(issue_meta, meta[:line], "DateTime") {ast, issues ++ List.wrap(new_issue)} end @@ -76,7 +76,7 @@ defmodule Credo.Check.Refactor.UtcNowTruncate do issues, issue_meta ) do - new_issue = issue_for(issue_meta, meta[:line], "utc_now_truncate") + new_issue = issue_for(issue_meta, meta[:line], "DateTime") {ast, issues ++ List.wrap(new_issue)} end @@ -95,7 +95,7 @@ defmodule Credo.Check.Refactor.UtcNowTruncate do issues, issue_meta ) do - new_issue = issue_for(issue_meta, meta[:line], "utc_now_truncate") + new_issue = issue_for(issue_meta, meta[:line], "DateTime") {ast, issues ++ List.wrap(new_issue)} end @@ -112,7 +112,7 @@ defmodule Credo.Check.Refactor.UtcNowTruncate do issues, issue_meta ) do - new_issue = issue_for(issue_meta, meta[:line], "utc_now_truncate") + new_issue = issue_for(issue_meta, meta[:line], "NaiveDateTime") {ast, issues ++ List.wrap(new_issue)} end @@ -128,7 +128,7 @@ defmodule Credo.Check.Refactor.UtcNowTruncate do issues, issue_meta ) do - new_issue = issue_for(issue_meta, meta[:line], "utc_now_truncate") + new_issue = issue_for(issue_meta, meta[:line], "NaiveDateTime") {ast, issues ++ List.wrap(new_issue)} end @@ -147,7 +147,7 @@ defmodule Credo.Check.Refactor.UtcNowTruncate do issues, issue_meta ) do - new_issue = issue_for(issue_meta, meta[:line], "utc_now_truncate") + new_issue = issue_for(issue_meta, meta[:line], "NaiveDateTime") {ast, issues ++ List.wrap(new_issue)} end @@ -166,7 +166,7 @@ defmodule Credo.Check.Refactor.UtcNowTruncate do issues, issue_meta ) do - new_issue = issue_for(issue_meta, meta[:line], "utc_now_truncate") + new_issue = issue_for(issue_meta, meta[:line], "NaiveDateTime") {ast, issues ++ List.wrap(new_issue)} end @@ -174,12 +174,12 @@ defmodule Credo.Check.Refactor.UtcNowTruncate do {ast, issues} end - defp issue_for(issue_meta, line_no, trigger) do + defp issue_for(issue_meta, line_no, module) do format_issue( issue_meta, message: - "Pass time unit to `{Naive}DateTime.utc_now` instead of composing with `{Naive}DateTime.truncate/2`.", - trigger: trigger, + "Pass time unit to `#{module}.utc_now` instead of composing with `#{module}.truncate/2`.", + trigger: "#{module}.truncate/2", line_no: line_no ) end From 728bf4dc033e03faf0626fd324c60d69a47fbc01 Mon Sep 17 00:00:00 2001 From: Frerich Raabe Date: Sat, 16 Dec 2023 12:53:03 +0100 Subject: [PATCH 6/8] Adjust test descriptions to match naming conventions --- .../check/refactor/utc_now_truncate_test.exs | 40 +++++++++---------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/test/credo/check/refactor/utc_now_truncate_test.exs b/test/credo/check/refactor/utc_now_truncate_test.exs index c1c7055cb..631cae922 100644 --- a/test/credo/check/refactor/utc_now_truncate_test.exs +++ b/test/credo/check/refactor/utc_now_truncate_test.exs @@ -3,7 +3,7 @@ defmodule Credo.Check.Refactor.UtcNowTruncateTest do @described_check Credo.Check.Refactor.UtcNowTruncate - test "triggers when applying DateTime.truncate/2 to DateTime.utc_now/0" do + test "should report a violation when applying DateTime.truncate/2 to DateTime.utc_now/0" do """ defmodule M do def f do @@ -16,7 +16,7 @@ defmodule Credo.Check.Refactor.UtcNowTruncateTest do |> assert_issue() end - test "triggers when applying DateTime.truncate/2 to DateTime.utc_now/1" do + test "should report a violation when applying DateTime.truncate/2 to DateTime.utc_now/1" do """ defmodule M do def f do @@ -29,7 +29,7 @@ defmodule Credo.Check.Refactor.UtcNowTruncateTest do |> assert_issue() end - test "triggers when applying DateTime.truncate/2 to DateTime.utc_now/2" do + test "should report a violation when applying DateTime.truncate/2 to DateTime.utc_now/2" do """ defmodule M do def f do @@ -42,7 +42,7 @@ defmodule Credo.Check.Refactor.UtcNowTruncateTest do |> assert_issue() end - test "triggers when piping result of DateTime.utc_now/0 into DateTime.truncate/2" do + test "should report a violation when piping result of DateTime.utc_now/0 into DateTime.truncate/2" do """ defmodule M do def f do @@ -55,7 +55,7 @@ defmodule Credo.Check.Refactor.UtcNowTruncateTest do |> assert_issue() end - test "triggers when piping result of DateTime.utc_now/1 into DateTime.truncate/2" do + test "should report a violation when piping result of DateTime.utc_now/1 into DateTime.truncate/2" do """ defmodule M do def f do @@ -68,7 +68,7 @@ defmodule Credo.Check.Refactor.UtcNowTruncateTest do |> assert_issue() end - test "triggers when piping result of DateTime.utc_now/2 into DateTime.truncate/2" do + test "should report a violation when piping result of DateTime.utc_now/2 into DateTime.truncate/2" do """ defmodule M do def f do @@ -81,7 +81,7 @@ defmodule Credo.Check.Refactor.UtcNowTruncateTest do |> assert_issue() end - test "triggers when piping argument to DateTime.utc_now/1 and applying DateTime.truncate/2 to that" do + test "should report a violation when piping argument to DateTime.utc_now/1 and applying DateTime.truncate/2 to that" do """ defmodule M do def f do @@ -94,7 +94,7 @@ defmodule Credo.Check.Refactor.UtcNowTruncateTest do |> assert_issue() end - test "triggers when piping argument to DateTime.utc_now/2 and applying DateTime.truncate/2 to that" do + test "should report a violation when piping argument to DateTime.utc_now/2 and applying DateTime.truncate/2 to that" do """ defmodule M do def f do @@ -107,7 +107,7 @@ defmodule Credo.Check.Refactor.UtcNowTruncateTest do |> assert_issue() end - test "triggers when piping argument to DateTime.utc_now/1 and piping result to DateTime.truncate/2" do + test "should report a violation when piping argument to DateTime.utc_now/1 and piping result to DateTime.truncate/2" do """ defmodule M do def f do @@ -120,7 +120,7 @@ defmodule Credo.Check.Refactor.UtcNowTruncateTest do |> assert_issue() end - test "triggers when piping argument to DateTime.utc_now/2 and piping result to DateTime.truncate/2" do + test "should report a violation when piping argument to DateTime.utc_now/2 and piping result to DateTime.truncate/2" do """ defmodule M do def f do @@ -133,7 +133,7 @@ defmodule Credo.Check.Refactor.UtcNowTruncateTest do |> assert_issue() end - test "triggers when applying NaiveDateTime.truncate/2 to NaiveDateTime.utc_now/0" do + test "should report a violation when applying NaiveDateTime.truncate/2 to NaiveDateTime.utc_now/0" do """ defmodule M do def f do @@ -146,7 +146,7 @@ defmodule Credo.Check.Refactor.UtcNowTruncateTest do |> assert_issue() end - test "triggers when applying NaiveDateTime.truncate/2 to NaiveDateTime.utc_now/1" do + test "should report a violation when applying NaiveDateTime.truncate/2 to NaiveDateTime.utc_now/1" do """ defmodule M do def f do @@ -159,7 +159,7 @@ defmodule Credo.Check.Refactor.UtcNowTruncateTest do |> assert_issue() end - test "triggers when applying NaiveDateTime.truncate/2 to NaiveDateTime.utc_now/2" do + test "should report a violation when applying NaiveDateTime.truncate/2 to NaiveDateTime.utc_now/2" do """ defmodule M do def f do @@ -172,7 +172,7 @@ defmodule Credo.Check.Refactor.UtcNowTruncateTest do |> assert_issue() end - test "triggers when piping result of NaiveDateTime.utc_now/0 into NaiveDateTime.truncate/2" do + test "should report a violation when piping result of NaiveDateTime.utc_now/0 into NaiveDateTime.truncate/2" do """ defmodule M do def f do @@ -185,7 +185,7 @@ defmodule Credo.Check.Refactor.UtcNowTruncateTest do |> assert_issue() end - test "triggers when piping result of NaiveDateTime.utc_now/1 into NaiveDateTime.truncate/2" do + test "should report a violation when piping result of NaiveDateTime.utc_now/1 into NaiveDateTime.truncate/2" do """ defmodule M do def f do @@ -198,7 +198,7 @@ defmodule Credo.Check.Refactor.UtcNowTruncateTest do |> assert_issue() end - test "triggers when piping result of NaiveDateTime.utc_now/2 into NaiveDateTime.truncate/2" do + test "should report a violation when piping result of NaiveDateTime.utc_now/2 into NaiveDateTime.truncate/2" do """ defmodule M do def f do @@ -211,7 +211,7 @@ defmodule Credo.Check.Refactor.UtcNowTruncateTest do |> assert_issue() end - test "triggers when piping argument to NaiveDateTime.utc_now/1 and applying NaiveDateTime.truncate/2 to that" do + test "should report a violation when piping argument to NaiveDateTime.utc_now/1 and applying NaiveDateTime.truncate/2 to that" do """ defmodule M do def f do @@ -224,7 +224,7 @@ defmodule Credo.Check.Refactor.UtcNowTruncateTest do |> assert_issue() end - test "triggers when piping argument to NaiveDateTime.utc_now/2 and applying NaiveDateTime.truncate/2 to that" do + test "should report a violation when piping argument to NaiveDateTime.utc_now/2 and applying NaiveDateTime.truncate/2 to that" do """ defmodule M do def f do @@ -237,7 +237,7 @@ defmodule Credo.Check.Refactor.UtcNowTruncateTest do |> assert_issue() end - test "triggers when piping argument to NaiveDateTime.utc_now/1 and piping result to NaiveDateTime.truncate/2" do + test "should report a violation when piping argument to NaiveDateTime.utc_now/1 and piping result to NaiveDateTime.truncate/2" do """ defmodule M do def f do @@ -250,7 +250,7 @@ defmodule Credo.Check.Refactor.UtcNowTruncateTest do |> assert_issue() end - test "triggers when piping argument to NaiveDateTime.utc_now/2 and piping result to NaiveDateTime.truncate/2" do + test "should report a violation when piping argument to NaiveDateTime.utc_now/2 and piping result to NaiveDateTime.truncate/2" do """ defmodule M do def f do From aa498e183a03750e624e0560940c9e02a02eec7c Mon Sep 17 00:00:00 2001 From: Frerich Raabe Date: Fri, 29 Dec 2023 07:37:07 +0100 Subject: [PATCH 7/8] Corrected trigger This should be a quote from the source code, Credo.Check.add_column_if_missing/5 relies on this to calculate the column value for the issue. --- lib/credo/check/refactor/utc_now_truncate.ex | 2 +- .../check/refactor/utc_now_truncate_test.exs | 26 +++++++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/lib/credo/check/refactor/utc_now_truncate.ex b/lib/credo/check/refactor/utc_now_truncate.ex index 0a79b1a78..0a21a6587 100644 --- a/lib/credo/check/refactor/utc_now_truncate.ex +++ b/lib/credo/check/refactor/utc_now_truncate.ex @@ -179,7 +179,7 @@ defmodule Credo.Check.Refactor.UtcNowTruncate do issue_meta, message: "Pass time unit to `#{module}.utc_now` instead of composing with `#{module}.truncate/2`.", - trigger: "#{module}.truncate/2", + trigger: "#{module}.truncate", line_no: line_no ) end diff --git a/test/credo/check/refactor/utc_now_truncate_test.exs b/test/credo/check/refactor/utc_now_truncate_test.exs index 631cae922..b559d3cb2 100644 --- a/test/credo/check/refactor/utc_now_truncate_test.exs +++ b/test/credo/check/refactor/utc_now_truncate_test.exs @@ -262,4 +262,30 @@ defmodule Credo.Check.Refactor.UtcNowTruncateTest do |> run_check(@described_check) |> assert_issue() end + + test "should report a violaton with a correct trigger value for DateTime.truncate/2" do + """ + defmodule M do + def f do + DateTime.truncate(DateTime.utc_now(), :second) + end + end + """ + |> to_source_file + |> run_check(@described_check) + |> assert_issue(fn issue -> assert issue.trigger == "DateTime.truncate" end) + end + + test "should report a violaton with a correct trigger value for NaiveDateTime.truncate/2" do + """ + defmodule M do + def f do + NaiveDateTime.truncate(NaiveDateTime.utc_now(), :second) + end + end + """ + |> to_source_file + |> run_check(@described_check) + |> assert_issue(fn issue -> assert issue.trigger == "NaiveDateTime.truncate" end) + end end From 301397d7aa15da895b0f2481814a77b25b64bb3f Mon Sep 17 00:00:00 2001 From: Frerich Raabe Date: Fri, 29 Dec 2023 07:37:57 +0100 Subject: [PATCH 8/8] Correct line number for reported issues The `line_no` value should be the line where the `trigger` appears, i.e. we should be using the meta information of the `truncate` AST node here. --- lib/credo/check/refactor/utc_now_truncate.ex | 16 +++++----- .../check/refactor/utc_now_truncate_test.exs | 30 +++++++++++++++++++ 2 files changed, 38 insertions(+), 8 deletions(-) diff --git a/lib/credo/check/refactor/utc_now_truncate.ex b/lib/credo/check/refactor/utc_now_truncate.ex index 0a21a6587..66899a0fb 100644 --- a/lib/credo/check/refactor/utc_now_truncate.ex +++ b/lib/credo/check/refactor/utc_now_truncate.ex @@ -49,10 +49,10 @@ defmodule Credo.Check.Refactor.UtcNowTruncate do # DateTime.utc_now(_) |> DateTime.truncate(_) # DateTime.utc_now(_, _) |> DateTime.truncate(_) defp traverse( - {:|>, meta, + {:|>, _, [ {{:., _, [{:__aliases__, _, [:DateTime]}, :utc_now]}, _, _}, - {{:., _, [{:__aliases__, _, [:DateTime]}, :truncate]}, _, [_]} + {{:., meta, [{:__aliases__, _, [:DateTime]}, :truncate]}, _, [_]} ]} = ast, issues, issue_meta @@ -83,14 +83,14 @@ defmodule Credo.Check.Refactor.UtcNowTruncate do # _ |> DateTime.utc_now() |> DateTime.truncate(_) # _ |> DateTime.utc_now(_) |> DateTime.truncate(_) defp traverse( - {:|>, meta, + {:|>, _, [ {:|>, _, [ _, {{:., _, [{:__aliases__, _, [:DateTime]}, :utc_now]}, _, _} ]}, - {{:., _, [{:__aliases__, _, [:DateTime]}, :truncate]}, _, [_]} + {{:., meta, [{:__aliases__, _, [:DateTime]}, :truncate]}, _, [_]} ]} = ast, issues, issue_meta @@ -120,10 +120,10 @@ defmodule Credo.Check.Refactor.UtcNowTruncate do # NaiveDateTime.utc_now(_) |> NaiveDateTime.truncate(_) # NaiveDateTime.utc_now(_, _) |> NaiveDateTime.truncate(_) defp traverse( - {:|>, meta, + {:|>, _, [ {{:., _, [{:__aliases__, _, [:NaiveDateTime]}, :utc_now]}, _, _}, - {{:., _, [{:__aliases__, _, [:NaiveDateTime]}, :truncate]}, _, [_]} + {{:., meta, [{:__aliases__, _, [:NaiveDateTime]}, :truncate]}, _, [_]} ]} = ast, issues, issue_meta @@ -154,14 +154,14 @@ defmodule Credo.Check.Refactor.UtcNowTruncate do # _ |> NaiveDateTime.utc_now() |> NaiveDateTime.truncate(_) # _ |> NaiveDateTime.utc_now(_) |> NaiveDateTime.truncate(_) defp traverse( - {:|>, meta, + {:|>, _, [ {:|>, _, [ _, {{:., _, [{:__aliases__, _, [:NaiveDateTime]}, :utc_now]}, _, _} ]}, - {{:., _, [{:__aliases__, _, [:NaiveDateTime]}, :truncate]}, _, [_]} + {{:., meta, [{:__aliases__, _, [:NaiveDateTime]}, :truncate]}, _, [_]} ]} = ast, issues, issue_meta diff --git a/test/credo/check/refactor/utc_now_truncate_test.exs b/test/credo/check/refactor/utc_now_truncate_test.exs index b559d3cb2..82d49b5e1 100644 --- a/test/credo/check/refactor/utc_now_truncate_test.exs +++ b/test/credo/check/refactor/utc_now_truncate_test.exs @@ -288,4 +288,34 @@ defmodule Credo.Check.Refactor.UtcNowTruncateTest do |> run_check(@described_check) |> assert_issue(fn issue -> assert issue.trigger == "NaiveDateTime.truncate" end) end + + test "should report a violaton with a correct line_no value for DateTime.truncate/2" do + """ + defmodule M do + def f do + DateTime.utc_now() + |> + DateTime.truncate(:second) + end + end + """ + |> to_source_file + |> run_check(@described_check) + |> assert_issue(fn issue -> assert issue.line_no == 5 end) + end + + test "should report a violaton with a correct line_no value for NaiveDateTime.truncate/2" do + """ + defmodule M do + def f do + NaiveDateTime.utc_now() + |> + NaiveDateTime.truncate(:second) + end + end + """ + |> to_source_file + |> run_check(@described_check) + |> assert_issue(fn issue -> assert issue.line_no == 5 end) + end end