refactor(biome_fs): make paths absolute to correctly match against globs

[Conaclos]

Summary

We recently added a normalization step before matching against glob pattern.
The normalization step strips the working directory from checked evaluated path.
Some passed paths are not absolute making it impossible to properly normalize.

This PR fixes the issue, by converting relative paths passed to the CLI into absolute paths.

I first modified the traverse_inputs function in biome_cli/src/traverse.rs.
The fix was correct, however this makes fails the implementation of MemoryFs that assumes relative paths.
Thus, I moved the fix to OsTraversalScope::evaluate in biome_fs/src/fs/os.rs.
This may make the fix less performant.

It is not the first time I encounter issues with our memory FS.
I wonder if we should not remove this abstraction and directly use a temporary folder for each test.
This could have the nice effect of actually testing real code paths and to identify bugs like the one this PR fixes.

Test Plan

I manually tested the fix.
It isn't possible to test it with memory fs because the change is in the os filesystem.

[codspeed-hq]

CodSpeed Performance Report

Merging #5017 will not alter performance

_{Comparing conaclos/absolutize-paths (bd79c92) with main (9280cba)}

Summary

✅ 94 untouched benchmarks

[arendjr]

I do wonder if we can’t fix MemoryFS instead. I recently wrote a few tests with it where I used absolute paths instead and it worked fine. So I suspect the issue isn’t so much that absolute paths can’t work there, just that many tests are written to assume relative paths. I understand it would be painful to update those tests, but it would give us better coverage and better performance.

[arendjr]

Using unwrap_or() may make it very hard to debug if there is ever an issue with an absolute path, since it means we’ll unexpectedly pass a relative path instead. I’d just use .expect() here.

[ematipico]

I would appreciate we could, at least, add one test that covers the issue we're trying to fix.

We already have tests that use the OS file system:

biome/crates/biome_cli/tests/commands/lint.rs

Lines 896 to 916 in be56c2e

	let _ = remove_dir_all(&root_path);
	create_dir_all(&subdir1_path).unwrap();
	create_dir_all(&subdir2_path).unwrap();
	#[cfg(target_family = "unix")]
	{
	symlink(&subdir2_path, subdir1_path.join("symlink1")).unwrap();
	symlink(subdir1_path, subdir2_path.join("symlink2")).unwrap();
	}
	#[cfg(target_os = "windows")]
	{
	check_windows_symlink!(symlink_dir(&subdir2_path, &subdir1_path.join("symlink1")));
	check_windows_symlink!(symlink_dir(subdir1_path, subdir2_path.join("symlink2")));
	}
	let result = run_cli(
	DynRef::Owned(Box::new(OsFileSystem::new(root_path.clone()))),
	&mut console,
	Args::from([("lint"), (root_path.display().to_string().as_str())].as_slice()),
	);

[Conaclos]

@ematipico
Thanks for the pointer.

However, it is not possible to declare a working directory with run_cli_with_dyn_fs.
I don't know if it is easily doable.
Any suggestion ?

[ematipico]

The argument that you pass to OsFileSystem::new is the working directory

biome/crates/biome_cli/tests/commands/lint.rs

Lines 808 to 811 in 2696970

	let result = run_cli_with_dyn_fs(
	Box::new(OsFileSystem::new(root_path.clone())),
	&mut console,
	Args::from(["lint", root_path.as_ref()].as_slice()),

biome/crates/biome_fs/src/fs/os.rs

Lines 30 to 39 in 7f6d37d

	pub fn new(working_directory: PathBuf) -> Self {
	Self {
	working_directory: Some(working_directory),
	configuration_resolver: AssertUnwindSafe(Resolver::new(ResolveOptions {
	condition_names: vec!["node".to_string(), "import".to_string()],
	extensions: vec![".json".to_string(), ".jsonc".to_string()],
	..ResolveOptions::default()
	})),
	}
	}

[ematipico]

Hopefully this panic won't bite us

[Conaclos]

I do wonder if we can’t fix MemoryFS instead. I recently wrote a few tests with it where I used absolute paths instead and it worked fine. So I suspect the issue isn’t so much that absolute paths can’t work there, just that many tests are written to assume relative paths. I understand it would be painful to update those tests, but it would give us better coverage and better performance.

I agree. Feel free to investigate the case once the PR merged.
I added a FIXME in the code.

[Conaclos]

I added an end-to-end test.

Once the CI passes, I will merge the PR.