This is an automated email from the ASF dual-hosted git repository.
Jefffrey pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow-rs.git
The following commit(s) were added to refs/heads/main by this push:
new a5c18c2ee8 Enable `allow_attributes` lint for flight and variant
compute (#10680)
a5c18c2ee8 is described below
commit a5c18c2ee8310631b356e8d2c85a5af09654c98f
Author: WaterWhisperer <[email protected]>
AuthorDate: Fri Aug 14 17:02:21 2026 +0800
Enable `allow_attributes` lint for flight and variant compute (#10680)
# Which issue does this PR close?
<!--
We generally require a GitHub issue to be filed for all bug fixes and
enhancements and this helps us generate change logs for our releases.
You can link an issue to this PR using the GitHub syntax.
-->
- Part of #10458.
# Rationale for this change
<!--
Why are you proposing this change? If this is already explained clearly
in the issue then this section is not needed.
Explaining clearly why changes are proposed helps reviewers understand
your changes and offer better suggestions for fixes.
-->
Enable `clippy::allow_attributes` for two crates
# What changes are included in this PR?
<!--
There is no need to duplicate the description in the issue here but it
is sometimes worth providing a summary of the individual changes in this
PR.
-->
- Enable `clippy::allow_attributes` for `arrow-flight` and
`parquet-variant-compute`.
- Replace active `allow` with `expect` attributes.
# Are these changes tested?
<!--
We typically require tests for all PRs in order to:
1. Prevent the code from being accidentally broken by subsequent changes
2. Serve as another way to document the expected behavior of the code
If tests are not included in your PR, please explain why (for example,
are they covered by existing tests)?
If this PR claims a performance improvement, please include evidence
such as benchmark results.
-->
Yes.
`cargo clippy --workspace --all-targets --all-features -- -D warnings`
# Are there any user-facing changes?
<!--
If there are user-facing changes then we may require documentation to be
updated before approving the PR.
If there are any breaking changes to public APIs, please call them out.
-->
No.
---
arrow-flight/benches/common/mod.rs | 8 --------
arrow-flight/examples/flight_sql_server.rs | 1 -
arrow-flight/src/encode.rs | 2 +-
arrow-flight/src/lib.rs | 4 +++-
arrow-flight/src/sql/mod.rs | 3 ++-
arrow-flight/tests/common/fixture.rs | 3 +++
arrow-flight/tests/common/server.rs | 23 +++++++++++++++++++++++
arrow-flight/tests/common/utils.rs | 3 +++
parquet-variant-compute/src/arrow_to_variant.rs | 2 +-
parquet-variant-compute/src/lib.rs | 2 ++
parquet-variant-compute/src/variant_array.rs | 1 -
parquet-variant-compute/src/variant_to_arrow.rs | 1 +
12 files changed, 39 insertions(+), 14 deletions(-)
diff --git a/arrow-flight/benches/common/mod.rs
b/arrow-flight/benches/common/mod.rs
index b716d3f31f..3acb384907 100644
--- a/arrow-flight/benches/common/mod.rs
+++ b/arrow-flight/benches/common/mod.rs
@@ -81,13 +81,6 @@ pub struct BenchServer {
frames: Arc<RwLock<Vec<FlightData>>>,
}
-impl BenchServer {
- #[allow(dead_code)]
- pub fn set_frames(&self, frames: Vec<FlightData>) {
- *self.frames.write().unwrap() = frames;
- }
-}
-
fn unimpl<T>() -> Result<T, Status> {
Err(Status::unimplemented(""))
}
@@ -126,7 +119,6 @@ impl FlightService for BenchServer {
async fn do_action(&self, _: Request<Action>) ->
Result<Response<Self::DoActionStream>, Status> { unimpl() }
async fn list_actions(&self, _: Request<Empty>) ->
Result<Response<Self::ListActionsStream>, Status> { unimpl() }
}
-#[allow(dead_code)]
pub async fn start_server() -> (Channel, BenchServer) {
const DUMMY_URL: &str = "http://localhost:50051";
diff --git a/arrow-flight/examples/flight_sql_server.rs
b/arrow-flight/examples/flight_sql_server.rs
index ae03cac285..f0d08b4057 100644
--- a/arrow-flight/examples/flight_sql_server.rs
+++ b/arrow-flight/examples/flight_sql_server.rs
@@ -112,7 +112,6 @@ static TABLES: Lazy<Vec<&'static str>> = Lazy::new(||
vec!["flight_sql.example.t
pub struct FlightSqlServiceImpl {}
impl FlightSqlServiceImpl {
- #[allow(clippy::result_large_err)]
fn check_token<T>(&self, req: &Request<T>) -> Result<(), Status> {
let metadata = req.metadata();
let auth = metadata.get("authorization").ok_or_else(|| {
diff --git a/arrow-flight/src/encode.rs b/arrow-flight/src/encode.rs
index 437d910deb..29d3eb105c 100644
--- a/arrow-flight/src/encode.rs
+++ b/arrow-flight/src/encode.rs
@@ -551,7 +551,7 @@ fn prepare_field_for_flight(
send_dictionaries,
);
dictionary_tracker.next_dict_id();
- #[allow(deprecated)]
+ #[expect(deprecated)]
Field::new_dict(
field.name(),
field.data_type().clone(),
diff --git a/arrow-flight/src/lib.rs b/arrow-flight/src/lib.rs
index 94b449425d..7a87b2338c 100644
--- a/arrow-flight/src/lib.rs
+++ b/arrow-flight/src/lib.rs
@@ -42,6 +42,7 @@
html_favicon_url =
"https://arrow.apache.org/img/arrow-logo_chevrons_black-txt_transparent-bg.svg"
)]
#![cfg_attr(docsrs, feature(doc_cfg))]
+#![deny(clippy::allow_attributes)]
#![allow(rustdoc::invalid_html_tags)]
#![warn(missing_docs)]
// The unused_crate_dependencies lint does not work well for crates defining
additional examples/bin targets
@@ -59,9 +60,10 @@ use std::{fmt, ops::Deref};
type ArrowResult<T> = std::result::Result<T, ArrowError>;
+// This code is generated so we don't want to fix any lint violations manually
+#[allow(clippy::allow_attributes)]
#[allow(clippy::all)]
mod r#gen {
- // Since this file is auto-generated, we suppress all warnings
#![allow(missing_docs)]
include!("arrow.flight.protocol.rs");
}
diff --git a/arrow-flight/src/sql/mod.rs b/arrow-flight/src/sql/mod.rs
index 1bf7b1a1b3..39ea0f1edf 100644
--- a/arrow-flight/src/sql/mod.rs
+++ b/arrow-flight/src/sql/mod.rs
@@ -42,9 +42,10 @@ use arrow_schema::ArrowError;
use bytes::Bytes;
use prost::Message;
+// This code is generated so we don't want to fix any lint violations manually
+#[allow(clippy::allow_attributes)]
#[allow(clippy::all)]
mod r#gen {
- // Since this file is auto-generated, we suppress all warnings
#![allow(missing_docs)]
include!("arrow.flight.protocol.sql.rs");
}
diff --git a/arrow-flight/tests/common/fixture.rs
b/arrow-flight/tests/common/fixture.rs
index a666fa5d0d..a6c0808722 100644
--- a/arrow-flight/tests/common/fixture.rs
+++ b/arrow-flight/tests/common/fixture.rs
@@ -41,6 +41,7 @@ pub struct TestFixture {
impl TestFixture {
/// create a new test fixture from the server
+ #[expect(clippy::allow_attributes)] // some issue where expect(dead_code)
doesn't fire properly
#[allow(dead_code)]
pub async fn new<T: FlightService>(test_server: FlightServiceServer<T>) ->
Self {
// let OS choose a free port
@@ -78,6 +79,7 @@ impl TestFixture {
}
/// Return a [`Channel`] connected to the TestServer
+ #[expect(clippy::allow_attributes)] // some issue where expect(dead_code)
doesn't fire properly
#[allow(dead_code)]
pub async fn channel(&self) -> Channel {
let url = format!("http://{}", self.addr);
@@ -90,6 +92,7 @@ impl TestFixture {
}
/// Stops the test server and waits for the server to shutdown
+ #[expect(clippy::allow_attributes)] // some issue where expect(dead_code)
doesn't fire properly
#[allow(dead_code)]
pub async fn shutdown_and_wait(mut self) {
if let Some(shutdown) = self.shutdown.take() {
diff --git a/arrow-flight/tests/common/server.rs
b/arrow-flight/tests/common/server.rs
index 5aa22a8696..11e24ab387 100644
--- a/arrow-flight/tests/common/server.rs
+++ b/arrow-flight/tests/common/server.rs
@@ -38,6 +38,7 @@ pub struct TestFlightServer {
impl TestFlightServer {
/// Create a `TestFlightServer`
+ #[expect(clippy::allow_attributes)] // some issue where expect(dead_code)
doesn't fire properly
#[allow(dead_code)]
pub fn new() -> Self {
Self {
@@ -47,6 +48,7 @@ impl TestFlightServer {
/// Return an [`FlightServiceServer`] that can be used with a
/// [`Server`](tonic::transport::Server)
+ #[expect(clippy::allow_attributes)] // some issue where expect(dead_code)
doesn't fire properly
#[allow(dead_code)]
pub fn service(&self) -> FlightServiceServer<TestFlightServer> {
// wrap up tonic goop
@@ -54,6 +56,7 @@ impl TestFlightServer {
}
/// Specify the response returned from the next call to handshake
+ #[expect(clippy::allow_attributes)] // some issue where expect(dead_code)
doesn't fire properly
#[allow(dead_code)]
pub fn set_handshake_response(&self, response: Result<HandshakeResponse,
Status>) {
let mut state = self.state.lock().expect("mutex not poisoned");
@@ -61,6 +64,7 @@ impl TestFlightServer {
}
/// Take and return last handshake request sent to the server,
+ #[expect(clippy::allow_attributes)] // some issue where expect(dead_code)
doesn't fire properly
#[allow(dead_code)]
pub fn take_handshake_request(&self) -> Option<HandshakeRequest> {
self.state
@@ -71,6 +75,7 @@ impl TestFlightServer {
}
/// Specify the response returned from the next call to get_flight_info
+ #[expect(clippy::allow_attributes)] // some issue where expect(dead_code)
doesn't fire properly
#[allow(dead_code)]
pub fn set_get_flight_info_response(&self, response: Result<FlightInfo,
Status>) {
let mut state = self.state.lock().expect("mutex not poisoned");
@@ -78,6 +83,7 @@ impl TestFlightServer {
}
/// Take and return last get_flight_info request sent to the server,
+ #[expect(clippy::allow_attributes)] // some issue where expect(dead_code)
doesn't fire properly
#[allow(dead_code)]
pub fn take_get_flight_info_request(&self) -> Option<FlightDescriptor> {
self.state
@@ -88,6 +94,7 @@ impl TestFlightServer {
}
/// Specify the response returned from the next call to poll_flight_info
+ #[expect(clippy::allow_attributes)] // some issue where expect(dead_code)
doesn't fire properly
#[allow(dead_code)]
pub fn set_poll_flight_info_response(&self, response: Result<PollInfo,
Status>) {
let mut state = self.state.lock().expect("mutex not poisoned");
@@ -95,6 +102,7 @@ impl TestFlightServer {
}
/// Take and return last poll_flight_info request sent to the server,
+ #[expect(clippy::allow_attributes)] // some issue where expect(dead_code)
doesn't fire properly
#[allow(dead_code)]
pub fn take_poll_flight_info_request(&self) -> Option<FlightDescriptor> {
self.state
@@ -105,6 +113,7 @@ impl TestFlightServer {
}
/// Specify the response returned from the next call to `do_get`
+ #[expect(clippy::allow_attributes)] // some issue where expect(dead_code)
doesn't fire properly
#[allow(dead_code)]
pub fn set_do_get_response(&self, response: Vec<Result<RecordBatch,
Status>>) {
let mut state = self.state.lock().expect("mutex not poisoned");
@@ -112,6 +121,7 @@ impl TestFlightServer {
}
/// Take and return last do_get request send to the server,
+ #[expect(clippy::allow_attributes)] // some issue where expect(dead_code)
doesn't fire properly
#[allow(dead_code)]
pub fn take_do_get_request(&self) -> Option<Ticket> {
self.state
@@ -122,6 +132,7 @@ impl TestFlightServer {
}
/// Specify the response returned from the next call to `do_put`
+ #[expect(clippy::allow_attributes)] // some issue where expect(dead_code)
doesn't fire properly
#[allow(dead_code)]
pub fn set_do_put_response(&self, response: Vec<Result<PutResult,
Status>>) {
let mut state = self.state.lock().expect("mutex not poisoned");
@@ -129,6 +140,7 @@ impl TestFlightServer {
}
/// Take and return last do_put request sent to the server,
+ #[expect(clippy::allow_attributes)] // some issue where expect(dead_code)
doesn't fire properly
#[allow(dead_code)]
pub fn take_do_put_request(&self) -> Option<Vec<FlightData>> {
self.state
@@ -139,6 +151,7 @@ impl TestFlightServer {
}
/// Specify the response returned from the next call to `do_exchange`
+ #[expect(clippy::allow_attributes)] // some issue where expect(dead_code)
doesn't fire properly
#[allow(dead_code)]
pub fn set_do_exchange_response(&self, response: Vec<Result<FlightData,
Status>>) {
let mut state = self.state.lock().expect("mutex not poisoned");
@@ -146,6 +159,7 @@ impl TestFlightServer {
}
/// Take and return last do_exchange request send to the server,
+ #[expect(clippy::allow_attributes)] // some issue where expect(dead_code)
doesn't fire properly
#[allow(dead_code)]
pub fn take_do_exchange_request(&self) -> Option<Vec<FlightData>> {
self.state
@@ -156,6 +170,7 @@ impl TestFlightServer {
}
/// Specify the response returned from the next call to `list_flights`
+ #[expect(clippy::allow_attributes)] // some issue where expect(dead_code)
doesn't fire properly
#[allow(dead_code)]
pub fn set_list_flights_response(&self, response: Vec<Result<FlightInfo,
Status>>) {
let mut state = self.state.lock().expect("mutex not poisoned");
@@ -163,6 +178,7 @@ impl TestFlightServer {
}
/// Take and return last list_flights request send to the server,
+ #[expect(clippy::allow_attributes)] // some issue where expect(dead_code)
doesn't fire properly
#[allow(dead_code)]
pub fn take_list_flights_request(&self) -> Option<Criteria> {
self.state
@@ -173,6 +189,7 @@ impl TestFlightServer {
}
/// Specify the response returned from the next call to `get_schema`
+ #[expect(clippy::allow_attributes)] // some issue where expect(dead_code)
doesn't fire properly
#[allow(dead_code)]
pub fn set_get_schema_response(&self, response: Result<Schema, Status>) {
let mut state = self.state.lock().expect("mutex not poisoned");
@@ -180,6 +197,7 @@ impl TestFlightServer {
}
/// Take and return last get_schema request send to the server,
+ #[expect(clippy::allow_attributes)] // some issue where expect(dead_code)
doesn't fire properly
#[allow(dead_code)]
pub fn take_get_schema_request(&self) -> Option<FlightDescriptor> {
self.state
@@ -190,6 +208,7 @@ impl TestFlightServer {
}
/// Specify the response returned from the next call to `list_actions`
+ #[expect(clippy::allow_attributes)] // some issue where expect(dead_code)
doesn't fire properly
#[allow(dead_code)]
pub fn set_list_actions_response(&self, response: Vec<Result<ActionType,
Status>>) {
let mut state = self.state.lock().expect("mutex not poisoned");
@@ -197,6 +216,7 @@ impl TestFlightServer {
}
/// Take and return last list_actions request send to the server,
+ #[expect(clippy::allow_attributes)] // some issue where expect(dead_code)
doesn't fire properly
#[allow(dead_code)]
pub fn take_list_actions_request(&self) -> Option<Empty> {
self.state
@@ -207,6 +227,7 @@ impl TestFlightServer {
}
/// Specify the response returned from the next call to `do_action`
+ #[expect(clippy::allow_attributes)] // some issue where expect(dead_code)
doesn't fire properly
#[allow(dead_code)]
pub fn set_do_action_response(&self, response:
Vec<Result<arrow_flight::Result, Status>>) {
let mut state = self.state.lock().expect("mutex not poisoned");
@@ -214,6 +235,7 @@ impl TestFlightServer {
}
/// Take and return last do_action request send to the server,
+ #[expect(clippy::allow_attributes)] // some issue where expect(dead_code)
doesn't fire properly
#[allow(dead_code)]
pub fn take_do_action_request(&self) -> Option<Action> {
self.state
@@ -224,6 +246,7 @@ impl TestFlightServer {
}
/// Returns the last metadata from a request received by the server
+ #[expect(clippy::allow_attributes)] // some issue where expect(dead_code)
doesn't fire properly
#[allow(dead_code)]
pub fn take_last_request_metadata(&self) -> Option<MetadataMap> {
self.state
diff --git a/arrow-flight/tests/common/utils.rs
b/arrow-flight/tests/common/utils.rs
index f36b41cba3..4d11571251 100644
--- a/arrow-flight/tests/common/utils.rs
+++ b/arrow-flight/tests/common/utils.rs
@@ -30,6 +30,7 @@ use arrow_schema::{DataType, Field, Schema};
/// Example:
/// i: 0, 1, None, 3, 4
/// f: 5.0, 4.0, None, 2.0, 1.0
+#[expect(clippy::allow_attributes)] // some issue where expect(dead_code)
doesn't fire properly
#[allow(dead_code)]
pub fn make_primitive_batch(num_rows: usize) -> RecordBatch {
let i: UInt8Array = (0..num_rows)
@@ -59,6 +60,7 @@ pub fn make_primitive_batch(num_rows: usize) -> RecordBatch {
///
/// Example:
/// a: value0, value1, value2, None, value1, value2
+#[expect(clippy::allow_attributes)] // some issue where expect(dead_code)
doesn't fire properly
#[allow(dead_code)]
pub fn make_dictionary_batch(num_rows: usize) -> RecordBatch {
let values: Vec<_> = (0..num_rows)
@@ -81,6 +83,7 @@ pub fn make_dictionary_batch(num_rows: usize) -> RecordBatch {
RecordBatch::try_from_iter(vec![("a", Arc::new(a) as ArrayRef)]).unwrap()
}
+#[expect(clippy::allow_attributes)] // some issue where expect(dead_code)
doesn't fire properly
#[allow(dead_code)]
pub fn make_view_batches(num_rows: usize) -> RecordBatch {
const LONG_TEST_STRING: &str =
diff --git a/parquet-variant-compute/src/arrow_to_variant.rs
b/parquet-variant-compute/src/arrow_to_variant.rs
index b9e7fff064..a4b06cbbe9 100644
--- a/parquet-variant-compute/src/arrow_to_variant.rs
+++ b/parquet-variant-compute/src/arrow_to_variant.rs
@@ -353,7 +353,7 @@ macro_rules! define_row_builder {
// legitimate compiler warnings if an infallible value
transform fails to use
// its first extra field.
$(
- #[allow(unused)]
+ #[expect(unused)]
$( let $field = &self.$field; )+
)?
diff --git a/parquet-variant-compute/src/lib.rs
b/parquet-variant-compute/src/lib.rs
index f1e87661c8..46d04b8c66 100644
--- a/parquet-variant-compute/src/lib.rs
+++ b/parquet-variant-compute/src/lib.rs
@@ -15,6 +15,8 @@
// specific language governing permissions and limitations
// under the License.
+#![deny(clippy::allow_attributes)]
+
//! [`VariantArray`] and compute kernels for the [Variant Binary Encoding]
from [Apache Parquet].
//!
//! ## Main APIs
diff --git a/parquet-variant-compute/src/variant_array.rs
b/parquet-variant-compute/src/variant_array.rs
index 7af504572e..8eec681530 100644
--- a/parquet-variant-compute/src/variant_array.rs
+++ b/parquet-variant-compute/src/variant_array.rs
@@ -697,7 +697,6 @@ pub struct ShreddedVariantFieldArray {
shredding_state: ShreddingState,
}
-#[allow(unused)]
impl ShreddedVariantFieldArray {
/// Creates a new `ShreddedVariantFieldArray` from a [`StructArray`].
///
diff --git a/parquet-variant-compute/src/variant_to_arrow.rs
b/parquet-variant-compute/src/variant_to_arrow.rs
index a6f32288eb..038680a9e1 100644
--- a/parquet-variant-compute/src/variant_to_arrow.rs
+++ b/parquet-variant-compute/src/variant_to_arrow.rs
@@ -1008,6 +1008,7 @@ macro_rules! define_variant_to_primitive_builder {
// Add this to silence unused mut warning from macro-generated code
// This is mainly for `FakeNullBuilder`
+ #[expect(clippy::allow_attributes)]
#[allow(unused_mut)]
fn finish(mut self) -> Result<ArrayRef> {
// If the builder produces T: Array, the compiler infers
`<Arc<T> as From<T>>::from`