Skip to content

Check for more Clippy lints #91

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

Closed
hug-dev opened this issue Jan 24, 2020 · 5 comments
Closed

Check for more Clippy lints #91

hug-dev opened this issue Jan 24, 2020 · 5 comments
Assignees
Labels
enhancement New feature or request

Comments

@hug-dev
Copy link
Member

hug-dev commented Jan 24, 2020

The following command:

cargo clippy --all-targets --all-features -- -D clippy::all -D clippy::pedantic -D clippy::cargo

will enable all Clippy lints in all Parsec's code.
We should investigate which ones of those should be fixed.

@hug-dev hug-dev added the enhancement New feature or request label Jan 24, 2020
@hug-dev hug-dev added this to the Parsec production ready milestone Jan 24, 2020
@hug-dev
Copy link
Member Author

hug-dev commented Jan 24, 2020

The same should be done in our other crates.

@hug-dev
Copy link
Member Author

hug-dev commented Feb 3, 2020

All of our operation structures (example: OpPing) are declared in their src/operations/... (example: ping.rs) file.
Clippy, with the pedantic lint group enabled complains about that as it is a duplication of name:

error: item name ends with its containing module's name
  --> src/operations/ping.rs:18:1
   |
18 | pub struct OpPing;
   | ^^^^^^^^^^^^^^^^^^
   |
   = note: `-D clippy::module-name-repetitions` implied by `-D clippy::pedantic`
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#module_name_repetitions

error: item name ends with its containing module's name
  --> src/operations/ping.rs:26:1
   |
26 | / pub struct ResultPing {
27 | |     pub supp_version_maj: u8,
28 | |     pub supp_version_min: u8,
29 | | }
   | |_^
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#module_name_repetitions

The absolute name of the structure is operations::ping::OpPing but using namespacing we could rename this structure to be Operation only and so its absolute name would be operations::ping::Operation
In context where we need to use all the OpXXX or all the ResultXXX, we can import operations::{ping, etc} and use the different ones with ping::Operation, ping::Result, etc...

What are you thoughts about that?

@ionut-arm
Copy link
Member

That sounds good! We could eiither use ping::Operation or simply re-export them with an alias at the root of the library, i.e. pub use ...::ping::Operation as OpPing

@hug-dev
Copy link
Member Author

hug-dev commented Feb 3, 2020

Following the same logic we should also rename all of our Protobuf operations to remove the PingProto from OpPingProto and ResultPingProto (and all of them) as it will be namespaced correctly (to be checked in other languages, like C).
Otherwise we have the same issue:

use super::generated_ops::ping::{OpPingProto, ResultPingProto};

which seems a bit weird if we do not fix as well.
That would incur a lot of changes to leaving that issue for now.

@hug-dev
Copy link
Member Author

hug-dev commented Feb 3, 2020

I will for now add cargo clippy --all-targets --all-features -- -D clippy::all -D clippy::cargo to our CIs and create a new issue (out of the 1.0.0 milestone) to add the pedantic one.

hug-dev added a commit to hug-dev/rust-tss-esapi that referenced this issue Feb 3, 2020
As per parallaxsecond/parsec#91 adds the stronger lints. They currently
all pass.

Signed-off-by: Hugues de Valon <[email protected]>
hug-dev added a commit to hug-dev/parsec-client-test that referenced this issue Feb 3, 2020
As per parallaxsecond/parsec#91

Signed-off-by: Hugues de Valon <[email protected]>
hug-dev added a commit to hug-dev/parsec-client-test that referenced this issue Feb 4, 2020
As per parallaxsecond/parsec#91

Signed-off-by: Hugues de Valon <[email protected]>
@hug-dev hug-dev closed this as completed Feb 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants