From 1c3160e392d5ce9d5e855c5918c25baaaa73e809 Mon Sep 17 00:00:00 2001 From: Taiki Endo Date: Tue, 31 Oct 2023 03:04:15 +0900 Subject: [PATCH] Use [lints] in Cargo.toml and apply more lints --- .clippy.toml | 8 ++++++ Cargo.toml | 46 ++++++++++++++++++++++++++++++-- build.rs | 2 -- src/lib.rs | 1 - test_suite/Cargo.toml | 3 +++ test_suite/build.rs | 2 -- test_suite/tests/test.rs | 8 ++++-- tools/tidy.sh | 57 +++++++++++++++++++++++++++++----------- 8 files changed, 103 insertions(+), 24 deletions(-) diff --git a/.clippy.toml b/.clippy.toml index 85a79aa..27a1e09 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 1f9a558..29892ef 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -15,8 +15,50 @@ A lightweight attribute for easy generation of const functions with conditional [package.metadata.docs.rs] targets = ["x86_64-unknown-linux-gnu"] +[lib] +proc-macro = true + +[dependencies] + +[dev-dependencies] + +[lints] +workspace = true + [workspace] members = ["test_suite"] -[lib] -proc-macro = true +# 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/build.rs b/build.rs index fea0dae..b4a81b9 100644 --- a/build.rs +++ b/build.rs @@ -2,8 +2,6 @@ // The rustc-cfg emitted by the build script are *not* public API. -#![warn(rust_2018_idioms, single_use_lifetimes, clippy::pedantic)] - use std::{ env, fs, path::{Path, PathBuf}, diff --git a/src/lib.rs b/src/lib.rs index fdda11e..8311004 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -76,7 +76,6 @@ be maintained manually) ) ))] #![forbid(unsafe_code)] -#![warn(rust_2018_idioms, single_use_lifetimes, unreachable_pub, clippy::pedantic)] #![allow(clippy::cast_lossless)] // older compilers require explicit `extern crate`. diff --git a/test_suite/Cargo.toml b/test_suite/Cargo.toml index a0569bf..2880f5b 100644 --- a/test_suite/Cargo.toml +++ b/test_suite/Cargo.toml @@ -10,3 +10,6 @@ autocfg = "1" [dev-dependencies] const_fn = { path = ".." } rustversion = "1" + +[lints] +workspace = true diff --git a/test_suite/build.rs b/test_suite/build.rs index f45e7be..ec47f78 100644 --- a/test_suite/build.rs +++ b/test_suite/build.rs @@ -1,7 +1,5 @@ // SPDX-License-Identifier: Apache-2.0 OR MIT -#![warn(rust_2018_idioms, single_use_lifetimes)] - use std::{env, process::Command}; fn main() { diff --git a/test_suite/tests/test.rs b/test_suite/tests/test.rs index 0e62684..e04c0d1 100644 --- a/test_suite/tests/test.rs +++ b/test_suite/tests/test.rs @@ -1,8 +1,12 @@ // SPDX-License-Identifier: Apache-2.0 OR MIT #![cfg_attr(const_unstable, feature(const_extern_fn))] -#![warn(rust_2018_idioms, single_use_lifetimes)] -#![allow(clippy::missing_safety_doc, clippy::unused_async, improper_ctypes_definitions)] // this is test +#![allow( + improper_ctypes_definitions, + clippy::missing_safety_doc, + clippy::unused_async, + clippy::undocumented_unsafe_blocks +)] // this is test pub mod signature { #![allow(dead_code)] diff --git a/tools/tidy.sh b/tools/tidy.sh index 7e8ce5a..8b6a2aa 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