diff --git a/lib/credo/check/params.ex b/lib/credo/check/params.ex index 7f6683ab5..e250d78a1 100644 --- a/lib/credo/check/params.ex +++ b/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 diff --git a/lib/credo/check/runner.ex b/lib/credo/check/runner.ex index f1c4d47ff..ddd8ece74 100644 --- a/lib/credo/check/runner.ex +++ b/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 diff --git a/test/credo/check/runner_test.exs b/test/credo/check/runner_test.exs index 5c15aa120..b8bbd3b69 100644 --- a/test/credo/check/runner_test.exs +++ b/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