Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix clippy lints for rlp-derive #345

Merged
merged 1 commit into from
Feb 21, 2020
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
74 changes: 37 additions & 37 deletions rlp-derive/src/de.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,10 @@ fn decodable_wrapper_parse_quotes() -> ParseQuotes {
}

pub fn impl_decodable(ast: &syn::DeriveInput) -> TokenStream {
let body = match ast.data {
syn::Data::Struct(ref s) => s,
_ => panic!("#[derive(RlpDecodable)] is only defined for structs."),
let body = if let syn::Data::Struct(s) = &ast.data {
s
} else {
panic!("#[derive(RlpDecodable)] is only defined for structs.");
};

let mut default_attribute_encountered = false;
Expand Down Expand Up @@ -59,9 +60,10 @@ pub fn impl_decodable(ast: &syn::DeriveInput) -> TokenStream {
}

pub fn impl_decodable_wrapper(ast: &syn::DeriveInput) -> TokenStream {
let body = match ast.data {
syn::Data::Struct(ref s) => s,
_ => panic!("#[derive(RlpDecodableWrapper)] is only defined for structs."),
let body = if let syn::Data::Struct(s) = &ast.data {
s
} else {
panic!("#[derive(RlpDecodableWrapper)] is only defined for structs.");
};

let stmt = {
Expand Down Expand Up @@ -98,20 +100,21 @@ pub fn impl_decodable_wrapper(ast: &syn::DeriveInput) -> TokenStream {
}

fn decodable_field(
index: usize,
mut index: usize,
field: &syn::Field,
quotes: ParseQuotes,
default_attribute_encountered: &mut bool,
) -> TokenStream {
let id = match field.ident {
Some(ref ident) => quote! { #ident },
None => {
let index: syn::Index = index.into();
quote! { #index }
}
let id = if let Some(ident) = &field.ident {
quote! { #ident }
} else {
let index = syn::Index::from(index);
quote! { #index }
};

let index = index - *default_attribute_encountered as usize;
if *default_attribute_encountered {
index -= 1;
}
let index = quote! { #index };

let single = quotes.single;
Expand All @@ -123,7 +126,7 @@ fn decodable_field(
panic!("only 1 #[rlp(default)] attribute is allowed in a struct")
}
match attr.parse_args() {
Ok(proc_macro2::TokenTree::Ident(ident)) if ident.to_string() == "default" => {}
Ok(proc_macro2::TokenTree::Ident(ident)) if ident == "default" => {}
_ => panic!("only #[rlp(default)] attribute is supported"),
}
*default_attribute_encountered = true;
Expand All @@ -132,32 +135,29 @@ fn decodable_field(
false
};

match field.ty {
syn::Type::Path(ref path) => {
let ident = &path.path.segments.first().expect("there must be at least 1 segment").ident;
let ident_type = ident.to_string();
if &ident_type == "Vec" {
if quotes.takes_index {
if default {
quote! { #id: #list(#index).unwrap_or_default(), }
} else {
quote! { #id: #list(#index)?, }
}
if let syn::Type::Path(path) = &field.ty {
let ident = &path.path.segments.first().expect("there must be at least 1 segment").ident;
let ident_type = ident.to_string();
if ident_type == "Vec" {
if quotes.takes_index {
if default {
quote! { #id: #list(#index).unwrap_or_default(), }
} else {
quote! { #id: #list()?, }
quote! { #id: #list(#index)?, }
}
} else {
if quotes.takes_index {
if default {
quote! { #id: #single(#index).unwrap_or_default(), }
} else {
quote! { #id: #single(#index)?, }
}
} else {
quote! { #id: #single()?, }
}
quote! { #id: #list()?, }
}
} else if quotes.takes_index {
if default {
quote! { #id: #single(#index).unwrap_or_default(), }
} else {
quote! { #id: #single(#index)?, }
}
} else {
quote! { #id: #single()?, }
}
_ => panic!("rlp_derive not supported"),
} else {
panic!("rlp_derive not supported");
}
}
67 changes: 34 additions & 33 deletions rlp-derive/src/en.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,10 @@ use proc_macro2::TokenStream;
use quote::quote;

pub fn impl_encodable(ast: &syn::DeriveInput) -> TokenStream {
let body = match ast.data {
syn::Data::Struct(ref s) => s,
_ => panic!("#[derive(RlpEncodable)] is only defined for structs."),
let body = if let syn::Data::Struct(s) = &ast.data {
s
} else {
panic!("#[derive(RlpEncodable)] is only defined for structs.");
};

let stmts: Vec<_> = body.fields.iter().enumerate().map(|(i, field)| encodable_field(i, field)).collect();
Expand All @@ -38,9 +39,10 @@ pub fn impl_encodable(ast: &syn::DeriveInput) -> TokenStream {
}

pub fn impl_encodable_wrapper(ast: &syn::DeriveInput) -> TokenStream {
let body = match ast.data {
syn::Data::Struct(ref s) => s,
_ => panic!("#[derive(RlpEncodableWrapper)] is only defined for structs."),
let body = if let syn::Data::Struct(s) = &ast.data {
s
} else {
panic!("#[derive(RlpEncodableWrapper)] is only defined for structs.");
};

let stmt = {
Expand Down Expand Up @@ -72,38 +74,37 @@ pub fn impl_encodable_wrapper(ast: &syn::DeriveInput) -> TokenStream {
}

fn encodable_field(index: usize, field: &syn::Field) -> TokenStream {
let ident = match field.ident {
Some(ref ident) => quote! { #ident },
None => {
let index: syn::Index = index.into();
quote! { #index }
}
let ident = if let Some(ident) = &field.ident {
quote! { #ident }
} else {
let index = syn::Index::from(index);
quote! { #index }
};

let id = quote! { self.#ident };

match field.ty {
syn::Type::Path(ref path) => {
let top_segment = path.path.segments.first().expect("there must be at least 1 segment");
let ident = &top_segment.ident;
if &ident.to_string() == "Vec" {
let inner_ident = match top_segment.arguments {
syn::PathArguments::AngleBracketed(ref angle) => {
let ty = angle.args.first().expect("Vec has only one angle bracketed type; qed");
match *ty {
syn::GenericArgument::Type(syn::Type::Path(ref path)) => {
&path.path.segments.first().expect("there must be at least 1 segment").ident
}
_ => panic!("rlp_derive not supported"),
}
if let syn::Type::Path(path) = &field.ty {
let top_segment = path.path.segments.first().expect("there must be at least 1 segment");
let ident = &top_segment.ident;
if ident == "Vec" {
let inner_ident = {
if let syn::PathArguments::AngleBracketed(angle) = &top_segment.arguments {
if let syn::GenericArgument::Type(syn::Type::Path(path)) =
angle.args.first().expect("Vec has only one angle bracketed type; qed")
{
&path.path.segments.first().expect("there must be at least 1 segment").ident
} else {
panic!("rlp_derive not supported");
}
_ => unreachable!("Vec has only one angle bracketed type; qed"),
};
quote! { stream.append_list::<#inner_ident, _>(&#id); }
} else {
quote! { stream.append(&#id); }
}
} else {
unreachable!("Vec has only one angle bracketed type; qed")
}
};
quote! { stream.append_list::<#inner_ident, _>(&#id); }
} else {
quote! { stream.append(&#id); }
}
_ => panic!("rlp_derive not supported"),
} else {
panic!("rlp_derive not supported");
}
}
2 changes: 2 additions & 0 deletions rlp-derive/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
//! the field deserialization fails, as we don't serialize field
//! names and there is no way to tell if it is present or not.

#![warn(clippy::all, clippy::pedantic, clippy::nursery)]

extern crate proc_macro;

mod de;
Expand Down
40 changes: 20 additions & 20 deletions rlp-derive/tests/rlp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,62 +10,62 @@ use rlp::{decode, encode};
use rlp_derive::{RlpDecodable, RlpDecodableWrapper, RlpEncodable, RlpEncodableWrapper};

#[derive(Debug, PartialEq, RlpEncodable, RlpDecodable)]
struct Foo {
struct Item {
Copy link
Member

Choose a reason for hiding this comment

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

lol, I didn't know it this lint existed https://rust-lang.github.io/rust-clippy/current/index.html#blacklisted_name
Not sure how useful it is though

a: String,
}

#[derive(Debug, PartialEq, RlpEncodableWrapper, RlpDecodableWrapper)]
struct FooWrapper {
struct ItemWrapper {
a: String,
}

#[test]
fn test_encode_foo() {
let foo = Foo { a: "cat".into() };
fn test_encode_item() {
let item = Item { a: "cat".into() };

let expected = vec![0xc4, 0x83, b'c', b'a', b't'];
let out = encode(&foo);
let out = encode(&item);
assert_eq!(out, expected);

let decoded = decode(&expected).expect("decode failure");
assert_eq!(foo, decoded);
assert_eq!(item, decoded);
}

#[test]
fn test_encode_foo_wrapper() {
let foo = FooWrapper { a: "cat".into() };
fn test_encode_item_wrapper() {
let item = ItemWrapper { a: "cat".into() };

let expected = vec![0x83, b'c', b'a', b't'];
let out = encode(&foo);
let out = encode(&item);
assert_eq!(out, expected);

let decoded = decode(&expected).expect("decode failure");
assert_eq!(foo, decoded);
assert_eq!(item, decoded);
}

#[test]
fn test_encode_foo_default() {
fn test_encode_item_default() {
#[derive(Debug, PartialEq, RlpEncodable, RlpDecodable)]
struct FooDefault {
struct ItemDefault {
a: String,
/// It works with other attributes.
#[rlp(default)]
b: Option<Vec<u8>>,
}

let attack_of = String::from("clones");
let foo = Foo { a: attack_of.clone() };
let attack_of = "clones";
let item = Item { a: attack_of.into() };
Copy link
Member

Choose a reason for hiding this comment

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

it was a pun on attack of clones :(

Copy link
Contributor

Choose a reason for hiding this comment

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

Clippy has exactly zero sense of humour.


let expected = vec![0xc7, 0x86, b'c', b'l', b'o', b'n', b'e', b's'];
let out = encode(&foo);
let out = encode(&item);
assert_eq!(out, expected);

let foo_default = FooDefault { a: attack_of.clone(), b: None };
let item_default = ItemDefault { a: attack_of.into(), b: None };

let decoded = decode(&expected).expect("default failure");
assert_eq!(foo_default, decoded);
assert_eq!(item_default, decoded);

let foo_some = FooDefault { a: attack_of.clone(), b: Some(vec![1, 2, 3]) };
let out = encode(&foo_some);
assert_eq!(decode(&out), Ok(foo_some));
let item_some = ItemDefault { a: attack_of.into(), b: Some(vec![1, 2, 3]) };
let out = encode(&item_some);
assert_eq!(decode(&out), Ok(item_some));
}