diff --git a/Cargo.toml b/Cargo.toml index 47b88c26c..8253d3fac 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -95,3 +95,39 @@ codegen-units = 1 [lib] crate-type = ["staticlib", "cdylib", "rlib"] + + + +[lints] +workspace = true + +[workspace.lints.clippy] +#perf = { level = "deny", priority = -1 } +#pedantic = { level = "warn", priority = -1 } +#nursery = { level = "warn", priority = -1 } +#cargo = { level = "warn", priority = -1 } +#panic = "deny" +#unwrap_used = "deny" +clone_on_ref_ptr = "warn" +empty_enum_variants_with_brackets = "warn" +empty_structs_with_brackets = "warn" +enum_glob_use = "warn" +error_impl_error = "warn" +format_push_string = "warn" +infinite_loop = "warn" +rc_buffer = "warn" +rc_mutex = "warn" +expect_used = "warn" +missing_docs_in_private_items = "allow" +cargo_common_metadata = "allow" # should be removed if we release our crates to crates.io +cast_possible_truncation = "allow" # should this be on allow? +cast_precision_loss = "allow" +cast_sign_loss = "allow" +cast_possible_wrap = "allow" +or_fun_call = "allow" +multiple_crate_versions = "allow" # should this be on allow? +missing_errors_doc = "allow" +cognitive_complexity = "allow" +float_cmp = "allow" #TODO: this should be changed and f32/f64::EPSILON should be used +doc_markdown = "allow" +too_many_lines = "allow" \ No newline at end of file diff --git a/benches/tree_iterator.rs b/benches/tree_iterator.rs index 0532ab999..0c430f4df 100644 --- a/benches/tree_iterator.rs +++ b/benches/tree_iterator.rs @@ -1,3 +1,4 @@ +#[allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)] use std::fs::File; use criterion::{criterion_group, criterion_main, Criterion}; @@ -25,7 +26,7 @@ fn wikipedia_main_page(c: &mut Criterion) { b.iter(|| { let tree_iterator = TreeIterator::new(&main_document); let _ = tree_iterator.collect::>(); - }) + }); }); group.finish(); @@ -51,7 +52,7 @@ fn stackoverflow_home(c: &mut Criterion) { b.iter(|| { let tree_iterator = TreeIterator::new(&main_document); let _ = tree_iterator.collect::>(); - }) + }); }); group.finish(); diff --git a/clippy.toml b/clippy.toml new file mode 100644 index 000000000..7bada5473 --- /dev/null +++ b/clippy.toml @@ -0,0 +1,3 @@ +allow-unwrap-in-tests = true +allow-expect-in-tests = true +allow-panic-in-tests = true diff --git a/crates/gosub_bindings/Cargo.toml b/crates/gosub_bindings/Cargo.toml index e6d390d72..861d6eeb3 100644 --- a/crates/gosub_bindings/Cargo.toml +++ b/crates/gosub_bindings/Cargo.toml @@ -12,3 +12,7 @@ gosub_html5 = { path = "../gosub_html5" } [lib] crate-type = ["staticlib"] + + +[lints] +workspace = true \ No newline at end of file diff --git a/crates/gosub_bindings/src/lib.rs b/crates/gosub_bindings/src/lib.rs index 0a61eb7c3..6410bcefb 100644 --- a/crates/gosub_bindings/src/lib.rs +++ b/crates/gosub_bindings/src/lib.rs @@ -19,8 +19,8 @@ use wrapper::node::CNode; /// to build a render tree. DO NOT take ownership of this pointer in Rust or the /// universe might collapse. /// -/// Moves an owning pointer to the rendertree using Box::into_raw() to the C API. -/// This pointer MUST be passed to gosub_rendertree_free() after usage for proper cleanup. +/// Moves an owning pointer to the rendertree using `Box::into_raw()` to the C API. +/// This pointer MUST be passed to `gosub_rendertree_free()` after usage for proper cleanup. #[no_mangle] pub unsafe extern "C" fn gosub_rendertree_init(html: *const c_char) -> *mut RenderTree { let html_str = unsafe { @@ -50,8 +50,8 @@ pub unsafe extern "C" fn gosub_rendertree_init(html: *const c_char) -> *mut Rend /// Construct a tree iterator for a render tree and return an owning pointer to it. /// /// # Safety -/// Moves an owning pointer to the tree iterator using Box::into_raw() to the C API. -/// This pointer MUST be passed to gosub_rendertree_iterator_free() after usage for proper cleanup. +/// Moves an owning pointer to the tree iterator using `Box::into_raw()` to the C API. +/// This pointer MUST be passed to `gosub_rendertree_iterator_free()` after usage for proper cleanup. #[no_mangle] pub unsafe extern "C" fn gosub_rendertree_iterator_init( rendertree: *const RenderTree, @@ -60,10 +60,10 @@ pub unsafe extern "C" fn gosub_rendertree_iterator_init( Box::into_raw(tree_iterator) } -/// Takes a tree_iterator and returns a non-owning pointer to the next node +/// Takes a `tree_iterator` and returns a non-owning pointer to the next node /// /// # Safety -/// Takes a tree_iterator pointer (owned by the C API generated by gosub_rendertree_iterator_init()) +/// Takes a `tree_iterator` pointer (owned by the C API generated by `gosub_rendertree_iterator_init()`) /// and modifies it to point to the next tree-order node in the tree. Any heap-allocated data /// on the current node is free'd before pointing to the next node. Returns a ready-only pointer /// to the next node. @@ -73,16 +73,16 @@ pub unsafe extern "C" fn gosub_rendertree_next_node( ) -> *const Node { let next = (*tree_iterator).next(); if let Some(next) = next { - next.as_ptr() as *const Node + next.as_ptr().cast_const() } else { ptr::null() } } -/// Fetch the node data according to the NodeType of the current node. +/// Fetch the node data according to the `NodeType` of the current node. /// /// # Safety -/// Uses a read-only pointer obtained from gosub_rendertree_next_node() +/// Uses a read-only pointer obtained from `gosub_rendertree_next_node()` /// and a mutable pointer owned by the C API to write (copy) the contents /// of the read-only pointer into the mutable pointer. #[no_mangle] @@ -93,7 +93,7 @@ pub unsafe extern "C" fn gosub_rendertree_get_node_data(node: *const Node, c_nod } } -/// Free the iterator pointer obtained from gosub_rendertree_iterator_init() +/// Free the iterator pointer obtained from `gosub_rendertree_iterator_init()` /// /// # Safety /// This takes ownership of the pointer from the C API and transfers it to Rust so it can @@ -103,7 +103,7 @@ pub unsafe extern "C" fn gosub_rendertree_iterator_free(tree_iterator: *mut Tree let _ = Box::from_raw(tree_iterator); } -/// Free the rendertree pointer obtained from gosub_rendertree_init() +/// Free the rendertree pointer obtained from `gosub_rendertree_init()` /// /// # Safety /// This takes ownership of the pointer from the C API and transfers it to Rust so it can diff --git a/crates/gosub_bindings/src/wrapper.rs b/crates/gosub_bindings/src/wrapper.rs index 705f04f73..30fd46def 100644 --- a/crates/gosub_bindings/src/wrapper.rs +++ b/crates/gosub_bindings/src/wrapper.rs @@ -1,7 +1,7 @@ pub mod node; pub mod text; -/// Numerical values that map rendertree::NodeType to C +/// Numerical values that map `rendertree::NodeType` to C #[repr(C)] pub enum CNodeType { Root = 0, diff --git a/crates/gosub_bindings/src/wrapper/node.rs b/crates/gosub_bindings/src/wrapper/node.rs index 10e35027f..4649b6833 100644 --- a/crates/gosub_bindings/src/wrapper/node.rs +++ b/crates/gosub_bindings/src/wrapper/node.rs @@ -31,10 +31,12 @@ impl Default for CNode { } impl CNode { + #[must_use] pub fn new_root() -> Self { Self::default() } + #[must_use] pub fn new_text(node: &Node, text_node: &TextNode) -> Self { Self { tag: CNodeType::Text, diff --git a/crates/gosub_bindings/src/wrapper/text.rs b/crates/gosub_bindings/src/wrapper/text.rs index 78c89312b..44705cd85 100644 --- a/crates/gosub_bindings/src/wrapper/text.rs +++ b/crates/gosub_bindings/src/wrapper/text.rs @@ -2,7 +2,7 @@ use gosub_rendering::render_tree::text::TextNode; use std::ffi::c_char; use std::ffi::CString; -/// This is a C-friendly wrapper around gosub_render_utils::rendertree::text::TextNode +/// This is a C-friendly wrapper around `gosub_render_utils::rendertree::text::TextNode` /// that converts Rust Strings to owned pointers to pass to the C API. #[repr(C)] pub struct CTextNode { diff --git a/crates/gosub_config/Cargo.toml b/crates/gosub_config/Cargo.toml index cfabf5fa2..3017c07fd 100644 --- a/crates/gosub_config/Cargo.toml +++ b/crates/gosub_config/Cargo.toml @@ -19,4 +19,7 @@ anyhow = "1.0.86" [target.'cfg(not(target_arch = "wasm32"))'.dependencies] sqlite = "0.36.1" -ureq = "2.10.1" \ No newline at end of file +ureq = "2.10.1" + +[lints] +workspace = true diff --git a/crates/gosub_config/src/errors.rs b/crates/gosub_config/src/errors.rs index 4b4f9b93d..4521737c7 100644 --- a/crates/gosub_config/src/errors.rs +++ b/crates/gosub_config/src/errors.rs @@ -2,7 +2,7 @@ use thiserror::Error; /// Parser error that defines an error (message) on the given position -#[derive(Clone, Debug, PartialEq)] +#[derive(Clone, Debug, PartialEq, Eq)] pub struct ParseError { /// Parse error message pub message: String, @@ -16,7 +16,7 @@ pub struct ParseError { /// Serious errors and errors from third-party libraries #[derive(Debug, Error)] -pub enum Error { +pub enum ConfigError { #[error("config error: {0}")] Config(String), diff --git a/crates/gosub_config/src/lib.rs b/crates/gosub_config/src/lib.rs index 40e5f9472..66f45dded 100644 --- a/crates/gosub_config/src/lib.rs +++ b/crates/gosub_config/src/lib.rs @@ -1,24 +1,28 @@ -mod errors; -pub mod settings; -pub mod storage; +use std::collections::HashMap; +use std::mem; +use std::str::FromStr; +use std::sync::RwLock; -use crate::settings::{Setting, SettingInfo}; -use crate::storage::MemoryStorageAdapter; -use gosub_shared::types::Result; +use anyhow::anyhow; use lazy_static::lazy_static; use log::warn; use serde_derive::Deserialize; use serde_json::Value; -use std::collections::HashMap; -use std::mem; -use std::str::FromStr; -use std::sync::RwLock; use wildmatch::WildMatch; +use gosub_shared::types::Result; + +use crate::settings::{Setting, SettingInfo}; +use crate::storage::MemoryStorageAdapter; + +mod errors; +pub mod settings; +pub mod storage; + /// Settings are stored in a json file, but this is included in the binary for mostly easy editting. const SETTINGS_JSON: &str = include_str!("./settings.json"); -/// StoreAdapter is the interface for storing and retrieving settings +/// `StoreAdapter` is the interface for storing and retrieving settings /// This can be used to storage settings in a database, json file, etc /// Note that we need to implement Send so we can send the storage adapter /// to other threads. @@ -32,7 +36,7 @@ pub trait StorageAdapter: Send + Sync { fn set(&self, key: &str, value: Setting); /// Retrieves all the settings in the storage in one go. This is used for preloading the settings - /// into the ConfigStore and is more performant normally than calling get_setting manually for each + /// into the `ConfigStore` and is more performant normally than calling `get_setting` manually for each /// setting. fn all(&self) -> Result>; } @@ -44,22 +48,28 @@ lazy_static! { } /// Returns a reference to the config store, which is locked by a mutex. -/// Any callers of the config store can just do config::config_store().get("dns.local.enabled") +/// Any callers of the config store can just do `config::config_store().get("dns.local.enabled`") +/// # Panics +/// if the config store is currently write locked +#[allow(clippy::expect_used)] pub fn config_store() -> std::sync::RwLockReadGuard<'static, ConfigStore> { - CONFIG_STORE.read().unwrap() + CONFIG_STORE.read().expect("Failed to read config store") } +/// # Panics +/// if the config store is currently write locked +#[allow(clippy::expect_used)] pub fn config_store_write() -> std::sync::RwLockWriteGuard<'static, ConfigStore> { - CONFIG_STORE.write().unwrap() + CONFIG_STORE.write().expect("Failed to write config store") } /// These macro's can be used to simplify the calls to the config store. You can simply do: /// -/// let enabled = config!(bool "dns.local.enabled").unwrap(); -/// config_set!(bool "dns.local.enabled", false); +/// let enabled = config!(bool "`dns.local.enabled").unwrap()`; +/// `config_set!(bool` "dns.local.enabled", false); /// /// Note that when you cannot find the key, it will return a default value. This is not always -/// what you want, but you can test for existence of the key with config_store().has("key") +/// what you want, but you can test for existence of the key with `config_store().has("key`") #[allow(clippy::crate_in_macro_def)] #[macro_export] macro_rules! config { @@ -115,7 +125,7 @@ macro_rules! config_set { }; } -/// JsonEntry is used for parsing the settings.json file +/// `JsonEntry` is used for parsing the settings.json file #[derive(Debug, Deserialize)] struct JsonEntry { key: String, @@ -130,7 +140,7 @@ pub struct ConfigStore { /// A hashmap of all settings so we can search o(1) time /// The mutex allows to share between multiple threads, /// The refcell allows us to use mutable references in a non-mutable way (ie: settings can be - /// stored while doing a immutable get()) + /// stored while doing a immutable `get()`) settings: std::sync::Mutex>>, /// A hashmap of all setting descriptions, default values and type information settings_info: HashMap, @@ -142,7 +152,7 @@ pub struct ConfigStore { impl Default for ConfigStore { fn default() -> Self { - let mut store = ConfigStore { + let mut store = Self { settings: std::sync::Mutex::new(std::cell::RefCell::new(HashMap::new())), settings_info: HashMap::new(), setting_keys: Vec::new(), @@ -166,18 +176,24 @@ impl ConfigStore { // Find all keys, and add them to the configuration store if let Ok(all_settings) = self.storage.all() { for (key, value) in all_settings { - self.settings + if self + .settings .lock() - .unwrap() - .borrow_mut() - .insert(key, value); + .map(|x| x.borrow_mut().insert(key.clone(), value)) + .is_err() + { + warn!("config: Failed to set setting {key}"); + } } } } /// Returns true when the storage knows about the given key pub fn has(&self, key: &str) -> bool { - self.settings.lock().unwrap().borrow().contains_key(key) + self.settings + .lock() + .map(|x| x.borrow().contains_key(key)) + .unwrap_or(false) } /// Returns a list of keys that matches the given search string (can use ? and *) for search @@ -203,10 +219,13 @@ impl ConfigStore { /// Returns the setting with the given key. If the setting is not found in the current /// storage, it will load the key from the storage. If the key is still not found, it will - /// return the default value for the given key. Note that if the key is not found and no - /// default value is specified, this function will panic. + /// return the default value for the given key. + /// + /// # Panics + /// if the key is not found and no default value is specified + #[allow(clippy::panic)] pub fn get(&self, key: &str) -> Option { - if let Some(setting) = self.settings.lock().unwrap().borrow().get(key) { + if let Some(setting) = self.settings.lock().ok()?.borrow().get(key) { return Some(setting.clone()); } @@ -214,10 +233,10 @@ impl ConfigStore { if let Some(setting) = self.storage.get(key) { self.settings .lock() - .unwrap() + .ok()? .borrow_mut() .insert(key.to_string(), setting.clone()); - return Some(setting.clone()); + return Some(setting); } // Return the default value for the setting when nothing is found @@ -227,19 +246,16 @@ impl ConfigStore { // At this point we haven't found the key in the store, we haven't found it in storage, and we // don't have a default value. This is a programming error, so we panic. - panic!("config: Setting {} is not known", key); + panic!("config: Setting {key} is not known"); } /// Sets the given setting to the given value. Will persist the setting to the /// storage. Note that the setting MUST have a settings-info entry, otherwise /// this function will not store the setting. pub fn set(&self, key: &str, value: Setting) { - let info = match self.settings_info.get(key) { - Some(info) => info, - None => { - warn!("config: Setting {key} is not known"); - return; - } + let Some(info) = self.settings_info.get(key) else { + warn!("config: Setting {key} is not known"); + return; }; if mem::discriminant(&info.default) != mem::discriminant(&value) { @@ -247,25 +263,29 @@ impl ConfigStore { return; } - self.settings + if self + .settings .lock() - .unwrap() - .borrow_mut() - .insert(key.to_owned(), value.clone()); + .map(|x| x.borrow_mut().insert(key.to_owned(), value.clone())) + .is_err() + { + warn!("config: Failed to set setting {key}"); + return; + } self.storage.set(key, value); } /// Populates the settings in the storage from the settings.json file fn populate_default_settings(&mut self) -> Result<()> { - let json_data: Value = - serde_json::from_str(SETTINGS_JSON).expect("Failed to parse settings.json"); + let json_data: Value = serde_json::from_str(SETTINGS_JSON) + .map_err(|_| anyhow!("Failed to parse settings.json"))?; if let Value::Object(data) = json_data { for (section_prefix, section_entries) in &data { let section_entries: Vec = serde_json::from_value(section_entries.clone()) - .expect("Failed to parse settings.json"); + .map_err(|_| anyhow!("Failed to parse settings.json"))?; for entry in section_entries { let key = format!("{}.{}", section_prefix, entry.key); @@ -281,7 +301,7 @@ impl ConfigStore { self.settings_info.insert(key.clone(), info.clone()); self.settings .lock() - .unwrap() + .map_err(|_| anyhow!("Failed to lock settings"))? .borrow_mut() .insert(key.clone(), info.default.clone()); } @@ -294,9 +314,10 @@ impl ConfigStore { #[cfg(test)] mod test { - use super::*; use storage::MemoryStorageAdapter; + use super::*; + #[test] fn test_config_store() { config_store_write().set_storage(Box::new(MemoryStorageAdapter::new())); @@ -338,7 +359,7 @@ mod test { } #[test] - #[should_panic] + #[should_panic(expected = "Setting this.key.doesnt.exist is not known")] fn macro_usage_with_panic() { config_set!(string "this.key.doesnt.exist", "yesitdoes".into()); let s = config!(string "this.key.doesnt.exist"); diff --git a/crates/gosub_config/src/settings.rs b/crates/gosub_config/src/settings.rs index 56be2af6e..26656e792 100644 --- a/crates/gosub_config/src/settings.rs +++ b/crates/gosub_config/src/settings.rs @@ -1,4 +1,4 @@ -use crate::errors::Error; +use crate::errors::ConfigError; use core::fmt::Display; use log::warn; use serde::{Deserialize, Deserializer, Serialize, Serializer}; @@ -6,7 +6,7 @@ use std::str::FromStr; /// A setting can be either a signed integer, unsigned integer, string, map or boolean. /// Maps could be created by using comma separated strings maybe -#[derive(Clone, PartialEq, Debug)] +#[derive(Clone, PartialEq, Eq, Debug)] pub enum Setting { SInt(isize), UInt(usize), @@ -16,60 +16,64 @@ pub enum Setting { } impl Setting { + #[must_use] pub fn to_bool(&self) -> bool { - if !matches!(self, Setting::Bool(_)) { + if !matches!(self, Self::Bool(_)) { warn!("setting is not a boolean"); } match self { - Setting::Bool(value) => *value, - Setting::SInt(value) => *value != 0, - Setting::UInt(value) => *value != 0, - Setting::String(value) => is_bool_value(value), - Setting::Map(values) => !values.is_empty(), + Self::Bool(value) => *value, + Self::SInt(value) => *value != 0, + Self::UInt(value) => *value != 0, + Self::String(value) => is_bool_value(value), + Self::Map(values) => !values.is_empty(), } } + #[must_use] pub fn to_sint(&self) -> isize { - if !matches!(self, Setting::SInt(_)) { + if !matches!(self, Self::SInt(_)) { warn!("setting is not an signed integer"); } match self { - Setting::SInt(value) => *value, - Setting::UInt(value) => *value as isize, - Setting::Bool(value) => *value as isize, - Setting::String(value) => is_bool_value(value) as isize, - Setting::Map(values) => values.len() as isize, + Self::SInt(value) => *value, + Self::UInt(value) => *value as isize, + Self::Bool(value) => isize::from(*value), + Self::String(value) => isize::from(is_bool_value(value)), + Self::Map(values) => values.len() as isize, } } + #[must_use] pub fn to_uint(&self) -> usize { - if !matches!(self, Setting::UInt(_)) { + if !matches!(self, Self::UInt(_)) { warn!("setting is not an unsigned integer"); } match self { - Setting::UInt(value) => *value, - Setting::SInt(value) => *value as usize, - Setting::Bool(value) => *value as usize, - Setting::String(value) => is_bool_value(value) as usize, - Setting::Map(values) => values.len(), + Self::UInt(value) => *value, + Self::SInt(value) => *value as usize, + Self::Bool(value) => usize::from(*value), + Self::String(value) => usize::from(is_bool_value(value)), + Self::Map(values) => values.len(), } } #[allow(clippy::inherent_to_string_shadow_display)] + #[must_use] pub fn to_string(&self) -> String { - if !matches!(self, Setting::String(_)) { + if !matches!(self, Self::String(_)) { warn!("setting is not a string"); } match self { - Setting::SInt(value) => value.to_string(), - Setting::UInt(value) => value.to_string(), - Setting::String(value) => value.clone(), - Setting::Bool(value) => value.to_string(), - Setting::Map(values) => { + Self::SInt(value) => value.to_string(), + Self::UInt(value) => value.to_string(), + Self::String(value) => value.clone(), + Self::Bool(value) => value.to_string(), + Self::Map(values) => { let mut result = String::new(); for value in values { result.push_str(value); @@ -81,13 +85,14 @@ impl Setting { } } + #[must_use] pub fn to_map(&self) -> Vec { - if !matches!(self, Setting::Map(_)) { + if !matches!(self, Self::Map(_)) { warn!("setting is not a map"); } match self { - Setting::Map(values) => values.clone(), + Self::Map(values) => values.clone(), other => vec![other.to_string()], } } @@ -118,7 +123,7 @@ impl<'de> Deserialize<'de> for Setting { D: Deserializer<'de>, { let value = String::deserialize(deserializer)?; - Setting::from_str(&value) + Self::from_str(&value) .map_err(|err| serde::de::Error::custom(format!("cannot deserialize: {err}"))) } } @@ -126,11 +131,11 @@ impl<'de> Deserialize<'de> for Setting { impl Display for Setting { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { - Setting::SInt(value) => write!(f, "i:{value}"), - Setting::UInt(value) => write!(f, "u:{value}"), - Setting::String(value) => write!(f, "s:{value}"), - Setting::Bool(value) => write!(f, "b:{value}"), - Setting::Map(values) => { + Self::SInt(value) => write!(f, "i:{value}"), + Self::UInt(value) => write!(f, "u:{value}"), + Self::String(value) => write!(f, "s:{value}"), + Self::Bool(value) => write!(f, "b:{value}"), + Self::Map(values) => { let mut result = String::new(); for value in values { result.push_str(value); @@ -144,7 +149,7 @@ impl Display for Setting { } impl FromStr for Setting { - type Err = Error; + type Err = ConfigError; // first element is the type: // b:true @@ -154,43 +159,40 @@ impl FromStr for Setting { // m:foo,bar,baz /// Converts a string to a setting or None when the string is invalid - fn from_str(key: &str) -> Result { - let (key_type, key_value) = key.split_once(':').expect(""); - - let setting = match key_type { - "b" => Setting::Bool( - key_value - .parse::() - .map_err(|err| Error::Config(format!("error parsing {key_value}: {err}")))?, - ), - "i" => Setting::SInt( - key_value - .parse::() - .map_err(|err| Error::Config(format!("error parsing {key_value}: {err}")))?, - ), - "u" => Setting::UInt( - key_value - .parse::() - .map_err(|err| Error::Config(format!("error parsing {key_value}: {err}")))?, - ), - "s" => Setting::String(key_value.to_string()), - - "m" => { - let mut values = Vec::new(); - for value in key_value.split(',') { - values.push(value.to_string()); + fn from_str(key: &str) -> Result { + let (key_type, key_value) = key + .split_once(':') + .ok_or(ConfigError::Config("invalid setting".to_owned()))?; + + let setting = + match key_type { + "b" => Self::Bool(key_value.parse::().map_err(|err| { + ConfigError::Config(format!("error parsing {key_value}: {err}")) + })?), + "i" => Self::SInt(key_value.parse::().map_err(|err| { + ConfigError::Config(format!("error parsing {key_value}: {err}")) + })?), + "u" => Self::UInt(key_value.parse::().map_err(|err| { + ConfigError::Config(format!("error parsing {key_value}: {err}")) + })?), + "s" => Self::String(key_value.to_string()), + + "m" => { + let mut values = Vec::new(); + for value in key_value.split(',') { + values.push(value.to_string()); + } + Self::Map(values) } - Setting::Map(values) - } - _ => return Err(Error::Config(format!("unknown setting: {key_value}"))), - }; + _ => return Err(ConfigError::Config(format!("unknown setting: {key_value}"))), + }; Ok(setting) } } -/// SettingInfo returns information about a given setting -#[derive(Clone, PartialEq, Debug)] +/// `SettingInfo` returns information about a given setting +#[derive(Clone, PartialEq, Eq, Debug)] pub struct SettingInfo { /// Name of the key (dot notation, (ie: dns.resolver.enabled pub key: String, @@ -214,15 +216,15 @@ mod test { assert_eq!(1, s.to_sint()); assert_eq!(1, s.to_uint()); assert_eq!("true", s.to_string()); - assert_eq!(vec!("true"), s.to_map()); + assert_eq!(vec!["true"], s.to_map()); let s = Setting::from_str("i:-1").unwrap(); assert_eq!(s, Setting::SInt(-1)); assert!(s.to_bool()); assert_eq!(-1, s.to_sint()); - assert_eq!(18446744073709551615, s.to_uint()); + assert_eq!(18_446_744_073_709_551_615, s.to_uint()); assert_eq!("-1", s.to_string()); - assert_eq!(vec!("-1"), s.to_map()); + assert_eq!(vec!["-1"], s.to_map()); let s = Setting::from_str("i:1").unwrap(); assert_eq!(s, Setting::SInt(1)); @@ -230,7 +232,7 @@ mod test { assert_eq!(1, s.to_sint()); assert_eq!(1, s.to_uint()); assert_eq!("1", s.to_string()); - assert_eq!(vec!("1"), s.to_map()); + assert_eq!(vec!["1"], s.to_map()); let s = Setting::from_str("s:hello world").unwrap(); assert_eq!(s, Setting::String("hello world".into())); @@ -238,7 +240,7 @@ mod test { assert_eq!(0, s.to_sint()); assert_eq!(0, s.to_uint()); assert_eq!("hello world", s.to_string()); - assert_eq!(vec!("hello world"), s.to_map()); + assert_eq!(vec!["hello world"], s.to_map()); let s = Setting::from_str("m:foo,bar,baz").unwrap(); assert_eq!( @@ -252,16 +254,16 @@ mod test { assert_eq!(vec!["foo", "bar", "baz"], s.to_map()); let s = Setting::from_str("notexist:true"); - assert!(matches!(s, Err(Error::Config(_)))); + assert!(matches!(s, Err(ConfigError::Config(_)))); let s = Setting::from_str("b:foobar"); - assert!(matches!(s, Err(Error::Config(_)))); + assert!(matches!(s, Err(ConfigError::Config(_)))); let s = Setting::from_str("i:foobar"); - assert!(matches!(s, Err(Error::Config(_)))); + assert!(matches!(s, Err(ConfigError::Config(_)))); let s = Setting::from_str("u:-1"); - assert!(matches!(s, Err(Error::Config(_)))); + assert!(matches!(s, Err(ConfigError::Config(_)))); let s = Setting::from_str("s:true").unwrap(); assert!(s.to_bool()); diff --git a/crates/gosub_config/src/storage/json.rs b/crates/gosub_config/src/storage/json.rs index be43b5e62..286489da0 100644 --- a/crates/gosub_config/src/storage/json.rs +++ b/crates/gosub_config/src/storage/json.rs @@ -21,21 +21,17 @@ impl TryFrom<&String> for JsonStorageAdapter { let _ = if let Ok(metadata) = fs::metadata(path) { assert!(metadata.is_file(), "json file is not a regular file"); - File::options() - .read(true) - .write(true) - .open(path) - .expect("failed to open json file") + File::options().read(true).write(true).open(path)? } else { let json = "{}"; - let mut file = File::create(path).expect("cannot create json file"); + let mut file = File::create(path)?; file.write_all(json.as_bytes())?; file }; - let mut adapter = JsonStorageAdapter { + let mut adapter = Self { path: path.to_string(), elements: Mutex::new(HashMap::new()), }; @@ -97,17 +93,17 @@ impl JsonStorageAdapter { /// Write the self.elements hashmap back to the file by truncating the file and writing the /// data again. #[allow(dead_code)] - fn write_file(&mut self) { + fn write_file(&mut self) -> std::io::Result<()> { // @TODO: We need some kind of OS lock file here. We should protect against concurrent threads but also // against concurrent processes. - let mut file = File::open(&self.path).expect("failed to open json file"); + let mut file = File::open(&self.path)?; + + let json = serde_json::to_string_pretty(&self.elements)?; - let json = serde_json::to_string_pretty(&self.elements).expect("failed to serialize"); + file.set_len(0)?; + file.seek(std::io::SeekFrom::Start(0))?; + file.write_all(json.as_bytes())?; - file.set_len(0).expect("failed to truncate file"); - file.seek(std::io::SeekFrom::Start(0)) - .expect("failed to seek"); - file.write_all(json.as_bytes()) - .expect("failed to write file"); + Ok(()) } } diff --git a/crates/gosub_config/src/storage/sqlite.rs b/crates/gosub_config/src/storage/sqlite.rs index 8790f6283..264c06ce7 100644 --- a/crates/gosub_config/src/storage/sqlite.rs +++ b/crates/gosub_config/src/storage/sqlite.rs @@ -14,7 +14,7 @@ impl TryFrom<&String> for SqliteStorageAdapter { type Error = anyhow::Error; fn try_from(path: &String) -> Result { - let conn = sqlite::open(path).expect("cannot open db file"); + let conn = sqlite::open(path)?; let query = "CREATE TABLE IF NOT EXISTS settings ( id INTEGER PRIMARY KEY, @@ -23,7 +23,7 @@ impl TryFrom<&String> for SqliteStorageAdapter { )"; conn.execute(query)?; - Ok(SqliteStorageAdapter { + Ok(Self { connection: Mutex::new(conn), }) } @@ -66,7 +66,7 @@ impl StorageAdapter for SqliteStorageAdapter { let mut statement = db_lock.prepare(query).unwrap(); let mut settings = HashMap::new(); - while let sqlite::State::Row = statement.next().unwrap() { + while statement.next().unwrap() == sqlite::State::Row { let key = statement.read::(1).unwrap(); let value = statement.read::(2).unwrap(); settings.insert(key, Setting::from_str(&value)?); diff --git a/crates/gosub_css3/Cargo.toml b/crates/gosub_css3/Cargo.toml index 3740e33db..c0c94f5c6 100644 --- a/crates/gosub_css3/Cargo.toml +++ b/crates/gosub_css3/Cargo.toml @@ -11,4 +11,7 @@ lazy_static = "1.5" log = "0.4.22" simple_logger = "5.0.0" anyhow = { version = "1.0.86", features = [] } -colors-transform = "0.2.11" \ No newline at end of file +colors-transform = "0.2.11" + +[lints] +workspace = true diff --git a/crates/gosub_css3/src/lib.rs b/crates/gosub_css3/src/lib.rs index 8c0a966f6..557654d88 100644 --- a/crates/gosub_css3/src/lib.rs +++ b/crates/gosub_css3/src/lib.rs @@ -17,7 +17,6 @@ pub mod walker; /// This CSS3 parser is heavily based on the MIT licensed CssTree parser written by /// Roman Dvornov (https://github.com/lahmatiy). /// The original version can be found at https://github.com/csstree/csstree - pub struct Css3<'stream> { /// The tokenizer is responsible for reading the input stream and pub tokenizer: Tokenizer<'stream>, @@ -36,8 +35,8 @@ pub struct Error { } impl Error { - pub(crate) fn new(message: String, location: Location) -> Error { - Error { message, location } + pub(crate) const fn new(message: String, location: Location) -> Self { + Self { message, location } } } @@ -72,7 +71,7 @@ impl<'stream> Css3<'stream> { Self { tokenizer: Tokenizer::new(it, Location::default()), allow_values_in_argument_list: Vec::new(), - config: Default::default(), + config: ParserConfig::default(), } } diff --git a/crates/gosub_css3/src/parser.rs b/crates/gosub_css3/src/parser.rs index 6d2649f18..0295fd2f3 100644 --- a/crates/gosub_css3/src/parser.rs +++ b/crates/gosub_css3/src/parser.rs @@ -21,9 +21,9 @@ mod value; impl Css3<'_> { /// Consumes a specific token - pub fn consume(&mut self, token_type: TokenType) -> Result { + pub fn consume(&mut self, token_type: &TokenType) -> Result { let t = self.tokenizer.consume(); - if t.token_type != token_type { + if t.token_type != *token_type { return Err(Error::new( format!("Expected {:?}, got {:?}", token_type, t), self.tokenizer.current_location(), @@ -157,7 +157,7 @@ impl Css3<'_> { while !self.tokenizer.eof() { let t = self.tokenizer.consume(); - if let TokenType::LCurly = t.token_type { + if t.token_type == TokenType::LCurly { self.tokenizer.reconsume(); break; } diff --git a/crates/gosub_css3/src/parser/at_rule.rs b/crates/gosub_css3/src/parser/at_rule.rs index 6fb154577..e6fd03b39 100644 --- a/crates/gosub_css3/src/parser/at_rule.rs +++ b/crates/gosub_css3/src/parser/at_rule.rs @@ -127,7 +127,7 @@ impl Css3<'_> { // if we did a block, we need to close it if node.is_some() { - self.consume(TokenType::RCurly)?; + self.consume(&TokenType::RCurly)?; } Ok(node) diff --git a/crates/gosub_css3/src/parser/at_rule/media.rs b/crates/gosub_css3/src/parser/at_rule/media.rs index 50e05f2cc..ab2b4f95d 100644 --- a/crates/gosub_css3/src/parser/at_rule/media.rs +++ b/crates/gosub_css3/src/parser/at_rule/media.rs @@ -18,7 +18,7 @@ impl Css3<'_> { TokenType::Function(name) => { let name = name.to_lowercase(); let args = self.parse_pseudo_function(name.as_str())?; - self.consume(TokenType::RParen)?; + self.consume(&TokenType::RParen)?; Ok(Node::new( NodeType::Function { @@ -91,7 +91,7 @@ impl Css3<'_> { let loc = self.tokenizer.current_location(); - self.consume(TokenType::LParen)?; + self.consume(&TokenType::LParen)?; let mut value: Option = None; @@ -119,7 +119,7 @@ impl Css3<'_> { TokenType::Function(name) => { let name = name.to_lowercase(); let args = self.parse_pseudo_function(name.as_str())?; - self.consume(TokenType::RParen)?; + self.consume(&TokenType::RParen)?; Some(Node::new( NodeType::Function { @@ -140,7 +140,7 @@ impl Css3<'_> { self.consume_whitespace_comments(); if !self.tokenizer.eof() { - self.consume(TokenType::RParen)?; + self.consume(&TokenType::RParen)?; } } @@ -153,7 +153,7 @@ impl Css3<'_> { let loc = self.tokenizer.current_location(); self.consume_whitespace_comments(); - self.consume(TokenType::LParen)?; + self.consume(&TokenType::LParen)?; let left = self.parse_media_read_term()?; let left_comparison = self.parse_media_read_comparison()?; diff --git a/crates/gosub_css3/src/parser/at_rule/scope.rs b/crates/gosub_css3/src/parser/at_rule/scope.rs index af0f3a1e0..003e89b89 100644 --- a/crates/gosub_css3/src/parser/at_rule/scope.rs +++ b/crates/gosub_css3/src/parser/at_rule/scope.rs @@ -17,19 +17,19 @@ impl Css3<'_> { root = Some(self.parse_selector_list()?); self.consume_whitespace_comments(); - self.consume(TokenType::RParen)?; + self.consume(&TokenType::RParen)?; } if let TokenType::Ident(_value) = t.token_type { self.consume_whitespace_comments(); self.consume_ident("to")?; self.consume_whitespace_comments(); - self.consume(TokenType::RParen)?; + self.consume(&TokenType::RParen)?; self.consume_whitespace_comments(); limit = Some(self.parse_selector_list()?); self.consume_whitespace_comments(); - self.consume(TokenType::RParen)?; + self.consume(&TokenType::RParen)?; } Ok(Node::new(NodeType::Scope { root, limit }, t.location)) diff --git a/crates/gosub_css3/src/parser/at_rule/supports.rs b/crates/gosub_css3/src/parser/at_rule/supports.rs index dfae55621..28ab8f463 100644 --- a/crates/gosub_css3/src/parser/at_rule/supports.rs +++ b/crates/gosub_css3/src/parser/at_rule/supports.rs @@ -29,6 +29,6 @@ mod tests { let node = parser.parse_at_rule_supports_prelude().unwrap(); let w = Walker::new(&node); - assert_eq!(w.walk_to_string(), "[Raw] (display: flex)\n") + assert_eq!(w.walk_to_string(), "[Raw] (display: flex)\n"); } } diff --git a/crates/gosub_css3/src/parser/calc.rs b/crates/gosub_css3/src/parser/calc.rs index 74f3643d7..fad5fe986 100644 --- a/crates/gosub_css3/src/parser/calc.rs +++ b/crates/gosub_css3/src/parser/calc.rs @@ -48,7 +48,7 @@ impl Css3<'_> { #[cfg(test)] mod tests { #[test] - fn test_parse_calc() { + const fn test_parse_calc() { // test!(parse_calc, "calc(1px + 2px)", Box::new(NodeType::AnPlusB { a: "1".to_string(), b: "2".to_string() } )); // test!(parse_calc, "calc(100px + (200px - 100px) * ((100vh - 500px) / (800 - 500)))", Box::new(NodeType::AnPlusB { a: "1".to_string(), b: "2".to_string() } )); // test!(parse_calc, "calc(12px + (20 - 12) * ((100vw - 300px) / (700 - 300))", Box::new(NodeType::AnPlusB { a: "1".to_string(), b: "2".to_string() } )); diff --git a/crates/gosub_css3/src/parser/condition.rs b/crates/gosub_css3/src/parser/condition.rs index e53234e5a..f7f54da95 100644 --- a/crates/gosub_css3/src/parser/condition.rs +++ b/crates/gosub_css3/src/parser/condition.rs @@ -35,9 +35,9 @@ impl Css3<'_> { }; if term.is_err() { - self.consume(TokenType::RParen)?; + self.consume(&TokenType::RParen)?; let res = self.parse_condition(kind.clone())?; - self.consume(TokenType::LParen)?; + self.consume(&TokenType::LParen)?; return Ok(res); } diff --git a/crates/gosub_css3/src/parser/declaration.rs b/crates/gosub_css3/src/parser/declaration.rs index 4569ae8e4..cf0e0eac4 100644 --- a/crates/gosub_css3/src/parser/declaration.rs +++ b/crates/gosub_css3/src/parser/declaration.rs @@ -7,11 +7,7 @@ impl Css3<'_> { log::trace!("parse_property_name"); let t = self.consume_any()?; match t.token_type { - TokenType::Delim('*') - | TokenType::Delim('$') - | TokenType::Delim('+') - | TokenType::Delim('#') - | TokenType::Delim('&') => {} //next + TokenType::Delim('*' | '$' | '+' | '#' | '&') => {} //next TokenType::Delim('/') => { let t = self.tokenizer.lookahead(1); if t.token_type == TokenType::Delim('/') { @@ -60,7 +56,7 @@ impl Css3<'_> { let custom_property = property.starts_with("--"); self.consume_whitespace_comments(); - self.consume(TokenType::Colon)?; + self.consume(&TokenType::Colon)?; if !custom_property { self.consume_whitespace_comments(); } diff --git a/crates/gosub_css3/src/parser/function.rs b/crates/gosub_css3/src/parser/function.rs index 56ae01dfe..663eef8a1 100644 --- a/crates/gosub_css3/src/parser/function.rs +++ b/crates/gosub_css3/src/parser/function.rs @@ -24,7 +24,7 @@ impl Css3<'_> { }; if !self.tokenizer.eof() { - self.consume(TokenType::RParen)?; + self.consume(&TokenType::RParen)?; } Ok(Node::new(NodeType::Function { name, arguments }, loc)) diff --git a/crates/gosub_css3/src/parser/rule.rs b/crates/gosub_css3/src/parser/rule.rs index c76fa1049..c2344931c 100644 --- a/crates/gosub_css3/src/parser/rule.rs +++ b/crates/gosub_css3/src/parser/rule.rs @@ -29,12 +29,12 @@ impl Css3<'_> { let prelude = self.parse_selector_list()?; - self.consume(TokenType::LCurly)?; + self.consume(&TokenType::LCurly)?; self.consume_whitespace_comments(); let block = self.parse_block(BlockParseMode::StyleBlock)?; - self.consume(TokenType::RCurly)?; + self.consume(&TokenType::RCurly)?; Ok(Node::new( NodeType::Rule { diff --git a/crates/gosub_css3/src/parser/selector.rs b/crates/gosub_css3/src/parser/selector.rs index 3a446a308..21f3ba708 100644 --- a/crates/gosub_css3/src/parser/selector.rs +++ b/crates/gosub_css3/src/parser/selector.rs @@ -37,7 +37,7 @@ impl Css3<'_> { let loc = self.tokenizer.current_location(); - self.consume(TokenType::Delim('.'))?; + self.consume(&TokenType::Delim('.'))?; let value = self.consume_any_ident()?; @@ -49,7 +49,7 @@ impl Css3<'_> { let loc = self.tokenizer.current_location(); - self.consume(TokenType::Delim('&'))?; + self.consume(&TokenType::Delim('&'))?; Ok(Node::new(NodeType::NestingSelector, loc)) } @@ -117,7 +117,7 @@ impl Css3<'_> { let mut matcher = None; let mut value = String::new(); - self.consume(TokenType::LBracket)?; + self.consume(&TokenType::LBracket)?; self.consume_whitespace_comments(); let name = self.consume_any_ident()?; @@ -153,7 +153,7 @@ impl Css3<'_> { } } - self.consume(TokenType::RBracket)?; + self.consume(&TokenType::RBracket)?; self.consume_whitespace_comments(); Ok(Node::new( @@ -172,7 +172,7 @@ impl Css3<'_> { let loc = self.tokenizer.current_location(); - self.consume(TokenType::Delim('#'))?; + self.consume(&TokenType::Delim('#'))?; let t = self.consume_any()?; let value = match t.token_type { @@ -193,8 +193,8 @@ impl Css3<'_> { let loc = self.tokenizer.current_location(); - self.consume(TokenType::Colon)?; - self.consume(TokenType::Colon)?; + self.consume(&TokenType::Colon)?; + self.consume(&TokenType::Colon)?; let t = self.tokenizer.lookahead(0); let value = if t.is_ident() { @@ -214,7 +214,7 @@ impl Css3<'_> { let loc = self.tokenizer.current_location(); - self.consume(TokenType::Colon)?; + self.consume(&TokenType::Colon)?; let t = self.tokenizer.consume(); let value = match t.token_type { @@ -222,7 +222,7 @@ impl Css3<'_> { TokenType::Function(name) => { let name = name.to_lowercase(); let args = self.parse_pseudo_function(name.as_str())?; - self.consume(TokenType::RParen)?; + self.consume(&TokenType::RParen)?; Node::new( NodeType::Function { @@ -297,11 +297,8 @@ impl Css3<'_> { Node::new(NodeType::Dimension { value, unit }, t.location) } - TokenType::Delim('+') - | TokenType::Delim('>') - | TokenType::Delim('~') - | TokenType::Delim('/') => { - // Dont add descendant combinator since we are now adding another one + TokenType::Delim('+' | '>' | '~' | '/') => { + // Don't add descendant combinator since we are now adding another one space = false; self.tokenizer.reconsume(); @@ -312,7 +309,7 @@ impl Css3<'_> { self.tokenizer.reconsume(); self.parse_class_selector()? } - TokenType::Delim('|') | TokenType::Delim('*') => { + TokenType::Delim('|' | '*') => { self.tokenizer.reconsume(); self.parse_type_selector()? } diff --git a/crates/gosub_css3/src/parser/url.rs b/crates/gosub_css3/src/parser/url.rs index 979ea42de..3fa5f1853 100644 --- a/crates/gosub_css3/src/parser/url.rs +++ b/crates/gosub_css3/src/parser/url.rs @@ -27,7 +27,7 @@ impl Css3<'_> { } }; - self.consume(TokenType::RParen)?; + self.consume(&TokenType::RParen)?; Ok(Node::new(NodeType::Url { url }, loc)) } diff --git a/crates/gosub_css3/src/parser/value.rs b/crates/gosub_css3/src/parser/value.rs index af72cac3a..5404cb007 100644 --- a/crates/gosub_css3/src/parser/value.rs +++ b/crates/gosub_css3/src/parser/value.rs @@ -100,7 +100,7 @@ impl Css3<'_> { } TokenType::Ident(value) => { if value.eq_ignore_ascii_case("progid") { - let _ = self.consume(TokenType::Colon)?; + let _ = self.consume(&TokenType::Colon)?; let _ = self.consume_ident_ci("dximagetransform")?; let _ = self.consume_delim('.')?; let _ = self.consume_ident_ci("microsoft")?; diff --git a/crates/gosub_css3/src/stylesheet.rs b/crates/gosub_css3/src/stylesheet.rs index 528eccfe4..e5507b8a4 100644 --- a/crates/gosub_css3/src/stylesheet.rs +++ b/crates/gosub_css3/src/stylesheet.rs @@ -7,6 +7,7 @@ use std::fmt::Display; /// Defines a complete stylesheet with all its rules and the location where it was found #[derive(Debug, PartialEq, Clone)] +#[allow(clippy::module_name_repetitions)] pub struct CssStylesheet { /// List of rules found in this stylesheet pub rules: Vec, @@ -17,7 +18,7 @@ pub struct CssStylesheet { } /// Defines the origin of the stylesheet (or declaration) -#[derive(Debug, PartialEq, Clone)] +#[derive(Debug, PartialEq, Eq, Clone)] pub enum CssOrigin { /// Browser/user agent defined stylesheets UserAgent, @@ -37,11 +38,13 @@ pub struct CssRule { } impl CssRule { - pub fn selectors(&self) -> &Vec { + #[must_use] + pub const fn selectors(&self) -> &Vec { &self.selectors } - pub fn declarations(&self) -> &Vec { + #[must_use] + pub const fn declarations(&self) -> &Vec { &self.declarations } } @@ -58,7 +61,7 @@ pub struct CssDeclaration { pub important: bool, } -#[derive(Debug, PartialEq, Clone)] +#[derive(Debug, PartialEq, Eq, Clone)] pub struct CssSelector { // List of parts that make up this selector pub parts: Vec, @@ -66,6 +69,7 @@ pub struct CssSelector { impl CssSelector { /// Generate specificity for this selector + #[must_use] pub fn specificity(&self) -> Specificity { let mut id_count = 0; let mut class_count = 0; @@ -90,7 +94,7 @@ impl CssSelector { /// @todo: it would be nicer to have a struct for each type of selector part, but for now we'll keep it simple /// Represents a CSS selector part, which has a type and value (e.g. type=Class, class="my-class") -#[derive(PartialEq, Clone, Default)] +#[derive(PartialEq, Eq, Clone, Default)] pub struct CssSelectorPart { pub type_: CssSelectorType, pub value: String, @@ -135,7 +139,7 @@ impl Debug for CssSelectorPart { } /// Represents a CSS selector type for this part -#[derive(Debug, PartialEq, Clone, Default)] +#[derive(Debug, PartialEq, Eq, Clone, Default)] pub enum CssSelectorType { Universal, // '*' #[default] @@ -149,7 +153,7 @@ pub enum CssSelectorType { } /// Represents which type of matcher is used (in case of an attribute selector type) -#[derive(Default, PartialEq, Clone)] +#[derive(Default, PartialEq, Eq, Clone)] pub enum MatcherType { #[default] None, // No matcher @@ -164,13 +168,13 @@ pub enum MatcherType { impl Display for MatcherType { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { - MatcherType::None => write!(f, ""), - MatcherType::Equals => write!(f, "="), - MatcherType::Includes => write!(f, "~="), - MatcherType::DashMatch => write!(f, "|="), - MatcherType::PrefixMatch => write!(f, "^="), - MatcherType::SuffixMatch => write!(f, "$="), - MatcherType::SubstringMatch => write!(f, "*="), + Self::None => write!(f, ""), + Self::Equals => write!(f, "="), + Self::Includes => write!(f, "~="), + Self::DashMatch => write!(f, "|="), + Self::PrefixMatch => write!(f, "^="), + Self::SuffixMatch => write!(f, "$="), + Self::SubstringMatch => write!(f, "*="), } } } @@ -180,7 +184,8 @@ impl Display for MatcherType { pub struct Specificity(u32, u32, u32); impl Specificity { - pub fn new(a: u32, b: u32, c: u32) -> Self { + #[must_use] + pub const fn new(a: u32, b: u32, c: u32) -> Self { Self(a, b, c) } } diff --git a/crates/gosub_css3/src/tokenizer.rs b/crates/gosub_css3/src/tokenizer.rs index ca7b943e4..095e5a341 100644 --- a/crates/gosub_css3/src/tokenizer.rs +++ b/crates/gosub_css3/src/tokenizer.rs @@ -1,4 +1,4 @@ -use crate::unicode::{get_unicode_char, UnicodeChar}; +use crate::unicode::UnicodeChar; use gosub_shared::byte_stream::Character::Ch; use gosub_shared::byte_stream::{ByteStream, Character}; use gosub_shared::byte_stream::{Location, LocationHandler, Stream}; @@ -738,7 +738,7 @@ impl<'stream> Tokenizer<'stream> { let mut value = String::new(); - let default_char = get_unicode_char(&UnicodeChar::ReplacementCharacter); + let default_char = UnicodeChar::REPLACEMENT_CHARACTER; // eof: parser error if self.stream.eof() { return default_char; @@ -760,9 +760,7 @@ impl<'stream> Tokenizer<'stream> { // todo: look for better implementation if let Some(char) = char::from_u32(as_u32) { - if char == get_unicode_char(&UnicodeChar::Null) - || char >= get_unicode_char(&UnicodeChar::MaxAllowed) - { + if char == UnicodeChar::NULL || char >= UnicodeChar::MAX_ALLOWED { return default_char; } @@ -855,12 +853,11 @@ impl<'stream> Tokenizer<'stream> { /// def: [non-printable code point](https://www.w3.org/TR/css-syntax-3/#non-printable-code-point) fn is_non_printable_char(&self) -> bool { if let Ch(char) = self.current_char() { - (char >= get_unicode_char(&UnicodeChar::Null) - && char <= get_unicode_char(&UnicodeChar::Backspace)) - || (char >= get_unicode_char(&UnicodeChar::ShiftOut) - && char <= get_unicode_char(&UnicodeChar::InformationSeparatorOne)) - || char == get_unicode_char(&UnicodeChar::Tab) - || char == get_unicode_char(&UnicodeChar::Delete) + char >= UnicodeChar::NULL && char <= UnicodeChar::BACKSPACE + || (char >= UnicodeChar::SHIFT_OUT + && char <= UnicodeChar::INFORMATION_SEPARATOR_ONE + || char == UnicodeChar::TAB + || char == UnicodeChar::DELETE) } else { false } @@ -1038,20 +1035,11 @@ mod test { let mut chars = ByteStream::new(Encoding::UTF8, None); let escaped_chars = vec![ - ("\\005F ", get_unicode_char(&UnicodeChar::LowLine)), + ("\\005F ", UnicodeChar::LOW_LINE), ("\\2A", '*'), - ( - "\\000000 ", - get_unicode_char(&UnicodeChar::ReplacementCharacter), - ), - ( - "\\FFFFFF ", - get_unicode_char(&UnicodeChar::ReplacementCharacter), - ), - ( - "\\10FFFF ", - get_unicode_char(&UnicodeChar::ReplacementCharacter), - ), + ("\\000000 ", UnicodeChar::REPLACEMENT_CHARACTER), + ("\\FFFFFF ", UnicodeChar::REPLACEMENT_CHARACTER), + ("\\10FFFF ", UnicodeChar::REPLACEMENT_CHARACTER), ]; let mut tokenizer = Tokenizer::new(&mut chars, Location::default()); diff --git a/crates/gosub_css3/src/unicode.rs b/crates/gosub_css3/src/unicode.rs index 6fd71467c..7b3fb3ef1 100644 --- a/crates/gosub_css3/src/unicode.rs +++ b/crates/gosub_css3/src/unicode.rs @@ -1,36 +1,17 @@ // note: file should be in a shared lib -use lazy_static::lazy_static; -use std::collections::HashMap; +#[allow(clippy::module_name_repetitions)] +pub struct UnicodeChar; -// note: should be shared -#[derive(Debug, Eq, Hash, PartialEq)] -pub enum UnicodeChar { - Null, - Backspace, - Tab, - ShiftOut, - Delete, - InformationSeparatorOne, - LowLine, - MaxAllowed, - ReplacementCharacter, -} - -lazy_static! { - static ref UNICODE_CHARS: HashMap = HashMap::from([ - (UnicodeChar::Null, '\u{0000}'), - (UnicodeChar::Backspace, '\u{0008}'), - (UnicodeChar::Tab, '\u{000B}'), - (UnicodeChar::ShiftOut, '\u{000E}'), - (UnicodeChar::Delete, '\u{007F}'), - (UnicodeChar::InformationSeparatorOne, '\u{001F}'), - (UnicodeChar::LowLine, '\u{005F}'), - (UnicodeChar::MaxAllowed, '\u{10FFFF}'), - (UnicodeChar::ReplacementCharacter, '\u{FFFD}') - ]); -} - -pub fn get_unicode_char(char: &UnicodeChar) -> char { - *UNICODE_CHARS.get(char).expect("Unknown unicode char.") +impl UnicodeChar { + pub const NULL: char = '\u{0000}'; + pub const BACKSPACE: char = '\u{0008}'; + pub const TAB: char = '\u{000B}'; + pub const SHIFT_OUT: char = '\u{000E}'; + pub const DELETE: char = '\u{007F}'; + pub const INFORMATION_SEPARATOR_ONE: char = '\u{001F}'; + #[allow(dead_code)] //TODO: why is this here? + pub const LOW_LINE: char = '\u{005F}'; + pub const MAX_ALLOWED: char = '\u{10FFFF}'; + pub const REPLACEMENT_CHARACTER: char = '\u{FFFD}'; } diff --git a/crates/gosub_html5/Cargo.toml b/crates/gosub_html5/Cargo.toml index d398a05ce..3dfb94900 100644 --- a/crates/gosub_html5/Cargo.toml +++ b/crates/gosub_html5/Cargo.toml @@ -37,3 +37,6 @@ harness = false [[bench]] name = "tree_construction" harness = false + +[lints] +workspace = true diff --git a/crates/gosub_html5/benches/tokenizer.rs b/crates/gosub_html5/benches/tokenizer.rs index e20824a6a..ea934f06e 100644 --- a/crates/gosub_html5/benches/tokenizer.rs +++ b/crates/gosub_html5/benches/tokenizer.rs @@ -23,7 +23,7 @@ fn criterion_benchmark(c: &mut Criterion) { test.tokenize(); } } - }) + }); }); group.finish(); diff --git a/crates/gosub_html5/benches/tree_construction.rs b/crates/gosub_html5/benches/tree_construction.rs index 077cf0aae..e63bd7ff1 100644 --- a/crates/gosub_html5/benches/tree_construction.rs +++ b/crates/gosub_html5/benches/tree_construction.rs @@ -1,3 +1,5 @@ +#![allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)] + use criterion::{criterion_group, criterion_main, Criterion}; use gosub_testing::testing::tree_construction; use gosub_testing::testing::tree_construction::Harness; diff --git a/crates/gosub_html5/src/element_class.rs b/crates/gosub_html5/src/element_class.rs index 3951e58c4..15a42061f 100644 --- a/crates/gosub_html5/src/element_class.rs +++ b/crates/gosub_html5/src/element_class.rs @@ -1,10 +1,10 @@ use std::collections::HashMap; -#[derive(Debug, Clone, PartialEq)] +#[derive(Debug, Clone, PartialEq, Eq)] pub struct ElementClass { /// a map of classes applied to an HTML element. - /// key = name, value = is_active - /// the is_active is used to toggle a class (JavaScript API) + /// key = name, value = `is_active` + /// the `is_active` is used to toggle a class (JavaScript API) class_map: HashMap, } @@ -15,7 +15,7 @@ impl Default for ElementClass { } impl ElementClass { - /// Initialise a new (empty) ElementClass + /// Initialise a new (empty) `ElementClass` #[must_use] pub fn new() -> Self { Self { @@ -25,16 +25,19 @@ impl ElementClass { /// Count the number of classes (active or inactive) /// assigned to an element + #[must_use] pub fn len(&self) -> usize { self.class_map.len() } /// Check if any classes are present + #[must_use] pub fn is_empty(&self) -> bool { self.class_map.is_empty() } /// Check if class name exists + #[must_use] pub fn contains(&self, name: &str) -> bool { self.class_map.contains_key(name) } @@ -70,6 +73,7 @@ impl ElementClass { } /// Check if a class is active. Returns false if class doesn't exist + #[must_use] pub fn is_active(&self, name: &str) -> bool { if let Some(is_active) = self.class_map.get(name) { return *is_active; @@ -88,7 +92,7 @@ impl From<&str> for ElementClass { .map(|class| (class.to_owned(), true)) .collect::>(); - ElementClass { + Self { class_map: class_map_local, } } diff --git a/crates/gosub_html5/src/errors.rs b/crates/gosub_html5/src/errors.rs index 5fa941a27..46dcfa435 100644 --- a/crates/gosub_html5/src/errors.rs +++ b/crates/gosub_html5/src/errors.rs @@ -3,7 +3,7 @@ use gosub_shared::byte_stream::Location; use thiserror::Error; /// Parser error that defines an error (message) on the given position -#[derive(Clone, Debug, PartialEq)] +#[derive(Clone, Debug, PartialEq, Eq)] pub struct ParseError { /// Parse error message pub message: String, diff --git a/crates/gosub_html5/src/node.rs b/crates/gosub_html5/src/node.rs index 25b0d34c7..46fc0d311 100644 --- a/crates/gosub_html5/src/node.rs +++ b/crates/gosub_html5/src/node.rs @@ -22,7 +22,7 @@ pub mod arena; pub mod data; /// Different types of nodes -#[derive(Debug, PartialEq)] +#[derive(Debug, PartialEq, Eq)] pub enum NodeType { Document, DocType, @@ -51,14 +51,14 @@ pub enum NodeData { pub struct NodeId(pub(crate) usize); impl From for usize { - /// Converts a NodeId into a usize + /// Converts a `NodeId` into a usize fn from(value: NodeId) -> Self { value.0 } } impl From for NodeId { - /// Converts a usize into a NodeId + /// Converts a usize into a `NodeId` fn from(value: usize) -> Self { Self(value) } @@ -79,7 +79,7 @@ impl From for u64 { } impl Default for &NodeId { - /// Returns the default NodeId, which is 0 + /// Returns the default `NodeId`, which is 0 fn default() -> Self { &NodeId(0) } @@ -90,11 +90,13 @@ impl NodeId { pub const ROOT_NODE: usize = 0; /// Returns the root node ID + #[must_use] pub fn root() -> Self { Self(Self::ROOT_NODE) } /// Returns true when this nodeId is the root node + #[must_use] pub fn is_root(&self) -> bool { self.0 == Self::ROOT_NODE } @@ -110,6 +112,7 @@ impl NodeId { } /// Returns the nodeID as usize + #[must_use] pub fn as_usize(&self) -> usize { self.0 } @@ -148,13 +151,14 @@ pub struct Node { } impl Node { + #[must_use] pub fn is_root(&self) -> bool { self.id.is_root() } } impl PartialEq for Node { - fn eq(&self, other: &Node) -> bool { + fn eq(&self, other: &Self) -> bool { self.id == other.id } } @@ -183,7 +187,7 @@ impl Debug for Node { impl Clone for Node { fn clone(&self) -> Self { - Node { + Self { id: self.id, parent: self.parent, children: self.children.clone(), @@ -281,12 +285,14 @@ impl Node { } /// Returns true if the given node is a "formatting" node + #[must_use] pub fn is_formatting(&self) -> bool { self.namespace == Some(HTML_NAMESPACE.into()) && FORMATTING_HTML_ELEMENTS.contains(&self.name.as_str()) } /// Returns true if the given node is "special" node based on the namespace and name + #[must_use] pub fn is_special(&self) -> bool { if self.namespace == Some(HTML_NAMESPACE.into()) && SPECIAL_HTML_ELEMENTS.contains(&self.name.as_str()) @@ -308,6 +314,7 @@ impl Node { } /// Returns true if this node is registered into an arena + #[must_use] pub fn is_registered(&self) -> bool { self.is_registered } @@ -315,6 +322,7 @@ impl Node { /// This will only compare against the tag, namespace and data same except element data. /// for element data compaare against the tag, namespace and attributes without order. /// Both nodes could still have other parents and children. + #[must_use] pub fn matches_tag_and_attrs_without_order(&self, other: &Self) -> bool { if self.name != other.name || self.namespace != other.namespace { return false; @@ -341,6 +349,7 @@ impl Node { } /// Returns true when the given node is of the given namespace + #[must_use] pub fn is_namespace(&self, namespace: &str) -> bool { self.namespace == Some(namespace.into()) } @@ -377,6 +386,7 @@ impl Node { } /// Returns true if the node is an element node + #[must_use] pub fn is_element(&self) -> bool { if let NodeData::Element(_) = &self.data { return true; @@ -385,6 +395,7 @@ impl Node { false } + #[must_use] pub fn is_text(&self) -> bool { if let NodeData::Text(_) = &self.data { return true; @@ -393,6 +404,7 @@ impl Node { false } + #[must_use] pub fn as_text(&self) -> &TextData { if let NodeData::Text(text) = &self.data { return text; @@ -401,6 +413,7 @@ impl Node { panic!("Node is not a text"); } + #[must_use] pub fn as_element(&self) -> &ElementData { if let NodeData::Element(element) = &self.data { return element; @@ -418,6 +431,7 @@ impl Node { } /// Returns true when the given attribute has been set on the node + #[must_use] pub fn has_attribute(&self, name: &str) -> bool { if let NodeData::Element(element) = &self.data { return element.attributes.contains_key(name); @@ -427,6 +441,7 @@ impl Node { } /// Returns the given attribute value or None when the attribute is not found + #[must_use] pub fn get_attribute(&self, name: &str) -> Option<&String> { if let NodeData::Element(element) = &self.data { return element.attributes.get(name); @@ -547,7 +562,7 @@ pub static SPECIAL_HTML_ELEMENTS: [&str; 83] = [ "xmp", ]; -/// MathML elements that are considered special elements +/// `MathML` elements that are considered special elements pub static SPECIAL_MATHML_ELEMENTS: [&str; 6] = ["mi", "mo", "mn", "ms", "mtext", "annotation-xml"]; /// SVG elements that are considered special elements @@ -565,12 +580,9 @@ mod tests { assert_eq!(node.id, NodeId::default()); assert_eq!(node.parent, None); assert!(node.children.is_empty()); - assert_eq!(node.name, "".to_string()); + assert_eq!(node.name, String::new()); assert_eq!(node.namespace, None); - match &node.data { - NodeData::Document(_) => (), - _ => panic!(), - } + if let NodeData::Document(_) = &node.data {} else { panic!() } } #[test] @@ -605,7 +617,7 @@ mod tests { assert_eq!(node.id, NodeId::default()); assert_eq!(node.parent, None); assert!(node.children.is_empty()); - assert_eq!(node.name, "".to_string()); + assert_eq!(node.name, String::new()); assert_eq!(node.namespace, None); let NodeData::Comment(CommentData { value, .. }) = &node.data else { panic!() @@ -620,7 +632,7 @@ mod tests { assert_eq!(node.id, NodeId::default()); assert_eq!(node.parent, None); assert!(node.children.is_empty()); - assert_eq!(node.name, "".to_string()); + assert_eq!(node.name, String::new()); assert_eq!(node.namespace, None); let NodeData::Text(TextData { value }) = &node.data else { panic!() @@ -667,7 +679,7 @@ mod tests { #[test] fn special_html_elements() { let document = Document::shared(None); - for element in SPECIAL_HTML_ELEMENTS.iter() { + for element in &SPECIAL_HTML_ELEMENTS { let mut attributes = HashMap::new(); attributes.insert("id".to_string(), "test".to_string()); let node = Node::new_element( @@ -684,7 +696,7 @@ mod tests { #[test] fn special_mathml_elements() { let document = Document::shared(None); - for element in SPECIAL_MATHML_ELEMENTS.iter() { + for element in &SPECIAL_MATHML_ELEMENTS { let mut attributes = HashMap::new(); attributes.insert("id".to_string(), "test".to_string()); let node = Node::new_element( @@ -701,7 +713,7 @@ mod tests { #[test] fn special_svg_elements() { let document = Document::shared(None); - for element in SPECIAL_SVG_ELEMENTS.iter() { + for element in &SPECIAL_SVG_ELEMENTS { let mut attributes = HashMap::new(); attributes.insert("id".to_string(), "test".to_string()); let node = Node::new_element( diff --git a/crates/gosub_html5/src/node/arena.rs b/crates/gosub_html5/src/node/arena.rs index 2d13b58b2..e1de9b7b8 100644 --- a/crates/gosub_html5/src/node/arena.rs +++ b/crates/gosub_html5/src/node/arena.rs @@ -13,7 +13,7 @@ pub struct NodeArena { } impl NodeArena { - /// Creates a new NodeArena + /// Creates a new `NodeArena` #[must_use] pub fn new() -> Self { Self { @@ -29,12 +29,13 @@ impl NodeArena { } /// Peek what the next node ID is without incrementing the internal counter. - /// Used by DocumentTaskQueue for create_element() tasks. + /// Used by `DocumentTaskQueue` for `create_element()` tasks. pub(crate) fn peek_next_id(&self) -> NodeId { self.next_id } /// Gets the node with the given id + #[must_use] pub fn get_node(&self, node_id: NodeId) -> Option<&Node> { self.nodes.get(&node_id) } diff --git a/crates/gosub_html5/src/node/data/comment.rs b/crates/gosub_html5/src/node/data/comment.rs index 21287ecfb..2d8193828 100644 --- a/crates/gosub_html5/src/node/data/comment.rs +++ b/crates/gosub_html5/src/node/data/comment.rs @@ -1,4 +1,4 @@ -#[derive(Debug, PartialEq, Clone)] +#[derive(Debug, PartialEq, Eq, Clone)] /// Data structure for comment nodes pub struct CommentData { /// The actual comment value @@ -25,6 +25,7 @@ impl CommentData { } } + #[must_use] pub fn value(&self) -> &str { &self.value } diff --git a/crates/gosub_html5/src/node/data/doctype.rs b/crates/gosub_html5/src/node/data/doctype.rs index 4be0bd56f..f7e1bba97 100644 --- a/crates/gosub_html5/src/node/data/doctype.rs +++ b/crates/gosub_html5/src/node/data/doctype.rs @@ -1,7 +1,7 @@ use core::fmt::{Debug, Formatter}; use std::fmt; -#[derive(PartialEq, Clone)] +#[derive(PartialEq, Eq, Clone)] /// Data structure for document nodes pub struct DocTypeData { pub name: String, diff --git a/crates/gosub_html5/src/node/data/document.rs b/crates/gosub_html5/src/node/data/document.rs index dbaf70b68..740fada53 100644 --- a/crates/gosub_html5/src/node/data/document.rs +++ b/crates/gosub_html5/src/node/data/document.rs @@ -1,7 +1,7 @@ use core::fmt::{Debug, Formatter}; use std::fmt; -#[derive(PartialEq, Clone)] +#[derive(PartialEq, Eq, Clone)] /// Data structure for document nodes pub struct DocumentData {} diff --git a/crates/gosub_html5/src/node/data/element.rs b/crates/gosub_html5/src/node/data/element.rs index ce64b8ece..dfe7070fb 100644 --- a/crates/gosub_html5/src/node/data/element.rs +++ b/crates/gosub_html5/src/node/data/element.rs @@ -15,7 +15,7 @@ pub struct ElementData { pub name: String, /// Element's attributes stored as key-value pairs. /// Note that it is NOT RECOMMENDED to modify this - /// attribute map directly and instead use TreeBuilder.insert_attribute + /// attribute map directly and instead use `TreeBuilder.insert_attribute` /// to keep attributes in sync with the DOM. pub attributes: HashMap, /// CSS classes @@ -67,6 +67,7 @@ impl ElementData { } } + #[must_use] pub fn name(&self) -> &str { &self.name } diff --git a/crates/gosub_html5/src/node/data/text.rs b/crates/gosub_html5/src/node/data/text.rs index d241e7cfe..77a4c8f54 100644 --- a/crates/gosub_html5/src/node/data/text.rs +++ b/crates/gosub_html5/src/node/data/text.rs @@ -1,4 +1,4 @@ -#[derive(Clone, Debug, PartialEq)] +#[derive(Clone, Debug, PartialEq, Eq)] /// Data structure for text nodes pub struct TextData { /// Actual text @@ -25,6 +25,7 @@ impl TextData { } } + #[must_use] pub fn value(&self) -> &str { &self.value } diff --git a/crates/gosub_html5/src/parser/helper.rs b/crates/gosub_html5/src/parser/helper.rs index 2ba256ac0..248f3ce5a 100644 --- a/crates/gosub_html5/src/parser/helper.rs +++ b/crates/gosub_html5/src/parser/helper.rs @@ -250,6 +250,7 @@ impl Html5Parser<'_> { } // @todo: where is the fragment case handled? (substep 4: https://html.spec.whatwg.org/multipage/parsing.html#appropriate-place-for-inserting-a-node) + #[must_use] pub fn appropriate_place_insert( &self, override_node: Option, diff --git a/crates/gosub_html5/src/parser/query.rs b/crates/gosub_html5/src/parser/query.rs index 4036e8952..ee6d4d130 100644 --- a/crates/gosub_html5/src/parser/query.rs +++ b/crates/gosub_html5/src/parser/query.rs @@ -1,4 +1,4 @@ -#[derive(Debug, PartialEq)] +#[derive(Debug, PartialEq, Eq)] pub enum Condition { EqualsTag(String), EqualsId(String), @@ -8,7 +8,7 @@ pub enum Condition { HasParentTag(String), } -#[derive(Debug, PartialEq)] +#[derive(Debug, PartialEq, Eq)] pub enum SearchType { Uninitialized, FindFirst, diff --git a/crates/gosub_html5/src/parser/quirks.rs b/crates/gosub_html5/src/parser/quirks.rs index 3049f1ba8..e7dfbd42c 100644 --- a/crates/gosub_html5/src/parser/quirks.rs +++ b/crates/gosub_html5/src/parser/quirks.rs @@ -1,6 +1,6 @@ use crate::parser::Html5Parser; -#[derive(PartialEq, Debug, Copy, Clone)] +#[derive(PartialEq, Eq, Debug, Copy, Clone)] pub enum QuirksMode { Quirks, LimitedQuirks, diff --git a/crates/gosub_html5/src/parser/tree_builder.rs b/crates/gosub_html5/src/parser/tree_builder.rs index d27c756f9..f9c0e58f1 100644 --- a/crates/gosub_html5/src/parser/tree_builder.rs +++ b/crates/gosub_html5/src/parser/tree_builder.rs @@ -2,10 +2,10 @@ use crate::parser::NodeId; use gosub_shared::byte_stream::Location; use gosub_shared::types::Result; -/// TreeBuilder is an interface to abstract DOM tree modifications. +/// `TreeBuilder` is an interface to abstract DOM tree modifications. /// -/// This is implemented by DocumentHandle to support direct immediate manipulation of the DOM -/// and implemented by DocumentTaskQueue to support queueing up several mutations to be performed at once. +/// This is implemented by `DocumentHandle` to support direct immediate manipulation of the DOM +/// and implemented by `DocumentTaskQueue` to support queueing up several mutations to be performed at once. pub trait TreeBuilder { /// Create a new element node with the given tag name and append it to a parent /// with an optional position parameter which places the element at a specific child index. diff --git a/crates/gosub_html5/src/tokenizer/state.rs b/crates/gosub_html5/src/tokenizer/state.rs index 6ce2c6c99..fcd611715 100644 --- a/crates/gosub_html5/src/tokenizer/state.rs +++ b/crates/gosub_html5/src/tokenizer/state.rs @@ -1,5 +1,5 @@ /// These are the states in which the tokenizer can be in. -#[derive(Clone, Copy, Debug, PartialEq)] +#[derive(Clone, Copy, Debug, PartialEq, Eq)] pub enum State { /// 8.2.4.36 After attribute name state AfterAttributeName, diff --git a/crates/gosub_html5/src/tokenizer/test_cases.rs b/crates/gosub_html5/src/tokenizer/test_cases.rs index ec4916c32..3573340e7 100644 --- a/crates/gosub_html5/src/tokenizer/test_cases.rs +++ b/crates/gosub_html5/src/tokenizer/test_cases.rs @@ -1,3 +1,5 @@ +#![allow(clippy::unwrap_used, clippy::expect_used)] + use gosub_testing::testing::tokenizer::{self, FixtureFile}; use lazy_static::lazy_static; use std::collections::HashSet; @@ -31,7 +33,7 @@ const DISABLED_CASES: &[&str] = &[ lazy_static! { static ref DISABLED: HashSet = DISABLED_CASES .iter() - .map(|s| s.to_string()) + .map(|s| (*s).to_string()) .collect::>(); } @@ -53,8 +55,7 @@ fn tokenization(filename: &str) { let root = tokenizer::fixture_from_filename(filename).unwrap(); let tests = match root { - FixtureFile::Tests { tests } => tests, - FixtureFile::XmlTests { tests } => tests, + FixtureFile::XmlTests { tests } | FixtureFile::Tests { tests } => tests, }; for test in tests { diff --git a/crates/gosub_jsapi/Cargo.toml b/crates/gosub_jsapi/Cargo.toml index a09fae587..599571545 100644 --- a/crates/gosub_jsapi/Cargo.toml +++ b/crates/gosub_jsapi/Cargo.toml @@ -9,3 +9,7 @@ license = "MIT" gosub_shared = { path = "../gosub_shared", features = [] } uuid = { version = "1.10.0", features = ["v4"] } regex = "1" + + +[lints] +workspace = true diff --git a/crates/gosub_jsapi/src/console.rs b/crates/gosub_jsapi/src/console.rs index 7fa8c9a8a..8a8d17393 100755 --- a/crates/gosub_jsapi/src/console.rs +++ b/crates/gosub_jsapi/src/console.rs @@ -1,15 +1,17 @@ //! Console api as described by -mod buffer; -mod formatter; -mod writable_printer; - -use crate::console::formatter::Formatter; use std::collections::HashMap; -use std::fmt; +use std::fmt::{self, Write as _}; use std::time::{SystemTime, UNIX_EPOCH}; + use uuid::Uuid; -/// LogLevel is the type of log level. +use crate::console::formatter::Formatter; + +mod buffer; +mod formatter; +mod writable_printer; + +/// `LogLevel` is the type of log level. #[derive(Debug)] pub enum LogLevel { Info, @@ -85,6 +87,7 @@ impl Console { } /// Returns the printer that is used by the console + #[must_use] pub fn get_printer(self) -> Box { self.printer } @@ -132,7 +135,7 @@ impl Console { } /// Emit table if tabular data is supported - pub fn table(&mut self, tabular_data: String, _properties: &[&str]) { + pub fn table(&mut self, tabular_data: &String, _properties: &[&str]) { // TODO: needs implementation self.printer.print(LogLevel::Log, &[&tabular_data], &[]); } @@ -160,10 +163,11 @@ impl Console { /// Create a counter named "label" pub fn count(&mut self, label: &str) { - let mut cnt = 1; - if self.count_map.contains_key(label) { - cnt = self.count_map.get(label).unwrap() + 1; - } + let cnt = if self.count_map.contains_key(label) { + self.count_map.get(label).unwrap_or(&0) + 1 + } else { + 1 + }; self.count_map.insert(label.to_owned(), cnt); @@ -186,12 +190,10 @@ impl Console { let group_label = if data.is_empty() { format!("console.group.{}", Uuid::new_v4()) } else { - self.formatter.format(data).to_string() + self.formatter.format(data) }; - let group = Group { - label: group_label.clone(), - }; + let group = Group { label: group_label }; // Group should be expanded self.printer.print(LogLevel::Group, &[&group.label], &[]); @@ -204,12 +206,10 @@ impl Console { let group_label = if data.is_empty() { format!("console.group.{}", Uuid::new_v4()) } else { - self.formatter.format(data).to_string() + self.formatter.format(data) }; - let group = Group { - label: group_label.clone(), - }; + let group = Group { label: group_label }; self.printer .print(LogLevel::GroupCollapsed, &[&group.label], &[]); @@ -231,10 +231,9 @@ impl Console { return; } - let start = match SystemTime::now().duration_since(UNIX_EPOCH) { - Ok(n) => n.as_millis(), - Err(_) => 0, - }; + let start = SystemTime::now() + .duration_since(UNIX_EPOCH) + .map_or(0, |n| n.as_millis()); self.timer_map.insert( label.to_owned(), @@ -250,14 +249,13 @@ impl Console { pub fn time_log(&mut self, label: &str, data: &[&dyn fmt::Display]) { let mut message = String::from(" "); for arg in data { - message.push_str(&format!("{arg} ")); + let _ = write!(message, "{arg}"); } let message = message.trim_end(); - let cur = match SystemTime::now().duration_since(UNIX_EPOCH) { - Ok(n) => n.as_millis(), - Err(_) => 0, - }; + let cur = SystemTime::now() + .duration_since(UNIX_EPOCH) + .map_or(0, |n| n.as_millis()); let concat = format!( "{}: {}ms{}", @@ -270,10 +268,9 @@ impl Console { /// End the timer with the given label pub fn time_end(&mut self, label: &str) { - let end = match SystemTime::now().duration_since(UNIX_EPOCH) { - Ok(n) => n.as_millis(), - Err(_) => 0, - }; + let end = SystemTime::now() + .duration_since(UNIX_EPOCH) + .map_or(0, |n| n.as_millis()); let concat = format!( "{}: {}ms", @@ -311,13 +308,16 @@ pub trait Printer { #[cfg(test)] mod tests { - use super::*; - use crate::console::{buffer::Buffer, writable_printer::WritablePrinter}; - use regex::Regex; use std::cell::RefCell; use std::rc::Rc; use std::thread::sleep; + use regex::Regex; + + use crate::console::{buffer::Buffer, writable_printer::WritablePrinter}; + + use super::*; + #[test] fn console() { let buffer = Rc::new(RefCell::new(Buffer::new())); diff --git a/crates/gosub_jsapi/src/console/formatter.rs b/crates/gosub_jsapi/src/console/formatter.rs index 5a89713c5..bb331ebe4 100755 --- a/crates/gosub_jsapi/src/console/formatter.rs +++ b/crates/gosub_jsapi/src/console/formatter.rs @@ -1,4 +1,4 @@ -use std::fmt; +use std::fmt::{self, Write as _}; /// Formatting structure pub struct Formatter; @@ -6,15 +6,16 @@ pub struct Formatter; impl Formatter { /// Returns a new formatter #[must_use] - pub fn new() -> Self { + pub const fn new() -> Self { Self {} } /// Formats the given string based on the formatting arguments and data provided + #[allow(clippy::unused_self)] pub fn format(&self, args: &[&dyn fmt::Display]) -> String { let mut s = String::new(); for arg in args { - s.push_str(&format!("{arg} ")); + write!(s, "{arg}").unwrap(); // unreachable } s.trim_end().to_owned() diff --git a/crates/gosub_jsapi/src/console/writable_printer.rs b/crates/gosub_jsapi/src/console/writable_printer.rs index 2fd30aa70..b3856d40c 100755 --- a/crates/gosub_jsapi/src/console/writable_printer.rs +++ b/crates/gosub_jsapi/src/console/writable_printer.rs @@ -12,7 +12,7 @@ pub struct Group { type Writer = Rc>; /// A writable printer that can be used to write to a buffer -pub(crate) struct WritablePrinter { +pub struct WritablePrinter { writer: Writer, groups: Vec, } @@ -20,7 +20,7 @@ pub(crate) struct WritablePrinter { impl WritablePrinter { /// Creates a new writable printer #[allow(dead_code)] - pub fn new(writer: Rc>) -> Self { + pub const fn new(writer: Rc>) -> Self { Self { writer, groups: vec![], @@ -29,7 +29,7 @@ impl WritablePrinter { /// Returns a reference to the writer #[allow(dead_code)] - pub fn get_writer(&self) -> &Writer { + pub const fn get_writer(&self) -> &Writer { &self.writer } diff --git a/crates/gosub_net/Cargo.toml b/crates/gosub_net/Cargo.toml index b8388f13e..cd2f704be 100644 --- a/crates/gosub_net/Cargo.toml +++ b/crates/gosub_net/Cargo.toml @@ -19,3 +19,6 @@ simple_logger = "5.0.0" cookie = { version = "0.18.1", features = ["secure", "private"] } http = "1.0.0" url = "2.5.2" + +[lints] +workspace = true diff --git a/crates/gosub_net/src/dns.rs b/crates/gosub_net/src/dns.rs index 0eb1f837a..f344e1f3b 100644 --- a/crates/gosub_net/src/dns.rs +++ b/crates/gosub_net/src/dns.rs @@ -2,7 +2,7 @@ mod cache; mod local; mod remote; -use crate::errors::Error; +use crate::errors::NetError; use core::str::FromStr; use derive_more::Display; use gosub_config::config_store; @@ -12,7 +12,7 @@ use std::net::IpAddr; use std::time::{SystemTime, UNIX_EPOCH}; /// A DNS entry is a mapping of a domain to zero or more IP address mapping -#[derive(Clone, Debug, Default, PartialEq)] +#[derive(Clone, Debug, Default, PartialEq, Eq)] pub struct DnsEntry { // domain name domain: String, @@ -57,6 +57,7 @@ impl DnsEntry { } /// Returns true if the dns entry has expired + #[must_use] pub fn expired(&self) -> bool { self.expires < SystemTime::now() @@ -82,7 +83,7 @@ impl DnsEntry { } /// Type of DNS resolution -#[derive(Clone, Debug, Display, PartialEq)] +#[derive(Clone, Debug, Display, PartialEq, Eq)] pub enum ResolveType { /// Only resolve IPV4 addresses (A) Ipv4, @@ -93,7 +94,7 @@ pub enum ResolveType { } trait DnsResolver { - /// Resolves a domain name for a given resolver_type + /// Resolves a domain name for a given `resolver_type` fn resolve(&mut self, domain: &str, resolve_type: ResolveType) -> Result; /// Announces the resolved dns entry for the domain to a resolver fn announce(&mut self, _domain: &str, _entry: &DnsEntry) {} @@ -147,7 +148,7 @@ impl Dns { Self { resolvers } } - /// Resolves a domain name to a set of IP addresses based on the resolve_type. + /// Resolves a domain name to a set of IP addresses based on the `resolve_type`. /// It can resolve either Ipv4, ipv6 or both addresses. /// /// Each request will be resolved by the resolvers in the order they are added. @@ -171,7 +172,7 @@ impl Dns { } if entry.is_none() { - return Err(Error::DnsDomainNotFound.into()); + return Err(NetError::DnsDomainNotFound.into()); } // Iterate all resolvers and add to all cache systems (normally, this is only the first resolver) @@ -179,7 +180,7 @@ impl Dns { resolver.announce(domain, &entry.clone().unwrap().clone()); } - Ok(entry.unwrap().clone()) + Ok(entry.unwrap()) } } @@ -199,26 +200,26 @@ mod test { let now = Instant::now(); let e = dns.resolve("example.org", ResolveType::Ipv4).unwrap(); let elapsed_time = now.elapsed(); - e.ipv4().iter().for_each(|x| println!("ipv4: {}", x)); + e.ipv4().iter().for_each(|x| println!("ipv4: {x}")); println!("Took {} microseconds.", elapsed_time.as_micros()); let now = Instant::now(); let e = dns.resolve("example.org", ResolveType::Ipv6).unwrap(); let elapsed_time = now.elapsed(); - e.ipv6().iter().for_each(|x| println!("ipv6: {}", x)); + e.ipv6().iter().for_each(|x| println!("ipv6: {x}")); println!("Took {} microseconds.", elapsed_time.as_micros()); let now = Instant::now(); let e = dns.resolve("example.org", ResolveType::Ipv4).unwrap(); let elapsed_time = now.elapsed(); - e.ipv4().iter().for_each(|x| println!("ipv4: {}", x)); + e.ipv4().iter().for_each(|x| println!("ipv4: {x}")); println!("Took {} microseconds.", elapsed_time.as_micros()); let now = Instant::now(); let e = dns.resolve("example.org", ResolveType::Both).unwrap(); let elapsed_time = now.elapsed(); - e.ipv4().iter().for_each(|x| println!("ipv4: {}", x)); - e.ipv6().iter().for_each(|x| println!("ipv6: {}", x)); + e.ipv4().iter().for_each(|x| println!("ipv4: {x}")); + e.ipv6().iter().for_each(|x| println!("ipv6: {x}")); println!("Took {} microseconds.", elapsed_time.as_micros()); } } diff --git a/crates/gosub_net/src/dns/cache.rs b/crates/gosub_net/src/dns/cache.rs index 050cab856..49f88d4bc 100644 --- a/crates/gosub_net/src/dns/cache.rs +++ b/crates/gosub_net/src/dns/cache.rs @@ -1,10 +1,10 @@ use crate::dns::{DnsCache, DnsEntry, DnsResolver, ResolveType}; -use crate::errors::Error; +use crate::errors::NetError; use gosub_shared::types::Result; use log::trace; use std::collections::{HashMap, VecDeque}; -pub(crate) struct CacheResolver { +pub struct CacheResolver { values: HashMap, max_entries: usize, lru: VecDeque, @@ -15,15 +15,15 @@ impl DnsResolver for CacheResolver { if let Some(entry) = self.values.get(domain) { if !entry.has_ipv4 && !entry.has_ipv6 && resolve_type == ResolveType::Both { trace!("{}: no addresses found in entry", domain); - return Err(Error::DnsNoIpAddressFound.into()); + return Err(NetError::DnsNoIpAddressFound.into()); } if !entry.has_ipv4 && resolve_type == ResolveType::Ipv4 { trace!("{}: no ipv4 addresses found in entry", domain); - return Err(Error::DnsNoIpAddressFound.into()); + return Err(NetError::DnsNoIpAddressFound.into()); } if !entry.has_ipv6 && resolve_type == ResolveType::Ipv6 { trace!("{}: no ipv6 addresses found in entry", domain); - return Err(Error::DnsNoIpAddressFound.into()); + return Err(NetError::DnsNoIpAddressFound.into()); } trace!("{}: found in cache with correct resolve type", domain); @@ -33,7 +33,7 @@ impl DnsResolver for CacheResolver { return Ok(entry.clone()); } - Err(Error::DnsNoIpAddressFound.into()) + Err(NetError::DnsNoIpAddressFound.into()) } /// When a domain is resolved, it will be announced to all resolvers. This cache resolver @@ -91,8 +91,8 @@ impl DnsCache for CacheResolver { } impl CacheResolver { - pub(crate) fn new(max_entries: usize) -> CacheResolver { - CacheResolver { + pub(crate) fn new(max_entries: usize) -> Self { + Self { values: HashMap::with_capacity(max_entries), max_entries, lru: VecDeque::with_capacity(max_entries), diff --git a/crates/gosub_net/src/dns/local.rs b/crates/gosub_net/src/dns/local.rs index 093cc8924..3c09108f7 100644 --- a/crates/gosub_net/src/dns/local.rs +++ b/crates/gosub_net/src/dns/local.rs @@ -1,5 +1,5 @@ use crate::dns::{DnsCache, DnsEntry, DnsResolver, ResolveType}; -use crate::errors::Error; +use crate::errors::NetError; use core::fmt; use domain_lookup_tree::DomainLookupTree; use gosub_shared::types::Result; @@ -27,7 +27,7 @@ impl DnsResolver for LocalTableResolver { fn resolve(&mut self, domain: &str, _resolve_type: ResolveType) -> Result { let Some(domain_entry) = self.tree.lookup(domain) else { trace!("{domain}: not found in local table"); - return Err(Error::DnsDomainNotFound.into()); + return Err(NetError::DnsDomainNotFound.into()); }; trace!("{domain_entry}: found in local tree"); @@ -39,7 +39,7 @@ impl DnsResolver for LocalTableResolver { } trace!("{domain}: not found in local table"); - Err(Error::DnsDomainNotFound.into()) + Err(NetError::DnsDomainNotFound.into()) } fn name(&self) -> &'static str { diff --git a/crates/gosub_net/src/dns/remote.rs b/crates/gosub_net/src/dns/remote.rs index 9e07b5540..fc63fd672 100644 --- a/crates/gosub_net/src/dns/remote.rs +++ b/crates/gosub_net/src/dns/remote.rs @@ -1,5 +1,5 @@ use crate::dns::{DnsEntry, DnsResolver, ResolveType}; -use crate::errors::Error; +use crate::errors::NetError; use core::str::FromStr; use gosub_shared::types::Result; use hickory_resolver::config::Protocol::Udp; @@ -65,7 +65,7 @@ impl DnsResolver for RemoteResolver { } if !entry.has_ipv4 && !entry.has_ipv6 { - return Err(Error::DnsNoIpAddressFound.into()); + return Err(NetError::DnsNoIpAddressFound.into()); } Ok(entry) diff --git a/crates/gosub_net/src/errors.rs b/crates/gosub_net/src/errors.rs index b4353527b..77bf7d91b 100644 --- a/crates/gosub_net/src/errors.rs +++ b/crates/gosub_net/src/errors.rs @@ -3,7 +3,7 @@ use thiserror::Error; /// Serious errors and errors from third-party libraries #[derive(Debug, Error)] -pub enum Error { +pub enum NetError { #[error("ureq error")] Request(#[from] Box), diff --git a/crates/gosub_net/src/http/request.rs b/crates/gosub_net/src/http/request.rs index b25ac4b72..10a2695af 100644 --- a/crates/gosub_net/src/http/request.rs +++ b/crates/gosub_net/src/http/request.rs @@ -12,6 +12,7 @@ pub struct Request { } impl Request { + #[must_use] pub fn new(method: &str, uri: &str, version: &str) -> Self { Self { method: method.to_string(), @@ -37,13 +38,13 @@ impl Display for Request { writeln!(f, "{} {} {}", self.method, self.uri, self.version)?; writeln!(f, "Headers:")?; for (key, value) in self.headers.sorted() { - writeln!(f, " {}: {}", key, value)?; + writeln!(f, " {key}: {value}")?; } writeln!(f, "Cookies:")?; let mut sorted_cookies = self.cookies.iter().collect::>(); sorted_cookies.sort_by(|a, b| a.name().cmp(b.name())); for cookie in sorted_cookies { - writeln!(f, " {}", cookie)?; + writeln!(f, " {cookie}")?; } writeln!(f, "Body: {} bytes", self.body.len())?; @@ -87,7 +88,7 @@ mod tests { req.headers.set_str("Accept", "text/html"); req.headers.set_str("Accept-Encoding", "gzip, deflate, br"); - let s = format!("{}", req); + let s = format!("{req}"); assert_eq!(s, "GET / HTTP/1.1\nHeaders:\n Accept: text/html\n Accept-Encoding: gzip, deflate, br\n Content-Type: application/json\nCookies:\n foo=bar\n qux=wok\nBody: 0 bytes\n"); } } diff --git a/crates/gosub_net/src/http/response.rs b/crates/gosub_net/src/http/response.rs index ac30620a7..4ad99372e 100644 --- a/crates/gosub_net/src/http/response.rs +++ b/crates/gosub_net/src/http/response.rs @@ -14,10 +14,11 @@ pub struct Response { } impl Response { - pub fn new() -> Response { + #[must_use] + pub fn new() -> Self { Self { status: 0, - status_text: "".to_string(), + status_text: String::new(), version: "HTTP/1.1".to_string(), headers: Default::default(), cookies: Default::default(), @@ -94,11 +95,11 @@ impl Display for Response { writeln!(f, "HTTP/1.1 {}", self.status)?; writeln!(f, "Headers:")?; for (key, value) in self.headers.all() { - writeln!(f, " {}: {}", key, value)?; + writeln!(f, " {key}: {value}")?; } writeln!(f, "Cookies:")?; for (key, value) in &self.cookies { - writeln!(f, " {}: {}", key, value)?; + writeln!(f, " {key}: {value}")?; } writeln!(f, "Body: {} bytes", self.body.len())?; @@ -114,7 +115,7 @@ mod tests { fn response() { let mut response = Response::new(); - let s = format!("{}", response); + let s = format!("{response}"); assert_eq!(s, "HTTP/1.1 0\nHeaders:\nCookies:\nBody: 0 bytes\n"); response.status = 200; @@ -124,7 +125,7 @@ mod tests { .insert("session".to_string(), "1234567890".to_string()); response.body = b"Hello, world!".to_vec(); - let s = format!("{}", response); + let s = format!("{response}"); assert_eq!(s, "HTTP/1.1 200\nHeaders:\n Content-Type: application/json\nCookies:\n session: 1234567890\nBody: 13 bytes\n"); } } diff --git a/crates/gosub_render_backend/Cargo.toml b/crates/gosub_render_backend/Cargo.toml index 250722f5e..197bb22cb 100644 --- a/crates/gosub_render_backend/Cargo.toml +++ b/crates/gosub_render_backend/Cargo.toml @@ -11,3 +11,6 @@ gosub_shared = { path = "../gosub_shared" } gosub_html5 = { path = "../gosub_html5" } gosub_typeface = { path = "../gosub_typeface" } + +[lints] +workspace = true diff --git a/crates/gosub_render_utils/Cargo.toml b/crates/gosub_render_utils/Cargo.toml index 2cf2ef8eb..4b620e84a 100644 --- a/crates/gosub_render_utils/Cargo.toml +++ b/crates/gosub_render_utils/Cargo.toml @@ -13,3 +13,8 @@ gosub_render_backend = { path = "../gosub_render_backend" } anyhow = "1.0.86" regex = "1.10.6" rstar = "0.12.0" + + + +[lints] +workspace = true diff --git a/crates/gosub_render_utils/src/render_tree.rs b/crates/gosub_render_utils/src/render_tree.rs index 4116970e0..34f792cbf 100644 --- a/crates/gosub_render_utils/src/render_tree.rs +++ b/crates/gosub_render_utils/src/render_tree.rs @@ -12,7 +12,7 @@ pub mod properties; pub mod text; pub mod util; -/// A RenderTree is a data structure to be consumed by a user agent +/// A `RenderTree` is a data structure to be consumed by a user agent /// that combines the DOM and CSSOM to compute layouts and styles /// for objects to draw on the screen. pub struct RenderTree { @@ -81,10 +81,10 @@ impl RenderTree { } } -/// An individual node that sits inside a RenderTree. -/// A RenderTree Node mimics a Node from the DOM but +/// An individual node that sits inside a `RenderTree`. +/// A `RenderTree` Node mimics a Node from the DOM but /// contains more visual information such as width, -/// height, font sizes, colors, etc. A RenderTree Node +/// height, font sizes, colors, etc. A `RenderTree` Node /// can have children just like regular DOM nodes. #[derive(Debug, PartialEq)] #[repr(C)] @@ -138,52 +138,52 @@ impl Node { let margin = 10.72; let heading = TextNode::new_heading1(); - Node::new_text(heading, margin, position) + Self::new_text(heading, margin, position) } pub fn new_heading2(position: &mut Position) -> Self { let margin = 9.96; let heading = TextNode::new_heading2(); - Node::new_text(heading, margin, position) + Self::new_text(heading, margin, position) } pub fn new_heading3(position: &mut Position) -> Self { let margin = 9.36; let heading = TextNode::new_heading3(); - Node::new_text(heading, margin, position) + Self::new_text(heading, margin, position) } pub fn new_heading4(position: &mut Position) -> Self { let margin = 10.64; let heading = TextNode::new_heading4(); - Node::new_text(heading, margin, position) + Self::new_text(heading, margin, position) } pub fn new_heading5(position: &mut Position) -> Self { let margin = 11.089; let heading = TextNode::new_heading5(); - Node::new_text(heading, margin, position) + Self::new_text(heading, margin, position) } pub fn new_heading6(position: &mut Position) -> Self { let margin = 12.489; let heading = TextNode::new_heading6(); - Node::new_text(heading, margin, position) + Self::new_text(heading, margin, position) } pub fn new_paragraph(position: &mut Position) -> Self { let margin = 8.; let paragraph = TextNode::new_paragraph(); - Node::new_text(paragraph, margin, position) + Self::new_text(paragraph, margin, position) } - pub fn add_child(&mut self, child: &Rc>) { + pub fn add_child(&mut self, child: &Rc>) { if let Some(last_child) = &self.children.last().borrow_mut() { last_child.as_ref().borrow_mut().next_sibling = Some(Rc::clone(child)); } @@ -197,7 +197,7 @@ impl Default for Node { } } -/// Different types of RenderTree Nodes +/// Different types of `RenderTree` Nodes // NOTE: tag size must be u32 otherwise it wont work with a C enum (originally I tried u8) #[derive(Debug, PartialEq)] #[repr(C, u32)] @@ -210,7 +210,7 @@ pub enum NodeType { // TODO: add more types as we build out the RenderTree } -/// Constructs an iterator for a RenderTree +/// Constructs an iterator for a `RenderTree` pub struct TreeIterator { current_node: Option>>, node_stack: Vec>>, @@ -225,6 +225,7 @@ impl TreeIterator { } } + #[must_use] pub fn current(&self) -> Option>> { if let Some(node) = &self.current_node { return Some(Rc::clone(node)); diff --git a/crates/gosub_render_utils/src/render_tree/properties.rs b/crates/gosub_render_utils/src/render_tree/properties.rs index b7ffc601c..31661756a 100644 --- a/crates/gosub_render_utils/src/render_tree/properties.rs +++ b/crates/gosub_render_utils/src/render_tree/properties.rs @@ -25,6 +25,7 @@ impl Rectangle { } } + #[must_use] pub fn with_values(top: f64, left: f64, right: f64, bottom: f64) -> Self { Self { top, @@ -50,7 +51,8 @@ impl Position { Self { x: 0., y: 0. } } - pub fn new_from_existing(position: &Position) -> Self { + #[must_use] + pub fn new_from_existing(position: &Self) -> Self { Self { x: position.x, y: position.y, @@ -64,21 +66,21 @@ impl Position { } /// Move position relative to another position. - /// x = relative.x + x_offset - /// y = relative.y + y_offset - pub fn move_relative_to(&mut self, relative_position: &Position, x_offset: f64, y_offset: f64) { + /// x = relative.x + `x_offset` + /// y = relative.y + `y_offset` + pub fn move_relative_to(&mut self, relative_position: &Self, x_offset: f64, y_offset: f64) { self.x = relative_position.x + x_offset; self.y = relative_position.y + y_offset; } /// Adjust y by an offset. - /// y += offset_y + /// y += `offset_y` pub fn offset_y(&mut self, offset_y: f64) { self.y += offset_y; } /// Adjust x by an offset. - /// x += offset_x + /// x += `offset_x` pub fn offset_x(&mut self, offset_x: f64) { self.x += offset_x; } diff --git a/crates/gosub_render_utils/src/render_tree/text.rs b/crates/gosub_render_utils/src/render_tree/text.rs index d83e79ce4..101ccb823 100644 --- a/crates/gosub_render_utils/src/render_tree/text.rs +++ b/crates/gosub_render_utils/src/render_tree/text.rs @@ -13,38 +13,45 @@ impl TextNode { #[must_use] fn new(fs: f64, bold: bool) -> Self { Self { - value: "".to_owned(), + value: String::new(), font: "Times New Roman".to_owned(), font_size: fs, is_bold: bold, } } + #[must_use] pub fn new_heading1() -> Self { - TextNode::new(37., true) + Self::new(37., true) } + #[must_use] pub fn new_heading2() -> Self { - TextNode::new(27.5, true) + Self::new(27.5, true) } + #[must_use] pub fn new_heading3() -> Self { - TextNode::new(21.5, true) + Self::new(21.5, true) } + #[must_use] pub fn new_heading4() -> Self { - TextNode::new(18.5, true) + Self::new(18.5, true) } + #[must_use] pub fn new_heading5() -> Self { - TextNode::new(15.5, true) + Self::new(15.5, true) } + #[must_use] pub fn new_heading6() -> Self { - TextNode::new(12., true) + Self::new(12., true) } + #[must_use] pub fn new_paragraph() -> Self { - TextNode::new(18.5, false) + Self::new(18.5, false) } } diff --git a/crates/gosub_renderer/Cargo.toml b/crates/gosub_renderer/Cargo.toml index a918c04b6..ff3eebca5 100644 --- a/crates/gosub_renderer/Cargo.toml +++ b/crates/gosub_renderer/Cargo.toml @@ -25,3 +25,6 @@ log = "0.4.22" [target.'cfg(target_arch = "wasm32")'.dependencies] wasm-bindgen-futures = "0.4.42" web-sys = "0.3.55" + +[lints] +workspace = true diff --git a/crates/gosub_shared/Cargo.toml b/crates/gosub_shared/Cargo.toml index 1da88317b..8864725de 100644 --- a/crates/gosub_shared/Cargo.toml +++ b/crates/gosub_shared/Cargo.toml @@ -23,4 +23,9 @@ getrandom = { version = "0.2.15", features = ["js"] } web-sys = { version = "0.3.69", features = ["Performance", "Window"] } [dev-dependencies] -wasm-bindgen-test = "0.3.20" \ No newline at end of file +wasm-bindgen-test = "0.3.20" + + + +[lints] +workspace = true diff --git a/crates/gosub_shared/src/byte_stream.rs b/crates/gosub_shared/src/byte_stream.rs index fc92aeee7..a05564eb5 100644 --- a/crates/gosub_shared/src/byte_stream.rs +++ b/crates/gosub_shared/src/byte_stream.rs @@ -41,7 +41,7 @@ pub enum Character { StreamEmpty, } -use Character::*; +use Character::{Ch, StreamEmpty, StreamEnd, Surrogate}; /// Converts the given character to a char. This is only valid for UTF8 characters. Surrogate /// and EOF characters are converted to 0x0000 @@ -420,7 +420,7 @@ impl ByteStream { /// Populates the current buffer with the contents of given file f pub fn read_from_file(&mut self, mut f: impl Read) -> io::Result<()> { // First we read the u8 bytes into a buffer - f.read_to_end(&mut self.buffer).expect("uh oh"); + f.read_to_end(&mut self.buffer)?; self.close(); self.reset_stream(); self.close(); @@ -541,7 +541,7 @@ impl ByteStream { } /// Location holds the start position of the given element in the data source -#[derive(Clone, PartialEq)] +#[derive(Clone, PartialEq, Eq)] pub struct Location { /// Line number, starting with 1 pub line: usize, @@ -877,10 +877,10 @@ mod test { let ch = Ch('a'); assert_eq!(char::from(&ch), 'a'); assert_eq!(char::from(ch), 'a'); - assert_eq!(format!("{}", ch), "a"); + assert_eq!(format!("{ch}"), "a"); let ch = Surrogate(0xDFA9); - assert_eq!(format!("{}", ch), "U+DFA9"); + assert_eq!(format!("{ch}"), "U+DFA9"); assert!(!ch.is_numeric()); assert!(!ch.is_whitespace()); diff --git a/crates/gosub_shared/src/timing.rs b/crates/gosub_shared/src/timing.rs index 4aff52c1e..9f1eaeef8 100644 --- a/crates/gosub_shared/src/timing.rs +++ b/crates/gosub_shared/src/timing.rs @@ -1,3 +1,4 @@ +#![allow(clippy::module_name_repetitions)] //will always be used via gosub_shared:: use std::collections::HashMap; use std::fmt::{Display, Formatter}; use std::sync::Mutex; @@ -14,7 +15,7 @@ fn new_timer_id() -> TimerId { uuid::Uuid::new_v4() } -#[derive(Debug, Clone)] +#[derive(Debug, Clone, Copy)] pub enum Scale { MicroSecond, MilliSecond, @@ -58,8 +59,9 @@ fn percentage_to_index(count: u64, percentage: f64) -> usize { } impl TimingTable { - pub fn new() -> TimingTable { - TimingTable { + #[must_use] + pub fn new() -> Self { + Self { timers: HashMap::new(), namespaces: HashMap::new(), } @@ -82,6 +84,7 @@ impl TimingTable { } } + #[must_use] pub fn get_stats(&self, timers: &Vec) -> Stats { let mut durations: Vec = Vec::new(); @@ -94,7 +97,7 @@ impl TimingTable { } } - durations.sort(); + durations.sort_unstable(); let count = durations.len() as u64; let total = durations.iter().sum(); let min = *durations.first().unwrap_or(&0); @@ -118,14 +121,14 @@ impl TimingTable { } } - fn scale(&self, value: u64, scale: Scale) -> String { + fn scale(value: u64, scale: Scale) -> String { match scale { - Scale::MicroSecond => format!("{}µs", value), + Scale::MicroSecond => format!("{value}µs"), Scale::MilliSecond => format!("{}ms", value / 1000), Scale::Second => format!("{}s", value / (1000 * 1000)), Scale::Auto => { if value < 1000 { - format!("{}µs", value) + format!("{value}µs") } else if value < 1000 * 1000 { format!("{}ms", value / 1000) } else { @@ -142,25 +145,27 @@ impl TimingTable { let stats = self.get_stats(timers); println!("{:20} | {:>8} | {:>10} | {:>10} | {:>10} | {:>10} | {:>10} | {:>10} | {:>10} | {:>10}", namespace, stats.count, - self.scale(stats.total, scale.clone()), - self.scale(stats.min, scale.clone()), - self.scale(stats.max, scale.clone()), - self.scale(stats.avg, scale.clone()), - self.scale(stats.p50, scale.clone()), - self.scale(stats.p75, scale.clone()), - self.scale(stats.p95, scale.clone()), - self.scale(stats.p99, scale.clone()), + Self::scale(stats.total, scale), + Self::scale(stats.min, scale), + Self::scale(stats.max, scale), + Self::scale(stats.avg, scale), + Self::scale(stats.p50, scale), + Self::scale(stats.p75, scale), + Self::scale(stats.p95, scale), + Self::scale(stats.p99, scale), ); if show_details { for timer_id in timers { - let timer = self.timers.get(timer_id).unwrap(); + let Some(timer) = self.timers.get(timer_id) else { + continue; + }; if timer.has_finished() { println!( " | {:>8} | {:>10} | {}", 1, - self.scale(timer.duration_us, scale.clone()), - timer.context.clone().unwrap_or("".into()) + Self::scale(timer.duration_us, scale), + timer.context.clone().unwrap_or_default() ); } } @@ -168,12 +173,9 @@ impl TimingTable { } } + #[must_use] pub fn duration(&self, timer_id: TimerId) -> u64 { - if let Some(timer) = self.timers.get(&timer_id) { - timer.duration() - } else { - 0 - } + self.timers.get(&timer_id).map_or(0, Timer::duration) } } @@ -251,7 +253,8 @@ pub struct Timer { } impl Timer { - pub fn new(context: Option) -> Timer { + #[must_use] + pub fn new(context: Option) -> Self { #[cfg(not(target_arch = "wasm32"))] let start = { Instant::now() }; @@ -263,7 +266,7 @@ impl Timer { .unwrap_or(f64::NAN) }; - Timer { + Self { id: new_timer_id(), context, start, @@ -287,8 +290,10 @@ impl Timer { #[cfg(not(target_arch = "wasm32"))] pub fn end(&mut self) { - self.end = Some(Instant::now()); - self.duration_us = self.end.expect("").duration_since(self.start).as_micros() as u64; + let now = Instant::now(); + + self.end = Some(now); + self.duration_us = now.duration_since(self.start).as_micros() as u64; } #[cfg(target_arch = "wasm32")] @@ -300,11 +305,12 @@ impl Timer { .unwrap_or(f64::NAN) as u64; } - pub(crate) fn has_finished(&self) -> bool { + pub(crate) const fn has_finished(&self) -> bool { self.end.is_some() } - pub fn duration(&self) -> u64 { + #[must_use] + pub const fn duration(&self) -> u64 { if self.end.is_some() { self.duration_us } else { diff --git a/crates/gosub_shared/src/types.rs b/crates/gosub_shared/src/types.rs index c6fb88afa..57fba7541 100644 --- a/crates/gosub_shared/src/types.rs +++ b/crates/gosub_shared/src/types.rs @@ -5,7 +5,7 @@ use std::ops::Add; use thiserror::Error; /// Parser error that defines an error (message) on the given position -#[derive(Clone, Debug, PartialEq)] +#[derive(Clone, Debug, PartialEq, Eq)] pub struct ParseError { /// Parse error message pub message: String, @@ -15,7 +15,7 @@ pub struct ParseError { /// Serious errors and errors from third-party libraries #[derive(Debug, Error)] -pub enum Error { +pub enum GosubError { #[error("config error: {0}")] Config(String), @@ -45,22 +45,22 @@ pub struct Size { } impl Size { - pub fn new(width: T, height: T) -> Self { + pub const fn new(width: T, height: T) -> Self { Self { width, height } } - pub fn uniform(size: T) -> Self { + pub const fn uniform(size: T) -> Self { Self { width: size, height: size, } } - pub fn width(&self) -> &T { + pub const fn width(&self) -> &T { &self.width } - pub fn height(&self) -> &T { + pub const fn height(&self) -> &T { &self.height } } @@ -72,15 +72,15 @@ pub struct Point { } impl Point { - pub fn new(x: T, y: T) -> Self { + pub const fn new(x: T, y: T) -> Self { Self { x, y } } - pub fn x(&self) -> &T { + pub const fn x(&self) -> &T { &self.x } - pub fn y(&self) -> &T { + pub const fn y(&self) -> &T { &self.y } } diff --git a/crates/gosub_styling/Cargo.toml b/crates/gosub_styling/Cargo.toml index 4e17916b1..1593577e5 100644 --- a/crates/gosub_styling/Cargo.toml +++ b/crates/gosub_styling/Cargo.toml @@ -30,3 +30,6 @@ serde_json = "1.0.127" memoize = "0.4.2" thiserror = "1.0.58" nom = "7.1.3" + +[lints] +workspace = true diff --git a/crates/gosub_svg/Cargo.toml b/crates/gosub_svg/Cargo.toml index 858973d13..f5d89f16c 100644 --- a/crates/gosub_svg/Cargo.toml +++ b/crates/gosub_svg/Cargo.toml @@ -16,3 +16,6 @@ anyhow = "1.0.86" [features] resvg = ["dep:resvg"] + +[lints] +workspace = true diff --git a/crates/gosub_taffy/Cargo.toml b/crates/gosub_taffy/Cargo.toml index 9bbf14983..fa2e1954b 100644 --- a/crates/gosub_taffy/Cargo.toml +++ b/crates/gosub_taffy/Cargo.toml @@ -13,3 +13,7 @@ anyhow = "1.0.86" regex = "1.10.5" parley = { git = "https://github.com/linebender/parley", rev = "14070d5" } log = "0.4.22" + + +[lints] +workspace = true diff --git a/crates/gosub_testing/Cargo.toml b/crates/gosub_testing/Cargo.toml index 644411f14..d6f0c3836 100644 --- a/crates/gosub_testing/Cargo.toml +++ b/crates/gosub_testing/Cargo.toml @@ -14,4 +14,9 @@ serde_derive = "1.0" lazy_static = "1.5" nom = "7.1.3" nom_locate = "4.2.0" -regex = "1" \ No newline at end of file +regex = "1" + + + +[lints] +workspace = true diff --git a/crates/gosub_testing/src/testing/tree_construction.rs b/crates/gosub_testing/src/testing/tree_construction.rs index 693c3c563..68cb92fb5 100644 --- a/crates/gosub_testing/src/testing/tree_construction.rs +++ b/crates/gosub_testing/src/testing/tree_construction.rs @@ -17,7 +17,7 @@ use result::TestResult; use result::{ResultStatus, TreeLineResult}; /// Holds a single parser test -#[derive(Debug, Default, PartialEq, Clone)] +#[derive(Debug, Default, PartialEq, Eq, Clone)] pub struct Test { /// Filename of the test pub file_path: String, @@ -31,6 +31,7 @@ pub struct Test { impl Test { /// Returns the script modes that should be tested as an array + #[must_use] pub fn script_modes(&self) -> &[bool] { match self.spec.script_mode { ScriptMode::ScriptOff => &[false], @@ -39,10 +40,12 @@ impl Test { } } + #[must_use] pub fn document_as_str(&self) -> &str { self.spec.document.as_str() } + #[must_use] pub fn spec_data(&self) -> &str { self.spec.data.as_str() } @@ -64,7 +67,7 @@ impl Default for Harness { } impl Harness { - /// Generated a new harness instance. It uses a dummy test that is replaced when run_test is called + /// Generated a new harness instance. It uses a dummy test that is replaced when `run_test` is called #[must_use] pub fn new() -> Self { Self { diff --git a/crates/gosub_testing/src/testing/tree_construction/fixture.rs b/crates/gosub_testing/src/testing/tree_construction/fixture.rs index a8b751567..9f4f27311 100644 --- a/crates/gosub_testing/src/testing/tree_construction/fixture.rs +++ b/crates/gosub_testing/src/testing/tree_construction/fixture.rs @@ -6,7 +6,7 @@ use std::fs; use std::path::{Path, PathBuf}; /// Holds all tests as found in the given fixture file -#[derive(Debug, PartialEq)] +#[derive(Debug, PartialEq, Eq)] pub struct FixtureFile { /// All the tests extracted from this fixture file pub tests: Vec, @@ -48,6 +48,7 @@ fn use_fixture(filenames: &[&str], path: impl AsRef) -> bool { } /// Returns the root path for the fixtures +#[must_use] pub fn fixture_root_path() -> PathBuf { PathBuf::from(FIXTURE_ROOT).join(TREE_CONSTRUCTION_PATH) } @@ -80,7 +81,7 @@ fn create_document_array(s: &str) -> Vec { .replace(QUOTED_DOUBLE_NEWLINE, "\"\n\n\"") .split('|') .skip(1) - .flat_map(|l| (!l.is_empty()).then(|| format!("|{}", l.trim_end()))) + .filter_map(|l| (!l.is_empty()).then(|| format!("|{}", l.trim_end()))) .collect::>(); document diff --git a/crates/gosub_testing/src/testing/tree_construction/parser.rs b/crates/gosub_testing/src/testing/tree_construction/parser.rs index 49a7de3e7..e4455c081 100644 --- a/crates/gosub_testing/src/testing/tree_construction/parser.rs +++ b/crates/gosub_testing/src/testing/tree_construction/parser.rs @@ -1,5 +1,5 @@ // See https://github.com/html5lib/html5lib-tests/tree/master/tree-construction -use gosub_shared::types::{Error, Result}; +use gosub_shared::types::{GosubError, Result}; use nom::{ branch::alt, bytes::complete::{tag, take_until, take_until1}, @@ -15,13 +15,13 @@ pub const QUOTED_DOUBLE_NEWLINE: &str = ":quoted-double-newline:"; type Span<'a> = LocatedSpan<&'a str>; -#[derive(Clone, Copy, Debug, Default, PartialEq)] +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] pub struct Position { pub line: usize, pub col: usize, } -#[derive(Clone, Debug, PartialEq)] +#[derive(Clone, Debug, PartialEq, Eq)] pub enum ErrorSpec { Message(String), @@ -42,7 +42,7 @@ pub enum ErrorSpec { }, } -#[derive(Clone, Debug, Default, PartialEq)] +#[derive(Clone, Debug, Default, PartialEq, Eq)] pub enum ScriptMode { ScriptOn, ScriptOff, @@ -50,7 +50,7 @@ pub enum ScriptMode { Both, } -#[derive(Debug, PartialEq, Default, Clone)] +#[derive(Debug, PartialEq, Eq, Default, Clone)] pub struct TestSpec { /// #data section pub data: String, @@ -244,7 +244,7 @@ fn old_errors(i: Span) -> IResult> { )), error_messages, ))), - |errors| errors.unwrap_or_default(), + std::option::Option::unwrap_or_default, ), multispace0, )(i) @@ -319,7 +319,7 @@ fn trim_last_newline(s: String) -> String { pub fn parse_fixture(i: &str) -> Result> { // Deal with a corner case that makes it hard to parse tricky01.dat. - let input = i.replace("\"\n\n\"", QUOTED_DOUBLE_NEWLINE).clone() + "\n"; + let input = i.replace("\"\n\n\"", QUOTED_DOUBLE_NEWLINE) + "\n"; let files = map( tuple((separated_list1(tag("\n\n"), test), multispace0)), @@ -328,7 +328,7 @@ pub fn parse_fixture(i: &str) -> Result> { let (_, tests) = all_consuming(files)(Span::new(&input)) .finish() - .map_err(|err| Error::Test(format!("{err}")))?; + .map_err(|err| GosubError::Test(format!("{err}")))?; Ok(tests) } @@ -359,7 +359,7 @@ mod tests { assert_eq!( *s, "

\n" - ) + ); } #[test] @@ -516,7 +516,7 @@ FOO