Skip to content

Commit

Permalink
Fix more clippy-isms, enable clippy in CI and Makefile (#796)
Browse files Browse the repository at this point in the history
  • Loading branch information
graydon committed May 4, 2023
1 parent 8952557 commit 0aaf89e
Show file tree
Hide file tree
Showing 43 changed files with 348 additions and 228 deletions.
160 changes: 160 additions & 0 deletions .cargo/config.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,160 @@
# paths = ["/path/to/override"] # path dependency overrides

# [alias] # command aliases
# b = "build"
# c = "check"
# t = "test"
# r = "run"
# rr = "run --release"
# recursive_example = "rr --example recursions"
# space_example = ["run", "--release", "--", "\"command list\""]

[build]
# jobs = 1 # number of parallel jobs, defaults to # of CPUs
# rustc = "rustc" # the rust compiler tool
# rustc-wrapper = "…" # run this wrapper instead of `rustc`
# rustc-workspace-wrapper = "…" # run this wrapper instead of `rustc` for workspace members
# rustdoc = "rustdoc" # the doc generator tool
# target = "triple" # build for the target triple (ignored by `cargo install`)
# target-dir = "target" # path of where to place all generated artifacts
rustflags = [
"-Dwarnings",
"-Aclippy::style",
# "-Dclippy::pedantic",
# "-Aclippy::module_name_repetitions",
# "-Aclippy::needless_pass_by_value",
# "-Aclippy::too_many_lines",
# "-Aclippy::must_use_candidate",
# "-Aclippy::missing_errors_doc",
# "-Aclippy::missing_safety_doc",
# "-Aclippy::inline_always",
# "-Aclippy::cast_possible_truncation",
# "-Aclippy::cast_sign_loss",
# "-Aclippy::cast_possible_wrap",
# "-Aclippy::similar_names",
# "-Aclippy::doc_markdown",
# "-Aclippy::default_trait_access",
# "-Aclippy::missing_safety_doc",
# "-Aclippy::struct_excessive_bools",
# "-Aclippy::missing_panics_doc",
# "-Aclippy::cast_lossless",
# "-Aclippy::trivially_copy_pass_by_ref",
# "-Aclippy::wrong_self_convention",
# "-Aclippy::unused_self",
# "-Aclippy::enum_glob_use",
# "-Aclippy::return_self_not_must_use",
# "-Aclippy::map_entry",
# "-Aclippy::match_same_arms",
# "-Aclippy::iter_not_returning_iterator",
# "-Aclippy::unnecessary_wraps",
# "-Aclippy::type_complexity",
] # custom flags to pass to all compiler invocations
# rustdocflags = ["…", "…"] # custom flags to pass to rustdoc
# incremental = true # whether or not to enable incremental compilation
# dep-info-basedir = "…" # path for the base directory for targets in depfiles

# [doc]
# browser = "chromium" # browser to use with `cargo doc --open`,
# # overrides the `BROWSER` environment variable

# [env]
# # Set ENV_VAR_NAME=value for any process run by Cargo
# ENV_VAR_NAME = "value"
# # Set even if already present in environment
# ENV_VAR_NAME_2 = { value = "value", force = true }
# # Value is relative to .cargo directory containing `config.toml`, make absolute
# ENV_VAR_NAME_3 = { value = "relative/path", relative = true }

# [future-incompat-report]
# frequency = 'always' # when to display a notification about a future incompat report

# [cargo-new]
# vcs = "none" # VCS to use ('git', 'hg', 'pijul', 'fossil', 'none')

# [http]
# debug = false # HTTP debugging
# proxy = "host:port" # HTTP proxy in libcurl format
# ssl-version = "tlsv1.3" # TLS version to use
# ssl-version.max = "tlsv1.3" # maximum TLS version
# ssl-version.min = "tlsv1.1" # minimum TLS version
# timeout = 30 # timeout for each HTTP request, in seconds
# low-speed-limit = 10 # network timeout threshold (bytes/sec)
# cainfo = "cert.pem" # path to Certificate Authority (CA) bundle
# check-revoke = true # check for SSL certificate revocation
# multiplexing = true # HTTP/2 multiplexing
# user-agent = "…" # the user-agent header

# [install]
# root = "/some/path" # `cargo install` destination directory

# [net]
# retry = 2 # network retries
# git-fetch-with-cli = true # use the `git` executable for git operations
# offline = true # do not access the network

# [net.ssh]
# known-hosts = ["..."] # known SSH host keys

# [patch.<registry>]
# # Same keys as for [patch] in Cargo.toml

# [profile.<name>] # Modify profile settings via config.
# inherits = "dev" # Inherits settings from [profile.dev].
# opt-level = 0 # Optimization level.
# debug = true # Include debug info.
# split-debuginfo = '...' # Debug info splitting behavior.
# debug-assertions = true # Enables debug assertions.
# overflow-checks = true # Enables runtime integer overflow checks.
# lto = false # Sets link-time optimization.
# panic = 'unwind' # The panic strategy.
# incremental = true # Incremental compilation.
# codegen-units = 16 # Number of code generation units.
# rpath = false # Sets the rpath linking option.
# [profile.<name>.build-override] # Overrides build-script settings.
# # Same keys for a normal profile.
# [profile.<name>.package.<name>] # Override profile for a package.
# # Same keys for a normal profile (minus `panic`, `lto`, and `rpath`).

# [registries.<name>] # registries other than crates.io
# index = "…" # URL of the registry index
# token = "…" # authentication token for the registry

# [registry]
# default = "…" # name of the default registry
# token = "…" # authentication token for crates.io

# [source.<name>] # source definition and replacement
# replace-with = "…" # replace this source with the given named source
# directory = "…" # path to a directory source
# registry = "…" # URL to a registry source
# local-registry = "…" # path to a local registry source
# git = "…" # URL of a git repository source
# branch = "…" # branch name for the git repository
# tag = "…" # tag name for the git repository
# rev = "…" # revision for the git repository

# [target.<triple>]
# linker = "…" # linker to use
# runner = "…" # wrapper to run executables
# rustflags = ["…", "…"] # custom flags for `rustc`

# [target.<cfg>]
# runner = "…" # wrapper to run executables
# rustflags = ["…", "…"] # custom flags for `rustc`

# [target.<triple>.<links>] # `links` build script override
# rustc-link-lib = ["foo"]
# rustc-link-search = ["/path/to/foo"]
# rustc-flags = ["-L", "/some/path"]
# rustc-cfg = ['key="value"']
# rustc-env = {key = "value"}
# rustc-cdylib-link-arg = ["…"]
# metadata_key1 = "value"
# metadata_key2 = "value"

# [term]
# quiet = false # whether cargo output is quiet
# verbose = false # whether cargo provides verbose output
# color = 'auto' # whether cargo colorizes output
# progress.when = 'auto' # whether cargo shows progress bar
# progress.width = 80 # width of progress bar
5 changes: 1 addition & 4 deletions .github/workflows/rust.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,6 @@ on:
branches: [main, release/**]
pull_request:

env:
RUSTFLAGS: -D warnings

jobs:

complete:
Expand Down Expand Up @@ -79,7 +76,7 @@ jobs:
with:
name: cargo-hack
version: 0.5.16
- run: cargo hack --feature-powerset check --locked --target ${{ matrix.sys.target }}
- run: cargo hack --feature-powerset clippy --locked --target ${{ matrix.sys.target }}
- if: matrix.sys.test
run: cargo hack --feature-powerset test --locked --target ${{ matrix.sys.target }}

Expand Down
4 changes: 1 addition & 3 deletions Makefile
Original file line number Diff line number Diff line change
@@ -1,12 +1,10 @@
all: build test

export RUSTFLAGS=-Dwarnings

test:
cargo hack --feature-powerset test

build:
cargo hack --feature-powerset check
cargo hack --feature-powerset clippy

watch:
cargo watch --clear --watch-when-idle --shell '$(MAKE)'
Expand Down
1 change: 1 addition & 0 deletions soroban-env-common/src/compare.rs
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ macro_rules! delegate_compare_to_wrapper {
}};
}

#[allow(clippy::comparison_chain)]
impl<E: Env> Compare<RawVal> for E {
type Error = E::Error;

Expand Down
4 changes: 1 addition & 3 deletions soroban-env-common/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,5 @@ pub use symbol::{Symbol, SymbolError, SymbolObject, SymbolSmall, SymbolSmallIter
// that did panic! in a const context and rt::trap in a non-const
// context but it's not clear how to actually do that.
pub const fn require(b: bool) {
if !b {
panic!();
}
assert!(b,);
}
8 changes: 4 additions & 4 deletions soroban-env-common/src/raw_val.rs
Original file line number Diff line number Diff line change
Expand Up @@ -703,7 +703,7 @@ impl Debug for RawVal {
let ss: SymbolStr =
unsafe { <SymbolSmall as RawValConvertible>::unchecked_from_val(*self) }.into();
let s: &str = ss.as_ref();
write!(f, "Symbol({})", s)
write!(f, "Symbol({s})")
}
Tag::LedgerKeyContractExecutable => write!(f, "LedgerKeyContractCode"),

Expand Down Expand Up @@ -791,9 +791,9 @@ fn test_tag_from_u8() {
let expected_tag = Tag::from_int(i);
let actual_tag = Tag::from_u8(i);
match expected_tag {
Ok(Tag::SmallCodeUpperBound)
| Ok(Tag::ObjectCodeLowerBound)
| Ok(Tag::ObjectCodeUpperBound) => {
Ok(
Tag::SmallCodeUpperBound | Tag::ObjectCodeLowerBound | Tag::ObjectCodeUpperBound,
) => {
assert_eq!(actual_tag, Tag::Bad);
}
Ok(expected_tag) => {
Expand Down
12 changes: 8 additions & 4 deletions soroban-env-common/src/status.rs
Original file line number Diff line number Diff line change
Expand Up @@ -146,16 +146,16 @@ impl Debug for Status {
};
write!(f, "Status({}(", st.name())?;
match st {
ScStatusType::Ok => write!(f, "{}", code),
ScStatusType::UnknownError => write!(f, "{}", code),
ScStatusType::Ok => write!(f, "{code}"),
ScStatusType::UnknownError => write!(f, "{code}"),
ScStatusType::HostValueError => fmt_named_code::<ScHostValErrorCode>(code, f),
ScStatusType::HostObjectError => fmt_named_code::<ScHostObjErrorCode>(code, f),
ScStatusType::HostFunctionError => fmt_named_code::<ScHostFnErrorCode>(code, f),
ScStatusType::HostStorageError => fmt_named_code::<ScHostStorageErrorCode>(code, f),
ScStatusType::HostContextError => fmt_named_code::<ScHostContextErrorCode>(code, f),
ScStatusType::HostAuthError => fmt_named_code::<ScHostAuthErrorCode>(code, f),
ScStatusType::VmError => fmt_named_code::<ScVmErrorCode>(code, f),
ScStatusType::ContractError => write!(f, "{}", code),
ScStatusType::ContractError => write!(f, "{code}"),
}?;
write!(f, "))")
}
Expand Down Expand Up @@ -341,7 +341,11 @@ mod tests {
// puts them all into a list, and sorts them with each comparison function,
// then checks that both lists are sorted the same.

use crate::xdr::*;
use crate::xdr::{
ScHostAuthErrorCode, ScHostContextErrorCode, ScHostFnErrorCode, ScHostObjErrorCode,
ScHostStorageErrorCode, ScHostValErrorCode, ScStatus, ScUnknownErrorCode,
ScVmErrorCode,
};

let xdr_vals = &[
ScStatus::Ok,
Expand Down
4 changes: 2 additions & 2 deletions soroban-env-common/src/string.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ impl<E: Env> TryFromVal<E, &str> for RawVal {
}

#[cfg(feature = "std")]
impl<'a, E: Env> TryFromVal<E, String> for StringObject {
impl<E: Env> TryFromVal<E, String> for StringObject {
type Error = ConversionError;

fn try_from_val(env: &E, v: &String) -> Result<Self, Self::Error> {
Expand All @@ -56,7 +56,7 @@ impl<'a, E: Env> TryFromVal<E, String> for StringObject {
}

#[cfg(feature = "std")]
impl<'a, E: Env> TryFromVal<E, String> for RawVal {
impl<E: Env> TryFromVal<E, String> for RawVal {
type Error = ConversionError;

fn try_from_val(env: &E, v: &String) -> Result<Self, Self::Error> {
Expand Down
10 changes: 3 additions & 7 deletions soroban-env-common/src/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,10 @@ impl core::fmt::Display for SymbolError {
fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
match self {
SymbolError::TooLong(len) => f.write_fmt(format_args!(
"symbol too long: length {}, max {}",
len, MAX_SMALL_CHARS
"symbol too long: length {len}, max {MAX_SMALL_CHARS}"
)),
SymbolError::BadChar(char) => f.write_fmt(format_args!(
"symbol bad char: encountered {}, supported range [a-zA-Z0-9_]",
char
"symbol bad char: encountered {char}, supported range [a-zA-Z0-9_]"
)),
}
}
Expand Down Expand Up @@ -359,11 +357,9 @@ impl Iterator for SymbolSmallIter {

impl FromIterator<char> for SymbolSmall {
fn from_iter<T: IntoIterator<Item = char>>(iter: T) -> Self {
let mut n = 0;
let mut accum: u64 = 0;
for i in iter {
for (n, i) in iter.into_iter().enumerate() {
require(n < MAX_SMALL_CHARS);
n += 1;
accum <<= CODE_BITS;
let v = match i {
'_' => 1,
Expand Down
2 changes: 1 addition & 1 deletion soroban-env-host/benches/common/cost_types/val_xdr_conv.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ impl HostCostMeasurement for ValXdrConvMeasure {
fn new_random_case(_host: &Host, rng: &mut StdRng, _input: u64) -> (Option<RawVal>, ScVal) {
let v = rng.next_u32();
let rv: RawVal = v.into();
let scv: ScVal = ScVal::U32(v as u32);
let scv: ScVal = ScVal::U32(v);
(Some(rv), scv)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ impl HostCostMeasurement for VerifyEd25519SigMeasure {
fn new_random_case(_host: &Host, rng: &mut StdRng, input: u64) -> VerifyEd25519SigSample {
let size = 1 + input * Self::STEP_SIZE;
let keypair: Keypair = Keypair::generate(rng);
let key: PublicKey = keypair.public.clone();
let key: PublicKey = keypair.public;
let msg: Vec<u8> = (0..size).map(|x| x as u8).collect();
let sig: Signature = keypair.sign(msg.as_slice());
VerifyEd25519SigSample { key, msg, sig }
Expand Down
6 changes: 3 additions & 3 deletions soroban-env-host/benches/common/cost_types/vm_ops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,21 +18,21 @@ impl HostCostMeasurement for VmInstantiationMeasure {

fn new_best_case(_host: &Host, _rng: &mut StdRng) -> VmInstantiationSample {
let id: xdr::Hash = [0; 32].into();
let wasm: Vec<u8> = soroban_test_wasms::ADD_I32.clone().into();
let wasm: Vec<u8> = soroban_test_wasms::ADD_I32.into();
VmInstantiationSample { id: Some(id), wasm }
}

fn new_worst_case(_host: &Host, _rng: &mut StdRng, input: u64) -> VmInstantiationSample {
let id: xdr::Hash = [0; 32].into();
let idx = input as usize % util::TEST_WASMS.len();
let wasm = util::TEST_WASMS[idx].clone().into();
let wasm = util::TEST_WASMS[idx].into();
VmInstantiationSample { id: Some(id), wasm }
}

fn new_random_case(_host: &Host, rng: &mut StdRng, _input: u64) -> VmInstantiationSample {
let id: xdr::Hash = [0; 32].into();
let idx = rng.gen_range(0, 10) % util::TEST_WASMS.len();
let wasm = util::TEST_WASMS[idx].clone().into();
let wasm = util::TEST_WASMS[idx].into();
VmInstantiationSample { id: Some(id), wasm }
}
}
Expand Down
2 changes: 1 addition & 1 deletion soroban-env-host/benches/common/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ pub(crate) trait Benchmark {
}

fn get_explicit_bench_names() -> Option<Vec<String>> {
let bare_args: Vec<_> = std::env::args().filter(|x| !x.starts_with("-")).collect();
let bare_args: Vec<_> = std::env::args().filter(|x| !x.starts_with('-')).collect();
match bare_args.len() {
0 | 1 => None,
_ => Some(bare_args[1..].into()),
Expand Down
6 changes: 3 additions & 3 deletions soroban-env-host/src/auth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -427,15 +427,15 @@ impl AuthorizationManager {
let (contract_id, function_name) = match frame {
#[cfg(feature = "vm")]
Frame::ContractVM(vm, fn_name, _) => {
(vm.contract_id.metered_clone(&self.budget)?, fn_name.clone())
(vm.contract_id.metered_clone(&self.budget)?, *fn_name)
}
// Just skip the host function stack frames for now.
// We could also make this included into the authorized stack to
// generalize all the host function invocations.
Frame::HostFunction(_) => return Ok(()),
Frame::Token(id, fn_name, _) => (id.metered_clone(&self.budget)?, *fn_name),
#[cfg(any(test, feature = "testutils"))]
Frame::TestContract(tc) => (tc.id.clone(), tc.func.clone()),
Frame::TestContract(tc) => (tc.id.clone(), tc.func),
};
let Ok(ScVal::Symbol(function_name)) = host.from_host_val(function_name.to_raw()) else {
return Err(host.err_status(xdr::ScHostObjErrorCode::UnexpectedType))
Expand Down Expand Up @@ -950,7 +950,7 @@ impl Host {
self.with_mut_storage(|storage| storage.get(&nonce_key, self.budget_ref()))?;
match &entry.data {
LedgerEntryData::ContractData(ContractDataEntry { val, .. }) => match val {
ScVal::U64(val) => val.clone(),
ScVal::U64(val) => *val,
_ => {
return Err(self.err_general("unexpected nonce entry type"));
}
Expand Down
1 change: 1 addition & 0 deletions soroban-env-host/src/cost_runner/mod.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#![allow(clippy::unit_arg)]
mod cost_types;
mod runner;

Expand Down
Loading

0 comments on commit 0aaf89e

Please sign in to comment.