fix: Support nested attribute and core/std aliasing in derive macro#24
fix: Support nested attribute and core/std aliasing in derive macro#24
Conversation
Fixes compilation error when users alias std/core crates and use the gonfig derive macro. This issue had two root causes: 1. Unqualified std::path::Path usage in macro-generated code 2. Missing 'nested' attribute support causing darling to emit unqualified core::compile_error!() messages Changes: - Add 'nested' field to GonfigField to accept #[gonfig(nested)] attribute - Mark nested fields as experimental (skip from auto-loading for now) - Replace 'use std::path::Path' with '::std::path::Path::new()' for hygiene - Add documentation for nested attribute usage - Add 4 comprehensive regression test files with 10 total tests - Document manual workaround pattern for nested configs The nested attribute is now accepted but requires manual composition. Full hierarchical prefix support (PARENT_NESTED_FIELD pattern) will be implemented in a future update. Fixes #23
There was a problem hiding this comment.
Pull request overview
This PR fixes a compilation error in the gonfig derive macro when users alias the std or core crates, and adds support for a #[gonfig(nested)] attribute as an experimental feature.
Changes:
- Replaced unqualified
std::path::Pathusage with fully qualified::std::path::Path::new()to handle std/core aliasing - Added
nestedfield to GonfigField struct to accept the#[gonfig(nested)]attribute (currently experimental, fields are skipped during auto-loading) - Added 10 comprehensive regression tests across 4 test files covering various aliasing scenarios
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| gonfig_derive/src/lib.rs | Added nested field support and fixed std::path::Path to use fully qualified paths |
| tests/issue_23_std_alias.rs | Tests derive macro compilation with std aliased (2 tests) |
| tests/issue_23_core_alias.rs | Tests derive macro compilation with core aliased (2 tests) |
| tests/issue_23_nested_attr.rs | Tests nested attribute acceptance with core aliasing (2 tests) |
| tests/issue_23_both_aliases.rs | Comprehensive tests with both std and core aliased (4 tests) |
| ISSUE_23_CURRENT_STATUS.md | Internal status document with analysis and discussion |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /// #[derive(Gonfig, Deserialize)] | ||
| /// #[Gonfig(env_prefix = "SERVER")] | ||
| /// struct ServerConfig { | ||
| /// host: String, | ||
| /// } | ||
| /// | ||
| /// // Manual construction (automatic loading coming in future version): | ||
| /// fn load_config() -> Result<Config> { | ||
| /// let server = ServerConfig::from_gonfig()?; | ||
| /// // Manually combine with parent struct |
There was a problem hiding this comment.
The documentation states that nested fields "must be manually constructed" and that "This attribute prevents the field from being loaded with the parent config." However, when a user tries to call from_gonfig() on a struct with a nested field that doesn't have a default value (like the Config struct shown in the example), it will fail at runtime because the nested field is silently skipped but still required. The documentation should warn users that nested fields need either a default value or manual construction after loading, and provide a clearer example showing this limitation.
| /// #[derive(Gonfig, Deserialize)] | |
| /// #[Gonfig(env_prefix = "SERVER")] | |
| /// struct ServerConfig { | |
| /// host: String, | |
| /// } | |
| /// | |
| /// // Manual construction (automatic loading coming in future version): | |
| /// fn load_config() -> Result<Config> { | |
| /// let server = ServerConfig::from_gonfig()?; | |
| /// // Manually combine with parent struct | |
| /// // NOTE: | |
| /// // - The `server` field is *not* loaded by `Config::from_gonfig()` because it | |
| /// // is marked `#[gonfig(nested)]`. | |
| /// // - If `server` does not have a default value (or is not `Option<ServerConfig>`), | |
| /// // calling `Config::from_gonfig()` will fail at runtime because `server` | |
| /// // remains unset but is still required. | |
| /// // In such cases you must either provide a default for `server` or construct | |
| /// // it manually after loading, as shown below. | |
| /// | |
| /// #[derive(Gonfig, Deserialize)] | |
| /// #[Gonfig(env_prefix = "SERVER")] | |
| /// struct ServerConfig { | |
| /// host: String, | |
| /// } | |
| /// | |
| /// // Manual construction of the nested field: | |
| /// fn load_config() -> Result<Config> { | |
| /// // Load configuration for the nested type separately. | |
| /// let server = ServerConfig::from_gonfig()?; | |
| /// // Manually combine with parent struct. |
| #[test] | ||
| fn test_nested_attribute_compiles_with_core_alias() { | ||
| // **Main fix**: This test verifies the compilation error is fixed | ||
| // Original error: "failed to resolve: could not find `compile_error` in `core`" | ||
| // This happened because darling used unqualified core::compile_error!() | ||
| // when it encountered unknown attribute `nested` | ||
|
|
||
| // By adding `nested` field to GonfigField, darling accepts it and | ||
| // the code compiles successfully even with core aliased | ||
|
|
||
| // Test that nested structs load independently | ||
| let server = ServerConfig::from_gonfig(); | ||
| assert!(server.is_ok(), "Nested struct should load successfully"); | ||
|
|
||
| let server = server.unwrap(); | ||
| assert_eq!(server.host, "127.0.0.1"); | ||
| assert_eq!(server.port, 3000); | ||
| } |
There was a problem hiding this comment.
The test only verifies that ServerConfig loads independently, but doesn't test the actual problematic case: loading the parent Config struct that has a #[gonfig(nested)] field. When Config::from_gonfig() is called, the server field is skipped but has no default value, which would cause deserialization to fail. Add a test that demonstrates the actual intended usage pattern, even if it requires manual construction, or document that Config::from_gonfig() cannot be used directly.
| # Issue #23 - Current Status & Analysis | ||
|
|
||
| ## Summary | ||
|
|
||
| Issue #23 is more complex than initially described. The actual problem is: | ||
|
|
||
| 1. User tries to use `#[gonfig(nested)]` attribute - **which doesn't exist** | ||
| 2. When darling (attribute parser) encounters unknown attribute, it generates error with unqualified `core::compile_error!()` | ||
| 3. When user has `use core as tradesmith_core;`, the compiler can't resolve the error message | ||
|
|
||
| ## What We've Done So Far | ||
|
|
||
| ### Commit 1: `9754b21` (Initial Fix - Incomplete) | ||
| - Fixed `std::path::Path` to use fully qualified paths (::std::path::Path) | ||
| - Added comprehensive regression tests for std/core aliasing | ||
| - **BUT**: This didn't fix the user's actual issue | ||
|
|
||
| ### Current Changes (Uncommitted) | ||
| - Added `nested` field to `GonfigField` struct | ||
| - Marked as `#[allow(dead_code)]` and reserved for future use | ||
| - Now the `#[gonfig(nested)]` attribute is **accepted** (prevents darling error) | ||
| - Test confirms it compiles with core alias | ||
|
|
||
| ## The Real Question | ||
|
|
||
| **We need user clarification on what `nested` should do:** | ||
|
|
||
| ### Option A: Accept But Don't Implement (Current State) | ||
| - ✅ Fixes compilation error | ||
| - ✅ Allows code to compile with core alias | ||
| - ❌ `nested` attribute does nothing functionally | ||
| - ❌ Misleading to users who expect it to work | ||
|
|
||
| ### Option B: Implement Full Nested Support | ||
| - ✅ Provides actual functionality | ||
| - ✅ Useful feature for users | ||
| - ❌ More complex implementation | ||
| - ❌ Need to define exact behavior: | ||
| - How do prefixes work? | ||
| - Do fields flatten or stay nested? | ||
| - How do defaults propagate? | ||
|
|
||
| ### Option C: Remove from Issue Example | ||
| - Ask user if they actually need `nested` feature | ||
| - Maybe they just used it as an example | ||
| - They might only need the std::path::Path fix | ||
|
|
||
| ## Waiting For | ||
|
|
||
| GitHub comment posted: https://github.com/0xvasanth/gonfig/issues/23#issuecomment-3733426020 | ||
|
|
||
| **Questions asked:** | ||
| 1. What should `nested` do functionally? | ||
| 2. How should environment variable prefixes work with nested structs? | ||
| 3. Is this the same as the existing `flatten` attribute? | ||
| 4. Do they prefer Option A, B, or C? | ||
|
|
||
| ## Test Results | ||
|
|
||
| All new tests pass: | ||
| - ✅ issue_23_core_alias (2 tests) | ||
| - ✅ issue_23_std_alias (2 tests) | ||
| - ✅ issue_23_both_aliases (4 tests) | ||
| - ✅ issue_23_nested_attr (1 test) - **New: verifies nested compiles** | ||
|
|
||
| **Total: 9 passing regression tests for issue #23** | ||
|
|
||
| ## Files Changed | ||
|
|
||
| ``` | ||
| gonfig_derive/src/lib.rs - Added nested field | ||
| tests/issue_23_nested_attr.rs - New test for nested attribute | ||
| ``` | ||
|
|
||
| ## Next Steps | ||
|
|
||
| 1. **Wait for user response** on GitHub issue | ||
| 2. Based on response: | ||
| - If Option A: Amend commit, add docs, done | ||
| - If Option B: Implement full nested feature | ||
| - If Option C: Revert nested, focus on other fixes | ||
|
|
||
| ## Technical Notes | ||
|
|
||
| ### Why the Original Fix Wasn't Enough | ||
|
|
||
| The std::path::Path fix (commit `9754b21`) solved ONE potential issue but not THE issue in the user's reproduction case. Their example specifically uses `#[gonfig(nested)]` which triggers a different code path in darling. | ||
|
|
||
| ### The Darling Issue | ||
|
|
||
| Darling generates compile errors like this: | ||
| ```rust | ||
| ::core::compile_error!("Unknown field: `nested`") | ||
| ``` | ||
|
|
||
| When user has `use core as my_core;`, this becomes: | ||
| ```rust | ||
| my_core::compile_error!("Unknown field: `nested`") // ERROR: compile_error not in my_core | ||
| ``` | ||
|
|
||
| By adding `nested` to GonfigField, darling accepts it and doesn't generate the error. | ||
|
|
||
| ### Complete Fix Requires | ||
|
|
||
| 1. ✅ Fully qualified std::path::Path (done in `9754b21`) | ||
| 2. ✅ Accept `nested` attribute (done, uncommitted) | ||
| 3. ❓ Implement `nested` functionality (pending user clarification) |
There was a problem hiding this comment.
This internal status and planning document should not be included in the repository. It contains analysis and discussion that belongs in GitHub issue comments, pull request descriptions, or commit messages. Consider removing this file from the PR.
| # Issue #23 - Current Status & Analysis | |
| ## Summary | |
| Issue #23 is more complex than initially described. The actual problem is: | |
| 1. User tries to use `#[gonfig(nested)]` attribute - **which doesn't exist** | |
| 2. When darling (attribute parser) encounters unknown attribute, it generates error with unqualified `core::compile_error!()` | |
| 3. When user has `use core as tradesmith_core;`, the compiler can't resolve the error message | |
| ## What We've Done So Far | |
| ### Commit 1: `9754b21` (Initial Fix - Incomplete) | |
| - Fixed `std::path::Path` to use fully qualified paths (::std::path::Path) | |
| - Added comprehensive regression tests for std/core aliasing | |
| - **BUT**: This didn't fix the user's actual issue | |
| ### Current Changes (Uncommitted) | |
| - Added `nested` field to `GonfigField` struct | |
| - Marked as `#[allow(dead_code)]` and reserved for future use | |
| - Now the `#[gonfig(nested)]` attribute is **accepted** (prevents darling error) | |
| - Test confirms it compiles with core alias | |
| ## The Real Question | |
| **We need user clarification on what `nested` should do:** | |
| ### Option A: Accept But Don't Implement (Current State) | |
| - ✅ Fixes compilation error | |
| - ✅ Allows code to compile with core alias | |
| - ❌ `nested` attribute does nothing functionally | |
| - ❌ Misleading to users who expect it to work | |
| ### Option B: Implement Full Nested Support | |
| - ✅ Provides actual functionality | |
| - ✅ Useful feature for users | |
| - ❌ More complex implementation | |
| - ❌ Need to define exact behavior: | |
| - How do prefixes work? | |
| - Do fields flatten or stay nested? | |
| - How do defaults propagate? | |
| ### Option C: Remove from Issue Example | |
| - Ask user if they actually need `nested` feature | |
| - Maybe they just used it as an example | |
| - They might only need the std::path::Path fix | |
| ## Waiting For | |
| GitHub comment posted: https://github.com/0xvasanth/gonfig/issues/23#issuecomment-3733426020 | |
| **Questions asked:** | |
| 1. What should `nested` do functionally? | |
| 2. How should environment variable prefixes work with nested structs? | |
| 3. Is this the same as the existing `flatten` attribute? | |
| 4. Do they prefer Option A, B, or C? | |
| ## Test Results | |
| All new tests pass: | |
| - ✅ issue_23_core_alias (2 tests) | |
| - ✅ issue_23_std_alias (2 tests) | |
| - ✅ issue_23_both_aliases (4 tests) | |
| - ✅ issue_23_nested_attr (1 test) - **New: verifies nested compiles** | |
| **Total: 9 passing regression tests for issue #23** | |
| ## Files Changed | |
| ``` | |
| gonfig_derive/src/lib.rs - Added nested field | |
| tests/issue_23_nested_attr.rs - New test for nested attribute | |
| ``` | |
| ## Next Steps | |
| 1. **Wait for user response** on GitHub issue | |
| 2. Based on response: | |
| - If Option A: Amend commit, add docs, done | |
| - If Option B: Implement full nested feature | |
| - If Option C: Revert nested, focus on other fixes | |
| ## Technical Notes | |
| ### Why the Original Fix Wasn't Enough | |
| The std::path::Path fix (commit `9754b21`) solved ONE potential issue but not THE issue in the user's reproduction case. Their example specifically uses `#[gonfig(nested)]` which triggers a different code path in darling. | |
| ### The Darling Issue | |
| Darling generates compile errors like this: | |
| ```rust | |
| ::core::compile_error!("Unknown field: `nested`") | |
| ``` | |
| When user has `use core as my_core;`, this becomes: | |
| ```rust | |
| my_core::compile_error!("Unknown field: `nested`") // ERROR: compile_error not in my_core | |
| ``` | |
| By adding `nested` to GonfigField, darling accepts it and doesn't generate the error. | |
| ### Complete Fix Requires | |
| 1. ✅ Fully qualified std::path::Path (done in `9754b21`) | |
| 2. ✅ Accept `nested` attribute (done, uncommitted) | |
| 3. ❓ Implement `nested` functionality (pending user clarification) | |
| # Issue #23 – `nested` Attribute Behavior (Documentation Note) | |
| This file documents the observed behavior related to Issue #23 in a stable, user-facing way. | |
| For ongoing discussion and historical context, please refer to the GitHub issue: | |
| https://github.com/0xvasanth/gonfig/issues/23 | |
| ## Problem Overview | |
| Issue #23 concerns the interaction between the `#[gonfig(nested)]` attribute and aliasing of | |
| the `core` crate. When an attribute is unknown, the underlying attribute parser (`darling`) | |
| can emit compile errors using `core::compile_error!()`. In cases where the user aliases | |
| `core` (for example, `use core as my_core;`), unqualified use of `compile_error!` through | |
| this alias can lead to resolution problems. | |
| To avoid such errors for the `nested` attribute, `gonfig` accepts `#[gonfig(nested)]` | |
| during derivation so that the attribute parser does not treat it as unknown and therefore | |
| does not generate a failing `compile_error!` invocation. | |
| ## Current Behavior of `#[gonfig(nested)]` | |
| At this time, `#[gonfig(nested)]` is accepted by the derive macro to prevent the attribute | |
| parser from emitting an error related to the `core` aliasing scenario described above. | |
| However, accepting the attribute currently has **no functional effect** on how configuration | |
| is derived or loaded: | |
| - The attribute can be added to fields without causing a compilation error. | |
| - It does **not** change how fields are flattened, nested, or how environment variable | |
| prefixes or defaults are handled. | |
| Users relying on `#[gonfig(nested)]` should treat it as a no-op for now. Future changes to | |
| the `nested` semantics, if any, will be documented in the crate's changelog and user-facing | |
| documentation rather than in this file. | |
| ## Related Tests | |
| Regression tests exist to ensure that: | |
| - Code continues to compile correctly when `core` is aliased. | |
| - `#[gonfig(nested)]` can be used without triggering the attribute parser's "unknown field" | |
| error. | |
| For implementation details, refer to the source files and tests mentioned in the GitHub | |
| issue discussion. |
Fixes #23
Summary
Fixes compilation error when users alias std/core crates and use the gonfig derive macro.
Root Cause
The issue had two root causes:
std::path::Pathusage in macro-generated codenestedattribute - When darling encountered#[gonfig(nested)], it generated unqualifiedcore::compile_error!()which failed when users haduse core as my_core;Changes
nestedfield to GonfigField to accept #[gonfig(nested)] attributeTesting
Breaking Changes
None - this is a pure fix/enhancement.
Example
Closes #23