chore: enforce clippy in CI, fix all warnings

.github/workflows/test-crates.yml

@@ -31,6 +31,63 @@ jobs:
       - name: Check formatting
         run: cargo fmt --all -- --check
 
+  clippy:
+    name: cargo clippy
+    needs: [test]
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout repository
+        uses: actions/checkout@v4
+
+      - name: Free up disk space
+        run: |
+          sudo rm -rf /usr/share/dotnet
+          sudo rm -rf /usr/local/lib/android
+          sudo rm -rf /opt/ghc
+          sudo rm -rf /opt/hostedtoolcache/CodeQL
+          sudo docker image prune --all --force
+
+      - name: Install system dependencies
+        run: |
+          sudo apt-get update
+          sudo apt-get install -y \
+            libexpat1-dev \
+            libfontconfig1-dev \
+            libfreetype6-dev \
+            libgl1-mesa-dev \
+            libgles2-mesa-dev \
+            libwayland-dev \
+            libx11-dev \
+            libx11-xcb-dev \
+            libxcb-render0-dev \
+            libxcb-shape0-dev \
+            libxcb-xfixes0-dev \
+            libxcursor-dev \
+            libxi-dev \
+            libxinerama-dev \
+            libxrandr-dev \
+            libxxf86vm-dev \
+            mesa-common-dev
+
+      - name: Setup Rust toolchain
+        uses: dtolnay/rust-toolchain@stable
+        with:
+          toolchain: 1.92.0
+          components: clippy
+
+      - name: Cache cargo build outputs
+        uses: Swatinem/rust-cache@v2
+        with:
+          shared-key: test-native
+          cache-all-crates: true
+          cache-workspace-crates: true
+          cache-on-failure: true
+
+      - name: Run clippy
+        env:
+          FORCE_SKIA_BINARIES_DOWNLOAD: "1"
+        run: cargo clippy --no-deps --workspace --exclude grida-canvas-wasm -- -D warnings
+
   test:
     name: cargo test
     runs-on: ubuntu-latest

Cargo.toml

@@ -9,6 +9,10 @@ members = [
   "crates/math2",
 ]
 
+[workspace.lints.clippy]
+# Style choices for a graphics engine — intentionally allowed
+upper_case_acronyms = "allow" # AABB, RGB, HSL, SVG are domain terms
+
 # Reduce debug information for tests to lower memory usage during linking in CI
 # opt-level = 1 speeds up compilation while maintaining debuggability
 [profile.test]

clippy.toml

@@ -0,0 +1,5 @@
+# Clippy configuration for the Grida workspace
+# https://doc.rust-lang.org/clippy/lint_configuration.html
+
+# Graphics/rendering functions legitimately accept many parameters
+too-many-arguments-threshold = 12

crates/csscascade/Cargo.toml

@@ -35,3 +35,6 @@ markup5ever = "0.36.1"
 tendril = "0.4.3"
 atomic_refcell = "0.1.13"
 
+[lints]
+workspace = true
+

crates/csscascade/src/adapter.rs

@@ -191,7 +191,7 @@ impl HtmlElement {
         self.attr_iter()
             .filter(|(attr, _)| namespace_matches(ns, &attr.name.ns))
             .find(|(_, stored)| *stored == local_name)
-            .map_or(false, |(attr, _)| operation.eval_str(attr.value.as_ref()))
+            .is_some_and(|(attr, _)| operation.eval_str(attr.value.as_ref()))
     }
 
     fn lang_attribute_value(&self) -> Option<&str> {
@@ -815,7 +815,7 @@ fn namespace_matches(
 }
 
 fn atom_ident_str(atom: &AtomIdent) -> &str {
-    atom.as_ref().as_ref()
+    atom.as_ref()
 }
 
 fn sibling_pair(id: NodeId) -> (Option<HtmlNode>, Option<HtmlNode>) {

crates/csscascade/src/dom.rs

@@ -207,8 +207,7 @@ impl ElemNameTrait for OwnedElemName {
 
 impl DemoDomBuilder {
     fn new() -> Self {
-        let mut nodes = Vec::new();
-        nodes.push(NodeTemp::new(NodeDataTemp::Document));
+        let nodes = vec![NodeTemp::new(NodeDataTemp::Document)];
         Self {
             nodes: RefCell::new(nodes),
             document: NodeId(0),
@@ -317,7 +316,7 @@ impl DemoDomBuilder {
         let mut existing = existing.borrow_mut();
         let existing_names: Vec<_> = existing.iter().map(|attr| attr.name.clone()).collect();
         for attr in attrs {
-            if existing_names.iter().any(|name| *name == attr.name) {
+            if existing_names.contains(&attr.name) {
                 continue;
             }
             existing.push(attr);
@@ -468,12 +467,11 @@ impl TreeSink for DemoDomBuilder {
     }
 
     fn append(&self, parent: &Self::Handle, child: NodeOrText<Self::Handle>) {
-        if let NodeOrText::AppendText(ref text) = child {
-            if let Some(last) = self.last_child(*parent) {
-                if self.append_to_existing_text(last, text) {
-                    return;
-                }
-            }
+        if let NodeOrText::AppendText(ref text) = child
+            && let Some(last) = self.last_child(*parent)
+            && self.append_to_existing_text(last, text)
+        {
+            return;
         }
 
         let new_child = match child {

crates/csscascade/src/rcdom/mod.rs

@@ -140,10 +140,9 @@ impl Drop for Node {
                 ref template_contents,
                 ..
             } = node.data
+                && let Some(template_contents) = template_contents.borrow_mut().take()
             {
-                if let Some(template_contents) = template_contents.borrow_mut().take() {
-                    nodes.push(template_contents);
-                }
+                nodes.push(template_contents);
             }
         }
     }
@@ -300,12 +299,11 @@ impl TreeSink for RcDom {
 
     fn append(&self, parent: &Handle, child: NodeOrText<Handle>) {
         // Append to an existing Text node if we have one.
-        if let NodeOrText::AppendText(text) = &child {
-            if let Some(h) = parent.children.borrow().last() {
-                if append_to_existing_text(h, text) {
-                    return;
-                }
-            }
+        if let NodeOrText::AppendText(text) = &child
+            && let Some(h) = parent.children.borrow().last()
+            && append_to_existing_text(h, text)
+        {
+            return;
         }
 
         append(

crates/csscascade/src/tree/mod.rs

@@ -74,6 +74,7 @@ pub struct Tree {
 
 impl Tree {
     /// Creates a tree from the supplied root node.
+    #[allow(clippy::arc_with_non_send_sync)]
     fn new(root: StyledNode, runtime: Arc<StyleRuntime>) -> Self {
         Self {
             root: Arc::new(root),
@@ -89,6 +90,7 @@ impl Tree {
     ///
     /// NOTE: The current implementation only mirrors the DOM structure with stub
     /// styles; full cascade integration will replace this once the Stylo bridge is wired.
+    #[allow(clippy::should_implement_trait)]
     pub fn from_str(input: &str) -> Result<Self, TreeError> {
         let runtime = StyleRuntime::new()?;
         let opts = default_parse_opts();
@@ -125,11 +127,13 @@ impl Tree {
         let handle = styled_node_to_dom(&self.root, options);
         let serializable: SerializableHandle = handle.into();
         let mut buffer = Vec::new();
-        let mut serialize_opts = SerializeOpts::default();
-        serialize_opts.traversal_scope = if options.include_root() {
-            TraversalScope::IncludeNode
-        } else {
-            TraversalScope::ChildrenOnly(None)
+        let serialize_opts = SerializeOpts {
+            traversal_scope: if options.include_root() {
+                TraversalScope::IncludeNode
+            } else {
+                TraversalScope::ChildrenOnly(None)
+            },
+            ..Default::default()
         };
         serialize(&mut buffer, &serializable, serialize_opts).map_err(TreeError::Serialize)?;
         String::from_utf8(buffer).map_err(TreeError::Utf8)
@@ -194,7 +198,7 @@ impl StyledNode {
 }
 
 /// A thin, type-safe wrapper around node identifiers.
-#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Default)]
 pub struct NodeId(u64);
 
 impl NodeId {
@@ -203,12 +207,6 @@ impl NodeId {
     }
 }
 
-impl Default for NodeId {
-    fn default() -> Self {
-        Self(0)
-    }
-}
-
 /// Describes what kind of content a node holds.
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub enum NodeKind {
@@ -422,14 +420,14 @@ fn styled_node_to_dom(node: &StyledNode, options: &WriteOptions) -> Handle {
                 })
                 .collect();
 
-            if options.inline_styles() {
-                if let Some(serialized) = serialize_all_properties(&node.get_style()) {
-                    attrs_vec.retain(|attr| attr.name.local.as_ref() != "style");
-                    attrs_vec.push(HtmlAttribute {
-                        name: QualName::new(None, ns!(), LocalName::from("style")),
-                        value: StrTendril::from(serialized.as_str()),
-                    });
-                }
+            if options.inline_styles()
+                && let Some(serialized) = serialize_all_properties(&node.get_style())
+            {
+                attrs_vec.retain(|attr| attr.name.local.as_ref() != "style");
+                attrs_vec.push(HtmlAttribute {
+                    name: QualName::new(None, ns!(), LocalName::from("style")),
+                    value: StrTendril::from(serialized.as_str()),
+                });
             }
             let handle = Node::new(NodeData::Element {
                 name: qual,
@@ -517,6 +515,7 @@ impl StyleRuntime {
 
         let stylist = Stylist::new(device, quirks_mode);
 
+        #[allow(clippy::arc_with_non_send_sync)]
         Ok(Arc::new(Self {
             stylist,
             shared_lock,

crates/grida-canvas-fonts/Cargo.toml

@@ -31,3 +31,6 @@ serde_json = "1.0"
 [features]
 default = []
 serde = ["dep:serde"]
+
+[lints]
+workspace = true

crates/grida-canvas-fonts/src/parse.rs

@@ -372,7 +372,7 @@ fn parse_stat(face: &Face<'_>, data: &[u8]) -> StatData {
 
     let mut axes: Vec<StatAxis> = Vec::new();
     let mut tags: Vec<String> = Vec::new();
-    for record in table.axes.clone() {
+    for record in table.axes {
         let tag = tag_to_string(&record.tag.to_bytes());
         let name = lookup_name(face, record.name_id).unwrap_or_default();
         tags.push(tag.clone());

crates/grida-canvas-fonts/src/parse_feature/mode_full.rs

@@ -15,6 +15,12 @@ pub struct ComprehensiveFeatureParser {
     // No state needed for this parser
 }
 
+impl Default for ComprehensiveFeatureParser {
+    fn default() -> Self {
+        Self::new()
+    }
+}
+
 impl ComprehensiveFeatureParser {
     pub fn new() -> Self {
         Self {}
@@ -128,8 +134,8 @@ fn parse_gpos_features_with_context(
     // Check if any scripts have languages
     let mut has_language_systems = false;
     for i in 0..gpos_table.scripts.len() {
-        if let Some(script) = gpos_table.scripts.get(i as u16) {
-            if script.languages.len() > 0 {
+        if let Some(script) = gpos_table.scripts.get(i) {
+            if !script.languages.is_empty() {
                 has_language_systems = true;
                 break;
             }
@@ -139,7 +145,7 @@ fn parse_gpos_features_with_context(
     if has_language_systems {
         // Parse features through script/language hierarchy (like GSUB)
         for i in 0..gpos_table.scripts.len() {
-            if let Some(script) = gpos_table.scripts.get(i as u16) {
+            if let Some(script) = gpos_table.scripts.get(i) {
                 let script_tag = script.tag.to_string();
 
                 for language in script.languages {
@@ -184,7 +190,7 @@ fn parse_gpos_features_with_context(
     } else {
         // No language systems - parse features directly (like Inter font)
         for i in 0..gpos_table.features.len() {
-            if let Some(feature) = gpos_table.features.get(i as u16) {
+            if let Some(feature) = gpos_table.features.get(i) {
                 let tag = feature.tag.to_string();
                 let lookup_indices: Vec<u16> = feature.lookup_indices.into_iter().collect();
 
@@ -334,7 +340,7 @@ fn analyze_gpos_feature(
 /// This function processes OpenType coverage tables to extract glyph IDs
 /// and convert them to Unicode characters for feature analysis.
 fn extract_coverage_glyphs(coverage: &Coverage, glyph_set: &mut HashSet<String>) {
-    let glyph_ids = coverage_glyphs(coverage.clone());
+    let glyph_ids = coverage_glyphs(*coverage);
     for glyph_id in glyph_ids {
         glyph_set.insert(glyph_id.to_string());
     }

crates/grida-canvas-fonts/src/parse_feature/mode_simple.rs

@@ -12,6 +12,12 @@ pub struct BuiltinFeatureParser {
     // No state needed for this parser
 }
 
+impl Default for BuiltinFeatureParser {
+    fn default() -> Self {
+        Self::new()
+    }
+}
+
 impl BuiltinFeatureParser {
     pub fn new() -> Self {
         Self {}
@@ -70,7 +76,7 @@ fn parse_gsub_features_builtin(
 
     // Iterate through features directly (similar to our chained approach)
     for i in 0..gsub_table.features.len() {
-        if let Some(feature) = gsub_table.features.get(i as u16) {
+        if let Some(feature) = gsub_table.features.get(i) {
             let tag = feature.tag.to_string();
             let name = utils::get_feature_name_from_name_table(
                 face,
@@ -172,7 +178,7 @@ fn parse_gpos_features_builtin(
 
     // Iterate through features directly
     for i in 0..gpos_table.features.len() {
-        if let Some(feature) = gpos_table.features.get(i as u16) {
+        if let Some(feature) = gpos_table.features.get(i) {
             let tag = feature.tag.to_string();
             let name = utils::get_feature_name_from_name_table(
                 face,

crates/grida-canvas-fonts/src/parse_feature_params.rs

@@ -247,7 +247,7 @@ fn lookup_name_by_id(font_data: &[u8], name_id: u16) -> Option<String> {
 
 /// Decodes UTF-16 BE string data
 fn decode_utf16_be(data: &[u8]) -> Option<String> {
-    if data.len() % 2 != 0 {
+    if !data.len().is_multiple_of(2) {
         return None;
     }