adjust lint groups, start fixing lints

This commit is contained in:
steven-omaha
2023-02-13 20:39:58 +01:00
parent 307d3ef74d
commit e94b0047bc
12 changed files with 91 additions and 34 deletions
+1
View File
@@ -210,4 +210,5 @@ topgrade
* add zsh completion * add zsh completion
* add license * add license
* in outermost Cargo.toml, add categories, keywords, readme
+10 -5
View File
@@ -1,5 +1,6 @@
use std::path::PathBuf; use std::path::PathBuf;
use anyhow::{Context, Result};
use clap::{Arg, ArgMatches, Command}; use clap::{Arg, ArgMatches, Command};
use path_absolutize::Absolutize; use path_absolutize::Absolutize;
@@ -71,12 +72,16 @@ pub fn get() -> clap::ArgMatches {
get_arg_parser().get_matches() get_arg_parser().get_matches()
} }
pub(crate) fn get_absolutized_file_paths(arg_match: &ArgMatches) -> Vec<PathBuf> { pub(crate) fn get_absolutized_file_paths(arg_match: &ArgMatches) -> Result<Vec<PathBuf>> {
arg_match Ok(arg_match
.get_many::<String>("files") .get_many::<String>("files")
.unwrap() .context("getting files from args")?
.cloned() .cloned()
.map(PathBuf::from) .map(PathBuf::from)
.map(|path| path.absolutize().unwrap().into_owned()) .map(|path| {
.collect() path.absolutize()
.expect("absolute path should exist")
.into_owned()
})
.collect())
} }
+7 -3
View File
@@ -30,13 +30,15 @@ pub(crate) trait Backend: Debug {
/// Get all packages that were installed in the system explicitly. /// Get all packages that were installed in the system explicitly.
fn get_explicitly_installed_packages(&self) -> Result<HashSet<Package>>; fn get_explicitly_installed_packages(&self) -> Result<HashSet<Package>>;
fn assign_group(&self, to_assign: Vec<(Package, Rc<Group>)>) { fn assign_group(&self, to_assign: Vec<(Package, Rc<Group>)>) -> Result<()> {
let group_package_map = get_group_packages_map(to_assign); let group_package_map = get_group_packages_map(to_assign);
let section_header = format!("[{}]", self.get_section()); let section_header = format!("[{}]", self.get_section());
for (group, packages) in group_package_map { for (group, packages) in group_package_map {
group.save_packages(&section_header, &packages); group.save_packages(&section_header, &packages)?;
} }
Ok(())
} }
/// Install the specified packages. /// Install the specified packages.
@@ -125,7 +127,9 @@ fn get_group_packages_map(
group_package_map.insert(group.clone(), vec![]); group_package_map.insert(group.clone(), vec![]);
} }
let inner = group_package_map.get_mut(&group).unwrap(); let inner = group_package_map
.get_mut(&group)
.expect("either it was already there or we created it");
inner.push(p); inner.push(p);
} }
+5 -1
View File
@@ -7,7 +7,11 @@ use crate::env::get_editor;
pub fn run_edit_command(files: &[PathBuf]) -> Result<ExitStatus> { pub fn run_edit_command(files: &[PathBuf]) -> Result<ExitStatus> {
let mut cmd = Command::new(get_editor().context("getting suitable editor")?); let mut cmd = Command::new(get_editor().context("getting suitable editor")?);
cmd.current_dir(files[0].parent().unwrap()); cmd.current_dir(
files[0]
.parent()
.context("getting parent dir of first file argument")?,
);
for f in files { for f in files {
cmd.arg(f.to_string_lossy().to_string()); cmd.arg(f.to_string_lossy().to_string());
} }
+1 -1
View File
@@ -30,7 +30,7 @@ impl Config {
}; };
} }
let content = from_file.unwrap(); let content = from_file.expect("we already handled that error");
serde_yaml::from_str(&content).context("parsing yaml config") serde_yaml::from_str(&content).context("parsing yaml config")
} }
+17 -5
View File
@@ -26,7 +26,7 @@ pub struct Pacdef {
impl Pacdef { impl Pacdef {
#[must_use] #[must_use]
pub fn new(args: ArgMatches, config: Config, groups: HashSet<Group>) -> Self { pub const fn new(args: ArgMatches, config: Config, groups: HashSet<Group>) -> Self {
Self { Self {
args, args,
config, config,
@@ -106,6 +106,7 @@ impl Pacdef {
to_install.install_missing_packages() to_install.install_missing_packages()
} }
#[allow(clippy::unused_self)]
fn edit_group_files(&self, groups: &ArgMatches) -> Result<()> { fn edit_group_files(&self, groups: &ArgMatches) -> Result<()> {
let group_dir = crate::path::get_pacdef_group_dir()?; let group_dir = crate::path::get_pacdef_group_dir()?;
@@ -135,6 +136,7 @@ impl Pacdef {
Ok(()) Ok(())
} }
#[allow(clippy::unused_self)]
fn show_version(self) { fn show_version(self) {
println!("{}", get_version_string()); println!("{}", get_version_string());
} }
@@ -188,7 +190,10 @@ impl Pacdef {
} }
fn show_group_content(&self, groups: &ArgMatches) -> Result<()> { fn show_group_content(&self, groups: &ArgMatches) -> Result<()> {
let mut iter = groups.get_many::<String>("group").unwrap().peekable(); let mut iter = groups
.get_many::<String>("group")
.context("getting groups from args")?
.peekable();
let show_more_than_one_group = iter.size_hint().0 > 1; let show_more_than_one_group = iter.size_hint().0 > 1;
@@ -217,12 +222,17 @@ impl Pacdef {
Ok(()) Ok(())
} }
#[allow(clippy::unused_self)]
fn import_groups(&self, args: &ArgMatches) -> Result<()> { fn import_groups(&self, args: &ArgMatches) -> Result<()> {
let files = args::get_absolutized_file_paths(args); let files = args::get_absolutized_file_paths(args)?;
let groups_dir = get_pacdef_group_dir()?; let groups_dir = get_pacdef_group_dir()?;
for target in files { for target in files {
let target_name = target.file_name().unwrap().to_str().unwrap(); let target_name = target
.file_name()
.context("path should not end in '..'")?
.to_str()
.context("filename is not valid UTF-8")?;
if !target.exists() { if !target.exists() {
println!("file {target_name} does not exist, skipping"); println!("file {target_name} does not exist, skipping");
@@ -242,6 +252,7 @@ impl Pacdef {
Ok(()) Ok(())
} }
#[allow(clippy::unused_self)]
fn remove_groups(&self, arg_match: &ArgMatches) -> Result<()> { fn remove_groups(&self, arg_match: &ArgMatches) -> Result<()> {
let paths = get_assumed_group_file_names(arg_match)?; let paths = get_assumed_group_file_names(arg_match)?;
@@ -256,6 +267,7 @@ impl Pacdef {
Ok(()) Ok(())
} }
#[allow(clippy::unused_self)]
fn new_groups(&self, arg: &ArgMatches) -> Result<()> { fn new_groups(&self, arg: &ArgMatches) -> Result<()> {
let paths = get_assumed_group_file_names(arg)?; let paths = get_assumed_group_file_names(arg)?;
@@ -284,7 +296,7 @@ fn get_assumed_group_file_names(arg_match: &ArgMatches) -> Result<Vec<PathBuf>>
let paths: Vec<_> = arg_match let paths: Vec<_> = arg_match
.get_many::<String>("groups") .get_many::<String>("groups")
.unwrap() .context("getting groups from args")?
.map(|s| { .map(|s| {
let mut possible_group_file = groups_dir.clone(); let mut possible_group_file = groups_dir.clone();
possible_group_file.push(s); possible_group_file.push(s);
+25 -12
View File
@@ -31,7 +31,7 @@ impl Group {
} }
let group = let group =
Group::try_from(&path).with_context(|| format!("reading group file {path:?}"))?; Self::try_from(&path).with_context(|| format!("reading group file {path:?}"))?;
result.insert(group); result.insert(group);
} }
@@ -111,17 +111,21 @@ impl Group {
}) })
} }
pub(crate) fn save_packages(&self, section_header: &str, packages: &[Package]) { pub(crate) fn save_packages(&self, section_header: &str, packages: &[Package]) -> Result<()> {
let mut content = read_to_string(&self.path).unwrap(); let mut content = read_to_string(&self.path)
.with_context(|| format!("reading existing file contents from {:?}", &self.path))?;
if content.contains(section_header) { if content.contains(section_header) {
write_packages_to_existing_section(&mut content, section_header, packages); write_packages_to_existing_section(&mut content, section_header, packages)
.context("existing section")?;
} else { } else {
add_new_section_with_packages(&mut content, section_header, packages); add_new_section_with_packages(&mut content, section_header, packages);
} }
let mut file = File::create(&self.path).unwrap(); let mut file = File::create(&self.path)
write!(file, "{content}").unwrap(); .with_context(|| format!("creating descriptor to output file {:?}", &self.path))?;
write!(file, "{content}").with_context(|| format!("writing file {:?}", &self.path))
} }
} }
@@ -146,9 +150,9 @@ fn write_packages_to_existing_section(
group_file_content: &mut String, group_file_content: &mut String,
section_header: &str, section_header: &str,
packages: &[Package], packages: &[Package],
) { ) -> Result<()> {
let idx_of_first_package_line_in_section = let idx_of_first_package_line_in_section =
find_first_package_line_in_section(group_file_content, section_header); find_first_package_line_in_section(group_file_content, section_header)?;
let after = group_file_content.split_off(idx_of_first_package_line_in_section); let after = group_file_content.split_off(idx_of_first_package_line_in_section);
@@ -157,13 +161,22 @@ fn write_packages_to_existing_section(
} }
group_file_content.push_str(&after); group_file_content.push_str(&after);
Ok(())
} }
fn find_first_package_line_in_section(group_file_content: &str, section_header: &str) -> usize { fn find_first_package_line_in_section(
let section_start = group_file_content.find(section_header).unwrap(); group_file_content: &str,
let distance_to_next_newline = group_file_content[section_start..].find('\n').unwrap(); section_header: &str,
) -> Result<usize> {
let section_start = group_file_content
.find(section_header)
.context("finding first package after section header")?;
section_start + distance_to_next_newline + 1 // + 1 to be after the newline let distance_to_next_newline = group_file_content[section_start..]
.find('\n')
.context("getting next newline")?;
Ok(section_start + distance_to_next_newline + 1) // + 1 to be after the newline
} }
fn add_new_section_with_packages( fn add_new_section_with_packages(
+2 -1
View File
@@ -27,7 +27,7 @@ impl From<String> for Package {
impl Package { impl Package {
fn split_into_name_and_repo(s: &str) -> (String, Option<String>) { fn split_into_name_and_repo(s: &str) -> (String, Option<String>) {
let mut iter = s.split('/').rev(); let mut iter = s.split('/').rev();
let name = iter.next().unwrap().to_string(); let name = iter.next().expect("we checked that earlier").to_string();
let repo = iter.next().map(|s| s.to_string()); let repo = iter.next().map(|s| s.to_string());
(name, repo) (name, repo)
} }
@@ -95,6 +95,7 @@ mod tests {
assert_eq!(repo, None); assert_eq!(repo, None);
} }
#[allow(clippy::unwrap_used)]
#[test] #[test]
fn from() { fn from() {
let x = "myrepo/somepackage # ".to_string(); let x = "myrepo/somepackage # ".to_string();
+9 -3
View File
@@ -32,9 +32,15 @@ impl Section {
.to_string(); .to_string();
let mut packages = HashSet::new(); let mut packages = HashSet::new();
// `while let` is unstable, unfortunately // `while let` chains are unstable, unfortunately
while iter.peek().is_some() && !iter.peek().unwrap().starts_with('[') { while iter.peek().is_some()
if let Some(package) = Package::try_from(iter.next().unwrap()) { && !iter
.peek()
.expect("we checked this is some")
.starts_with('[')
{
if let Some(package) = Package::try_from(iter.next().expect("we checked this is some"))
{
packages.insert(package); packages.insert(package);
} }
} }
+9
View File
@@ -1,3 +1,12 @@
#![warn(
clippy::as_conversions,
clippy::use_debug,
clippy::unwrap_used,
clippy::wildcard_dependencies,
clippy::use_self,
clippy::unused_self
)]
mod action; mod action;
pub mod args; pub mod args;
mod backend; mod backend;
+1 -1
View File
@@ -40,7 +40,7 @@ impl Strategy {
} }
if !self.assign_group.is_empty() { if !self.assign_group.is_empty() {
self.backend.assign_group(self.assign_group); self.backend.assign_group(self.assign_group)?;
} }
Ok(()) Ok(())
+4 -2
View File
@@ -5,7 +5,7 @@ use termios::*;
pub(crate) fn get_user_confirmation() -> Result<bool> { pub(crate) fn get_user_confirmation() -> Result<bool> {
print!("Continue? [Y/n] "); print!("Continue? [Y/n] ");
std::io::stdout().flush().unwrap(); std::io::stdout().flush().context("flushing stdout")?;
let mut reply = String::new(); let mut reply = String::new();
std::io::stdin() std::io::stdin()
@@ -29,7 +29,9 @@ pub(crate) fn read_single_char_from_terminal() -> Result<char> {
io::stdin() io::stdin()
.read_exact(&mut input[..]) .read_exact(&mut input[..])
.context("reading one byte from stdin")?; .context("reading one byte from stdin")?;
let result = input[0] as char; let result: char = input[0]
.try_into()
.context("reading a single byte from stdin")?;
// stdin is not echoed automatically in this terminal mode // stdin is not echoed automatically in this terminal mode
println!("{result}"); println!("{result}");