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 355c6f4 commit 8eabc5e
Show file tree
Hide file tree
Showing 11 changed files with 102 additions and 42 deletions.
8 changes: 5 additions & 3 deletions .clippy.toml
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,12 @@

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 = [
# https://internals.rust-lang.org/t/synchronized-ffi-access-to-posix-environment-variable-functions/15475
{ path = "std::env::remove_var", reason = "this function should be considered `unsafe`" },
{ path = "std::env::set_var", reason = "this function should be considered `unsafe`" },
{ 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" },
# Since we are using fs-err crate (as `fs`), `fs::*` functions will output a better error.
{ path = "std::fs::canonicalize", reason = "use `fs::canonicalize` instead" },
{ path = "std::fs::copy", reason = "use `fs::copy` instead" },
Expand Down
31 changes: 31 additions & 0 deletions .rustfmt.toml
Original file line number Diff line number Diff line change
@@ -1 +1,32 @@
# Rustfmt configuration
# https://github.com/rust-lang/rustfmt/blob/HEAD/Configurations.md

# Rustfmt cannot format long lines inside macros, but this option detects this.
# This is unstable (tracking issue: https://github.com/rust-lang/rustfmt/issues/3391)
error_on_line_overflow = true

# Override the default formatting style.
# See https://internals.rust-lang.org/t/running-rustfmt-on-rust-lang-rust-and-other-rust-lang-repositories/8732/81.
use_small_heuristics = "Max"
# See https://github.com/rust-dev-tools/fmt-rfcs/issues/149.
# This is unstable (tracking issue: https://github.com/rust-lang/rustfmt/issues/3370)
overflow_delimited_expr = true
# This is unstable (tracking issue: none).
imports_granularity = "Crate"
# This is unstable (tracking issue: none).
group_imports = "StdExternalCrate"

# Apply rustfmt to more places.
# This is unstable (tracking issue: https://github.com/rust-lang/rustfmt/issues/3348).
format_code_in_doc_comments = true

# Automatically fix deprecated style.
use_field_init_shorthand = true
use_try_shorthand = true

# Set the default settings again to always apply the proper formatting without
# being affected by the editor settings.
edition = "2021"
hard_tabs = false
newline_style = "Unix"
tab_spaces = 4
8 changes: 8 additions & 0 deletions docker/build-libunwind/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,8 +1,16 @@
[package]
name = "build-libunwind"
# TODO: Remove version and publish fields once 1.75 is stable.
version = "0.0.0"
edition = "2021"
publish = false

[dependencies]
anyhow = "1"
cc = "1"
fs-err = "2"

[lints.rust]
rust_2018_idioms = "warn"
single_use_lifetimes = "warn"
unsafe_op_in_unsafe_fn = "warn"
7 changes: 1 addition & 6 deletions docker/build-libunwind/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@

// Adapted from https://github.com/rust-lang/rust/blob/1.70.0/src/bootstrap/llvm.rs#L1140-L1295

#![warn(rust_2018_idioms)]

use std::{env, ffi::OsStr, path::PathBuf, process::Command};

use anyhow::Result;
Expand Down Expand Up @@ -173,10 +171,7 @@ fn main() -> Result<()> {
if file.is_file() && file.extension() == Some(OsStr::new("o")) {
// file name starts with "Unwind-EHABI", "Unwind-seh" or "libunwind"
let file_name = file.file_name().unwrap().to_str().expect("UTF-8 file name");
if cpp_sources
.iter()
.any(|f| file_name.starts_with(&f[..f.len() - 4]))
{
if cpp_sources.iter().any(|f| file_name.starts_with(&f[..f.len() - 4])) {
cc_cfg.object(&file);
count += 1;
}
Expand Down
11 changes: 8 additions & 3 deletions docker/test/fixtures/no-std-qemu/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,6 @@
name = "no-std-qemu-test"
edition = "2021"

[workspace]
resolver = "2"

[features]
c = []
cpp = ["c"]
Expand All @@ -20,3 +17,11 @@ semihosting-no-std-test-rt = { git = "https://github.com/taiki-e/semihosting.git

[target.'cfg(all(target_arch = "arm", target_feature = "mclass"))'.dependencies]
cortex-m-rt = "0.7"

[workspace]
resolver = "2"

[lints.rust]
rust_2018_idioms = "warn"
single_use_lifetimes = "warn"
unsafe_op_in_unsafe_fn = "warn"
7 changes: 1 addition & 6 deletions docker/test/fixtures/no-std-qemu/build.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
// SPDX-License-Identifier: Apache-2.0 OR MIT

#![warn(rust_2018_idioms)]

fn main() {
println!("cargo:rerun-if-changed=build.rs");
println!("cargo:rerun-if-changed=int_c.c");
Expand All @@ -15,9 +13,6 @@ fn main() {
}
#[cfg(feature = "cpp")]
{
cc::Build::new()
.cpp(true)
.file("int_cpp.cpp")
.compile("libint_cpp.a");
cc::Build::new().cpp(true).file("int_cpp.cpp").compile("libint_cpp.a");
}
}
1 change: 0 additions & 1 deletion docker/test/fixtures/no-std-qemu/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

#![no_main]
#![no_std]
#![warn(rust_2018_idioms)]

use semihosting::println;

Expand Down
5 changes: 5 additions & 0 deletions docker/test/fixtures/rust/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,8 @@ cpp = []
cc = "1"
# TODO: 0.1.49 fails for android
cmake = "=0.1.48"

[lints.rust]
rust_2018_idioms = "warn"
single_use_lifetimes = "warn"
unsafe_op_in_unsafe_fn = "warn"
7 changes: 1 addition & 6 deletions docker/test/fixtures/rust/build.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
// SPDX-License-Identifier: Apache-2.0 OR MIT

#![warn(rust_2018_idioms)]

use std::env;

fn main() {
Expand Down Expand Up @@ -30,10 +28,7 @@ fn main() {
}

if cfg!(feature = "cpp") {
cc::Build::new()
.cpp(true)
.file("hello_cpp.cpp")
.compile("libhello_cpp.a");
cc::Build::new().cpp(true).file("hello_cpp.cpp").compile("libhello_cpp.a");
} else {
println!(
"cargo:warning={}: C++ from Rust for '{target}' is currently disabled",
Expand Down
2 changes: 0 additions & 2 deletions docker/test/fixtures/rust/src/main.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
// SPDX-License-Identifier: Apache-2.0 OR MIT

#![warn(rust_2018_idioms)]

#[cfg(not(no_c))]
extern "C" {
fn hello_c();
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}" && ! grep -q '^\[lints\.rust\]' "${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 8eabc5e

Please sign in to comment.