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

rust: fix clippy issues #12189

Closed
wants to merge 4 commits into from
Closed

rust: fix clippy issues #12189

wants to merge 4 commits into from

Conversation

jlucovsky
Copy link
Contributor

Based on #12171

Backport clippy fixes to main-7.0.x

This prevents the clippy warning:

508 | #[derive(FromPrimitive, Debug)]
    |          ^------------
    |          |
    |          `FromPrimitive` is not local
    |          move the `impl` block outside of this constant `_IMPL_NUM_FromPrimitive_FOR_IsakmpPayloadType`
509 | pub enum IsakmpPayloadType {
    |          ----------------- `IsakmpPayloadType` is not local
    |
    = note: the derive macro `FromPrimitive` defines the non-local `impl`, and may need to be changed
    = note: the derive macro `FromPrimitive` may come from an old version of the `num_derive` crate, try updating your dependency with `cargo update -p num_derive`
    = note: an `impl` is never scoped, even when it is nested inside an item, as it may impact type checking outside of that item, which can be the case if neither the trait or the self type are at the same nesting level as the `impl`
    = note: items in an anonymous const item (`const _: () = { ... }`) are treated as in the same scope as the anonymous const's declaration for the purpose of this lint
    = note: this warning originates in the derive macro `FromPrimitive` (in Nightly builds, run with -Z macro-backtrace for more info)

(cherry picked from commit 8e408d3)
Fix provided by cargo clippy --fix.

(cherry picked from commit 7bdbe7e)
'///' style rust comments/documentation come before the item being
documented.

Spotted by clippy.

(cherry picked from commit aa6e94f)
But we should fix all these soon.

(cherry picked from commit 4c12165)
Copy link

codecov bot commented Dec 1, 2024

Codecov Report

Attention: Patch coverage is 0% with 6 lines in your changes missing coverage. Please review.

Project coverage is 83.16%. Comparing base (c3a6abf) to head (4e8f4db).
Report is 3 commits behind head on main-7.0.x.

Additional details and impacted files
@@              Coverage Diff               @@
##           main-7.0.x   #12189      +/-   ##
==============================================
- Coverage       83.18%   83.16%   -0.02%     
==============================================
  Files             922      922              
  Lines          260821   260827       +6     
==============================================
- Hits           216953   216922      -31     
- Misses          43868    43905      +37     
Flag Coverage Δ
fuzzcorpus 64.14% <0.00%> (+0.01%) ⬆️
suricata-verify 63.35% <0.00%> (-0.01%) ⬇️
unittests 62.38% <0.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

@suricata-qa
Copy link

Information: QA ran without warnings.

Pipeline 23619

Comment on lines +50 to +62
pub struct SCGeneralName<'a>(&'a GeneralName<'a>);

impl fmt::Display for SCGeneralName<'_> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self.0 {
GeneralName::DNSName(s) => write!(f, "{}", s),
GeneralName::URI(s) => write!(f, "{}", s),
GeneralName::IPAddress(s) => write!(f, "{:?}", s),
_ => write!(f, "{}", self.0)
}
}
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can skip this and the changes in this file. This was introduced for a feature that does not exist in 7.0.x.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Will make this change.

Copy link
Member

@inashivb inashivb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good. I think there is an accidental rebase/merge artifact added as pointed inline.

@jlucovsky
Copy link
Contributor Author

Continued in #12201

@jlucovsky jlucovsky closed this Dec 2, 2024
@jlucovsky jlucovsky deleted the clippy/1 branch December 5, 2024 13:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants