Skip to content

Commit

Permalink
AVRO-3175 Rust: fix lint/clippy errors (#1286)
Browse files Browse the repository at this point in the history
* 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

* INFRA-22085 Test actions-rs

* AVRO-3175 Build with all features enabled

* AVRO-3175 Change working folder to lang/rust

* AVRO-3175 Set working folder to lang/avro for GHA

* [squash] Debug pwd

* AVRO-3175 Workaround actions-rs/cargo#86

Use --manifest-path to specify the path to Cargo.yaml

* AVRO-3175 Format the code with `cargo fmt --all --"

This should fix the Github Actions CI workflow

* AVRO-3175 Specify the path to Cargo.yaml for clippy
  • Loading branch information
martin-g authored Aug 7, 2021
1 parent ba3ad6d commit d1b4810
Show file tree
Hide file tree
Showing 11 changed files with 118 additions and 65 deletions.
4 changes: 4 additions & 0 deletions .github/workflows/test-lang-rust-audit.yml
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,10 @@ on:
- lang/rust/Cargo.toml
- lang/rust/Cargo.lock

defaults:
run:
working-directory: lang/rust # Currently does not work. See https://github.com/actions-rs/audit-check/issues/194

jobs:
audit:
runs-on: ubuntu-latest
Expand Down
12 changes: 8 additions & 4 deletions .github/workflows/test-lang-rust-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -53,23 +57,23 @@ jobs:
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
6 changes: 5 additions & 1 deletion .github/workflows/test-lang-rust-clippy.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -39,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
6 changes: 6 additions & 0 deletions build.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -226,6 +228,8 @@ do
(cd lang/php; ./build.sh clean)

(cd lang/perl; ./build.sh clean)

(cd lang/rust; ./build.sh clean)
;;

veryclean)
Expand Down Expand Up @@ -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/
Expand Down
44 changes: 44 additions & 0 deletions lang/rust/build.sh
Original file line number Diff line number Diff line change
@@ -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 --all-features
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
2 changes: 1 addition & 1 deletion lang/rust/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
}
96 changes: 42 additions & 54 deletions lang/rust/src/schema_compatibility.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -597,10 +597,10 @@ mod tests {
&writer_schema(),
&reader_schema,
));
assert_eq!(
SchemaCompatibility::can_read(&reader_schema, &writer_schema()),
false
);
assert!(!SchemaCompatibility::can_read(
&reader_schema,
&writer_schema()
));
}

#[test]
Expand All @@ -617,10 +617,10 @@ mod tests {
&writer_schema(),
&reader_schema
));
assert_eq!(
SchemaCompatibility::can_read(&reader_schema, &writer_schema()),
false
);
assert!(!SchemaCompatibility::can_read(
&reader_schema,
&writer_schema()
));
}

#[test]
Expand Down Expand Up @@ -659,10 +659,10 @@ mod tests {
&writer_schema(),
&reader_schema
));
assert_eq!(
SchemaCompatibility::can_read(&reader_schema, &writer_schema()),
false
);
assert!(!SchemaCompatibility::can_read(
&reader_schema,
&writer_schema()
));
}

#[test]
Expand All @@ -676,14 +676,14 @@ mod tests {
"#,
)
.unwrap();
assert_eq!(
SchemaCompatibility::can_read(&writer_schema(), &reader_schema),
false
);
assert_eq!(
SchemaCompatibility::can_read(&reader_schema, &writer_schema()),
false
);
assert!(!SchemaCompatibility::can_read(
&writer_schema(),
&reader_schema
));
assert!(!SchemaCompatibility::can_read(
&reader_schema,
&writer_schema()
));
}

#[test]
Expand All @@ -695,10 +695,10 @@ 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
));
}

#[test]
Expand All @@ -708,10 +708,10 @@ mod tests {
&Schema::String,
&valid_reader
));
assert_eq!(
SchemaCompatibility::can_read(&Schema::Int, &Schema::String),
false
);
assert!(!SchemaCompatibility::can_read(
&Schema::Int,
&Schema::String
));
}

#[test]
Expand All @@ -720,10 +720,7 @@ 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));
}

Expand All @@ -747,10 +744,7 @@ mod tests {
)
.unwrap();

assert_eq!(
SchemaCompatibility::can_read(&string_schema, &int_schema),
false
);
assert!(!SchemaCompatibility::can_read(&string_schema, &int_schema));
}

#[test]
Expand All @@ -764,10 +758,7 @@ 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));
}

Expand Down Expand Up @@ -844,10 +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_eq!(
SchemaCompatibility::can_read(&point_2d_fullname_schema(), &read_schema),
false
);
assert!(!SchemaCompatibility::can_read(
&point_2d_fullname_schema(),
&read_schema
));
}

// TODO(nlopes): the below require named schemas to be fully supported. See:
Expand All @@ -862,9 +853,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)
// );
// }

Expand All @@ -877,9 +867,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)
// );
// }

Expand All @@ -892,9 +881,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)
// );
// }

Expand Down
4 changes: 2 additions & 2 deletions lang/rust/src/ser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -375,7 +375,7 @@ impl<'a> ser::SerializeTupleVariant for SeqVariantSerializer<'a> {
}

fn end(self) -> Result<Self::Ok, Self::Error> {
Ok(ser::SerializeSeq::end(self)?)
ser::SerializeSeq::end(self)
}
}

Expand Down Expand Up @@ -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),
Expand Down
2 changes: 1 addition & 1 deletion lang/rust/src/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
4 changes: 3 additions & 1 deletion lang/rust/tests/io.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,9 @@ 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
);
}
}
Expand Down
3 changes: 2 additions & 1 deletion lang/rust/tests/schema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.to_string()
"{}",
e
);
Ok(())
} else {
Expand Down

0 comments on commit d1b4810

Please sign in to comment.