From 8a16ff030602a2bf97d35e72117289ab9143f31e Mon Sep 17 00:00:00 2001 From: Marc-Andre Lafortune Date: Sat, 25 Jun 2022 01:39:50 -0400 Subject: [PATCH] Add `mix xref graph --group` option --- lib/mix/lib/mix/tasks/xref.ex | 105 +++++++++++++++++++++++++-- lib/mix/test/mix/tasks/xref_test.exs | 47 ++++++++++++ 2 files changed, 144 insertions(+), 8 deletions(-) diff --git a/lib/mix/lib/mix/tasks/xref.ex b/lib/mix/lib/mix/tasks/xref.ex index bf4f97829a0..62343314f8c 100644 --- a/lib/mix/lib/mix/tasks/xref.ex +++ b/lib/mix/lib/mix/tasks/xref.ex @@ -78,7 +78,7 @@ defmodule Mix.Tasks.Xref do The following options are accepted: - * `--exclude` - paths to exclude + * `--exclude` - path to exclude. Can be repeated to exclude multiple paths. * `--label` - only shows relationships with the given label. The labels are "compile", "export" and "runtime". By default, @@ -90,6 +90,13 @@ defmodule Mix.Tasks.Xref do with at least one transitive dependency. See "Dependencies types" section below. + * `--group` - provide comma-separated paths to consider as a group. Dependencies + from and into multiple files of the group are considered a single dependency. + Dependencies between the group elements are ignored. This is useful when you + are computing compile and compile-connected dependencies and you want a + series of files to be treated as one. The group is printed using the first path, + with a `+` suffix. Can be repeated to create multiple groups. + * `--only-direct` - keeps only files with the direct relationship given by `--label` @@ -97,10 +104,11 @@ defmodule Mix.Tasks.Xref do Generally useful with the `--sink` flag * `--source` - displays all files that the given source file - references (directly or indirectly) + references (directly or indirectly). Can be repeated to display + references from multiple sources. * `--sink` - displays all files that reference the given file - (directly or indirectly) + (directly or indirectly). Can be repeated. * `--min-cycle-size` - controls the minimum cycle size on formats like `stats` and `cycles` @@ -260,6 +268,7 @@ defmodule Mix.Tasks.Xref do exclude: :keep, fail_above: :integer, format: :string, + group: :keep, include_siblings: :boolean, label: :string, only_nodes: :boolean, @@ -584,6 +593,74 @@ defmodule Mix.Tasks.Xref do ## Graph + defp merge_groups(file_references, comma_separated_groups) do + for group_paths <- comma_separated_groups, + reduce: {file_references, %{}} do + {file_references, aliases} -> + group_paths + |> String.split(",") + |> check_files(file_references, :group) + |> group(file_references, aliases) + end + end + + @type_order %{ + compile: 0, + export: 1, + nil: 2 + } + + # Group the given paths. + # In graph theory vocabulary, this is done by vertex identification + # and removal of edges between contracting vertices. + defp group(paths, file_references, aliases) do + group_name = hd(paths) <> "+" + aliases = paths |> Map.new(&{&1, group_name}) |> Map.merge(aliases) + + # Merge the references *from* the paths to group + {from_group, file_references} = Map.split(file_references, paths) + + file_references = + Map.put(file_references, group_name, merge_references_from_group(from_group)) + + # Remap the references *to* the merged group + file_references = + Map.new(file_references, fn {file, references} -> + {file, remap_references_to_group(references, aliases, group_name)} + end) + + # Remove the resulting reference from the merged group to itself, if there is one + file_references = Map.update!(file_references, group_name, &List.keydelete(&1, group_name, 0)) + + {file_references, aliases} + end + + # Calculate the references from the merged group by concatenating all the references + # from its components; in case of duplicates keep the one with the most important type. + defp merge_references_from_group(file_references_to_merge) do + file_references_to_merge + |> Map.values() + |> Enum.concat() + |> Enum.sort_by(fn {_ref, type} -> @type_order[type] end) + |> Enum.uniq_by(fn {ref, _type} -> ref end) + |> Enum.sort() + end + + defp remap_references_to_group(references, aliases, group_name) do + case Enum.split_with(references, fn {ref, _type} -> Map.has_key?(aliases, ref) end) do + {[], _all_references} -> + references + + {refs_to_merge, other_refs} -> + type = + refs_to_merge + |> Enum.map(fn {_ref, type} -> type end) + |> Enum.min_by(&@type_order[&1]) + + Enum.sort([{group_name, type} | other_refs]) + end + end + defp exclude(file_references, nil), do: file_references defp exclude(file_references, excluded) do @@ -659,14 +736,22 @@ defmodule Mix.Tasks.Xref do end @humanize_option %{ + group: "Group files", source: "Sources", sink: "Sinks", exclude: "Excluded files" } - defp get_files(what, opts, file_references) do - files = Keyword.get_values(opts, what) + defp get_files(what, opts, file_references, aliases) do + files = + for file <- Keyword.get_values(opts, what) do + Map.get(aliases, file, file) + end + + check_files(files, file_references, what) + end + defp check_files(files, file_references, what) do case files -- Map.keys(file_references) do [_ | _] = missing -> Mix.raise("#{@humanize_option[what]} could not be found: #{Enum.join(missing, ", ")}") @@ -679,9 +764,13 @@ defmodule Mix.Tasks.Xref do end defp write_graph(file_references, filter, opts) do - file_references = exclude(file_references, get_files(:exclude, opts, file_references)) - sources = get_files(:source, opts, file_references) - sinks = get_files(:sink, opts, file_references) + {file_references, aliases} = merge_groups(file_references, Keyword.get_values(opts, :group)) + + file_references = + exclude(file_references, get_files(:exclude, opts, file_references, aliases)) + + sources = get_files(:source, opts, file_references, aliases) + sinks = get_files(:sink, opts, file_references, aliases) file_references = cond do diff --git a/lib/mix/test/mix/tasks/xref_test.exs b/lib/mix/test/mix/tasks/xref_test.exs index 89f54527e29..aecc5cfa447 100644 --- a/lib/mix/test/mix/tasks/xref_test.exs +++ b/lib/mix/test/mix/tasks/xref_test.exs @@ -842,6 +842,53 @@ defmodule Mix.Tasks.XrefTest do end) end + test "group with multiple unconnected files" do + assert_graph(~w[--group lib/a.ex,lib/c.ex,lib/e.ex], """ + lib/a.ex+ + |-- lib/b.ex (compile) + `-- lib/d.ex (compile) + lib/b.ex + `-- lib/a.ex+ (compile) + lib/d.ex + `-- lib/a.ex+ + """) + end + + test "group with directly dependent files and cycle" do + assert_graph(~w[--group lib/a.ex,lib/b.ex], """ + lib/a.ex+ + |-- lib/c.ex + `-- lib/e.ex (compile) + lib/c.ex + `-- lib/d.ex (compile) + lib/d.ex + `-- lib/e.ex + lib/e.ex + """) + end + + test "multiple groups" do + assert_graph(~w[--group lib/a.ex,lib/b.ex --group lib/c.ex,lib/e.ex], """ + lib/a.ex+ + `-- lib/c.ex+ (compile) + lib/c.ex+ + `-- lib/d.ex (compile) + lib/d.ex + `-- lib/c.ex+ + """) + end + + test "group with sink" do + assert_graph(~w[--group lib/a.ex,lib/c.ex,lib/e.ex --sink lib/e.ex], """ + lib/b.ex + `-- lib/a.ex+ (compile) + |-- lib/b.ex (compile) + `-- lib/d.ex (compile) + lib/d.ex + `-- lib/a.ex+ + """) + end + @default_files %{ "lib/a.ex" => """ defmodule A do