chore: clippy fixes

[willemneal]

What

Add a clippy check to CI and make initial pass at fixing clippy errors.

At first the number of lints was way too high to tackle all at once. However, disabling some that could be debatable allowed a lot of the obvious fixes to be done.

In future PRs the disabled lints can be addressed in batches to ensure so that it's not too overwhelming.

Why

Currently the only repo that requires that clippy is the CLI and this repo seems more important to have standard coding practices. Clippy can even help catch some logic errors.

Known limitations

[TODO or N/A]

[willemneal]

One other thing to note. Currently it should fail when run for the first time. The reason is that the latest commit to main added two methods which return a result, but will never return an error. I kept it in partly as a check that CI should fail the first time and unlike the initial batch of changes it is an API change to remove the Result.

[willemneal]

The CI failed as expected an I disabled unnecessary_wraps for now so it should pass now.

[dmkozh]

Thanks for this cleanup. This looks good to me for the most part, besides a couple questionable diagnostics/changes I've pointed in comments.

[dmkozh]

The change looks good to me - I think we can merge it when the fmt check is passed and the remaining few failures are resolved.

[dmkozh]

CI fails with error: rustup is not installed at '/home/runner/work/rs-soroban-env/rs-soroban-env/.cargo/ci/config.toml'

I'm not sure I understand how to fix it, but I suppose the reason has something to do with the newly introduced config.toml

[willemneal]

@dmkozh fixed it. Was an ENV VAR for cargo home, which was from a previous attempt to have the clippy lints in a file.

[dmkozh]

Seems like there are some failures in guest library.

[willemneal]

@dmkozh I also seem to get the following error if I ran that command locally:

cargo clippy --locked --target wasm32-unknown-unknown --no-default-features

I get:

error[E0152]: found duplicate lang item `panic_impl`
  --> soroban-env-common/tests/no_std/src/lib.rs:17:1
   |
17 | fn handle_panic(_: &core::panic::PanicInfo) -> ! {
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |
   = note: the lang item is first defined in crate `std` (which `soroban_env_common` depends on)
   = note: first definition in `std` loaded from /Users/willem/.rustup/toolchains/stable-aarch64-apple-darwin/lib/rustlib/wasm32-unknown-unknown/lib/libstd-a9c931054ca2959f.rlib
   = note: second definition in the local crate (`test_no_std`)

For more information about this error, try `rustc --explain E0152`.

[dmkozh]

@dmkozh I also seem to get the following error if I ran that command locally:

cargo clippy --locked --target wasm32-unknown-unknown --no-default-features

I get:

error[E0152]: found duplicate lang item `panic_impl`
  --> soroban-env-common/tests/no_std/src/lib.rs:17:1
   |
17 | fn handle_panic(_: &core::panic::PanicInfo) -> ! {
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |
   = note: the lang item is first defined in crate `std` (which `soroban_env_common` depends on)
   = note: first definition in `std` loaded from /Users/willem/.rustup/toolchains/stable-aarch64-apple-darwin/lib/rustlib/wasm32-unknown-unknown/lib/libstd-a9c931054ca2959f.rlib
   = note: second definition in the local crate (`test_no_std`)

For more information about this error, try `rustc --explain E0152`.

That's likely because you're missing --features testutils (probably that should be fixed separately...)

[graydon]

Hey @willemneal sorry for the delay (and inevitable bitrot).

I've merged a run of cargo clippy --fix in #790 and turned on clippy in CI without the style group in #796. So that accomplishes maybe half or a third of what this PR was trying to do, but lays groundwork. Assuming you want to enable all of the extras (or even all the way to enabling the full style group) I'd recommend refreshing this PR in stages -- doing a handful of new lints at a time, since it's easier to review a small or medium-sized PR than a giant one.