mirror of
https://github.com/zed-industries/zed.git
synced 2026-06-01 03:14:56 +07:00
Fix "Run Debugger" failing silently when project does not compile (#52439)
## Context Previously the Run Debugger gutter arrow would fail silently when the Cargo.toml had garbage lines such as “asdfasdf”. This fix makes it so that the error is detected and bubbles up to the editor, which will notify the user with a toast diagnostic. Closes #46716 ## Fix https://github.com/user-attachments/assets/2e9ac7e9-1306-4607-a762-457131473572 ## How to Review Small PR - focused on four different files: In - `crates/languages/src/rust.rs`: - `target_info_from_abs_path()` - The function signature was changed from `Option<(Option<TargetInfo>, Arc<Path>)>` to `Result<Option<(Option<TargetInfo>, Arc<Path>)>>`. A condition was added to ensure that if the Cargo metadata command is unsuccessful, the function returns an error instead of causing an EOF error while deserializing the stdout of the command. - `build_context()` - Added a `?` in `target_info_from_abs_path(path, project_env.as_ref()).await` in order to return the error. In - `crates/project/src/task_store.rs`: - `local_task_context_for_location()` and `remote_task_context_for_location()` - The functions signatures were changed from `Task<Option<TaskContext>>` to `Task<anyhow::Result<Option<TaskContext>>>` for the purpose of propagating the error. In - `crates/editor/src/editor_tests.rs`: - `build_tasks_context()` - The function signature was changed from `Task<Option<TaskContext>>` to `Task<anyhow::Result<Option<TaskContext>>>` . - `toggle_code_actions()` - In case `build_tasks_context()` fails, the functions notifies the error to the user as a Toast notification. In - `crates/editor/src/runnables.rs`: - Since `build_tasks_context()` and `task_store.task_context_for_location()` now return a Result, the callers` spawn_nearest_task() `and `task_context()` were modified. The resulting Result types are transformed to match the expected return types of `TaskContext` and `Task<Option<TaskContext>>` Two new tests were added. The first, `target_info_from_abs_path_failed` in `crates/languages/src/rust.rs`, checks if the system properly catches the error. The second, `test_toggle_code_actions_build_tasks_context_error_notifies` in `crates/editor/src/editor_tests.rs`, confirms that the editor triggers the expected error notification. ## Self-Review Checklist - [X] I've reviewed my own diff for quality, security, and reliability - [ ] Unsafe blocks (if any) have justifying comments - [X] The content is consistent with the [UI/UX checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) - [X] Tests cover the new/changed behavior - [X] Performance impact has been considered and is acceptable Release Notes: - Fixed the error: Run Debugger failing silently due to invalid Cargo.toml content --------- Co-authored-by: Lukas Wirth <lukas@zed.dev>
This commit is contained in:
parent
d320e0b768
commit
ae08089415
7 changed files with 158 additions and 32 deletions
1
Cargo.lock
generated
1
Cargo.lock
generated
|
|
@ -9791,6 +9791,7 @@ dependencies = [
|
|||
"smol",
|
||||
"snippet",
|
||||
"task",
|
||||
"tempfile",
|
||||
"terminal",
|
||||
"theme",
|
||||
"tree-sitter",
|
||||
|
|
|
|||
|
|
@ -225,7 +225,7 @@ use workspace::{
|
|||
OpenTerminal, Pane, RestoreOnStartupBehavior, SERIALIZATION_THROTTLE_TIME, SplitDirection,
|
||||
TabBarSettings, Toast, ViewId, Workspace, WorkspaceId, WorkspaceSettings,
|
||||
item::{ItemBufferKind, ItemHandle, PreviewTabsSettings, SaveOptions},
|
||||
notifications::{DetachAndPromptErr, NotificationId, NotifyTaskExt},
|
||||
notifications::{DetachAndPromptErr, NotificationId, NotifyResultExt, NotifyTaskExt},
|
||||
searchable::SearchEvent,
|
||||
};
|
||||
pub use zed_actions::editor::RevealInFileManager;
|
||||
|
|
@ -7055,7 +7055,8 @@ impl Editor {
|
|||
let runnable_task = match deployed_from {
|
||||
Some(CodeActionSource::Indicator(_)) => Task::ready(Ok(Default::default())),
|
||||
_ => {
|
||||
let mut task_context_task = Task::ready(None);
|
||||
let mut task_context_task = Task::ready(Ok(None));
|
||||
let workspace = self.workspace().map(|w| w.downgrade());
|
||||
if let Some(tasks) = &tasks
|
||||
&& let Some(project) = project
|
||||
{
|
||||
|
|
@ -7066,7 +7067,13 @@ impl Editor {
|
|||
cx.spawn_in(window, {
|
||||
let buffer = buffer.clone();
|
||||
async move |editor, cx| {
|
||||
let task_context = task_context_task.await;
|
||||
let task_context = match workspace {
|
||||
Some(ws) => task_context_task
|
||||
.await
|
||||
.notify_workspace_async_err(ws, cx)
|
||||
.flatten(),
|
||||
None => task_context_task.await.ok().flatten(),
|
||||
};
|
||||
|
||||
let resolved_tasks =
|
||||
tasks
|
||||
|
|
@ -9744,7 +9751,7 @@ impl Editor {
|
|||
buffer_row: u32,
|
||||
tasks: &Arc<RunnableTasks>,
|
||||
cx: &mut Context<Self>,
|
||||
) -> Task<Option<task::TaskContext>> {
|
||||
) -> Task<Result<Option<task::TaskContext>>> {
|
||||
let position = Point::new(buffer_row, tasks.column);
|
||||
let range_start = buffer.read(cx).anchor_at(position, Bias::Right);
|
||||
let location = Location {
|
||||
|
|
|
|||
|
|
@ -18,15 +18,16 @@ use buffer_diff::{BufferDiff, DiffHunkSecondaryStatus, DiffHunkStatus, DiffHunkS
|
|||
use collections::HashMap;
|
||||
use futures::{StreamExt, channel::oneshot};
|
||||
use gpui::{
|
||||
BackgroundExecutor, DismissEvent, TestAppContext, UpdateGlobal, VisualTestContext,
|
||||
BackgroundExecutor, DismissEvent, Task, TestAppContext, UpdateGlobal, VisualTestContext,
|
||||
WindowBounds, WindowOptions, div,
|
||||
};
|
||||
use indoc::indoc;
|
||||
use language::{
|
||||
BracketPair, BracketPairConfig,
|
||||
Capability::ReadWrite,
|
||||
DiagnosticSourceKind, FakeLspAdapter, IndentGuideSettings, LanguageConfig,
|
||||
LanguageConfigOverride, LanguageMatcher, LanguageName, LanguageQueries, Override, Point,
|
||||
ContextLocation, ContextProvider, DiagnosticSourceKind, FakeLspAdapter, IndentGuideSettings,
|
||||
LanguageConfig, LanguageConfigOverride, LanguageMatcher, LanguageName, LanguageQueries,
|
||||
LanguageToolchainStore, Override, Point,
|
||||
language_settings::{
|
||||
CompletionSettingsContent, FormatterList, LanguageSettingsContent, LspInsertMode,
|
||||
},
|
||||
|
|
@ -59,6 +60,7 @@ use std::{
|
|||
iter,
|
||||
sync::atomic::{self, AtomicUsize},
|
||||
};
|
||||
use task::TaskVariables;
|
||||
use test::build_editor_with_project;
|
||||
use unindent::Unindent;
|
||||
use util::{
|
||||
|
|
@ -26844,6 +26846,99 @@ async fn test_find_enclosing_node_with_task(cx: &mut TestAppContext) {
|
|||
});
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
async fn test_toggle_code_actions_build_tasks_context_error_notifies(cx: &mut TestAppContext) {
|
||||
init_test(cx, |_| {});
|
||||
|
||||
struct FailingContextProvider;
|
||||
impl ContextProvider for FailingContextProvider {
|
||||
fn build_context(
|
||||
&self,
|
||||
_: &TaskVariables,
|
||||
_: ContextLocation<'_>,
|
||||
_: Option<HashMap<String, String>>,
|
||||
_: Arc<dyn LanguageToolchainStore>,
|
||||
_: &mut gpui::App,
|
||||
) -> Task<anyhow::Result<TaskVariables>> {
|
||||
Task::ready(Err(anyhow::anyhow!("Task context provider failed")))
|
||||
}
|
||||
}
|
||||
|
||||
let language = Arc::new(
|
||||
Arc::try_unwrap(rust_lang())
|
||||
.unwrap()
|
||||
.with_context_provider(Some(Arc::new(FailingContextProvider))),
|
||||
);
|
||||
|
||||
let fs = FakeFs::new(cx.executor());
|
||||
fs.insert_tree(path!("/a"), json!({ "main.rs": "fn main() {}" }))
|
||||
.await;
|
||||
|
||||
let project = Project::test(fs, [path!("/a").as_ref()], cx).await;
|
||||
let language_registry = project.read_with(cx, |project, _| project.languages().clone());
|
||||
language_registry.add(language.clone());
|
||||
|
||||
let window = cx.add_window(|window, cx| MultiWorkspace::test_new(project.clone(), window, cx));
|
||||
let mut cx = VisualTestContext::from_window(*window, cx);
|
||||
let workspace = window
|
||||
.read_with(&mut cx, |mw, _| mw.workspace().clone())
|
||||
.unwrap();
|
||||
|
||||
let worktree_id = workspace.update_in(&mut cx, |workspace, _, cx| {
|
||||
workspace.project().update(cx, |project, cx| {
|
||||
project.worktrees(cx).next().unwrap().read(cx).id()
|
||||
})
|
||||
});
|
||||
|
||||
let editor = workspace
|
||||
.update_in(&mut cx, |workspace, window, cx| {
|
||||
workspace.open_path((worktree_id, rel_path("main.rs")), None, true, window, cx)
|
||||
})
|
||||
.await
|
||||
.unwrap()
|
||||
.downcast::<Editor>()
|
||||
.unwrap();
|
||||
|
||||
editor.update_in(&mut cx, |editor, window, cx| {
|
||||
let buffer = editor.buffer().read(cx).as_singleton().unwrap();
|
||||
buffer.update(cx, |buffer, cx| {
|
||||
buffer.set_language(Some(language.clone()), cx)
|
||||
});
|
||||
|
||||
let snapshot = editor.buffer().read(cx).snapshot(cx);
|
||||
editor.runnables.insert(
|
||||
buffer.read(cx).remote_id(),
|
||||
0,
|
||||
buffer.read(cx).version(),
|
||||
RunnableTasks {
|
||||
templates: Vec::new(),
|
||||
offset: snapshot.anchor_before(MultiBufferOffset(0)),
|
||||
column: 0,
|
||||
extra_variables: HashMap::default(),
|
||||
context_range: BufferOffset(0)..BufferOffset(0),
|
||||
},
|
||||
);
|
||||
editor.change_selections(SelectionEffects::no_scroll(), window, cx, |s| {
|
||||
s.select_ranges([Point::new(0, 0)..Point::new(0, 0)])
|
||||
});
|
||||
|
||||
editor.toggle_code_actions(
|
||||
&ToggleCodeActions {
|
||||
deployed_from: None,
|
||||
quick_launch: false,
|
||||
},
|
||||
window,
|
||||
cx,
|
||||
);
|
||||
});
|
||||
|
||||
cx.run_until_parked();
|
||||
|
||||
workspace.update_in(&mut cx, |workspace, _, _| {
|
||||
assert!(!workspace.notification_ids().is_empty());
|
||||
});
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
async fn test_folding_buffers(cx: &mut TestAppContext) {
|
||||
init_test(cx, |_| {});
|
||||
|
|
|
|||
|
|
@ -310,7 +310,7 @@ impl Editor {
|
|||
let reveal_strategy = action.reveal;
|
||||
let task_context = Self::build_tasks_context(&project, &buffer, buffer_row, &tasks, cx);
|
||||
cx.spawn_in(window, async move |_, cx| {
|
||||
let context = task_context.await?;
|
||||
let context = task_context.await.ok().flatten()?;
|
||||
let (task_source_kind, mut resolved_task) = tasks.resolve(&context).next()?;
|
||||
|
||||
let resolved = &mut resolved_task.resolved;
|
||||
|
|
@ -405,11 +405,12 @@ impl Editor {
|
|||
variables
|
||||
};
|
||||
|
||||
project.update(cx, |project, cx| {
|
||||
let task = project.update(cx, |project, cx| {
|
||||
project.task_store().update(cx, |task_store, cx| {
|
||||
task_store.task_context_for_location(captured_variables, location, cx)
|
||||
})
|
||||
})
|
||||
});
|
||||
cx.background_spawn(async move { task.await.ok().flatten() })
|
||||
}
|
||||
|
||||
pub fn lsp_task_sources(
|
||||
|
|
|
|||
|
|
@ -67,6 +67,7 @@ util.workspace = true
|
|||
[dev-dependencies]
|
||||
fs = { workspace = true, features = ["test-support"] }
|
||||
pretty_assertions.workspace = true
|
||||
tempfile = { workspace = true}
|
||||
settings = { workspace = true, features = ["test-support"] }
|
||||
theme = { workspace = true, features = ["test-support"] }
|
||||
tree-sitter-bash.workspace = true
|
||||
|
|
|
|||
|
|
@ -898,7 +898,7 @@ impl ContextProvider for RustContextProvider {
|
|||
}
|
||||
if let Some(path) = local_abs_path.as_ref()
|
||||
&& let Some((target, manifest_path)) =
|
||||
target_info_from_abs_path(path, project_env.as_ref()).await
|
||||
target_info_from_abs_path(path, project_env.as_ref()).await?
|
||||
{
|
||||
if let Some(target) = target {
|
||||
variables.extend(TaskVariables::from_iter([
|
||||
|
|
@ -1164,24 +1164,31 @@ struct TargetInfo {
|
|||
async fn target_info_from_abs_path(
|
||||
abs_path: &Path,
|
||||
project_env: Option<&HashMap<String, String>>,
|
||||
) -> Option<(Option<TargetInfo>, Arc<Path>)> {
|
||||
) -> Result<Option<(Option<TargetInfo>, Arc<Path>)>> {
|
||||
let mut command = util::command::new_command("cargo");
|
||||
if let Some(envs) = project_env {
|
||||
command.envs(envs);
|
||||
}
|
||||
let output = command
|
||||
.current_dir(abs_path.parent()?)
|
||||
.current_dir(
|
||||
abs_path
|
||||
.parent()
|
||||
.ok_or_else(|| anyhow::anyhow!("failed to get parent directory"))?,
|
||||
)
|
||||
.arg("metadata")
|
||||
.arg("--no-deps")
|
||||
.arg("--format-version")
|
||||
.arg("1")
|
||||
.output()
|
||||
.await
|
||||
.log_err()?
|
||||
.stdout;
|
||||
.await?;
|
||||
|
||||
let metadata: CargoMetadata = serde_json::from_slice(&output).log_err()?;
|
||||
target_info_from_metadata(metadata, abs_path)
|
||||
if !output.status.success() {
|
||||
let stderr_msg = String::from_utf8_lossy(&output.stderr);
|
||||
anyhow::bail!("Cargo metadata failed\n {stderr_msg}");
|
||||
}
|
||||
|
||||
let metadata: CargoMetadata = serde_json::from_slice(&output.stdout)?;
|
||||
Ok(target_info_from_metadata(metadata, abs_path))
|
||||
}
|
||||
|
||||
fn target_info_from_metadata(
|
||||
|
|
@ -2092,6 +2099,21 @@ mod tests {
|
|||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn target_info_from_abs_path_failed() {
|
||||
let project_root = tempfile::tempdir().unwrap();
|
||||
let cargo_toml_path = project_root.path().join("Cargo.toml");
|
||||
let src_dir = project_root.path().join("src");
|
||||
let main_rs_path = src_dir.join("main.rs");
|
||||
|
||||
std::fs::create_dir_all(&src_dir).unwrap();
|
||||
std::fs::write(&cargo_toml_path, "invalid_toml = {[[{").unwrap();
|
||||
std::fs::write(&main_rs_path, "// rust").unwrap();
|
||||
|
||||
let e = smol::block_on(target_info_from_abs_path(&main_rs_path, None)).unwrap_err();
|
||||
assert!(e.to_string().contains("Cargo metadata failed"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_rust_test_fragment() {
|
||||
#[track_caller]
|
||||
|
|
|
|||
|
|
@ -145,7 +145,7 @@ impl TaskStore {
|
|||
};
|
||||
store.task_context_for_location(captured_variables, location, cx)
|
||||
});
|
||||
let task_context = context_task.await.unwrap_or_default();
|
||||
let task_context = context_task.await?.unwrap_or_default();
|
||||
Ok(proto::TaskContext {
|
||||
project_env: task_context.project_env.into_iter().collect(),
|
||||
cwd: task_context
|
||||
|
|
@ -207,7 +207,7 @@ impl TaskStore {
|
|||
captured_variables: TaskVariables,
|
||||
location: Location,
|
||||
cx: &mut App,
|
||||
) -> Task<Option<TaskContext>> {
|
||||
) -> Task<anyhow::Result<Option<TaskContext>>> {
|
||||
match self {
|
||||
TaskStore::Functional(state) => match &state.mode {
|
||||
StoreMode::Local { environment, .. } => local_task_context_for_location(
|
||||
|
|
@ -233,7 +233,7 @@ impl TaskStore {
|
|||
cx,
|
||||
),
|
||||
},
|
||||
TaskStore::Noop => Task::ready(None),
|
||||
TaskStore::Noop => Task::ready(Ok(None)),
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -315,7 +315,7 @@ fn local_task_context_for_location(
|
|||
captured_variables: TaskVariables,
|
||||
location: Location,
|
||||
cx: &App,
|
||||
) -> Task<Option<TaskContext>> {
|
||||
) -> Task<anyhow::Result<Option<TaskContext>>> {
|
||||
let worktree_id = location.buffer.read(cx).file().map(|f| f.worktree_id(cx));
|
||||
let worktree_abs_path = worktree_id
|
||||
.and_then(|worktree_id| worktree_store.read(cx).worktree_for_id(worktree_id, cx))
|
||||
|
|
@ -342,16 +342,16 @@ fn local_task_context_for_location(
|
|||
cx,
|
||||
)
|
||||
})
|
||||
.await
|
||||
.log_err()?;
|
||||
.await?;
|
||||
|
||||
// Remove all custom entries starting with _, as they're not intended for use by the end user.
|
||||
task_variables.sweep();
|
||||
|
||||
Some(TaskContext {
|
||||
Ok(Some(TaskContext {
|
||||
project_env: project_env.unwrap_or_default(),
|
||||
cwd: worktree_abs_path.map(|p| p.to_path_buf()),
|
||||
task_variables,
|
||||
})
|
||||
}))
|
||||
})
|
||||
}
|
||||
|
||||
|
|
@ -364,7 +364,7 @@ fn remote_task_context_for_location(
|
|||
location: Location,
|
||||
toolchain_store: Arc<dyn LanguageToolchainStore>,
|
||||
cx: &mut App,
|
||||
) -> Task<Option<TaskContext>> {
|
||||
) -> Task<anyhow::Result<Option<TaskContext>>> {
|
||||
cx.spawn(async move |cx| {
|
||||
// We need to gather a client context, as the headless one may lack certain information (e.g. tree-sitter parsing is disabled there, so symbols are not available).
|
||||
let mut remote_context = cx
|
||||
|
|
@ -401,8 +401,8 @@ fn remote_task_context_for_location(
|
|||
.map(|(k, v)| (k.to_string(), v))
|
||||
.collect(),
|
||||
});
|
||||
let task_context = context_task.await.log_err()?;
|
||||
Some(TaskContext {
|
||||
let task_context = context_task.await?;
|
||||
Ok(Some(TaskContext {
|
||||
cwd: task_context.cwd.map(PathBuf::from),
|
||||
task_variables: task_context
|
||||
.task_variables
|
||||
|
|
@ -418,7 +418,7 @@ fn remote_task_context_for_location(
|
|||
)
|
||||
.collect(),
|
||||
project_env: task_context.project_env.into_iter().collect(),
|
||||
})
|
||||
}))
|
||||
})
|
||||
}
|
||||
|
||||
|
|
@ -496,8 +496,7 @@ fn combine_task_variables(
|
|||
cx,
|
||||
)
|
||||
})
|
||||
.await
|
||||
.context("building provider context")?,
|
||||
.await?,
|
||||
);
|
||||
}
|
||||
Ok(captured_variables)
|
||||
|
|
|
|||
Loading…
Reference in a new issue