Skip to content

Commit

Permalink
Use [lints] in Cargo.toml and apply more lints
Browse files Browse the repository at this point in the history
  • Loading branch information
taiki-e committed Oct 30, 2023
1 parent bb35171 commit e5c14f2
Show file tree
Hide file tree
Showing 10 changed files with 124 additions and 49 deletions.
8 changes: 8 additions & 0 deletions .clippy.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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 <https://github.com/rust-lang/rust/issues/27970> for more" },
{ path = "std::env::set_var", reason = "this is not thread-safe and inherently unsafe; see <https://github.com/rust-lang/rust/issues/27970> for more" },
]
disallowed-types = []
48 changes: 44 additions & 4 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,53 @@ targets = ["x86_64-unknown-linux-gnu"]
# The following are external types that are allowed to be exposed in our public API.
allowed_external_types = []

[workspace]
resolver = "2"
members = ["tools/codegen"]

[lib]
doc-scrape-examples = false

[features]
default = ["std"]
std = []

[dev-dependencies]

[lints]
workspace = true

[workspace]
resolver = "2"
members = ["tools/codegen"]

# 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 }
2 changes: 0 additions & 2 deletions src/duration.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
// SPDX-License-Identifier: Apache-2.0 OR MIT

#![allow(deprecated_in_future)]

use core::{
cmp::Ordering,
fmt,
Expand Down
9 changes: 0 additions & 9 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,6 @@ fn foo(secs: u64, nanos: u32, instant: Instant) -> Option<Duration> {
))]
#![forbid(unsafe_code)]
#![warn(
rust_2018_idioms,
single_use_lifetimes,
unreachable_pub,
clippy::pedantic,
// Lints that may help when writing public library.
missing_debug_implementations,
missing_docs,
Expand All @@ -91,9 +87,6 @@ fn foo(secs: u64, nanos: u32, instant: Instant) -> Option<Duration> {
clippy::cast_possible_truncation,
clippy::cast_precision_loss,
clippy::cast_sign_loss,
clippy::manual_map, // Option::map is not const
clippy::manual_range_contains,
clippy::module_name_repetitions,
clippy::must_use_candidate,
)]
#![cfg_attr(docsrs, feature(doc_cfg))]
Expand All @@ -112,7 +105,6 @@ mod assert_impl;
mod utils;

mod duration;
#[allow(unreachable_pub)] // false positive: https://github.com/rust-lang/rust/issues/102352
pub use crate::duration::Duration;

#[cfg(feature = "std")]
Expand All @@ -121,5 +113,4 @@ mod instant;
pub use crate::instant::Instant;

mod error;
#[allow(unreachable_pub)] // false positive: https://github.com/rust-lang/rust/issues/102352
pub use crate::error::TryFromTimeError;
7 changes: 5 additions & 2 deletions tests/duration.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
// SPDX-License-Identifier: Apache-2.0 OR MIT

#![warn(rust_2018_idioms, single_use_lifetimes)]
#![allow(clippy::zero_prefixed_literal, clippy::non_ascii_literal, clippy::assertions_on_constants)]
#![allow(
clippy::assertions_on_constants,
clippy::items_after_statements,
clippy::zero_prefixed_literal
)]

use core::time;

Expand Down
1 change: 0 additions & 1 deletion tests/instant.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
// SPDX-License-Identifier: Apache-2.0 OR MIT

#![cfg(feature = "std")]
#![warn(rust_2018_idioms, single_use_lifetimes)]
#![allow(clippy::eq_op)]

// https://github.com/rust-lang/rust/blob/1.63.0/library/std/src/time/tests.rs
Expand Down
3 changes: 3 additions & 0 deletions tools/codegen/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,6 @@ prettyplease = "0.2"
proc-macro2 = "1"
quote = "1"
syn = { version = "2", features = ["full", "visit-mut"] }

[lints]
workspace = true
18 changes: 13 additions & 5 deletions tools/codegen/src/file.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,15 @@ macro_rules! function_name {
}};
}

pub fn workspace_root() -> PathBuf {
pub(crate) fn workspace_root() -> PathBuf {
let mut dir = PathBuf::from(env!("CARGO_MANIFEST_DIR"));
dir.pop(); // codegen
dir.pop(); // tools
dir
}

#[track_caller]
pub fn header(function_name: &str) -> String {
pub(crate) fn header(function_name: &str) -> String {
// rust-analyzer does not respect outer attribute (#[rustfmt::skip]) on
// a module without a body. So use inner attribute under cfg(rustfmt).
format!(
Expand All @@ -47,7 +47,11 @@ pub fn header(function_name: &str) -> String {
}

#[track_caller]
pub fn write(function_name: &str, path: impl AsRef<Path>, contents: TokenStream) -> Result<()> {
pub(crate) fn write(
function_name: &str,
path: impl AsRef<Path>,
contents: TokenStream,
) -> Result<()> {
write_raw(function_name, path.as_ref(), format_tokens(contents)?)
}

Expand Down Expand Up @@ -119,7 +123,11 @@ fn test_format_macros() {
}

#[track_caller]
pub fn write_raw(function_name: &str, path: &Path, contents: impl AsRef<[u8]>) -> Result<()> {
pub(crate) fn write_raw(
function_name: &str,
path: &Path,
contents: impl AsRef<[u8]>,
) -> Result<()> {
static LINGUIST_GENERATED: OnceLock<Vec<globset::GlobMatcher>> = OnceLock::new();
let linguist_generated = LINGUIST_GENERATED.get_or_init(|| {
let gitattributes = fs::read_to_string(workspace_root().join(".gitattributes")).unwrap();
Expand Down Expand Up @@ -148,7 +156,7 @@ pub fn write_raw(function_name: &str, path: &Path, contents: impl AsRef<[u8]>) -
Ok(())
}

pub fn git_ls_files(dir: &Path, filters: &[&str]) -> Result<Vec<(String, PathBuf)>> {
pub(crate) fn git_ls_files(dir: &Path, filters: &[&str]) -> Result<Vec<(String, PathBuf)>> {
let mut cmd = Command::new("git");
cmd.arg("ls-files").args(filters).current_dir(dir);
let output = cmd.output().with_context(|| format!("could not execute process `{cmd:?}`"))?;
Expand Down
20 changes: 9 additions & 11 deletions tools/codegen/src/main.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
// SPDX-License-Identifier: Apache-2.0 OR MIT

#![warn(rust_2018_idioms, single_use_lifetimes)]
#![allow(clippy::match_on_vec_items, clippy::needless_pass_by_value)]
#![allow(clippy::match_on_vec_items, clippy::needless_pass_by_value, clippy::wildcard_imports)]

#[macro_use]
mod file;
Expand Down Expand Up @@ -69,12 +68,11 @@ fn gen_assert_impl() -> Result<()> {
visited_types.insert(path_string.clone());

let has_generics = generics.type_params().count() != 0;
if generics.const_params().count() != 0 {
panic!(
"gen_assert_impl doesn't support const generics yet; \
skipped `{path_string}`"
);
}
assert_eq!(
generics.const_params().count(),
0,
"gen_assert_impl doesn't support const generics yet; skipped `{path_string}`"
);

let lt = generics.lifetimes().map(|_| quote! { '_ }).collect::<Vec<_>>();
if has_generics {
Expand Down Expand Up @@ -169,10 +167,10 @@ fn gen_assert_impl() -> Result<()> {
});
}
} else {
let lt = if !lt.is_empty() {
quote! { <#(#lt),*> }
} else {
let lt = if lt.is_empty() {
quote! {}
} else {
quote! { <#(#lt),*> }
};
if NOT_SEND.contains(&path_string.as_str()) {
use_macros = true;
Expand Down
57 changes: 42 additions & 15 deletions tools/tidy.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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" ;;
Expand Down Expand Up @@ -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')
Expand All @@ -237,13 +258,19 @@ 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')
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
Expand Down

0 comments on commit e5c14f2

Please sign in to comment.