Skip to content

feat(config): warn on unknown config keys in foundry.toml #10621

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

zarkk01
Copy link
Contributor

@zarkk01 zarkk01 commented May 24, 2025

Motivation

Right now, if a user makes a typo or adds a deprecated field in their local foundry.toml, Figment/Serde simply ignores it and the user never knows something went unused. That can lead to confusion or misconfiguration.

Solution

Before Figment does .extract() into the Config struct, we read foundry.toml as raw TOML, run it through [serde_ignored] to collect any unused/unknown keys, and if any are found we log a warn! listing them. This is non-breaking (we still load all the valid settings) and only affects root keys for now.

About #10550 . For now, could I have some feedback on the implementation?

PR Checklist

  • Added Tests
  • Added Documentation
  • Breaking changes

Copy link
Contributor

@mablr mablr left a comment

Choose a reason for hiding this comment

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

I've given you a little feedback 👍

I think it might be nice to implement this verification as a separate helper function.

@@ -641,6 +643,23 @@ impl Config {
}

fn from_figment(figment: Figment) -> Result<Self, ExtractConfigError> {
let file_path = Path::new("foundry.toml");
Copy link
Contributor

Choose a reason for hiding this comment

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

It's a good practice to not use hardcoded paths, a FILE_NAME const is already defined in this module.

@@ -641,6 +643,23 @@ impl Config {
}

fn from_figment(figment: Figment) -> Result<Self, ExtractConfigError> {
let file_path = Path::new("foundry.toml");
if let Ok(raw) = fs::read_to_string(&file_path) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Clippy says there is a needless borrow here, so you can remove the & to pass the check.

if !ignored.is_empty() {
warn!(
"Found unknown config keys in {}: {}",
file_path.display(),
Copy link
Contributor

Choose a reason for hiding this comment

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

cf remark about filename.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

2 participants