Skip to content

Commit

Permalink
WIP: actually fix clippy lints
Browse files Browse the repository at this point in the history
  • Loading branch information
DCNick3 committed Aug 22, 2023
1 parent ef36616 commit 253155d
Show file tree
Hide file tree
Showing 5 changed files with 38 additions and 34 deletions.
3 changes: 3 additions & 0 deletions ffi/derive/src/attr_parse/derive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ impl RustcDerive {
}
}

#[allow(variant_size_differences)] // it's not like it's possible to change that..
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
pub enum Derive {
Rustc(RustcDerive),
Expand Down Expand Up @@ -79,6 +80,8 @@ impl FromAttributes for DeriveAttr {
};

for path in paths {
// what clippy suggests here is much harder to read
#[allow(clippy::option_if_let_else)]
let derive = if let Some(derive) = RustcDerive::try_from_path(&path) {
Derive::Rustc(derive)
} else if let Some(derive) = GetSetDerive::try_from_path(&path) {
Expand Down
23 changes: 11 additions & 12 deletions ffi/derive/src/attr_parse/getset.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,17 +22,16 @@ impl GetSetDerive {
// try to be smart and handle two cases:
// - bare attribute name (like `Getters`, when it's imported)
// - fully qualified path (like `getset::Getters`, when it's not imported)
let ident = match path.get_ident() {
Some(i) => i.clone(),
None => {
let mut segments = path.segments.iter();
if segments.len() == 2
&& segments.next().unwrap().ident.to_string().as_str() == "getset"
{
segments.next().unwrap().ident.clone()
} else {
return None;
}
let ident = if let Some(i) = path.get_ident() {
i.clone()
} else {
let mut segments = path.segments.iter();
if segments.len() == 2
&& segments.next().unwrap().ident.to_string().as_str() == "getset"
{
segments.next().unwrap().ident.clone()
} else {
return None;
}
};

Expand All @@ -45,7 +44,7 @@ impl GetSetDerive {
}
}

pub fn get_mode(&self) -> GetSetGenMode {
pub fn get_mode(self) -> GetSetGenMode {
match self {
Self::Setters => GetSetGenMode::Set,
Self::Getters => GetSetGenMode::Get,
Expand Down
38 changes: 20 additions & 18 deletions ffi/derive/src/convert.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,5 @@
use core::str::FromStr as _;
use std::{
fmt::{Display, Formatter},
ops::Deref,
};
use std::fmt::{Display, Formatter};

use darling::{
ast::Style, util::SpannedValue, FromAttributes, FromDeriveInput, FromField, FromVariant,
Expand Down Expand Up @@ -157,7 +154,7 @@ fn parse_ffi_type_attr<T: syn2::parse::Parse>(attrs: &[Attribute]) -> darling::R
darling::Error::custom("Only one #[ffi_type] attribute is allowed!").with_span(
&tail
.iter()
.map(|a| a.span())
.map(syn2::spanned::Spanned::span)
.reduce(|a, b| a.join(b).unwrap())
.unwrap(),
),
Expand Down Expand Up @@ -293,8 +290,8 @@ impl FromField for FfiTypeField {
}
}

pub fn derive_ffi_type(emitter: &mut Emitter, input: syn2::DeriveInput) -> TokenStream {
let Some(mut input) = emitter.handle(FfiTypeInput::from_derive_input(&input)) else {
pub fn derive_ffi_type(emitter: &mut Emitter, input: &syn2::DeriveInput) -> TokenStream {
let Some(mut input) = emitter.handle(FfiTypeInput::from_derive_input(input)) else {
return quote!();
};

Expand Down Expand Up @@ -335,7 +332,12 @@ pub fn derive_ffi_type(emitter: &mut Emitter, input: syn2::DeriveInput) -> Token
)
}

derive_ffi_type_for_fieldless_enum(emitter, &input.ident, variants, input.repr_attr)
derive_ffi_type_for_fieldless_enum(
emitter,
&input.ident,
variants,
&input.repr_attr,
)
} else {
verify_is_non_owning(emitter, &input.data);
let local = input.ffi_type_attr.kind == Some(FfiTypeKindAttribute::Local);
Expand Down Expand Up @@ -467,23 +469,23 @@ fn derive_ffi_type_for_fieldless_enum(
emitter: &mut Emitter,
enum_name: &Ident,
variants: &[SpannedValue<FfiTypeVariant>],
repr: Repr,
repr: &Repr,
) -> TokenStream {
let enum_repr_type = get_enum_repr_type(emitter, enum_name, repr, variants.is_empty());
// FIXME: I think this doesn't actually require variant names, just using a range would suffice
// (not that we don't support custom discriminants)
let (discriminants, discriminant_decls) =
gen_discriminants(enum_name, variants, &enum_repr_type);

let match_ = if !discriminants.is_empty() {
let match_ = if discriminants.is_empty() {
quote! {false}
} else {
quote! {
match *target {
#( | #discriminants )* => true,
_ => false,
}
}
} else {
quote! {false}
};

quote! {
Expand Down Expand Up @@ -883,23 +885,23 @@ fn gen_repr_c_enum_payload_name(enum_name: &Ident) -> Ident {
// that is at the same time Robust and also transfers ownership
/// Verifies for each pointer type found inside the `FfiTypeData` that it is marked as non-owning
fn verify_is_non_owning(emitter: &mut Emitter, data: &FfiTypeData) {
struct PtrVistor<'a> {
struct PtrVisitor<'a> {
emitter: &'a mut Emitter,
}
impl<'a> syn2::visit::Visit<'_> for PtrVistor<'a> {
impl syn2::visit::Visit<'_> for PtrVisitor<'_> {
fn visit_type_ptr(&mut self, node: &syn2::TypePtr) {
emit!(self.emitter, node, "Raw pointer found. If the pointer doesn't own the data, attach `#[ffi_type(unsafe {{non_owning}})` to the field. Otherwise, mark the entire type as opaque with `#[ffi_type(opaque)]`");
}
}

fn visit_field(ptr_visitor: &mut PtrVistor, field: &FfiTypeField) {
fn visit_field(ptr_visitor: &mut PtrVisitor, field: &FfiTypeField) {
if field.ffi_type_attr.kind == Some(FfiTypeKindFieldAttribute::UnsafeNonOwning) {
return;
}
ptr_visitor.visit_type(&field.ty);
}

let mut ptr_visitor = PtrVistor { emitter };
let mut ptr_visitor = PtrVisitor { emitter };
match data {
FfiTypeData::Enum(variants) => {
for variant in variants {
Expand All @@ -919,7 +921,7 @@ fn verify_is_non_owning(emitter: &mut Emitter, data: &FfiTypeData) {
fn get_enum_repr_type(
emitter: &mut Emitter,
enum_name: &Ident,
repr: Repr,
repr: &Repr,
is_empty: bool,
) -> syn2::Type {
let Some(kind) = repr.kind else {
Expand All @@ -932,7 +934,7 @@ fn get_enum_repr_type(
return syn2::parse_quote! {u32}
};

let ReprKind::Primitive(primitive) = kind.deref() else {
let ReprKind::Primitive(primitive) = &*kind else {
emit!(emitter, &kind.span(), "Enum should have a primitive representation (like `#[repr(u32)]`)");
return syn2::parse_quote! {u32}
};
Expand Down
6 changes: 3 additions & 3 deletions ffi/derive/src/impl_visitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,7 @@ impl<'ast, 'emitter> FnVisitor<'ast, 'emitter> {
}
}

impl<'ast, 'emitter> Visit<'ast> for ImplVisitor<'ast, 'emitter> {
impl<'ast> Visit<'ast> for ImplVisitor<'ast, '_> {
fn visit_attribute(&mut self, node: &'ast syn2::Attribute) {
self.attrs.push(node);
}
Expand Down Expand Up @@ -320,7 +320,7 @@ impl<'ast, 'emitter> Visit<'ast> for ImplVisitor<'ast, 'emitter> {
_ => None,
}));

for item in node.items.iter() {
for item in &node.items {
if let syn2::ImplItem::Fn(method) = item {
// NOTE: private methods in inherent impl are skipped
if self.trait_name.is_none() && !matches!(method.vis, Visibility::Public(_)) {
Expand All @@ -336,7 +336,7 @@ impl<'ast, 'emitter> Visit<'ast> for ImplVisitor<'ast, 'emitter> {
}
}

impl<'ast, 'emitter> Visit<'ast> for FnVisitor<'ast, 'emitter> {
impl<'ast> Visit<'ast> for FnVisitor<'ast, '_> {
fn visit_attribute(&mut self, node: &'ast syn2::Attribute) {
if is_doc_attr(node) {
self.doc.push(node);
Expand Down
2 changes: 1 addition & 1 deletion ffi/derive/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ pub fn ffi_type_derive(input: TokenStream) -> TokenStream {
manyhow::emit!(emitter, item, "Only public types are allowed in FFI");
}

let result = derive_ffi_type(&mut emitter, item);
let result = derive_ffi_type(&mut emitter, &item);
emitter.finish_token_stream(result)
}

Expand Down

0 comments on commit 253155d

Please sign in to comment.