github / semantic

Parsing, analyzing, and comparing source code across many languages
8.94k stars 454 forks source link

Rethink approach to typed paths #616

Open patrickt opened 4 years ago

patrickt commented 4 years ago

We’ve been using the pathtype package to provide type-level distinctions between absolute/relative file/directory paths. Though in theory this is a better solution than plain old FilePath, which just wraps a string. Though using pathtype has uncovered some weird semantics, the journey has not been particularly smooth, and has recently worsened (see point 7 below). Here are some of the things that I don’t like about pathtype:

  1. The API is weird and inconsistent.
  2. Smart constructors call error on failure, which is hard to track down without stack traces or without mucking about in ghci.
  3. The documentation is extremely poorly organized.
  4. It’s never clear when to write a function that is polymorphic in its abs/rel type or to use something like AbsRelFile (which in theory represents either absolute or relative paths).
  5. The library is largely unmaintained.
  6. External packages like Glob don’t interface with it without a lot of unchecked smart constructor calls.
  7. Most notably, the introduction of Bazel caused a schism in how we access fixtures. Bazel copies all the files it needs into a runfiles directory; access to those files is computed using the bazel-runfiles package, which normalizes what look like relative paths into absolute paths inside /tmp. However, this is a problem when trying to retain cabal compatibility: in those circumstances, normalization is not necessary, and indeed not desired: we just want to use paths relative to $PWD. I’ve hacked around this by implementing System.Path.Fixture, which uses CPP and ImplicitParams to try to do the right thing across cabal and bazel invocations. This kind of works but is a pain in the ass to debug when you encounter point 2.
  8. The above means that most paths to fixtures are of type AbsRelFile—something that is probably a file but either absolute or relative—and imposing that everywhere is not hugely helpful. A FixturePath type or something could perhaps paper over this.

I lost a good chunk of this morning tooling around in ghci trying to find where pathtype was calling error. Though I was indeed holding the code wrong—there was a bug in my Fixture module—it was boring and frustrating.

I can envision a few approaches we could take:

  1. Do nothing—it’s not bad, it’s just not good, you know?
  2. Say “to heck with it” and reverting back to FilePath everywhere.
  3. Try out one of the alternative libraries like path.
  4. Institute our own solution. I have done something like this already.

I feel bad for introducing this complexity and then changing my mind 😩 I beg forgiveness!