Fix formatting performance problems with .formatter.exs in subdirectories

[jonleighton]

When dealing with .formatter.exs files that are in subdirectories of the
main project, the behaviour was incorrect:
#361

And also caused performance problems:
#381

A good example is a project I currently work on. We have a top-level
.formatter.exs, as well as a test/.formatter.exs.

The one in test/ has the :inputs option set to ["**/*.{ex,exs}"].
Before this commit, should_format?/3 would take this pattern and
concatenate it with the project directory. This would result in
Path.wildcard/2 globbing for all source files in the entire project, rather
than just under test/. This causes formatting to take about 5 seconds on
our project.

This commit fixes the problem by taking the file to be formatted, and
finding the .formatter.exs file that exists in the closest parent
directory (so long as that directory is still within the project root). We
then perform the Path.wildcard/2 relative to that directory.

I’ve removed the hack that was in place to support umbrella projects,
because this is a more general solution to the problem.

On our project, I am now able to format a file in 100ms or so, which isn’t
a bad speed-up!

[jonleighton]

Thanks for the review @lukaszsamson 🙇

[jonleighton]

I've updated the branch to address your comments @lukaszsamson

[jonleighton]

Ok, I've added one more commit which avoids the filesystem traversal entirely:

• I wrote a library called path_glob that implements matching for globs without interacting with the filesystem
• I've implemented it by writing a parser with nimble_parsec, so I believe it to be fairly robust. I wrote a lot of test cases and fixed lots of edge cases.
• That said, it's a new library so more testing/review is welcome. But I'm pretty sure it will handle the most common use cases without trouble.
• The new commit just drops in this library.

If there is a concern about maintenance, I'd be happy for the library to be hosted under the elixir-lsp org.

[arnodirlam]

Thank you for all the effort you already put into this proper solution, @jonleighton!

Looking forward to having this released, and have migrations formatted on Save with the phoenix default .formatter.exs files (#361) 🙌 🚀

[axelson]

Thanks! This approach looks like a serious improvement over what we were doing previously 🎉

I haven't taken an in-depth look at the code yet (but I plan to soon!) but we will want to vendor path_glob (similar to what we did with jason) in case a user tries to use a different version of path_glob in their project. And since I'd like to avoid vendoring nimble_parsec as well (since it is larger and more widely used) I'm submitting a couple PR's:

• Make nimble_parsec runtime: false jonleighton/path_glob#1
• Skip runtime_false deps mix_task_archive_deps#1

[jonleighton]

@axelson Thanks for that, I've merged your PR and released v0.1.1 with it.

I'll just note for completeness that I had a chat with José Valim about adding a Path.match?() function to Elixir's standard library. He was keen on the idea but said that because Path.wildcard() is built on Erlang's filelib, it would need to be added to Erlang first. It turns out that there is a private function in filelib that provides a parse tree for a glob expression so it wouldn't require implementing a parser, just converting the parse tree into a regex. (I didn't realise this when writing path_glob... but it was a fun exploration of nimble_parsec anyway.)

I put that in the "maybe one day" basket. I don't think I'm going to make time to do that in the near future but it would be a good addition to the standard library I think and I understand why he doesn't want to just include/maintain a whole new parser in Elixir. In any case, elixir_ls couldn't use it until the minimum Elixir version supported includes this function.

[axelson]

@jonleighton Thanks for that write-up and additional detail. I just opened #628 once we've tested and merged that I think this PR should be good to go!

[axelson]

This is great! Thanks! ❤️