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

clippy: fix multiple issues #293

Merged
merged 5 commits into from
Mar 30, 2023
Merged

Conversation

micolous
Copy link
Collaborator

@micolous micolous commented Mar 29, 2023

Fixes multiple clippy failures in CI:

  • (error) derivable_impls for many Default implementations (added in new clippy)
  • (error) expect_used regressions missed in making a basic test for a webauthn object from a chrome extension #289
  • (error) mismatched image library versions from bardecoder
  • (warning) wrong_self_convention in Eid::to_bytes()
  • (warning) bool_assert_comparison in nfc/atr.rs tests
  • (warning) box_default in multiple places
  • cargo test has been run and passes
  • documentation has been updated with relevant examples (if relevant)

@yaleman
Copy link
Member

yaleman commented Mar 29, 2023

That's ... a lot of CI failures O_O

@conr2d conr2d mentioned this pull request Mar 29, 2023
2 tasks
@micolous
Copy link
Collaborator Author

I missed two issues which #294 got; I've cherry-picked the extra fixes in b7b9ab9 onto my branch as 4fa07ac.

@micolous micolous mentioned this pull request Mar 29, 2023
2 tasks
@micolous
Copy link
Collaborator Author

micolous commented Mar 29, 2023

Remaining clippy failures appear to be regressions introduced in e9ebff3 (#289), where the existing CI failures stopped clippy checking webauthn-rs where these new failures were present:

error: used `expect()` on a `Result` value
    --> webauthn-rs/src/lib.rs:1234:21
     |
1234 |     let rp_origin = Url::parse(&format!("chrome-extension://{rp_id}")).expect("Invalid URL");
     |                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
     |
     = help: if this value is an `Err`, it will panic
     = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#expect_used
note: the lint level is defined here
    --> webauthn-rs/src/lib.rs:117:9
     |
117  | #![deny(clippy::expect_used)]
     |         ^^^^^^^^^^^^^^^^^^^

error: used `expect()` on a `Result` value
    --> webauthn-rs/src/lib.rs:1236:19
     |
1236 |     let builder = WebauthnBuilder::new(rp_id, &rp_origin).expect("Invalid configuration");
     |                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
     |
     = help: if this value is an `Err`, it will panic
     = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#expect_used

error: used `expect()` on a `Result` value
    --> webauthn-rs/src/lib.rs:1238:17
     |
1238 |     let built = builder.build().expect("Failed to build");
     |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
     |
     = help: if this value is an `Err`, it will panic
     = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#expect_used

These are both used in a test.

@micolous micolous changed the title clippy: fix multiple derivable_impls issues clippy: fix multiple issues Mar 29, 2023
@micolous
Copy link
Collaborator Author

There ended up being more clippy failures than just that, but I believe this is all in order now.

@yaleman yaleman merged commit 9c54c07 into kanidm:master Mar 30, 2023
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 this pull request may close these issues.

4 participants