From 2f5038c8d68b6245b5e77407ef7d79bd13cac4d5 Mon Sep 17 00:00:00 2001
From: Hans Krutzer <git@pixelspaceships.com>
Date: Sun, 2 Mar 2025 21:52:05 +0100
Subject: [PATCH] Add check for piping into Logger functions

---
 lib/credo/check/warning/pipe_to_logger.ex     |  57 +++++++
 .../check/warning/pipe_to_logger_test.exs     | 153 ++++++++++++++++++
 2 files changed, 210 insertions(+)
 create mode 100644 lib/credo/check/warning/pipe_to_logger.ex
 create mode 100644 test/credo/check/warning/pipe_to_logger_test.exs

diff --git a/lib/credo/check/warning/pipe_to_logger.ex b/lib/credo/check/warning/pipe_to_logger.ex
new file mode 100644
index 000000000..f991221d4
--- /dev/null
+++ b/lib/credo/check/warning/pipe_to_logger.ex
@@ -0,0 +1,57 @@
+defmodule Credo.Check.Warning.PipeToLogger do
+  @moduledoc false
+
+  use Credo.Check,
+    id: "EX??",
+    base_priority: :normal,
+    category: :warning,
+    explanations: [
+      check: """
+      Calls into Logger can be purged when `:compile_time_purge_matching` is enabled in
+      the Logger configuration. This will remove the entire pipeline.
+      Use |> tap(&Logger.info(&1)) instead.
+      """
+    ]
+
+  @logger_functions [
+    :debug,
+    :info,
+    :notice,
+    :warn,
+    :warning,
+    :error,
+    :critical,
+    :alert,
+    :emergency
+  ]
+
+  def run(%SourceFile{} = source_file, params) do
+    issue_meta = IssueMeta.for(source_file, params)
+
+    Credo.Code.prewalk(source_file, &traverse(&1, &2, issue_meta))
+  end
+
+  defp traverse(
+         {:|>, meta, [_, {{:., _, [{:__aliases__, _, [:Logger]}, function]}, _, _}]} = ast,
+         issues,
+         issue_meta
+       )
+       when function in @logger_functions do
+    trigger = "|> Logger.#{function}"
+
+    new_issue =
+      format_issue(
+        issue_meta,
+        message: "There should be no `@#{trigger} calls.",
+        trigger: "#{trigger}",
+        line_no: meta[:line],
+        column: meta[:column]
+      )
+
+    {ast, issues ++ [new_issue]}
+  end
+
+  defp traverse(ast, issues, _issue_meta) do
+    {ast, issues}
+  end
+end
diff --git a/test/credo/check/warning/pipe_to_logger_test.exs b/test/credo/check/warning/pipe_to_logger_test.exs
new file mode 100644
index 000000000..813ade741
--- /dev/null
+++ b/test/credo/check/warning/pipe_to_logger_test.exs
@@ -0,0 +1,153 @@
+defmodule Credo.Check.Warning.PipeToLoggerTest do
+  use Credo.Test.Case
+
+  @described_check Credo.Check.Warning.PipeToLogger
+
+  #
+  # cases NOT raising issues
+  #
+
+  test "it should NOT report when Logger is used without piping" do
+    """
+    defmodule CredoSampleModule do
+      require Logger
+
+      def some_function(parameter1) do
+        Logger.warn("This is a direct call")
+        Logger.info("Another direct call")
+
+        something_else()
+      end
+    end
+    """
+    |> to_source_file
+    |> run_check(@described_check)
+    |> refute_issues()
+  end
+
+  test "it should NOT report when piping to other functions" do
+    """
+    defmodule CredoSampleModule do
+      def some_function(parameter1) do
+        parameter1
+        |> process_data()
+        |> transform_result()
+      end
+    end
+    """
+    |> to_source_file
+    |> run_check(@described_check)
+    |> refute_issues()
+  end
+
+  test "it should NOT report when Logger is in a different position" do
+    """
+    defmodule CredoSampleModule do
+      require Logger
+
+      def some_function(parameter1) do
+        Logger.warn(parameter1 |> transform_data())
+
+        # Other code
+        result = other_function() |> process_result()
+
+        result
+      end
+    end
+    """
+    |> to_source_file
+    |> run_check(@described_check)
+    |> refute_issues()
+  end
+
+  #
+  # cases raising issues
+  #
+
+  test "it should report a violation when piping to Logger.warn" do
+    """
+    defmodule CredoSampleModule do
+      require Logger
+
+      def some_function(parameter1) do
+        parameter1
+        |> transform_data()
+        |> Logger.warn()
+
+        :ok
+      end
+    end
+    """
+    |> to_source_file
+    |> run_check(@described_check)
+    |> assert_issue(fn issue ->
+      assert issue.line_no == 7
+      assert issue.column == 5
+      assert issue.trigger == "|> Logger.warn"
+    end)
+  end
+
+  test "it should report a violation when piping to any Logger function" do
+    """
+    defmodule CredoSampleModule do
+      require Logger
+
+      def some_function(parameter1) do
+        "Starting process"
+        |> Logger.info()
+
+        parameter1
+        |> transform_data()
+        |> Logger.error()
+
+        "Debug information"
+        |> Logger.debug()
+
+        :ok
+      end
+    end
+    """
+    |> to_source_file
+    |> run_check(@described_check)
+    |> assert_issues(fn issues ->
+      assert length(issues) == 3
+
+      [issue1, issue2, issue3] = issues
+
+      assert issue1.line_no == 6
+      assert issue1.column == 5
+      assert issue1.trigger == "|> Logger.info"
+
+      assert issue2.line_no == 10
+      assert issue2.column == 5
+      assert issue2.trigger == "|> Logger.error"
+
+      assert issue3.line_no == 13
+      assert issue3.column == 5
+      assert issue3.trigger == "|> Logger.debug"
+    end)
+  end
+
+  test "it should report a violation when piping to Logger with options" do
+    """
+    defmodule CredoSampleModule do
+      require Logger
+
+      def some_function(error) do
+        error
+        |> inspect()
+        |> Logger.error(some_metadata: "value")
+
+        :error
+      end
+    end
+    """
+    |> to_source_file
+    |> run_check(@described_check)
+    |> assert_issue(fn issue ->
+      assert issue.line_no == 7
+      assert issue.column == 5
+      assert issue.trigger == "|> Logger.error"
+    end)
+  end
+end