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.

Sign up for GitHub

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

Jump to bottom

[suggestion] Deprecate cargo lints #3622

Closed
appetrosyan opened this issue Jun 19, 2023 · 2 comments
Closed

[suggestion] Deprecate cargo lints #3622

appetrosyan opened this issue Jun 19, 2023 · 2 comments
Labels
Chore This is a small task that can be done at any point in time and is easier than others Enhancement New feature or request iroha2-dev The re-implementation of a BFT hyperledger in RUST

Comments

@appetrosyan
Copy link
Contributor

Feature request

Inline all of the general deny lints into the top levels of each affected crate.

Motivation

cargo lints is a package that is effecitvely a thin wrapper around clippy. The issue with building our CI around it is as follows

  1. It applies to the entire workspace. Some lints are enforced as a matter of policy, e.g. groups like pedantic and restriction. However, most are specific to the crates in which they exist. For example, arithmetic_side_effects is only sensible in crates where we do arithmetic. This rules out logger and telemetry.
  2. It applies to tests and benchmarks, even though the standards for those crates are different. Sometimes expressive tests take shortcuts for the sake of readability and coverage.
  3. It requires more tools to be installed on the potential contributor's system.
  4. cargo lints hasn't been updated since May 2021. Likely won't be.
  5. It doesn't properly use clippy-driver which can potentially break Rust in the future.

Who can help?

@appetrosyan
@appetrosyan appetrosyan added Enhancement New feature or request iroha2-dev The re-implementation of a BFT hyperledger in RUST labels Jun 19, 2023
@appetrosyan appetrosyan mentioned this issue Jul 19, 2023
Closed
4 tasks
@appetrosyan appetrosyan self-assigned this Jul 19, 2023
@appetrosyan appetrosyan added the Chore This is a small task that can be done at any point in time and is easier than others label Jul 19, 2023
@DCNick3 DCNick3 self-assigned this Sep 18, 2023
DCNick3 added a commit to DCNick3/iroha that referenced this issue Sep 20, 2023
@DCNick3
[ci] hyperledger-iroha#3622: Use cargo manifest lints instead of unma…
e6667c7 
…intained cargo-lints

Since the implementation of rust-lang/rfcs#3389, it is now possible to specify workspace-level lints for rustc and clippy. This PR updates the cargo configuration and CI to use this new feature instead of cargo-lints.

Note that it was only stabilized in `nightly-2023-09-10`. Using it with out current toolchain requires either a -Zlints flag or a modification to `.cargo/config.toml`:

```
[unstable]
lints = true
```

Note that unlike the original suggestion in hyperledger-iroha#3622, this doesn't make the lints crate level, but merely replaces a clunky unmaintained tool with a standard solution for configuring lints.

In particular this PR:
- Removes old lints.toml configuration files for cargo-lints
- Adds [lint] tables to Cargo.toml of the root and wasm workspaces. The lints are duplicated between the two
- Replaces `cargo lints clippy` invocations with `cargo clippy -Zlints` in CI scripts
- Silences/fixes some new lints that popped up due to differences in how between `cargo lints` and workspaces
- Does a drive-by fix to iroha_genesis: it now too shares cargo metadata as do other crates

Signed-off-by: Nikita Strygin <[email protected]>
@DCNick3 DCNick3 mentioned this issue Sep 20, 2023
Merged
1 task
DCNick3 added a commit that referenced this issue Sep 21, 2023
@DCNick3
[ci] #3622: Use cargo manifest lints instead of unmaintained cargo-lints
dc5b0f3 
Since the implementation of rust-lang/rfcs#3389, it is now possible to specify workspace-level lints for rustc and clippy. This PR updates the cargo configuration and CI to use this new feature instead of cargo-lints.

Note that it was only stabilized in `nightly-2023-09-10`. Using it with out current toolchain requires either a -Zlints flag or a modification to `.cargo/config.toml`:

```
[unstable]
lints = true
```

Note that unlike the original suggestion in #3622, this doesn't make the lints crate level, but merely replaces a clunky unmaintained tool with a standard solution for configuring lints.

In particular this PR:
- Removes old lints.toml configuration files for cargo-lints
- Adds [lint] tables to Cargo.toml of the root and wasm workspaces. The lints are duplicated between the two
- Replaces `cargo lints clippy` invocations with `cargo clippy -Zlints` in CI scripts
- Silences/fixes some new lints that popped up due to differences in how between `cargo lints` and workspaces
- Does a drive-by fix to iroha_genesis: it now too shares cargo metadata as do other crates

Signed-off-by: Nikita Strygin <[email protected]>
@DCNick3 DCNick3 removed their assignment Sep 21, 2023
@DCNick3
Copy link
Contributor

DCNick3 commented Sep 21, 2023

With the merge of #3904, cargo-lints is no longer being used and points 3, 4 and 5 no longer apply. We might still want to have the lints be more granular

@mversic
Copy link
Contributor

mversic commented Sep 26, 2023

closed in favor of #3927

@mversic mversic closed this as completed Sep 26, 2023
6r1d pushed a commit that referenced this issue Oct 17, 2023
@DCNick3 @6r1d
[ci] #3622: Use cargo manifest lints instead of unmaintained cargo-lints
9f77e55 
Since the implementation of rust-lang/rfcs#3389, it is now possible to specify workspace-level lints for rustc and clippy. This PR updates the cargo configuration and CI to use this new feature instead of cargo-lints.

Note that it was only stabilized in `nightly-2023-09-10`. Using it with out current toolchain requires either a -Zlints flag or a modification to `.cargo/config.toml`:

```
[unstable]
lints = true
```

Note that unlike the original suggestion in #3622, this doesn't make the lints crate level, but merely replaces a clunky unmaintained tool with a standard solution for configuring lints.

In particular this PR:
- Removes old lints.toml configuration files for cargo-lints
- Adds [lint] tables to Cargo.toml of the root and wasm workspaces. The lints are duplicated between the two
- Replaces `cargo lints clippy` invocations with `cargo clippy -Zlints` in CI scripts
- Silences/fixes some new lints that popped up due to differences in how between `cargo lints` and workspaces
- Does a drive-by fix to iroha_genesis: it now too shares cargo metadata as do other crates

Signed-off-by: Nikita Strygin <[email protected]>
6r1d pushed a commit that referenced this issue Oct 17, 2023
@DCNick3 @6r1d
[ci] #3622: Use cargo manifest lints instead of unmaintained cargo-lints
76da488 
Since the implementation of rust-lang/rfcs#3389, it is now possible to specify workspace-level lints for rustc and clippy. This PR updates the cargo configuration and CI to use this new feature instead of cargo-lints.

Note that it was only stabilized in `nightly-2023-09-10`. Using it with out current toolchain requires either a -Zlints flag or a modification to `.cargo/config.toml`:

```
[unstable]
lints = true
```

Note that unlike the original suggestion in #3622, this doesn't make the lints crate level, but merely replaces a clunky unmaintained tool with a standard solution for configuring lints.

In particular this PR:
- Removes old lints.toml configuration files for cargo-lints
- Adds [lint] tables to Cargo.toml of the root and wasm workspaces. The lints are duplicated between the two
- Replaces `cargo lints clippy` invocations with `cargo clippy -Zlints` in CI scripts
- Silences/fixes some new lints that popped up due to differences in how between `cargo lints` and workspaces
- Does a drive-by fix to iroha_genesis: it now too shares cargo metadata as do other crates

Signed-off-by: Nikita Strygin <[email protected]>
mversic pushed a commit that referenced this issue Oct 17, 2023
@DCNick3 @mversic
[ci] #3622: Use cargo manifest lints instead of unmaintained cargo-lints
e10c860 
Since the implementation of rust-lang/rfcs#3389, it is now possible to specify workspace-level lints for rustc and clippy. This PR updates the cargo configuration and CI to use this new feature instead of cargo-lints.

Note that it was only stabilized in `nightly-2023-09-10`. Using it with out current toolchain requires either a -Zlints flag or a modification to `.cargo/config.toml`:

```
[unstable]
lints = true
```

Note that unlike the original suggestion in #3622, this doesn't make the lints crate level, but merely replaces a clunky unmaintained tool with a standard solution for configuring lints.

In particular this PR:
- Removes old lints.toml configuration files for cargo-lints
- Adds [lint] tables to Cargo.toml of the root and wasm workspaces. The lints are duplicated between the two
- Replaces `cargo lints clippy` invocations with `cargo clippy -Zlints` in CI scripts
- Silences/fixes some new lints that popped up due to differences in how between `cargo lints` and workspaces
- Does a drive-by fix to iroha_genesis: it now too shares cargo metadata as do other crates

Signed-off-by: Nikita Strygin <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Chore This is a small task that can be done at any point in time and is easier than others Enhancement New feature or request iroha2-dev The re-implementation of a BFT hyperledger in RUST
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@DCNick3 @appetrosyan @mversic