Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix more clippy-isms, enable clippy in CI and Makefile #796

Merged
merged 1 commit into from
May 4, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
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