librasn / rasn

A Safe #[no_std] ASN.1 Codec Framework
Other
183 stars 43 forks source link

WIP: Add clippy and fix reported issues #183

Closed wiktor-k closed 3 months ago

wiktor-k commented 8 months ago

Hi, I've been working on adding clippy to this project and most of the issues are minor but I'll need your help to resolve the remaining ones:

  1. Using to_* name but not consuming self: This would require changing function signature:
warning: methods with the following characteristics: (`to_*` and `self` type is `Copy`) usually take `self` by value
  --> macros/src/tag.rs:37:22
   |
37 |     pub fn to_tokens(&self, crate_root: &syn::Path) -> proc_macro2::TokenStream {
   |                      ^^^^^
   = 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
  1. Interior mutability of OctetString:
warning: a `const` item should never be interior mutable
  --> standards/cms/src/authenticode.rs:31:1
   |
31 |   pub const SPC_CLASS_UUID: OctetString = OctetString::from_static(&[
   |   ^----
   |   |
   |  _make this a static item (maybe with lazy_static)
   | |
32 | |     0xa6, 0xb5, 0x86, 0xd5, 0xb4, 0xa1, 0x24, 0x66, 0xae, 0x05, 0xa2, 0x17, 0xda, 0x8e, 0x60, 0xd6,
33 | | ]);
   | |___^
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#declare_interior_mutable_const
  1. Encode adds mutable key type (which annoys Hash derive):
warning: mutable key type
  --> standards/cms/src/pkcs7_compat.rs:24:41
   |
24 | #[derive(AsnType, Clone, Debug, Decode, Encode, PartialEq, Eq, PartialOrd, Ord, Hash)]
   |                                         ^^^^^^ in this derive macro expansion
   |
  ::: /home/wiktor/src/open-source/rasn/macros/src/lib.rs:54:1
   |
54 | pub fn encode_derive(input: proc_macro::TokenStream) -> proc_macro::TokenStream {
   | ------------------------------------------------------------------------------- in this expansion of `#[derive(Encode)]`
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#mutable_key_type

I'm not quite sure if some of them are false positives and I think you could comment on that.

(for the record I ran cargo clippy --all locally)

XAMPPRocky commented 8 months ago

to_tokens

We can probably make this as_tokens.

warning: a const item should never be interior mutable

This can be set to allow, making it static makes no sense, the behaviour of const is the intuitive behaviour here.

Encode adds mutable key type (which annoys Hash derive):

I'd need to see the underlying issue for it, you can check the code using cargo expand to generate and then run clippy on it.

XAMPPRocky commented 3 months ago

@wiktor-k Friendly ping 🙂

wiktor-k commented 3 months ago

Thanks for the ping, I actually got busy with other work :)

I've replaced the PR with https://github.com/librasn/rasn/pull/234 (it was easier to start from main and re-fix what was left).