Skip to content

chore: Add unused deps lint #10666

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

Merged
merged 8 commits into from
Jun 2, 2025

Conversation

nguyenethan01
Copy link
Contributor

Motivation

Closes issue #10665

Solution

Add #![cfg_attr(not(test), warn(unused_crate_dependencies))] to ALL crates

PR Checklist

  • Added Tests N/A
  • Added Documentation N/A
  • Breaking changes

Add unused_crate_dependencies lint to all crates

This adds #![cfg_attr(not(test), warn(unused_crate_dependencies))] to all
library and binary crates as requested in #10665.

@@ -1,6 +1,8 @@
//! The `forge` CLI: build, test, fuzz, debug and deploy Solidity contracts, like Hardhat, Brownie,
//! Ape.

#![cfg_attr(not(test), warn(unused_crate_dependencies))]
Copy link
Member

Choose a reason for hiding this comment

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

sorry i should've specified to do it only in lib.rs, as main.rs will have unnecessary warnings

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No worries, re-added to main files. Still seeing the false positives where removing the warned dependencies throws errors as the dependencies are used in test files and the linter specifies not(test). In this case should I add use toml as _; etc. in the crate root to silence the warnings as make pr is currenly failing due to the linting?

Copy link
Member

Choose a reason for hiding this comment

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

in that case you need to move them to dev-dependencies

Copy link
Contributor Author

Choose a reason for hiding this comment

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

addressed, thanks!

@nguyenethan01 nguyenethan01 requested a review from DaniPopes May 31, 2025 20:21
Copy link
Member

@zerosnacks zerosnacks left a comment

Choose a reason for hiding this comment

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

Thanks! lgtm

Pending final check by @DaniPopes

@DaniPopes DaniPopes merged commit b42d512 into foundry-rs:master Jun 2, 2025
22 checks passed
@github-project-automation github-project-automation bot moved this to Done in Foundry Jun 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants