Refactor UrlId into FilePath enum

[lionel-]

Branched from #1250 (see that PR first for context)
Progress towards #1212

• Rename UrlId to FilePath
• Make it our own custom enum instead of piggybacking on the Url enum. Variants: File (lexically normalised) and Virtual (wrap an Uri e.g. for untitled files). We could add Vendored later for vendored base R sources, mirroring ty.

This refactor better reflects that the main representation for our files are on-disk paths.

[DavisVaughan]

I really like the idea of FilePath, but we are currently in a state where A LOT of existing code is still using url, url_id, or something similar, and I think it results in a net negative in terms of code readability. I have marked a few cases, but have definitely not captured them all. I'd like to advocate for a close review of all uses of FilePath before merging to ensure that the corresponding binding is also called path or file_path.

[DavisVaughan]

I think this should just be from_path() now, because I saw FilePath::from_file_path() elsewhere and was pretty confused by that naming

Or actually, probably from_path_buf() to mimic the AbsPathBuf change

[lionel-]

I pondered these alternatives and went with from_file_path() for consistency with UrlId. Happy to go back on that though.

[DavisVaughan]

Suggested change

	fn canonical_path(url: &FilePath) -> Option<PathBuf> {
	if !url.is_file() {
	return None;
	}
	let path = url.to_file_path().ok()?;
	let path = url.to_path_buf()?;
	std::fs::canonicalize(path).ok()
	}
	fn canonical_path(url: &FilePath) -> Option<PathBuf> {
	match url {
	FilePath::File(path) => std::fs::canonicalize(path.as_path()).ok(),
	FilePath::Virtual(_) => None,
	}
	}

Hmm. I'd like to challenge the addition of the to_path_buf() method.

I understand the desire to have a way to go from FilePath -> PathBuf for compatibility, but I have a strong feeling that it would be better to stay in UTF8PathBuf form as long as possible.

Like, I think this rewrite of canonical_path() is more obvious and takes advantage of UTF8PathBuf features

Can we avoid adding to_path_buf() entirely in favor of rewrites likes this one? And should canonical_path() itself also return another UTF8PathBuf?

[lionel-]

That makes sense, I've removed it

[DavisVaughan]

Put this in lib.rs?

[DavisVaughan]

Suggested change

	let Some(path) = url.to_path_buf() else {
	log::warn!("Skipping add_watched_file: URL is not a file path");
	return;
	};
	let Some(path) = url.as_file() else {
	log::warn!("Skipping add_watched_file: URL is not a file path");
	return;
	};
	let Some(placement) = classify(db, path.as_path().as_std_path()) else {
	// Either the URL falls outside every workspace, or it lives
	// inside a package subdir we don't track (tests/, inst/, ...).
	return;
	};

i would have written as this to avoid the expensive to_path_buf(). no need to make a clone here.

more evidence to me that to_path_buf() is a possible anti pattern

[DavisVaughan]

and really we'd probably make classify() take &Utf8PathBuf