Port changes to gpui2

This commit is contained in:
Kirill Bulatov 2023-11-09 17:18:41 +02:00
parent 2e957bc564
commit f55d89088b
2 changed files with 122 additions and 48 deletions

View file

@ -7,6 +7,7 @@ use lsp::{LanguageServer, LanguageServerId};
use node_runtime::NodeRuntime; use node_runtime::NodeRuntime;
use serde::{Deserialize, Serialize}; use serde::{Deserialize, Serialize};
use std::{ use std::{
ops::ControlFlow,
path::{Path, PathBuf}, path::{Path, PathBuf},
sync::Arc, sync::Arc,
}; };
@ -58,11 +59,17 @@ impl Prettier {
fs: &dyn Fs, fs: &dyn Fs,
installed_prettiers: &HashSet<PathBuf>, installed_prettiers: &HashSet<PathBuf>,
locate_from: &Path, locate_from: &Path,
) -> anyhow::Result<Option<PathBuf>> { ) -> anyhow::Result<ControlFlow<(), Option<PathBuf>>> {
let mut path_to_check = locate_from let mut path_to_check = locate_from
.components() .components()
.take_while(|component| component.as_os_str().to_string_lossy() != "node_modules") .take_while(|component| component.as_os_str().to_string_lossy() != "node_modules")
.collect::<PathBuf>(); .collect::<PathBuf>();
if path_to_check != locate_from {
log::debug!(
"Skipping prettier location for path {path_to_check:?} that is inside node_modules"
);
return Ok(ControlFlow::Break(()));
}
let path_to_check_metadata = fs let path_to_check_metadata = fs
.metadata(&path_to_check) .metadata(&path_to_check)
.await .await
@ -76,14 +83,14 @@ impl Prettier {
loop { loop {
if installed_prettiers.contains(&path_to_check) { if installed_prettiers.contains(&path_to_check) {
log::debug!("Found prettier path {path_to_check:?} in installed prettiers"); log::debug!("Found prettier path {path_to_check:?} in installed prettiers");
return Ok(Some(path_to_check)); return Ok(ControlFlow::Continue(Some(path_to_check)));
} else if let Some(package_json_contents) = } else if let Some(package_json_contents) =
read_package_json(fs, &path_to_check).await? read_package_json(fs, &path_to_check).await?
{ {
if has_prettier_in_package_json(&package_json_contents) { if has_prettier_in_package_json(&package_json_contents) {
if has_prettier_in_node_modules(fs, &path_to_check).await? { if has_prettier_in_node_modules(fs, &path_to_check).await? {
log::debug!("Found prettier path {path_to_check:?} in both package.json and node_modules"); log::debug!("Found prettier path {path_to_check:?} in both package.json and node_modules");
return Ok(Some(path_to_check)); return Ok(ControlFlow::Continue(Some(path_to_check)));
} else if project_path_with_prettier_dependency.is_none() { } else if project_path_with_prettier_dependency.is_none() {
project_path_with_prettier_dependency = Some(path_to_check.clone()); project_path_with_prettier_dependency = Some(path_to_check.clone());
} }
@ -109,7 +116,7 @@ impl Prettier {
}) { }) {
anyhow::ensure!(has_prettier_in_node_modules(fs, &path_to_check).await?, "Found prettier path {path_to_check:?} in the workspace root for project in {project_path_with_prettier_dependency:?}, but it's not installed into workspace root's node_modules"); anyhow::ensure!(has_prettier_in_node_modules(fs, &path_to_check).await?, "Found prettier path {path_to_check:?} in the workspace root for project in {project_path_with_prettier_dependency:?}, but it's not installed into workspace root's node_modules");
log::info!("Found prettier path {path_to_check:?} in the workspace root for project in {project_path_with_prettier_dependency:?}"); log::info!("Found prettier path {path_to_check:?} in the workspace root for project in {project_path_with_prettier_dependency:?}");
return Ok(Some(path_to_check)); return Ok(ControlFlow::Continue(Some(path_to_check)));
} else { } else {
log::warn!("Skipping path {path_to_check:?} that has prettier in its 'node_modules' subdirectory, but is not included in its package.json workspaces {workspaces:?}"); log::warn!("Skipping path {path_to_check:?} that has prettier in its 'node_modules' subdirectory, but is not included in its package.json workspaces {workspaces:?}");
} }
@ -132,7 +139,7 @@ impl Prettier {
} }
None => { None => {
log::debug!("Found no prettier in ancestors of {locate_from:?}"); log::debug!("Found no prettier in ancestors of {locate_from:?}");
return Ok(None); return Ok(ControlFlow::Continue(None));
} }
} }
} }
@ -527,37 +534,40 @@ mod tests {
.await; .await;
assert!( assert!(
matches!(
Prettier::locate_prettier_installation( Prettier::locate_prettier_installation(
fs.as_ref(), fs.as_ref(),
&HashSet::default(), &HashSet::default(),
Path::new("/root/.config/zed/settings.json"), Path::new("/root/.config/zed/settings.json"),
) )
.await .await,
.unwrap() Ok(ControlFlow::Continue(None))
.is_none(), ),
"Should successfully find no prettier for path hierarchy without it" "Should successfully find no prettier for path hierarchy without it"
); );
assert!( assert!(
matches!(
Prettier::locate_prettier_installation( Prettier::locate_prettier_installation(
fs.as_ref(), fs.as_ref(),
&HashSet::default(), &HashSet::default(),
Path::new("/root/work/project/src/index.js") Path::new("/root/work/project/src/index.js")
) )
.await .await,
.unwrap() Ok(ControlFlow::Continue(None))
.is_none(), ),
"Should successfully find no prettier for path hierarchy that has node_modules with prettier, but no package.json mentions of it" "Should successfully find no prettier for path hierarchy that has node_modules with prettier, but no package.json mentions of it"
); );
assert!( assert!(
matches!(
Prettier::locate_prettier_installation( Prettier::locate_prettier_installation(
fs.as_ref(), fs.as_ref(),
&HashSet::default(), &HashSet::default(),
Path::new("/root/work/project/node_modules/expect/build/print.js") Path::new("/root/work/project/node_modules/expect/build/print.js")
) )
.await .await,
.unwrap() Ok(ControlFlow::Break(()))
.is_none(), ),
"Even though it has package.json with prettier in it and no prettier on node_modules along the path, nothing should fail since declared inside node_modules" "Should not format files inside node_modules/"
); );
} }
@ -610,7 +620,7 @@ mod tests {
) )
.await .await
.unwrap(), .unwrap(),
Some(PathBuf::from("/root/web_blog")), ControlFlow::Continue(Some(PathBuf::from("/root/web_blog"))),
"Should find a preinstalled prettier in the project root" "Should find a preinstalled prettier in the project root"
); );
assert_eq!( assert_eq!(
@ -621,8 +631,8 @@ mod tests {
) )
.await .await
.unwrap(), .unwrap(),
Some(PathBuf::from("/root/web_blog")), ControlFlow::Break(()),
"Should find a preinstalled prettier in the project root even for node_modules files" "Should not allow formatting node_modules/ contents"
); );
} }
@ -634,6 +644,18 @@ mod tests {
json!({ json!({
"work": { "work": {
"web_blog": { "web_blog": {
"node_modules": {
"expect": {
"build": {
"print.js": "// print.js file contents",
},
"package.json": r#"{
"devDependencies": {
"prettier": "2.5.1"
}
}"#,
},
},
"pages": { "pages": {
"[slug].tsx": "// [slug].tsx file contents", "[slug].tsx": "// [slug].tsx file contents",
}, },
@ -654,18 +676,16 @@ mod tests {
) )
.await; .await;
let path = "/root/work/web_blog/node_modules/pages/[slug].tsx";
match Prettier::locate_prettier_installation( match Prettier::locate_prettier_installation(
fs.as_ref(), fs.as_ref(),
&HashSet::default(), &HashSet::default(),
Path::new(path) Path::new("/root/work/web_blog/pages/[slug].tsx")
) )
.await { .await {
Ok(path) => panic!("Expected to fail for prettier in package.json but not in node_modules found, but got path {path:?}"), Ok(path) => panic!("Expected to fail for prettier in package.json but not in node_modules found, but got path {path:?}"),
Err(e) => { Err(e) => {
let message = e.to_string(); let message = e.to_string();
assert!(message.contains(path), "Error message should mention which start file was used for location"); assert!(message.contains("/root/work/web_blog"), "Error message should mention which project had prettier defined");
assert!(message.contains("/root/work/web_blog"), "Error message should mention potential candidates without prettier node_modules contents");
}, },
}; };
@ -675,12 +695,37 @@ mod tests {
&HashSet::from_iter( &HashSet::from_iter(
[PathBuf::from("/root"), PathBuf::from("/root/work")].into_iter() [PathBuf::from("/root"), PathBuf::from("/root/work")].into_iter()
), ),
Path::new("/root/work/web_blog/node_modules/pages/[slug].tsx") Path::new("/root/work/web_blog/pages/[slug].tsx")
) )
.await .await
.unwrap(), .unwrap(),
Some(PathBuf::from("/root/work")), ControlFlow::Continue(Some(PathBuf::from("/root/work"))),
"Should return first cached value found without path checks" "Should return closest cached value found without path checks"
);
assert_eq!(
Prettier::locate_prettier_installation(
fs.as_ref(),
&HashSet::default(),
Path::new("/root/work/web_blog/node_modules/expect/build/print.js")
)
.await
.unwrap(),
ControlFlow::Break(()),
"Should not allow formatting files inside node_modules/"
);
assert_eq!(
Prettier::locate_prettier_installation(
fs.as_ref(),
&HashSet::from_iter(
[PathBuf::from("/root"), PathBuf::from("/root/work")].into_iter()
),
Path::new("/root/work/web_blog/node_modules/expect/build/print.js")
)
.await
.unwrap(),
ControlFlow::Break(()),
"Should ignore cache lookup for files inside node_modules/"
); );
} }
@ -704,7 +749,9 @@ mod tests {
}, },
}, },
}, },
"node_modules": {}, "node_modules": {
"test.js": "// test.js contents",
},
"package.json": r#"{ "package.json": r#"{
"devDependencies": { "devDependencies": {
"prettier": "^3.0.3" "prettier": "^3.0.3"
@ -733,9 +780,32 @@ mod tests {
&HashSet::default(), &HashSet::default(),
Path::new("/root/work/full-stack-foundations/exercises/03.loading/01.problem.loader/app/routes/users+/$username_+/notes.tsx"), Path::new("/root/work/full-stack-foundations/exercises/03.loading/01.problem.loader/app/routes/users+/$username_+/notes.tsx"),
).await.unwrap(), ).await.unwrap(),
Some(PathBuf::from("/root/work/full-stack-foundations")), ControlFlow::Continue(Some(PathBuf::from("/root/work/full-stack-foundations"))),
"Should ascend to the multi-workspace root and find the prettier there", "Should ascend to the multi-workspace root and find the prettier there",
); );
assert_eq!(
Prettier::locate_prettier_installation(
fs.as_ref(),
&HashSet::default(),
Path::new("/root/work/full-stack-foundations/node_modules/prettier/index.js")
)
.await
.unwrap(),
ControlFlow::Break(()),
"Should not allow formatting files inside root node_modules/"
);
assert_eq!(
Prettier::locate_prettier_installation(
fs.as_ref(),
&HashSet::default(),
Path::new("/root/work/full-stack-foundations/exercises/03.loading/01.problem.loader/node_modules/test.js")
)
.await
.unwrap(),
ControlFlow::Break(()),
"Should not allow formatting files inside submodule's node_modules/"
);
} }
#[gpui::test] #[gpui::test]

View file

@ -69,7 +69,7 @@ use std::{
hash::Hash, hash::Hash,
mem, mem,
num::NonZeroU32, num::NonZeroU32,
ops::Range, ops::{ControlFlow, Range},
path::{self, Component, Path, PathBuf}, path::{self, Component, Path, PathBuf},
process::Stdio, process::Stdio,
str, str,
@ -8488,7 +8488,10 @@ impl Project {
}) })
.await .await
{ {
Ok(None) => { Ok(ControlFlow::Break(())) => {
return None;
}
Ok(ControlFlow::Continue(None)) => {
match project.update(&mut cx, |project, _| { match project.update(&mut cx, |project, _| {
project project
.prettiers_per_worktree .prettiers_per_worktree
@ -8520,7 +8523,7 @@ impl Project {
.shared())), .shared())),
} }
} }
Ok(Some(prettier_dir)) => { Ok(ControlFlow::Continue(Some(prettier_dir))) => {
match project.update(&mut cx, |project, _| { match project.update(&mut cx, |project, _| {
project project
.prettiers_per_worktree .prettiers_per_worktree
@ -8662,7 +8665,7 @@ impl Project {
.await .await
}) })
} }
None => Task::ready(Ok(None)), None => Task::ready(Ok(ControlFlow::Break(()))),
}; };
let mut plugins_to_install = prettier_plugins; let mut plugins_to_install = prettier_plugins;
let previous_installation_process = let previous_installation_process =
@ -8692,8 +8695,9 @@ impl Project {
.context("locate prettier installation") .context("locate prettier installation")
.map_err(Arc::new)? .map_err(Arc::new)?
{ {
Some(_non_default_prettier) => return Ok(()), ControlFlow::Break(()) => return Ok(()),
None => { ControlFlow::Continue(Some(_non_default_prettier)) => return Ok(()),
ControlFlow::Continue(None) => {
let mut needs_install = match previous_installation_process { let mut needs_install = match previous_installation_process {
Some(previous_installation_process) => { Some(previous_installation_process) => {
previous_installation_process.await.is_err() previous_installation_process.await.is_err()