Skip to content

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

Open
dbrattli wants to merge 2 commits into
mainfrom
fix/py-4634-attachmembers-union-static
Open

fix(python): make [<AttachMembers>] union static members work (#4634)#4636
dbrattli wants to merge 2 commits into
mainfrom
fix/py-4634-attachmembers-union-static

Conversation

@dbrattli

@dbrattli dbrattli commented Jun 9, 2026

Copy link
Copy Markdown
Collaborator

Fixes #4634

Problem

A discriminated union with [<AttachMembers>] is emitted in Python as three pieces:

  • a private base class _Demo(Union) that holds the attached static members,
  • the case classes Demo_A, Demo_B,
  • a public type alias type Demo = Demo_A | Demo_B.

Given:

[<AttachMembers>]
type Demo =
    | A of string
    | B
    static member propDefault = A "prop"
    static member methDefault() = A "meth"

this shape was broken in two ways (plus a latent third):

  1. Eager evaluation, raising NameError at import. The static property was emitted as a class-body assignment inside _Demo:

    prop_default = StaticProperty["Demo"](Demo_A("prop"))

    Python evaluates Demo_A("prop") while defining _Demo, before the case class exists, so the module failed to import.

  2. Use sites hit the type alias. Demo.propDefault / Demo.methDefault() compiled to Demo.prop_default / Demo.meth_default(), but Demo is a TypeAliasType that does not carry the members, raising AttributeError. This broke both static methods and static properties.

  3. (Latent) bad type annotation. When the union lived in a module, the descriptor annotation was emitted as a bare StaticLazyProperty[Issue4634_Demo] instead of a string forward reference, also raising NameError. The eager-evaluation bug masked it.

Fix

  • getInitialValue / transformStaticProperty (Fable2Python.Transforms.fs): defer a read-only union static property value with a lambda, producing StaticLazyProperty["Demo"](lambda: Demo_A("prop")). This avoids the forward reference and matches F# getter semantics (re-evaluated per access). Scoped to getter-only to preserve settable-field semantics.
  • transformStaticProperty: emit the descriptor type annotation as a string forward reference whenever the property type is the enclosing union (compare by FullName, not only DisplayName).
  • makeCallWithArgInfo (FSharp2Fable.Util.fs): when referencing a union to reach a static member in Python, target the _Demo base class instead of the Demo type alias — the same underscore convention already used for reflection. This change is gated to Python + IsFSharpUnion and is a no-op for every other target.

Verification

  • New regression test Issue4634 in tests/Python/TestNonRegression.fs.
  • Full Python suite: 2351 passed.
  • Pyright: no new errors introduced (the pre-existing 34 are in unrelated files).
  • Generated module now imports and runs: x = A "prop", y = A "meth".

🤖 Generated with Claude Code

A discriminated union with [<AttachMembers>] is emitted in Python as a
private base class `_Demo` (which holds the attached static members), the
case classes `Demo_A`/`Demo_B`, and a public type alias `Demo = Demo_A | Demo_B`.

This shape was broken in two ways:

1. A static property such as `static member propDefault = A "prop"` was
   emitted eagerly in the base-class body as
   `prop_default = StaticProperty["Demo"](Demo_A("prop"))`. Python evaluates
   `Demo_A("prop")` while defining `_Demo`, before the case class exists, so
   the module failed at import with `NameError`. Read-only static property
   values on unions are now deferred with a lambda (`StaticLazyProperty`),
   which also matches F# getter semantics (re-evaluated on each access).

2. Use sites such as `Demo.propDefault`/`Demo.methDefault()` referenced the
   public type alias, which is a `TypeAliasType` and does not carry the
   members, raising `AttributeError`. Static member access on a union now
   targets the underscore-prefixed base class `_Demo`, consistent with the
   convention already used for reflection.

Also emit the descriptor's type annotation as a string forward reference
whenever the property type is the enclosing union, since the type alias is
only defined after the base class.

The FSharp2Fable change is gated to Python + IsFSharpUnion and is a no-op
for all other targets.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@github-actions

github-actions Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Python Type Checking Results (Pyright)

Metric Value
Total errors 34
Files with errors 4
Excluded files 4
New errors ✅ No
Excluded files with errors (4 files)

These files have known type errors and are excluded from CI. Remove from pyrightconfig.ci.json as errors are fixed.

File Errors Status
temp/tests/Python/test_hash_set.py 18 Excluded
temp/tests/Python/test_applicative.py 12 Excluded
temp/tests/Python/test_nested_and_recursive_pattern.py 2 Excluded
temp/tests/Python/fable_modules/thoth_json_python/encode.py 2 Excluded

The original fix only rewrote the same-file `IdentExpr` reference to the
union base class (`_Demo`). A union defined in another file is referenced
via an `Import`, which fell through to the type alias and raised
`AttributeError` on static member access. Rewrite the import `Selector`
to the base class too. Adds a cross-module regression test and cleans up
the self-reference type-annotation check.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@dbrattli

dbrattli commented Jun 9, 2026

Copy link
Copy Markdown
Collaborator Author

Follow-up: cross-module static member access was still broken

While adding a cross-module regression test, I found the original fix only handled the same-file case. entityIdent yields a bare IdentExpr for a same-file union but an Import for a union defined in another file — and the Import case fell through to the type alias, so static member access across modules still raised:

AttributeError: 'typing.TypeAliasType' object has no attribute 'prop_default'

Changes (commit 7733d0594)

  • FSharp2Fable.Util.fs — added an Import arm alongside the IdentExpr arm that rewrites the import Selector to the underscore-prefixed base class (_Demo). transformImport uses the selector string as the imported name, so cross-file references now resolve to the base class that actually holds the static members.
  • MiscTestsHelper.fs + TestNonRegression.fs — added a cross-module regression test: CrossModuleDemo is defined in a separately-compiled module and its static property/method are referenced from TestNonRegression.fs. This exercises the Import path; the prior commit's test only covered same-file references.
  • Fable2Python.Transforms.fs — minor cleanup: replaced the dense inline let … in inside the when guard with a named isSelfReference binding.

Generated output (cross-module)

from misc_tests_helper import (CrossModuleDemo_A, _CrossModuleDemo)
...
Testing_equal(CrossModuleDemo_A("prop"), _CrossModuleDemo.prop_default)
Testing_equal(CrossModuleDemo_A("meth"), _CrossModuleDemo.meth_default())

Verification

  • Full Python suite: 2352 passed (was 2351; +1 for the new cross-module test, now green).

Note

build.sh test python returned exit code 0 even while the new test was failing — the failure only showed in the pytest summary line, not the process exit. Worth keeping in mind for CI.

🤖 Generated with Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fable Python: [<AttachMembers>] union static property is emitted eagerly before union cases exist

1 participant