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

Improve error messages #103

Merged
merged 1 commit into from
Oct 15, 2021
Merged
Show file tree
Hide file tree
Changes from all 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
4 changes: 2 additions & 2 deletions cargo-dylint/tests/custom_toolchain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// "Only the MSVC toolchain is supported on Windows"
#[cfg(not(target_os = "windows"))]
mod custom_toolchain {
use anyhow::{anyhow, Result};
use anyhow::{anyhow, Context, Result};
use dylint_internal::{
find_and_replace,
rustup::{toolchain_path, SanitizeEnvironment},
Expand Down Expand Up @@ -36,7 +36,7 @@ mod custom_toolchain {
}

fn random_string() -> Result<String> {
let tempfile = NamedTempFile::new()?;
let tempfile = NamedTempFile::new().with_context(|| "Could not create named temp file")?;
tempfile
.path()
.file_name()
Expand Down
10 changes: 8 additions & 2 deletions cargo-dylint/tests/package_options.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use anyhow::{anyhow, Result};
use anyhow::{anyhow, Context, Result};
use assert_cmd::prelude::*;
use dylint_internal::rustup::SanitizeEnvironment;
use regex::Regex;
Expand Down Expand Up @@ -95,7 +95,13 @@ fn upgrade_package() {

fn rust_version(path: &Path) -> Result<Version> {
let re = Regex::new(r#"^clippy_utils = .*\btag = "rust-([^"]*)""#).unwrap();
let file = read_to_string(path.join("Cargo.toml"))?;
let manifest = path.join("Cargo.toml");
let file = read_to_string(&manifest).with_context(|| {
format!(
"`read_to_string` failed for `{}`",
manifest.to_string_lossy()
)
})?;
let rust_version = file
.lines()
.find_map(|line| re.captures(line).map(|captures| captures[1].to_owned()))
Expand Down
10 changes: 8 additions & 2 deletions dylint-link/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

#[cfg(target_os = "windows")]
use anyhow::ensure;
use anyhow::{anyhow, Result};
use anyhow::{anyhow, Context, Result};
use dylint_internal::{
env::{self, var},
library_filename, Command,
Expand Down Expand Up @@ -137,7 +137,13 @@ fn copy_library(path: &Path) -> Result<()> {
.parent()
.ok_or_else(|| anyhow!("Could not get parent directory"))?;
let path_with_toolchain = strip_deps(parent).join(filename_with_toolchain);
copy(path, path_with_toolchain)?;
copy(&path, &path_with_toolchain).with_context(|| {
format!(
"Could not copy `{}` to `{}`",
path.to_string_lossy(),
path_with_toolchain.to_string_lossy()
)
})?;
}
}

Expand Down
74 changes: 50 additions & 24 deletions dylint/src/driver_builder.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::error::warn;
use anyhow::{anyhow, ensure, Result};
use anyhow::{anyhow, ensure, Context, Result};
use cargo_metadata::MetadataCommand;
use dylint_internal::{
env::{self, var},
Expand Down Expand Up @@ -71,7 +71,12 @@ pub fn get(opts: &crate::Dylint, toolchain: &str) -> Result<PathBuf> {

let driver_dir = dylint_drivers.join(&toolchain);
if !driver_dir.is_dir() {
create_dir_all(&driver_dir)?;
create_dir_all(&driver_dir).with_context(|| {
format!(
"`create_dir_all` failed for `{}`",
driver_dir.to_string_lossy()
)
})?;
}

let driver = driver_dir.join("dylint-driver");
Expand All @@ -91,8 +96,16 @@ fn dylint_drivers() -> Result<PathBuf> {
let home = dirs::home_dir().ok_or_else(|| anyhow!("Could not find HOME directory"))?;
let dylint_drivers = Path::new(&home).join(".dylint_drivers");
if !dylint_drivers.is_dir() {
create_dir_all(&dylint_drivers)?;
write(dylint_drivers.join("README.txt"), README_TXT)?;
create_dir_all(&dylint_drivers).with_context(|| {
format!(
"`create_dir_all` failed for `{}`",
dylint_drivers.to_string_lossy()
)
})?;
let readme_txt = dylint_drivers.join("README.txt");
write(&readme_txt, README_TXT).with_context(|| {
format!("`write` failed for `{}`", readme_txt.to_string_lossy())
})?;
}
Ok(dylint_drivers)
}
Expand All @@ -104,9 +117,9 @@ fn is_outdated(opts: &crate::Dylint, toolchain: &str, driver: &Path) -> Result<b
let stdout = std::str::from_utf8(&output.stdout)?;
let theirs = stdout
.trim_end()
.rsplitn(2, ' ')
.next()
.ok_or_else(|| anyhow!("Could not parse driver version"))?;
.rsplit_once(' ')
.map(|pair| pair.1)
.ok_or_else(|| anyhow!("Could not determine driver version"))?;

let result = Version::parse(theirs);

Expand All @@ -115,7 +128,7 @@ fn is_outdated(opts: &crate::Dylint, toolchain: &str, driver: &Path) -> Result<b
Err(err) => {
warn(
opts,
&format!("Could not determine driver version: {}", err),
&format!("Could not parse driver version `{}`: {}", theirs, err),
);
return Ok(true);
}
Expand All @@ -129,7 +142,7 @@ fn is_outdated(opts: &crate::Dylint, toolchain: &str, driver: &Path) -> Result<b
#[allow(clippy::assertions_on_constants)]
#[allow(clippy::expect_used)]
fn build(opts: &crate::Dylint, toolchain: &str, driver: &Path) -> Result<()> {
let tempdir = tempdir()?;
let tempdir = tempdir().with_context(|| "`tempdir` failed")?;
let package = tempdir.path();

initialize(toolchain, package)?;
Expand Down Expand Up @@ -160,14 +173,18 @@ fn build(opts: &crate::Dylint, toolchain: &str, driver: &Path) -> Result<()> {
}
command.success()?;

copy(
metadata.target_directory.join("debug").join(format!(
"dylint_driver-{}{}",
toolchain,
consts::EXE_SUFFIX
)),
driver,
)?;
let binary = metadata.target_directory.join("debug").join(format!(
"dylint_driver-{}{}",
toolchain,
consts::EXE_SUFFIX
));
copy(&binary, driver).with_context(|| {
format!(
"Could not copy `{}` to `{}`",
binary,
driver.to_string_lossy()
)
})?;

Ok(())
}
Expand All @@ -180,6 +197,7 @@ fn initialize(toolchain: &str, package: &Path) -> Result<()> {
#[cfg(any(not(debug_assertions), not(feature = "dylint_driver_local")))]
let path_spec = "";
#[cfg(all(debug_assertions, feature = "dylint_driver_local"))]
#[allow(clippy::expect_used)]
let path_spec = format!(
", path = \"{}\"",
Path::new(env!("CARGO_MANIFEST_DIR"))
Expand All @@ -192,14 +210,22 @@ fn initialize(toolchain: &str, package: &Path) -> Result<()> {

let dylint_driver_spec = format!("{}{}", version_spec, path_spec);

write(
package.join("Cargo.toml"),
cargo_toml(toolchain, &dylint_driver_spec),
)?;
write(package.join("rust-toolchain"), rust_toolchain(toolchain))?;
let cargo_toml_path = package.join("Cargo.toml");
write(&cargo_toml_path, cargo_toml(toolchain, &dylint_driver_spec))
.with_context(|| format!("`write` failed for `{}`", cargo_toml_path.to_string_lossy()))?;
let rust_toolchain_path = package.join("rust-toolchain");
write(&rust_toolchain_path, rust_toolchain(toolchain)).with_context(|| {
format!(
"`write` failed for `{}`",
rust_toolchain_path.to_string_lossy()
)
})?;
let src = package.join("src");
create_dir_all(&src)?;
write(&src.join("main.rs"), MAIN_RS)?;
create_dir_all(&src)
.with_context(|| format!("`create_dir_all` failed for `{}`", src.to_string_lossy()))?;
let main_rs = src.join("main.rs");
write(&main_rs, MAIN_RS)
.with_context(|| format!("`write` failed for `{}`", main_rs.to_string_lossy()))?;

Ok(())
}
6 changes: 4 additions & 2 deletions dylint/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,9 +142,11 @@ fn dylint_libraries_in(
) -> Result<impl Iterator<Item = Result<(String, String, PathBuf)>>> {
let iter = read_dir(path)
.with_context(|| format!("`read_dir` failed for `{}`", path.to_string_lossy()))?;
let path = path.to_path_buf();
Ok(iter
.map(|entry| -> Result<Option<(String, String, PathBuf)>> {
let entry = entry?;
.map(move |entry| -> Result<Option<(String, String, PathBuf)>> {
let entry = entry
.with_context(|| format!("`read_dir` failed for `{}`", path.to_string_lossy()))?;
let path = entry.path();

Ok(if let Some(filename) = path.file_name() {
Expand Down
56 changes: 43 additions & 13 deletions dylint/src/package_options.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
use crate::Dylint;
use anyhow::{anyhow, Result};
use anyhow::{anyhow, Context, Result};
use dylint_internal::find_and_replace;
use git2::Repository;
use heck::*;
use heck::{CamelCase, KebabCase, ShoutySnakeCase, SnakeCase};
use lazy_static::lazy_static;
use semver::Version;
use std::{
Expand Down Expand Up @@ -35,8 +35,8 @@ pub fn new_package(opts: &Dylint, path: &Path) -> Result<()> {
.map(|s| s.to_string_lossy().to_string())
.ok_or_else(|| anyhow!("Could not determine library name from {:?}", path))?;

let checked_out = tempdir()?;
let filtered = tempdir()?;
let checked_out = tempdir().with_context(|| "`tempdir` failed")?;
let filtered = tempdir().with_context(|| "`tempdir` failed")?;

dylint_internal::clone(DYLINT_TEMPLATE_URL, "master", checked_out.path())?;

Expand All @@ -55,13 +55,25 @@ fn filter(name: &str, from: &Path, to: &Path) -> Result<()> {
let lower_snake_case = name.to_snake_case();

for path in &*PATHS {
let from_path = from.join(path);
let to_path = if path == &Path::new("src").join("fill_me_in.rs") {
to.join("src").join(&lower_snake_case).with_extension("rs")
} else {
to.join(path)
};
to_path.parent().map(create_dir_all).transpose()?;
copy(from.join(path), to_path)?;
let parent = to_path
.parent()
.ok_or_else(|| anyhow!("Could not get parent directory"))?;
create_dir_all(parent).with_context(|| {
format!("`create_dir_all` failed for `{}`", parent.to_string_lossy())
})?;
copy(&from_path, &to_path).with_context(|| {
format!(
"Could not copy `{}` to `{}`",
from_path.to_string_lossy(),
to_path.to_string_lossy()
)
})?;
}

Ok(())
Expand Down Expand Up @@ -99,15 +111,28 @@ fn rename(name: &str, from: &Path, to: &Path) -> Result<()> {
&[&format!(r#"s/\bFillMeIn\b/{}/g"#, camel_case)],
)?;

to.join(rel_path).parent().map(create_dir_all).transpose()?;
copy(from.join(rel_path), to.join(rel_path))?;
let from_path = from.join(rel_path);
let to_path = to.join(rel_path);
let parent = to_path
.parent()
.ok_or_else(|| anyhow!("Could not get parent directory"))?;
create_dir_all(parent).with_context(|| {
format!("`create_dir_all` failed for `{}`", parent.to_string_lossy())
})?;
copy(&from_path, &to_path).with_context(|| {
format!(
"Could not copy `{}` to `{}`",
from_path.to_string_lossy(),
to_path.to_string_lossy()
)
})?;
}

Ok(())
}

pub fn upgrade_package(opts: &Dylint, path: &Path) -> Result<()> {
let tempdir = tempdir()?;
let tempdir = tempdir().with_context(|| "`tempdir` failed")?;

let refname = match &opts.rust_version {
Some(rust_version) => format!("rust-{}", rust_version),
Expand All @@ -120,7 +145,7 @@ pub fn upgrade_package(opts: &Dylint, path: &Path) -> Result<()> {
Some(rust_version) => format!("rust-{}", rust_version),
None => {
let version = latest_rust_version(&repository)?;
format!("rust-{}", version.to_string())
format!("rust-{}", version)
}
};

Expand Down Expand Up @@ -148,8 +173,7 @@ fn latest_rust_version(repository: &Repository) -> Result<Version> {
let tags = repository.tag_names(Some("rust-*"))?;
let mut rust_versions = tags
.iter()
.map(|s| s.map(|s| s.strip_prefix("rust-")).flatten())
.flatten()
.filter_map(|s| s.and_then(|s| s.strip_prefix("rust-")))
.map(Version::parse)
.collect::<std::result::Result<Vec<_>, _>>()?;
rust_versions.sort();
Expand All @@ -159,7 +183,13 @@ fn latest_rust_version(repository: &Repository) -> Result<Version> {
}

fn channel_line(path: &Path) -> Result<String> {
let file = read_to_string(path.join("rust-toolchain"))?;
let rust_toolchain = path.join("rust-toolchain");
let file = read_to_string(&rust_toolchain).with_context(|| {
format!(
"`read_to_string` failed for `{}`",
rust_toolchain.to_string_lossy(),
)
})?;
file.lines()
.find(|line| line.starts_with("channel = "))
.map(ToOwned::to_owned)
Expand Down
21 changes: 15 additions & 6 deletions examples/clippy/tests/ui.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use anyhow::{anyhow, Result};
use anyhow::{anyhow, Context, Result};
use cargo_metadata::Dependency;
use dylint_internal::{cargo::current_metadata, env, find_and_replace, packaging::isolate};
use std::{
Expand Down Expand Up @@ -100,8 +100,11 @@ fn clippy_lints_dependency() -> Result<Dependency> {
}

fn disable_rustfix(src_base: &Path) -> Result<()> {
for entry in read_dir(src_base)? {
let entry = entry?;
for entry in read_dir(src_base)
.with_context(|| format!("`read_dir` failed for `{}`", src_base.to_string_lossy()))?
{
let entry = entry
.with_context(|| format!("`read_dir` failed for `{}`", src_base.to_string_lossy()))?;
let path = entry.path();
if path.extension() != Some(OsStr::new("rs")) {
continue;
Expand All @@ -118,7 +121,12 @@ fn disable_rustfix(src_base: &Path) -> Result<()> {
#[allow(clippy::shadow_unrelated)]
fn adjust_macro_use_imports_test(src_base: &Path) -> Result<()> {
let stderr_file = src_base.join("macro_use_imports.stderr");
let contents = read_to_string(&stderr_file)?;
let contents = read_to_string(&stderr_file).with_context(|| {
format!(
"`read_to_string` failed for `{}`",
stderr_file.to_string_lossy()
)
})?;
let lines: Vec<String> = contents.lines().map(ToString::to_string).collect();

let (first_error, rest) = lines.split_at(ERROR_LINES);
Expand Down Expand Up @@ -151,12 +159,13 @@ fn adjust_macro_use_imports_test(src_base: &Path) -> Result<()> {
assert_eq!(lines_sorted, permuted_sorted);

write(
stderr_file,
&stderr_file,
permuted
.iter()
.map(|line| format!("{}\n", line))
.collect::<String>(),
)?;
)
.with_context(|| format!("Could not write to `{}`", stderr_file.to_string_lossy()))?;

Ok(())
}
Loading