Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 16 commits into from
Mar 21, 2023
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 4 additions & 7 deletions .github/workflows/rust.yml
Original file line number Diff line number Diff line change
Expand Up @@ -202,8 +202,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

Expand All @@ -214,11 +214,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
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/toml.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions BUILD.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions Cranky.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
4 changes: 2 additions & 2 deletions ci_docker/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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; \
Expand Down
3 changes: 3 additions & 0 deletions ci_docker/publish.sh
Original file line number Diff line number Diff line change
@@ -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
Expand Down
10 changes: 6 additions & 4 deletions clippy.toml
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# There is also a clippy_wasm.toml which forbids some mthods that are not available in wasm.

msrv = "1.67"


Expand All @@ -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

Expand All @@ -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.toml`!
"GitHub",
"GLB",
"GLTF",
Expand Down
40 changes: 40 additions & 0 deletions clippy_wasm.toml
Original file line number Diff line number Diff line change
@@ -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 `clippy.toml`!
"GitHub",
"GLB",
"GLTF",
"iOS",
"macOS",
"NaN",
"OBJ",
"PyPI",
"sRGB",
"sRGBA",
"WebGL",
"WebSockets",
"..",
]
2 changes: 1 addition & 1 deletion crates/re_arrow_store/src/store_format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<_>) = {
Expand Down
2 changes: 1 addition & 1 deletion crates/re_log/src/setup.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)))
)));
}
2 changes: 1 addition & 1 deletion crates/re_viewer/src/stream_rrd_from_http.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
9 changes: 5 additions & 4 deletions crates/re_viewer/src/ui/data_ui/image.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"))]
Expand Down Expand Up @@ -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)) = (
Expand Down Expand Up @@ -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"))
}
};

Expand Down
2 changes: 1 addition & 1 deletion rerun_py/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion rust-toolchain
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
20 changes: 20 additions & 0 deletions scripts/clippy_wasm.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
#!/usr/bin/env bash
# This scripts run clippy on the wasm32-unknown-unknown target,
# using a special clippy_wasm.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

mv clippy.toml clippy.toml.bak
cp clippy_wasm.toml clippy.toml

function cleanup()
{
mv clippy.toml.bak clippy.toml
}

trap cleanup EXIT
emilk marked this conversation as resolved.
Show resolved Hide resolved

cargo cranky --all-features --target wasm32-unknown-unknown --target-dir target_wasm -p re_viewer -- --deny warnings
2 changes: 1 addition & 1 deletion scripts/setup.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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!"