From 0d639f3ff61d2f32544905ada88f40e7eef647e4 Mon Sep 17 00:00:00 2001
From: Trevor Brown <trevorbrown@rentpath.com>
Date: Wed, 5 Jan 2022 17:05:31 -0500
Subject: [PATCH 1/8] Create CondCatchallTrue readability check

---
 .../check/readability/cond_catchall_true.ex   |  78 ++++++++
 .../readability/cond_catchall_true_test.exs   | 178 ++++++++++++++++++
 2 files changed, 256 insertions(+)
 create mode 100644 lib/credo/check/readability/cond_catchall_true.ex
 create mode 100644 test/credo/check/readability/cond_catchall_true_test.exs

diff --git a/lib/credo/check/readability/cond_catchall_true.ex b/lib/credo/check/readability/cond_catchall_true.ex
new file mode 100644
index 000000000..b32c2a6d9
--- /dev/null
+++ b/lib/credo/check/readability/cond_catchall_true.ex
@@ -0,0 +1,78 @@
+defmodule Credo.Check.Readability.CondCatchallTrue do
+  use Credo.Check,
+    explanations: [
+      check: """
+      If a cond expresion ends in an "always true" statement. That last
+      statement should be simply `true`. Other literal truthy values (such as
+      `:else`, `:always`, etc... aren't allowed.
+
+      Example:
+
+          cond do
+            x == y -> 0
+            x > y -> 0
+            :else -> 1
+          end
+
+          # should be written as
+
+          cond do
+            x == y -> 0
+            x > y -> 0
+            true -> 1
+          end
+      """
+    ]
+
+  @doc false
+  @impl true
+  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({:cond, meta, arguments} = ast, issues, issue_meta) do
+    conditions =
+      arguments
+      |> Credo.Code.Block.do_block_for!()
+      |> List.wrap()
+
+    if conditions
+      |> List.last()
+      |> catchall_other_than_true?() do
+      {ast, issues ++ [issue_for(issue_meta, meta[:line], :cond)]}
+    else
+      {ast, issues}
+    end
+  end
+
+  defp traverse(ast, issues, _issue_meta) do
+    {ast, issues}
+  end
+
+  defp catchall_other_than_true?({:->, _meta, [[true], _args]}), do: false
+  # Integer literal catch-all clause
+  defp catchall_other_than_true?({:->, _meta, ['{', _args]}), do: true
+  # Binary literal catch-all clause
+  defp catchall_other_than_true?({:->, _meta, [[binary], _args]}) when is_binary(binary), do: true
+  # List literal catch-all clause
+  defp catchall_other_than_true?({:->, _meta, [[list], _args]}) when is_list(list), do: true
+  # Map literal catch-all clause
+  defp catchall_other_than_true?({:->, _meta, [[{:%{}, _meta2, []}], _args]}), do: true
+  # Tuple literal catch-all clause
+  defp catchall_other_than_true?({:->, _meta, [[{:{}, _meta2, _values}], _args]}), do: true
+  # Atom literal catch-all clause
+  defp catchall_other_than_true?({:->, _meta, [[name], _args]}) when is_atom(name), do: true
+  defp catchall_other_than_true?(_), do: false
+
+  defp issue_for(issue_meta, line_no, trigger) do
+    format_issue(
+      issue_meta,
+      message:
+      "Cond statements that end with an \"always true\" condition should use `true` instead of some other literal.",
+      trigger: trigger,
+      line_no: line_no
+    )
+  end
+end
diff --git a/test/credo/check/readability/cond_catchall_true_test.exs b/test/credo/check/readability/cond_catchall_true_test.exs
new file mode 100644
index 000000000..2f1d66813
--- /dev/null
+++ b/test/credo/check/readability/cond_catchall_true_test.exs
@@ -0,0 +1,178 @@
+defmodule Credo.Check.Readability.CondCatchallTrueTest do
+  use Credo.Test.Case
+
+  alias Credo.Check.Readability.CondCatchallTrue
+
+  test "it should NOT report conds with a last condition of true" do
+    """
+    defmodule CredoSampleModule do
+      def cond_true(a) do
+        cond do
+          a + 2 == 5 ->
+            "Nope"
+
+          a + 3 == 5 ->
+            "Uh, uh"
+
+          true ->
+            "OK"
+        end
+      end
+    end
+    """
+    |> to_source_file()
+    |> run_check(CondCatchallTrue)
+    |> refute_issues()
+  end
+
+  test "it should NOT report conds with a last condition that uses a variable" do
+    """
+    defmodule CredoSampleModule do
+      def cond_true(a) do
+        cond do
+          a + 2 == 5 ->
+            "Nope"
+
+          a + 3 == 5 ->
+            "Uh, uh"
+        end
+      end
+    end
+    """
+    |> to_source_file()
+    |> run_check(CondCatchallTrue)
+    |> refute_issues()
+  end
+
+  test "it should report conds that with a last condition that is some other literal" do
+    """
+    defmodule CredoSampleModule do
+      def cond_true(a) do
+        cond do
+          a + 2 == 5 ->
+            "Nope"
+
+          a + 3 == 5 ->
+            "Uh, uh"
+
+          :else ->
+            "OK"
+        end
+      end
+    end
+    """
+    |> to_source_file()
+    |> run_check(CondCatchallTrue)
+    |> assert_issue()
+  end
+
+  test "it should report conds that with a last condition that is binary literal" do
+    """
+    defmodule CredoSampleModule do
+      def cond_true(a) do
+        cond do
+          a + 2 == 5 ->
+            "Nope"
+
+          a + 3 == 5 ->
+            "Uh, uh"
+
+          "else" ->
+            "OK"
+        end
+      end
+    end
+    """
+    |> to_source_file()
+    |> run_check(CondCatchallTrue)
+    |> assert_issue()
+  end
+
+  test "it should report conds that with a last condition that is integer literal" do
+    """
+    defmodule CredoSampleModule do
+      def cond_true(a) do
+        cond do
+          a + 2 == 5 ->
+            "Nope"
+
+          a + 3 == 5 ->
+            "Uh, uh"
+
+          123 ->
+            "OK"
+        end
+      end
+    end
+    """
+    |> to_source_file()
+    |> run_check(CondCatchallTrue)
+    |> assert_issue()
+  end
+
+  test "it should report conds that with a last condition that is list literal" do
+    """
+    defmodule CredoSampleModule do
+      def cond_true(a) do
+        cond do
+          a + 2 == 5 ->
+            "Nope"
+
+          a + 3 == 5 ->
+            "Uh, uh"
+
+          [:else] ->
+            "OK"
+        end
+      end
+    end
+    """
+    |> to_source_file()
+    |> run_check(CondCatchallTrue)
+    |> assert_issue()
+  end
+
+  test "it should report conds that with a last condition that is tuple literal" do
+    """
+    defmodule CredoSampleModule do
+      def cond_true(a) do
+        cond do
+          a + 2 == 5 ->
+            "Nope"
+
+          a + 3 == 5 ->
+            "Uh, uh"
+
+          {:else} ->
+            "OK"
+        end
+      end
+    end
+    """
+    |> to_source_file()
+    |> run_check(CondCatchallTrue)
+    |> assert_issue()
+  end
+
+  test "it should report conds that with a last condition that is map literal" do
+    """
+    defmodule CredoSampleModule do
+      def cond_true(a) do
+        cond do
+          a + 2 == 5 ->
+            "Nope"
+
+          a + 3 == 5 ->
+            "Uh, uh"
+
+          %{} ->
+            "OK"
+        end
+      end
+    end
+    """
+    |> to_source_file()
+    |> run_check(CondCatchallTrue)
+    |> assert_issue()
+  end
+end

From eb82fbe72b74ff7e937b3afc776e9186dd52cc1e Mon Sep 17 00:00:00 2001
From: Trevor Brown <trevorbrown@rentpath.com>
Date: Mon, 10 Jan 2022 09:28:15 -0500
Subject: [PATCH 2/8] Rename CondCatchallTrue to CondFinalCondition

---
 ...tchall_true.ex => cond_final_condition.ex} |  2 +-
 ...test.exs => cond_final_condition_test.exs} | 20 +++++++++----------
 2 files changed, 11 insertions(+), 11 deletions(-)
 rename lib/credo/check/readability/{cond_catchall_true.ex => cond_final_condition.ex} (97%)
 rename test/credo/check/readability/{cond_catchall_true_test.exs => cond_final_condition_test.exs} (88%)

diff --git a/lib/credo/check/readability/cond_catchall_true.ex b/lib/credo/check/readability/cond_final_condition.ex
similarity index 97%
rename from lib/credo/check/readability/cond_catchall_true.ex
rename to lib/credo/check/readability/cond_final_condition.ex
index b32c2a6d9..4f3708dc5 100644
--- a/lib/credo/check/readability/cond_catchall_true.ex
+++ b/lib/credo/check/readability/cond_final_condition.ex
@@ -1,4 +1,4 @@
-defmodule Credo.Check.Readability.CondCatchallTrue do
+defmodule Credo.Check.Readability.CondFinalCondition do
   use Credo.Check,
     explanations: [
       check: """
diff --git a/test/credo/check/readability/cond_catchall_true_test.exs b/test/credo/check/readability/cond_final_condition_test.exs
similarity index 88%
rename from test/credo/check/readability/cond_catchall_true_test.exs
rename to test/credo/check/readability/cond_final_condition_test.exs
index 2f1d66813..3264e30d2 100644
--- a/test/credo/check/readability/cond_catchall_true_test.exs
+++ b/test/credo/check/readability/cond_final_condition_test.exs
@@ -1,7 +1,7 @@
-defmodule Credo.Check.Readability.CondCatchallTrueTest do
+defmodule Credo.Check.Readability.CondFinalConditionTest do
   use Credo.Test.Case
 
-  alias Credo.Check.Readability.CondCatchallTrue
+  alias Credo.Check.Readability.CondFinalCondition
 
   test "it should NOT report conds with a last condition of true" do
     """
@@ -21,7 +21,7 @@ defmodule Credo.Check.Readability.CondCatchallTrueTest do
     end
     """
     |> to_source_file()
-    |> run_check(CondCatchallTrue)
+    |> run_check(CondFinalCondition)
     |> refute_issues()
   end
 
@@ -40,7 +40,7 @@ defmodule Credo.Check.Readability.CondCatchallTrueTest do
     end
     """
     |> to_source_file()
-    |> run_check(CondCatchallTrue)
+    |> run_check(CondFinalCondition)
     |> refute_issues()
   end
 
@@ -62,7 +62,7 @@ defmodule Credo.Check.Readability.CondCatchallTrueTest do
     end
     """
     |> to_source_file()
-    |> run_check(CondCatchallTrue)
+    |> run_check(CondFinalCondition)
     |> assert_issue()
   end
 
@@ -84,7 +84,7 @@ defmodule Credo.Check.Readability.CondCatchallTrueTest do
     end
     """
     |> to_source_file()
-    |> run_check(CondCatchallTrue)
+    |> run_check(CondFinalCondition)
     |> assert_issue()
   end
 
@@ -106,7 +106,7 @@ defmodule Credo.Check.Readability.CondCatchallTrueTest do
     end
     """
     |> to_source_file()
-    |> run_check(CondCatchallTrue)
+    |> run_check(CondFinalCondition)
     |> assert_issue()
   end
 
@@ -128,7 +128,7 @@ defmodule Credo.Check.Readability.CondCatchallTrueTest do
     end
     """
     |> to_source_file()
-    |> run_check(CondCatchallTrue)
+    |> run_check(CondFinalCondition)
     |> assert_issue()
   end
 
@@ -150,7 +150,7 @@ defmodule Credo.Check.Readability.CondCatchallTrueTest do
     end
     """
     |> to_source_file()
-    |> run_check(CondCatchallTrue)
+    |> run_check(CondFinalCondition)
     |> assert_issue()
   end
 
@@ -172,7 +172,7 @@ defmodule Credo.Check.Readability.CondCatchallTrueTest do
     end
     """
     |> to_source_file()
-    |> run_check(CondCatchallTrue)
+    |> run_check(CondFinalCondition)
     |> assert_issue()
   end
 end

From 992f7bf29487d3c3cf1c3989cdda32452b7d5791 Mon Sep 17 00:00:00 2001
From: Trevor Brown <trevorbrown@rentpath.com>
Date: Mon, 10 Jan 2022 10:04:23 -0500
Subject: [PATCH 3/8] Add final_condition_value config option to
 CondFinalCondition check

---
 .../check/readability/cond_final_condition.ex | 34 ++++++++------
 .../readability/cond_final_condition_test.exs | 44 +++++++++++++++++++
 2 files changed, 65 insertions(+), 13 deletions(-)

diff --git a/lib/credo/check/readability/cond_final_condition.ex b/lib/credo/check/readability/cond_final_condition.ex
index 4f3708dc5..7894e5d08 100644
--- a/lib/credo/check/readability/cond_final_condition.ex
+++ b/lib/credo/check/readability/cond_final_condition.ex
@@ -1,5 +1,8 @@
 defmodule Credo.Check.Readability.CondFinalCondition do
   use Credo.Check,
+    # Default to the value specified here:
+    # https://github.com/christopheradams/elixir_style_guide#true-as-last-condition
+    param_defaults: [final_condition_value: true],
     explanations: [
       check: """
       If a cond expresion ends in an "always true" statement. That last
@@ -21,7 +24,10 @@ defmodule Credo.Check.Readability.CondFinalCondition do
             x > y -> 0
             true -> 1
           end
-      """
+      """,
+      params: [
+        final_condition_value: "Set the expected value for the final condition"
+      ]
     ]
 
   @doc false
@@ -29,10 +35,12 @@ defmodule Credo.Check.Readability.CondFinalCondition do
   def run(%SourceFile{} = source_file, params) do
     issue_meta = IssueMeta.for(source_file, params)
 
-    Credo.Code.prewalk(source_file, &traverse(&1, &2, issue_meta))
+    final_condition_value = Params.get(params, :final_condition_value, __MODULE__)
+
+    Credo.Code.prewalk(source_file, &traverse(&1, &2, issue_meta, final_condition_value))
   end
 
-  defp traverse({:cond, meta, arguments} = ast, issues, issue_meta) do
+  defp traverse({:cond, meta, arguments} = ast, issues, issue_meta, final_condition_value) do
     conditions =
       arguments
       |> Credo.Code.Block.do_block_for!()
@@ -40,31 +48,31 @@ defmodule Credo.Check.Readability.CondFinalCondition do
 
     if conditions
       |> List.last()
-      |> catchall_other_than_true?() do
+      |> catchall_other_than_value?(final_condition_value) do
       {ast, issues ++ [issue_for(issue_meta, meta[:line], :cond)]}
     else
       {ast, issues}
     end
   end
 
-  defp traverse(ast, issues, _issue_meta) do
+  defp traverse(ast, issues, _issue_meta, _min_pipeline_length) do
     {ast, issues}
   end
 
-  defp catchall_other_than_true?({:->, _meta, [[true], _args]}), do: false
+  defp catchall_other_than_value?({:->, _meta, [[value], _args]}, value), do: false
   # Integer literal catch-all clause
-  defp catchall_other_than_true?({:->, _meta, ['{', _args]}), do: true
+  defp catchall_other_than_value?({:->, _meta, ['{', _args]}, _value), do: true
   # Binary literal catch-all clause
-  defp catchall_other_than_true?({:->, _meta, [[binary], _args]}) when is_binary(binary), do: true
+  defp catchall_other_than_value?({:->, _meta, [[binary], _args]}, _value) when is_binary(binary), do: true
   # List literal catch-all clause
-  defp catchall_other_than_true?({:->, _meta, [[list], _args]}) when is_list(list), do: true
+  defp catchall_other_than_value?({:->, _meta, [[list], _args]}, _value) when is_list(list), do: true
   # Map literal catch-all clause
-  defp catchall_other_than_true?({:->, _meta, [[{:%{}, _meta2, []}], _args]}), do: true
+  defp catchall_other_than_value?({:->, _meta, [[{:%{}, _meta2, []}], _args]}, _value), do: true
   # Tuple literal catch-all clause
-  defp catchall_other_than_true?({:->, _meta, [[{:{}, _meta2, _values}], _args]}), do: true
+  defp catchall_other_than_value?({:->, _meta, [[{:{}, _meta2, _values}], _args]}, _value), do: true
   # Atom literal catch-all clause
-  defp catchall_other_than_true?({:->, _meta, [[name], _args]}) when is_atom(name), do: true
-  defp catchall_other_than_true?(_), do: false
+  defp catchall_other_than_value?({:->, _meta, [[name], _args]}, _value) when is_atom(name), do: true
+  defp catchall_other_than_value?(_clause, _value), do: false
 
   defp issue_for(issue_meta, line_no, trigger) do
     format_issue(
diff --git a/test/credo/check/readability/cond_final_condition_test.exs b/test/credo/check/readability/cond_final_condition_test.exs
index 3264e30d2..072e80da6 100644
--- a/test/credo/check/readability/cond_final_condition_test.exs
+++ b/test/credo/check/readability/cond_final_condition_test.exs
@@ -175,4 +175,48 @@ defmodule Credo.Check.Readability.CondFinalConditionTest do
     |> run_check(CondFinalCondition)
     |> assert_issue()
   end
+
+  test "it should report conds that with a last condition that differ from config" do
+    """
+    defmodule CredoSampleModule do
+      def cond_true(a) do
+        cond do
+          a + 2 == 5 ->
+            "Nope"
+
+          a + 3 == 5 ->
+            "Uh, uh"
+
+          true ->
+            "OK"
+        end
+      end
+    end
+    """
+    |> to_source_file()
+    |> run_check(CondFinalCondition, final_condition_value: :else)
+    |> assert_issue()
+  end
+
+  test "it should NOT report conds that with a last condition that match config" do
+    """
+    defmodule CredoSampleModule do
+      def cond_true(a) do
+        cond do
+          a + 2 == 5 ->
+            "Nope"
+
+          a + 3 == 5 ->
+            "Uh, uh"
+
+          :else ->
+            "OK"
+        end
+      end
+    end
+    """
+    |> to_source_file()
+    |> run_check(CondFinalCondition, final_condition_value: :else)
+    |> refute_issues()
+  end
 end

From e68742b2af1b054357508cfc904b6f410d470354 Mon Sep 17 00:00:00 2001
From: Trevor Brown <trevorbrown@rentpath.com>
Date: Thu, 13 Jan 2022 13:06:12 -0500
Subject: [PATCH 4/8] Update explaination for the CondFinalCondition check

---
 lib/credo/check/readability/cond_final_condition.ex | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/lib/credo/check/readability/cond_final_condition.ex b/lib/credo/check/readability/cond_final_condition.ex
index 7894e5d08..530e73e17 100644
--- a/lib/credo/check/readability/cond_final_condition.ex
+++ b/lib/credo/check/readability/cond_final_condition.ex
@@ -5,9 +5,9 @@ defmodule Credo.Check.Readability.CondFinalCondition do
     param_defaults: [final_condition_value: true],
     explanations: [
       check: """
-      If a cond expresion ends in an "always true" statement. That last
-      statement should be simply `true`. Other literal truthy values (such as
-      `:else`, `:always`, etc... aren't allowed.
+      If a cond expresion ends in an "always true" statement the statement
+      should be the literal `true`, or the literal value specified in this
+      check's `final_condition_value` parameter.
 
       Example:
 

From 635abb27a34eef24a88e73c43cd368403c440270 Mon Sep 17 00:00:00 2001
From: Trevor Brown <trevorbrown@rentpath.com>
Date: Fri, 14 Jan 2022 09:10:48 -0500
Subject: [PATCH 5/8] Update test names for CondFinalCondition check

---
 .../readability/cond_final_condition_test.exs      | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/test/credo/check/readability/cond_final_condition_test.exs b/test/credo/check/readability/cond_final_condition_test.exs
index 072e80da6..ae1234aba 100644
--- a/test/credo/check/readability/cond_final_condition_test.exs
+++ b/test/credo/check/readability/cond_final_condition_test.exs
@@ -66,7 +66,7 @@ defmodule Credo.Check.Readability.CondFinalConditionTest do
     |> assert_issue()
   end
 
-  test "it should report conds that with a last condition that is binary literal" do
+  test "it should report conds with a last condition that is a binary literal" do
     """
     defmodule CredoSampleModule do
       def cond_true(a) do
@@ -88,7 +88,7 @@ defmodule Credo.Check.Readability.CondFinalConditionTest do
     |> assert_issue()
   end
 
-  test "it should report conds that with a last condition that is integer literal" do
+  test "it should report conds with a last condition that is an integer literal" do
     """
     defmodule CredoSampleModule do
       def cond_true(a) do
@@ -110,7 +110,7 @@ defmodule Credo.Check.Readability.CondFinalConditionTest do
     |> assert_issue()
   end
 
-  test "it should report conds that with a last condition that is list literal" do
+  test "it should report conds with a last condition that is a list literal" do
     """
     defmodule CredoSampleModule do
       def cond_true(a) do
@@ -132,7 +132,7 @@ defmodule Credo.Check.Readability.CondFinalConditionTest do
     |> assert_issue()
   end
 
-  test "it should report conds that with a last condition that is tuple literal" do
+  test "it should report conds with a last condition that is a tuple literal" do
     """
     defmodule CredoSampleModule do
       def cond_true(a) do
@@ -154,7 +154,7 @@ defmodule Credo.Check.Readability.CondFinalConditionTest do
     |> assert_issue()
   end
 
-  test "it should report conds that with a last condition that is map literal" do
+  test "it should report conds with a last condition that is a map literal" do
     """
     defmodule CredoSampleModule do
       def cond_true(a) do
@@ -176,7 +176,7 @@ defmodule Credo.Check.Readability.CondFinalConditionTest do
     |> assert_issue()
   end
 
-  test "it should report conds that with a last condition that differ from config" do
+  test "it should report conds with a last condition that differ from the config" do
     """
     defmodule CredoSampleModule do
       def cond_true(a) do
@@ -198,7 +198,7 @@ defmodule Credo.Check.Readability.CondFinalConditionTest do
     |> assert_issue()
   end
 
-  test "it should NOT report conds that with a last condition that match config" do
+  test "it should NOT report conds with a last condition that match the config" do
     """
     defmodule CredoSampleModule do
       def cond_true(a) do

From 3ce6622750571be9d5dd75948e01759066d4fd51 Mon Sep 17 00:00:00 2001
From: Trevor Brown <trevorbrown@rentpath.com>
Date: Fri, 21 Jan 2022 12:25:34 -0500
Subject: [PATCH 6/8] =?UTF-8?q?Update=20PR=20based=20on=20Ren=C3=A9's=20fe?=
 =?UTF-8?q?edback?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

---
 .../check/readability/cond_final_condition.ex | 67 +++++++++++++------
 .../readability/cond_final_condition_test.exs | 56 ++++++++--------
 2 files changed, 74 insertions(+), 49 deletions(-)

diff --git a/lib/credo/check/readability/cond_final_condition.ex b/lib/credo/check/readability/cond_final_condition.ex
index 530e73e17..7698ac7fa 100644
--- a/lib/credo/check/readability/cond_final_condition.ex
+++ b/lib/credo/check/readability/cond_final_condition.ex
@@ -1,13 +1,14 @@
 defmodule Credo.Check.Readability.CondFinalCondition do
   use Credo.Check,
+    tags: [:controversial],
     # Default to the value specified here:
     # https://github.com/christopheradams/elixir_style_guide#true-as-last-condition
-    param_defaults: [final_condition_value: true],
+    param_defaults: [value: true],
     explanations: [
       check: """
       If a cond expresion ends in an "always true" statement the statement
       should be the literal `true`, or the literal value specified in this
-      check's `final_condition_value` parameter.
+      check's `value` parameter.
 
       Example:
 
@@ -24,9 +25,13 @@ defmodule Credo.Check.Readability.CondFinalCondition do
             x > y -> 0
             true -> 1
           end
+
+      Like all `Readability` issues, this one is not a technical concern.
+      But you can improve the odds of other reading and liking your code by making
+      it easier to follow.
       """,
       params: [
-        final_condition_value: "Set the expected value for the final condition"
+        value: "Set the expected value for the final condition"
       ]
     ]
 
@@ -35,23 +40,23 @@ defmodule Credo.Check.Readability.CondFinalCondition do
   def run(%SourceFile{} = source_file, params) do
     issue_meta = IssueMeta.for(source_file, params)
 
-    final_condition_value = Params.get(params, :final_condition_value, __MODULE__)
+    final_condition_value = Params.get(params, :value, __MODULE__)
 
     Credo.Code.prewalk(source_file, &traverse(&1, &2, issue_meta, final_condition_value))
   end
 
   defp traverse({:cond, meta, arguments} = ast, issues, issue_meta, final_condition_value) do
-    conditions =
+    last_cond_clause =
       arguments
       |> Credo.Code.Block.do_block_for!()
       |> List.wrap()
-
-    if conditions
       |> List.last()
-      |> catchall_other_than_value?(final_condition_value) do
-      {ast, issues ++ [issue_for(issue_meta, meta[:line], :cond)]}
-    else
-      {ast, issues}
+
+    case is_catchall_clause_with_invalid_value?(last_cond_clause, final_condition_value) do
+      {true, other_literal} ->
+        {ast, issues ++ [issue_for(issue_meta, final_condition_value, meta[:line], other_literal)]}
+      _ ->
+        {ast, issues}
     end
   end
 
@@ -59,26 +64,46 @@ defmodule Credo.Check.Readability.CondFinalCondition do
     {ast, issues}
   end
 
-  defp catchall_other_than_value?({:->, _meta, [[value], _args]}, value), do: false
+  defp is_catchall_clause_with_invalid_value?({:->, _meta, [[value], _args]}, value), do: false
   # Integer literal catch-all clause
-  defp catchall_other_than_value?({:->, _meta, ['{', _args]}, _value), do: true
+  defp is_catchall_clause_with_invalid_value?({:->, _meta, ['{', _args]}, _value), do: {true, :integer}
   # Binary literal catch-all clause
-  defp catchall_other_than_value?({:->, _meta, [[binary], _args]}, _value) when is_binary(binary), do: true
+  defp is_catchall_clause_with_invalid_value?({:->, _meta, [[binary], _args]}, _value)
+       when is_binary(binary),
+       do: {true, binary}
+
   # List literal catch-all clause
-  defp catchall_other_than_value?({:->, _meta, [[list], _args]}, _value) when is_list(list), do: true
+  defp is_catchall_clause_with_invalid_value?({:->, _meta, [[list], _args]}, _value)
+       when is_list(list),
+       do: {true, list}
+
   # Map literal catch-all clause
-  defp catchall_other_than_value?({:->, _meta, [[{:%{}, _meta2, []}], _args]}, _value), do: true
+  defp is_catchall_clause_with_invalid_value?(
+         {:->, _meta, [[{:%{}, _meta2, _values}], _args]},
+         _value
+       ),
+       do: {true, :map}
+
   # Tuple literal catch-all clause
-  defp catchall_other_than_value?({:->, _meta, [[{:{}, _meta2, _values}], _args]}, _value), do: true
+  defp is_catchall_clause_with_invalid_value?(
+         {:->, _meta, [[{:{}, _meta2, _values}], _args]},
+         _value
+       ),
+       do: {true, :tuple}
+
   # Atom literal catch-all clause
-  defp catchall_other_than_value?({:->, _meta, [[name], _args]}, _value) when is_atom(name), do: true
-  defp catchall_other_than_value?(_clause, _value), do: false
+  defp is_catchall_clause_with_invalid_value?({:->, _meta, [[name], _args]}, _value)
+       when is_atom(name),
+       do: {true, name}
+
+  # Any other cond clause expression
+  defp is_catchall_clause_with_invalid_value?(_clause, _value), do: false
 
-  defp issue_for(issue_meta, line_no, trigger) do
+  defp issue_for(issue_meta, expected_value, line_no, trigger) do
     format_issue(
       issue_meta,
       message:
-      "Cond statements that end with an \"always true\" condition should use `true` instead of some other literal.",
+        "Cond statements that end with an \"always true\" condition should use `#{expected_value}` instead of some other literal.",
       trigger: trigger,
       line_no: line_no
     )
diff --git a/test/credo/check/readability/cond_final_condition_test.exs b/test/credo/check/readability/cond_final_condition_test.exs
index ae1234aba..67fd6352f 100644
--- a/test/credo/check/readability/cond_final_condition_test.exs
+++ b/test/credo/check/readability/cond_final_condition_test.exs
@@ -1,7 +1,7 @@
 defmodule Credo.Check.Readability.CondFinalConditionTest do
   use Credo.Test.Case
 
-  alias Credo.Check.Readability.CondFinalCondition
+  @described_check Credo.Check.Readability.CondFinalCondition
 
   test "it should NOT report conds with a last condition of true" do
     """
@@ -21,7 +21,7 @@ defmodule Credo.Check.Readability.CondFinalConditionTest do
     end
     """
     |> to_source_file()
-    |> run_check(CondFinalCondition)
+    |> run_check(@described_check)
     |> refute_issues()
   end
 
@@ -40,11 +40,11 @@ defmodule Credo.Check.Readability.CondFinalConditionTest do
     end
     """
     |> to_source_file()
-    |> run_check(CondFinalCondition)
+    |> run_check(@described_check)
     |> refute_issues()
   end
 
-  test "it should report conds that with a last condition that is some other literal" do
+  test "it should NOT report conds with a last condition that match the config" do
     """
     defmodule CredoSampleModule do
       def cond_true(a) do
@@ -62,11 +62,11 @@ defmodule Credo.Check.Readability.CondFinalConditionTest do
     end
     """
     |> to_source_file()
-    |> run_check(CondFinalCondition)
-    |> assert_issue()
+    |> run_check(@described_check, value: :else)
+    |> refute_issues()
   end
 
-  test "it should report conds with a last condition that is a binary literal" do
+  test "it should report conds that with a last condition that is some other literal" do
     """
     defmodule CredoSampleModule do
       def cond_true(a) do
@@ -77,18 +77,18 @@ defmodule Credo.Check.Readability.CondFinalConditionTest do
           a + 3 == 5 ->
             "Uh, uh"
 
-          "else" ->
+          :else ->
             "OK"
         end
       end
     end
     """
     |> to_source_file()
-    |> run_check(CondFinalCondition)
+    |> run_check(@described_check)
     |> assert_issue()
   end
 
-  test "it should report conds with a last condition that is an integer literal" do
+  test "it should report conds with a last condition that is a binary literal" do
     """
     defmodule CredoSampleModule do
       def cond_true(a) do
@@ -99,18 +99,18 @@ defmodule Credo.Check.Readability.CondFinalConditionTest do
           a + 3 == 5 ->
             "Uh, uh"
 
-          123 ->
+          "else" ->
             "OK"
         end
       end
     end
     """
     |> to_source_file()
-    |> run_check(CondFinalCondition)
+    |> run_check(@described_check)
     |> assert_issue()
   end
 
-  test "it should report conds with a last condition that is a list literal" do
+  test "it should report conds with a last condition that is an integer literal" do
     """
     defmodule CredoSampleModule do
       def cond_true(a) do
@@ -121,18 +121,18 @@ defmodule Credo.Check.Readability.CondFinalConditionTest do
           a + 3 == 5 ->
             "Uh, uh"
 
-          [:else] ->
+          123 ->
             "OK"
         end
       end
     end
     """
     |> to_source_file()
-    |> run_check(CondFinalCondition)
+    |> run_check(@described_check)
     |> assert_issue()
   end
 
-  test "it should report conds with a last condition that is a tuple literal" do
+  test "it should report conds with a last condition that is a list literal" do
     """
     defmodule CredoSampleModule do
       def cond_true(a) do
@@ -143,18 +143,18 @@ defmodule Credo.Check.Readability.CondFinalConditionTest do
           a + 3 == 5 ->
             "Uh, uh"
 
-          {:else} ->
+          [:else] ->
             "OK"
         end
       end
     end
     """
     |> to_source_file()
-    |> run_check(CondFinalCondition)
+    |> run_check(@described_check)
     |> assert_issue()
   end
 
-  test "it should report conds with a last condition that is a map literal" do
+  test "it should report conds with a last condition that is a tuple literal" do
     """
     defmodule CredoSampleModule do
       def cond_true(a) do
@@ -165,18 +165,18 @@ defmodule Credo.Check.Readability.CondFinalConditionTest do
           a + 3 == 5 ->
             "Uh, uh"
 
-          %{} ->
+          {:else} ->
             "OK"
         end
       end
     end
     """
     |> to_source_file()
-    |> run_check(CondFinalCondition)
+    |> run_check(@described_check)
     |> assert_issue()
   end
 
-  test "it should report conds with a last condition that differ from the config" do
+  test "it should report conds with a last condition that is a map literal" do
     """
     defmodule CredoSampleModule do
       def cond_true(a) do
@@ -187,18 +187,18 @@ defmodule Credo.Check.Readability.CondFinalConditionTest do
           a + 3 == 5 ->
             "Uh, uh"
 
-          true ->
+          %{} ->
             "OK"
         end
       end
     end
     """
     |> to_source_file()
-    |> run_check(CondFinalCondition, final_condition_value: :else)
+    |> run_check(@described_check)
     |> assert_issue()
   end
 
-  test "it should NOT report conds with a last condition that match the config" do
+  test "it should report conds with a last condition that differ from the config" do
     """
     defmodule CredoSampleModule do
       def cond_true(a) do
@@ -209,14 +209,14 @@ defmodule Credo.Check.Readability.CondFinalConditionTest do
           a + 3 == 5 ->
             "Uh, uh"
 
-          :else ->
+          true ->
             "OK"
         end
       end
     end
     """
     |> to_source_file()
-    |> run_check(CondFinalCondition, final_condition_value: :else)
-    |> refute_issues()
+    |> run_check(@described_check, value: :else)
+    |> assert_issue()
   end
 end

From be4f8244f5a8e6e68a3643d32cd00f231150d50d Mon Sep 17 00:00:00 2001
From: Trevor Brown <trevorbrown@rentpath.com>
Date: Fri, 21 Jan 2022 12:36:05 -0500
Subject: [PATCH 7/8] Rename variable for clarity

---
 lib/credo/check/readability/cond_final_condition.ex | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/lib/credo/check/readability/cond_final_condition.ex b/lib/credo/check/readability/cond_final_condition.ex
index 7698ac7fa..5b1932030 100644
--- a/lib/credo/check/readability/cond_final_condition.ex
+++ b/lib/credo/check/readability/cond_final_condition.ex
@@ -40,21 +40,21 @@ defmodule Credo.Check.Readability.CondFinalCondition do
   def run(%SourceFile{} = source_file, params) do
     issue_meta = IssueMeta.for(source_file, params)
 
-    final_condition_value = Params.get(params, :value, __MODULE__)
+    config_catchall_value = Params.get(params, :value, __MODULE__)
 
-    Credo.Code.prewalk(source_file, &traverse(&1, &2, issue_meta, final_condition_value))
+    Credo.Code.prewalk(source_file, &traverse(&1, &2, issue_meta, config_catchall_value))
   end
 
-  defp traverse({:cond, meta, arguments} = ast, issues, issue_meta, final_condition_value) do
+  defp traverse({:cond, meta, arguments} = ast, issues, issue_meta, config_catchall_value) do
     last_cond_clause =
       arguments
       |> Credo.Code.Block.do_block_for!()
       |> List.wrap()
       |> List.last()
 
-    case is_catchall_clause_with_invalid_value?(last_cond_clause, final_condition_value) do
+    case is_catchall_clause_with_invalid_value?(last_cond_clause, config_catchall_value) do
       {true, other_literal} ->
-        {ast, issues ++ [issue_for(issue_meta, final_condition_value, meta[:line], other_literal)]}
+        {ast, issues ++ [issue_for(issue_meta, config_catchall_value, meta[:line], other_literal)]}
       _ ->
         {ast, issues}
     end

From 3534ed73d6b94da95d2dd6ce655295570bc013c2 Mon Sep 17 00:00:00 2001
From: Trevor Brown <trevorbrown@rentpath.com>
Date: Tue, 25 Jan 2022 11:35:10 -0500
Subject: [PATCH 8/8] Set issue trigger to cond clause that triggers the issue

---
 .../check/readability/cond_final_condition.ex | 19 +++++++-----
 .../readability/cond_final_condition_test.exs | 30 ++++++++++++++-----
 2 files changed, 34 insertions(+), 15 deletions(-)

diff --git a/lib/credo/check/readability/cond_final_condition.ex b/lib/credo/check/readability/cond_final_condition.ex
index 5b1932030..045126cda 100644
--- a/lib/credo/check/readability/cond_final_condition.ex
+++ b/lib/credo/check/readability/cond_final_condition.ex
@@ -53,8 +53,10 @@ defmodule Credo.Check.Readability.CondFinalCondition do
       |> List.last()
 
     case is_catchall_clause_with_invalid_value?(last_cond_clause, config_catchall_value) do
-      {true, other_literal} ->
-        {ast, issues ++ [issue_for(issue_meta, config_catchall_value, meta[:line], other_literal)]}
+      {true, ast} ->
+        expression = Macro.to_string(ast)
+        {ast, issues ++ [issue_for(issue_meta, config_catchall_value, meta[:line], expression)]}
+
       _ ->
         {ast, issues}
     end
@@ -66,7 +68,10 @@ defmodule Credo.Check.Readability.CondFinalCondition do
 
   defp is_catchall_clause_with_invalid_value?({:->, _meta, [[value], _args]}, value), do: false
   # Integer literal catch-all clause
-  defp is_catchall_clause_with_invalid_value?({:->, _meta, ['{', _args]}, _value), do: {true, :integer}
+  defp is_catchall_clause_with_invalid_value?({:->, _meta, [[integer], _args]}, _value)
+       when is_integer(integer),
+       do: {true, integer}
+
   # Binary literal catch-all clause
   defp is_catchall_clause_with_invalid_value?({:->, _meta, [[binary], _args]}, _value)
        when is_binary(binary),
@@ -79,17 +84,17 @@ defmodule Credo.Check.Readability.CondFinalCondition do
 
   # Map literal catch-all clause
   defp is_catchall_clause_with_invalid_value?(
-         {:->, _meta, [[{:%{}, _meta2, _values}], _args]},
+         {:->, _meta, [[{:%{}, _meta2, _values} = ast], _args]},
          _value
        ),
-       do: {true, :map}
+       do: {true, ast}
 
   # Tuple literal catch-all clause
   defp is_catchall_clause_with_invalid_value?(
-         {:->, _meta, [[{:{}, _meta2, _values}], _args]},
+         {:->, _meta, [[{:{}, _meta2, _values} = ast], _args]},
          _value
        ),
-       do: {true, :tuple}
+       do: {true, ast}
 
   # Atom literal catch-all clause
   defp is_catchall_clause_with_invalid_value?({:->, _meta, [[name], _args]}, _value)
diff --git a/test/credo/check/readability/cond_final_condition_test.exs b/test/credo/check/readability/cond_final_condition_test.exs
index 67fd6352f..292e9ac91 100644
--- a/test/credo/check/readability/cond_final_condition_test.exs
+++ b/test/credo/check/readability/cond_final_condition_test.exs
@@ -85,7 +85,9 @@ defmodule Credo.Check.Readability.CondFinalConditionTest do
     """
     |> to_source_file()
     |> run_check(@described_check)
-    |> assert_issue()
+    |> assert_issue(fn %Credo.Issue{trigger: trigger} ->
+      assert ":else" == trigger
+    end)
   end
 
   test "it should report conds with a last condition that is a binary literal" do
@@ -107,7 +109,9 @@ defmodule Credo.Check.Readability.CondFinalConditionTest do
     """
     |> to_source_file()
     |> run_check(@described_check)
-    |> assert_issue()
+    |> assert_issue(fn %Credo.Issue{trigger: trigger} ->
+      assert "\"else\"" == trigger
+    end)
   end
 
   test "it should report conds with a last condition that is an integer literal" do
@@ -121,7 +125,7 @@ defmodule Credo.Check.Readability.CondFinalConditionTest do
           a + 3 == 5 ->
             "Uh, uh"
 
-          123 ->
+          12345 ->
             "OK"
         end
       end
@@ -129,7 +133,9 @@ defmodule Credo.Check.Readability.CondFinalConditionTest do
     """
     |> to_source_file()
     |> run_check(@described_check)
-    |> assert_issue()
+    |> assert_issue(fn %Credo.Issue{trigger: trigger} ->
+      assert "12345" == trigger
+    end)
   end
 
   test "it should report conds with a last condition that is a list literal" do
@@ -151,7 +157,9 @@ defmodule Credo.Check.Readability.CondFinalConditionTest do
     """
     |> to_source_file()
     |> run_check(@described_check)
-    |> assert_issue()
+    |> assert_issue(fn %Credo.Issue{trigger: trigger} ->
+      assert "[:else]" == trigger
+    end)
   end
 
   test "it should report conds with a last condition that is a tuple literal" do
@@ -173,7 +181,9 @@ defmodule Credo.Check.Readability.CondFinalConditionTest do
     """
     |> to_source_file()
     |> run_check(@described_check)
-    |> assert_issue()
+    |> assert_issue(fn %Credo.Issue{trigger: trigger} ->
+      assert "{:else}" == trigger
+    end)
   end
 
   test "it should report conds with a last condition that is a map literal" do
@@ -195,7 +205,9 @@ defmodule Credo.Check.Readability.CondFinalConditionTest do
     """
     |> to_source_file()
     |> run_check(@described_check)
-    |> assert_issue()
+    |> assert_issue(fn %Credo.Issue{trigger: trigger} ->
+      assert "%{}" == trigger
+    end)
   end
 
   test "it should report conds with a last condition that differ from the config" do
@@ -217,6 +229,8 @@ defmodule Credo.Check.Readability.CondFinalConditionTest do
     """
     |> to_source_file()
     |> run_check(@described_check, value: :else)
-    |> assert_issue()
+    |> assert_issue(fn %Credo.Issue{trigger: trigger} ->
+      assert "true" == trigger
+    end)
   end
 end