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 lints #149

Merged
merged 1 commit into from
Apr 26, 2018
Merged

Fix clippy lints #149

merged 1 commit into from
Apr 26, 2018

Conversation

hmvp
Copy link
Contributor

@hmvp hmvp commented Mar 2, 2018

The docs are not yet published for the latest clippy but this pull request fixes two clippy warnings you get when using bitflags! in your code

https://rust-lang-nursery.github.io/rust-clippy/v0.0.187/index.html#redundant_field_names
https://rust-lang-nursery.github.io/rust-clippy/v0.0.187/index.html#suspicious_arithmetic_impl

Copy link
Member

@KodrAus KodrAus left a comment

Choose a reason for hiding this comment

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

Thanks @hmvp! This looks good to me. For reference, looks like suspicious_arithmetic_impl is just about using different operations in implementations for things like Add and Subtract than the one being implemented (like using - in an impl for Add), which is why we ignore that one.

chrisduerr added a commit to alacritty/alacritty that referenced this pull request Mar 4, 2018
Unwrapping inside the config file parsing can lead to some issues that
prevent us from falling back to a default configuration file.

One instance of that issue was mentioned in #1135.

Now all instances of `unwrap()` have been removed and replaced with
proper error handling. This will make the config more robust and
prevents live reload from silently breaking while alacritty is running.

This also fixes a few currently existing clippy issues.
Clippy added an additonal lint which complains about `MyStruct { field:
field }`.

These issues have been fixed, except for some false-positives and issues
in external macros which will probably be fixed with future updates (bitflags/bitflags#149)
@niklasf
Copy link
Contributor

niklasf commented Mar 6, 2018

Regarding the implementation of Sub we should consider making clippy a bit more relaxed: rust-lang/rust-clippy#2510

@niklasf
Copy link
Contributor

niklasf commented Mar 6, 2018

Update: Clippy has been adjusted rust-lang/rust-clippy#2511, so we no longer need to ignore suspicious_arithmetic_impl.

@hmvp
Copy link
Contributor Author

hmvp commented Mar 6, 2018

Updated the change to remove the now unneeded allow

metayan pushed a commit to metayan/alacritty that referenced this pull request Mar 11, 2018
Unwrapping inside the config file parsing can lead to some issues that
prevent us from falling back to a default configuration file.

One instance of that issue was mentioned in alacritty#1135.

Now all instances of `unwrap()` have been removed and replaced with
proper error handling. This will make the config more robust and
prevents live reload from silently breaking while alacritty is running.

This also fixes a few currently existing clippy issues.
Clippy added an additonal lint which complains about `MyStruct { field:
field }`.

These issues have been fixed, except for some false-positives and issues
in external macros which will probably be fixed with future updates (bitflags/bitflags#149)
@mulkieran
Copy link

It looks like clippy may have made both allows unneeded now: stratis-storage/devicemapper-rs#287

@niklasf
Copy link
Contributor

niklasf commented Mar 16, 2018

Yes, redundant field names are ignored in macros now: rust-lang/rust-clippy@ed769a3.

We should still commit this small cleanup. It's not a breaking change because the shorthand syntax was stabilized in Rust 1.17 and bitflags already requires 1.20.

chrisduerr added a commit to chrisduerr/alacritty that referenced this pull request Apr 14, 2018
Unwrapping inside the config file parsing can lead to some issues that
prevent us from falling back to a default configuration file.

One instance of that issue was mentioned in alacritty#1135.

Now all instances of `unwrap()` have been removed and replaced with
proper error handling. This will make the config more robust and
prevents live reload from silently breaking while alacritty is running.

This also fixes a few currently existing clippy issues.
Clippy added an additonal lint which complains about `MyStruct { field:
field }`.

These issues have been fixed, except for some false-positives and issues
in external macros which will probably be fixed with future updates (bitflags/bitflags#149)
Copy link
Contributor

@dtolnay dtolnay left a comment

Choose a reason for hiding this comment

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

Thanks! Indeed this is a nice cleanup whether or not it still triggers Clippy, and our minimum supported Rust version of 1.20.0 should be fine with the field init shorthand syntax.

@dtolnay dtolnay merged commit a9a1dd8 into bitflags:master Apr 26, 2018
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.

None yet

5 participants