Use failure for error handling

[ebkalderon]

Added

• Add dependency on failure crate.

Changed

• Use String instead of &'a str in ParseError and LicenseExpr to satisfy 'static bound on Fail trait.
• Eliminate use of error_chain in fetch-license-from-spdx example; use failure::Error instead.
• Run rustfmt on codebase.

[wking]

I don't know enough Rust to evaluate this. Is failure better-maintained than error-chain? Or does it provide a more convenient API to consumers? I'm hesitant about adding a new runtime dependency without a clear idea of what we're improving.

[ebkalderon]

@wking I'd say a bit of both. The entire ecosystem is encouraged to move toward failure over error-chain. The crate itself was designed by many of the same developers and is meant to eventually supersede it entirely. Plus, it is hosted in rust-lang-nursery and is currently seeking API stabilization with 1.0.

See the original announcement by @withoutboats and the nursery repo for more details. Also, please take a look at rust-lang-deprecated/failure#181.

[Nemo157]

Just a note: this would be a breaking change. Seeing as the cargo registry is the only public crate that currently depends on this I don't think releasing a new minor version would be too bad though.

[Nemo157]

Actually, I'm highly against changing the interface to clone the substrings, there's no reason the parsed license expression can't borrow back into the input string.

EDIT: Especially when the only public API doesn't even give you access to the parsed expression currently, so you're wasting all this work cloning the substrings then just discarding them.

[ebkalderon]

Part of the problem with returning substrings with lifetimes in the error types is that I cannot easily use them with other projects that implement failure or interface with it, as Fail: 'static. The substrings cannot be easily converted to Box<Error + 'static>, passed around, and downcasted. I am currently abstracting over license-exprs in the form of a License newtype with serde integration, and the errors requiring non-static lifetimes is a pain in my backside. Do you happen to have another solution to this?

[Nemo157]

The LicenseExpr enums could be changed to use Cow for storage, the errors then can include a LicenseExpr<'static> and when an error occurs the failing syntax tree can be promoted from Cow::Borrowed to Cow::Owned. This means when you're on the happy path everything stays with nice cheap borrowed data, and only once you fail for some reason do you have to spend the time cloning all the substrings.

[ebkalderon]

That sounds like an excellent solution! It doesn't lock the crate to using failure, but it certainly helps facilitate its use with software that does. Would you like to open a new pull request for this change, or should I? Either way, I think this PR can be closed once the changes have been proposed.

[Nemo157]

I'm going to be trying to replace the current parser with one that allows getting access to the actual parsed expression this week, I'll definitely keep using Cow for the parsed tree and errors in mind while I work on that. If you're fine with waiting for that work to be complete then that's one solution.

[ebkalderon]

Sure! It sounds like important work is being done; that's terrific. If it should likely be completed sometime this week, then I'd be happy to wait. We can close this PR once the other one gets merged. Thanks for the heads up.

[carols10cents]

Doing some cleanup; it looks like this change is stale and not what's ultimately desired anyway, so I'm closing this PR. Thank you!

[ebkalderon]

@carols10cents Thanks for the cleanup! I hope that the parser and error handling improvements are incorporated sometime so this crate can be more useful to the public at large.