From a7957db8247da025dd383d74af0d382279468971 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcel=20M=C3=A4rtens?= Date: Wed, 13 Jan 2021 14:22:54 +0100 Subject: [PATCH 1/3] After some discussion with XVar, Song and Yakei we found out that `cargo clippy` is a superset of `cargo check`. There are multiple hints: - one guy in discord https://discord.gg/nWGhnbRb https://discord.com/channels/273534239310479360/335502067432947748/798886188923617290 - a old stackoverflow https://stackoverflow.com/questions/57449356/is-cargo-clippy-a-superset-of-cargo-check which lead us to the source code: https://github.com/rust-lang/rust-clippy/blob/7fa1d78c89f74c73d645901d6ee4728bcd6a72bf/src/main.rs#L73 which either uses `check` or `fix`. cargo fix is documented in the docs with (https://doc.rust-lang.org/cargo/commands/cargo-fix.html) "executing cargo fix will under the hood execute cargo-check(1)." - `cargo clippy` fails after running `cargo check` prob as there is nothing to do. - `cargo clippy --help` points us to `cargo check --help` Thus we are removing `cargo check` from the CI as a seperate check. However `cargo check --examples` did check the examples. In order to have them covered we are also running clippy now for examples, benches and all bins. Also we moved `--locked` from cargo check to clippy. --- .gitlab/CI/check.gitlab-ci.yml | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/.gitlab/CI/check.gitlab-ci.yml b/.gitlab/CI/check.gitlab-ci.yml index 247df426ff..6cd3dfc639 100644 --- a/.gitlab/CI/check.gitlab-ci.yml +++ b/.gitlab/CI/check.gitlab-ci.yml @@ -1,18 +1,10 @@ -check: - extends: .recompile-branch - stage: check - script: - - ln -s /dockercache/cache-all target - - cargo check --locked - - cargo check --examples --locked - +# cargo clippy is a superset of cargo check, so we don't check manually code-quality: extends: .recompile-branch stage: check script: - ln -s /dockercache/cache-all target - - cargo clippy -- -D warnings - - cargo clippy --tests -- -D warnings + - cargo clippy --tests --examples --benches --bins --locked -- -D warnings - cargo fmt --all -- --check security: From bda6390f1aa3be189c9f2e1c326369855dacc7d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcel=20M=C3=A4rtens?= Date: Wed, 13 Jan 2021 14:42:49 +0100 Subject: [PATCH 2/3] fix clippy in all examples. added a ignore for plugins, as we cannot remove the `Result<>` type, it is necessary --- network/examples/fileshare/server.rs | 1 + plugin/derive/src/lib.rs | 1 + world/examples/water.rs | 12 ++++-------- 3 files changed, 6 insertions(+), 8 deletions(-) diff --git a/network/examples/fileshare/server.rs b/network/examples/fileshare/server.rs index 33ceda100f..080b85fff5 100644 --- a/network/examples/fileshare/server.rs +++ b/network/examples/fileshare/server.rs @@ -117,6 +117,7 @@ impl Server { trace!("Stop connect_manager"); } + #[allow(clippy::eval_order_dependence)] async fn loop_participant(&self, p: Participant) { if let (Ok(cmd_out), Ok(file_out), Ok(cmd_in), Ok(file_in)) = ( p.open(15, Promises::ORDERED | Promises::CONSISTENCY).await, diff --git a/plugin/derive/src/lib.rs b/plugin/derive/src/lib.rs index 860c98179a..77ee375937 100644 --- a/plugin/derive/src/lib.rs +++ b/plugin/derive/src/lib.rs @@ -14,6 +14,7 @@ pub fn event_handler(_args: TokenStream, item: TokenStream) -> TokenStream { let fn_return = sig.output; // comma separated args let out: proc_macro2::TokenStream = quote! { + #[allow(clippy::unnecessary_wraps)] #[no_mangle] pub fn #fn_name(intern__ptr: i32, intern__len: u32) -> i32 { let input = ::veloren_plugin_rt::read_input(intern__ptr,intern__len).unwrap(); diff --git a/world/examples/water.rs b/world/examples/water.rs index 0434bc9eaa..0188148d6b 100644 --- a/world/examples/water.rs +++ b/world/examples/water.rs @@ -375,20 +375,16 @@ fn main() { if is_camera { focus.z += spd * scale; samples_changed = true; - } else { - if (scale * 2.0).is_normal() { - scale *= 2.0; - } + } else if (scale * 2.0).is_normal() { + scale *= 2.0; } } if win.is_key_down(minifb::Key::F) { if is_camera { focus.z -= spd * scale; samples_changed = true; - } else { - if (scale / 2.0).is_normal() { - scale /= 2.0; - } + } else if (scale / 2.0).is_normal() { + scale /= 2.0; } } From 37d4f7b62dff67caccd1d929a746e13beeb77104 Mon Sep 17 00:00:00 2001 From: Marcel Date: Thu, 14 Jan 2021 08:16:25 +0000 Subject: [PATCH 3/3] use `--all-targets` which is the same as before AND `--lib`. Just to be future proof, lib should not matter ATM, but might in the future when we produce libs that are not used in any binary target --- .gitlab/CI/check.gitlab-ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.gitlab/CI/check.gitlab-ci.yml b/.gitlab/CI/check.gitlab-ci.yml index 6cd3dfc639..0459d2ecc3 100644 --- a/.gitlab/CI/check.gitlab-ci.yml +++ b/.gitlab/CI/check.gitlab-ci.yml @@ -4,7 +4,7 @@ code-quality: stage: check script: - ln -s /dockercache/cache-all target - - cargo clippy --tests --examples --benches --bins --locked -- -D warnings + - cargo clippy --all-targets --locked -- -D warnings - cargo fmt --all -- --check security: