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

chore(all): Migrate all instances of #[allow] to #[expect] #1561

Merged
merged 8 commits into from
Sep 30, 2024
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
4 changes: 3 additions & 1 deletion .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,9 @@ jobs:
- name: run pedantic clippy on workspace crates
run: |
cargo clippy --all-targets --all-features \
-- --warn clippy::pedantic --warn clippy::arithmetic-side-effects --deny warnings
-- --warn clippy::pedantic --warn clippy::arithmetic-side-effects \
--warn clippy::allow_attributes --warn clippy::allow_attributes_without_reason \
--deny warnings
- name: run pedantic clippy on tools/protobuf-compiler
run: |
cargo clippy --manifest-path tools/protobuf-compiler/Cargo.toml \
Expand Down
1 change: 1 addition & 0 deletions crates/astria-bridge-contracts/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
name = "astria-bridge-contracts"
version = "0.1.0"
edition = "2021"
rust-version = "1.81.0"

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

Expand Down
2 changes: 1 addition & 1 deletion crates/astria-bridge-contracts/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
#[rustfmt::skip]
#[allow(clippy::pedantic)]
#[expect(clippy::pedantic, clippy::allow_attributes, clippy::allow_attributes_without_reason)]
mod generated;
use std::{
borrow::Cow,
Expand Down
3 changes: 0 additions & 3 deletions crates/astria-bridge-withdrawer/src/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ pub(crate) fn start(socket_addr: SocketAddr, withdrawer_state: WithdrawerState)
axum::Server::bind(&socket_addr).serve(app.into_make_service())
}

#[allow(clippy::unused_async)] // Permit because axum handlers must be async
#[instrument(skip_all)]
async fn get_healthz(State(withdrawer_state): State<WithdrawerState>) -> Healthz {
if withdrawer_state.borrow().is_healthy() {
Expand All @@ -67,7 +66,6 @@ async fn get_healthz(State(withdrawer_state): State<WithdrawerState>) -> Healthz
///
/// + there is a current sequencer height (implying a block from sequencer was received)
/// + there is a current data availability height (implying a height was received from the DA)
#[allow(clippy::unused_async)] // Permit because axum handlers must be async
#[instrument(skip_all)]
async fn get_readyz(State(withdrawer_state): State<WithdrawerState>) -> Readyz {
let is_withdrawer_online = withdrawer_state.borrow().is_ready();
Expand All @@ -78,7 +76,6 @@ async fn get_readyz(State(withdrawer_state): State<WithdrawerState>) -> Readyz {
}
}

#[allow(clippy::unused_async)] // Permit because axum handlers must be async
#[instrument(skip_all)]
async fn get_status(State(withdrawer_state): State<WithdrawerState>) -> Json<StateSnapshot> {
Json(withdrawer_state.borrow().clone())
Expand Down
12 changes: 9 additions & 3 deletions crates/astria-bridge-withdrawer/src/bridge_withdrawer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -167,8 +167,11 @@ impl BridgeWithdrawer {
self.api_server.local_addr()
}

// Panic won't happen because `startup_task` is unwraped lazily after checking if it's `Some`.
#[allow(clippy::missing_panics_doc)]
#[expect(
clippy::missing_panics_doc,
reason = "Panic won't happen because `startup_task` is unwraped lazily after checking if \
it's `Some`."
)]
pub async fn run(self) {
let Self {
shutdown_token,
Expand Down Expand Up @@ -255,7 +258,10 @@ impl BridgeWithdrawer {
}
}

#[allow(clippy::struct_field_names)] // allow: for parity with the `Shutdown` struct.
#[expect(
clippy::struct_field_names,
reason = "for parity with the `Shutdown` struct"
)]
struct TaskHandles {
api_task: JoinHandle<eyre::Result<()>>,
startup_task: Option<JoinHandle<eyre::Result<()>>>,
Expand Down
8 changes: 5 additions & 3 deletions crates/astria-bridge-withdrawer/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,11 @@ use serde::{
Serialize,
};

// Allowed `struct_excessive_bools` because this is used as a container
// for deserialization. Making this a builder-pattern is not actionable.
#[allow(clippy::struct_excessive_bools)]
#[expect(
clippy::struct_excessive_bools,
reason = "This is used as a container for deserialization. Making this a builder-pattern is \
not actionable"
)]
#[derive(Clone, Debug, Deserialize, Serialize, PartialEq)]
/// The single config for creating an astria-bridge service.
pub struct Config {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,10 @@ use super::test_bridge_withdrawer::{
default_native_asset,
};

// allow: want the name to reflect this is a test config.
#[allow(clippy::module_name_repetitions)]
#[expect(
clippy::module_name_repetitions,
reason = "want the name to reflect this is a test config"
)]
pub struct TestEthereum {
contract_address: ethers::types::Address,
provider: Arc<Provider<Ws>>,
Expand Down Expand Up @@ -226,7 +228,10 @@ impl TestEthereumConfig {
}
}

#[allow(clippy::struct_field_names)]
#[expect(
clippy::struct_field_names,
reason = "we want struct field names to be specific"
)]
pub struct AstriaWithdrawerDeployerConfig {
pub base_chain_asset_precision: u32,
pub base_chain_bridge_address: astria_core::primitive::v1::Address,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,10 @@ use tonic::{

const GET_PENDING_NONCE_GRPC_NAME: &str = "get_pending_nonce";

#[allow(clippy::module_name_repetitions)]
#[expect(
clippy::module_name_repetitions,
reason = "naming is helpful for clarity here"
)]
pub struct MockSequencerServer {
_server: JoinHandle<eyre::Result<()>>,
pub(crate) mock_server: MockServer,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,3 @@
// These are tests; failing with panics is ok.
#![allow(clippy::missing_panics_doc)]

mod ethereum;
mod mock_cometbft;
mod mock_sequencer;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ impl TestBridgeWithdrawer {
}
}

#[allow(clippy::module_name_repetitions)]
#[expect(clippy::module_name_repetitions, reason = "naming is for clarity here")]
pub struct TestBridgeWithdrawerConfig {
/// Configures the rollup's withdrawal smart contract to either native or ERC20.
pub ethereum_config: TestEthereumConfig,
Expand Down
5 changes: 5 additions & 0 deletions crates/astria-bridge-withdrawer/tests/blackbox/main.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
#![expect(
clippy::missing_panics_doc,
reason = "These are tests; failing with panics is ok."
)]

use astria_core::protocol::transaction::v1alpha1::Action;
use helpers::{
assert_actions_eq,
Expand Down
1 change: 1 addition & 0 deletions crates/astria-build-info/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
name = "astria-build-info"
version = "0.1.0"
edition = "2021"
rust-version = "1.81.0"

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

Expand Down
8 changes: 5 additions & 3 deletions crates/astria-cli/src/cli/bridge.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,11 @@ use clap::Subcommand;
use color_eyre::eyre;

/// Interact with a Sequencer node
// allow: these are one-shot variants. the size doesn't matter as they are
// passed around only once.
#[allow(clippy::large_enum_variant)]
#[expect(
clippy::large_enum_variant,
reason = "these are one-shot variants. the size doesn't matter as they are passed around only \
once"
)]
#[derive(Debug, Subcommand)]
pub(crate) enum Command {
/// Commands for interacting with Sequencer accounts
Expand Down
3 changes: 0 additions & 3 deletions crates/astria-composer/src/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,9 +74,6 @@ impl IntoResponse for Readyz {
}
}

// axum does not allow non-async handlers. This attribute can be removed
// once this method contains `await` statements.
#[allow(clippy::unused_async)]
#[instrument(skip_all)]
async fn readyz(State(composer_status): State<ComposerStatus>) -> Readyz {
debug!("received readyz request");
Expand Down
8 changes: 5 additions & 3 deletions crates/astria-composer/src/composer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -217,9 +217,11 @@ impl Composer {
///
/// # Panics
/// It panics if the Composer cannot set the SIGTERM listener.
// allow: it seems splitting this into smaller functions makes the code less readable due to
// the high number of params needed for these functions.
#[allow(clippy::too_many_lines)]
#[expect(
clippy::too_many_lines,
reason = "it seems splitting this into smaller functions makes the code less readable due \
to the high number of params needed for these functions"
)]
pub async fn run_until_stopped(self) -> eyre::Result<()> {
let Self {
api_server,
Expand Down
6 changes: 4 additions & 2 deletions crates/astria-composer/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,10 @@ use crate::rollup::{
Rollup,
};

// this is a config, may have many boolean values
#[allow(clippy::struct_excessive_bools)]
#[expect(
clippy::struct_excessive_bools,
reason = "this is a config, may have many boolean values"
)]
#[derive(Debug, Deserialize, Serialize)]
/// The high-level config for creating an astria-composer service.
pub struct Config {
Expand Down
12 changes: 8 additions & 4 deletions crates/astria-composer/src/executor/bundle_factory/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -164,8 +164,10 @@ mod bundle_factory {

// assert that the bundle factory has one bundle in the finished queue, that the factory is
// full and that err was returned
// allow: this is intended to match all possible variants
#[allow(clippy::match_wildcard_for_single_variants)]
#[expect(
clippy::match_wildcard_for_single_variants,
reason = "this is intended to match all possible variants"
)]
match err {
BundleFactoryError::FinishedQueueFull(_) => {}
other => panic!("expected a FinishedQueueFull variant, but got {other:?}"),
Expand Down Expand Up @@ -211,8 +213,10 @@ mod bundle_factory {

// assert that the bundle factory has one bundle in the finished queue, that the factory is
// full and that err was returned
// allow: this is intended to match all possible variants
#[allow(clippy::match_wildcard_for_single_variants)]
#[expect(
clippy::match_wildcard_for_single_variants,
reason = "this is intended to match all possible variants"
)]
match err {
BundleFactoryError::FinishedQueueFull(_) => {}
other => panic!("expected a FinishedQueueFull variant, but got {other:?}"),
Expand Down
3 changes: 2 additions & 1 deletion crates/astria-composer/src/executor/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -669,7 +669,8 @@ pin_project! {
impl Future for SubmitFut {
type Output = eyre::Result<u32>;

#[allow(clippy::too_many_lines)]
// FIXME (https://github.com/astriaorg/astria/issues/1572): This function is too long and should be refactored.
#[expect(clippy::too_many_lines, reason = "this may warrant a refactor")]
fn poll(mut self: Pin<&mut Self>, cx: &mut std::task::Context<'_>) -> Poll<Self::Output> {
const INVALID_NONCE: Code = Code::Err(AbciErrorCode::INVALID_NONCE.value());
loop {
Expand Down
16 changes: 10 additions & 6 deletions crates/astria-conductor/src/celestia/verify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -456,9 +456,11 @@ impl RateLimitedVerificationClient {
mut self,
height: SequencerHeight,
) -> Result<Box<tendermint_rpc::endpoint::commit::Response>, BoxError> {
// allow: it is desired that the wildcard matches all future added variants because
// this call must only return a single specific variant, panicking otherwise.
#[allow(clippy::match_wildcard_for_single_variants)]
#[expect(
clippy::match_wildcard_for_single_variants,
reason = "it is desired that the wildcard matches all future added variants because \
this call must only return a single specific variant, panicking otherwise"
)]
match self
.inner
.ready()
Expand All @@ -479,9 +481,11 @@ impl RateLimitedVerificationClient {
prev_height: SequencerHeight,
height: SequencerHeight,
) -> Result<Box<tendermint_rpc::endpoint::validators::Response>, BoxError> {
// allow: it is desired that the wildcard matches all future added variants because
// this call must only return a single specific variant, panicking otherwise.
#[allow(clippy::match_wildcard_for_single_variants)]
#[expect(
clippy::match_wildcard_for_single_variants,
reason = "it is desired that the wildcard matches all future added variants because \
this call must only return a single specific variant, panicking otherwise"
)]
match self
.inner
.ready()
Expand Down
8 changes: 5 additions & 3 deletions crates/astria-conductor/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,11 @@ impl std::fmt::Display for CommitLevel {
}
}

// Allowed `struct_excessive_bools` because this is used as a container
// for deserialization. Making this a builder-pattern is not actionable.
#[allow(clippy::struct_excessive_bools)]
#[expect(
clippy::struct_excessive_bools,
reason = "This is used as a container for deserialization. Making this a builder-pattern is \
not actionable"
)]
#[derive(Debug, Clone, Serialize, Deserialize)]
pub struct Config {
/// The block time of Celestia network in milliseconds.
Expand Down
4 changes: 0 additions & 4 deletions crates/astria-conductor/src/executor/channel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,6 @@ impl<T> From<TokioSendError<T>> for SendError {
}
}

// allow: this is mimicking tokio's `SendError` that returns the stack-allocated object.
#[allow(clippy::result_large_err)]
#[derive(Debug, thiserror::Error, PartialEq)]
pub(crate) enum TrySendError<T> {
#[error("the channel is closed")]
Expand Down Expand Up @@ -105,8 +103,6 @@ impl<T> Sender<T> {
/// Attempts to send a block without blocking.
///
/// Returns an error if the channel is out of permits or if it has been closed.
// allow: this is mimicking tokio's `TrySendError` that returns the stack-allocated object.
#[allow(clippy::result_large_err)]
pub(super) fn try_send(&self, block: T) -> Result<(), TrySendError<T>> {
let sem = match self.sem.upgrade() {
None => return Err(TrySendError::Closed(block)),
Expand Down
4 changes: 0 additions & 4 deletions crates/astria-conductor/src/executor/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -153,8 +153,6 @@ impl Handle<StateIsInit> {
Ok(())
}

// allow: return value of tokio's mpsc send try_send method
#[allow(clippy::result_large_err)]
pub(crate) fn try_send_firm_block(
&self,
block: ReconstructedBlock,
Expand All @@ -178,8 +176,6 @@ impl Handle<StateIsInit> {
Ok(())
}

// allow: this is mimicking tokio's `SendError` that returns the stack-allocated object.
#[allow(clippy::result_large_err)]
pub(crate) fn try_send_soft_block(
&self,
block: FilteredSequencerBlock,
Expand Down
7 changes: 5 additions & 2 deletions crates/astria-conductor/tests/blackbox/main.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
// allow: clippy lints that are not ok in production code but acceptable or wanted in tests
#![expect(
clippy::missing_panics_doc,
reason = "clippy lints that are not ok in production code but acceptable or wanted in tests"
)]

pub mod firm_only;
#[allow(clippy::missing_panics_doc)]
pub mod helpers;
pub mod shutdown;
pub mod soft_and_firm;
Expand Down
7 changes: 5 additions & 2 deletions crates/astria-conductor/tests/blackbox/soft_and_firm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -325,7 +325,7 @@ async fn executes_firm_then_soft_at_next_height() {
);
}

#[allow(clippy::too_many_lines)] // it's a test, it's fine
#[expect(clippy::too_many_lines, reason = "it's a test, it's fine")]
#[tokio::test(flavor = "multi_thread", worker_threads = 1)]
async fn missing_block_is_fetched_for_updating_firm_commitment() {
let test_conductor = spawn_conductor(CommitLevel::SoftAndFirm).await;
Expand Down Expand Up @@ -451,7 +451,10 @@ async fn missing_block_is_fetched_for_updating_firm_commitment() {
/// Astria Geth will return a `PermissionDenied` error if the `execute_block` RPC is called
/// before `get_genesis_info` and `get_commitment_state` are called, which would happen in the
/// case of a restart. This response is mounted to cause the conductor to restart.
#[allow(clippy::too_many_lines)] // allow: all lines fairly necessary, and I don't think a test warrants a refactor
#[expect(
clippy::too_many_lines,
reason = "all lines fairly necessary, and I don't think a test warrants a refactor"
)]
#[tokio::test(flavor = "multi_thread", worker_threads = 1)]
async fn conductor_restarts_on_permission_denied() {
let test_conductor = spawn_conductor(CommitLevel::SoftAndFirm).await;
Expand Down
1 change: 0 additions & 1 deletion crates/astria-core/src/celestia.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ pub use celestia_types::nmt::Namespace;
/// Panics if `bytes` contains less then 10 bytes.
#[must_use = "a celestia namespace must be used in order to be useful"]
pub const fn namespace_v0_from_first_10_bytes(bytes: &[u8]) -> Namespace {
#[allow(clippy::assertions_on_constants)]
const _: () = assert!(
10 == celestia_types::nmt::NS_ID_V0_SIZE,
"verify that the celestia v0 namespace was changed from 10 bytes"
Expand Down
7 changes: 4 additions & 3 deletions crates/astria-core/src/crypto.rs
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,6 @@ impl VerificationKey {
}
/// this ensures that `ADDRESS_LEN` is never accidentally changed to a value
/// that would violate this assumption.
#[allow(clippy::assertions_on_constants)]
const _: () = assert!(ADDRESS_LEN <= 32);
let bytes: [u8; 32] = Sha256::digest(self).into();
first_20(bytes)
Expand Down Expand Up @@ -311,8 +310,10 @@ mod tests {

// From https://doc.rust-lang.org/std/cmp/trait.PartialOrd.html
#[test]
// allow: we want explicit assertions here to match the documented expected behavior.
#[allow(clippy::nonminimal_bool)]
#[expect(
clippy::nonminimal_bool,
reason = "we want explicit assertions here to match the documented expected behavior"
)]
fn verification_key_comparisons_should_be_consistent() {
// A key which compares greater than "low" ones below, and with its address uninitialized.
let high_uninit = VerificationKey {
Expand Down
Loading
Loading