Merge pull request #24 from 0xvasanth/fix/core-alias-qualified-paths

fix: Support nested attribute and core/std aliasing in derive macro

Author: 0xvasanth

ISSUE_23_CURRENT_STATUS.md

@@ -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)

gonfig_derive/src/lib.rs

@@ -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>,
 }
@@ -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
+///     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.
@@ -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
         {
@@ -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),

tests/issue_23_both_aliases.rs

@@ -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");
+    }
+}

tests/issue_23_core_alias.rs

@@ -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);
+    }
+}