Create CondFinalCondition readability check

lib/credo/check/readability/cond_final_condition.ex

@@ -0,0 +1,86 @@
+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 the statement
+      should be the literal `true`, or the literal value specified in this
+      check's `final_condition_value` parameter.
+
+      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
+      """,
+      params: [
+        final_condition_value: "Set the expected value for the final condition"
+      ]
+    ]
+
+  @doc false
+  @impl true
+  def run(%SourceFile{} = source_file, params) do
+    issue_meta = IssueMeta.for(source_file, params)
+
+    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, final_condition_value) do
+    conditions =
+      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}
+    end
+  end
+
+  defp traverse(ast, issues, _issue_meta, _min_pipeline_length) do
+    {ast, issues}
+  end
+
+  defp catchall_other_than_value?({:->, _meta, [[value], _args]}, value), do: false
+  # Integer literal catch-all clause
+  defp catchall_other_than_value?({:->, _meta, ['{', _args]}, _value), do: true
+  # Binary literal catch-all clause
+  defp catchall_other_than_value?({:->, _meta, [[binary], _args]}, _value) when is_binary(binary), do: true
+  # List literal catch-all clause
+  defp catchall_other_than_value?({:->, _meta, [[list], _args]}, _value) when is_list(list), do: true
+  # Map literal catch-all clause
+  defp catchall_other_than_value?({:->, _meta, [[{:%{}, _meta2, []}], _args]}, _value), do: true
+  # Tuple literal catch-all clause
+  defp catchall_other_than_value?({:->, _meta, [[{:{}, _meta2, _values}], _args]}, _value), do: true
+  # 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 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

test/credo/check/readability/cond_final_condition_test.exs

@@ -0,0 +1,222 @@
+defmodule Credo.Check.Readability.CondFinalConditionTest do
+  use Credo.Test.Case
+
+  alias Credo.Check.Readability.CondFinalCondition
+
+  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(CondFinalCondition)
+    |> 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(CondFinalCondition)
+    |> 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(CondFinalCondition)
+    |> 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(CondFinalCondition)
+    |> 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(CondFinalCondition)
+    |> 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(CondFinalCondition)
+    |> 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(CondFinalCondition)
+    |> 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(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