ci: add clippy rules back to cli call

[yihau]

Problem

we moved the clippy rules from the cli call to clippy.toml in #5673, then moved the rule again to the workspace (Cargo.toml) in #5710. however, it caused some issues:

1. worksapce lints don't apply to all crates by default

we will need to add something like

[lints]
workspace = true

to get crates follow the clippy rule. unfortunately, we already had something sneak into master. you can check the build from the first commit in this PR: https://buildkite.com/anza/agave/builds/22874#0196663a-0a7b-4a56-801a-b0b624917fed.

2. multiple workspaces in agave

for now, we have several different workspaces in Agave. if we want all of them to follow the same rules as before, we’ll need to copy the rules into each one. maintaining consistency will likely become an issue in the future.

Summary of Changes

• add the rules back to cli call to ensure the protection is still working
• add an issue to track the newly added code that doesn’t follow the Clippy rule.

---

note: part of #5952

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.8%. Comparing base (8d5c4ba) to head (d56c01b).

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #5953   +/-   ##
=======================================
  Coverage    82.8%    82.8%           
=======================================
  Files         830      830           
  Lines      377289   377289           
=======================================
+ Hits       312526   312603   +77     
+ Misses      64763    64686   -77     

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[alexpyattaev]

This is still going to miss a bunch of stuff. Our current configuration is pretty close to correct, but we need to call something like

cargo +nightly clippy --workspace --all-targets --features dummy-for-ci-check,frozen-abi

in the workspace root. At current master it catches this many things it can autofix with --fix:

git diff --numstat
7       7       banks-client/src/error.rs
6       12      clap-utils/src/keypair.rs
6       12      clap-v3-utils/src/keypair.rs
2       2       cli-config/src/lib.rs
1       2       cli-output/src/display.rs
5       9       core/src/consensus/tower_storage.rs
1       4       core/src/system_monitor_service.rs
2       2       faucet/src/faucet_mock.rs
10      17      genesis/src/main.rs
1       2       gossip/src/gossip_service.rs
1       1       install/src/command.rs
4       4       install/src/config.rs
1       1       install/src/update_manifest.rs
1       2       ledger-tool/src/main.rs
3       6       ledger/src/blockstore.rs
12      18      local-cluster/src/local_cluster.rs
4       6       net-utils/src/ip_echo_server.rs
5       10      net-utils/src/lib.rs
1       1       perf/src/cuda_runtime.rs
1       2       program-test/src/lib.rs
3       5       runtime/src/bank_client.rs
2       4       storage-bigtable/src/compression.rs
4       8       thin-client/src/thin_client.rs
1       1       tpu-client-next/src/quic_networking.rs
2       2       tpu-client/src/nonblocking/tpu_client.rs
1       1       tpu-client/src/tpu_client.rs
3       3       zk-sdk/src/encryption/elgamal.rs
3       3       zk-token-sdk/src/encryption/elgamal.rs

There is way more it can not autofix of course.

[t-nelson]

when we had these in clippy.toml, did they apply to all workspaces? agreed that putting them in Cargo.toml is pretty unmaintainable so long as we have multiple workspaces in the monorepo

[alexpyattaev]

Keeping them in sync is a matter of a grep checking for relevant section in cargo.toml, no?

[yihau]

if I don't miss anything, clippy.toml couldn't do that. for allowing/denying lints, we can only do either

• embedded in code
• cli flags
• Cargo.toml

ref: https://doc.rust-lang.org/clippy/configuration.html#allowingdenying-lints

[alexpyattaev]

yes this is correct, we either do CLI flags (which is not ideal as it prevents simply running cargo +nightly clippy during development as you have to call the script which will set all the CLAs for you) or we make sure crates inherit main config correctly. I'd prefer crates inheriting correctly from workspace.

[alexpyattaev]

files=`find . -name "Cargo.toml" `
for file in $files; do
    if ! grep -q "^\[lints\]" "$file"; then
      echo "Lints missing in $file"
      #echo -e "\n[lints]\nworkspace = true" >> "$file"
      #echo "Added [lints] to $file"
      # maybe call cargo sort here?
    else
      echo "$file already has [lints]"
    fi
  done

[yihau]

I prefer not to rely on Cargo.toml if we don’t have CI to ensure the rules are consistent in this repo and to enforce that newly added crates include lints.workspace = true

[alexpyattaev]

plug that shell script above into CI?

[yihau]

I’m happy to do improvements in separate PRs. for this PR, I would like to focus on reverting the lint checks back to how they were before.

[t-nelson]

Keeping them in sync is a matter of a grep checking for relevant section in cargo.toml, no?

we want fewer, not more, rube goldberg machines