Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
107 changes: 107 additions & 0 deletions ISSUE_23_CURRENT_STATUS.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
# 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)
Comment on lines +1 to +107
Copy link

Copilot AI Jan 10, 2026

Choose a reason for hiding this comment

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

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.

Suggested change
# 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.

Copilot uses AI. Check for mistakes.
49 changes: 45 additions & 4 deletions gonfig_derive/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,11 @@ struct GonfigField {
#[darling(default)]
flatten: bool,

// Reserved for future use (nested configuration feature)
#[allow(dead_code)]
#[darling(default)]
nested: bool,

#[darling(default)]
default: Option<String>,
}
Expand Down Expand Up @@ -148,6 +153,35 @@ struct GonfigField {
/// }
/// ```
///
/// ## `#[gonfig(nested)]`
/// **[Experimental]** Marks a field as a nested configuration struct.
///
/// Currently, nested fields must be manually constructed. This attribute prevents
/// the field from being loaded with the parent config.
///
/// **Example:**
/// ```rust,ignore
/// #[derive(Gonfig, Deserialize)]
/// #[Gonfig(env_prefix = "APP")]
/// struct Config {
/// #[gonfig(nested)]
/// server: ServerConfig,
/// }
///
/// #[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
Comment on lines +171 to +180
Copy link

Copilot AI Jan 10, 2026

Choose a reason for hiding this comment

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

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.

Suggested change
/// #[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.

Copilot uses AI. Check for mistakes.
/// Ok(Config { server })
/// }
/// ```
///
/// ## `#[skip]` or `#[skip_gonfig]`
/// Exclude a field from configuration loading. Useful for non-serializable fields or
/// fields that should only be set at runtime.
Expand Down Expand Up @@ -260,6 +294,12 @@ fn generate_gonfig_impl(opts: &GonfigOpts) -> proc_macro2::TokenStream {
let field_name = f.ident.as_ref().unwrap();
let field_str = field_name.to_string();

// Skip nested fields - they should be loaded separately
// TODO: Implement automatic nested struct loading with prefix composition
if f.nested {
continue;
}

// Note: flatten feature is not yet fully implemented
// For now, treat all fields as regular fields
{
Expand Down Expand Up @@ -338,19 +378,20 @@ fn generate_gonfig_impl(opts: &GonfigOpts) -> proc_macro2::TokenStream {

if #allow_config {
// Config file support - check for default config files
use std::path::Path;
// Note: Using fully qualified paths to avoid conflicts with user's std/core aliases
// See: https://github.com/0xvasanth/gonfig/issues/23

if Path::new("config.toml").exists() {
if ::std::path::Path::new("config.toml").exists() {
builder = match builder.with_file("config.toml") {
Ok(b) => b,
Err(e) => return Err(e),
};
} else if Path::new("config.yaml").exists() {
} else if ::std::path::Path::new("config.yaml").exists() {
builder = match builder.with_file("config.yaml") {
Ok(b) => b,
Err(e) => return Err(e),
};
} else if Path::new("config.json").exists() {
} else if ::std::path::Path::new("config.json").exists() {
builder = match builder.with_file("config.json") {
Ok(b) => b,
Err(e) => return Err(e),
Expand Down
103 changes: 103 additions & 0 deletions tests/issue_23_both_aliases.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
// Regression test for issue #23: https://github.com/0xvasanth/gonfig/issues/23
// Comprehensive test with both core and std aliased and all features enabled

#![allow(unused_imports)]

use core as my_core;
use gonfig::Gonfig;
use serde::{Deserialize, Serialize};
use std as my_std;

#[derive(Debug, Clone, Serialize, Deserialize, Gonfig)]
#[gonfig(env_prefix = "BOTH_ALIAS", allow_config, allow_cli)]
pub struct FullFeaturedConfig {
#[gonfig(default = "localhost")]
pub hostname: String,

#[gonfig(default = "8080")]
pub port: u16,

#[gonfig(default = "info")]
#[gonfig(env_name = "LOG_LEVEL")]
pub log_level: String,
}

// Test nested configuration
#[derive(Debug, Clone, Serialize, Deserialize, Gonfig)]
#[gonfig(env_prefix = "DATABASE", allow_config)]
pub struct DatabaseConfig {
#[gonfig(default = "postgres://localhost/mydb")]
pub url: String,

#[gonfig(default = "10")]
pub max_connections: u32,

#[gonfig(default = "30")]
pub timeout_seconds: u64,
}

// Test with skip attribute
#[derive(Debug, Clone, Serialize, Deserialize, Gonfig)]
#[gonfig(env_prefix = "APP", allow_config, allow_cli)]
pub struct AppConfigWithSkip {
#[gonfig(default = "production")]
pub environment: String,

#[skip]
#[serde(skip)]
pub runtime_data: Option<String>,
}

#[cfg(test)]
mod tests {
use super::*;

#[test]
fn test_both_aliases_all_features() {
// Most comprehensive test: both std and core aliased, all features enabled
let config = FullFeaturedConfig::from_gonfig();
assert!(
config.is_ok(),
"Should compile and run with both std and core aliased"
);

let config = config.unwrap();
assert_eq!(config.hostname, "localhost");
assert_eq!(config.port, 8080);
assert_eq!(config.log_level, "info");
}

#[test]
fn test_both_aliases_nested_config() {
// Test with database configuration
let config = DatabaseConfig::from_gonfig();
assert!(
config.is_ok(),
"Nested config should work with both aliases"
);

let config = config.unwrap();
assert_eq!(config.url, "postgres://localhost/mydb");
assert_eq!(config.max_connections, 10);
assert_eq!(config.timeout_seconds, 30);
}

#[test]
fn test_both_aliases_with_skip() {
// Test with skip attribute
let config = AppConfigWithSkip::from_gonfig();
assert!(config.is_ok(), "Config with skip should work with aliases");

let config = config.unwrap();
assert_eq!(config.environment, "production");
assert_eq!(config.runtime_data, None);
}

#[test]
fn test_builder_pattern_with_aliases() {
// Test the builder pattern also works with aliases
let builder = FullFeaturedConfig::gonfig_builder();
let config = FullFeaturedConfig::from_gonfig_with_builder(builder);
assert!(config.is_ok(), "Builder pattern should work with aliases");
}
}
59 changes: 59 additions & 0 deletions tests/issue_23_core_alias.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
// Regression test for issue #23: https://github.com/0xvasanth/gonfig/issues/23
// Tests that the gonfig derive macro works correctly when core crate is aliased

#![allow(unused_imports)]

use core as tradesmith_core; // Alias core to simulate user's environment
use gonfig::Gonfig;
use serde::{Deserialize, Serialize};

#[derive(Debug, Clone, Serialize, Deserialize, Gonfig)]
#[gonfig(env_prefix = "CORE_ALIAS_TEST", allow_config)]
pub struct ConfigWithCoreAlias {
#[gonfig(default = "127.0.0.1")]
pub host: String,

#[gonfig(default = "8080")]
pub port: u16,
}

#[derive(Debug, Clone, Serialize, Deserialize, Gonfig)]
#[gonfig(env_prefix = "SERVER", allow_config, allow_cli)]
pub struct ServerConfigWithCoreAlias {
#[gonfig(default = "localhost")]
pub address: String,

#[gonfig(default = "3000")]
pub server_port: u16,
}

#[cfg(test)]
mod tests {
use super::*;

#[test]
fn test_core_alias_basic_with_config() {
// This test verifies that the macro-generated code compiles
// when core is aliased and allow_config is enabled
let config = ConfigWithCoreAlias::from_gonfig();
assert!(config.is_ok(), "Should compile with core alias");

let config = config.unwrap();
assert_eq!(config.host, "127.0.0.1");
assert_eq!(config.port, 8080);
}

#[test]
fn test_core_alias_with_all_features() {
// Test with both allow_config and allow_cli enabled
let config = ServerConfigWithCoreAlias::from_gonfig();
assert!(
config.is_ok(),
"Should compile with core alias and all features"
);

let config = config.unwrap();
assert_eq!(config.address, "localhost");
assert_eq!(config.server_port, 3000);
}
}
Loading
Loading