fix more warnings

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),
 

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<Self, crate::errors::Error> {
-        let (key_type, key_value) = key.split_once(':').expect("");
-
-        let setting = match key_type {
-            "b" => Self::Bool(
-                key_value
-                    .parse::<bool>()
-                    .map_err(|err| Error::Config(format!("error parsing {key_value}: {err}")))?,
-            ),
-            "i" => Self::SInt(
-                key_value
-                    .parse::<isize>()
-                    .map_err(|err| Error::Config(format!("error parsing {key_value}: {err}")))?,
-            ),
-            "u" => Self::UInt(
-                key_value
-                    .parse::<usize>()
-                    .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<Self, crate::errors::ConfigError> {
+        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::<bool>().map_err(|err| {
+                    ConfigError::Config(format!("error parsing {key_value}: {err}"))
+                })?),
+                "i" => Self::SInt(key_value.parse::<isize>().map_err(|err| {
+                    ConfigError::Config(format!("error parsing {key_value}: {err}"))
+                })?),
+                "u" => Self::UInt(key_value.parse::<usize>().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());

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(())
     }
 }

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<Self> {
-        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,

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()?;

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<CssSelector>,
@@ -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,

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
         }

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)

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

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<DnsEntry> {
         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 {

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)

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<ureq::Error>),
 

crates/gosub_useragent/src/window.rs

@@ -68,7 +68,7 @@ impl<'a, D: SceneDrawer<B, L, LT>, B: RenderBackend, L: Layouter, LT: LayoutTree
     ) -> Result<Self> {
         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, L, LT>, 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, L, LT>, 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);
         }
 

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: <missing information>");
             };