More work on better triggers in issues

lib/credo/check.ex

@@ -713,11 +713,13 @@ defmodule Credo.Check do
     line_no = opts[:line_no]
     column = opts[:column]
     severity = opts[:severity] || Severity.default_value()
+    trigger = opts[:trigger]
 
     trigger =
-      case opts[:trigger] do
-        {:__no_trigger__} -> {:__no_trigger__}
-        trigger -> to_string(trigger)
+      if trigger == Issue.no_trigger() do
+        trigger
+      else
+        to_string(trigger)
       end
 
     %Issue{

lib/credo/check/design/duplicated_code.ex

@@ -248,7 +248,7 @@ defmodule Credo.Check.Design.DuplicatedCode do
           issue_meta,
           message: "Duplicate code found in #{filenames} (mass: #{node_mass}).",
           line_no: line_no,
-          trigger: {:__no_trigger__},
+          trigger: Issue.no_trigger(),
           severity: Severity.compute(1 + Enum.count(other_nodes), 1)
         )
       end

lib/credo/check/readability/impl_true.ex

@@ -47,7 +47,7 @@ defmodule Credo.Check.Readability.ImplTrue do
     format_issue(
       issue_meta,
       message: "@impl true should be @impl MyBehaviour",
-      trigger: "@impl true",
+      trigger: "@impl",
       line_no: line_no
     )
   end

lib/credo/check/readability/trailing_blank_line.ex

@@ -42,7 +42,7 @@ defmodule Credo.Check.Readability.TrailingBlankLine do
       issue_meta,
       message: "There should be a final \\n at the end of each file.",
       line_no: line_no,
-      trigger: {:__no_trigger__}
+      trigger: Issue.no_trigger()
     )
   end
 end

lib/credo/check/refactor/pipe_chain_start.ex

@@ -84,7 +84,7 @@ defmodule Credo.Check.Refactor.PipeChainStart do
     if valid_chain_start?(lhs, excluded_functions, excluded_argument_types) do
       {ast, issues}
     else
-      {ast, issues ++ [issue_for(issue_meta, meta[:line], "TODO")]}
+      {ast, issues ++ [issue_for(issue_meta, meta[:line], "|>")]}
     end
   end
 

lib/credo/cli/output/formatter/sarif.ex

@@ -149,6 +149,13 @@ defmodule Credo.CLI.Output.Formatter.SARIF do
         issue.column + String.length(to_string(issue.trigger))
       end
 
+    trigger =
+      if issue.trigger == Credo.Issue.no_trigger() do
+        ""
+      else
+        to_string(issue.trigger)
+      end
+
     rule_and_issue = {
       %{
         "id" => issue.check.id,
@@ -184,7 +191,7 @@ defmodule Credo.CLI.Output.Formatter.SARIF do
                 "startColumn" => issue.column || 1,
                 "endColumn" => column_end,
                 "snippet" => %{
-                  "text" => to_trigger(issue.trigger)
+                  "text" => trigger
                 }
               }
             },
@@ -204,9 +211,6 @@ defmodule Credo.CLI.Output.Formatter.SARIF do
     |> remove_redundant_name(issue.check.id == Credo.Code.Name.full(issue.check))
   end
 
-  defp to_trigger({:__no_trigger__}), do: ""
-  defp to_trigger(trigger), do: to_string(trigger)
-
   defp remove_nil_endcolumn(sarif, false), do: sarif
 
   defp remove_nil_endcolumn(sarif, true) do

lib/credo/issue.ex

@@ -23,4 +23,9 @@ defmodule Credo.Issue do
             # optional: the name of the module, macro or
             #  function where the issue was found
             scope: nil
+
+  @doc """
+  A value that be assigned to `:trigger` if there is no actual trigger.
+  """
+  def no_trigger, do: {:__no_trigger__}
 end

lib/credo/test/case.ex

@@ -181,9 +181,11 @@ defmodule Credo.Test.Case do
   end
 
   defp warn_on_malformed_issues(_source_files, issues) do
+    no_trigger = Credo.Issue.no_trigger()
+
     Enum.each(issues, fn issue ->
       case issue.trigger do
-        {:__no_trigger__} ->
+        ^no_trigger ->
           :ok
 
         trigger when is_nil(trigger) ->

test/credo/check/housekeeping_trigger.exs

@@ -8,7 +8,10 @@ defmodule Credo.Check.HousekeepingHeredocsInTestsTest do
       |> Path.wildcard()
       |> Enum.reject(&String.match?(&1, ~r/(collector|helper)/))
       |> Enum.reject(
-        &String.match?(&1, ~r/(wrong_test_file|unreachable_code|regex_multiple_spaces)/)
+        &String.match?(
+          &1,
+          ~r/(wrong_test_file|unreachable_code|regex_multiple_spaces|regex_empty_character_classes|module_size|case_trivial_matches)/
+        )
       )
       |> Enum.map(&{&1, File.read!(&1)})
       |> Enum.flat_map(fn {filename, source} ->

test/credo/check/readability/impl_true_test.exs

@@ -45,6 +45,8 @@ defmodule Credo.Check.Readability.ImplTrueTest do
     """
     |> to_source_file()
     |> run_check(@described_check)
-    |> assert_issue()
+    |> assert_issue(fn issue ->
+      assert issue.trigger == "@impl"
+    end)
   end
 end

test/credo/check/readability/large_numbers_test.exs

@@ -282,7 +282,10 @@ defmodule Credo.Check.Readability.LargeNumbersTest do
     """
     |> to_source_file
     |> run_check(@described_check)
-    |> assert_issue()
+    |> assert_issue(fn issue ->
+      assert issue.line_no == 2
+      assert issue.trigger == "66000.3"
+    end)
   end
 
   test "it should report trailing digits which are not configured" do

test/credo/check/readability/module_attribute_names_test.exs

@@ -60,6 +60,9 @@ defmodule Credo.Check.Readability.ModuleAttributeNamesTest do
     """
     |> to_source_file
     |> run_check(@described_check)
-    |> assert_issue()
+    |> assert_issue(fn issue ->
+      assert issue.line_no == 2
+      assert issue.trigger == "@someFoobar"
+    end)
   end
 end

test/credo/check/readability/prefer_implicit_try_test.exs

@@ -96,6 +96,9 @@ defmodule Credo.Check.Readability.PreferImplicitTryTest do
     """
     |> to_source_file
     |> run_check(@described_check)
-    |> assert_issue()
+    |> assert_issue(fn issue ->
+      assert issue.line_no == 3
+      assert issue.trigger == "try"
+    end)
   end
 end

test/credo/check/readability/specs_test.exs

@@ -113,6 +113,18 @@ defmodule Credo.Check.Readability.SpecsTest do
     |> refute_issues()
   end
 
+  test "it should NOT report functions with `@impl true`" do
+    """
+    defmodule CredoTypespecTest do
+      @impl true
+      def foo(a), do: a
+    end
+    """
+    |> to_source_file()
+    |> run_check(@described_check)
+    |> refute_issues()
+  end
+
   #
   # cases raising issues
   #
@@ -179,18 +191,8 @@ defmodule Credo.Check.Readability.SpecsTest do
     """
     |> to_source_file()
     |> run_check(@described_check)
-    |> assert_issue()
-  end
-
-  test "it should NOT report functions with `@impl true`" do
-    """
-    defmodule CredoTypespecTest do
-      @impl true
-      def foo(a), do: a
-    end
-    """
-    |> to_source_file()
-    |> run_check(@described_check)
-    |> refute_issues()
+    |> assert_issue(fn issue ->
+      assert issue.trigger == "foo"
+    end)
   end
 end

test/credo/check/readability/trailing_blank_line_test.exs

@@ -25,6 +25,8 @@ defmodule Credo.Check.Readability.TrailingBlankLineTest do
     "defmodule CredoSampleModule do\nend"
     |> to_source_file
     |> run_check(@described_check)
-    |> assert_issue()
+    |> assert_issue(fn issue ->
+      assert issue.trigger == Credo.Issue.no_trigger()
+    end)
   end
 end

test/credo/check/readability/variable_names_test.exs

@@ -241,7 +241,9 @@ defmodule Credo.Check.Readability.VariableNamesTest do
     """
     |> to_source_file
     |> run_check(@described_check)
-    |> assert_issue()
+    |> assert_issue(fn issue ->
+      assert issue.trigger == "otherParam"
+    end)
   end
 
   test "it should report a violation /17" do
@@ -257,7 +259,10 @@ defmodule Credo.Check.Readability.VariableNamesTest do
     """
     |> to_source_file
     |> run_check(@described_check)
-    |> assert_issue()
+    |> assert_issue(fn issue ->
+      assert issue.line_no == 5
+      assert issue.trigger == "someValue"
+    end)
   end
 
   test "it should report multiple violations" do

test/credo/check/readability/with_single_clause_test.exs

@@ -83,6 +83,9 @@ defmodule Credo.Check.Readability.WithSingleClauseTest do
     """
     |> to_source_file
     |> run_check(@described_check)
-    |> assert_issue()
+    |> assert_issue(fn issue ->
+      assert issue.line_no == 3
+      assert issue.trigger == "with"
+    end)
   end
 end

test/credo/check/refactor/double_boolean_negation_test.exs

@@ -27,8 +27,8 @@ defmodule Credo.Check.Refactor.DoubleBooleanNegationTest do
     """
     |> to_source_file
     |> run_check(@described_check)
-    |> assert_issue(fn %Credo.Issue{trigger: trigger} ->
-      assert "!!" == trigger
+    |> assert_issue(fn issue ->
+      assert issue.trigger == "!!"
     end)
   end
 
@@ -38,8 +38,8 @@ defmodule Credo.Check.Refactor.DoubleBooleanNegationTest do
     """
     |> to_source_file
     |> run_check(@described_check)
-    |> assert_issue(fn %Credo.Issue{trigger: trigger} ->
-      assert "!!" == trigger
+    |> assert_issue(fn issue ->
+      assert issue.trigger == "!!"
     end)
   end
 
@@ -58,8 +58,8 @@ defmodule Credo.Check.Refactor.DoubleBooleanNegationTest do
     """
     |> to_source_file
     |> run_check(@described_check)
-    |> assert_issue(fn %Credo.Issue{trigger: trigger} ->
-      assert "not not" == trigger
+    |> assert_issue(fn issue ->
+      assert issue.trigger == "not not"
     end)
   end
 end

test/credo/check/refactor/function_arity_test.exs

@@ -47,7 +47,10 @@ defmodule Credo.Check.Refactor.FunctionArityTest do
     """
     |> to_source_file
     |> run_check(@described_check)
-    |> assert_issue()
+    |> assert_issue(fn issue ->
+      assert issue.line_no == 2
+      assert issue.trigger == "some_function"
+    end)
   end
 
   test "it should report a violation for :unless" do
@@ -60,6 +63,9 @@ defmodule Credo.Check.Refactor.FunctionArityTest do
     """
     |> to_source_file
     |> run_check(@described_check, max_arity: 4)
-    |> assert_issue()
+    |> assert_issue(fn issue ->
+      assert issue.line_no == 2
+      assert issue.trigger == "some_function"
+    end)
   end
 end

test/credo/check/refactor/io_puts_test.exs

@@ -48,7 +48,10 @@ defmodule Credo.Check.Refactor.IoPutsTest do
     """
     |> to_source_file
     |> run_check(@described_check)
-    |> assert_issue()
+    |> assert_issue(fn issue ->
+      assert issue.line_no == 4
+      assert issue.trigger == "IO.puts"
+    end)
   end
 
   test "it should report a violation /3" do
@@ -61,6 +64,9 @@ defmodule Credo.Check.Refactor.IoPutsTest do
     """
     |> to_source_file
     |> run_check(@described_check)
-    |> assert_issue()
+    |> assert_issue(fn issue ->
+      assert issue.line_no == 3
+      assert issue.trigger == "IO.puts"
+    end)
   end
 end

test/credo/check/refactor/long_quote_blocks_test.exs

@@ -104,6 +104,9 @@ defmodule Credo.Check.Refactor.LongQuoteBlocksTest do
     """
     |> to_source_file
     |> run_check(@described_check, max_line_count: 7)
-    |> assert_issue()
+    |> assert_issue(fn issue ->
+      assert issue.line_no == 3
+      assert issue.trigger == "quote"
+    end)
   end
 end

test/credo/check/refactor/match_in_condition_test.exs

@@ -199,6 +199,9 @@ defmodule Credo.Check.Refactor.MatchInConditionTest do
     """
     |> to_source_file
     |> run_check(@described_check)
-    |> assert_issue()
+    |> assert_issue(fn issue ->
+      assert issue.line_no == 3
+      assert issue.trigger == "="
+    end)
   end
 end

test/credo/check/refactor/negated_conditions_in_unless_test.exs

@@ -57,7 +57,10 @@ defmodule Credo.Check.Refactor.NegatedConditionsInUnlessTest do
     """
     |> to_source_file
     |> run_check(@described_check)
-    |> assert_issue()
+    |> assert_issue(fn issue ->
+      assert issue.line_no == 3
+      assert issue.trigger == "!"
+    end)
   end
 
   test "it should report a violation with not" do
@@ -72,6 +75,9 @@ defmodule Credo.Check.Refactor.NegatedConditionsInUnlessTest do
     """
     |> to_source_file
     |> run_check(@described_check)
-    |> assert_issue()
+    |> assert_issue(fn issue ->
+      assert issue.line_no == 3
+      assert issue.trigger == "not"
+    end)
   end
 end