Run clippy for wasm, with own clippy.toml config file (#1628)

* Run clippy for wasm, with own clippy.toml config file

* Use --profile default in order to install clippy

* Detect errors in ci_docker/publish.sh

* Make sure clippy is installed on CI

* install clippy in a nicer way

* Name the CI step better

* Fix clippy warnings

* Fix clippy warning that for some reason only happens on wasm!??!?!

* Figure ouit what cargo version is being run

* Update rust to 1.67.1

Small change with how the lint `clippy::uninlined_format_args` acts

* Forbid clippy::uninlined_format_args

* Use exactly 1.67.1 everywhere, since it matters for clippy

* Forbid threads on wasm

* Document clippy.toml a bit better

* Cleanup

* Use CLIPPY_CONF_DIR

.github/workflows/rust.yml

@@ -209,8 +209,8 @@ jobs:
 
       - uses: actions-rs/toolchain@v1
         with:
-          profile: minimal
-          toolchain: 1.67.0
+          profile: default
+          toolchain: 1.67.1
           target: wasm32-unknown-unknown
           override: true
 
@@ -221,11 +221,8 @@ jobs:
           # See: https://github.com/rerun-io/rerun/pull/497
           save-if: ${{ github.event_name == 'push'}}
 
-      - name: Check re_viewer wasm32
-        uses: actions-rs/cargo@v1
-        with:
-          command: check
-          args: --locked --all-features --lib --target wasm32-unknown-unknown --target-dir target_wasm -p re_viewer
+      - name: clippy check re_viewer wasm32
+        run: ./scripts/clippy_wasm.sh
 
       - name: Check re_renderer examples wasm32
         uses: actions-rs/cargo@v1

.github/workflows/toml.yml

@@ -20,7 +20,7 @@ jobs:
       - uses: actions-rs/toolchain@v1
         with:
           profile: minimal
-          toolchain: 1.67.0
+          toolchain: 1.67.1
           override: true
 
       - name: Set up cargo cache

BUILD.md

@@ -15,15 +15,15 @@ This is a guide to how to build Rerun.
 * Install the Rust toolchain: <https://rustup.rs/>
 * `git clone [email protected]:rerun-io/rerun.git && cd rerun`
 * Run `./scripts/setup_dev.sh`.
-* Make sure `cargo --version` prints `1.67.0` once you are done
+* Make sure `cargo --version` prints `1.67.1` once you are done
 
 
 ### Apple-silicon Macs
 
 If you are using an Apple-silicon Mac (M1, M2), make sure `rustc -vV` outputs `host: aarch64-apple-darwin`. If not, this should fix it:
 
 ```sh
-rustup set default-host aarch64-apple-darwin && rustup install 1.67
+rustup set default-host aarch64-apple-darwin && rustup install 1.67.1
 ```
 
 ## Building the docs

Cranky.toml

@@ -104,6 +104,7 @@ warn = [
   "clippy::unchecked_duration_subtraction",
   "clippy::undocumented_unsafe_blocks",
   "clippy::unimplemented",
+  "clippy::uninlined_format_args",
   "clippy::unnecessary_safety_doc",
   "clippy::unnecessary_wraps",
   "clippy::unnested_or_patterns",

ci_docker/Dockerfile

@@ -36,7 +36,7 @@ RUN curl -L https://github.com/NixOS/patchelf/releases/download/0.17.2/patchelf-
 ENV RUSTUP_HOME=/usr/local/rustup \
     CARGO_HOME=/usr/local/cargo \
     PATH=/usr/local/cargo/bin:$PATH \
-    RUST_VERSION=1.67 \
+    RUST_VERSION=1.67.1 \
     RUSTUP_VERSION=1.25.2
 
 # Install Rust
@@ -48,7 +48,7 @@ RUN set -eux; \
     wget "$url"; \
     echo "${rustupSha256} *rustup-init" | sha256sum -c -; \
     chmod +x rustup-init; \
-    ./rustup-init -y --no-modify-path --profile minimal --default-toolchain $RUST_VERSION --default-host ${rustArch}; \
+    ./rustup-init -y --no-modify-path --profile default --default-toolchain $RUST_VERSION --default-host ${rustArch}; \
     rm rustup-init; \
     chmod -R a+w $RUSTUP_HOME $CARGO_HOME; \
     rustup --version; \

ci_docker/publish.sh

@@ -1,3 +1,6 @@
+#!/usr/bin/env bash
+set -eu
+
 VERSION=0.6 # Bump on each new version. Remember to update the version in the Dockerfile too.
 
 # The build needs to run from top of repo to access the requirements.txt

clippy.toml

@@ -1,3 +1,5 @@
+# There is also a clippy_wasm/clippy.toml which forbids some mthods that are not available in wasm.
+
 msrv = "1.67"
 
 
@@ -18,10 +20,9 @@ disallowed-macros = [
 disallowed-methods = [
   "std::env::temp_dir", # Use the tempdir crate instead
 
-  # Disabled because of https://github.com/rust-lang/rust-clippy/issues/10406
-  #   "std::time::Instant::now",      # use `instant` crate instead for wasm/web compatibility
-  "std::time::Duration::elapsed", # use `instant` crate instead for wasm/web compatibility
-  "std::time::SystemTime::now",   # use `instant` or `time` crates instead for wasm/web compatibility
+  # There are many things that aren't allowed on wasm,
+  # but we cannot disable them all here (because of e.g. https://github.com/rust-lang/rust-clippy/issues/10406)
+  # so we do that in `clipppy_wasm.toml` instead.
 
   "std::thread::spawn", # Use `std::thread::Builder` and name the thread
 
@@ -44,6 +45,7 @@ disallowed-types = [
 
 # Allow-list of words for markdown in dosctrings https://rust-lang.github.io/rust-clippy/master/index.html#doc_markdown
 doc-valid-idents = [
+  # You must also update the same list in `clippy_wasm/clippy.toml`!
   "GitHub",
   "GLB",
   "GLTF",

clippy_wasm/clippy.toml

@@ -0,0 +1,40 @@
+# This is used by `scripts/clippy_wasm.sh` so we can forbid some methods that are not available in wasm.
+#
+# We cannot forbid all these methods in the main `clippy.toml` because of
+# https://github.com/rust-lang/rust-clippy/issues/10406
+
+msrv = "1.67"
+
+# https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_methods
+disallowed-methods = [
+  "std::time::Instant::now",      # use `instant` crate instead for wasm/web compatibility
+  "std::time::Duration::elapsed", # use `instant` crate instead for wasm/web compatibility
+  "std::time::SystemTime::now",   # use `instant` or `time` crates instead for wasm/web compatibility
+
+  # Cannot spawn threads on wasm:
+  "std::thread::spawn",
+]
+
+# https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_types
+disallowed-types = [
+  # Cannot spawn threads on wasm:
+  "std::thread::Builder",
+]
+
+# Allow-list of words for markdown in dosctrings https://rust-lang.github.io/rust-clippy/master/index.html#doc_markdown
+doc-valid-idents = [
+  # You must also update the same list in the root `clippy.toml`!
+  "GitHub",
+  "GLB",
+  "GLTF",
+  "iOS",
+  "macOS",
+  "NaN",
+  "OBJ",
+  "PyPI",
+  "sRGB",
+  "sRGBA",
+  "WebGL",
+  "WebSockets",
+  "..",
+]

crates/re_arrow_store/src/store_format.rs

@@ -231,7 +231,7 @@ impl std::fmt::Display for IndexBucket {
                 "time range: N/A\n".to_owned()
             }
         };
-        f.write_fmt(format_args!("{}\n", time_range))?;
+        f.write_fmt(format_args!("{time_range}\n"))?;
 
         let (timeline_name, times) = self.times();
         let (col_names, cols): (Vec<_>, Vec<_>) = {

crates/re_log/src/setup.rs

@@ -51,5 +51,5 @@ pub fn setup_web_logging() {
     log::set_max_level(log::LevelFilter::Debug);
     crate::add_boxed_logger(Box::new(crate::web_logger::WebLogger::new(
         log::LevelFilter::Debug,
-    )))
+    )));
 }

crates/re_viewer/src/stream_rrd_from_http.rs

@@ -61,7 +61,7 @@ fn decode_rrd(rrd_bytes: Vec<u8>, on_msg: Box<dyn Fn(re_log_types::LogMsg) + Sen
 #[cfg(target_arch = "wasm32")]
 mod web_decode {
     pub fn decode_rrd(rrd_bytes: Vec<u8>, on_msg: Box<dyn Fn(re_log_types::LogMsg) + Send>) {
-        wasm_bindgen_futures::spawn_local(decode_rrd_async(rrd_bytes, on_msg))
+        wasm_bindgen_futures::spawn_local(decode_rrd_async(rrd_bytes, on_msg));
     }
 
     /// Decodes the file in chunks, with an yield between each chunk.

crates/re_viewer/src/ui/data_ui/image.rs

@@ -96,6 +96,7 @@ impl DataUi for Tensor {
                         }
                     }
 
+                    #[allow(clippy::collapsible_match)] // false positive on wasm32
                     if let Some(dynamic_img) = tensor_view.dynamic_img() {
                         // TODO(emilk): support copying and saving images on web
                         #[cfg(not(target_arch = "wasm32"))]
@@ -342,10 +343,10 @@ pub fn show_zoomed_image_region(
                 let tensor = tensor_view.tensor;
 
                 let text = match tensor.num_dim() {
-                    2 => tensor.get(&[x, y]).map(|v| format!("Val: {}", v)),
+                    2 => tensor.get(&[x, y]).map(|v| format!("Val: {v}")),
                     3 => match tensor.shape()[2].size {
                         0 => Some("Cannot preview 0-size channel".to_owned()),
-                        1 => tensor.get(&[x, y, 0]).map(|v| format!("Val: {}", v)),
+                        1 => tensor.get(&[x, y, 0]).map(|v| format!("Val: {v}")),
                         3 => {
                             // TODO(jleibs): Track RGB ordering somehow -- don't just assume it
                             if let (Some(r), Some(g), Some(b)) = (
@@ -395,11 +396,11 @@ pub fn show_zoomed_image_region(
                             }
                         },
                         channels => {
-                            Some(format!("Cannot preview {}-channel image", channels))
+                            Some(format!("Cannot preview {channels}-channel image"))
                         }
                     },
                     dims => {
-                        Some(format!("Cannot preview {}-dimensional image", dims))
+                        Some(format!("Cannot preview {dims}-dimensional image"))
                     }
                 };
 

rerun_py/README.md

@@ -61,7 +61,7 @@ Setup:
 * Install the Rust toolchain: <https://rustup.rs/>
 * `git clone [email protected]:rerun-io/rerun.git && cd rerun`
 * Run `./scripts/setup_dev.sh`.
-* Make sure `cargo --version` prints `1.67.0` once you are done
+* Make sure `cargo --version` prints `1.67.1` once you are done
 
 ## Building
 To build from source and install Rerun into your *current* Python environment run:

rust-toolchain

@@ -5,6 +5,6 @@
 # to the user in the error, instead of "error: invalid channel name '[toolchain]'".
 
 [toolchain]
-channel = "1.67"
+channel = "1.67.1"
 components = ["rustfmt", "clippy"]
 targets = ["wasm32-unknown-unknown"]

scripts/clippy_wasm.sh

@@ -0,0 +1,13 @@
+#!/usr/bin/env bash
+# This scripts run clippy on the wasm32-unknown-unknown target,
+# using a special clippy.toml config file which forbids a few more things.
+
+set -eu
+script_path=$( cd "$(dirname "${BASH_SOURCE[0]}")" ; pwd -P )
+cd "$script_path/.."
+set -x
+
+# Use clippy_wasm/clippy.toml
+export CLIPPY_CONF_DIR="clippy_wasm"
+
+cargo cranky --all-features --target wasm32-unknown-unknown --target-dir target_wasm -p re_viewer -- --deny warnings

scripts/setup.sh

@@ -32,7 +32,7 @@ elif [ -x "$(command -v dnf)" ];   then
 fi
 
 # Needed to compile and check the code:
-rustup install 1.67.0
+rustup install 1.67.1
 ./scripts/setup_web.sh
 
 echo "setup.sh completed!"