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 for generated code #254

Closed
tarikeshaq opened this issue Aug 25, 2020 · 1 comment · Fixed by #256
Closed

Fix clippy for generated code #254

tarikeshaq opened this issue Aug 25, 2020 · 1 comment · Fixed by #256

Comments

@tarikeshaq
Copy link
Contributor

tarikeshaq commented Aug 25, 2020

Right now running cargo clippy in a project that uses uniffi results in a few clippy errors, we could either try to fix them, or just disable clippy warnings in any spot that we can't cleanly make it happy. (We could also just say that consumers can deal with it as well if it's not worth our time)

Here is a long and ugly log from running clippy with the nimbus SDK:

/Users/teshaq/code/nimbus-sdk/experiments/target/debug/build/experiments-e671f8ff694721e7/out/nimbus.uniffi.rs:343:3
    |
343 | ) -> () {
    |   ^^^^^ help: remove the `-> ()`
    |
    = note: `#[warn(clippy::unused_unit)]` on by default
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unused_unit

warning: returning the result of a `let` binding from a block
   --> /Users/teshaq/code/nimbus-sdk/experiments/target/debug/build/experiments-e671f8ff694721e7/out/nimbus.uniffi.rs:357:9
    |
352 | /         let _retval = Experiments::opt_in_with_branch(
353 | |             obj,
354 | |             <String as uniffi::ViaFfi>::try_lift(experiment_slug).unwrap(),
355 | |             <String as uniffi::ViaFfi>::try_lift(branch).unwrap(),
356 | |         );
    | |__________- unnecessary `let` binding
357 |           _retval
    |           ^^^^^^^
    |
    = note: `#[warn(clippy::let_and_return)]` on by default
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#let_and_return
help: return the expression directly
    |
352 |         
353 |         Experiments::opt_in_with_branch(
354 |             obj,
355 |             <String as uniffi::ViaFfi>::try_lift(experiment_slug).unwrap(),
356 |             <String as uniffi::ViaFfi>::try_lift(branch).unwrap(),
357 |         )
    |

warning: unneeded unit return type
   --> /Users/teshaq/code/nimbus-sdk/experiments/target/debug/build/experiments-e671f8ff694721e7/out/nimbus.uniffi.rs:365:3
    |
365 | ) -> () {
    |   ^^^^^ help: remove the `-> ()`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unused_unit

warning: returning the result of a `let` binding from a block
   --> /Users/teshaq/code/nimbus-sdk/experiments/target/debug/build/experiments-e671f8ff694721e7/out/nimbus.uniffi.rs:378:9
    |
374 | /         let _retval = Experiments::opt_out(
375 | |             obj,
376 | |             <String as uniffi::ViaFfi>::try_lift(experiment_slug).unwrap(),
377 | |         );
    | |__________- unnecessary `let` binding
378 |           _retval
    |           ^^^^^^^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#let_and_return
help: return the expression directly
    |
374 |         
375 |         Experiments::opt_out(
376 |             obj,
377 |             <String as uniffi::ViaFfi>::try_lift(experiment_slug).unwrap(),
378 |         )
    |

warning: unneeded unit return type
   --> /Users/teshaq/code/nimbus-sdk/experiments/target/debug/build/experiments-e671f8ff694721e7/out/nimbus.uniffi.rs:383:63
    |
383 | pub extern "C" fn nimbus_Experiments_opt_out_all(handle: u64) -> () {
    |                                                               ^^^^^ help: remove the `-> ()`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unused_unit

warning: returning the result of a `let` binding from a block
   --> /Users/teshaq/code/nimbus-sdk/experiments/target/debug/build/experiments-e671f8ff694721e7/out/nimbus.uniffi.rs:393:9
    |
392 |         let _retval = Experiments::opt_out_all(obj);
    |         -------------------------------------------- unnecessary `let` binding
393 |         _retval
    |         ^^^^^^^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#let_and_return
help: return the expression directly
    |
392 |         
393 |         Experiments::opt_out_all(obj)
    |

warning: unneeded unit return type
   --> /Users/teshaq/code/nimbus-sdk/experiments/target/debug/build/experiments-e671f8ff694721e7/out/nimbus.uniffi.rs:401:3
    |
401 | ) -> () {
    |   ^^^^^ help: remove the `-> ()`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unused_unit

/ pub unsafe extern "C" fn ffi_nimbus_string_alloc_from(
28 | |     cstr: *const std::os::raw::c_char,
29 | |     err: &mut uniffi::deps::ffi_support::ExternError,
30 | | ) -> *mut std::os::raw::c_char {
...  |
44 | |     })
45 | | }
   | |_^
   |
   = note: `#[warn(clippy::missing_safety_doc)]` on by default
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#missing_safety_doc

warning: unsafe function's docs miss `# Safety` section
  --> /Users/teshaq/code/nimbus-sdk/experiments/target/debug/build/experiments-e671f8ff694721e7/out/nimbus.uniffi.rs:52:1
   |
52 | / pub unsafe extern "C" fn ffi_nimbus_string_free(cstr: *mut std::os::raw::c_char) {
53 | |     // We deliberately don't check the `Result` here, so that callers don't need to pass an out `err`.
54 | |     // There was nothing for us to free in the failure case anyway, so no point in propagating the error.
55 | |     let s = <String as uniffi::ViaFfi>::try_lift(cstr);
56 | |     drop(s)
57 | | }
   | |_^
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#missing_safety_doc

warning: passing a unit value to a function
   --> /Users/teshaq/code/nimbus-sdk/experiments/target/debug/build/experiments-e671f8ff694721e7/out/nimbus.uniffi.rs:408:12
    |
408 |         Ok(_retval)
    |            ^^^^^^^
    |
    = note: `#[warn(clippy::unit_arg)]` on by default
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unit_arg
help: if you intended to pass a unit value, use a unit literal instead
    |
408 |         Ok(())

┆Issue is synchronized with this Jira Task

@eoger
Copy link
Contributor

eoger commented Aug 27, 2020

Seems like these errors come from these 2 macros, but it's kinda hard to fix. I would suggest simply disabling clippy as we don't really expect the consumer of these bindings to read the source code directly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants