From 9361ab14a380acd962a9a9e80b74779d8317531b Mon Sep 17 00:00:00 2001 From: Ben-PH Date: Mon, 28 Oct 2024 10:44:56 +0100 Subject: [PATCH 1/4] Clear some clippy::pedantic pings --- src/eval.rs | 8 ++------ src/location.rs | 3 +-- src/main.rs | 21 +++++++++++++-------- src/nix_file.rs | 6 +++--- src/structure.rs | 2 +- src/validation.rs | 3 +-- 6 files changed, 21 insertions(+), 22 deletions(-) diff --git a/src/eval.rs b/src/eval.rs index ed972a2..005b5ff 100644 --- a/src/eval.rs +++ b/src/eval.rs @@ -155,7 +155,7 @@ fn mutate_nix_instatiate_arguments_based_on_cfg( pub fn check_values( nixpkgs_path: &Path, nix_file_store: &mut NixFileStore, - package_names: Vec, + package_names: &[String], ) -> validation::Result { let work_dir = Builder::new() .prefix("nixpkgs-vet") @@ -534,11 +534,7 @@ fn handle_non_by_name_attribute( // At this point, we completed two different checks for whether it's a `callPackage`. match (is_semantic_call_package, optional_syntactic_call_package) { // Something like ` = { }` - (false, None) - // Something like ` = pythonPackages.callPackage ...` - | (false, Some(_)) - // Something like ` = bar` where `bar = pkgs.callPackage ...` - | (true, None) => { + (_, None) | (false, Some(_)) => { // In all of these cases, it's not possible to migrate the package to // `pkgs/by-name`. NonApplicable diff --git a/src/location.rs b/src/location.rs index 85257c7..0d9d99e 100644 --- a/src/location.rs +++ b/src/location.rs @@ -45,8 +45,7 @@ impl LineIndex { pub fn line(&self, index: usize) -> usize { match self.newlines.binary_search(&index) { // +1 because lines are 1-indexed - Ok(x) => x + 1, - Err(x) => x + 1, + Ok(x) | Err(x) => x + 1, } } diff --git a/src/main.rs b/src/main.rs index f2ea1b0..0652675 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1,3 +1,10 @@ +// #![warn(clippy::pedantic)] +// #![allow(clippy::uninlined_format_args)] +// #![allow(clippy::enum_glob_use)] +// #![allow(clippy::module_name_repetitions)] +// #![allow(clippy::doc_markdown)] +// #![allow(clippy::if_not_else)] +// #![allow(clippy::ignored_unit_patterns)] mod eval; mod location; mod nix_file; @@ -112,7 +119,7 @@ fn check_nixpkgs(nixpkgs_path: &Path) -> validation::Result { // Only if we could successfully parse the structure, we do the evaluation checks let result = structure.result_map(|package_names| { - eval::check_values(&nixpkgs_path, &mut nix_file_store, package_names) + eval::check_values(&nixpkgs_path, &mut nix_file_store, package_names.as_slice()) })?; Ok(result) @@ -209,7 +216,7 @@ mod tests { "symlinked_tmpdir", Path::new("tests/success"), "Validated successfully\n", - ) + ); }); Ok(()) } @@ -245,12 +252,10 @@ mod tests { let actual_errors = format!("{status}\n"); - if !expected_errors_regex.is_match(&actual_errors) { - panic!( - "Failed test case {name}: {}", - StrComparison::new(expected_errors, &actual_errors) - ); - } + assert!(expected_errors_regex.is_match(&actual_errors), + "Failed test case {name}: {}", + StrComparison::new(expected_errors, &actual_errors) + ); } /// Check whether a path is in a case-insensitive filesystem diff --git a/src/nix_file.rs b/src/nix_file.rs index 2b5447c..765608d 100644 --- a/src/nix_file.rs +++ b/src/nix_file.rs @@ -566,7 +566,7 @@ mod tests { ( Position { line, column }, result - .map(|node| node.to_string()) + .map(ToString::to_string) .map_right(|node| node.to_string()), ) })); @@ -589,7 +589,7 @@ mod tests { fn detects_call_package() -> anyhow::Result<()> { let temp_dir = tests::tempdir()?; let file = temp_dir.path().join("file.nix"); - let contents = indoc! {r#" + let contents = indoc! {r" self: with self; { a.sub = null; b = null; @@ -601,7 +601,7 @@ mod tests { h = callPackage ./file.nix { x = 0; }; i = callPackage ({ }: { }) (let in { }); } - "#}; + "}; std::fs::write(&file, contents)?; diff --git a/src/structure.rs b/src/structure.rs index d862bb0..93b5975 100644 --- a/src/structure.rs +++ b/src/structure.rs @@ -27,7 +27,7 @@ pub fn read_dir_sorted(base_dir: &Path) -> anyhow::Result> { process_results(listing, |listing| { use itertools::Itertools; - Itertools::collect_vec(listing.sorted_by_key(|entry| entry.file_name())) + Itertools::collect_vec(listing.sorted_by_key(DirEntry::file_name)) }) .with_context(ctx) } diff --git a/src/validation.rs b/src/validation.rs index 51c3668..f76e04b 100644 --- a/src/validation.rs +++ b/src/validation.rs @@ -78,9 +78,8 @@ impl Validation<()> { pub fn and(self, other: Validation) -> Validation { match (self, other) { (Success(_), Success(right_value)) => Success(right_value), - (Failure(errors), Success(_)) => Failure(errors), - (Success(_), Failure(errors)) => Failure(errors), (Failure(errors_l), Failure(errors_r)) => Failure(concat([errors_l, errors_r])), + (Failure(errors), Success(_)) | (Success(_), Failure(errors)) => Failure(errors), } } } From d0de17f08bb01290f5d0e13b2891ac4a938f4c3a Mon Sep 17 00:00:00 2001 From: Ben-PH Date: Mon, 28 Oct 2024 11:01:49 +0100 Subject: [PATCH 2/4] Explicit use of tempfile::Builder --- src/eval.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/eval.rs b/src/eval.rs index 005b5ff..f7c4dbb 100644 --- a/src/eval.rs +++ b/src/eval.rs @@ -4,7 +4,6 @@ use std::{env, fs, process}; use anyhow::Context; use relative_path::RelativePathBuf; use serde::Deserialize; -use tempfile::Builder; use crate::nix_file::CallPackageArgumentInfo; use crate::problem::{ @@ -157,7 +156,7 @@ pub fn check_values( nix_file_store: &mut NixFileStore, package_names: &[String], ) -> validation::Result { - let work_dir = Builder::new() + let work_dir = tempfile::Builder::new() .prefix("nixpkgs-vet") .tempdir() .with_context(|| "Failed to create a working directory")?; From 5b4aae35ae10fc12bfed5888bba338bc34cc5fe3 Mon Sep 17 00:00:00 2001 From: Ben-PH Date: Mon, 28 Oct 2024 18:37:25 +0100 Subject: [PATCH 3/4] Revert callPackage match --- src/eval.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/eval.rs b/src/eval.rs index f7c4dbb..c4f73ba 100644 --- a/src/eval.rs +++ b/src/eval.rs @@ -533,7 +533,11 @@ fn handle_non_by_name_attribute( // At this point, we completed two different checks for whether it's a `callPackage`. match (is_semantic_call_package, optional_syntactic_call_package) { // Something like ` = { }` - (_, None) | (false, Some(_)) => { + (false, None) + // Something like ` = pythonPackages.callPackage ...` + | (false, Some(_)) + // Something like ` = bar` where `bar = pkgs.callPackage ...` + | (true, None) => { // In all of these cases, it's not possible to migrate the package to // `pkgs/by-name`. NonApplicable From f68244b851d4bd229ed9d221d3205bbd36c219b7 Mon Sep 17 00:00:00 2001 From: Ben-PH Date: Mon, 28 Oct 2024 18:37:53 +0100 Subject: [PATCH 4/4] Cargo fmt --- src/main.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/main.rs b/src/main.rs index 0652675..d810d53 100644 --- a/src/main.rs +++ b/src/main.rs @@ -252,7 +252,8 @@ mod tests { let actual_errors = format!("{status}\n"); - assert!(expected_errors_regex.is_match(&actual_errors), + assert!( + expected_errors_regex.is_match(&actual_errors), "Failed test case {name}: {}", StrComparison::new(expected_errors, &actual_errors) );