Skip to content

Conversation

@demerphq
Copy link
Contributor

@demerphq demerphq commented Mar 13, 2025

This patch adds support for output the xref graph as a two level Map of Maps which can be trivially loaded by anything that can process JSON data. The top level Map contains keys which specify source files whose values are maps contain sink files as keys, and dependency type data as values. Source files with no dependencies have empty maps as values. For example a if "lib/foo.ex" has a compile time dependency on "lib/bar.ex" which had no dependencies at all, then the output would look like this:

   {
     "lib/foo.ex": { "lib/bar.ex": "compile" },
     "lib/bar.ex": {},
   }

At the same time it adds support for renaming existing xref_graph.dot and xref_graph.json files to have a .bak extension instead of overwriting them.

This patch also includes logic to fix some misleading verbiage output by the 'dot' format when the output file is not in the current working directory. This change is included with the JSON changes because I refactored the logic for writing to a file so that both the 'json' and 'dot' formats would use the same code.

This patch also includes some new test functions to make it easier to test the behavior of the 'dot' and 'json' functions. It also includes documentation changes to reflect the above changes.

This patch fixes the bugs reported in #14324, with the exception that it does not include a way to control/silence the additional verbiage produced by the 'dot' format.

Copy link
Contributor

@amalbuquerque amalbuquerque left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a small nit 💅 Thank you for this!

@demerphq demerphq force-pushed the yves/mix_xref_graph_json branch from 68e3e90 to 2564779 Compare March 13, 2025 14:14
This patch adds support for output the xref graph as a two level Map of
Maps which can be trivially loaded by anything that can process JSON
data. The top level Map contains keys which specify source files whose
values are maps contain sink files as keys, and dependency type data as
values. Source files with no dependencies have empty maps as values. For
example a if "lib/foo.ex" has a compile time dependency on "lib/bar.ex"
which had no dependencies at all, then the output would look like this:

   {
     "lib/foo.ex": { "lib/bar.ex": "compile" },
     "lib/bar.ex": {},
   }

At the same time it adds support for renaming existing xref_graph.dot
and xref_graph.json files to have a .bak extension instead of
overwriting them.

This patch also includes logic to fix some misleading verbiage output by
the 'dot' format when the output file is not in the current working
directory. This change is included with the JSON changes because I
refactored the logic for writing to a file so that both the 'json' and
'dot' formats would use the same code.

This patch also includes some new test functions to make it easier to
test the behavior of the 'dot' and 'json' functions. It also includes
documentation changes to reflect the above changes.

This patch fixes the bugs reported in elixir-lang#14324, with the exception that it
does not include a way to control/silence the additional verbiage
produced by the 'dot' format.
@demerphq demerphq force-pushed the yves/mix_xref_graph_json branch from 19ad49d to 427baf9 Compare March 14, 2025 22:57
@demerphq
Copy link
Contributor Author

Ok, all requested changes made. Also added some additional docs and fixed one or two additional nits I noticed as I did the other requested changes.

Copy link
Member

@josevalim josevalim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @demerphq! I did another pass, now we have mostly some nitpicks left and some improvements to the new test :)

Will be rebased away later.

Co-authored-by: José Valim <[email protected]>
Copy link
Contributor Author

@demerphq demerphq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Ill follow up on the remaining points and squash it down and repush shortly.

@demerphq
Copy link
Contributor Author

All feedback applied. To make it easier to review I didn't squash it down yet. Only thing i wasnt sure about was the way I did the capture_io() test. It made sense to me to wrap it in an in_tmp() call, on the premise that if the write to STDOUT for some reason didn't occur and the write went to the default file instead we would clean it up properly. Let me know if you think that was overkill.

@demerphq
Copy link
Contributor Author

Ok, those tweaks are done. if you are good with that, I'll squash it down to a single commit.

@josevalim
Copy link
Member

No worries, I can squash when merging. If CI passes, I will ship it!

@josevalim josevalim merged commit 37b6f0b into elixir-lang:main Mar 17, 2025
8 of 10 checks passed
@josevalim
Copy link
Member

💚 💙 💜 💛 ❤️

@demerphq demerphq deleted the yves/mix_xref_graph_json branch March 17, 2025 09:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants