Skip to content

Commit

Permalink
Merge #853
Browse files Browse the repository at this point in the history 
853: Clippy warnings on nightly + first cargo-deny entry r=Bromeon a=Bromeon

### clippy warnings

The first commit fixes warnings on nightly:
```rs
  Checking gdnative-derive v0.9.3 (/home/runner/work/godot-rust/godot-rust/gdnative-derive)
error: methods called `from_*` usually take no `self`
   --> gdnative-derive/src/variant/repr.rs:174:9
    |
174 |         &self,
    |         ^^^^^
    |
    = note: `-D clippy::wrong-self-convention` implied by `-D clippy::style`
    = help: consider choosing a less ambiguous name
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#wrong_self_convention

error: methods called `from_*` usually take no `self`
   --> gdnative-derive/src/variant/repr.rs:330:21
    |
330 |     fn from_variant(&self, variant: &TokenStream2) -> TokenStream2 {
    |                     ^^^^^
    |
    = help: consider choosing a less ambiguous name
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#wrong_self_convention
```

---

### cargo-deny entry

After reading [this Reddit post](https://www.reddit.com/r/rust/comments/sdgcg9/unsoundness_in_owning_ref), the `owning_ref` crate seemed to be a good candidate for cargo-deny. First, it hasn't been maintained in two years, and second, there is [known unsoundness](https://github.com/noamtashma/owning-ref-unsoundness) but it's not marked in any CVE database.

I'm not suggesting we go hunt down crates now, but if people come across other problematic crates, let's add them. That doesn't mean there will be a strict ban on such crates forever, but it means adding a potentially problematic dependency is a concious, opt-in process.

bors try

Co-authored-by: Jan Haller <bromeon@gmail.com>
  • Loading branch information
@bors @Bromeon
bors[bot] and Bromeon committed Jan 29, 2022
2 parents 42a6e76 + 668f3a1 commit 1f3b19d
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 21 deletions.
5 changes: 3 additions & 2 deletions gdnative-derive/src/variant/from.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ pub(crate) fn expand_from_variant(derive_data: DeriveData) -> Result<TokenStream

let return_expr = match repr {
Repr::Struct(var_repr) => {
let from_variant = var_repr.from_variant(&input_ident, &quote! { #ident })?;
let from_variant = var_repr.make_from_variant_expr(&input_ident, &quote! { #ident })?;
quote! {
{
#from_variant
Expand Down Expand Up @@ -54,7 +54,8 @@ pub(crate) fn expand_from_variant(derive_data: DeriveData) -> Result<TokenStream
let var_from_variants = variants
.iter()
.map(|(var_ident, var_repr)| {
var_repr.from_variant(&var_input_ident, &quote! { #ident::#var_ident })
var_repr
.make_from_variant_expr(&var_input_ident, &quote! { #ident::#var_ident })
})
.collect::<Result<Vec<_>, _>>()?;

Expand Down
20 changes: 10 additions & 10 deletions gdnative-derive/src/variant/repr.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ impl VariantRepr {
}
}

pub(crate) fn to_variant(
pub(crate) fn make_to_variant_expr(
&self,
trait_kind: ToVariantTrait,
) -> Result<TokenStream2, syn::Error> {
Expand All @@ -119,13 +119,13 @@ impl VariantRepr {
"cannot skip the only field in a tuple",
));
}
field.to_variant(trait_kind)
field.make_to_variant_expr(trait_kind)
} else {
let exprs = fields.iter().filter_map(|f| {
if f.attr.skip_to_variant {
None
} else {
Some(f.to_variant(trait_kind))
Some(f.make_to_variant_expr(trait_kind))
}
});

Expand All @@ -150,7 +150,7 @@ impl VariantRepr {
let name_string_literals =
name_strings.iter().map(|string| Literal::string(string));

let exprs = fields.iter().map(|f| f.to_variant(trait_kind));
let exprs = fields.iter().map(|f| f.make_to_variant_expr(trait_kind));

quote! {
{
Expand All @@ -170,7 +170,7 @@ impl VariantRepr {
Ok(tokens)
}

pub(crate) fn from_variant(
pub(crate) fn make_from_variant_expr(
&self,
variant: &Ident,
ctor: &TokenStream2,
Expand Down Expand Up @@ -199,7 +199,7 @@ impl VariantRepr {
"cannot skip the only field in a tuple",
));
}
let expr = field.from_variant(&quote!(#variant));
let expr = field.make_from_variant_expr(&quote!(#variant));
quote! {
{
#expr.map(#ctor)
Expand All @@ -224,7 +224,7 @@ impl VariantRepr {
let expr_variant = &quote!(&__array.get(__index));
let non_skipped_exprs = non_skipped_fields
.iter()
.map(|f| f.from_variant(expr_variant));
.map(|f| f.make_from_variant_expr(expr_variant));

quote! {
{
Expand Down Expand Up @@ -283,7 +283,7 @@ impl VariantRepr {
let expr_variant = &quote!(&__dict.get_or_nil(&__key));
let exprs = non_skipped_fields
.iter()
.map(|f| f.from_variant(expr_variant));
.map(|f| f.make_from_variant_expr(expr_variant));

quote! {
{
Expand Down Expand Up @@ -317,7 +317,7 @@ impl VariantRepr {
}

impl Field {
fn to_variant(&self, trait_kind: ToVariantTrait) -> TokenStream2 {
fn make_to_variant_expr(&self, trait_kind: ToVariantTrait) -> TokenStream2 {
let Field { ident, attr, .. } = self;
if let Some(to_variant_with) = &attr.to_variant_with {
quote!(#to_variant_with(#ident))
Expand All @@ -327,7 +327,7 @@ impl Field {
}
}

fn from_variant(&self, variant: &TokenStream2) -> TokenStream2 {
fn make_from_variant_expr(&self, variant: &TokenStream2) -> TokenStream2 {
if let Some(from_variant_with) = &self.attr.from_variant_with {
quote!(#from_variant_with(#variant))
} else {
Expand Down
4 changes: 2 additions & 2 deletions gdnative-derive/src/variant/to.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ pub(crate) fn expand_to_variant(
let return_expr = match repr {
Repr::Struct(var_repr) => {
let destructure_pattern = var_repr.destructure_pattern();
let to_variant = var_repr.to_variant(trait_kind)?;
let to_variant = var_repr.make_to_variant_expr(trait_kind)?;
quote! {
{
let #ident #destructure_pattern = self;
Expand All @@ -43,7 +43,7 @@ pub(crate) fn expand_to_variant(
.iter()
.map(|(var_ident, var_repr)| {
let destructure_pattern = var_repr.destructure_pattern();
let to_variant = var_repr.to_variant(trait_kind)?;
let to_variant = var_repr.make_to_variant_expr(trait_kind)?;
let var_ident_string = format!("{}", var_ident);
let var_ident_string_literal = Literal::string(&var_ident_string);
let tokens = quote! {
Expand Down
16 changes: 9 additions & 7 deletions tools/deny.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -165,14 +165,16 @@ allow = [
#{ name = "ansi_term", version = "=0.11.0" },
]
# List of crates to deny
# Each entry the name of a crate and a version range. If version is
# not specified, all versions will be matched.
#{ name = "ansi_term", version = "=0.11.0" },
#
# Wrapper crates can optionally be specified to allow the crate when it
# is a direct dependency of the otherwise banned crate
#{ name = "ansi_term", version = "=0.11.0", wrappers = [] },
deny = [
# Each entry the name of a crate and a version range. If version is
# not specified, all versions will be matched.
#{ name = "ansi_term", version = "=0.11.0" },
#
# Wrapper crates can optionally be specified to allow the crate when it
# is a direct dependency of the otherwise banned crate
#{ name = "ansi_term", version = "=0.11.0", wrappers = [] },
# unmaintained since Feb 20 + https://github.com/noamtashma/owning-ref-unsoundness
{ name = "owning_ref" },
]
# Certain crates/versions that will be skipped when doing duplicate detection.
skip = [
Expand Down

0 comments on commit 1f3b19d

Please sign in to comment.