From bf4200f225c9fd838b6cb94466556e8288d264e5 Mon Sep 17 00:00:00 2001 From: Martin Tzvetanov Grigorov Date: Wed, 7 Jul 2021 15:20:00 +0300 Subject: [PATCH 1/9] AVRO-3175 Rust: fix lint/clippy errors Fix all linting/clippy errors & warnings. Add build.sh for Rust that currently supports clean, dist, lint and test targets --- build.sh | 6 +++ lang/rust/build.sh | 44 ++++++++++++++++ lang/rust/src/lib.rs | 2 +- lang/rust/src/schema_compatibility.rs | 72 +++++++++++---------------- lang/rust/src/ser.rs | 4 +- lang/rust/src/util.rs | 2 +- lang/rust/tests/io.rs | 2 +- lang/rust/tests/schema.rs | 2 +- 8 files changed, 85 insertions(+), 49 deletions(-) create mode 100755 lang/rust/build.sh diff --git a/build.sh b/build.sh index 554c647555f..a441fe6fa10 100755 --- a/build.sh +++ b/build.sh @@ -101,6 +101,7 @@ do (cd lang/ruby; ./build.sh lint test) (cd lang/php; ./build.sh lint test) (cd lang/perl; ./build.sh lint test) + (cd lang/rust; ./build.sh lint test) (cd lang/py; ./build.sh interop-data-generate) (cd lang/c; ./build.sh interop-data-generate) @@ -166,6 +167,7 @@ do (cd lang/js; ./build.sh dist) (cd lang/ruby; ./build.sh dist) (cd lang/php; ./build.sh dist) + (cd lang/rust; ./build.sh dist) mkdir -p dist/perl (cd lang/perl; ./build.sh dist) @@ -226,6 +228,8 @@ do (cd lang/php; ./build.sh clean) (cd lang/perl; ./build.sh clean) + + (cd lang/rust; ./build.sh clean) ;; veryclean) @@ -253,6 +257,8 @@ do (cd lang/perl; ./build.sh clean) + (cd lang/rust; ./build.sh clean) + rm -rf lang/c++/build rm -rf lang/js/node_modules rm -rf lang/perl/inc/ diff --git a/lang/rust/build.sh b/lang/rust/build.sh new file mode 100755 index 00000000000..dba358096ff --- /dev/null +++ b/lang/rust/build.sh @@ -0,0 +1,44 @@ +#!/bin/bash + +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# https://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +set -e + +cd `dirname "$0"` + +for target in "$@" +do + case "$target" in + clean) + cargo clean + ;; + lint) + cargo clippy --all-targets --all-features -- -Dclippy::all + ;; + test) + cargo test + ;; + dist) + cargo build --release --lib + cargo package + mkdir -p ../../dist/rust + cp target/package/avro-rs-*.crate ../../dist/rust + ;; + *) + echo "Usage: $0 {lint|test|dist|clean}" >&2 + exit 1 + esac +done diff --git a/lang/rust/src/lib.rs b/lang/rust/src/lib.rs index 5376d3a09e0..21530d9a65a 100644 --- a/lang/rust/src/lib.rs +++ b/lang/rust/src/lib.rs @@ -970,7 +970,7 @@ mod tests { // Would allocated 18446744073709551605 bytes let illformed: &[u8] = &[0x3e, 0x15, 0xff, 0x1f, 0x15, 0xff]; - let value = from_avro_datum(&schema, &mut &illformed[..], None); + let value = from_avro_datum(&schema, &mut &*illformed, None); assert!(value.is_err()); } } diff --git a/lang/rust/src/schema_compatibility.rs b/lang/rust/src/schema_compatibility.rs index 7236c64c3d2..34a3b467895 100644 --- a/lang/rust/src/schema_compatibility.rs +++ b/lang/rust/src/schema_compatibility.rs @@ -96,7 +96,7 @@ impl Checker { symbols: r_symbols, .. } = readers_schema { - return w_symbols.iter().find(|e| !r_symbols.contains(e)).is_none(); + return !w_symbols.iter().any(|e| !r_symbols.contains(e)); } } false @@ -597,9 +597,8 @@ mod tests { &writer_schema(), &reader_schema, )); - assert_eq!( - SchemaCompatibility::can_read(&reader_schema, &writer_schema()), - false + assert!( + !SchemaCompatibility::can_read(&reader_schema, &writer_schema()) ); } @@ -617,9 +616,8 @@ mod tests { &writer_schema(), &reader_schema )); - assert_eq!( - SchemaCompatibility::can_read(&reader_schema, &writer_schema()), - false + assert!( + !SchemaCompatibility::can_read(&reader_schema, &writer_schema()) ); } @@ -659,9 +657,8 @@ mod tests { &writer_schema(), &reader_schema )); - assert_eq!( - SchemaCompatibility::can_read(&reader_schema, &writer_schema()), - false + assert!( + !SchemaCompatibility::can_read(&reader_schema, &writer_schema()) ); } @@ -676,13 +673,11 @@ mod tests { "#, ) .unwrap(); - assert_eq!( - SchemaCompatibility::can_read(&writer_schema(), &reader_schema), - false + assert!( + !SchemaCompatibility::can_read(&writer_schema(), &reader_schema) ); - assert_eq!( - SchemaCompatibility::can_read(&reader_schema, &writer_schema()), - false + assert!( + !SchemaCompatibility::can_read(&reader_schema, &writer_schema()) ); } @@ -695,9 +690,8 @@ mod tests { &string_array_schema(), &valid_reader )); - assert_eq!( - SchemaCompatibility::can_read(&string_array_schema(), &invalid_reader), - false + assert!( + !SchemaCompatibility::can_read(&string_array_schema(), &invalid_reader) ); } @@ -708,9 +702,8 @@ mod tests { &Schema::String, &valid_reader )); - assert_eq!( - SchemaCompatibility::can_read(&Schema::Int, &Schema::String), - false + assert!( + !SchemaCompatibility::can_read(&Schema::Int, &Schema::String) ); } @@ -720,9 +713,8 @@ mod tests { let union_writer = union_schema(vec![Schema::Int, Schema::String]); let union_reader = union_schema(vec![Schema::String]); - assert_eq!( - SchemaCompatibility::can_read(&union_writer, &union_reader), - false + assert!( + !SchemaCompatibility::can_read(&union_writer, &union_reader) ); assert!(SchemaCompatibility::can_read(&union_reader, &union_writer)); } @@ -747,9 +739,8 @@ mod tests { ) .unwrap(); - assert_eq!( - SchemaCompatibility::can_read(&string_schema, &int_schema), - false + assert!( + !SchemaCompatibility::can_read(&string_schema, &int_schema) ); } @@ -764,9 +755,8 @@ mod tests { let enum_schema2 = Schema::parse_str(r#"{"type":"enum", "name":"MyEnum", "symbols":["A","B","C"]}"#) .unwrap(); - assert_eq!( - SchemaCompatibility::can_read(&enum_schema2, &enum_schema1), - false + assert!( + !SchemaCompatibility::can_read(&enum_schema2, &enum_schema1) ); assert!(SchemaCompatibility::can_read(&enum_schema1, &enum_schema2)); } @@ -844,9 +834,8 @@ mod tests { fn test_union_resolution_no_structure_match() { // short name match, but no structure match let read_schema = union_schema(vec![Schema::Null, point_3d_no_default_schema()]); - assert_eq!( - SchemaCompatibility::can_read(&point_2d_fullname_schema(), &read_schema), - false + assert!( + !SchemaCompatibility::can_read(&point_2d_fullname_schema(), &read_schema) ); } @@ -862,9 +851,8 @@ mod tests { // point_2d_schema(), // point_3d_schema(), // ]); - // assert_eq!( - // SchemaCompatibility::can_read(&point_2d_fullname_schema(), &read_schema), - // false + // assert!( + // !SchemaCompatibility::can_read(&point_2d_fullname_schema(), &read_schema) // ); // } @@ -877,9 +865,8 @@ mod tests { // point_3d_schema(), // point_2d_schema(), // ]); - // assert_eq!( - // SchemaCompatibility::can_read(&point_2d_fullname_schema(), &read_schema), - // false + // assert!( + // !SchemaCompatibility::can_read(&point_2d_fullname_schema(), &read_schema) // ); // } @@ -892,9 +879,8 @@ mod tests { // point_3d_match_name_schema(), // point_3d_schema(), // ]); - // assert_eq!( - // SchemaCompatibility::can_read(&point_2d_fullname_schema(), &read_schema), - // false + // assert!( + // !SchemaCompatibility::can_read(&point_2d_fullname_schema(), &read_schema) // ); // } diff --git a/lang/rust/src/ser.rs b/lang/rust/src/ser.rs index 4ce62bea7d2..480ea0e4cd9 100644 --- a/lang/rust/src/ser.rs +++ b/lang/rust/src/ser.rs @@ -375,7 +375,7 @@ impl<'a> ser::SerializeTupleVariant for SeqVariantSerializer<'a> { } fn end(self) -> Result { - Ok(ser::SerializeSeq::end(self)?) + ser::SerializeSeq::end(self) } } @@ -792,7 +792,7 @@ mod tests { a: SingleValueInternalEnum::Double(64.0), }; - assert_eq!(to_value(test).is_err(), true); + assert!(to_value(test).is_err(), "{}", true); let test = TestSingleValueAdjacentEnum { a: SingleValueAdjacentEnum::Double(64.0), diff --git a/lang/rust/src/util.rs b/lang/rust/src/util.rs index 1d7559faeba..f9daf285998 100644 --- a/lang/rust/src/util.rs +++ b/lang/rust/src/util.rs @@ -207,7 +207,7 @@ mod tests { #[test] fn test_overflow() { let causes_left_shift_overflow: &[u8] = &[0xe1, 0xe1, 0xe1, 0xe1, 0xe1]; - assert!(decode_variable(&mut &causes_left_shift_overflow[..]).is_err()); + assert!(decode_variable(&mut &*causes_left_shift_overflow).is_err()); } #[test] diff --git a/lang/rust/tests/io.rs b/lang/rust/tests/io.rs index a6edd541942..fc4b83dd0d4 100644 --- a/lang/rust/tests/io.rs +++ b/lang/rust/tests/io.rs @@ -102,7 +102,7 @@ fn test_validate() { let schema = Schema::parse_str(raw_schema).unwrap(); assert!( value.validate(&schema), - format!("value {:?} does not validate schema: {}", value, raw_schema) + "value {:?} does not validate schema: {}", value, raw_schema ); } } diff --git a/lang/rust/tests/schema.rs b/lang/rust/tests/schema.rs index 9e602b0a9f0..8b6d1044ecf 100644 --- a/lang/rust/tests/schema.rs +++ b/lang/rust/tests/schema.rs @@ -1170,7 +1170,7 @@ fn test_root_error_is_not_swallowed_on_parse_error() -> Result<(), String> { if let Error::ParseSchemaJson(e) = error { assert!( e.to_string().contains("expected value at line 1 column 1"), - e.to_string() + "{}", e ); Ok(()) } else { From 1795839e9eeb1ac4ef9b38a08341b78e85ff86b3 Mon Sep 17 00:00:00 2001 From: Martin Tzvetanov Grigorov Date: Thu, 8 Jul 2021 09:41:43 +0300 Subject: [PATCH 2/9] INFRA-22085 Test actions-rs From ed23421d31bf9f952fb02ffe491babdb0d766d7c Mon Sep 17 00:00:00 2001 From: Martin Tzvetanov Grigorov Date: Thu, 8 Jul 2021 09:46:25 +0300 Subject: [PATCH 3/9] AVRO-3175 Build with all features enabled --- lang/rust/build.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lang/rust/build.sh b/lang/rust/build.sh index dba358096ff..d9a24849c32 100755 --- a/lang/rust/build.sh +++ b/lang/rust/build.sh @@ -32,7 +32,7 @@ do cargo test ;; dist) - cargo build --release --lib + cargo build --release --lib --all-features cargo package mkdir -p ../../dist/rust cp target/package/avro-rs-*.crate ../../dist/rust From 23c83566acf3f943bf398192bb8f9d391c3aa4c3 Mon Sep 17 00:00:00 2001 From: Martin Tzvetanov Grigorov Date: Thu, 8 Jul 2021 10:04:50 +0300 Subject: [PATCH 4/9] AVRO-3175 Change working folder to lang/rust --- .github/workflows/test-lang-rust-ci.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.github/workflows/test-lang-rust-ci.yml b/.github/workflows/test-lang-rust-ci.yml index 449f55882bc..daaae46ecc8 100644 --- a/.github/workflows/test-lang-rust-ci.yml +++ b/.github/workflows/test-lang-rust-ci.yml @@ -26,6 +26,10 @@ on: - .github/workflows/test-lang-rust-ci.yml - lang/rust/** +defaults: + run: + working-directory: lang/rust + jobs: ci: runs-on: ubuntu-latest From a73769938d486eae6ac56b9ef9027fce6916edb2 Mon Sep 17 00:00:00 2001 From: Martin Tzvetanov Grigorov Date: Thu, 8 Jul 2021 10:06:12 +0300 Subject: [PATCH 5/9] AVRO-3175 Set working folder to lang/avro for GHA --- .github/workflows/test-lang-rust-audit.yml | 4 ++++ .github/workflows/test-lang-rust-clippy.yml | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/.github/workflows/test-lang-rust-audit.yml b/.github/workflows/test-lang-rust-audit.yml index 33b5113017d..3e6cfb3e313 100644 --- a/.github/workflows/test-lang-rust-audit.yml +++ b/.github/workflows/test-lang-rust-audit.yml @@ -27,6 +27,10 @@ on: - lang/rust/Cargo.toml - lang/rust/Cargo.lock +defaults: + run: + working-directory: lang/rust + jobs: audit: runs-on: ubuntu-latest diff --git a/.github/workflows/test-lang-rust-clippy.yml b/.github/workflows/test-lang-rust-clippy.yml index 9ac107b3aa1..c1455b900c4 100644 --- a/.github/workflows/test-lang-rust-clippy.yml +++ b/.github/workflows/test-lang-rust-clippy.yml @@ -26,6 +26,10 @@ on: - .github/workflows/test-lang-rust-clippy.yml - lang/rust/** +defaults: + run: + working-directory: lang/rust + jobs: clippy_check: runs-on: ubuntu-latest From f4c954c248dc1e05757868de1b6c3d6a451bf7d2 Mon Sep 17 00:00:00 2001 From: Martin Tzvetanov Grigorov Date: Thu, 8 Jul 2021 10:10:15 +0300 Subject: [PATCH 6/9] [squash] Debug pwd --- .github/workflows/test-lang-rust-ci.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/test-lang-rust-ci.yml b/.github/workflows/test-lang-rust-ci.yml index daaae46ecc8..819490e09bb 100644 --- a/.github/workflows/test-lang-rust-ci.yml +++ b/.github/workflows/test-lang-rust-ci.yml @@ -53,6 +53,9 @@ jobs: override: true components: rustfmt + - name: Print PWD + run: pwd + - name: Rust Format uses: actions-rs/cargo@v1 with: From e2c8c1bc5101011e424dfb03e97ba6504d8dc90a Mon Sep 17 00:00:00 2001 From: Martin Tzvetanov Grigorov Date: Thu, 8 Jul 2021 10:16:07 +0300 Subject: [PATCH 7/9] AVRO-3175 Workaround https://github.com/actions-rs/cargo/issues/86 Use --manifest-path to specify the path to Cargo.yaml --- .github/workflows/test-lang-rust-ci.yml | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/.github/workflows/test-lang-rust-ci.yml b/.github/workflows/test-lang-rust-ci.yml index 819490e09bb..977ea110731 100644 --- a/.github/workflows/test-lang-rust-ci.yml +++ b/.github/workflows/test-lang-rust-ci.yml @@ -53,30 +53,27 @@ jobs: override: true components: rustfmt - - name: Print PWD - run: pwd - - name: Rust Format uses: actions-rs/cargo@v1 with: command: fmt - args: --all -- --check + args: --manifest-path lang/rust/Cargo.toml --all -- --check - name: Rust Build uses: actions-rs/cargo@v1 with: command: build - args: --all-features --all-targets + args: --manifest-path lang/rust/Cargo.toml --all-features --all-targets - name: Rust Test uses: actions-rs/cargo@v1 with: command: test - args: --all-features --all-targets + args: --manifest-path lang/rust/Cargo.toml --all-features --all-targets # because of https://github.com/rust-lang/cargo/issues/6669 - name: Rust Test docs uses: actions-rs/cargo@v1 with: command: test - args: --doc + args: --manifest-path lang/rust/Cargo.toml --doc From edb08af346f9f2463f63e917d18de54727111984 Mon Sep 17 00:00:00 2001 From: Martin Tzvetanov Grigorov Date: Thu, 8 Jul 2021 10:20:06 +0300 Subject: [PATCH 8/9] AVRO-3175 Format the code with `cargo fmt --all --" This should fix the Github Actions CI workflow --- lang/rust/src/schema_compatibility.rs | 68 ++++++++++++++------------- lang/rust/tests/io.rs | 4 +- lang/rust/tests/schema.rs | 3 +- 3 files changed, 40 insertions(+), 35 deletions(-) diff --git a/lang/rust/src/schema_compatibility.rs b/lang/rust/src/schema_compatibility.rs index 34a3b467895..7f2a5cfdad0 100644 --- a/lang/rust/src/schema_compatibility.rs +++ b/lang/rust/src/schema_compatibility.rs @@ -597,9 +597,10 @@ mod tests { &writer_schema(), &reader_schema, )); - assert!( - !SchemaCompatibility::can_read(&reader_schema, &writer_schema()) - ); + assert!(!SchemaCompatibility::can_read( + &reader_schema, + &writer_schema() + )); } #[test] @@ -616,9 +617,10 @@ mod tests { &writer_schema(), &reader_schema )); - assert!( - !SchemaCompatibility::can_read(&reader_schema, &writer_schema()) - ); + assert!(!SchemaCompatibility::can_read( + &reader_schema, + &writer_schema() + )); } #[test] @@ -657,9 +659,10 @@ mod tests { &writer_schema(), &reader_schema )); - assert!( - !SchemaCompatibility::can_read(&reader_schema, &writer_schema()) - ); + assert!(!SchemaCompatibility::can_read( + &reader_schema, + &writer_schema() + )); } #[test] @@ -673,12 +676,14 @@ mod tests { "#, ) .unwrap(); - assert!( - !SchemaCompatibility::can_read(&writer_schema(), &reader_schema) - ); - assert!( - !SchemaCompatibility::can_read(&reader_schema, &writer_schema()) - ); + assert!(!SchemaCompatibility::can_read( + &writer_schema(), + &reader_schema + )); + assert!(!SchemaCompatibility::can_read( + &reader_schema, + &writer_schema() + )); } #[test] @@ -690,9 +695,10 @@ mod tests { &string_array_schema(), &valid_reader )); - assert!( - !SchemaCompatibility::can_read(&string_array_schema(), &invalid_reader) - ); + assert!(!SchemaCompatibility::can_read( + &string_array_schema(), + &invalid_reader + )); } #[test] @@ -702,9 +708,10 @@ mod tests { &Schema::String, &valid_reader )); - assert!( - !SchemaCompatibility::can_read(&Schema::Int, &Schema::String) - ); + assert!(!SchemaCompatibility::can_read( + &Schema::Int, + &Schema::String + )); } #[test] @@ -713,9 +720,7 @@ mod tests { let union_writer = union_schema(vec![Schema::Int, Schema::String]); let union_reader = union_schema(vec![Schema::String]); - assert!( - !SchemaCompatibility::can_read(&union_writer, &union_reader) - ); + assert!(!SchemaCompatibility::can_read(&union_writer, &union_reader)); assert!(SchemaCompatibility::can_read(&union_reader, &union_writer)); } @@ -739,9 +744,7 @@ mod tests { ) .unwrap(); - assert!( - !SchemaCompatibility::can_read(&string_schema, &int_schema) - ); + assert!(!SchemaCompatibility::can_read(&string_schema, &int_schema)); } #[test] @@ -755,9 +758,7 @@ mod tests { let enum_schema2 = Schema::parse_str(r#"{"type":"enum", "name":"MyEnum", "symbols":["A","B","C"]}"#) .unwrap(); - assert!( - !SchemaCompatibility::can_read(&enum_schema2, &enum_schema1) - ); + assert!(!SchemaCompatibility::can_read(&enum_schema2, &enum_schema1)); assert!(SchemaCompatibility::can_read(&enum_schema1, &enum_schema2)); } @@ -834,9 +835,10 @@ mod tests { fn test_union_resolution_no_structure_match() { // short name match, but no structure match let read_schema = union_schema(vec![Schema::Null, point_3d_no_default_schema()]); - assert!( - !SchemaCompatibility::can_read(&point_2d_fullname_schema(), &read_schema) - ); + assert!(!SchemaCompatibility::can_read( + &point_2d_fullname_schema(), + &read_schema + )); } // TODO(nlopes): the below require named schemas to be fully supported. See: diff --git a/lang/rust/tests/io.rs b/lang/rust/tests/io.rs index fc4b83dd0d4..93edf0d7b45 100644 --- a/lang/rust/tests/io.rs +++ b/lang/rust/tests/io.rs @@ -102,7 +102,9 @@ fn test_validate() { let schema = Schema::parse_str(raw_schema).unwrap(); assert!( value.validate(&schema), - "value {:?} does not validate schema: {}", value, raw_schema + "value {:?} does not validate schema: {}", + value, + raw_schema ); } } diff --git a/lang/rust/tests/schema.rs b/lang/rust/tests/schema.rs index 8b6d1044ecf..4f20c8cb4f3 100644 --- a/lang/rust/tests/schema.rs +++ b/lang/rust/tests/schema.rs @@ -1170,7 +1170,8 @@ fn test_root_error_is_not_swallowed_on_parse_error() -> Result<(), String> { if let Error::ParseSchemaJson(e) = error { assert!( e.to_string().contains("expected value at line 1 column 1"), - "{}", e + "{}", + e ); Ok(()) } else { From 1a9f634d8dcafdfb8e6adb0b0ff7009fd2f5291e Mon Sep 17 00:00:00 2001 From: Martin Tzvetanov Grigorov Date: Thu, 8 Jul 2021 10:43:05 +0300 Subject: [PATCH 9/9] AVRO-3175 Specify the path to Cargo.yaml for clippy --- .github/workflows/test-lang-rust-audit.yml | 2 +- .github/workflows/test-lang-rust-clippy.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/test-lang-rust-audit.yml b/.github/workflows/test-lang-rust-audit.yml index 3e6cfb3e313..ee3fe47dee6 100644 --- a/.github/workflows/test-lang-rust-audit.yml +++ b/.github/workflows/test-lang-rust-audit.yml @@ -29,7 +29,7 @@ on: defaults: run: - working-directory: lang/rust + working-directory: lang/rust # Currently does not work. See https://github.com/actions-rs/audit-check/issues/194 jobs: audit: diff --git a/.github/workflows/test-lang-rust-clippy.yml b/.github/workflows/test-lang-rust-clippy.yml index c1455b900c4..cedc5f5f042 100644 --- a/.github/workflows/test-lang-rust-clippy.yml +++ b/.github/workflows/test-lang-rust-clippy.yml @@ -43,4 +43,4 @@ jobs: - uses: actions-rs/clippy-check@v1 with: token: ${{ secrets.GITHUB_TOKEN }} - args: --all-features --all-targets -- -Dclippy::all -Dunused_imports + args: --manifest-path lang/rust/Cargo.toml --all-features --all-targets -- -Dclippy::all -Dunused_imports