Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Find cargo toml up the fs #3309

Merged
merged 8 commits into from
Feb 29, 2020
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 64 additions & 10 deletions crates/ra_project_model/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ mod sysroot;

use std::{
error::Error,
fs::File,
fs::{read_dir, File, ReadDir},
io::BufReader,
path::{Path, PathBuf},
process::Command,
Expand All @@ -25,15 +25,35 @@ pub use crate::{
};

#[derive(Clone, PartialEq, Eq, Hash, Debug)]
pub struct CargoTomlNotFoundError(pub PathBuf);
pub struct CargoTomlNoneFoundError(pub PathBuf);

impl std::fmt::Display for CargoTomlNotFoundError {
#[derive(Clone, PartialEq, Eq, Hash, Debug)]
pub struct CargoTomlMultipleValidFoundError(pub Vec<PathBuf>);

#[derive(Clone, PartialEq, Eq, Hash, Debug)]
pub struct CargoTomlSearchFileSystemError(pub PathBuf, pub String);
Copy link
Contributor Author

@not-much-io not-much-io Feb 27, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I went this route to enumerate the possibilities, could also have been one error like:

pub enum CargoTomlFindErrorType {
    NoneFound
    MultipleValidFound
    FilesystemError
}

pub struct CargoTomlFindError(pub PathBuf, typ CargoTomlFindErrorType)

However I don't THINK this is "rusty" or a common practice in general nor in this codebase?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's stick with a single error type here

struct CargoTomlNotFoundError {
    searched_at: PathBuf,
    reason: String,
}


impl std::fmt::Display for CargoTomlNoneFoundError {
fn fmt(&self, fmt: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
write!(fmt, "can't find Cargo.toml at {}", self.0.display())
}
}

impl Error for CargoTomlNotFoundError {}
impl std::fmt::Display for CargoTomlMultipleValidFoundError {
fn fmt(&self, fmt: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
write!(fmt, "found multiple valid Cargo.toml files {:?}", self.0)
}
}

impl std::fmt::Display for CargoTomlSearchFileSystemError {
fn fmt(&self, fmt: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
write!(fmt, "a filesystem error occurred while searching for Cargo.toml: {}", self.1)
}
}

impl Error for CargoTomlNoneFoundError {}
impl Error for CargoTomlMultipleValidFoundError {}
impl Error for CargoTomlSearchFileSystemError {}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hope this method of grouping is acceptable? Group related structs and impls together.

I don't have a strong preference, I can more them around if that is preferred. :)


#[derive(Debug, Clone)]
pub enum ProjectWorkspace {
Expand Down Expand Up @@ -406,19 +426,53 @@ fn find_rust_project_json(path: &Path) -> Option<PathBuf> {
None
}

fn find_cargo_toml(path: &Path) -> Result<PathBuf> {
if path.ends_with("Cargo.toml") {
return Ok(path.to_path_buf());
}
fn find_cargo_toml_in_parent_dir(path: &Path) -> Option<PathBuf> {
let mut curr = Some(path);
while let Some(path) = curr {
let candidate = path.join("Cargo.toml");
if candidate.exists() {
return Ok(candidate);
return Some(candidate);
}
curr = path.parent();
}
Err(CargoTomlNotFoundError(path.to_path_buf()).into())

None
}

fn find_cargo_toml_in_child_dir(entities: ReadDir) -> Vec<PathBuf> {
// Only one level down to avoid cycles the easy way and stop a runaway scan with large projects
let mut valid_canditates = vec![];
for entity in entities.filter_map(Result::ok) {
let candidate = entity.path().join("Cargo.toml");
if candidate.exists() {
valid_canditates.push(candidate)
}
}
valid_canditates
}

fn find_cargo_toml(path: &Path) -> Result<PathBuf> {
let path_as_buf = path.to_path_buf();

if path.ends_with("Cargo.toml") {
return Ok(path.to_path_buf());
}

if let Some(p) = find_cargo_toml_in_parent_dir(path) {
return Ok(p);
}

let entities = match read_dir(path) {
Ok(entities) => entities,
Err(e) => return Err(CargoTomlSearchFileSystemError(path_as_buf, e.to_string()).into()),
};

let mut valid_canditates = find_cargo_toml_in_child_dir(entities);
match valid_canditates.len() {
1 => Ok(valid_canditates.remove(0)),
0 => Err(CargoTomlNoneFoundError(path_as_buf).into()),
_ => Err(CargoTomlMultipleValidFoundError(valid_canditates).into()),
}
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, the naming is confusing... Let's do find_in_parent_dir, find_in_child_dir mabye?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you might have meant this anyway but just mentioning that I prefixed with find_cargo_toml_... because these functions are in a fairly "large" file so a new reader might wonder - find what?

pub fn get_rustc_cfg_options() -> CfgOptions {
Expand Down
2 changes: 1 addition & 1 deletion crates/rust-analyzer/src/main_loop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ pub fn main_loop(
Ok(workspace) => loaded_workspaces.push(workspace),
Err(e) => {
log::error!("loading workspace failed: {:?}", e);
if let Some(ra_project_model::CargoTomlNotFoundError(_)) = e.downcast_ref()
if let Some(ra_project_model::CargoTomlNoneFoundError(_)) = e.downcast_ref()
{
if !feature_flags.get("notifications.cargo-toml-not-found") {
continue;
Expand Down