fix(python): make [<AttachMembers>] union static members work (#4634) (#4636)

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: dbrattli

src/Fable.Transforms/FSharp2Fable.Util.fs

@@ -2276,6 +2276,21 @@ module Util =
 
     let entityIdent (com: Compiler) (ent: Fable.EntityRef) = entityIdentWithSuffix com ent ""
 
+    /// In Python a union is emitted as a public type alias (`type Demo = Demo_A | Demo_B`)
+    /// plus a private base class (`_Demo`) that holds the runtime members: attached static
+    /// members and the `cases()` method used by reflection. The type alias is typing-only,
+    /// so a reference to the union as a runtime value must be redirected to the base class.
+    /// Handles the two shapes `entityIdent` produces: a bare identifier for a same-file
+    /// union and an internal class import for a union defined in another file. References
+    /// to external entities (`[<Import>]`, kind `UserImport`) are left untouched.
+    /// See Python/PYTHON-UNION.md.
+    let redirectUnionToPythonBaseClass (expr: Fable.Expr) =
+        match expr with
+        | Fable.IdentExpr ident -> Fable.IdentExpr { ident with Name = "_" + ident.Name }
+        | Fable.Import({ Kind = Fable.ClassImport _ } as info, typ, range) ->
+            Fable.Import({ info with Selector = "_" + info.Selector }, typ, range)
+        | expr -> expr
+
     /// First checks if the entity is global or imported
     let tryEntityIdentMaybeGlobalOrImported (com: Compiler) (ent: Fable.Entity) =
         match tryGlobalOrImportedEntity com ent with
@@ -2740,7 +2755,14 @@ module Util =
                     && not (isInline memb)
                     && (isAttachMembersEntity com e || isPojoDefinedByConsArgsFSharpEntity e)
                     ->
-                    FsEnt.Ref e |> entityIdent com |> Some
+                    let classExpr = FsEnt.Ref e |> entityIdent com
+
+                    // In Python the union type alias carries no members, so attached static
+                    // members must be accessed on the private base class.
+                    if com.Options.Language = Python && e.IsFSharpUnion then
+                        redirectUnionToPythonBaseClass classExpr |> Some
+                    else
+                        Some classExpr
                 | None -> None
 
             match moduleOrClassExpr, callInfo.ThisArg with

src/Fable.Transforms/Python/Fable2Python.Transforms.fs

@@ -4622,14 +4622,24 @@ let getInitialValue
     (className: string)
     (getterMemb: Fable.MemberDecl)
     (wrapInLambda: bool)
+    (isUnion: bool)
     (fallback: Expression)
     : (Expression * bool * string list * Statement list)
     =
     match getterMemb.Body with
     | Fable.Value(kind, r) ->
         // Use transformValue for literal values - never wrapped
         let value, stmts = transformValue com ctx r kind
-        value, false, [], stmts
+
+        if isUnion && wrapInLambda then
+            // A union is emitted as a base class (`_Demo`) holding the static property
+            // followed by the case classes (`Demo_A`, ...). Constructing a case eagerly in
+            // the class body raises `NameError` because the case class doesn't exist yet.
+            // Defer with a lambda (StaticLazyProperty); this also matches F# getter
+            // semantics where the body is re-evaluated on each access.
+            Expression.lambda (Arguments.arguments [], value), true, [], stmts
+        else
+            value, false, [], stmts
     | Fable.Get(Fable.IdentExpr _, Fable.FieldGet fieldInfo, _, _) when
         fieldInfo.Name.EndsWith("@", System.StringComparison.Ordinal)
         ->
@@ -4697,11 +4707,21 @@ let transformStaticProperty
 
         // Check if the property type references the current class (forward reference needed)
         let typeAnnotation =
-            match propType with
-            | Fable.DeclaredType(entRef, _) when com.GetEntity(entRef).DisplayName = name ->
+            // The property type is self-referencing when it is the enclosing entity. Compare by
+            // FullName as well as DisplayName: for unions the descriptor lives on the base class
+            // and the type alias (`name`) is only defined afterwards, so a bare name would raise
+            // NameError at class-body evaluation.
+            let isSelfReference =
+                match propType with
+                | Fable.DeclaredType(entRef, _) ->
+                    let e = com.GetEntity(entRef)
+                    e.DisplayName = name || e.FullName = ent.FullName
+                | _ -> false
+
+            if isSelfReference then
                 // Use string forward reference for self-referencing types
                 Expression.stringConstant name
-            | _ ->
+            else
                 // Use normal type annotation
                 let ta, _ = Annotation.typeAnnotation com ctx None propType
                 ta
@@ -4728,7 +4748,7 @@ let transformStaticProperty
         let fallback = Util.getDefaultValueForType com ctx getterMemb.Body.Type
 
         let initialValue, isFactory, externalFields, initialValueStmts =
-            getInitialValue com ctx name getterMemb true fallback
+            getInitialValue com ctx name getterMemb true ent.IsFSharpUnion fallback
 
         let propExpr = makeStaticProperty getterMemb.Body.Type [ initialValue ] isFactory
 
@@ -4745,7 +4765,7 @@ let transformStaticProperty
         let fallback = Util.getDefaultValueForType com ctx getterMemb.Body.Type
 
         let initialValue, isFactory, externalFields, initialValueStmts =
-            getInitialValue com ctx name getterMemb true fallback
+            getInitialValue com ctx name getterMemb true false fallback
 
         // Check if setter has custom logic beyond simple assignment to the property itself
         // For simple properties, we don't need setter functions since StaticProperty handles storage

src/Fable.Transforms/Python/PYTHON-UNION.md

@@ -49,8 +49,8 @@ class MyUnion_CaseB(_MyUnion):
 
 @tagged_union(2)
 class MyUnion_CaseC(_MyUnion):
-    x: float
-    y: float
+    x_: float
+    y_: float
 
 
 # Type alias - THE public union type for annotations
@@ -115,6 +115,27 @@ class ModuleB_Result_Error(_ModuleB_Result):
 
 The scoped name is derived from `FSharp2Fable.Helpers.getEntityDeclarationName`, which includes the module path to ensure unique names across the entire compilation.
 
+### Case Field Naming: `toFieldSnakeCase`
+
+Case class field annotations use `Naming.toFieldSnakeCase` (in `Prelude.fs`), the same
+convention as record fields:
+
+- PascalCase names (including the compiler-generated `Item`, `Item1`, ...) convert to
+  plain snake_case: `Item` → `item`, `MyField` → `my_field`.
+- camelCase/lowercase names convert to snake_case **with a trailing underscore**:
+  `x` → `x_`, `name` → `name_`.
+
+The suffix exists because the `Union` base class defines properties such as `name`. A
+field annotation that shadows an inherited property is treated by `@dataclass` as having
+a *default value* (the inherited property object found via `getattr`), which breaks
+construction with "non-default argument follows default argument" as soon as another
+field without a default follows (issue #4645, PR #4647).
+
+This is safe because the dataclass field name is purely internal: union construction is
+positional (`Union_Case("v1", "v2")`) and compiled field access goes through the case
+class attributes generated with the same convention, while indexed access uses
+`self.fields[i]`.
+
 ### Library Types Keep Simple Names
 
 F# core library types (`Result`, `FSharpChoice`) use simple case names without prefix for cleaner interop:
@@ -234,15 +255,15 @@ def make_union(uci: CaseInfo, values: Array[Any]) -> Any:
 u = MyUnion_CaseA(42)
 u = MyUnion_CaseC(1.0, 2.0)
 
-# Field access - direct attributes with F# names
+# Field access - direct attributes (camelCase F# names get a '_' suffix, see Case Field Naming)
 print(u.item)      # For CaseA/CaseB
-print(u.x, u.y)    # For CaseC
+print(u.x_, u.y_)  # For CaseC
 
 # Pattern matching - __match_args__ automatic from dataclass
 match u:
     case MyUnion_CaseA(item=value):
         print(f"CaseA: {value}")
-    case MyUnion_CaseC(x=x, y=y):
+    case MyUnion_CaseC(x_=x, y_=y):
         print(f"CaseC: {x}, {y}")
 
 # isinstance works with base class (underscore-prefixed)
@@ -319,15 +340,40 @@ In `Fable2Python.Annotation.fs`, the `makeEntityTypeAnnotation` function:
 1. If inside the same union base class (`ctx.EnclosingUnionBaseClass = Some name`): use base class name with underscore
 2. Otherwise: use type alias (strip underscore prefix)
 
-### Reflection Constructor
+### Runtime References Target the Base Class
 
-In `Replacements.fs`, `tryConstructor` adds underscore prefix for union types so reflection gets the base class:
+The type alias (`Demo`) is typing-only: at runtime it is a `TypeAliasType` that carries no
+members. Whenever the compiler needs to reference the union as a *runtime value*, the
+reference must be redirected to the private base class (`_Demo`). This redirect is
+centralized in one helper, `FSharp2Fable.Util.redirectUnionToPythonBaseClass`:
 
 ```fsharp
-| Some(IdentExpr ident) when ent.IsFSharpUnion ->
-    Some(IdentExpr { ident with Name = "_" + ident.Name })
+let redirectUnionToPythonBaseClass (expr: Fable.Expr) =
+    match expr with
+    | Fable.IdentExpr ident -> Fable.IdentExpr { ident with Name = "_" + ident.Name }
+    | Fable.Import({ Kind = Fable.ClassImport _ } as info, typ, range) ->
+        Fable.Import({ info with Selector = "_" + info.Selector }, typ, range)
+    | expr -> expr
 ```
 
+It handles both shapes a union entity reference can take — a bare identifier for a
+same-file union and an internal class import for a union defined in another file — and
+leaves external entities (`[<Import>]`, kind `UserImport`) untouched.
+
+Current call sites:
+
+1. **Reflection constructor** — `tryConstructor` in `Python/Replacements.fs` redirects so
+   reflection gets the base class (which has the `cases()` method).
+2. **Attached static members** — in `FSharp2Fable.Util.fs`, when resolving a call to a
+   static member of an `[<AttachMembers>]` union (issue #4634). The static members live on
+   the base class; accessing them on the type alias raises `AttributeError` at runtime.
+
+Note that call site 2 lives in a *shared* compiler file (`FSharp2Fable.Util.fs`), gated on
+`com.Options.Language = Python` — it is the one place that still knows both the entity
+(`IsFSharpUnion`) and that the reference is for static member access. If the naming
+convention ever changes, update the helper and the emission sites in
+`Fable2Python.Transforms.fs` together.
+
 ## Files Modified
 
 1. [src/fable-library-py/fable_library/union.py](src/fable-library-py/fable_library/union.py) - `@tagged_union` decorator
@@ -336,7 +382,8 @@ In `Replacements.fs`, `tryConstructor` adds underscore prefix for union types so
 4. [src/Fable.Transforms/Python/Fable2Python.Transforms.fs](src/Fable.Transforms/Python/Fable2Python.Transforms.fs) - `transformUnion`, context tracking
 5. [src/Fable.Transforms/Python/Fable2Python.Annotation.fs](src/Fable.Transforms/Python/Fable2Python.Annotation.fs) - Type annotation logic for union types
 6. [src/Fable.Transforms/Python/Fable2Python.Reflection.fs](src/Fable.Transforms/Python/Fable2Python.Reflection.fs) - Case constructor generation
-7. [src/Fable.Transforms/Python/Replacements.fs](src/Fable.Transforms/Python/Replacements.fs) - `tryConstructor` adds underscore for unions
+7. [src/Fable.Transforms/Python/Replacements.fs](src/Fable.Transforms/Python/Replacements.fs) - `tryConstructor` redirects unions to the base class
+8. [src/Fable.Transforms/FSharp2Fable.Util.fs](../FSharp2Fable.Util.fs) - `redirectUnionToPythonBaseClass` helper; static member access on `[<AttachMembers>]` unions (Python-gated)
 
 ## Comparison with Original Fable Design
 

src/Fable.Transforms/Python/Replacements.fs

@@ -775,11 +775,10 @@ let tryConstructor com (ent: Entity) =
         tryEntityIdent com (ent.FullName |> Naming.toPythonNaming)
     else
         match FSharp2Fable.Util.tryEntityIdentMaybeGlobalOrImported com ent with
-        | Some(IdentExpr ident) when ent.IsFSharpUnion ->
-            // For F# union types, the base class is prefixed with underscore (_UnionName)
-            // This is needed for both reflection (base class has cases() method) and
-            // type annotations (self inside base class methods)
-            Some(IdentExpr { ident with Name = "_" + ident.Name })
+        | Some expr when ent.IsFSharpUnion ->
+            // The union runtime members (e.g. the cases() method used by reflection) live
+            // on the private base class, not the type alias. See PYTHON-UNION.md.
+            FSharp2Fable.Util.redirectUnionToPythonBaseClass expr |> Some
         | other -> other
 
 let constructor com ent =

tests/Python/MiscTestsHelper.fs

@@ -31,3 +31,14 @@ type Vector2<[<Measure>] 'u> = Vector2 of x: float<'u> * y: float<'u> with
 
   static member inline ( + ) (Vector2(ax, ay), Vector2(bx, by)) = Vector2(ax + bx, ay + by)
   static member inline ( * ) (scalar, Vector2(x, y)) = Vector2(scalar * x, scalar * y)
+
+// Issue4634: an [<AttachMembers>] union with static members, defined in a separate
+// module/file so use sites reference it across modules (an Import, not a same-file
+// identifier). This exercises the cross-file path of the static-member-on-union fix.
+[<Fable.Core.AttachMembers>]
+type CrossModuleDemo =
+    | A of string
+    | B
+
+    static member propDefault = A "prop"
+    static member methDefault() = A "meth"

tests/Python/TestNonRegression.fs

@@ -182,6 +182,30 @@ let ``test class name casing`` () =
     let x = Issue3811.flowchartDirection.tb' ()
     equal Issue3811.FlowchartDirection.TB x
 
+module Issue4634 =
+    // An attached static property on a union whose getter constructs a union case must
+    // not be emitted eagerly in the base class body (the case classes don't exist yet),
+    // and use sites must reach the static member on the base class, not the type alias.
+    [<AttachMembers>]
+    type Demo =
+        | A of string
+        | B
+
+        static member propDefault = A "prop"
+        static member methDefault() = A "meth"
+
+[<Fact>]
+let ``test attached static members on union work`` () =
+    equal (Issue4634.A "prop") Issue4634.Demo.propDefault
+    equal (Issue4634.A "meth") (Issue4634.Demo.methDefault())
+
+[<Fact>]
+let ``test attached static members on union work across modules`` () =
+    // CrossModuleDemo is defined in MiscTestsHelper.fs, so these use sites compile to an
+    // import of the union rather than a same-file identifier — covering the cross-file path.
+    equal (Fable.Tests.MiscTestsHelper.A "prop") Fable.Tests.MiscTestsHelper.CrossModuleDemo.propDefault
+    equal (Fable.Tests.MiscTestsHelper.A "meth") (Fable.Tests.MiscTestsHelper.CrossModuleDemo.methDefault())
+
 module Issue3972 =
     type IInterface =
         abstract member LOL : int