Skip to content

Commit

Permalink
🦀 Comprehensive Clippy run (#380)
Browse files Browse the repository at this point in the history
## What?
Run clippy::all, which contains the default lint groups (correctness, suspicious, style, complexity, perf)

## Why?
We haven't checked clippy in a long time, it provides useful feedback on code style

## How?
We define in the workspace the clippy lints we want to run, and the ones we want to ignore

In certain pallets like democracy, which are forks, we don't want to check the clippy there, so we ignore everything.

## Anything else?
I started trying to do both all and pedantic groups, but pedantic is huge and would have made the PR bloated.

I implemented some of those changes like the module name repetition, but then stopped.

I will in a later PR cherry pick some lints from pedantic and add them to the workspace
  • Loading branch information
JuaniRios authored Aug 23, 2024
1 parent c4f1fe9 commit ea31031
Show file tree
Hide file tree
Showing 81 changed files with 617 additions and 612 deletions.
15 changes: 15 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,21 @@ members = [
default-members = ["nodes/*", "pallets/*"]
resolver = "2"

[workspace.lints.clippy]
all = { level = "warn", priority = -1}
#all = { level = "allow", priority = -1}
#pedantic = { level = "warn", priority = -1}
#pedantic = { level = "allow", priority = -1}

inconsistent_digit_grouping = "allow"
zero_prefixed_literal = "allow"
missing_errors_doc = "allow"
must_use_candidate = "allow"
identity_op = "allow"

[workspace.lints.rust]
unreachable_patterns = "deny"

[workspace.package]
authors = ['Polimec Foundation <info@polimec.org>']
documentation = "https://wiki.polimec.org/"
Expand Down
Empty file added clippy.toml
Empty file.
3 changes: 3 additions & 0 deletions integration-tests/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ readme.workspace = true
repository.workspace = true
version.workspace = true

[lints]
workspace = true

[build-dependencies]
substrate-wasm-builder.workspace = true

Expand Down
3 changes: 3 additions & 0 deletions integration-tests/penpal/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ homepage = "https://substrate.io"
repository.workspace = true
edition.workspace = true

[lints]
workspace = true

[package.metadata.docs.rs]
targets = ["x86_64-unknown-linux-gnu"]

Expand Down
1 change: 0 additions & 1 deletion integration-tests/src/tests/ct_migration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,6 @@ fn full_pallet_migration_test() {
let (project_id, participants) = create_settled_project();
let project_status =
PolimecNet::execute_with(|| pallet_funding::ProjectsDetails::<PolimecRuntime>::get(project_id).unwrap().status);
dbg!(project_status);

mock_hrmp_establishment(project_id);

Expand Down
5 changes: 1 addition & 4 deletions integration-tests/src/tests/e2e.rs
Original file line number Diff line number Diff line change
Expand Up @@ -499,7 +499,6 @@ fn ct_migrated() {

PenNet::execute_with(|| {
println!("penpal events:");
dbg!(PenNet::events());
});

// Migration is ready
Expand All @@ -519,10 +518,8 @@ fn ct_migrated() {
// Migrate CTs
let accounts = excel_ct_amounts().iter().map(|item| item.0.clone()).unique().collect::<Vec<_>>();
let total_ct_sold = excel_ct_amounts().iter().fold(FixedU128::zero(), |acc, item| acc + item.1);
dbg!(total_ct_sold);
let polimec_sov_acc = PenNet::sovereign_account_id_of((Parent, Parachain(polimec::PARA_ID)).into());
let polimec_fund_balance = PenNet::account_data_of(polimec_sov_acc);
dbg!(polimec_fund_balance);

let names = names();

Expand Down Expand Up @@ -550,7 +547,7 @@ fn ct_migrated() {
let data = PenNet::account_data_of(item.0.clone());
let key: [u8; 32] = item.0.clone().into();
println!("Participant {} has {} CTs. Expected {}", names[&key], data.free.clone(), item.1);
dbg!(data.clone());

let amount_as_balance = item.1.saturating_mul_int(CT_UNIT);
assert_close_enough!(
data.free,
Expand Down
3 changes: 3 additions & 0 deletions macros/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@ name = "macros"
version.workspace = true
edition.workspace = true

[lints]
workspace = true

[lib]
name = "macros"
path = "src/lib.rs"
Expand Down
6 changes: 2 additions & 4 deletions macros/src/generate_accounts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ use syn::{
Expr, GenericArgument, GenericParam, Generics, Ident, ItemMod, Result, Token, Type, Visibility, WhereClause,
};

pub fn generate_accounts_impl(input: TokenStream) -> TokenStream {
pub fn macro_impl(input: TokenStream) -> TokenStream {
let inputs = parse_macro_input!(input with Punctuated::<Ident, Token![,]>::parse_terminated);
let mut output = quote! {};
let mut insertions = Vec::new();
Expand All @@ -40,9 +40,7 @@ pub fn generate_accounts_impl(input: TokenStream) -> TokenStream {
let name = input.to_string();

// Ensure the name is all uppercase
if name != name.to_uppercase() {
panic!("Name must be in all uppercase");
}
assert_eq!(name, name.to_uppercase(), "Name must be in all uppercase");

// Generate a unique [u8; 32] value for the constant
let mut value = [0u8; 32];
Expand Down
2 changes: 1 addition & 1 deletion macros/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,5 +21,5 @@ mod generate_accounts;

#[proc_macro]
pub fn generate_accounts(input: TokenStream) -> TokenStream {
generate_accounts::generate_accounts_impl(input)
generate_accounts::macro_impl(input)
}
3 changes: 3 additions & 0 deletions macros/tests/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@ name = "macros-tests"
version.workspace = true
edition.workspace = true

[lints]
workspace = true

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
Expand Down
4 changes: 4 additions & 0 deletions nodes/parachain/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@ readme.workspace = true
repository.workspace = true
version.workspace = true

[lints]
clippy.all = "allow"
clippy.pedantic = "allow"

[dependencies]
clap = { workspace = true, features = ["derive"] }
log.workspace = true
Expand Down
5 changes: 4 additions & 1 deletion nodes/parachain/src/chain_spec/polimec_paseo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,10 @@

use sc_service::ChainType;

use crate::chain_spec::{common::*, get_properties, Extensions, GenericChainSpec, DEFAULT_PARA_ID};
use crate::chain_spec::{
common::{alice, bob, charlie, dave, eve, genesis_config, GenesisConfigParams},
get_properties, Extensions, GenericChainSpec, DEFAULT_PARA_ID,
};
use polimec_runtime::{AccountId, MinCandidateStk};

pub fn get_local_chain_spec() -> GenericChainSpec {
Expand Down
4 changes: 4 additions & 0 deletions pallets/democracy/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@ repository.workspace = true
description = "FRAME pallet for democracy"
readme = "README.md"

[lints]
clippy.all = "allow"
clippy.pedantic = "allow"

[package.metadata.docs.rs]
targets = ["x86_64-unknown-linux-gnu"]

Expand Down
2 changes: 2 additions & 0 deletions pallets/democracy/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,8 @@

#![recursion_limit = "256"]
#![cfg_attr(not(feature = "std"), no_std)]
// Needed due to empty sections raising the warning
#![allow(unreachable_patterns)]

use frame_support::{
ensure,
Expand Down
3 changes: 3 additions & 0 deletions pallets/dispenser/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ readme.workspace = true
repository.workspace = true
version.workspace = true

[lints]
workspace = true

[package.metadata.docs.rs]
targets = ["x86_64-unknown-linux-gnu"]

Expand Down
16 changes: 4 additions & 12 deletions pallets/dispenser/src/extensions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

// If you feel like getting in touch with us, you can do so at info@polimec.org

use crate::*;
use crate::{Call, Config};
use frame_support::{
dispatch::{CheckIfFeeless, DispatchInfo},
pallet_prelude::*,
Expand Down Expand Up @@ -120,13 +120,7 @@ where
let provides = vec![Encode::encode(&(who, self.0))];
let requires = if account.nonce < self.0 { vec![Encode::encode(&(who, self.0 - One::one()))] } else { vec![] };

Ok(ValidTransaction {
priority: 0,
requires,
provides,
longevity: TransactionLongevity::max_value(),
propagate: true,
})
Ok(ValidTransaction { priority: 0, requires, provides, longevity: TransactionLongevity::MAX, propagate: true })
}
}

Expand Down Expand Up @@ -219,10 +213,8 @@ where
len: usize,
result: &DispatchResult,
) -> Result<(), TransactionValidityError> {
if let Some(pre) = pre {
if let Some(pre) = pre {
S::post_dispatch(Some(pre), info, post_info, len, result)?;
}
if let Some(Some(pre)) = pre {
S::post_dispatch(Some(pre), info, post_info, len, result)?;
}
Ok(())
}
Expand Down
9 changes: 7 additions & 2 deletions pallets/dispenser/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@
// If you feel like getting in touch with us, you can do so at info@polimec.org

#![cfg_attr(not(feature = "std"), no_std)]
// Needed due to empty sections raising the warning
#![allow(unreachable_patterns)]
#![allow(clippy::large_enum_variant)]

pub use pallet::*;

pub use crate::weights::WeightInfo;
Expand Down Expand Up @@ -45,6 +49,7 @@ pub type AccountIdOf<T> = <T as frame_system::Config>::AccountId;
type CurrencyOf<T> = <<T as Config>::VestingSchedule as VestingSchedule<AccountIdOf<T>>>::Currency;
#[frame_support::pallet]
pub mod pallet {
#[allow(clippy::wildcard_imports)]
use super::*;
use crate::weights::WeightInfo;
use frame_support::{
Expand Down Expand Up @@ -145,9 +150,9 @@ pub mod pallet {
impl<T: Config> Pallet<T> {
#[pallet::feeless_if( | origin: &OriginFor<T>, jwt: &UntrustedToken | -> bool {
if let Ok((_, did, _, _)) = T::InvestorOrigin::ensure_origin(origin.clone(), jwt, T::VerifierPublicKey::get()) {
return Dispensed::<T>::get(did).is_none()
Dispensed::<T>::get(did).is_none()
} else {
return false
false
}
})]
#[pallet::call_index(0)]
Expand Down
4 changes: 4 additions & 0 deletions pallets/elections-phragmen/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@ repository.workspace = true
description = "FRAME pallet based on seq-Phragmén election method."
readme = "README.md"

[lints]
clippy.all = "allow"
clippy.pedantic = "allow"

[package.metadata.docs.rs]
targets = ["x86_64-unknown-linux-gnu"]

Expand Down
3 changes: 2 additions & 1 deletion pallets/elections-phragmen/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,8 @@
//! - [`Module`]

#![cfg_attr(not(feature = "std"), no_std)]
// Needed due to empty sections raising the warning
#![allow(unreachable_patterns)]

use frame_support::{
pallet_prelude::DispatchResult,
Expand All @@ -102,7 +104,6 @@ use frame_support::{
},
weights::Weight,
};
use log;
use parity_scale_codec::{Decode, Encode};
use scale_info::TypeInfo;
use sp_npos_elections::{ElectionResult, ExtendedBalance};
Expand Down
4 changes: 2 additions & 2 deletions pallets/elections-phragmen/src/weights.rs
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,7 @@ impl<T: frame_system::Config> WeightInfo for SubstrateWeight<T> {
// Measured: `0 + e * (28 ±0) + v * (606 ±0)`
// Estimated: `178887 + c * (2135 ±7) + e * (12 ±0) + v * (2653 ±6)`
// Minimum execution time: 1_281_877_000 picoseconds.
Weight::from_parts(1_288_147_000, 178887)
Weight::from_parts(1_288_147_000, 178_887)
// Standard Error: 528_851
.saturating_add(Weight::from_parts(17_761_407, 0).saturating_mul(v.into()))
// Standard Error: 33_932
Expand Down Expand Up @@ -559,7 +559,7 @@ impl WeightInfo for () {
// Measured: `0 + e * (28 ±0) + v * (606 ±0)`
// Estimated: `178887 + c * (2135 ±7) + e * (12 ±0) + v * (2653 ±6)`
// Minimum execution time: 1_281_877_000 picoseconds.
Weight::from_parts(1_288_147_000, 178887)
Weight::from_parts(1_288_147_000, 178_887)
// Standard Error: 528_851
.saturating_add(Weight::from_parts(17_761_407, 0).saturating_mul(v.into()))
// Standard Error: 33_932
Expand Down
6 changes: 6 additions & 0 deletions pallets/funding/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,12 @@ readme.workspace = true
repository.workspace = true
version.workspace = true

[lints]
workspace = true

#[lints.clippy]
#inconsistent_digit_grouping = "allow"

[dependencies]
serde = { workspace = true }
parity-scale-codec = { workspace = true, features = [
Expand Down
4 changes: 2 additions & 2 deletions pallets/funding/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -510,7 +510,7 @@ mod benchmarks {
project_metadata.clone().policy_ipfs_cid.unwrap(),
);
#[extrinsic_call]
start_evaluation(RawOrigin::Signed(issuer), jwt, project_id);
start_evaluation(RawOrigin::Signed(issuer), jwt, probject_id);

// * validity checks *
// Storage
Expand Down Expand Up @@ -1396,7 +1396,7 @@ mod benchmarks {
account: bidder.clone(),
id: bid_to_settle.id,
final_ct_amount: bid_to_settle.original_ct_amount,
final_ct_price: wap,
final_ct_usd_price: wap,
}
.into(),
);
Expand Down
3 changes: 3 additions & 0 deletions pallets/funding/src/functions/1_application.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
#![allow(clippy::wildcard_imports)]
#![allow(clippy::type_complexity)]

use super::*;

impl<T: Config> Pallet<T> {
Expand Down
1 change: 1 addition & 0 deletions pallets/funding/src/functions/2_evaluation.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#[allow(clippy::wildcard_imports)]
use super::*;

impl<T: Config> Pallet<T> {
Expand Down
Loading

0 comments on commit ea31031

Please sign in to comment.