Add a Source::with_path method to set the path on a Source

[hansl]

This should allow any path to be added to a Source, even when using from_bytes or from_reader.

[codecov]

Codecov Report

Attention: Patch coverage is 0% with 3 lines in your changes missing coverage. Please review.

Project coverage is 51.57%. Comparing base (6ddc2b4) to head (f9ff7c0).
Report is 223 commits behind head on main.

Files	Patch %	Lines
core/parser/src/source/mod.rs	0.00%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3941      +/-   ##
==========================================
+ Coverage   47.24%   51.57%   +4.32%     
==========================================
  Files         476      468       -8     
  Lines       46892    44963    -1929     
==========================================
+ Hits        22154    23188    +1034     
+ Misses      24738    21775    -2963     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[nekevss]

nit: doc might be a bit unclear wording wise. As it's not adding the path to the current Source, but is creating a new Source from the combination.

Suggested change

	/// Add a path to the current [`Source`] instance.
	/// Returns a new source from the current [`Source`] instance combined with the provided `Path`.

[hansl]

It takes ownership of the source, so it's used like any with_ method. I can use the same wording as JsError::with_message; Sets the path of this Source.

[nekevss]

Ah, I've been dealing with different with_ methods recently with temporal 😆 I guess I'm a bit curious about taking self vs. mut self in this instance.

Basically, if I'm reading this correctly, this method could probably be:

// ... impl block

    pub fn with_path(mut self, new_path: &'path Path) -> Self {
        let _ = self.path.insert(new_path);
        self
    }

// cont ...

I think the above tends to be more in line with convention, and we also aren't potentially dropping the previous self, granted maybe rustc optimizes that out and it doesn't matter that much.

[jedel1043]

@nekevss Uhh I think your method doesn't work in general because it doesn't change the lifetime of Source to be the same as the lifetime of Path. It would (probably) work with lifetimes that are sub-lifetimes of 'path, but we don't want to add that arbitrary restriction.

[nekevss]

True. I think I'm getting caught up on a bit of a bikeshed around the name/behavior of this method.

😕 if we are destroying Source anyways, then wouldn't it be a bit clearer convention wise for the method to be a to_ (something like to_source_with_path)

[jedel1043]

I think using with_* methods that take self is pretty common e.g. builder types. I don't see a problem with using the same convention for this.

[jedel1043]

Looks good!