Skip to content

Deny anyhow #32135

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

Closed
wants to merge 2 commits into from
Closed
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
3 changes: 3 additions & 0 deletions clippy.toml
Original file line number Diff line number Diff line change
Expand Up @@ -88,9 +88,12 @@ disallowed-macros = [
{ path = "proptest::prop_oneof", reason = "use `proptest::strategy::Union::new` instead" },
{ path = "log::log", reason = "use the macros provided by `tracing` instead (database-issues#3001)" },
{ path = "tracing::instrument", reason = "use `mz_ore::instrument` instead" },
{ path = "anyhow::anyhow", reason = "use structured errors instead" },
{ path = "anyhow::bail", reason = "use structured errors instead" },
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

imo this would be more valuable with a fleshed out example to point people towards

]

disallowed-types = [
{ path = "anyhow::Result", reason = "use structured errors instead" },
{ path = "std::collections::HashMap", reason = "use `std::collections::BTreeMap` or `mz_ore::collections::HashMap` instead" },
{ path = "std::collections::HashSet", reason = "use `std::collections::BTreeSet` or `mz_ore::collections::HashSet` instead" },
]
3 changes: 3 additions & 0 deletions misc/bazel/cargo-gazelle/src/bin/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@
// the Business Source License, use of this software will be governed
// by the Apache License, Version 2.0.

// database-issues#9092: anyhow should not be used.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the breadcrumb trail here should also lead folks to the right solution

#![allow(clippy::disallowed_macros)]

use std::fmt;
use std::io::{Read, Write};
use std::path::Path;
Expand Down
2 changes: 2 additions & 0 deletions misc/bazel/cargo-gazelle/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,8 @@ pub struct BuildScriptContext {
}

impl BuildScriptContext {
// database-issues#9092: anyhow should not be used.
#[allow(clippy::disallowed_macros)]
pub fn generate(
config: &GlobalConfig,
build_script_path: impl AsRef<Path>,
Expand Down
6 changes: 6 additions & 0 deletions misc/bazel/cargo-gazelle/src/targets.rs
Original file line number Diff line number Diff line change
Expand Up @@ -289,6 +289,8 @@ impl RustTarget for RustBinary {
}

impl RustBinary {
// database-issues#9092: anyhow should not be used.
#[allow(clippy::disallowed_macros)]
pub fn generate(
config: &GlobalConfig,
metadata: &PackageMetadata,
Expand Down Expand Up @@ -575,6 +577,8 @@ impl RustTest {
)
}

// database-issues#9092: anyhow should not be used.
#[allow(clippy::disallowed_macros)]
pub fn integration(
config: &GlobalConfig,
metadata: &PackageMetadata,
Expand Down Expand Up @@ -818,6 +822,8 @@ impl RustTarget for CargoBuildScript {
}

impl CargoBuildScript {
// database-issues#9092: anyhow should not be used.
#[allow(clippy::disallowed_macros)]
pub fn generate(
config: &GlobalConfig,
context: &CrateContext,
Expand Down
4 changes: 4 additions & 0 deletions src/adapter/src/active_compute_sink.rs
Original file line number Diff line number Diff line change
Expand Up @@ -413,6 +413,8 @@ impl ActiveCopyTo {
///
/// Either this method or `retire` must be called on every copy to before it
/// is dropped.
// database-issues#9092: anyhow should not be used.
#[allow(clippy::disallowed_macros)]
pub fn retire_with_response(self, response: Result<u64, anyhow::Error>) {
let response = match response {
Ok(n) => Ok(ExecuteResponse::Copied(usize::cast_from(n))),
Expand All @@ -425,6 +427,8 @@ impl ActiveCopyTo {
///
/// Either this method or `retire_with_response` must be called on every
/// copy to before it is dropped.
// database-issues#9092: anyhow should not be used.
#[allow(clippy::disallowed_macros)]
pub fn retire(self, reason: ActiveComputeSinkRetireReason) {
let message = match reason {
ActiveComputeSinkRetireReason::Finished => return,
Expand Down
2 changes: 2 additions & 0 deletions src/adapter/src/catalog.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1000,6 +1000,8 @@ impl Catalog {
}
}

// database-issues#9092: anyhow should not be used.
#[allow(clippy::disallowed_macros)]
pub fn active_cluster(&self, session: &Session) -> Result<&Cluster, AdapterError> {
// TODO(benesch): this check here is not sufficiently protective. It'd
// be very easy for a code path to accidentally avoid this check by
Expand Down
2 changes: 2 additions & 0 deletions src/adapter/src/catalog/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -991,6 +991,8 @@ impl CatalogState {

/// Parses the given SQL string into a `CatalogItem`.
#[mz_ore::instrument]
// database-issues#9092: anyhow should not be used.
#[allow(clippy::disallowed_macros)]
pub(crate) fn parse_item(
&self,
global_id: GlobalId,
Expand Down
2 changes: 2 additions & 0 deletions src/adapter/src/catalog/transact.rs
Original file line number Diff line number Diff line change
Expand Up @@ -369,6 +369,8 @@ impl Catalog {
}

#[instrument(name = "catalog::transact")]
// database-issues#9092: anyhow should not be used.
#[allow(clippy::disallowed_macros)]
pub async fn transact(
&mut self,
// n.b. this is an option to prevent us from needing to build out a
Expand Down
4 changes: 4 additions & 0 deletions src/adapter/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -354,6 +354,8 @@ Issue a SQL query to get started. Need help?

/// Executes a single SQL statement that returns rows as the
/// `mz_support` user.
// database-issues#9092: anyhow should not be used.
#[allow(clippy::disallowed_macros)]
pub async fn support_execute_one(
&self,
sql: &str,
Expand Down Expand Up @@ -406,6 +408,8 @@ Issue a SQL query to get started. Need help?
}

/// Get a metadata and a channel that can be used to append to a webhook source.
// database-issues#9092: anyhow should not be used.
#[allow(clippy::disallowed_macros)]
pub async fn get_webhook_appender(
&self,
database: String,
Expand Down
2 changes: 2 additions & 0 deletions src/adapter/src/config/frontend.rs
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,8 @@ async fn ld_client(sync_config: &SystemParameterSyncConfig) -> Result<ld::Client
Ok(ld_client)
}

// database-issues#9092: anyhow should not be used.
#[allow(clippy::disallowed_macros)]
fn ld_ctx(
env_id: &EnvironmentId,
build_info: &'static BuildInfo,
Expand Down
2 changes: 2 additions & 0 deletions src/adapter/src/coord.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3888,6 +3888,8 @@ impl Drop for LastMessage {
/// BOXED FUTURE: As of Nov 2023 the returned Future from this function was 42KB. This would
/// get stored on the stack which is bad for runtime performance, and blow up our stack usage.
/// Because of that we purposefully move this Future onto the heap (i.e. Box it).
// database-issues#9092: anyhow should not be used.
#[allow(clippy::disallowed_macros)]
pub fn serve(
Config {
controller_config,
Expand Down
4 changes: 4 additions & 0 deletions src/adapter/src/coord/ddl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,8 @@ impl Coordinator {
/// called after this function successfully returns on any built
/// [`DataflowDesc`](mz_compute_types::dataflows::DataflowDesc).
#[instrument(name = "coord::catalog_transact_inner")]
// database-issues#9092: anyhow should not be used.
#[allow(clippy::disallowed_macros)]
pub(crate) async fn catalog_transact_inner<'a>(
&mut self,
conn_id: Option<&ConnectionId>,
Expand Down Expand Up @@ -1164,6 +1166,8 @@ impl Coordinator {
self.drop_sources(source_ids)
}

// database-issues#9092: anyhow should not be used.
#[allow(clippy::disallowed_macros)]
fn drop_vpc_endpoints_in_background(&self, vpc_endpoints: Vec<CatalogItemId>) {
let cloud_resource_controller = Arc::clone(self.cloud_resource_controller
.as_ref()
Expand Down
2 changes: 2 additions & 0 deletions src/adapter/src/coord/introspection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -519,6 +519,8 @@ pub(super) struct SubscribeSpec {
}

impl SubscribeSpec {
// database-issues#9092: anyhow should not be used.
#![allow(clippy::disallowed_macros)]
fn to_plan(&self, catalog: &dyn SessionCatalog) -> Result<SubscribePlan, anyhow::Error> {
let parsed = mz_sql::parse::parse(self.sql)?.into_element();
let (stmt, resolved_ids) = mz_sql::names::resolve(catalog, parsed.ast)?;
Expand Down
6 changes: 6 additions & 0 deletions src/adapter/src/coord/sequencer/inner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -770,6 +770,8 @@ impl Coordinator {
}

#[instrument]
// database-issues#9092: anyhow should not be used.
#[allow(clippy::disallowed_macros)]
pub(super) async fn sequence_create_connection(
&mut self,
mut ctx: ExecuteContext,
Expand Down Expand Up @@ -1764,6 +1766,8 @@ impl Coordinator {
Ok(ExecuteResponse::DroppedOwned)
}

// database-issues#9092: anyhow should not be used.
#[allow(clippy::disallowed_macros)]
fn sequence_drop_common(
&self,
session: &Session,
Expand Down Expand Up @@ -2797,6 +2801,8 @@ impl Coordinator {
/// or read-then-writes can occur between the Peek and SendDiff otherwise a
/// serializability violation could occur.
#[instrument]
// database-issues#9092: anyhow should not be used.
#[allow(clippy::disallowed_macros)]
pub(super) async fn sequence_read_then_write(
&mut self,
mut ctx: ExecuteContext,
Expand Down
2 changes: 2 additions & 0 deletions src/adapter/src/coord/sequencer/inner/cluster.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,8 @@ impl Staged for ClusterStage {
}
}

// database-issues#9092: anyhow should not be used.
#[allow(clippy::disallowed_macros)]
impl Coordinator {
#[instrument]
pub(crate) async fn sequence_alter_cluster_staged(
Expand Down
4 changes: 4 additions & 0 deletions src/adapter/src/coord/sequencer/inner/copy_from.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ use crate::session::{TransactionOps, WriteOp};
use crate::{AdapterError, ExecuteContext, ExecuteResponse};

impl Coordinator {
// database-issues#9092: anyhow should not be used.
#[allow(clippy::disallowed_macros)]
pub(crate) async fn sequence_copy_from(
&mut self,
ctx: ExecuteContext,
Expand Down Expand Up @@ -212,6 +214,8 @@ impl Coordinator {
.await;
}

// database-issues#9092: anyhow should not be used.
#[allow(clippy::disallowed_macros)]
pub(crate) fn commit_staged_batches(
&mut self,
conn_id: ConnectionId,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -877,6 +877,8 @@ impl Coordinator {
Ok(StageResult::Response(Self::send_immediate_rows(rows)))
}

// database-issues#9092: anyhow should not be used.
#[allow(clippy::disallowed_macros)]
pub(crate) async fn explain_pushdown_materialized_view(
&self,
ctx: ExecuteContext,
Expand Down
2 changes: 2 additions & 0 deletions src/adapter/src/coord/sequencer/inner/secret.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,8 @@ impl Coordinator {
)))
}

// database-issues#9092: anyhow should not be used.
#[allow(clippy::disallowed_macros)]
fn extract_secret(
&self,
session: &Session,
Expand Down
6 changes: 6 additions & 0 deletions src/adapter/src/coord/timestamp_selection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,8 @@ pub trait TimestampProvider {
}

/// Determines the timestamp for a query using the classical logic (as opposed to constraint-based).
// database-issues#9092: anyhow should not be used.
#[allow(clippy::disallowed_macros)]
fn determine_timestamp_classical(
&self,
session: &Session,
Expand Down Expand Up @@ -407,6 +409,8 @@ pub trait TimestampProvider {
/// Uses constraints and preferences to determine a timestamp for a query.
/// Returns the determined timestamp, the constraints that were applied, and
/// session_oracle_read_ts.
// database-issues#9092: anyhow should not be used.
#[allow(clippy::disallowed_macros)]
fn determine_timestamp_via_constraints(
&self,
session: &Session,
Expand Down Expand Up @@ -937,6 +941,8 @@ impl Coordinator {
}
}

// database-issues#9092: anyhow should not be used.
#[allow(clippy::disallowed_macros)]
pub(crate) fn evaluate_when(
catalog: &CatalogState,
mut timestamp: MirScalarExpr,
Expand Down
2 changes: 2 additions & 0 deletions src/adapter/src/explain/optimizer_trace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,8 @@ impl OptimizerTrace {

/// Convert the optimizer trace into a vector or rows that can be returned
/// to the client.
// database-issues#9092: anyhow should not be used.
#[allow(clippy::disallowed_macros)]
pub async fn into_rows(
self,
format: ExplainFormat,
Expand Down
2 changes: 2 additions & 0 deletions src/adapter/src/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -745,6 +745,8 @@ impl<T: TimestampManipulation> Session<T> {
}

/// Creates and installs a new portal.
// database-issues#9092: anyhow should not be used.
#[allow(clippy::disallowed_macros)]
pub fn create_new_portal(
&mut self,
stmt: Option<Statement<Raw>>,
Expand Down
12 changes: 12 additions & 0 deletions src/arrow-util/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ pub struct ArrowBuilder {

impl ArrowBuilder {
/// Helper to validate that a RelationDesc can be encoded into Arrow.
// database-issues#9092: anyhow should not be used.
#[allow(clippy::disallowed_macros)]
pub fn validate_desc(desc: &RelationDesc) -> Result<(), anyhow::Error> {
let mut errs = vec![];
for (col_name, col_type) in desc.iter() {
Expand All @@ -54,6 +56,8 @@ impl ArrowBuilder {
/// the number of values that can be appended to each column before reallocating.
/// `data_capacity` is used to initialize the buffer size of the string and binary builders.
/// Errors if the relation contains an unimplemented type.
// database-issues#9092: anyhow should not be used.
#[allow(clippy::disallowed_macros)]
pub fn new(
desc: &RelationDesc,
item_capacity: usize,
Expand Down Expand Up @@ -138,6 +142,8 @@ impl ArrowBuilder {
/// Return the appropriate Arrow DataType for the given ScalarType, plus a string
/// that should be used as part of the Arrow 'Extension Type' name for fields using
/// this type: <https://arrow.apache.org/docs/format/Columnar.html#extension-types>
// database-issues#9092: anyhow should not be used.
#[allow(clippy::disallowed_macros)]
fn scalar_to_arrow_datatype(scalar_type: &ScalarType) -> Result<(DataType, String), anyhow::Error> {
let (data_type, extension_name) = match scalar_type {
ScalarType::Bool => (DataType::Boolean, "boolean"),
Expand Down Expand Up @@ -288,6 +294,8 @@ fn scalar_to_arrow_datatype(scalar_type: &ScalarType) -> Result<(DataType, Strin
Ok((data_type, extension_name.to_lowercase()))
}

// database-issues#9092: anyhow should not be used.
#[allow(clippy::disallowed_macros)]
fn builder_for_datatype(
data_type: &DataType,
item_capacity: usize,
Expand Down Expand Up @@ -439,6 +447,8 @@ fn field_with_typename(
}

/// Extract the materialize 'type name' from the metadata of a Field.
// database-issues#9092: anyhow should not be used.
#[allow(clippy::disallowed_macros)]
fn typename_from_field(field: &Field) -> Result<String, anyhow::Error> {
let metadata = field.metadata();
let extension_name = metadata
Expand Down Expand Up @@ -581,6 +591,8 @@ make_col_builder!(
);

impl ArrowColumn {
// database-issues#9092: anyhow should not be used.
#[allow(clippy::disallowed_macros)]
fn append_datum(&mut self, datum: Datum) -> Result<(), anyhow::Error> {
match (&mut self.inner, datum) {
(s, Datum::Null) => s.append_null(),
Expand Down
6 changes: 6 additions & 0 deletions src/arrow-util/src/reader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,8 @@ impl ArrowReader {
/// columns, perform some lightweight casting, and matching not on column name but column
/// position.
/// TODO(cf2): Allow specifying an optional `arrow::Schema` for extra metadata.
// database-issues#9092: anyhow should not be used.
#[allow(clippy::disallowed_macros)]
pub fn new(desc: &RelationDesc, array: StructArray) -> Result<Self, anyhow::Error> {
let inner_columns = array.columns();
let desc_columns = desc.typ().columns();
Expand Down Expand Up @@ -113,6 +115,8 @@ impl ArrowReader {
}
}

// database-issues#9092: anyhow should not be used.
#[allow(clippy::disallowed_macros)]
fn scalar_type_and_array_to_reader(
scalar_type: &ScalarType,
array: Arc<dyn Array>,
Expand Down Expand Up @@ -448,6 +452,8 @@ enum ColReader {
}

impl ColReader {
// database-issues#9092: anyhow should not be used.
#[allow(clippy::disallowed_macros)]
fn read(&self, idx: usize, packer: &mut RowPacker) -> Result<(), anyhow::Error> {
let datum = match self {
ColReader::Boolean(array) => array.is_valid(idx).then(|| array.value(idx)).map(|x| {
Expand Down
2 changes: 2 additions & 0 deletions src/aws-secrets-controller/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,8 @@ impl AwsSecretsClient {

#[async_trait]
impl SecretsReader for AwsSecretsClient {
// database-issues#9092: anyhow should not be used.
#[allow(clippy::disallowed_macros)]
async fn read(&self, id: CatalogItemId) -> Result<Vec<u8>, anyhow::Error> {
let op_id = Uuid::new_v4();
info!(secret_id = %id, %op_id, "reading secret from AWS");
Expand Down
2 changes: 2 additions & 0 deletions src/aws-util/src/s3.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ pub fn new_presigned_config() -> PresigningConfig {
PresigningConfig::expires_in(Duration::from_secs(5 * 60)).expect("known valid")
}

// database-issues#9092: anyhow should not be used.
#[allow(clippy::disallowed_macros)]
pub async fn list_bucket_path(
client: &Client,
bucket: &str,
Expand Down
Loading