Merge pull request #1073 from jaytaph/linting

Added linting checks on all crates

Author: jaytaph

Cargo.toml

@@ -2,7 +2,7 @@
 name = "gosub_bin"
 version = "0.1.1"
 edition = "2021"
-rust-version = "1.80"
+rust-version = "1.82"
 authors = ["Gosub Community <info@gosub.io>"]
 description = "Gosub Browser Engine"
 license = "MIT"

crates/gosub_config/Cargo.toml

@@ -24,4 +24,6 @@ cow-utils = { workspace = true }
 rusqlite = "0.39.0"
 
 [dev-dependencies]
-testing_logger = "0.1.1"
+testing_logger = "0.1.1"
+[lints]
+workspace = true

crates/gosub_css3/Cargo.toml

@@ -26,3 +26,6 @@ indexmap = { version = "2.13.0", optional = true }
 [features]
 default = []
 unresolved_syntax = ["dep:indexmap"]
+
+[lints]
+workspace = true

crates/gosub_css3/src/ast.rs

@@ -182,10 +182,13 @@ fn collect_rule(node: &CssNode) -> CssResult<Option<CssRule>> {
                 continue;
             }
 
-            let value = if css_values.len() == 1 {
-                css_values.pop().expect("unreachable")
-            } else {
-                CssValue::List(css_values)
+            let value = match css_values.pop() {
+                Some(value) if css_values.is_empty() => value,
+                Some(value) => {
+                    css_values.push(value);
+                    CssValue::List(css_values)
+                }
+                None => CssValue::List(css_values),
             };
 
             rule.declarations.push(CssDeclaration {
@@ -294,7 +297,7 @@ mod tests {
 
     #[test]
     fn convert_font_family() {
-        let stylesheet = Css3::parse_str(
+        let _stylesheet = Css3::parse_str(
             r#"
               body {
                 border: 1px solid black;
@@ -310,8 +313,6 @@ mod tests {
             "test.css",
         )
         .unwrap();
-
-        dbg!(&stylesheet);
     }
 
     #[test]

crates/gosub_css3/src/colors.rs

@@ -62,37 +62,31 @@ impl From<&str> for RgbColor {
         }
         if value.starts_with("rgb(") {
             // Rgb function
-            let rgb = Rgb::from_str(value);
-            if rgb.is_err() {
+            let Ok(rgb) = Rgb::from_str(value) else {
                 return RgbColor::default();
-            }
-            let rgb = rgb.unwrap();
+            };
             return RgbColor::new(rgb.get_red(), rgb.get_green(), rgb.get_blue(), 255.0);
         }
         if value.starts_with("rgba(") {
             // Rgba function — alpha from colors_transform is in 0..1 range; scale to 0..255
-            let rgb = Rgb::from_str(value);
-            if rgb.is_err() {
+            let Ok(rgb) = Rgb::from_str(value) else {
                 return RgbColor::default();
-            }
-            let rgb = rgb.unwrap();
+            };
             return RgbColor::new(rgb.get_red(), rgb.get_green(), rgb.get_blue(), rgb.get_alpha() * 255.0);
         }
         if value.starts_with("hsl(") {
-            let hsl = Hsl::from_str(value);
-            if hsl.is_err() {
+            let Ok(hsl) = Hsl::from_str(value) else {
                 return RgbColor::default();
-            }
-            let rgb: Rgb = hsl.unwrap().to_rgb();
+            };
+            let rgb: Rgb = hsl.to_rgb();
             return RgbColor::new(rgb.get_red(), rgb.get_green(), rgb.get_blue(), 255.0);
         }
         if value.starts_with("hsla(") {
             // hsla() — alpha from colors_transform is in 0..1 range; scale to 0..255
-            let hsl = Hsl::from_str(value);
-            if hsl.is_err() {
+            let Ok(hsl) = Hsl::from_str(value) else {
                 return RgbColor::default();
-            }
-            let rgb: Rgb = hsl.unwrap().to_rgb();
+            };
+            let rgb: Rgb = hsl.to_rgb();
             return RgbColor::new(rgb.get_red(), rgb.get_green(), rgb.get_blue(), rgb.get_alpha() * 255.0);
         }
 
@@ -307,15 +301,14 @@ fn parse_hex(value: &str) -> RgbColor {
 fn convert_from_hex_str_to_vec_of_ints(hex_value: &str, hex_size: usize) -> Vec<i32> {
     const HEX_RADIX: u32 = 16;
     const LINES_TO_SKIP: usize = 1;
-    const EXPECT_MESSAGE: &str = "is_hex has failed us";
     // Get the individual chars from the hex then convert from hex -> decimal
     match hex_size {
         // if each hex char is only 1 char long
         1 => {
             hex_value
                 .chars()
                 .skip(LINES_TO_SKIP) // Skip the # at the front
-                .map(|char| i32::from_str_radix(char.to_string().as_str(), HEX_RADIX).expect(EXPECT_MESSAGE)) // Can parse with an unwrap because is_hex made sure it's digits
+                .map(|char| i32::from_str_radix(char.to_string().as_str(), HEX_RADIX).unwrap_or(0)) // is_hex() above guarantees digits
                 .collect::<Vec<i32>>()
         }
         // if each hex char is 2 char long
@@ -331,7 +324,7 @@ fn convert_from_hex_str_to_vec_of_ints(hex_value: &str, hex_size: usize) -> Vec<
 
             hex_vec
                 .iter()
-                .map(|str| i32::from_str_radix(str, HEX_RADIX).expect(EXPECT_MESSAGE))
+                .map(|str| i32::from_str_radix(str, HEX_RADIX).unwrap_or(0))
                 .collect::<Vec<i32>>()
         }
         _ => {

crates/gosub_css3/src/lib.rs

@@ -20,6 +20,9 @@ pub mod errors;
 mod functions;
 #[allow(dead_code)]
 pub mod matcher;
+// The as_* accessors panic by contract when called on the wrong node type;
+// callers are expected to check the matching is_* predicate first.
+#[allow(clippy::panic)]
 pub mod node;
 pub mod parser;
 pub mod stylesheet;
@@ -119,6 +122,7 @@ pub fn load_default_useragent_stylesheet() -> CssStylesheet {
     };
 
     let css_data = include_str!("../resources/useragent.css");
+    #[allow(clippy::expect_used)] // PANIC-SAFE: compiled-in stylesheet, exercised by every parser test
     Css3::parse_str(css_data, config, CssOrigin::UserAgent, url).expect("Could not parse useragent stylesheet")
 }
 

crates/gosub_css3/src/matcher/property_definitions.rs

@@ -249,13 +249,11 @@ impl CssDefinitions {
             return;
         }
 
-        if !self.properties.contains_key(name) {
+        // Resolve the element
+        let Some(mut element) = self.properties.get(name).cloned() else {
             // Property not found to resolve
             return;
-        }
-
-        // Resolve the element
-        let mut element = self.properties.get_mut(name).unwrap().clone();
+        };
         let mut resolved_components = vec![];
         for component in &element.syntax.components {
             let component = self.resolve_component(component, name);
@@ -329,7 +327,11 @@ impl CssDefinitions {
                     };
                 }
 
-                panic!("Unknown datatype encountered: {datatype:?}");
+                #[allow(clippy::panic)]
+                // PANIC-SAFE: datatypes come from the compiled-in definitions; the test suite resolves them all
+                {
+                    panic!("Unknown datatype encountered: {datatype:?}");
+                }
             }
             SyntaxComponent::Group {
                 components,
@@ -379,11 +381,13 @@ pub static CSS_PROPERTIES: LazyLock<indexmap::IndexMap<String, PropertyDefinitio
 pub const DEFINITIONS_VALUES: &str = include_str!("../../resources/definitions/definitions_values.json");
 pub const DEFINITIONS_PROPERTIES: &str = include_str!("../../resources/definitions/definitions_properties.json");
 
+#[allow(clippy::expect_used)] // PANIC-SAFE: compiled-in definitions file, validated by the test suite
 fn get_values<M: Map<String, SyntaxDefinition>>() -> M {
     let json: serde_json::Value = serde_json::from_str(DEFINITIONS_VALUES).expect("JSON was not well-formatted");
     parse_syntax_file(json)
 }
 
+#[allow(clippy::expect_used)] // PANIC-SAFE: compiled-in definitions file, validated by the test suite
 fn get_properties<M: Map<String, PropertyDefinition>>() -> M {
     let json: serde_json::Value = serde_json::from_str(DEFINITIONS_PROPERTIES).expect("JSON was not well-formatted");
     parse_property_file(json)
@@ -455,6 +459,7 @@ impl<K: Eq + Hash, V> Map<K, V> for indexmap::IndexMap<K, V> {
 }
 
 /// Parses a syntax JSON import file
+#[allow(clippy::unwrap_used)] // PANIC-SAFE: parses the compiled-in definitions; validated by the test suite
 fn parse_syntax_file<M: Map<String, SyntaxDefinition>>(json: serde_json::Value) -> M {
     let mut syntaxes = M::new();
 
@@ -517,6 +522,7 @@ fn parse_syntax_file<M: Map<String, SyntaxDefinition>>(json: serde_json::Value)
 }
 
 /// Parses the JSON input into a CSS property definitions structure
+#[allow(clippy::unwrap_used, clippy::panic)] // PANIC-SAFE: parses the compiled-in definitions; validated by the test suite
 fn parse_property_file<M: Map<String, PropertyDefinition>>(json: serde_json::Value) -> M {
     let mut properties = M::new();
 

crates/gosub_css3/src/matcher/shorthands.rs

@@ -575,9 +575,7 @@ impl CssDefinitions {
             }
         }
 
-        if syntax.components.len() == 1 {
-            let component = syntax.components.first().unwrap();
-
+        if let [component] = syntax.components.as_slice() {
             match component {
                 SyntaxComponent::Definition { datatype, .. } => {
                     if let Some(d) = self.syntax.get(datatype) {
@@ -731,8 +729,6 @@ mod tests {
                 current_info: None
             }
         );
-
-        dbg!(fix_list);
     }
 
     #[test]
@@ -752,21 +748,15 @@ mod tests {
             &mut fix_list,
         ));
 
-        dbg!(&fix_list);
-
         fix_list.resolve_nested(definitions);
 
-        dbg!(&fix_list);
-
         fix_list = FixList::new();
 
         assert!(prop.clone().matches_and_shorthands(
             &[str!("solid"), CssValue::Color(RgbColor::new(0.0, 0.0, 0.0, 0.0))],
             &mut fix_list,
         ));
 
-        dbg!(fix_list);
-
         fix_list = FixList::new();
 
         assert!(prop.clone().matches_and_shorthands(
@@ -777,7 +767,5 @@ mod tests {
             ],
             &mut fix_list,
         ));
-
-        dbg!(fix_list);
     }
 }

crates/gosub_css3/src/matcher/syntax.rs

@@ -284,7 +284,7 @@ fn parse_unit(input: &str) -> IResult<&str, SyntaxComponent> {
     let (input, value) = float(input)?;
     let (input, suffix) = opt(alpha1).parse(input)?;
 
-    if suffix.is_none() {
+    let Some(suffix) = suffix else {
         return if value == 0.0 {
             // 0 is a special case as it doesn't need a unit
             Ok((
@@ -297,12 +297,12 @@ fn parse_unit(input: &str) -> IResult<&str, SyntaxComponent> {
         } else {
             Err(Err::Error(nom::error::Error::new(input, nom::error::ErrorKind::Alpha)))
         };
-    }
+    };
 
     Ok((
         input,
         SyntaxComponent::Value {
-            value: CssValue::Unit(value, suffix.unwrap().to_string()),
+            value: CssValue::Unit(value, suffix.to_string()),
             multipliers: vec![SyntaxComponentMultiplier::Once],
         },
     ))
@@ -512,13 +512,11 @@ fn juxtaposition_or_separated_list(input: &str) -> IResult<&str, Vec<SyntaxCompo
         // Check for a separator
         let result = juxtaseparator(next_input);
 
-        // If errored, we return what we've got
-        if result.is_err() {
-            return Ok((next_input, elements));
-        }
-
+        // If errored, we return what we've got.
         // If found, the sep boolean determines if we are done or not.
-        let (next_input, sep) = result.unwrap();
+        let Ok((next_input, sep)) = result else {
+            return Ok((next_input, elements));
+        };
         if !sep {
             return Ok((next_input, elements));
         }
@@ -560,7 +558,7 @@ fn parse_unit_inner(input: &str) -> IResult<&str, SyntaxComponent> {
     let (input, _) = multispace0(input)?;
     let (input, suffixes) = opt(separated_list0(ws(tag("|")), alpha1)).parse(input)?;
 
-    if suffixes.is_none() {
+    let Some(suffixes) = suffixes else {
         // No suffixes, just a range
         return Ok((
             input,
@@ -571,10 +569,10 @@ fn parse_unit_inner(input: &str) -> IResult<&str, SyntaxComponent> {
                 multipliers: vec![SyntaxComponentMultiplier::Once],
             },
         ));
-    }
+    };
 
     // Convert the suffixes to a vector of strings
-    let suffixes: Vec<String> = suffixes.unwrap().iter().map(|s| (*s).to_string()).collect();
+    let suffixes: Vec<String> = suffixes.iter().map(|s| (*s).to_string()).collect();
     Ok((
         input,
         SyntaxComponent::Unit {