Skip to content

Commit 638eaf4

Browse files
authored
ci(rust): check MSRV in CI and fix clippy (lancedb#3054)
1 parent 54053e6 commit 638eaf4

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

76 files changed

+216
-159
lines changed

.cargo/config.toml

-18
Original file line numberDiff line numberDiff line change
@@ -9,24 +9,6 @@ debug = true
99
codegen-units = 16
1010
lto = "thin"
1111

12-
[target.'cfg(all())']
13-
rustflags = [
14-
"-Wclippy::all",
15-
"-Wclippy::style",
16-
"-Wclippy::fallible_impl_from",
17-
"-Wclippy::manual_let_else",
18-
"-Wclippy::redundant_pub_crate",
19-
"-Wclippy::string_add_assign",
20-
"-Wclippy::string_add",
21-
"-Wclippy::string_lit_as_bytes",
22-
"-Wclippy::string_to_string",
23-
"-Wclippy::use_self",
24-
"-Dclippy::cargo",
25-
"-Dclippy::dbg_macro",
26-
# not too much we can do to avoid multiple crate versions
27-
"-Aclippy::multiple-crate-versions",
28-
]
29-
3012
[target.x86_64-unknown-linux-gnu]
3113
rustflags = ["-C", "target-cpu=haswell", "-C", "target-feature=+avx2,+fma,+f16c"]
3214

.github/workflows/rust.yml

+35-4
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,8 @@ jobs:
3838
- nightly
3939
env:
4040
# Need up-to-date compilers for kernels
41-
CC: clang-18
42-
CXX: clang-18
41+
CC: clang
42+
CXX: clang
4343
steps:
4444
- uses: actions/checkout@v4
4545
# pin the toolchain version to avoid surprises
@@ -81,7 +81,7 @@ jobs:
8181
- name: Run tests
8282
if: ${{ matrix.toolchain == 'stable' }}
8383
run: |
84-
cargo llvm-cov --workspace --codecov --output-path coverage.codecov --features dynamodb,tensorflow,dynamodb_tests,cli
84+
cargo llvm-cov --workspace --codecov --output-path coverage.codecov --all-features
8585
- name: Run tests (nightly)
8686
if: ${{ matrix.toolchain != 'stable' }}
8787
run: |
@@ -127,6 +127,10 @@ jobs:
127127
clippy_and_benchmark:
128128
runs-on: ubuntu-24.04
129129
timeout-minutes: 30
130+
env:
131+
# Need up-to-date compilers for kernels
132+
CC: clang
133+
CXX: clang
130134
steps:
131135
- uses: actions/checkout@v4
132136
- uses: Swatinem/rust-cache@v2
@@ -137,7 +141,9 @@ jobs:
137141
sudo apt update
138142
sudo apt install -y protobuf-compiler libssl-dev
139143
- name: Run clippy
140-
run: cargo clippy --features cli,dynamodb,tensorflow,dynamodb_tests --tests --benches -- -D warnings
144+
run: |
145+
cargo clippy --version
146+
cargo clippy --all-features --tests --benches -- -D warnings
141147
- name: Build benchmarks
142148
run: cargo build --benches
143149
mac-build:
@@ -196,3 +202,28 @@ jobs:
196202
run: |
197203
cargo build --tests --benches --all-features --workspace
198204
cargo test
205+
msrv:
206+
# Check the minimum supported Rust version
207+
name: MSRV Check - Rust v${{ matrix.msrv }}
208+
runs-on: ubuntu-24.04
209+
strategy:
210+
matrix:
211+
msrv: ["1.78.0"] # This should match up with rust-version in Cargo.toml
212+
env:
213+
# Need up-to-date compilers for kernels
214+
CC: clang
215+
CXX: clang
216+
steps:
217+
- uses: actions/checkout@v4
218+
with:
219+
submodules: true
220+
- name: Install dependencies
221+
run: |
222+
sudo apt update
223+
sudo apt install -y protobuf-compiler libssl-dev
224+
- name: Install ${{ matrix.msrv }}
225+
uses: dtolnay/rust-toolchain@master
226+
with:
227+
toolchain: ${{ matrix.msrv }}
228+
- name: cargo +${{ matrix.msrv }} check
229+
run: cargo check --workspace --tests --benches --all-features

Cargo.toml

+18
Original file line numberDiff line numberDiff line change
@@ -163,3 +163,21 @@ zstd = "0.13.1"
163163
opt-level = 3
164164
debug = true
165165
strip = false
166+
167+
[workspace.lints.clippy]
168+
all = { level = "deny", priority = -1 }
169+
style = { level = "deny", priority = -1 }
170+
cargo = { level = "deny", priority = -1 }
171+
fallible_impl_from = "deny"
172+
manual_let_else = "deny"
173+
redundant_pub_crate = "deny"
174+
string_add_assign = "deny"
175+
string_add = "deny"
176+
string_lit_as_bytes = "deny"
177+
string_to_string = "deny"
178+
use_self = "deny"
179+
dbg_macro = "deny"
180+
trait_duplication_in_bounds = "deny"
181+
redundant_clone = "deny"
182+
# not too much we can do to avoid multiple crate versions
183+
multiple-crate-versions = "allow"

rust/lance-arrow/Cargo.toml

+3
Original file line numberDiff line numberDiff line change
@@ -26,3 +26,6 @@ rand.workspace = true
2626

2727
[target.'cfg(target_arch = "wasm32")'.dependencies]
2828
getrandom = { version = "0.2", features = ["js"] }
29+
30+
[lints]
31+
workspace = true

rust/lance-arrow/src/deepcopy.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -56,5 +56,5 @@ pub fn deep_copy_batch(batch: &RecordBatch) -> crate::Result<RecordBatch> {
5656
.iter()
5757
.map(|array| deep_copy_array(array))
5858
.collect::<Vec<_>>();
59-
RecordBatch::try_new(batch.schema().clone(), arrays)
59+
RecordBatch::try_new(batch.schema(), arrays)
6060
}

rust/lance-arrow/src/lib.rs

+4-4
Original file line numberDiff line numberDiff line change
@@ -475,7 +475,7 @@ impl RecordBatchExt for RecordBatch {
475475
.position(|f| f.name() == name)
476476
.ok_or_else(|| ArrowError::SchemaError(format!("Field {} does not exist", name)))?;
477477
columns[field_i] = column;
478-
Self::try_new(self.schema().clone(), columns)
478+
Self::try_new(self.schema(), columns)
479479
}
480480

481481
fn column_by_qualified_name(&self, name: &str) -> Option<&ArrayRef> {
@@ -637,7 +637,7 @@ pub fn interleave_batches(
637637
let first_batch = batches.first().ok_or_else(|| {
638638
ArrowError::InvalidArgumentError("Cannot interleave zero RecordBatches".to_string())
639639
})?;
640-
let schema = first_batch.schema().clone();
640+
let schema = first_batch.schema();
641641
let num_columns = first_batch.num_columns();
642642
let mut columns = Vec::with_capacity(num_columns);
643643
let mut chunks = Vec::with_capacity(batches.len());
@@ -785,7 +785,7 @@ mod tests {
785785
.with_metadata(metadata.clone().into()),
786786
);
787787
let batch = RecordBatch::try_new(
788-
schema.clone(),
788+
schema,
789789
vec![
790790
Arc::new(Int32Array::from_iter_values(0..20)),
791791
Arc::new(StringArray::from_iter_values(
@@ -830,7 +830,7 @@ mod tests {
830830
// Sub schema
831831
let sub_schema = Schema::new(vec![Field::new("a", DataType::Int32, true)]);
832832
let sub_projected = batch.project_by_schema(&sub_schema).unwrap();
833-
let expected_schema = Arc::new(sub_schema.with_metadata(metadata.clone().into()));
833+
let expected_schema = Arc::new(sub_schema.with_metadata(metadata.into()));
834834
assert_eq!(
835835
sub_projected,
836836
RecordBatch::try_new(

rust/lance-core/Cargo.toml

+3
Original file line numberDiff line numberDiff line change
@@ -54,3 +54,6 @@ proptest.workspace = true
5454

5555
[features]
5656
datafusion = ["datafusion-common", "datafusion-sql"]
57+
58+
[lints]
59+
workspace = true

rust/lance-core/src/utils/mask.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -901,7 +901,7 @@ mod tests {
901901
right_rows in proptest::collection::vec(0..u64::MAX, 0..1000),
902902
) {
903903
let mut left = RowIdTreeMap::default();
904-
for fragment in left_full_fragments.clone() {
904+
for fragment in left_full_fragments {
905905
left.insert_fragment(fragment);
906906
}
907907
left.extend(left_rows.iter().copied());
@@ -925,7 +925,7 @@ mod tests {
925925
left_rows in proptest::collection::vec(0..u64::MAX, 0..1000),
926926
) {
927927
let mut left = RowIdTreeMap::default();
928-
for fragment in left_full_fragments.clone() {
928+
for fragment in left_full_fragments {
929929
left.insert_fragment(fragment);
930930
}
931931
left.extend(left_rows.iter().copied());

rust/lance-datafusion/Cargo.toml

+3
Original file line numberDiff line numberDiff line change
@@ -37,3 +37,6 @@ lance-datagen.workspace = true
3737

3838
[features]
3939
substrait = ["dep:datafusion-substrait"]
40+
41+
[lints]
42+
workspace = true

rust/lance-datafusion/src/chunker.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,7 @@ pub fn chunk_concat_stream(
180180
stream: SendableRecordBatchStream,
181181
chunk_size: usize,
182182
) -> SendableRecordBatchStream {
183-
let schema = stream.schema().clone();
183+
let schema = stream.schema();
184184
let schema_copy = schema.clone();
185185
let chunked = chunk_stream(stream, chunk_size);
186186
let chunk_concat = chunked

rust/lance-datafusion/src/exec.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ pub struct OneShotExec {
5050
impl OneShotExec {
5151
/// Create a new instance from a given stream
5252
pub fn new(stream: SendableRecordBatchStream) -> Self {
53-
let schema = stream.schema().clone();
53+
let schema = stream.schema();
5454
Self {
5555
stream: Mutex::new(Some(stream)),
5656
schema: schema.clone(),
@@ -272,7 +272,7 @@ struct OneShotPartitionStream {
272272

273273
impl OneShotPartitionStream {
274274
fn new(data: SendableRecordBatchStream) -> Self {
275-
let schema = data.schema().clone();
275+
let schema = data.schema();
276276
Self {
277277
data: Arc::new(Mutex::new(Some(data))),
278278
schema,
@@ -298,7 +298,7 @@ impl SessionContextExt for SessionContext {
298298
&self,
299299
data: SendableRecordBatchStream,
300300
) -> datafusion::common::Result<DataFrame> {
301-
let schema = data.schema().clone();
301+
let schema = data.schema();
302302
let part_stream = Arc::new(OneShotPartitionStream::new(data));
303303
let provider = StreamingTable::try_new(schema, vec![part_stream])?;
304304
self.read_table(Arc::new(provider))

rust/lance-datafusion/src/planner.rs

+5-5
Original file line numberDiff line numberDiff line change
@@ -608,7 +608,7 @@ impl Planner {
608608
))?;
609609
}
610610
}
611-
Field::new("item", data_type.clone(), true)
611+
Field::new("item", data_type, true)
612612
} else {
613613
Field::new("item", ArrowDataType::Null, true)
614614
};
@@ -825,7 +825,7 @@ impl Planner {
825825
let simplifier =
826826
datafusion::optimizer::simplify_expressions::ExprSimplifier::new(simplify_context);
827827

828-
let expr = simplifier.simplify(expr.clone())?;
828+
let expr = simplifier.simplify(expr)?;
829829
let expr = simplifier.coerce(expr, &df_schema)?;
830830

831831
Ok(expr)
@@ -1009,7 +1009,7 @@ mod tests {
10091009
),
10101010
]));
10111011

1012-
let planner = Planner::new(schema.clone());
1012+
let planner = Planner::new(schema);
10131013

10141014
fn assert_column_eq(planner: &Planner, expr: &str, expected: &Expr) {
10151015
let expr = planner.parse_filter(&format!("{expr} = 'val'")).unwrap();
@@ -1082,7 +1082,7 @@ mod tests {
10821082
true,
10831083
)]));
10841084

1085-
let planner = Planner::new(schema.clone());
1085+
let planner = Planner::new(schema);
10861086

10871087
let expected = array_element(col("l"), lit(0_i64));
10881088
let expr = planner.parse_expr("l[0]").unwrap();
@@ -1132,7 +1132,7 @@ mod tests {
11321132
fn test_negative_array_expressions() {
11331133
let schema = Arc::new(Schema::new(vec![Field::new("x", DataType::Int64, false)]));
11341134

1135-
let planner = Planner::new(schema.clone());
1135+
let planner = Planner::new(schema);
11361136

11371137
let expected = Expr::Literal(ScalarValue::List(Arc::new(
11381138
ListArray::from_iter_primitive::<Float64Type, _, _>(vec![Some(

rust/lance-datagen/Cargo.toml

+3
Original file line numberDiff line numberDiff line change
@@ -32,3 +32,6 @@ bench = false
3232
[[bench]]
3333
name = "array_gen"
3434
harness = false
35+
36+
[lints]
37+
workspace = true

rust/lance-datagen/src/generator.rs

+11-11
Original file line numberDiff line numberDiff line change
@@ -805,7 +805,7 @@ impl<T: ByteArrayType> CycleBinaryGenerator<T> {
805805
Self {
806806
values,
807807
lengths,
808-
data_type: T::DATA_TYPE.clone(),
808+
data_type: T::DATA_TYPE,
809809
array_type: PhantomData,
810810
width,
811811
idx: 0,
@@ -863,7 +863,7 @@ impl<T: ByteArrayType> FixedBinaryGenerator<T> {
863863
pub fn new(value: Vec<u8>) -> Self {
864864
Self {
865865
value,
866-
data_type: T::DATA_TYPE.clone(),
866+
data_type: T::DATA_TYPE,
867867
array_type: PhantomData,
868868
}
869869
}
@@ -910,7 +910,7 @@ pub struct DictionaryGenerator<K: ArrowDictionaryKeyType> {
910910

911911
impl<K: ArrowDictionaryKeyType> DictionaryGenerator<K> {
912912
fn new(generator: Box<dyn ArrayGenerator>) -> Self {
913-
let key_type = Box::new(K::DATA_TYPE.clone());
913+
let key_type = Box::new(K::DATA_TYPE);
914914
let key_width = key_type
915915
.primitive_width()
916916
.expect("dictionary key types should have a known width")
@@ -1352,7 +1352,7 @@ pub mod array {
13521352
let mut values_idx = 0;
13531353
Box::new(
13541354
FnGen::<DataType::Native, PrimitiveArray<DataType>, _>::new_known_size(
1355-
DataType::DATA_TYPE.clone(),
1355+
DataType::DATA_TYPE,
13561356
move |_| {
13571357
let y = values[values_idx];
13581358
values_idx = (values_idx + 1) % values.len();
@@ -1377,7 +1377,7 @@ pub mod array {
13771377
let mut x = DataType::Native::default();
13781378
Box::new(
13791379
FnGen::<DataType::Native, PrimitiveArray<DataType>, _>::new_known_size(
1380-
DataType::DATA_TYPE.clone(),
1380+
DataType::DATA_TYPE,
13811381
move |_| {
13821382
let y = x;
13831383
x += DataType::Native::ONE;
@@ -1411,7 +1411,7 @@ pub mod array {
14111411
let mut x = start;
14121412
Box::new(
14131413
FnGen::<DataType::Native, PrimitiveArray<DataType>, _>::new_known_size(
1414-
DataType::DATA_TYPE.clone(),
1414+
DataType::DATA_TYPE,
14151415
move |_| {
14161416
let y = x;
14171417
x += step;
@@ -1435,7 +1435,7 @@ pub mod array {
14351435
{
14361436
Box::new(
14371437
FnGen::<DataType::Native, PrimitiveArray<DataType>, _>::new_known_size(
1438-
DataType::DATA_TYPE.clone(),
1438+
DataType::DATA_TYPE,
14391439
move |_| value,
14401440
1,
14411441
DataType::DATA_TYPE
@@ -1470,7 +1470,7 @@ pub mod array {
14701470
{
14711471
Box::new(
14721472
FnGen::<DataType::Native, PrimitiveArray<DataType>, _>::new_known_size(
1473-
DataType::DATA_TYPE.clone(),
1473+
DataType::DATA_TYPE,
14741474
move |rng| rng.gen(),
14751475
1,
14761476
DataType::DATA_TYPE
@@ -1495,7 +1495,7 @@ pub mod array {
14951495
{
14961496
Box::new(
14971497
FnGen::<DataType::Native, PrimitiveArray<DataType>, _>::new_known_size(
1498-
DataType::DATA_TYPE.clone(),
1498+
DataType::DATA_TYPE,
14991499
move |rng| rng.sample(dist.clone()),
15001500
1,
15011501
DataType::DATA_TYPE
@@ -1632,7 +1632,7 @@ pub mod array {
16321632
let dist = Uniform::new(start_days, end_days);
16331633

16341634
Box::new(FnGen::<i32, Date32Array, _>::new_known_size(
1635-
data_type.clone(),
1635+
data_type,
16361636
move |rng| dist.sample(rng),
16371637
1,
16381638
DataType::Date32
@@ -1727,7 +1727,7 @@ pub mod array {
17271727
let dist = Uniform::new(start_days, end_days);
17281728

17291729
Box::new(FnGen::<i64, Date64Array, _>::new_known_size(
1730-
data_type.clone(),
1730+
data_type,
17311731
move |rng| (dist.sample(rng)) * MS_PER_DAY,
17321732
1,
17331733
DataType::Date64

rust/lance-encoding-datafusion/Cargo.toml

+3
Original file line numberDiff line numberDiff line change
@@ -45,3 +45,6 @@ prost-build.workspace = true
4545

4646
[target.'cfg(target_os = "linux")'.dev-dependencies]
4747
pprof = { workspace = true }
48+
49+
[lints]
50+
workspace = true

0 commit comments

Comments
 (0)