fix(config): fix warnings

This commit is contained in:
Federico Terzi 2021-10-05 22:05:53 +02:00
parent d8412865f7
commit 42d4351f4b
10 changed files with 53 additions and 64 deletions

View File

@ -240,7 +240,7 @@ impl Config for ResolvedConfig {
} }
fn word_separators(&self) -> Vec<String> { fn word_separators(&self) -> Vec<String> {
self.parsed.word_separators.clone().unwrap_or(vec![ self.parsed.word_separators.clone().unwrap_or_else(|| vec![
" ".to_string(), " ".to_string(),
",".to_string(), ",".to_string(),
".".to_string(), ".".to_string(),

View File

@ -22,8 +22,8 @@ use crate::error::NonFatalErrorSet;
use super::{resolve::ResolvedConfig, Config, ConfigStore, ConfigStoreError}; use super::{resolve::ResolvedConfig, Config, ConfigStore, ConfigStoreError};
use anyhow::{Context, Result}; use anyhow::{Context, Result};
use log::{debug, error}; use log::{debug, error};
use std::{collections::HashSet, path::Path};
use std::sync::Arc; use std::sync::Arc;
use std::{collections::HashSet, path::Path};
pub(crate) struct DefaultConfigStore { pub(crate) struct DefaultConfigStore {
default: Arc<dyn Config>, default: Arc<dyn Config>,
@ -39,18 +39,17 @@ impl ConfigStore for DefaultConfigStore {
// Find a custom config that matches or fallback to the default one // Find a custom config that matches or fallback to the default one
for custom in self.customs.iter() { for custom in self.customs.iter() {
if custom.is_match(app) { if custom.is_match(app) {
return Arc::clone(&custom); return Arc::clone(custom);
} }
} }
Arc::clone(&self.default) Arc::clone(&self.default)
} }
fn configs(&self) -> Vec<Arc<dyn Config>> { fn configs(&self) -> Vec<Arc<dyn Config>> {
let mut configs = Vec::new(); let mut configs = vec![Arc::clone(&self.default)];
configs.push(Arc::clone(&self.default));
for custom in self.customs.iter() { for custom in self.customs.iter() {
configs.push(Arc::clone(&custom)); configs.push(Arc::clone(custom));
} }
configs configs
@ -83,8 +82,8 @@ impl DefaultConfigStore {
let mut non_fatal_errors = Vec::new(); let mut non_fatal_errors = Vec::new();
let default = let default = ResolvedConfig::load(&default_file, None)
ResolvedConfig::load(&default_file, None).context("failed to load default.yml configuration")?; .context("failed to load default.yml configuration")?;
debug!("loaded default config at path: {:?}", default_file); debug!("loaded default config at path: {:?}", default_file);
// Then the others // Then the others

View File

@ -560,8 +560,7 @@ impl LegacyConfigSet {
let mut name_set = HashSet::new(); let mut name_set = HashSet::new();
let mut children_map: HashMap<String, Vec<LegacyConfig>> = HashMap::new(); let mut children_map: HashMap<String, Vec<LegacyConfig>> = HashMap::new();
let mut package_map: HashMap<String, Vec<LegacyConfig>> = HashMap::new(); let mut package_map: HashMap<String, Vec<LegacyConfig>> = HashMap::new();
let mut root_configs = Vec::new(); let mut root_configs = vec![default];
root_configs.push(default);
let mut file_loader = |entry: walkdir::Result<DirEntry>, let mut file_loader = |entry: walkdir::Result<DirEntry>,
dest_map: &mut HashMap<String, Vec<LegacyConfig>>| dest_map: &mut HashMap<String, Vec<LegacyConfig>>|
@ -592,7 +591,7 @@ impl LegacyConfigSet {
return Ok(()); return Ok(());
} }
let mut config = LegacyConfig::load_config(&path)?; let mut config = LegacyConfig::load_config(path)?;
// Make sure the config does not contain reserved fields // Make sure the config does not contain reserved fields
if !config.validate_user_defined_config() { if !config.validate_user_defined_config() {
@ -811,7 +810,7 @@ mod tests {
#[test] #[test]
fn test_config_file_not_found() { fn test_config_file_not_found() {
let config = LegacyConfig::load_config(Path::new("invalid/path")); let config = LegacyConfig::load_config(Path::new("invalid/path"));
assert_eq!(config.is_err(), true); assert!(config.is_err());
assert_eq!(config.unwrap_err(), ConfigLoadError::FileNotFound); assert_eq!(config.unwrap_err(), ConfigLoadError::FileNotFound);
} }
@ -833,13 +832,13 @@ mod tests {
let mut result = true; let mut result = true;
validate_field!(result, 3, 3); validate_field!(result, 3, 3);
assert_eq!(result, true); assert!(result);
validate_field!(result, 10, 3); validate_field!(result, 10, 3);
assert_eq!(result, false); assert!(!result);
validate_field!(result, 3, 3); validate_field!(result, 3, 3);
assert_eq!(result, false); assert!(!result);
} }
#[test] #[test]
@ -852,7 +851,7 @@ mod tests {
"###, "###,
); );
let config = LegacyConfig::load_config(working_config_file.path()); let config = LegacyConfig::load_config(working_config_file.path());
assert_eq!(config.unwrap().validate_user_defined_config(), true); assert!(config.unwrap().validate_user_defined_config());
} }
#[test] #[test]
@ -866,7 +865,7 @@ mod tests {
"###, "###,
); );
let config = LegacyConfig::load_config(working_config_file.path()); let config = LegacyConfig::load_config(working_config_file.path());
assert_eq!(config.unwrap().validate_user_defined_config(), false); assert!(!config.unwrap().validate_user_defined_config());
} }
#[test] #[test]
@ -880,7 +879,7 @@ mod tests {
"###, "###,
); );
let config = LegacyConfig::load_config(working_config_file.path()); let config = LegacyConfig::load_config(working_config_file.path());
assert_eq!(config.unwrap().validate_user_defined_config(), false); assert!(!config.unwrap().validate_user_defined_config());
} }
#[test] #[test]
@ -894,7 +893,7 @@ mod tests {
"###, "###,
); );
let config = LegacyConfig::load_config(working_config_file.path()); let config = LegacyConfig::load_config(working_config_file.path());
assert_eq!(config.unwrap().validate_user_defined_config(), false); assert!(!config.unwrap().validate_user_defined_config());
} }
#[test] #[test]
@ -908,14 +907,14 @@ mod tests {
"###, "###,
); );
let config = LegacyConfig::load_config(working_config_file.path()); let config = LegacyConfig::load_config(working_config_file.path());
assert_eq!(config.unwrap().validate_user_defined_config(), false); assert!(!config.unwrap().validate_user_defined_config());
} }
#[test] #[test]
fn test_config_loaded_correctly() { fn test_config_loaded_correctly() {
let working_config_file = create_tmp_file(TEST_WORKING_CONFIG_FILE); let working_config_file = create_tmp_file(TEST_WORKING_CONFIG_FILE);
let config = LegacyConfig::load_config(working_config_file.path()); let config = LegacyConfig::load_config(working_config_file.path());
assert_eq!(config.is_ok(), true); assert!(config.is_ok());
} }
// Test ConfigSet // Test ConfigSet
@ -936,7 +935,7 @@ mod tests {
(data_dir, package_dir) (data_dir, package_dir)
} }
pub fn create_temp_file_in_dir(tmp_dir: &PathBuf, name: &str, content: &str) -> PathBuf { pub fn create_temp_file_in_dir(tmp_dir: &Path, name: &str, content: &str) -> PathBuf {
let user_defined_path = tmp_dir.join(name); let user_defined_path = tmp_dir.join(name);
let user_defined_path_copy = user_defined_path.clone(); let user_defined_path_copy = user_defined_path.clone();
fs::write(user_defined_path, content).unwrap(); fs::write(user_defined_path, content).unwrap();
@ -979,7 +978,7 @@ mod tests {
fn test_config_set_load_fail_bad_directory() { fn test_config_set_load_fail_bad_directory() {
let config_set = let config_set =
LegacyConfigSet::load(Path::new("invalid/path"), Path::new("invalid/path")); LegacyConfigSet::load(Path::new("invalid/path"), Path::new("invalid/path"));
assert_eq!(config_set.is_err(), true); assert!(config_set.is_err());
assert_eq!( assert_eq!(
config_set.unwrap_err(), config_set.unwrap_err(),
ConfigLoadError::InvalidConfigDirectory ConfigLoadError::InvalidConfigDirectory
@ -992,7 +991,7 @@ mod tests {
let package_dir = TempDir::new().expect("unable to create package directory"); let package_dir = TempDir::new().expect("unable to create package directory");
let config_set = LegacyConfigSet::load(data_dir.path(), package_dir.path()); let config_set = LegacyConfigSet::load(data_dir.path(), package_dir.path());
assert_eq!(config_set.is_err(), true); assert!(config_set.is_err());
assert_eq!(config_set.unwrap_err(), ConfigLoadError::FileNotFound); assert_eq!(config_set.unwrap_err(), ConfigLoadError::FileNotFound);
} }
@ -1607,9 +1606,8 @@ mod tests {
#[test] #[test]
fn test_list_has_conflict_no_conflict() { fn test_list_has_conflict_no_conflict() {
assert_eq!( assert!(
LegacyConfigSet::list_has_conflicts(&[":ab".to_owned(), ":bc".to_owned()]), !LegacyConfigSet::list_has_conflicts(&[":ab".to_owned(), ":bc".to_owned()])
false
); );
} }
@ -1617,7 +1615,7 @@ mod tests {
fn test_list_has_conflict_conflict() { fn test_list_has_conflict_conflict() {
let mut list = vec!["ac".to_owned(), "ab".to_owned(), "abc".to_owned()]; let mut list = vec!["ac".to_owned(), "ab".to_owned(), "abc".to_owned()];
list.sort(); list.sort();
assert_eq!(LegacyConfigSet::list_has_conflicts(&list), true); assert!(LegacyConfigSet::list_has_conflicts(&list));
} }
#[test] #[test]
@ -1645,9 +1643,8 @@ mod tests {
); );
let config_set = LegacyConfigSet::load(data_dir.path(), package_dir.path()).unwrap(); let config_set = LegacyConfigSet::load(data_dir.path(), package_dir.path()).unwrap();
assert_eq!( assert!(
LegacyConfigSet::has_conflicts(&config_set.default, &config_set.specific), !LegacyConfigSet::has_conflicts(&config_set.default, &config_set.specific),
false
); );
} }
@ -1678,9 +1675,8 @@ mod tests {
); );
let config_set = LegacyConfigSet::load(data_dir.path(), package_dir.path()).unwrap(); let config_set = LegacyConfigSet::load(data_dir.path(), package_dir.path()).unwrap();
assert_eq!( assert!(
LegacyConfigSet::has_conflicts(&config_set.default, &config_set.specific), LegacyConfigSet::has_conflicts(&config_set.default, &config_set.specific),
true
); );
} }
@ -1709,9 +1705,8 @@ mod tests {
); );
let config_set = LegacyConfigSet::load(data_dir.path(), package_dir.path()).unwrap(); let config_set = LegacyConfigSet::load(data_dir.path(), package_dir.path()).unwrap();
assert_eq!( assert!(
LegacyConfigSet::has_conflicts(&config_set.default, &config_set.specific), LegacyConfigSet::has_conflicts(&config_set.default, &config_set.specific),
true
); );
} }
@ -1751,9 +1746,8 @@ mod tests {
); );
let config_set = LegacyConfigSet::load(data_dir.path(), package_dir.path()).unwrap(); let config_set = LegacyConfigSet::load(data_dir.path(), package_dir.path()).unwrap();
assert_eq!( assert!(
LegacyConfigSet::has_conflicts(&config_set.default, &config_set.specific), !LegacyConfigSet::has_conflicts(&config_set.default, &config_set.specific),
false
); );
} }

View File

@ -414,11 +414,7 @@ impl LegacyMatchStore {
impl MatchStore for LegacyMatchStore { impl MatchStore for LegacyMatchStore {
fn query(&self, paths: &[String]) -> MatchSet { fn query(&self, paths: &[String]) -> MatchSet {
let group = if !paths.is_empty() { let group = if !paths.is_empty() {
if let Some(group) = self.groups.get(&paths[0]) { self.groups.get(&paths[0])
Some(group)
} else {
None
}
} else { } else {
None None
}; };
@ -437,7 +433,7 @@ impl MatchStore for LegacyMatchStore {
} }
fn loaded_paths(&self) -> Vec<String> { fn loaded_paths(&self) -> Vec<String> {
self.groups.keys().map(|key| key.clone()).collect() self.groups.keys().cloned().collect()
} }
} }

View File

@ -20,6 +20,7 @@
use serde::{Deserialize, Serialize}; use serde::{Deserialize, Serialize};
#[allow(non_camel_case_types)] #[allow(non_camel_case_types)]
#[allow(clippy::upper_case_acronyms)]
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
pub enum KeyModifier { pub enum KeyModifier {
CTRL, CTRL,

View File

@ -33,6 +33,7 @@ mod legacy;
pub mod matches; pub mod matches;
mod util; mod util;
#[allow(clippy::type_complexity)]
pub fn load(base_path: &Path) -> Result<(Box<dyn ConfigStore>, Box<dyn MatchStore>, Vec<error::NonFatalErrorSet>)> { pub fn load(base_path: &Path) -> Result<(Box<dyn ConfigStore>, Box<dyn MatchStore>, Vec<error::NonFatalErrorSet>)> {
let config_dir = base_path.join("config"); let config_dir = base_path.join("config");
if !config_dir.exists() || !config_dir.is_dir() { if !config_dir.exists() || !config_dir.is_dir() {
@ -125,7 +126,7 @@ mod tests {
) )
.unwrap(); .unwrap();
let (config_store, match_store, errors) = load(&base).unwrap(); let (config_store, match_store, errors) = load(base).unwrap();
assert_eq!(errors.len(), 0); assert_eq!(errors.len(), 0);
assert_eq!(config_store.default().match_paths().len(), 2); assert_eq!(config_store.default().match_paths().len(), 2);
@ -220,7 +221,7 @@ mod tests {
) )
.unwrap(); .unwrap();
let (config_store, match_store, errors) = load(&base).unwrap(); let (config_store, match_store, errors) = load(base).unwrap();
assert_eq!(errors.len(), 3); assert_eq!(errors.len(), 3);
// It shouldn't have loaded the "config.yml" one because of the YAML error // It shouldn't have loaded the "config.yml" one because of the YAML error
@ -248,7 +249,7 @@ mod tests {
let config_file = config_dir.join("default.yml"); let config_file = config_dir.join("default.yml");
std::fs::write(&config_file, r#""#).unwrap(); std::fs::write(&config_file, r#""#).unwrap();
let (config_store, match_store, errors) = load(&base).unwrap(); let (config_store, match_store, errors) = load(base).unwrap();
assert_eq!(errors.len(), 1); assert_eq!(errors.len(), 1);
assert_eq!(errors[0].file, base_file); assert_eq!(errors[0].file, base_file);
@ -286,7 +287,7 @@ mod tests {
"#).unwrap(); "#).unwrap();
// A syntax error in the default.yml file cannot be handled gracefully // A syntax error in the default.yml file cannot be handled gracefully
assert!(load(&base).is_err()); assert!(load(base).is_err());
}); });
} }
@ -294,7 +295,7 @@ mod tests {
fn load_without_valid_config_dir() { fn load_without_valid_config_dir() {
use_test_directory(|_, match_dir, _| { use_test_directory(|_, match_dir, _| {
// To correcly load the configs, the "load" method looks for the "config" directory // To correcly load the configs, the "load" method looks for the "config" directory
assert!(load(&match_dir).is_err()); assert!(load(match_dir).is_err());
}); });
} }
} }

View File

@ -111,8 +111,8 @@ impl Importer for YAMLImporter {
Ok(( Ok((
MatchGroup { MatchGroup {
imports: resolved_imports, imports: resolved_imports,
global_vars: global_vars, global_vars,
matches: matches, matches,
}, },
non_fatal_error_set, non_fatal_error_set,
)) ))
@ -130,10 +130,8 @@ pub fn try_convert_into_match(yaml_match: YAMLMatch) -> Result<(Match, Vec<Warni
let triggers = if let Some(trigger) = yaml_match.trigger { let triggers = if let Some(trigger) = yaml_match.trigger {
Some(vec![trigger]) Some(vec![trigger])
} else if let Some(triggers) = yaml_match.triggers {
Some(triggers)
} else { } else {
None yaml_match.triggers
}; };
let uppercase_style = match yaml_match let uppercase_style = match yaml_match

View File

@ -50,9 +50,9 @@ impl Default for Match {
impl Match { impl Match {
// TODO: test // TODO: test
pub fn description<'a>(&'a self) -> &'a str { pub fn description(&self) -> &str {
if let Some(label) = &self.label { if let Some(label) = &self.label {
&label label
} else if let MatchEffect::Text(text_effect) = &self.effect { } else if let MatchEffect::Text(text_effect) = &self.effect {
&text_effect.replace &text_effect.replace
} else if let MatchEffect::Image(_) = &self.effect { } else if let MatchEffect::Image(_) = &self.effect {
@ -63,7 +63,7 @@ impl Match {
} }
// TODO: test // TODO: test
pub fn cause_description<'a>(&'a self) -> Option<&'a str> { pub fn cause_description(&self) -> Option<&str> {
self.cause.description() self.cause.description()
} }
} }
@ -80,7 +80,7 @@ pub enum MatchCause {
impl MatchCause { impl MatchCause {
// TODO: test // TODO: test
pub fn description<'a>(&'a self) -> Option<&'a str> { pub fn description(&self) -> Option<&str> {
if let MatchCause::Trigger(trigger_cause) = &self { if let MatchCause::Trigger(trigger_cause) = &self {
trigger_cause.triggers.first().map(|s| s.as_str()) trigger_cause.triggers.first().map(|s| s.as_str())
} else { } else {
@ -91,7 +91,7 @@ impl MatchCause {
} }
// TODO: test // TODO: test
pub fn long_description<'a>(&'a self) -> String { pub fn long_description(&self) -> String {
if let MatchCause::Trigger(trigger_cause) = &self { if let MatchCause::Trigger(trigger_cause) = &self {
format!("triggers: {:?}", trigger_cause.triggers) format!("triggers: {:?}", trigger_cause.triggers)
} else { } else {

View File

@ -72,7 +72,7 @@ impl MatchStore for DefaultMatchStore {
} }
fn loaded_paths(&self) -> Vec<String> { fn loaded_paths(&self) -> Vec<String> {
self.groups.keys().map(|key| key.clone()).collect() self.groups.keys().cloned().collect()
} }
} }

View File

@ -54,21 +54,21 @@ pub mod tests {
#[test] #[test]
fn is_yaml_empty_document_empty() { fn is_yaml_empty_document_empty() {
assert_eq!(is_yaml_empty(""), true); assert!(is_yaml_empty(""));
} }
#[test] #[test]
fn is_yaml_empty_document_with_comments() { fn is_yaml_empty_document_with_comments() {
assert_eq!(is_yaml_empty("\n#comment \n \n"), true); assert!(is_yaml_empty("\n#comment \n \n"));
} }
#[test] #[test]
fn is_yaml_empty_document_with_comments_and_content() { fn is_yaml_empty_document_with_comments_and_content() {
assert_eq!(is_yaml_empty("\n#comment \n field: true\n"), false); assert!(!is_yaml_empty("\n#comment \n field: true\n"));
} }
#[test] #[test]
fn is_yaml_empty_document_with_content() { fn is_yaml_empty_document_with_content() {
assert_eq!(is_yaml_empty("\nfield: true\n"), false); assert!(!is_yaml_empty("\nfield: true\n"));
} }
} }