Improved CI Actions

.github/workflows/ci.yml

@@ -9,57 +9,66 @@ on:
 env:
   RUSTFLAGS: -Dwarnings
   RUST_BACKTRACE: 1
+  CARGO_INCREMENTAL: 0 # makes cache smaller
+  CARGO_PROFILE_DEV_DEBUG: 0
 
 jobs:
   clippy:
-    name: clippy
+    name: Clipy and format
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v4
         with:
           submodules: 'recursive'
-      - name: Install Rust
-        run: rustup update stable && rustup default stable
-      - name: Get rust version
-        id: rust-version
-        run: echo "::set-output name=version::$(rustc --version)"
-      - name: Cache cargo index and registry
+      - uses: dtolnay/rust-toolchain@stable
+        with:
+          components: clippy, rustfmt
+      - name: Check format
+        run: cargo fmt -- --check
+      - name: Cache cargo registry
         uses: actions/cache@v4
         with:
-          key: immutable # this will always be hit because it's a constant
+          key: index-${{ hashFiles('Cargo.toml') }}
           path: |
-            ~/.cargo/registry/cache
             ~/.cargo/registry/index
-      - name: Create lockfile
-        run: cargo generate-lockfile
+            ~/.cargo/registry/cache
       - name: Fetch dependencies
         run: cargo fetch
       - name: Cache target directory
         uses: actions/cache@v4
         with:
+          key: clippy-${{ hashFiles('Cargo.lock') }}
           path: target
-          key: clippy-target-${{ runner.os }}-${{ steps.rust-version.outputs.version }}-${{ hashFiles('Cargo.lock') }}
       - name: Run clippy
-        run: cargo clippy --all --all-targets
+        run: cargo clippy --all --all-targets --target-dir=target
+      - name: Run clippy on integration tests
+        run: cargo clippy --all --all-targets --features=integration_test --target-dir=target
+      - name: Run clippy on C API
+        run: cargo clippy --all-targets --manifest-path=c-api/Cargo.toml --target-dir=target
+      - name: Run clippy on JS API
+        run: cargo clippy --all-targets --manifest-path=js-api/Cargo.toml --target-dir=target
+
   test:
     name: Test
     runs-on: ubuntu-latest
-    steps:
-    - uses: actions/checkout@v4
-    - name: Install Rust (rustup)
-      run: rustup update stable --no-self-update && rustup default stable
-      shell: bash
-    - name: Run tests
-      run: scripts/test.sh
-  format:
-    name: Check code format
-    runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v4
-      - name: Install Rust (rustup)
-        run: |
-          rustup update stable --no-self-update && rustup default stable
-          rustup component add rustfmt
-        shell: bash
-      - name: Check format
-        run: cargo fmt -- --check
+        with:
+          submodules: 'recursive'
+      - uses: dtolnay/rust-toolchain@stable
+      - name: Cache cargo registry
+        uses: actions/cache@v4
+        with:
+          key: index-${{ hashFiles('Cargo.toml') }}
+          path: |
+            ~/.cargo/registry/index
+            ~/.cargo/registry/cache
+      - name: Fetch dependencies
+        run: cargo fetch
+      - name: Cache target directory
+        uses: actions/cache@v4
+        with:
+          key: target-${{ hashFiles('Cargo.lock') }}
+          path: target
+      - name: Run Rust tests
+        run: scripts/test.sh

c-api/src/lib.rs

@@ -1,3 +1,5 @@
+#![allow(clippy::missing_safety_doc)]
+
 pub use crate::streaming::CStreamingHandler;
 use libc::{c_char, c_int, c_void, size_t};
 use lol_html::html_content::*;

c-api/src/rewriter_builder.rs

@@ -58,6 +58,7 @@ pub struct ExternDocumentContentHandlers {
 }
 
 impl ExternDocumentContentHandlers {
+    #[must_use]
     pub fn as_safe_document_content_handlers(&self) -> DocumentContentHandlers {
         let mut handlers = DocumentContentHandlers::default();
 
@@ -77,6 +78,7 @@ pub struct ExternElementContentHandlers {
 }
 
 impl ExternElementContentHandlers {
+    #[must_use]
     pub fn as_safe_element_content_handlers(&self) -> ElementContentHandlers {
         let mut handlers = ElementContentHandlers::default();
 
@@ -100,6 +102,7 @@ pub struct HtmlRewriterBuilder {
 }
 
 impl HtmlRewriterBuilder {
+    #[must_use]
     pub fn get_safe_handlers(&self) -> SafeContentHandlers {
         SafeContentHandlers {
             document: self

c-api/src/streaming.rs

@@ -59,6 +59,7 @@ pub unsafe extern "C" fn lol_html_streaming_sink_write_utf8_chunk(
 }
 
 /// Safety: the user data and the callbacks must be safe to use from a different thread (e.g. can't rely on thread-local storage).
+///
 /// It doesn't have to be `Sync`, it will be used only by one thread at a time.
 ///
 /// Handler functions copy this struct. It can (and should) be created on the stack.

c-api/src/string.rs

@@ -35,7 +35,7 @@ impl Str {
 
 impl Drop for Str {
     fn drop(&mut self) {
-        if self.data == ptr::null() {
+        if self.data.is_null() {
             return;
         }
         let bytes = unsafe { slice::from_raw_parts_mut(self.data.cast_mut(), self.len) };

js-api/src/comment.rs

@@ -9,7 +9,7 @@ impl_mutations!(Comment);
 
 #[wasm_bindgen]
 impl Comment {
-    #[wasm_bindgen(method, getter)]
+    #[wasm_bindgen(getter)]
     pub fn text(&self) -> JsResult<String> {
         self.0.get().map(|c| c.text())
     }

js-api/src/doctype.rs

@@ -8,17 +8,17 @@ impl_from_native!(NativeDoctype --> Doctype);
 
 #[wasm_bindgen]
 impl Doctype {
-    #[wasm_bindgen(method, getter)]
+    #[wasm_bindgen(getter)]
     pub fn name(&self) -> JsResult<Option<String>> {
         self.0.get().map(|d| d.name())
     }
 
-    #[wasm_bindgen(method, getter=publicId)]
+    #[wasm_bindgen(getter=publicId)]
     pub fn public_id(&self) -> JsResult<Option<String>> {
         self.0.get().map(|d| d.public_id())
     }
 
-    #[wasm_bindgen(method, getter=systemId)]
+    #[wasm_bindgen(getter=systemId)]
     pub fn system_id(&self) -> JsResult<Option<String>> {
         self.0.get().map(|d| d.system_id())
     }

js-api/src/element.rs

@@ -26,22 +26,22 @@ impl_mutations!(Element);
 
 #[wasm_bindgen]
 impl Element {
-    #[wasm_bindgen(method, getter=tagName)]
+    #[wasm_bindgen(getter=tagName)]
     pub fn tag_name(&self) -> JsResult<String> {
         self.0.get().map(|e| e.tag_name())
     }
 
-    #[wasm_bindgen(method, setter=tagName)]
+    #[wasm_bindgen(setter=tagName)]
     pub fn set_tag_name(&mut self, name: &str) -> JsResult<()> {
         self.0.get_mut()?.set_tag_name(name).into_js_result()
     }
 
-    #[wasm_bindgen(method, getter=namespaceURI)]
+    #[wasm_bindgen(getter=namespaceURI)]
     pub fn namespace_uri(&self) -> JsResult<JsValue> {
         self.0.get().map(|e| e.namespace_uri().into())
     }
 
-    #[wasm_bindgen(method, getter)]
+    #[wasm_bindgen(getter)]
     pub fn attributes(&self) -> JsResult<JsValue> {
         self.0
             .get()
@@ -54,25 +54,25 @@ impl Element {
             .and_then(|a| to_js_value(&a).into_js_result())
     }
 
-    #[wasm_bindgen(method, js_name=getAttribute)]
+    #[wasm_bindgen(js_name=getAttribute)]
     pub fn get_attribute(&self, name: &str) -> JsResult<Option<String>> {
         self.0.get().map(|e| e.get_attribute(name))
     }
 
-    #[wasm_bindgen(method, js_name=hasAttribute)]
+    #[wasm_bindgen(js_name=hasAttribute)]
     pub fn has_attribute(&self, name: &str) -> JsResult<bool> {
         self.0.get().map(|e| e.has_attribute(name))
     }
 
-    #[wasm_bindgen(method, js_name=setAttribute)]
+    #[wasm_bindgen(js_name=setAttribute)]
     pub fn set_attribute(&mut self, name: &str, value: &str) -> JsResult<()> {
         self.0
             .get_mut()?
             .set_attribute(name, value)
             .into_js_result()
     }
 
-    #[wasm_bindgen(method, js_name=removeAttribute)]
+    #[wasm_bindgen(js_name=removeAttribute)]
     pub fn remove_attribute(&mut self, name: &str) -> JsResult<()> {
         self.0.get_mut().map(|e| e.remove_attribute(name))
     }
@@ -97,7 +97,7 @@ impl Element {
             .map(|e| e.append(content, content_type.into_native()))
     }
 
-    #[wasm_bindgen(method, js_name=setInnerContent)]
+    #[wasm_bindgen(js_name=setInnerContent)]
     pub fn set_inner_content(
         &mut self,
         content: &str,
@@ -108,7 +108,7 @@ impl Element {
             .map(|e| e.set_inner_content(content, content_type.into_native()))
     }
 
-    #[wasm_bindgen(method, js_name=removeAndKeepContent)]
+    #[wasm_bindgen(js_name=removeAndKeepContent)]
     pub fn remove_and_keep_content(&mut self) -> Result<(), JsValue> {
         self.0.get_mut().map(|e| e.remove_and_keep_content())
     }

js-api/src/handlers.rs

@@ -22,9 +22,11 @@ unsafe impl Sync for HandlerJsErrorWrap {}
 
 macro_rules! make_handler {
     ($handler:ident, $JsArgType:ident, $typehint:ty) => {{
-        fn type_hint(h: $typehint) -> $typehint { h }
+        fn type_hint(h: $typehint) -> $typehint {
+            h
+        }
         type_hint(Box::new(move |arg: &mut _| {
-            let (js_arg, anchor) = $JsArgType::from_native(arg);
+            let (js_arg, anchor) = unsafe { $JsArgType::from_native(arg) };
 
             let res = match $handler.call1(&JsValue::NULL, &JsValue::from(js_arg)) {
                 Ok(_) => Ok(()),

js-api/src/html_rewriter.rs

@@ -49,12 +49,12 @@ pub struct HTMLRewriter(RewriterState);
 #[wasm_bindgen]
 impl HTMLRewriter {
     #[wasm_bindgen(constructor)]
-    pub fn new(encoding: String, output_sink: &JsFunction) -> JsResult<HTMLRewriter> {
+    pub fn new(encoding: String, output_sink: &JsFunction) -> JsResult<Self> {
         let encoding = Encoding::for_label(encoding.as_bytes())
             .and_then(AsciiCompatibleEncoding::new)
             .ok_or_else(|| JsError::new("Invalid encoding"))?;
 
-        Ok(HTMLRewriter(RewriterState::Before {
+        Ok(Self(RewriterState::Before {
             output_sink: JsOutputSink::new(output_sink),
             settings: Settings {
                 encoding,