✅ Deploy Preview for docs-rometools canceled.

Built without sensitive environment variables

As pointed out in the comments of type, one thing we wanted to do later on was to implement Copy and/or Clone for Error. Something that it's possible to do with the previous implementation, although it requires more unsafe code.

How flexible/permissive is this proposed solution?

The std::io::Error wrapped in rome_diagnostics::adapters::IoError is not Clone, nor is the Box<dyn std::error::Error> wrapped in rome_diagnostics::adapters::StdError. So neither the current unsafe solution nor this Box<Box<dyn Diagnostic>> solution can be made Clone as written.

However, we could switch to Arc<Box<dyn Diagnostic>> or Box<Arc<dyn Diagnostic>> if we wanted to. Arc is Clone even if its contents aren’t.

Alternatively, if we wrap just std::io::Error and dyn std::error::Error in Arc to make IoError and StdError implement Clone, then we can implement Clone for Error just by adding this method to Diagnostic:

pub trait Diagnostic: Debug {
    …
    fn to_owned_diagnostic<'a>(&self) -> Box<dyn Diagnostic + Send + Sync + 'a>
    where
        Self: Send + Sync + 'a;
}

Proof of concept: andersk@clone-error.

I’m not sure whether there’s an advantage to doing it this way; depends on what you want Clone for I guess. But it’s possible at least.

Great proof of concept @andersk ! Thank you very much for taking the time to look into it!

Having Clone for Error is not a high priority, but there have been cases where they would have been nice to have.

Inside the analyzer infrastructure, we have the concept of "signals". Signals are simply closures stored while analyzing some code, and eventually, we call these closures and use their result. Among these signals, we also have diagnostics, and there have been cases where a .clone() was nice to have, but I managed to make it work.

I think we can hold off on the Clone implementation for now, but it's good to know that we can implement it if needed!