From 531a7da636ce8d64cb88ec859676d89f3b24fa4a Mon Sep 17 00:00:00 2001 From: Edward Shen Date: Thu, 2 Jun 2022 22:23:35 -0700 Subject: [PATCH] Clippy --- src/config.rs | 30 +++++++++++++++--------------- src/error.rs | 1 + src/main.rs | 43 ++++++++++++++++++++++--------------------- src/routes.rs | 32 +++++++++++++++++--------------- src/template_args.rs | 4 ++-- 5 files changed, 57 insertions(+), 53 deletions(-) diff --git a/src/config.rs b/src/config.rs index 61cadc8..b7598bf 100644 --- a/src/config.rs +++ b/src/config.rs @@ -92,7 +92,7 @@ impl From<&'static str> for Route { /// web path. This incurs a disk check operation, but since users shouldn't be /// updating the config that frequently, it should be fine. impl<'de> Deserialize<'de> for Route { - fn deserialize(deserializer: D) -> Result + fn deserialize(deserializer: D) -> Result where D: Deserializer<'de>, { @@ -233,7 +233,7 @@ pub enum RouteType { Internal, } -pub struct ConfigData { +pub struct FileData { pub path: PathBuf, pub file: File, } @@ -242,19 +242,19 @@ pub struct ConfigData { /// locations for a place to write a config file to. In order, it checks the /// system-wide config location (`/etc/`, in Linux), followed by the config /// folder, followed by the user's home folder. -pub fn get_config_data() -> Result { +pub fn get_config_data() -> Result { // Locations to check, with highest priority first let locations: Vec<_> = { let mut folders = vec![PathBuf::from("/etc/")]; // Config folder if let Some(folder) = config_dir() { - folders.push(folder) + folders.push(folder); } // Home folder if let Some(folder) = home_dir() { - folders.push(folder) + folders.push(folder); } folders @@ -271,13 +271,13 @@ pub fn get_config_data() -> Result { match file { Ok(file) => { debug!("Found file at {location:?}."); - return Ok(ConfigData { + return Ok(FileData { path: location.clone(), file, }); } Err(e) => { - debug!("Tried to read '{location:?}' but failed due to error: {e}",) + debug!("Tried to read '{location:?}' but failed due to error: {e}"); } } } @@ -298,7 +298,7 @@ pub fn get_config_data() -> Result { file.write_all(DEFAULT_CONFIG)?; let file = OpenOptions::new().read(true).open(location.clone())?; - return Ok(ConfigData { + return Ok(FileData { path: location, file, }); @@ -314,19 +314,19 @@ pub fn get_config_data() -> Result { /// Assumes that the user knows what they're talking about and will only try /// to load the config at the given path. -pub fn load_custom_path_config( +pub fn load_custom_file( path: impl Into, -) -> Result { +) -> Result { let path = path.into(); let file = OpenOptions::new() .read(true) .open(&path) .map_err(|e| BunBunError::InvalidConfigPath(path.clone(), e))?; - Ok(ConfigData { file, path }) + Ok(FileData { path, file }) } -pub fn read_config( +pub fn load_file( mut config_file: File, large_config: bool, ) -> Result { @@ -417,7 +417,7 @@ mod read_config { fn empty_file() -> Result<()> { let config_file = tempfile::tempfile()?; assert!(matches!( - read_config(config_file, false), + load_file(config_file, false), Err(BunBunError::ZeroByteConfig) )); Ok(()) @@ -428,7 +428,7 @@ mod read_config { let mut config_file = tempfile::tempfile()?; let size_to_write = (LARGE_FILE_SIZE_THRESHOLD + 1) as usize; config_file.write(&[0].repeat(size_to_write))?; - match read_config(config_file, false) { + match load_file(config_file, false) { Err(BunBunError::ConfigTooLarge(size)) if size as usize == size_to_write => {} Err(BunBunError::ConfigTooLarge(size)) => { @@ -441,7 +441,7 @@ mod read_config { #[test] fn valid_config() -> Result<()> { - assert!(read_config(File::open("bunbun.default.yaml")?, false).is_ok()); + assert!(load_file(File::open("bunbun.default.yaml")?, false).is_ok()); Ok(()) } } diff --git a/src/error.rs b/src/error.rs index 9644bf9..85249d5 100644 --- a/src/error.rs +++ b/src/error.rs @@ -2,6 +2,7 @@ use std::error::Error; use std::fmt; #[derive(Debug)] +#[allow(clippy::module_name_repetitions)] pub enum BunBunError { Io(std::io::Error), Parse(serde_yaml::Error), diff --git a/src/main.rs b/src/main.rs index b58ad93..cab22f3 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1,13 +1,13 @@ #![forbid(unsafe_code)] #![deny(missing_docs)] +#![warn(clippy::nursery, clippy::pedantic)] //! Bunbun is a pure-Rust implementation of bunny1 that provides a customizable //! search engine and quick-jump tool in one small binary. For information on //! usage, please take a look at the readme. use crate::config::{ - get_config_data, load_custom_path_config, read_config, ConfigData, Route, - RouteGroup, + get_config_data, load_custom_file, load_file, FileData, Route, RouteGroup, }; use anyhow::Result; use arc_swap::ArcSwap; @@ -50,15 +50,15 @@ async fn main() -> Result<()> { init_logger(opts.verbose, opts.quiet)?; let conf_data = match opts.config { - Some(file_name) => load_custom_path_config(file_name), + Some(file_name) => load_custom_file(file_name), None => get_config_data(), }?; - let conf = read_config(conf_data.file.try_clone()?, opts.large_config)?; + let conf = load_file(conf_data.file.try_clone()?, opts.large_config)?; let state = Arc::from(ArcSwap::from_pointee(State { public_address: conf.public_address, default_route: conf.default_route, - routes: cache_routes(&conf.groups), + routes: cache_routes(conf.groups.clone()), groups: conf.groups, })); @@ -106,14 +106,15 @@ fn init_logger(num_verbose_flags: u8, num_quiet_flags: u8) -> Result<()> { /// Generates a hashmap of routes from the data structure created by the config /// file. This should improve runtime performance and is a better solution than /// just iterating over the config object for every hop resolution. -fn cache_routes(groups: &[RouteGroup]) -> HashMap { +fn cache_routes(groups: Vec) -> HashMap { let mut mapping = HashMap::new(); for group in groups { - for (kw, dest) in &group.routes { + for (kw, dest) in group.routes { + // This function isn't called often enough to not be a performance issue. match mapping.insert(kw.clone(), dest.clone()) { - None => trace!("Inserting {} into mapping.", kw), + None => trace!("Inserting {kw} into mapping."), Some(old_value) => { - trace!("Overriding {} route from {} to {}.", kw, old_value, dest) + trace!("Overriding {kw} route from {old_value} to {dest}."); } } } @@ -158,16 +159,14 @@ fn compile_templates() -> Result> { #[cfg(not(tarpaulin_include))] fn start_watch( state: Arc>, - config_data: ConfigData, + config_data: FileData, large_config: bool, ) -> Result { let mut watch = Hotwatch::new_with_custom_delay(Duration::from_millis(500))?; - let ConfigData { path, mut file } = config_data; + let FileData { path, mut file } = config_data; let watch_result = watch.watch(&path, move |e: Event| { if let Event::Create(ref path) = e { - file = load_custom_path_config(path) - .expect("file to exist at path") - .file; + file = load_custom_file(path).expect("file to exist at path").file; trace!("Getting new file handler as file was recreated."); } @@ -175,7 +174,7 @@ fn start_watch( Event::Write(_) | Event::Create(_) => { trace!("Grabbing writer lock on state..."); trace!("Obtained writer lock on state!"); - match read_config( + match load_file( file.try_clone().expect("Failed to clone file handle"), large_config, ) { @@ -183,7 +182,7 @@ fn start_watch( state.store(Arc::new(State { public_address: conf.public_address, default_route: conf.default_route, - routes: cache_routes(&conf.groups), + routes: cache_routes(conf.groups.clone()), groups: conf.groups, })); info!("Successfully updated active state"); @@ -198,7 +197,9 @@ fn start_watch( match watch_result { Ok(_) => info!("Watcher is now watching {path:?}"), Err(e) => { - warn!("Couldn't watch {path:?}: {e}. Changes to this file won't be seen!",) + warn!( + "Couldn't watch {path:?}: {e}. Changes to this file won't be seen!" + ); } } @@ -255,7 +256,7 @@ mod cache_routes { #[test] fn empty_groups_yield_empty_routes() { - assert_eq!(cache_routes(&[]), HashMap::new()); + assert_eq!(cache_routes(Vec::new()), HashMap::new()); } #[test] @@ -275,7 +276,7 @@ mod cache_routes { }; assert_eq!( - cache_routes(&[group1, group2]), + cache_routes(vec![group1, group2]), generate_external_routes(&[ ("a", "b"), ("c", "d"), @@ -302,7 +303,7 @@ mod cache_routes { }; assert_eq!( - cache_routes(&[group1.clone(), group2]), + cache_routes(vec![group1.clone(), group2]), generate_external_routes(&[("a", "1"), ("c", "2")]) ); @@ -314,7 +315,7 @@ mod cache_routes { }; assert_eq!( - cache_routes(&[group1, group3]), + cache_routes(vec![group1, group3]), generate_external_routes(&[("a", "1"), ("b", "2"), ("c", "d")]) ); } diff --git a/src/routes.rs b/src/routes.rs index 4fa0587..5b19559 100644 --- a/src/routes.rs +++ b/src/routes.rs @@ -11,11 +11,11 @@ use log::{debug, error}; use percent_encoding::{utf8_percent_encode, AsciiSet, CONTROLS}; use serde::Deserialize; use std::collections::HashMap; -use std::path::PathBuf; +use std::path::Path; use std::process::Command; use std::sync::Arc; -/// https://url.spec.whatwg.org/#fragment-percent-encode-set +// https://url.spec.whatwg.org/#fragment-percent-encode-set const FRAGMENT_ENCODE_SET: &AsciiSet = &CONTROLS .add(b' ') .add(b'"') @@ -27,6 +27,7 @@ const FRAGMENT_ENCODE_SET: &AsciiSet = &CONTROLS .add(b'#') // Interpreted as a hyperlink section target .add(b'\''); +#[allow(clippy::unused_async)] pub async fn index( Extension(data): Extension>>, Extension(handlebars): Extension>, @@ -40,6 +41,7 @@ pub async fn index( .map_err(|_| StatusCode::INTERNAL_SERVER_ERROR) } +#[allow(clippy::unused_async)] pub async fn opensearch( Extension(data): Extension>>, Extension(handlebars): Extension>, @@ -62,6 +64,7 @@ pub async fn opensearch( .map_err(|_| StatusCode::INTERNAL_SERVER_ERROR) } +#[allow(clippy::unused_async)] pub async fn list( Extension(data): Extension>>, Extension(handlebars): Extension>, @@ -77,6 +80,7 @@ pub struct SearchQuery { to: String, } +#[allow(clippy::unused_async)] pub async fn hop( Extension(data): Extension>>, Extension(handlebars): Extension>, @@ -91,7 +95,7 @@ pub async fn hop( route_type: RouteType::Internal, path, .. - } => resolve_path(PathBuf::from(path), &args), + } => resolve_path(Path::new(path), &args), ConfigRoute { route_type: RouteType::External, path, @@ -190,7 +194,7 @@ fn resolve_hop<'a>( /// Checks if the user provided string has the correct properties required by /// the route to be successfully matched. -fn check_route(route: &Route, arg_count: usize) -> bool { +const fn check_route(route: &Route, arg_count: usize) -> bool { if let Some(min_args) = route.min_args { if arg_count < min_args { return false; @@ -217,7 +221,7 @@ enum HopAction { /// so long as the executable was successfully executed. Returns an Error if the /// file doesn't exist or bunbun did not have permission to read and execute the /// file. -fn resolve_path(path: PathBuf, args: &str) -> Result { +fn resolve_path(path: &Path, args: &str) -> Result { let output = Command::new(path.canonicalize()?) .args(args.split(' ')) .output()?; @@ -369,18 +373,16 @@ mod resolve_path { use super::{resolve_path, HopAction}; use anyhow::Result; use std::env::current_dir; - use std::path::PathBuf; + use std::path::{Path, PathBuf}; #[test] fn invalid_path_returns_err() { - assert!(resolve_path(PathBuf::from("/bin/aaaa"), "aaaa").is_err()); + assert!(resolve_path(&Path::new("/bin/aaaa"), "aaaa").is_err()); } #[test] fn valid_path_returns_ok() { - assert!( - resolve_path(PathBuf::from("/bin/echo"), r#"{"body": "a"}"#).is_ok() - ); + assert!(resolve_path(&Path::new("/bin/echo"), r#"{"body": "a"}"#).is_ok()); } #[test] @@ -389,7 +391,7 @@ mod resolve_path { let nest_level = current_dir()?.ancestors().count() - 1; let mut rel_path = PathBuf::from("../".repeat(nest_level)); rel_path.push("./bin/echo"); - assert!(resolve_path(rel_path, r#"{"body": "a"}"#).is_ok()); + assert!(resolve_path(&rel_path, r#"{"body": "a"}"#).is_ok()); Ok(()) } @@ -399,7 +401,7 @@ mod resolve_path { // Trying to run a command without permission format!( "{}", - resolve_path(PathBuf::from("/root/some_exec"), "").unwrap_err() + resolve_path(&Path::new("/root/some_exec"), "").unwrap_err() ) .contains("Permission denied") ); @@ -408,13 +410,13 @@ mod resolve_path { #[test] fn non_success_exit_code_yields_err() { // cat-ing a folder always returns exit code 1 - assert!(resolve_path(PathBuf::from("/bin/cat"), "/").is_err()); + assert!(resolve_path(&Path::new("/bin/cat"), "/").is_err()); } #[test] fn return_body() -> Result<()> { assert_eq!( - resolve_path(PathBuf::from("/bin/echo"), r#"{"body": "a"}"#)?, + resolve_path(&Path::new("/bin/echo"), r#"{"body": "a"}"#)?, HopAction::Body("a".to_string()) ); @@ -424,7 +426,7 @@ mod resolve_path { #[test] fn return_redirect() -> Result<()> { assert_eq!( - resolve_path(PathBuf::from("/bin/echo"), r#"{"redirect": "a"}"#)?, + resolve_path(&Path::new("/bin/echo"), r#"{"redirect": "a"}"#)?, HopAction::Redirect("a".to_string()) ); Ok(()) diff --git a/src/template_args.rs b/src/template_args.rs index d9b84fe..ae85c7e 100644 --- a/src/template_args.rs +++ b/src/template_args.rs @@ -3,7 +3,7 @@ use std::borrow::Cow; use percent_encoding::PercentEncode; use serde::Serialize; -pub fn query<'a>(query: PercentEncode<'a>) -> impl Serialize + 'a { +pub fn query(query: PercentEncode<'_>) -> impl Serialize + '_ { #[derive(Serialize)] struct TemplateArgs<'a> { query: Cow<'a, str>, @@ -13,7 +13,7 @@ pub fn query<'a>(query: PercentEncode<'a>) -> impl Serialize + 'a { } } -pub fn hostname<'a>(hostname: &'a str) -> impl Serialize + 'a { +pub fn hostname(hostname: &'_ str) -> impl Serialize + '_ { #[derive(Serialize)] pub struct TemplateArgs<'a> { pub hostname: &'a str,