From 5c3ffbef7d132990fa4c8726f55afd156b0c6978 Mon Sep 17 00:00:00 2001 From: Taiki Endo Date: Tue, 31 Oct 2023 03:59:53 +0900 Subject: [PATCH] Use [lints] in Cargo.toml and apply more lints --- .clippy.toml | 8 ++++++ Cargo.toml | 50 +++++++++++++++++++++++++++++++----- bench/Cargo.toml | 3 +++ bench/vs_boxed.rs | 2 -- src/derive/mod.rs | 3 +-- src/lib.rs | 2 -- src/utils.rs | 2 ++ tests/auto_enum.rs | 19 ++++++-------- tests/compiletest.rs | 7 ------ tests/enum_derive.rs | 1 - tests/expandtest.rs | 12 +-------- tests/type_analysis.rs | 1 - tools/tidy.sh | 57 +++++++++++++++++++++++++++++++----------- 13 files changed, 109 insertions(+), 58 deletions(-) diff --git a/.clippy.toml b/.clippy.toml index 85a79aaf..27a1e091 100644 --- a/.clippy.toml +++ b/.clippy.toml @@ -3,3 +3,11 @@ avoid-breaking-exported-api = false disallowed-names = [] +disallowed-macros = [ + { path = "std::dbg", reason = "it is okay to use during development, but please do not include it in main branch" }, +] +disallowed-methods = [ + { path = "std::env::remove_var", reason = "this is not thread-safe and inherently unsafe; see for more" }, + { path = "std::env::set_var", reason = "this is not thread-safe and inherently unsafe; see for more" }, +] +disallowed-types = [] diff --git a/Cargo.toml b/Cargo.toml index d224fa8d..177954d2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -16,10 +16,6 @@ A library for to allow multiple return types by automatically generated enum. all-features = true targets = ["x86_64-unknown-linux-gnu"] -[workspace] -resolver = "2" -members = ["bench"] - [lib] proc-macro = true @@ -89,9 +85,9 @@ quote = "1" syn = { version = "2.0.6", features = ["full", "visit-mut"] } [dev-dependencies] -macrotest = { git = "https://github.com/taiki-e/macrotest.git", branch = "dev" } # 2021 edition support + syn 2.0 + no cargo-expand +macrotest = { git = "https://github.com/taiki-e/macrotest.git", branch = "dev" } # 2021 edition support + syn 2.0 + no cargo-expand + adjust overwrite behavior rustversion = "1" -trybuild = "1.0.49" +trybuild = { git = "https://github.com/taiki-e/trybuild.git", branch = "dev" } # adjust overwrite behavior # for `#[enum_derive]` futures03_crate = { package = "futures", version = "0.3", default-features = false, features = ["std"] } @@ -102,3 +98,45 @@ tokio02_crate = { package = "tokio", version = "0.2", default-features = false } tokio01_crate = { package = "tokio", version = "0.1", default-features = false, features = ["io"] } rayon_crate = { package = "rayon", version = "1" } serde_crate = { package = "serde", version = "1" } + +[lints] +workspace = true + +[workspace] +resolver = "2" +members = ["bench"] + +# This table is shared by projects under https://github.com/taiki-e. +# It is not intended for manual editing. +[workspace.lints.rust] +improper_ctypes = "warn" +improper_ctypes_definitions = "warn" +non_ascii_idents = "warn" +rust_2018_idioms = "warn" +single_use_lifetimes = "warn" +unreachable_pub = "warn" +# unsafe_op_in_unsafe_fn = "warn" # Set at crate-level instead since https://github.com/rust-lang/rust/pull/100081 is not available on MSRV +[workspace.lints.clippy] +all = "warn" # Downgrade deny-by-default lints +pedantic = "warn" +as_ptr_cast_mut = "warn" +default_union_representation = "warn" +inline_asm_x86_att_syntax = "warn" +trailing_empty_array = "warn" +transmute_undefined_repr = "warn" +undocumented_unsafe_blocks = "warn" +# Suppress buggy or noisy clippy lints +borrow_as_ptr = { level = "allow", priority = 1 } # https://github.com/rust-lang/rust-clippy/issues/8286 +doc_markdown = { level = "allow", priority = 1 } +float_cmp = { level = "allow", priority = 1 } # https://github.com/rust-lang/rust-clippy/issues/7725 +manual_assert = { level = "allow", priority = 1 } +manual_range_contains = { level = "allow", priority = 1 } # https://github.com/rust-lang/rust-clippy/issues/6455#issuecomment-1225966395 +missing_errors_doc = { level = "allow", priority = 1 } +module_name_repetitions = { level = "allow", priority = 1 } +similar_names = { level = "allow", priority = 1 } +single_match = { level = "allow", priority = 1 } +single_match_else = { level = "allow", priority = 1 } +struct_excessive_bools = { level = "allow", priority = 1 } +too_many_arguments = { level = "allow", priority = 1 } +too_many_lines = { level = "allow", priority = 1 } +type_complexity = { level = "allow", priority = 1 } diff --git a/bench/Cargo.toml b/bench/Cargo.toml index f7994610..beea7a83 100644 --- a/bench/Cargo.toml +++ b/bench/Cargo.toml @@ -14,3 +14,6 @@ rand = "0.8" name = "vs_boxed" path = "vs_boxed.rs" harness = false + +[lints] +workspace = true diff --git a/bench/vs_boxed.rs b/bench/vs_boxed.rs index 9689b854..b955ccb3 100644 --- a/bench/vs_boxed.rs +++ b/bench/vs_boxed.rs @@ -224,8 +224,6 @@ Found 5 outliers among 100 measurements (5.00%) */ -#![warn(rust_2018_idioms, single_use_lifetimes)] - use std::hint::black_box; use auto_enums::auto_enum; diff --git a/src/derive/mod.rs b/src/derive/mod.rs index e5272371..11918187 100644 --- a/src/derive/mod.rs +++ b/src/derive/mod.rs @@ -1,7 +1,6 @@ // SPDX-License-Identifier: Apache-2.0 OR MIT -#![allow(clippy::unnecessary_wraps)] -#![allow(clippy::wildcard_imports)] +#![allow(clippy::unnecessary_wraps, clippy::wildcard_imports)] pub(crate) mod core; pub(crate) mod external; diff --git a/src/lib.rs b/src/lib.rs index 01c7914c..06e22b98 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -869,8 +869,6 @@ Please be careful if you return another traits with the same name. ) ))] #![forbid(unsafe_code)] -#![warn(rust_2018_idioms, single_use_lifetimes, unreachable_pub, clippy::pedantic)] -#![allow(clippy::doc_markdown, clippy::too_many_lines, clippy::manual_assert)] #[cfg(all(feature = "coroutine_trait", not(feature = "unstable")))] compile_error!( diff --git a/src/utils.rs b/src/utils.rs index 3ff49ccd..bb15d3ff 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -200,6 +200,7 @@ macro_rules! attrs_impl { ($($Expr:ident($Struct:ident),)*) => { impl Attrs for Expr { fn attrs(&self) -> &[Attribute] { + // #[cfg_attr(test, deny(non_exhaustive_omitted_patterns))] match self { $(Expr::$Expr(syn::$Struct { attrs, .. }))|* => &attrs, _ => &[], @@ -207,6 +208,7 @@ macro_rules! attrs_impl { } fn attrs_mut(&mut self) -> Option<&mut Vec> { + // #[cfg_attr(test, deny(non_exhaustive_omitted_patterns))] match self { $(Expr::$Expr(syn::$Struct { attrs, .. }))|* => Some(attrs), _ => None, diff --git a/tests/auto_enum.rs b/tests/auto_enum.rs index 0a352560..3c1f583b 100644 --- a/tests/auto_enum.rs +++ b/tests/auto_enum.rs @@ -8,15 +8,17 @@ #![cfg_attr(feature = "fn_traits", feature(fn_traits, unboxed_closures))] #![cfg_attr(feature = "trusted_len", feature(trusted_len))] #![cfg_attr(not(feature = "std"), no_std)] -#![warn(rust_2018_idioms, single_use_lifetimes)] -#![allow(dead_code)] #![allow( + dead_code, + clippy::cast_possible_truncation, clippy::cast_possible_wrap, + clippy::items_after_statements, clippy::let_and_return, clippy::needless_return, clippy::never_loop, - clippy::unnecessary_wraps, - clippy::pedantic + clippy::no_effect_underscore_binding, + clippy::undocumented_unsafe_blocks, + clippy::unnecessary_wraps )] use core::iter; @@ -41,10 +43,7 @@ fn stable() { // block + unsafe block + parentheses #[rustfmt::skip] - #[allow(unknown_lints)] - #[allow(unused_parens)] - #[allow(unused_braces)] - #[allow(unused_unsafe)] + #[allow(unused_parens, unused_braces, unused_unsafe)] #[auto_enum(Iterator)] fn block(x: usize) -> impl Iterator { {{({ unsafe {{({ unsafe { unsafe {{ @@ -668,9 +667,7 @@ fn nested() { } #[rustfmt::skip] - #[allow(unknown_lints)] - #[allow(unused_unsafe)] - #[allow(unused_braces)] + #[allow(unused_braces, unused_unsafe)] #[auto_enum(Iterator)] fn in_block(x: usize) -> impl Iterator { {{{ unsafe {{{ unsafe { unsafe {{ diff --git a/tests/compiletest.rs b/tests/compiletest.rs index 18a3632a..1cc06bc9 100644 --- a/tests/compiletest.rs +++ b/tests/compiletest.rs @@ -15,17 +15,10 @@ feature = "tokio03", feature = "tokio1", ))] -#![warn(rust_2018_idioms, single_use_lifetimes)] - -use std::env; #[rustversion::attr(not(nightly), ignore)] #[test] fn ui() { - if env::var_os("CI").is_none() { - env::set_var("TRYBUILD", "overwrite"); - } - let t = trybuild::TestCases::new(); t.compile_fail("tests/ui/**/*.rs"); t.pass("tests/run-pass/**/*.rs"); diff --git a/tests/enum_derive.rs b/tests/enum_derive.rs index 78a3411c..f6814bca 100644 --- a/tests/enum_derive.rs +++ b/tests/enum_derive.rs @@ -3,7 +3,6 @@ #![cfg_attr(feature = "coroutine_trait", feature(coroutine_trait))] #![cfg_attr(feature = "fn_traits", feature(fn_traits, unboxed_closures))] #![cfg_attr(feature = "trusted_len", feature(trusted_len))] -#![warn(rust_2018_idioms, single_use_lifetimes)] #![allow(dead_code)] // See tests/run-pass for external crates. diff --git a/tests/expandtest.rs b/tests/expandtest.rs index 51b55960..3c23d8b6 100644 --- a/tests/expandtest.rs +++ b/tests/expandtest.rs @@ -2,20 +2,10 @@ #![cfg(not(miri))] #![cfg(not(careful))] -#![warn(rust_2018_idioms, single_use_lifetimes)] - -use std::env; - -const PATH: &str = "tests/expand/**/*.rs"; #[rustversion::attr(not(nightly), ignore)] #[test] fn expandtest() { let args = &["--all-features"]; - if env::var_os("CI").is_some() { - macrotest::expand_without_refresh_args(PATH, args); - } else { - env::set_var("MACROTEST", "overwrite"); - macrotest::expand_args(PATH, args); - } + macrotest::expand_args("tests/expand/**/*.rs", args); } diff --git a/tests/type_analysis.rs b/tests/type_analysis.rs index 52b0b336..f2958e3c 100644 --- a/tests/type_analysis.rs +++ b/tests/type_analysis.rs @@ -1,7 +1,6 @@ // SPDX-License-Identifier: Apache-2.0 OR MIT #![cfg(feature = "type_analysis")] -#![warn(rust_2018_idioms, single_use_lifetimes)] #![allow(dead_code)] use std::fmt; diff --git a/tools/tidy.sh b/tools/tidy.sh index 7e8ce5ae..8b6a2aaa 100755 --- a/tools/tidy.sh +++ b/tools/tidy.sh @@ -65,6 +65,9 @@ fi # Rust (if exists) if [[ -n "$(git ls-files '*.rs')" ]]; then info "checking Rust code style" + if [[ ! -e .rustfmt.toml ]]; then + warn "could not found .rustfmt.toml in the repository root" + fi if type -P rustup &>/dev/null; then # `cargo fmt` cannot recognize files not included in the current workspace and modules # defined inside macros, so run rustfmt directly. @@ -119,6 +122,10 @@ if [[ -n "$(git ls-files '*.rs')" ]]; then for id in $(jq <<<"${metadata}" '.workspace_members[]'); do pkg=$(jq <<<"${metadata}" ".packages[] | select(.id == ${id})") publish=$(jq <<<"${pkg}" -r '.publish') + manifest_path=$(jq <<<"${pkg}" -r '.manifest_path') + if ! grep -q '^\[lints\]' "${manifest_path}"; then + warn "no [lints] table in ${manifest_path} please add '[lints]' with 'workspace = true'" + fi # Publishing is unrestricted if null, and forbidden if an empty array. if [[ "${publish}" == "[]" ]]; then continue @@ -127,11 +134,19 @@ if [[ -n "$(git ls-files '*.rs')" ]]; then done if [[ -n "${has_public_crate}" ]]; then info "checking file permissions" - if [[ -f Cargo.toml ]] && grep -Eq '^\[package\]' Cargo.toml && ! grep -Eq '^publish = false' Cargo.toml; then - if ! grep -Eq '^exclude = \[.*\.\*.*\]' Cargo.toml; then - error "top-level Cargo.toml of real manifest should have exclude field with \"/.*\" and \"/tools\"" - elif ! grep -Eq '^exclude = \[.*/tools.*\]' Cargo.toml; then - error "top-level Cargo.toml of real manifest should have exclude field with \"/.*\" and \"/tools\"" + if [[ -f Cargo.toml ]]; then + root_manifest=$(cargo locate-project --message-format=plain --manifest-path Cargo.toml) + root_pkg=$(jq <<<"${metadata}" ".packages[] | select(.manifest_path == \"${root_manifest}\")") + if [[ -n "${root_pkg}" ]]; then + publish=$(jq <<<"${root_pkg}" -r '.publish') + # Publishing is unrestricted if null, and forbidden if an empty array. + if [[ "${publish}" != "[]" ]]; then + if ! grep -Eq '^exclude = \[.*\.\*.*\]' Cargo.toml; then + error "top-level Cargo.toml of real manifest should have exclude field with \"/.*\" and \"/tools\"" + elif ! grep -Eq '^exclude = \[.*/tools.*\]' Cargo.toml; then + error "top-level Cargo.toml of real manifest should have exclude field with \"/.*\" and \"/tools\"" + fi + fi fi fi for p in $(git ls-files); do @@ -158,27 +173,30 @@ if [[ -n "$(git ls-files '*.rs')" ]]; then fi # C/C++ (if exists) -if [[ -n "$(git ls-files '*.c')$(git ls-files '*.cpp')" ]]; then +if [[ -n "$(git ls-files '*.c' '*.h' '*.cpp' '*.hpp')" ]]; then info "checking C/C++ code style" if [[ ! -e .clang-format ]]; then - warn "could not fount .clang-format in the repository root" + warn "could not found .clang-format in the repository root" fi if type -P clang-format &>/dev/null; then - echo "+ clang-format -i \$(git ls-files '*.c') \$(git ls-files '*.cpp')" - clang-format -i $(git ls-files '*.c') $(git ls-files '*.cpp') - check_diff $(git ls-files '*.c') $(git ls-files '*.cpp') + echo "+ clang-format -i \$(git ls-files '*.c' '*.h' '*.cpp' '*.hpp')" + clang-format -i $(git ls-files '*.c' '*.h' '*.cpp' '*.hpp') + check_diff $(git ls-files '*.c' '*.h' '*.cpp' '*.hpp') else warn "'clang-format' is not installed; skipped C/C++ code style check" fi fi # YAML/JavaScript/JSON (if exists) -if [[ -n "$(git ls-files '*.yml')$(git ls-files '*.js')$(git ls-files '*.json')" ]]; then +if [[ -n "$(git ls-files '*.yml' '*.js' '*.json')" ]]; then info "checking YAML/JavaScript/JSON code style" + if [[ ! -e .editorconfig ]]; then + warn "could not found .editorconfig in the repository root" + fi if type -P npm &>/dev/null; then - echo "+ npx -y prettier -l -w \$(git ls-files '*.yml') \$(git ls-files '*.js') \$(git ls-files '*.json')" - npx -y prettier -l -w $(git ls-files '*.yml') $(git ls-files '*.js') $(git ls-files '*.json') - check_diff $(git ls-files '*.yml') $(git ls-files '*.js') $(git ls-files '*.json') + echo "+ npx -y prettier -l -w \$(git ls-files '*.yml' '*.js' '*.json')" + npx -y prettier -l -w $(git ls-files '*.yml' '*.js' '*.json') + check_diff $(git ls-files '*.yml' '*.js' '*.json') else warn "'npm' is not installed; skipped YAML/JavaScript/JSON code style check" fi @@ -188,7 +206,7 @@ if [[ -n "$(git ls-files '*.yml')$(git ls-files '*.js')$(git ls-files '*.json')" if type -P jq &>/dev/null && type -P yq &>/dev/null; then for workflow in .github/workflows/*.yml; do # The top-level permissions must be weak as they are referenced by all jobs. - permissions=$(yq '.permissions' "${workflow}" | jq -c) + permissions=$(yq -c '.permissions' "${workflow}") case "${permissions}" in '{"contents":"read"}' | '{"contents":"none"}') ;; null) error "${workflow}: top level permissions not found; it must be 'contents: read' or weaker permissions" ;; @@ -222,6 +240,9 @@ fi # Markdown (if exists) if [[ -n "$(git ls-files '*.md')" ]]; then info "checking Markdown style" + if [[ ! -e .markdownlint.yml ]]; then + warn "could not found .markdownlint.yml in the repository root" + fi if type -P npm &>/dev/null; then echo "+ npx -y markdownlint-cli2 \$(git ls-files '*.md')" npx -y markdownlint-cli2 $(git ls-files '*.md') @@ -237,6 +258,9 @@ fi # Shell scripts info "checking Shell scripts" if type -P shfmt &>/dev/null; then + if [[ ! -e .editorconfig ]]; then + warn "could not found .editorconfig in the repository root" + fi echo "+ shfmt -l -w \$(git ls-files '*.sh')" shfmt -l -w $(git ls-files '*.sh') check_diff $(git ls-files '*.sh') @@ -244,6 +268,9 @@ else warn "'shfmt' is not installed; skipped Shell scripts style check" fi if type -P shellcheck &>/dev/null; then + if [[ ! -e .shellcheckrc ]]; then + warn "could not found .shellcheckrc in the repository root" + fi echo "+ shellcheck \$(git ls-files '*.sh')" if ! shellcheck $(git ls-files '*.sh'); then should_fail=1