From 8f147b044c51e3e4313b6edfcb94dacc0b7e667c Mon Sep 17 00:00:00 2001 From: Shark Date: Tue, 17 Sep 2024 21:38:23 +0200 Subject: [PATCH] fix more warnings --- crates/gosub_config/src/errors.rs | 2 +- crates/gosub_config/src/settings.rs | 67 +++++++++---------- crates/gosub_config/src/storage/json.rs | 24 +++---- crates/gosub_config/src/storage/sqlite.rs | 2 +- crates/gosub_css3/src/parser/at_rule/media.rs | 4 +- crates/gosub_css3/src/stylesheet.rs | 6 +- crates/gosub_css3/src/tokenizer.rs | 19 +++--- crates/gosub_net/src/dns.rs | 4 +- crates/gosub_net/src/dns/cache.rs | 10 +-- crates/gosub_net/src/dns/local.rs | 6 +- crates/gosub_net/src/dns/remote.rs | 4 +- crates/gosub_net/src/errors.rs | 2 +- crates/gosub_useragent/src/window.rs | 9 ++- crates/gosub_v8/src/v8/context.rs | 3 +- crates/gosub_webexecutor/tests/interop.rs | 26 ++----- src/engine.rs | 2 +- 16 files changed, 84 insertions(+), 106 deletions(-) diff --git a/crates/gosub_config/src/errors.rs b/crates/gosub_config/src/errors.rs index 0f1869d4d..4521737c7 100644 --- a/crates/gosub_config/src/errors.rs +++ b/crates/gosub_config/src/errors.rs @@ -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/settings.rs b/crates/gosub_config/src/settings.rs index 5b0720f48..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}; @@ -149,7 +149,7 @@ impl Display for Setting { } impl FromStr for Setting { - type Err = Error; + type Err = ConfigError; // first element is the type: // b:true @@ -159,36 +159,33 @@ 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" => Self::Bool( - key_value - .parse::() - .map_err(|err| Error::Config(format!("error parsing {key_value}: {err}")))?, - ), - "i" => Self::SInt( - key_value - .parse::() - .map_err(|err| Error::Config(format!("error parsing {key_value}: {err}")))?, - ), - "u" => Self::UInt( - key_value - .parse::() - .map_err(|err| Error::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()); + 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) } - Self::Map(values) - } - _ => return Err(Error::Config(format!("unknown setting: {key_value}"))), - }; + _ => return Err(ConfigError::Config(format!("unknown setting: {key_value}"))), + }; Ok(setting) } @@ -257,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 df4ad2b0d..286489da0 100644 --- a/crates/gosub_config/src/storage/json.rs +++ b/crates/gosub_config/src/storage/json.rs @@ -21,15 +21,11 @@ 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 @@ -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 4dcf0189a..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, diff --git a/crates/gosub_css3/src/parser/at_rule/media.rs b/crates/gosub_css3/src/parser/at_rule/media.rs index f949a13a7..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 { @@ -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/stylesheet.rs b/crates/gosub_css3/src/stylesheet.rs index d1255c89f..e5507b8a4 100644 --- a/crates/gosub_css3/src/stylesheet.rs +++ b/crates/gosub_css3/src/stylesheet.rs @@ -6,7 +6,7 @@ use std::cmp::Ordering; use std::fmt::Display; /// Defines a complete stylesheet with all its rules and the location where it was found -#[derive(Debug, PartialEq, Eq, Clone)] +#[derive(Debug, PartialEq, Clone)] #[allow(clippy::module_name_repetitions)] pub struct CssStylesheet { /// List of rules found in this stylesheet @@ -29,7 +29,7 @@ pub enum CssOrigin { } /// A CSS rule, which contains a list of selectors and a list of declarations -#[derive(Debug, PartialEq, Eq, Clone)] +#[derive(Debug, PartialEq, Clone)] pub struct CssRule { /// Selectors that must match for the declarations to apply pub selectors: Vec, @@ -50,7 +50,7 @@ impl CssRule { } /// A CSS declaration, which contains a property, value and a flag for !important -#[derive(Debug, PartialEq, Eq, Clone)] +#[derive(Debug, PartialEq, Clone)] pub struct CssDeclaration { // Css property color pub property: String, diff --git a/crates/gosub_css3/src/tokenizer.rs b/crates/gosub_css3/src/tokenizer.rs index 522680f6b..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 } diff --git a/crates/gosub_net/src/dns.rs b/crates/gosub_net/src/dns.rs index be09aafd6..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; @@ -172,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) diff --git a/crates/gosub_net/src/dns/cache.rs b/crates/gosub_net/src/dns/cache.rs index cb5248120..49f88d4bc 100644 --- a/crates/gosub_net/src/dns/cache.rs +++ b/crates/gosub_net/src/dns/cache.rs @@ -1,5 +1,5 @@ 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}; @@ -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 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_useragent/src/window.rs b/crates/gosub_useragent/src/window.rs index 5cebaebfc..053daecd7 100644 --- a/crates/gosub_useragent/src/window.rs +++ b/crates/gosub_useragent/src/window.rs @@ -68,7 +68,7 @@ impl<'a, D: SceneDrawer, B: RenderBackend, L: Layouter, LT: LayoutTree ) -> Result { let window = create_window(event_loop)?; - let renderer_data = backend.create_window_data(window.clone())?; + let renderer_data = backend.create_window_data(Arc::clone(&window))?; Ok(Self { state: WindowState::Suspended, @@ -88,7 +88,8 @@ impl<'a, D: SceneDrawer, B: RenderBackend, L: Layouter, LT: LayoutTree let size = self.window.inner_size(); let size = SizeU32::new(size.width, size.height); - let data = backend.activate_window(self.window.clone(), &mut self.renderer_data, size)?; + let data = + backend.activate_window(Arc::clone(&self.window), &mut self.renderer_data, size)?; self.state = WindowState::Active { surface: data }; @@ -100,7 +101,9 @@ impl<'a, D: SceneDrawer, B: RenderBackend, L: Layouter, LT: LayoutTree return; }; - if let Err(e) = backend.suspend_window(self.window.clone(), data, &mut self.renderer_data) { + if let Err(e) = + backend.suspend_window(Arc::clone(&self.window), data, &mut self.renderer_data) + { warn!("Failed to suspend window: {}", e); } diff --git a/crates/gosub_v8/src/v8/context.rs b/crates/gosub_v8/src/v8/context.rs index 79fa94d36..174cc7131 100644 --- a/crates/gosub_v8/src/v8/context.rs +++ b/crates/gosub_v8/src/v8/context.rs @@ -1,3 +1,4 @@ +use std::fmt::Write; use std::ptr::NonNull; use v8::{ @@ -151,7 +152,7 @@ impl<'a> V8Ctx<'a> { err.push_str(&m.get(try_catch).to_rust_string_lossy(try_catch)); if let Some(stacktrace) = m.get_stack_trace(try_catch) { let st = Self::handle_stack_trace(try_catch, stacktrace); - err.push_str(&format!("\nStacktrace:\n{st}")) + err.write_fmt(format_args!("\nStacktrace: {st}")).unwrap(); } else { err.push_str("\nStacktrace: "); }; diff --git a/crates/gosub_webexecutor/tests/interop.rs b/crates/gosub_webexecutor/tests/interop.rs index 5582508f9..7f74f2c3e 100644 --- a/crates/gosub_webexecutor/tests/interop.rs +++ b/crates/gosub_webexecutor/tests/interop.rs @@ -343,11 +343,7 @@ impl JSInterop for Test2 { }; #[allow(clippy::unit_arg)] - let ret = s - .borrow_mut() - .add(arg0 as i32) - .to_js_value(ctx) - .unwrap(); + let ret = s.borrow_mut().add(arg0 as i32).to_js_value(ctx).unwrap(); cb.ret(ret); })? @@ -403,11 +399,7 @@ impl JSInterop for Test2 { return; }; - let ret = s - .borrow() - .takes_ref(&arg0) - .to_js_value(ctx) - .unwrap(); + let ret = s.borrow().takes_ref(&arg0).to_js_value(ctx).unwrap(); cb.ret(ret); })? @@ -486,11 +478,7 @@ impl JSInterop for Test2 { return; }; - let ret = s - .borrow() - .generic(arg0, arg1) - .to_js_value(ctx) - .unwrap(); + let ret = s.borrow().generic(arg0, arg1).to_js_value(ctx).unwrap(); cb.ret(ret); @@ -503,11 +491,7 @@ impl JSInterop for Test2 { return; }; - let ret = s - .borrow() - .generic(arg0, arg1) - .to_js_value(ctx) - .unwrap(); + let ret = s.borrow().generic(arg0, arg1).to_js_value(ctx).unwrap(); cb.ret(ret); } @@ -531,7 +515,7 @@ fn manual_js_interop() { other_field: "Hello, ".to_string(), })); - Test2::implement::(t2.clone(), context.clone()).unwrap(); + Test2::implement::(Rc::clone(&t2), context.clone()).unwrap(); let out = context .run( diff --git a/src/engine.rs b/src/engine.rs index 917c82379..2231ae531 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -9,7 +9,7 @@ use { dns::{Dns, ResolveType}, http::{headers::Headers, request::Request, response::Response}, }, - gosub_shared::types::{Error, ParseError, Result}, + gosub_shared::types::{ParseError, Result}, gosub_shared::{timing_start, timing_stop}, std::io::Read, url::Url,