Avoid scanning the filesystem to filter files per check

lib/credo/check/params.ex

@@ -110,13 +110,9 @@ defmodule Credo.Check.Params do
   end
 
   @doc false
-  def files_included(params, check_mod, known_files) do
+  def files_included(params, check_mod) do
     files = get(params, :__files__, check_mod) || get(params, :files, check_mod)
-
-    case files[:included] do
-      nil -> known_files
-      included -> included
-    end
+    files[:included]
   end
 
   @doc false

lib/credo/check/runner.ex

@@ -45,55 +45,14 @@ defmodule Credo.Check.Runner do
   defp do_run_check(exec, {check, params}) do
     rerun_files_that_changed = Params.get_rerun_files_that_changed(params)
 
-    known_files = exec |> Execution.get_source_files() |> Enum.map(& &1.filename)
-    files_included = Params.files_included(params, check, known_files)
+    files_included = Params.files_included(params, check)
     files_excluded = Params.files_excluded(params, check)
 
-    found_relevant_files =
-      cond do
-        files_included == known_files and files_excluded == [] ->
-          []
-
-        exec.config.read_from_stdin ->
-          # TODO: I am unhappy with how convoluted this gets
-          #       but it is necessary to avoid hitting the filesystem when reading from STDIN
-          [%Credo.SourceFile{filename: filename}] = Execution.get_source_files(exec)
-
-          file_included? =
-            if files_included != known_files do
-              Credo.Sources.filename_matches?(filename, files_included)
-            else
-              true
-            end
-
-          file_excluded? =
-            if files_excluded != [] do
-              Credo.Sources.filename_matches?(filename, files_excluded)
-            else
-              false
-            end
-
-          if !file_included? || file_excluded? do
-            :skip_run
-          else
-            []
-          end
-
-        true ->
-          exec
-          |> Execution.working_dir()
-          |> Credo.Sources.find_in_dir(files_included, files_excluded)
-          |> case do
-            [] -> :skip_run
-            files -> files
-          end
-      end
-
     source_files =
       exec
       |> Execution.get_source_files()
       |> filter_source_files(rerun_files_that_changed)
-      |> filter_source_files(found_relevant_files)
+      |> filter_source_files_for_check(files_included, files_excluded)
 
     try do
       check.run_on_all_source_files(exec, source_files, params)
@@ -109,17 +68,25 @@ defmodule Credo.Check.Runner do
     end
   end
 
-  defp filter_source_files(_source_files, :skip_run) do
-    []
-  end
-
   defp filter_source_files(source_files, []) do
     source_files
   end
 
-  defp filter_source_files(source_files, files_included) do
+  defp filter_source_files(source_files, files_that_changed) do
     Enum.filter(source_files, fn source_file ->
-      Enum.member?(files_included, Path.expand(source_file.filename))
+      Enum.member?(files_that_changed, Path.expand(source_file.filename))
+    end)
+  end
+
+  defp filter_source_files_for_check(source_files, nil, []) do
+    source_files
+  end
+
+  defp filter_source_files_for_check(source_files, files_included, files_excluded) do
+    Enum.filter(source_files, fn %{filename: filename} ->
+      included? = is_nil(files_included) or Credo.Sources.filename_matches?(filename, files_included)
+      excluded? = files_excluded != [] and Credo.Sources.filename_matches?(filename, files_excluded)
+      included? and not excluded?
     end)
   end
 

test/credo/check/runner_test.exs

@@ -1,2 +1,182 @@
 defmodule Credo.Check.RunnerTest do
+  use ExUnit.Case, async: true
+
+  alias Credo.Check.Runner
+  alias Credo.Execution
+  alias Credo.Execution.ExecutionSourceFiles
+  alias Credo.SourceFile
+
+  defmodule CollectFilenamesCheck do
+    def scheduled_in_group, do: 1
+
+    def param_defaults do
+      [
+        files: %{
+          included: nil,
+          excluded: []
+        }
+      ]
+    end
+
+    def run_on_all_source_files(_exec, source_files, params) do
+      filenames = Enum.map(source_files, & &1.filename)
+      send(params[:test_pid], {:checked_files, filenames})
+      :ok
+    end
+  end
+
+  describe "files pattern filtering" do
+    test "passes all files when no pattern is configured", %{test_pid: test_pid} do
+      foo = Path.expand("test/fixtures/example_code/foo.ex")
+      clean = Path.expand("test/fixtures/example_code/clean.ex")
+
+      source_files = [
+        %SourceFile{filename: foo},
+        %SourceFile{filename: clean}
+      ]
+
+      :ok =
+        run_checks(Execution.build(), source_files, [
+          {CollectFilenamesCheck, [test_pid: test_pid]}
+        ])
+
+      assert_receive {:checked_files, [^foo, ^clean]}
+    end
+
+    test "passes matching files to the check", %{test_pid: test_pid} do
+      foo = Path.expand("test/fixtures/example_code/foo.ex")
+      config = Path.expand("test/fixtures/custom-config.exs")
+
+      source_files = [
+        %SourceFile{filename: foo},
+        %SourceFile{filename: config}
+      ]
+
+      :ok =
+        run_checks(Execution.build(), source_files, [
+          {CollectFilenamesCheck, [files: %{included: ["test/fixtures/**/*.ex"], excluded: []}, test_pid: test_pid]}
+        ])
+
+      assert_receive {:checked_files, [^foo]}
+    end
+
+    test "passes no files when none match", %{test_pid: test_pid} do
+      source_files = [
+        %SourceFile{filename: Path.expand("test/fixtures/custom-config.exs")}
+      ]
+
+      :ok =
+        run_checks(Execution.build(), source_files, [
+          {CollectFilenamesCheck, [files: %{included: ["test/fixtures/**/*.ex"], excluded: []}, test_pid: test_pid]}
+        ])
+
+      assert_receive {:checked_files, []}
+    end
+
+    test "excludes files matching files.excluded", %{test_pid: test_pid} do
+      foo = Path.expand("test/fixtures/example_code/foo.ex")
+      clean = Path.expand("test/fixtures/example_code/clean.ex")
+
+      source_files = [
+        %SourceFile{filename: foo},
+        %SourceFile{filename: clean}
+      ]
+
+      :ok =
+        run_checks(Execution.build(), source_files, [
+          {CollectFilenamesCheck,
+           [files: %{included: ["test/fixtures/**/*.ex"], excluded: ["**/clean.ex"]}, test_pid: test_pid]}
+        ])
+
+      assert_receive {:checked_files, [^foo]}
+    end
+
+    test "excludes files when only files.excluded is configured", %{test_pid: test_pid} do
+      foo = Path.expand("test/fixtures/example_code/foo.ex")
+      clean = Path.expand("test/fixtures/example_code/clean.ex")
+
+      source_files = [
+        %SourceFile{filename: foo},
+        %SourceFile{filename: clean}
+      ]
+
+      :ok =
+        run_checks(Execution.build(), source_files, [
+          {CollectFilenamesCheck, [files: %{included: nil, excluded: ["**/clean.ex"]}, test_pid: test_pid]}
+        ])
+
+      assert_receive {:checked_files, [^foo]}
+    end
+  end
+
+  describe "stdin filtering" do
+    test "applies file pattern filtering when reading from stdin", %{test_pid: test_pid} do
+      source_files = [%SourceFile{filename: "lib/foo.ex"}]
+
+      :ok =
+        Execution.build()
+        |> Execution.put_config(:read_from_stdin, true)
+        |> run_checks(source_files, [
+          {CollectFilenamesCheck, [files: %{included: ["lib/**/*.ex"], excluded: []}, test_pid: test_pid]}
+        ])
+
+      assert_receive {:checked_files, ["lib/foo.ex"]}
+    end
+
+    test "skips file when it does not match pattern when reading from stdin", %{test_pid: test_pid} do
+      source_files = [%SourceFile{filename: "stdin"}]
+
+      :ok =
+        Execution.build()
+        |> Execution.put_config(:read_from_stdin, true)
+        |> run_checks(source_files, [
+          {CollectFilenamesCheck, [files: %{included: ["lib/**/*.ex"], excluded: []}, test_pid: test_pid]}
+        ])
+
+      assert_receive {:checked_files, []}
+    end
+  end
+
+  describe "rerun filtering" do
+    test "passes only the changed file to the check", %{test_pid: test_pid} do
+      foo = Path.expand("lib/foo.ex")
+      bar = Path.expand("lib/bar.ex")
+
+      source_files = [
+        %SourceFile{filename: foo},
+        %SourceFile{filename: bar}
+      ]
+
+      :ok =
+        run_checks(Execution.build(), source_files, [
+          {CollectFilenamesCheck, [__rerun_files_that_changed__: [foo], test_pid: test_pid]}
+        ])
+
+      assert_receive {:checked_files, [^foo]}
+    end
+
+    test "passes no files when the changed file is not a known source file", %{test_pid: test_pid} do
+      foo = Path.expand("lib/foo.ex")
+      bar = Path.expand("lib/bar.ex")
+      baz = Path.expand("lib/baz.ex")
+
+      source_files = [
+        %SourceFile{filename: foo},
+        %SourceFile{filename: bar}
+      ]
+
+      :ok =
+        run_checks(Execution.build(), source_files, [
+          {CollectFilenamesCheck, [__rerun_files_that_changed__: [baz], test_pid: test_pid]}
+        ])
+
+      assert_receive {:checked_files, []}
+    end
+  end
+
+  defp run_checks(exec, source_files, enabled_checks) do
+    exec = Execution.put_config(exec, :checks, %{enabled: enabled_checks, disabled: []})
+    ExecutionSourceFiles.put(exec, source_files)
+    Runner.run(source_files, exec)
+  end
 end