Skip to content

Commit

Permalink
feat: don't crash LSP when there are errors resolving the workspace (#…
Browse files Browse the repository at this point in the history
…6257)

# Description

## Problem

Before this PR, LSP would crash for assumed workspaces (when we can't
find a Nargo.toml) or when there were errors in the Nargo.toml.

## Summary

Now LSP doesn't crash in the above cases:
- for assumed workspaces we don't crash (we used to not crash but I made
some changes in the past that broke this)
- for errors in Nargo.toml we now output to STDERR, so you can see the
error in the output. This is what Rust Analyzer does, except that they
also have a "rust-analyzer" thing at the bottom that becomes yellow when
there's an error... I didn't implement this (it probably requires some
code on the extension side) but I think if something doesn't work you'd
check the logs or ask, and it's worse if it crashes.

## Additional Context

## Documentation

Check one:
- [x] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[For Experimental Features]** Documentation to be submitted in a
separate PR.

# PR Checklist

- [x] I have tested the changes locally.
- [x] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.
  • Loading branch information
asterite authored Oct 9, 2024
1 parent e13f617 commit 7cc7197
Showing 1 changed file with 22 additions and 6 deletions.
28 changes: 22 additions & 6 deletions tooling/lsp/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -267,12 +267,16 @@ fn byte_span_to_range<'a, F: files::Files<'a> + ?Sized>(

pub(crate) fn resolve_workspace_for_source_path(file_path: &Path) -> Result<Workspace, LspError> {
if let Some(toml_path) = find_file_manifest(file_path) {
return resolve_workspace_from_toml(
match resolve_workspace_from_toml(
&toml_path,
PackageSelection::All,
Some(NOIR_ARTIFACT_VERSION_STRING.to_string()),
)
.map_err(|err| LspError::WorkspaceResolutionError(err.to_string()));
) {
Ok(workspace) => return Ok(workspace),
Err(error) => {
eprintln!("Error while processing {:?}: {}", toml_path, error);
}
}
}

let Some(parent_folder) = file_path
Expand All @@ -285,14 +289,22 @@ pub(crate) fn resolve_workspace_for_source_path(file_path: &Path) -> Result<Work
file_path
)));
};

let crate_name = match CrateName::from_str(parent_folder) {
Ok(name) => name,
Err(error) => {
eprintln!("{}", error);
CrateName::from_str("root").unwrap()
}
};

let assumed_package = Package {
version: None,
compiler_required_version: Some(NOIR_ARTIFACT_VERSION_STRING.to_string()),
root_dir: PathBuf::from(parent_folder),
package_type: PackageType::Binary,
entry_path: PathBuf::from(file_path),
name: CrateName::from_str(parent_folder)
.map_err(|err| LspError::WorkspaceResolutionError(err.to_string()))?,
name: crate_name,
dependencies: BTreeMap::new(),
expression_width: None,
};
Expand All @@ -309,7 +321,11 @@ pub(crate) fn workspace_package_for_file<'a>(
workspace: &'a Workspace,
file_path: &Path,
) -> Option<&'a Package> {
workspace.members.iter().find(|package| file_path.starts_with(&package.root_dir))
if workspace.is_assumed {
workspace.members.first()
} else {
workspace.members.iter().find(|package| file_path.starts_with(&package.root_dir))
}
}

pub(crate) fn prepare_package<'file_manager, 'parsed_files>(
Expand Down

0 comments on commit 7cc7197

Please sign in to comment.