Skip to content
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
1 change: 1 addition & 0 deletions datafusion/catalog-listing/src/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
// specific language governing permissions and limitations
// under the License.

#![deny(clippy::allow_attributes)]
#![cfg_attr(test, allow(clippy::needless_pass_by_value))]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is removing the existing allow for needless_pass_by_value in tests intentional?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, the CI was failing with "unfulfilled lint expectations" because the needless_pass_by_value lint wasn't actually being triggered in the test code.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We might need to think of a way to keep this, as the intention is that for any future tests we have this lint disabled

#![doc(
html_logo_url = "https://raw.githubusercontent.com/apache/datafusion/19fe44cf2f30cbdd63d4a4f52c74055163c6cc38/docs/logos/standalone_logo/logo_original.svg",
Expand Down
3 changes: 3 additions & 0 deletions datafusion/core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
// specific language governing permissions and limitations
// under the License.

#![deny(clippy::allow_attributes)]
#![doc(
html_logo_url = "https://raw.githubusercontent.com/apache/datafusion/19fe44cf2f30cbdd63d4a4f52c74055163c6cc38/docs/logos/standalone_logo/logo_original.svg",
html_favicon_url = "https://raw.githubusercontent.com/apache/datafusion/19fe44cf2f30cbdd63d4a4f52c74055163c6cc38/docs/logos/standalone_logo/logo_original.svg"
Expand All @@ -35,6 +36,8 @@
)
)]
#![warn(missing_docs, clippy::needless_borrow)]
// Use `allow` instead of `expect` for test configuration to explicitly
// disable the lint for all test code rather than expecting violations
#![cfg_attr(test, allow(clippy::needless_pass_by_value))]

//! [DataFusion] is an extensible query engine written in Rust that
Expand Down
1 change: 1 addition & 0 deletions datafusion/doc/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
// specific language governing permissions and limitations
// under the License.

#![deny(clippy::allow_attributes)]
#![cfg_attr(test, allow(clippy::needless_pass_by_value))]
#![doc(
html_logo_url = "https://raw.githubusercontent.com/apache/datafusion/19fe44cf2f30cbdd63d4a4f52c74055163c6cc38/docs/logos/standalone_logo/logo_original.svg",
Expand Down
7 changes: 4 additions & 3 deletions datafusion/execution/src/disk_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,9 +115,10 @@ pub enum DiskManagerMode {
}

/// Configuration for temporary disk access
#[allow(deprecated)]
#[deprecated(since = "48.0.0", note = "Use DiskManagerBuilder instead")]
#[derive(Debug, Clone, Default)]
#[allow(clippy::allow_attributes)]
#[allow(deprecated)]
pub enum DiskManagerConfig {
/// Use the provided [DiskManager] instance
Existing(Arc<DiskManager>),
Expand All @@ -135,7 +136,7 @@ pub enum DiskManagerConfig {
Disabled,
}

#[allow(deprecated)]
#[expect(deprecated)]
impl DiskManagerConfig {
/// Create temporary files in a temporary directory chosen by the OS
pub fn new() -> Self {
Expand Down Expand Up @@ -177,7 +178,7 @@ impl DiskManager {
}

/// Create a DiskManager given the configuration
#[allow(deprecated)]
#[expect(deprecated)]
#[deprecated(since = "48.0.0", note = "Use DiskManager::builder() instead")]
pub fn try_new(config: DiskManagerConfig) -> Result<Arc<Self>> {
match config {
Expand Down
1 change: 1 addition & 0 deletions datafusion/execution/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
// https://github.com/apache/datafusion/issues/11143
#![deny(clippy::clone_on_ref_ptr)]
#![cfg_attr(test, allow(clippy::needless_pass_by_value))]
#![deny(clippy::allow_attributes)]

//! DataFusion execution configuration and runtime structures

Expand Down
2 changes: 1 addition & 1 deletion datafusion/execution/src/object_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ pub trait ObjectStoreRegistry: Send + Sync + std::fmt::Debug + 'static {

/// Deregister the store previously registered with the same key. Returns the
/// deregistered store if it existed.
#[allow(unused_variables)]
#[expect(unused_variables)]
fn deregister_store(&self, url: &Url) -> Result<Arc<dyn ObjectStore>> {
not_impl_err!(
"ObjectStoreRegistry::deregister_store is not implemented for this ObjectStoreRegistry"
Expand Down
10 changes: 5 additions & 5 deletions datafusion/execution/src/runtime_env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
//! Execution [`RuntimeEnv`] environment that manages access to object
//! store, memory manager, disk manager.

#[allow(deprecated)]
#[expect(deprecated)]
use crate::disk_manager::DiskManagerConfig;
use crate::{
disk_manager::{DiskManager, DiskManagerBuilder, DiskManagerMode},
Expand Down Expand Up @@ -279,7 +279,7 @@ impl Default for RuntimeEnv {
/// See example on [`RuntimeEnv`]
#[derive(Clone)]
pub struct RuntimeEnvBuilder {
#[allow(deprecated)]
#[expect(deprecated)]
/// DiskManager to manage temporary disk file usage
pub disk_manager: DiskManagerConfig,
/// DiskManager builder to manager temporary disk file usage
Expand Down Expand Up @@ -317,7 +317,7 @@ impl RuntimeEnvBuilder {
}
}

#[allow(deprecated)]
#[expect(deprecated)]
#[deprecated(since = "48.0.0", note = "Use with_disk_manager_builder instead")]
/// Customize disk manager
pub fn with_disk_manager(mut self, disk_manager: DiskManagerConfig) -> Self {
Expand Down Expand Up @@ -418,7 +418,7 @@ impl RuntimeEnvBuilder {
disk_manager: if let Some(builder) = disk_manager_builder {
Arc::new(builder.build()?)
} else {
#[allow(deprecated)]
#[expect(deprecated)]
DiskManager::try_new(disk_manager)?
},
cache_manager: CacheManager::try_new(&cache_manager)?,
Expand Down Expand Up @@ -451,7 +451,7 @@ impl RuntimeEnvBuilder {
};

Self {
#[allow(deprecated)]
#[expect(deprecated)]
disk_manager: DiskManagerConfig::Existing(Arc::clone(
&runtime_env.disk_manager,
)),
Expand Down
4 changes: 2 additions & 2 deletions datafusion/expr-common/src/dyn_eq.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ use std::hash::{Hash, Hasher};
///
/// Note: This trait should not be implemented directly. Implement `Eq` and `Any` and use
/// the blanket implementation.
#[allow(private_bounds)]
#[expect(private_bounds)]
pub trait DynEq: private::EqSealed {
fn dyn_eq(&self, other: &dyn Any) -> bool;
}
Expand All @@ -45,7 +45,7 @@ impl<T: Eq + Any> DynEq for T {
///
/// Note: This trait should not be implemented directly. Implement `Hash` and `Any` and use
/// the blanket implementation.
#[allow(private_bounds)]
#[expect(private_bounds)]
pub trait DynHash: private::HashSealed {
fn dyn_hash(&self, _state: &mut dyn Hasher);
}
Expand Down
1 change: 1 addition & 0 deletions datafusion/expr-common/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
// https://github.com/apache/datafusion/issues/11143
#![deny(clippy::clone_on_ref_ptr)]
#![cfg_attr(test, allow(clippy::needless_pass_by_value))]
#![deny(clippy::allow_attributes)]

pub mod accumulator;
pub mod casts;
Expand Down
2 changes: 1 addition & 1 deletion datafusion/expr/src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2104,7 +2104,7 @@ impl Expr {

impl Normalizeable for Expr {
fn can_normalize(&self) -> bool {
#[allow(clippy::match_like_matches_macro)]
#[expect(clippy::match_like_matches_macro)]
match self {
Expr::BinaryExpr(BinaryExpr {
op:
Expand Down
1 change: 1 addition & 0 deletions datafusion/expr/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
// https://github.com/apache/datafusion/issues/11143
#![deny(clippy::clone_on_ref_ptr)]
#![cfg_attr(test, allow(clippy::needless_pass_by_value))]
#![deny(clippy::allow_attributes)]

//! [DataFusion](https://github.com/apache/datafusion)
//! is an extensible query execution framework that uses
Expand Down
1 change: 1 addition & 0 deletions datafusion/expr/src/logical_plan/plan.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3235,6 +3235,7 @@ impl PartialOrd for Analyze {
// TODO(clippy): This clippy `allow` should be removed if
// the manual `PartialEq` is removed in favor of a derive.
// (see `PartialEq` the impl for details.)
#[allow(clippy::allow_attributes)]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why not change this

#[expect(clippy::derived_hash_with_manual_eq)]

?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Hi, if we change #[allow(clippy::derived_hash_with_manual_eq)] to
#[expect(clippy::derived_hash_with_manual_eq)], that will be the error:

  error: you are deriving `Hash` but have implemented `PartialEq` explicitly
      --> datafusion/expr/src/logical_plan/plan.rs:3239:28
       |
  3239 | #[derive(Debug, Clone, Eq, Hash)]
       |                            ^^^^
       |
  note: `PartialEq` implemented here
      --> datafusion/expr/src/logical_plan/plan.rs:3248:1
       |
  3248 | impl PartialEq for Extension {
       | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
       = help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.91.0/index.html#derived_hash_with_manual_eq
       = note: `#[deny(clippy::derived_hash_with_manual_eq)]` on by default

  warning: this lint expectation is unfulfilled
      --> datafusion/expr/src/logical_plan/plan.rs:3238:10
       |
  3238 | #[expect(clippy::derived_hash_with_manual_eq)]
       |          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
       |
       = note: `#[warn(unfulfilled_lint_expectations)]` on by default

  warning: `datafusion-expr` (lib) generated 1 warning
  error: could not compile `datafusion-expr` (lib) due to 1 previous error; 1 warning emitted

Seems like expect can't override clippy's default deny-level lint derived_hash_with_manual_eq :(, so maybe we need allow with a policy override here.

#[allow(clippy::derived_hash_with_manual_eq)]
#[derive(Debug, Clone, Eq, Hash)]
pub struct Extension {
Expand Down
2 changes: 1 addition & 1 deletion datafusion/expr/src/ptr_eq.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ pub fn arc_ptr_hash<T: ?Sized>(a: &Arc<T>, hasher: &mut impl Hasher) {
///
/// If you have pointers to a `dyn UDF impl` consider using [`super::udf_eq::UdfEq`].
#[derive(Clone)]
#[allow(private_bounds)] // This is so that PtrEq can only be used with allowed pointer types (e.g. Arc), without allowing misuse.
#[expect(private_bounds)] // This is so that PtrEq can only be used with allowed pointer types (e.g. Arc), without allowing misuse.
pub struct PtrEq<Ptr: PointerType>(Ptr);

impl<T> PartialEq for PtrEq<Arc<T>>
Expand Down
4 changes: 2 additions & 2 deletions datafusion/expr/src/udf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ impl ScalarUDF {

#[deprecated(since = "50.0.0", note = "Use `return_field_from_args` instead.")]
pub fn is_nullable(&self, args: &[Expr], schema: &dyn ExprSchema) -> bool {
#[allow(deprecated)]
#[expect(deprecated)]
self.inner.is_nullable(args, schema)
}

Expand Down Expand Up @@ -902,7 +902,7 @@ impl ScalarUDFImpl for AliasedScalarUDFImpl {
}

fn is_nullable(&self, args: &[Expr], schema: &dyn ExprSchema) -> bool {
#[allow(deprecated)]
#[expect(deprecated)]
self.inner.is_nullable(args, schema)
}

Expand Down
2 changes: 1 addition & 1 deletion datafusion/expr/src/udf_eq.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ use std::sync::Arc;
///
/// If you want to just compare pointers for equality, use [`super::ptr_eq::PtrEq`].
#[derive(Clone)]
#[allow(private_bounds)] // This is so that UdfEq can only be used with allowed pointer types (e.g. Arc), without allowing misuse.
#[expect(private_bounds)] // This is so that UdfEq can only be used with allowed pointer types (e.g. Arc), without allowing misuse.
pub struct UdfEq<Ptr: UdfPointer>(Ptr);

impl<Ptr> PartialEq for UdfEq<Ptr>
Expand Down
2 changes: 0 additions & 2 deletions datafusion/proto-common/src/from_proto/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -930,7 +930,6 @@ impl TryFrom<&protobuf::ParquetOptions> for ParquetOptions {
fn try_from(
value: &protobuf::ParquetOptions,
) -> datafusion_common::Result<Self, Self::Error> {
#[allow(deprecated)] // max_statistics_size
Ok(ParquetOptions {
enable_page_index: value.enable_page_index,
pruning: value.pruning,
Expand Down Expand Up @@ -1015,7 +1014,6 @@ impl TryFrom<&protobuf::ParquetColumnOptions> for ParquetColumnOptions {
fn try_from(
value: &protobuf::ParquetColumnOptions,
) -> datafusion_common::Result<Self, Self::Error> {
#[allow(deprecated)] // max_statistics_size
Ok(ParquetColumnOptions {
compression: value.compression_opt.clone().map(|opt| match opt {
protobuf::parquet_column_options::CompressionOpt::Compression(v) => Some(v),
Expand Down
1 change: 1 addition & 0 deletions datafusion/proto-common/src/generated/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
// specific language governing permissions and limitations
// under the License.

#[allow(clippy::allow_attributes)]
#[allow(clippy::all)]
Comment thread
Jefffrey marked this conversation as resolved.
#[rustfmt::skip]
pub mod datafusion_proto_common {
Expand Down
1 change: 1 addition & 0 deletions datafusion/proto-common/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
// Make sure fast / cheap clones on Arc are explicit:
// https://github.com/apache/datafusion/issues/11143
#![deny(clippy::clone_on_ref_ptr)]
#![deny(clippy::allow_attributes)]

//! Serialize / Deserialize DataFusion Primitive Types to bytes
//!
Expand Down