From 1a66c34f1cbb01d4115c380e8d549aa3e74af4ab Mon Sep 17 00:00:00 2001 From: Till Wegmueller Date: Sat, 25 Mar 2023 17:06:01 +0100 Subject: [PATCH] fix clippy Signed-off-by: Till Wegmueller --- pkg6dev/src/main.rs | 70 +++++++--------- ports/src/main.rs | 38 +++++---- ports/src/sources.rs | 27 +++--- ports/src/workspace.rs | 12 +-- specfile/src/lib.rs | 170 ++++++++++++++++++++++++-------------- specfile/src/macros.rs | 2 +- userland/src/component.rs | 9 +- userland/src/lib.rs | 8 +- userland/src/repology.rs | 2 +- 9 files changed, 187 insertions(+), 151 deletions(-) diff --git a/pkg6dev/src/main.rs b/pkg6dev/src/main.rs index 8dfb835..1c51f4a 100644 --- a/pkg6dev/src/main.rs +++ b/pkg6dev/src/main.rs @@ -1,13 +1,13 @@ use clap::{Parser, Subcommand}; -use libips::actions::{File, Manifest, ActionError}; +use libips::actions::{ActionError, File, Manifest}; -use anyhow::{Result}; +use anyhow::Result; use std::collections::HashMap; use std::fs::{read_dir, OpenOptions}; use std::io::Write; use std::path::{Path, PathBuf}; use userland::repology::find_newest_version; -use userland::{Makefile, Component}; +use userland::{Component, Makefile}; #[derive(Parser, Debug)] #[clap(author, version, about, long_about = None)] @@ -46,12 +46,12 @@ fn main() -> Result<()> { } } -fn parse_tripplet_replacements(replacements: &Vec) -> HashMap { +fn parse_tripplet_replacements(replacements: &[String]) -> HashMap { let mut map = HashMap::new(); for pair in replacements - .into_iter() + .iter() .map(|str| { - str.split_once(":") + str.split_once(':') .map(|s| (s.0.to_owned(), s.1.to_owned())) .unwrap_or((String::new(), String::new())) }) @@ -93,10 +93,8 @@ fn diff_component( .as_ref() .join("manifests/sample-manifest.p5m"); - let manifests_res: Result, ActionError> = manifest_files - .iter() - .map(|f| Manifest::parse_file(f.to_string())) - .collect(); + let manifests_res: Result, ActionError> = + manifest_files.iter().map(Manifest::parse_file).collect(); let sample_manifest = Manifest::parse_file(sample_manifest_file)?; @@ -109,12 +107,8 @@ fn diff_component( println!("file {} is missing in the manifests", f.path); } - let removed_files = find_removed_files( - &sample_manifest, - manifests.clone(), - &component_path, - &replacements, - )?; + let removed_files = + find_removed_files(&sample_manifest, manifests, &component_path, &replacements)?; for f in removed_files { println!( @@ -140,7 +134,7 @@ fn diff_component( fn show_component_info>(component_path: P) -> Result<()> { let makefile_path = component_path.as_ref().join("Makefile"); - let initial_makefile = Makefile::parse_single_file(&makefile_path)?; + let initial_makefile = Makefile::parse_single_file(makefile_path)?; let makefile = initial_makefile.parse_all()?; let mut name = String::new(); @@ -148,15 +142,15 @@ fn show_component_info>(component_path: P) -> Result<()> { let component = Component::new_from_makefile(&makefile)?; if let Some(var) = makefile.get("COMPONENT_NAME") { - println!("Name: {}", var.replace("\n", "\n\t")); + println!("Name: {}", var.replace('\n', "\n\t")); if let Some(component_name) = makefile.get_first_value_of_variable_by_name("COMPONENT_NAME") { - name = component_name.clone(); + name = component_name; } } if let Some(var) = makefile.get("COMPONENT_VERSION") { - println!("Version: {}", var.replace("\n", "\n\t")); + println!("Version: {}", var.replace('\n', "\n\t")); let latest_version = find_newest_version(&name); if latest_version.is_ok() { println!("Latest Version: {}", latest_version?); @@ -169,27 +163,27 @@ fn show_component_info>(component_path: P) -> Result<()> { } if let Some(var) = makefile.get("BUILD_BITS") { - println!("Build bits: {}", var.replace("\n", "\n\t")); + println!("Build bits: {}", var.replace('\n', "\n\t")); } if let Some(var) = makefile.get("COMPONENT_BUILD_ACTION") { - println!("Build action: {}", var.replace("\n", "\n\t")); + println!("Build action: {}", var.replace('\n', "\n\t")); } if let Some(var) = makefile.get("COMPONENT_PROJECT_URL") { - println!("Project URl: {}", var.replace("\n", "\n\t")); + println!("Project URl: {}", var.replace('\n', "\n\t")); } if let Some(var) = makefile.get("COMPONENT_ARCHIVE_URL") { - println!("Source URl: {}", var.replace("\n", "\n\t")); + println!("Source URl: {}", var.replace('\n', "\n\t")); } if let Some(var) = makefile.get("COMPONENT_ARCHIVE_HASH") { - println!("Source Archive File Hash: {}", var.replace("\n", "\n\t")); + println!("Source Archive File Hash: {}", var.replace('\n', "\n\t")); } if let Some(var) = makefile.get("REQUIRED_PACKAGES") { - println!("Dependencies:\n\t{}", var.replace("\n", "\n\t")); + println!("Dependencies:\n\t{}", var.replace('\n', "\n\t")); } if let Some(var) = makefile.get("COMPONENT_INSTALL_ACTION") { @@ -209,21 +203,17 @@ fn find_removed_files>( replacements: &Option>, ) -> Result> { let f_map = make_file_map(sample_manifest.files.clone()); - let all_files: Vec = manifests - .iter() - .map(|m| m.files.clone()) - .flatten() - .collect(); + let all_files: Vec = manifests.iter().flat_map(|m| m.files.clone()).collect(); let mut removed_files: Vec = Vec::new(); for f in all_files { match f.get_original_path() { Some(path) => { - if !f_map.contains_key(replace_func(path.clone(), replacements).as_str()) { - if !component_path.as_ref().join(path).exists() { - removed_files.push(f) - } + if !f_map.contains_key(replace_func(path.clone(), replacements).as_str()) + && !component_path.as_ref().join(path).exists() + { + removed_files.push(f) } } None => { @@ -243,11 +233,7 @@ fn find_files_missing_in_manifests( manifests: Vec, replacements: &Option>, ) -> Result> { - let all_files: Vec = manifests - .iter() - .map(|m| m.files.clone()) - .flatten() - .collect(); + let all_files: Vec = manifests.iter().flat_map(|m| m.files.clone()).collect(); let f_map = make_file_map(all_files); let mut missing_files: Vec = Vec::new(); @@ -273,7 +259,7 @@ fn find_files_missing_in_manifests( fn replace_func(orig: String, replacements: &Option>) -> String { if let Some(replacements) = replacements { let mut replacement = orig.clone(); - for (i, (from, to)) in replacements.into_iter().enumerate() { + for (i, (from, to)) in replacements.iter().enumerate() { let from: &str = &format!("$({})", from); if i == 0 { replacement = orig.replace(from, to); @@ -292,7 +278,7 @@ fn make_file_map(files: Vec) -> HashMap { .iter() .map(|f| { let orig_path_opt = f.get_original_path(); - if orig_path_opt == None { + if orig_path_opt.is_none() { return (f.path.clone(), f.clone()); } (orig_path_opt.unwrap(), f.clone()) diff --git a/ports/src/main.rs b/ports/src/main.rs index 760b0ec..d9feedb 100644 --- a/ports/src/main.rs +++ b/ports/src/main.rs @@ -1,22 +1,23 @@ -mod workspace; mod sources; +#[allow(clippy::result_large_err)] +mod workspace; +use crate::workspace::Workspace; use anyhow::anyhow; +use anyhow::Result; use clap::{Parser, Subcommand}; +use specfile::macros; +use specfile::parse; +use std::collections::HashMap; use std::fs; use std::path::Path; use std::path::PathBuf; -use specfile::parse; -use specfile::macros; -use std::collections::HashMap; -use crate::workspace::Workspace; -use anyhow::Result; -enum Verbose{ +enum Verbose { Off, Some, On, - Debug + Debug, } #[derive(Debug, Parser)] @@ -40,7 +41,7 @@ enum Commands { #[clap(value_parser)] specfile: PathBuf, - } + }, } fn main() -> Result<()> { @@ -54,7 +55,8 @@ fn main() -> Result<()> { 0 => Verbose::Off, 1 => Verbose::Some, 2 => Verbose::On, - 3 | _ => Verbose::Debug, + 3 => Verbose::Debug, + _ => Verbose::Debug, }; match cli.command { @@ -62,7 +64,7 @@ fn main() -> Result<()> { run_package_command(specfile, target)?; } } - + Ok(()) } @@ -73,17 +75,19 @@ fn run_package_command>(spec_file: P, _target: P) -> Result<()> { let downloaded = ws.get_sources(spec.sources)?; ws.unpack_all_sources(downloaded)?; - let mut macro_map= HashMap::::new(); + let mut macro_map = HashMap::::new(); for ws_macro in ws.get_macros() { macro_map.insert( - ws_macro.0, - ws_macro.1.to_str().ok_or(anyhow!("not string path {}", ws_macro.1.display()))?.to_owned() + ws_macro.0, + ws_macro + .1 + .to_str() + .ok_or_else(|| anyhow!("not string path {}", ws_macro.1.display()))? + .to_owned(), ); } - let mp = macros::MacroParser { - macros: macro_map - }; + let mp = macros::MacroParser { macros: macro_map }; let build_script = mp.parse(spec.build_script)?; ws.build(build_script)?; diff --git a/ports/src/sources.rs b/ports/src/sources.rs index 73c45e0..07734b9 100644 --- a/ports/src/sources.rs +++ b/ports/src/sources.rs @@ -1,6 +1,10 @@ -use url::{Url, ParseError}; +use std::{ + fmt::Display, + path::{Path, PathBuf}, + result::Result as StdResult, +}; use thiserror::Error; -use std::{result::Result as StdResult, path::{Path, PathBuf}, fmt::Display}; +use url::{ParseError, Url}; type Result = StdResult; @@ -9,7 +13,7 @@ pub enum SourceError { #[error("can't create source from url: {0}")] CantCreateSource(String), #[error("can not parse source url: {0}")] - UrlParseError(#[from]ParseError) + UrlParseError(#[from] ParseError), } #[derive(Debug, Clone)] @@ -28,16 +32,13 @@ impl Source { pub fn new>(url_string: &str, local_base: P) -> Result { let url = Url::parse(url_string)?; let path = url.path().to_owned(); - let path_vec: Vec<_> = path.split("/").collect(); + let path_vec: Vec<_> = path.split('/').collect(); match path_vec.last() { - Some(str) => { - let local_name = str.clone(); - Ok(Source { - url, - local_name: local_base.as_ref().join(local_name), - }) - } - None => Err(SourceError::CantCreateSource(url.into()))? + Some(local_name) => Ok(Source { + url, + local_name: local_base.as_ref().join(local_name), + }), + None => Err(SourceError::CantCreateSource(url.into()))?, } } -} \ No newline at end of file +} diff --git a/ports/src/workspace.rs b/ports/src/workspace.rs index 8fdb001..8acb696 100644 --- a/ports/src/workspace.rs +++ b/ports/src/workspace.rs @@ -17,7 +17,7 @@ type Result = StdResult; static DEFAULTWORKSPACEROOT: &str = "~/.ports/wks"; static DEFAULTARCH: &str = "i386"; static DEFAULTTAR: &str = "gtar"; -static DEFAULTSHEBANG: &'static [u8; 19usize] = b"#!/usr/bin/env bash"; +static DEFAULTSHEBANG: &[u8; 19usize] = b"#!/usr/bin/env bash"; #[derive(Debug, Error)] pub enum WorkspaceError { @@ -68,7 +68,7 @@ fn init_root(ws: &Workspace) -> Result<()> { impl Workspace { pub fn new(root: &str) -> Result { - let root_dir = if root == "" { + let root_dir = if root.is_empty() { DEFAULTWORKSPACEROOT } else { root @@ -145,8 +145,8 @@ impl Workspace { let mut tar_cmd = Command::new(DEFAULTTAR) .args([ "-C", - &self.build_dir.to_str().ok_or(WorkspaceError::UnextractableSource(src.clone()))?, - "-xaf", &src.local_name.to_str().ok_or(WorkspaceError::UnextractableSource(src.clone()))?, + self.build_dir.to_str().ok_or(WorkspaceError::UnextractableSource(src.clone()))?, + "-xaf", src.local_name.to_str().ok_or(WorkspaceError::UnextractableSource(src.clone()))?, "--strip-components=1" ]) .spawn()?; @@ -180,7 +180,7 @@ impl Workspace { let mut shell = Command::new(bash) .args([ "-ex", - &build_script_path.to_str().ok_or(WorkspaceError::UnrunableScript("build_script".into()))? + build_script_path.to_str().ok_or(WorkspaceError::UnrunableScript("build_script".into()))? ]) .env_clear() .envs(&filtered_env) @@ -201,7 +201,7 @@ impl Workspace { let cwd = current_dir()?; set_current_dir(Path::new(&self.proto_dir))?; for f in file_list { - if f.starts_with("/") { + if f.starts_with('/') { let mut f_mut = f.clone(); f_mut.remove(0); manifest.add_file(FileAction::read_from_path(Path::new(&f_mut))?) diff --git a/specfile/src/lib.rs b/specfile/src/lib.rs index 9936626..b97f013 100644 --- a/specfile/src/lib.rs +++ b/specfile/src/lib.rs @@ -1,9 +1,9 @@ pub mod macros; +use anyhow::Result; use pest::Parser; use pest_derive::Parser; use std::collections::HashMap; -use anyhow::{Result}; #[derive(Parser)] #[grammar = "specfile.pest"] @@ -37,16 +37,16 @@ enum KnownVariableControl { fn append_newline_string(s: &str, section_line: i32) -> String { if section_line == 0 { - return s.to_owned(); + s.to_owned() + } else { + "\n".to_owned() + s } - return "\n".to_owned() + s; } pub fn parse(file_contents: String) -> Result { let pairs = SpecFileParser::parse(Rule::file, &file_contents)?; let mut spec = SpecFile::default(); - for pair in pairs { // A pair can be converted to an iterator of the tokens which make it up: match pair.as_rule() { @@ -55,32 +55,43 @@ pub fn parse(file_contents: String) -> Result { let mut var_name_tmp = String::new(); for variable_rule in pair.clone().into_inner() { match variable_rule.as_rule() { - Rule::variable_name => { - match variable_rule.as_str() { - "Name" => var_control = KnownVariableControl::Name, - "Version" => var_control = KnownVariableControl::Version, - "Release" => var_control = KnownVariableControl::Release, - "Summary" => var_control = KnownVariableControl::Summary, - "License" => var_control = KnownVariableControl::License, - _ => var_control = { + Rule::variable_name => match variable_rule.as_str() { + "Name" => var_control = KnownVariableControl::Name, + "Version" => var_control = KnownVariableControl::Version, + "Release" => var_control = KnownVariableControl::Release, + "Summary" => var_control = KnownVariableControl::Summary, + "License" => var_control = KnownVariableControl::License, + _ => { + var_control = { var_name_tmp = variable_rule.as_str().to_string(); KnownVariableControl::None - }, - } - } - Rule::variable_text => { - match var_control { - KnownVariableControl::Name => spec.name = variable_rule.as_str().to_string(), - KnownVariableControl::Version => spec.version = variable_rule.as_str().to_string(), - KnownVariableControl::Release => spec.release = variable_rule.as_str().to_string(), - KnownVariableControl::Summary =>spec.summary = variable_rule.as_str().to_string(), - KnownVariableControl::License => spec.license = variable_rule.as_str().to_string(), - KnownVariableControl::None => { - spec.variables.insert(var_name_tmp.clone(), variable_rule.as_str().to_string()); } } - } - _ => () + }, + Rule::variable_text => match var_control { + KnownVariableControl::Name => { + spec.name = variable_rule.as_str().to_string() + } + KnownVariableControl::Version => { + spec.version = variable_rule.as_str().to_string() + } + KnownVariableControl::Release => { + spec.release = variable_rule.as_str().to_string() + } + KnownVariableControl::Summary => { + spec.summary = variable_rule.as_str().to_string() + } + KnownVariableControl::License => { + spec.license = variable_rule.as_str().to_string() + } + KnownVariableControl::None => { + spec.variables.insert( + var_name_tmp.clone(), + variable_rule.as_str().to_string(), + ); + } + }, + _ => (), } } } @@ -89,51 +100,84 @@ pub fn parse(file_contents: String) -> Result { let mut section_line = 0; for section_rule in pair.clone().into_inner() { match section_rule.as_rule() { - Rule::section_name => { - section_name_tmp = section_rule.as_str().to_string() - } + Rule::section_name => section_name_tmp = section_rule.as_str().to_string(), Rule::section_line => { for line_or_comment in section_rule.into_inner() { - match line_or_comment.as_rule() { - Rule::section_text => { - match section_name_tmp.as_str() { - "description" => { - spec.description.push_str(append_newline_string(line_or_comment.as_str(), section_line).as_str()); - section_line = section_line + 1 - }, - "prep" => { - spec.prep_script.push_str(append_newline_string(line_or_comment.as_str(), section_line).as_str()); - section_line = section_line + 1 - }, - "build" => { - spec.build_script.push_str(append_newline_string(line_or_comment.as_str(), section_line).as_str()); - section_line = section_line + 1 - }, - "files" => spec.files.push(line_or_comment.as_str().trim_end().to_string()), - "install" => { - spec.install_script.push_str(append_newline_string(line_or_comment.as_str(), section_line).as_str()); - section_line = section_line + 1 - }, - "changelog" => { - spec.changelog.push_str(append_newline_string(line_or_comment.as_str(), section_line).as_str()); - section_line = section_line + 1 - }, - _ => panic!( - "Unknown Section: {:?}", - line_or_comment.as_rule() - ), + if line_or_comment.as_rule() == Rule::section_text { + match section_name_tmp.as_str() { + "description" => { + spec.description.push_str( + append_newline_string( + line_or_comment.as_str(), + section_line, + ) + .as_str(), + ); + section_line += 1 } + "prep" => { + spec.prep_script.push_str( + append_newline_string( + line_or_comment.as_str(), + section_line, + ) + .as_str(), + ); + section_line += 1 + } + "build" => { + spec.build_script.push_str( + append_newline_string( + line_or_comment.as_str(), + section_line, + ) + .as_str(), + ); + section_line += 1 + } + "files" => spec + .files + .push(line_or_comment.as_str().trim_end().to_string()), + "install" => { + spec.install_script.push_str( + append_newline_string( + line_or_comment.as_str(), + section_line, + ) + .as_str(), + ); + section_line += 1 + } + "changelog" => { + spec.changelog.push_str( + append_newline_string( + line_or_comment.as_str(), + section_line, + ) + .as_str(), + ); + section_line += 1 + } + _ => panic!( + "Unknown Section: {:?}", + line_or_comment.as_rule() + ), } - _ => () } } } - _ => panic!("Rule not known please update the code: {:?}", section_rule.as_rule()), + _ => panic!( + "Rule not known please update the code: {:?}", + section_rule.as_rule() + ), } } } Rule::EOI => (), - _ => panic!("Rule not known please update the code: {:?}", pair.as_rule()), + _ => panic!( + "Rule not known please update the code: {:?}", + pair.as_rule() + ), } } @@ -142,8 +186,8 @@ pub fn parse(file_contents: String) -> Result { #[cfg(test)] mod tests { - use std::fs; use crate::parse; + use std::fs; #[test] fn it_works() { @@ -157,8 +201,8 @@ mod tests { Ok(file) => { let spec = parse(file); assert!(spec.is_ok(), "parsing error {:?}", spec) - }, - Err(e) => panic!("io error: {:}", e) + } + Err(e) => panic!("io error: {:}", e), } } } diff --git a/specfile/src/macros.rs b/specfile/src/macros.rs index 7240004..3675dae 100644 --- a/specfile/src/macros.rs +++ b/specfile/src/macros.rs @@ -33,7 +33,7 @@ impl MacroParser { for (i, line) in raw_string.lines().enumerate() { let mut replaced_line = String::new(); - let pairs = InternalMacroParser::parse(Rule::file, &line)?; + let pairs = InternalMacroParser::parse(Rule::file, line)?; for pair in pairs { for test_pair in pair.into_inner() { diff --git a/userland/src/component.rs b/userland/src/component.rs index 81eb1c0..adc5144 100644 --- a/userland/src/component.rs +++ b/userland/src/component.rs @@ -66,7 +66,7 @@ impl Component { let ver = semver::Version::parse( &m.get_first_value_of_variable_by_name("COMPONENT_VERSION") - .ok_or(anyhow::anyhow!("missing component version"))?, + .ok_or_else(|| anyhow::anyhow!("missing component version"))?, )?; let revision = m @@ -85,7 +85,8 @@ impl Component { "cmake" => BuildStyle::Cmake, "meson" => BuildStyle::Meson, "custom" => BuildStyle::Custom, - "configure" | _ => BuildStyle::Configure, + "configure" => BuildStyle::Configure, + _ => BuildStyle::Configure, }; //TODO: Custom build style variable checks // something like guess_buildstyle_from_options @@ -111,10 +112,10 @@ impl Component { Ok(Self { version: ver, - revision: revision, + revision, sources: HashMap::from([src]), options: opts, - build_style: build_style, + build_style, }) } } diff --git a/userland/src/lib.rs b/userland/src/lib.rs index c46573b..7b27dbc 100644 --- a/userland/src/lib.rs +++ b/userland/src/lib.rs @@ -125,7 +125,7 @@ impl Makefile { } pub fn get_includes(&self) -> Option> { - if self.includes.len() > 0 { + if !self.includes.is_empty() { Some(self.includes.clone()) } else { None @@ -133,7 +133,7 @@ impl Makefile { } pub fn has_includes(&self) -> bool { - self.includes.len() > 0 + !self.includes.is_empty() } pub fn parse_included_makefiles(&self) -> Result> { @@ -171,7 +171,7 @@ impl Makefile { } for captures in VARRE.captures_iter(maybe_nested_var) { if let Some(nested_var) = captures.name("var_name") { - let nested_var_name = nested_var.as_str().replace("$(", "").replace(")", ""); + let nested_var_name = nested_var.as_str().replace("$(", "").replace(')', ""); if let Some(resolved_nested_var) = self.get(&nested_var_name) { let mut new_string = vars_copy[i].clone(); new_string = @@ -195,7 +195,7 @@ impl Makefile { } fn vars_to_string(vars: &Vec) -> String { - if vars.len() == 0 { + if vars.is_empty() { String::new() } else if vars.len() == 1 { vars[0].clone() diff --git a/userland/src/repology.rs b/userland/src/repology.rs index 6078e38..39fde53 100644 --- a/userland/src/repology.rs +++ b/userland/src/repology.rs @@ -29,7 +29,7 @@ pub fn project(package: &str) -> Result> { let json = reqwest::blocking::get(url)?.json::>()?; - return Ok(json); + Ok(json) } pub fn find_newest_version(package: &str) -> Result {