Introduce Canonicalize

[FintanH]

Fixes #76

Refactor of File to be the combination of Directory and the file name,where Directory is the Vector of component paths.

The refactor meant changing some sections of the code where we were
parsing and manipulating Files/Directories.

This also includes a new trait Canonicalization which is needed for
import logic.

This is just the introduction of the trait, but calling it needs to be added as well. From what I could see that's involved with import chaining, is that right? Do we have that yet?

[Nadrieril]

Overall looks good !
I do have one big question : why is the file/directory distinction useful ? Since empty paths are allowed, we may have paths without a file, so what's the benefit of keeping the last component separatedly ?

[Nadrieril]

I believe you could use as_ref instead of clone here, since canonicalize only needs a reference.

[Nadrieril]

Since this feature isn't tested by the tests from dhall-lang, could you add some tests here ? You don't need to test every case, but a few basic ones would ensure it works and we don't accidentally break it in the future.

[FintanH]

I think I just need to add the tests, and we're good to go here.

[FintanH]

Just to check I'm on the right page. I have a skeleton test where ? is place holder for values:

    #[test]
    test_resolve_import_local() {
        let import = Import {
            mode: RawText,
            location: Local(?FilePrefix, FilePath { file_path: ?file_path }),
            hash: None,
        };
        let root = LocalDir(?cwd);
        let import_cache = HashMap::new();
        let import_stack = vec![];
        let import_result = resolve_import(import, root, import_cache, import_stack);
        assert_eq!(import_result, Ok(?some_result));
    }

[Nadrieril]

Oh I'm so sorry I missed this comment >< I was thinking more simply to just test canonicalize. For example

#[test]
test_canonicalize_whatever() {
    let filepath = FilePath {filepath = vec!["foo", "..", "bar"]};
    let expected = FilePath {filepath = vec!["bar"]};
    assert_eq!(filepath.canonicalize(), expected);
}

[Nadrieril]

Yeah sorry, that really wasn't clear from the diff this comment is pointing to

[Nadrieril]

@FintanH Need a hand with the code or with reading the semantics ?

[FintanH]

@Nadrieril: thanks for checking in ❤️ Sorry I've left this so long, got snapped up with switching jobs and what not. I have some time off now and fully on the Rust train 🚂 I'll look at this during the week and possibly a bit at MuniHac 😁

[Nadrieril]

@FintanH Since the parser changed completely in #110, I took the liberty of merging the latest master into your PR so that you don't need do to it yourself

[FintanH]

@Nadrieril I have a weird one:

error[E0308]: mismatched types
   --> dhall_generated_parser/build.rs:100:39
    |
100 |         pest_generator::derive_parser(pest, false)
    |                                       ^^^^ expected struct `proc_macro2::TokenStream`, found a different struct `proc_macro2::TokenStream`
    |
    = note: expected type `proc_macro2::TokenStream` (struct `proc_macro2::TokenStream`)
               found type `proc_macro2::TokenStream` (struct `proc_macro2::TokenStream`)
note: Perhaps two different versions of crate `proc_macro2` are being used?
   --> dhall_generated_parser/build.rs:100:39
    |
100 |         pest_generator::derive_parser(pest, false)
    |                                       ^^^^

error: aborting due to previous error

I did a cargo clean, cargo update, and cargo build is throwing that error 🤔

[Nadrieril]

Ah yeah it seems there was a conflict between the version of proc_macro2 used by pest and the one used by quote. This should be fixed on master now

[Nadrieril]

Actually we may be able to test canonicalization with normal tests using as Location.
Anyways this PR is fine and tests are not that important for now, so I'll just merge it.
Thanks a lot, and sorry for the wait !

[FintanH]

Thanks! Sorry for my lack of work on it towards the end 😅 been very busy!