Skip to content

Commit 283f0cd

Browse files
authored
[Python] Type annotation fixes (#4332)
1 parent 299b30d commit 283f0cd

File tree

4 files changed

+98
-24
lines changed

4 files changed

+98
-24
lines changed

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

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,18 @@ open Fable.Transforms.Python.AST
1212
open Fable.Transforms.Python.Types
1313
open Fable.Transforms.Python.Util
1414

15-
/// Check if type is an inref (in-reference) or Any type.
15+
/// Check if type is an inref wrapping a struct/value type.
1616
/// In F#, struct instance method's `this` parameter is represented as inref<StructType>,
1717
/// but in Python the struct is passed directly, not wrapped in FSharpRef.
18-
let isInRefOrAnyType (com: IPythonCompiler) =
18+
/// For regular inref<T> parameters (where T is a primitive or non-struct), we keep FSharpRef.
19+
let isStructInRefType (com: IPythonCompiler) =
1920
function
20-
| Replacements.Util.IsInRefType com _ -> true
21-
| Fable.Any -> true
21+
| Replacements.Util.IsInRefType com innerType ->
22+
match innerType with
23+
| Fable.DeclaredType(entRef, _) ->
24+
let ent = com.GetEntity(entRef)
25+
ent.IsValueType
26+
| _ -> false
2227
| _ -> false
2328

2429
let tryPyConstructor (com: IPythonCompiler) ctx ent =
@@ -698,9 +703,10 @@ let makeBuiltinTypeAnnotation com ctx typ repeatedGenerics kind =
698703
match kind with
699704
| Replacements.Util.BclGuid -> stdlibModuleTypeHint com ctx "uuid" "UUID" [] repeatedGenerics
700705
| Replacements.Util.FSharpReference genArg ->
701-
// In F#, struct instance method's `this` parameter is represented as inref<StructType>,
706+
// For struct instance methods, `this` is represented as inref<StructType> in F#,
702707
// but in Python the struct is passed directly, not wrapped in FSharpRef.
703-
if isInRefOrAnyType com typ then
708+
// For regular byref/inref/outref parameters, we use FSharpRef.
709+
if isStructInRefType com typ then
704710
typeAnnotation com ctx repeatedGenerics genArg
705711
else
706712
let resolved, stmts = resolveGenerics com ctx [ genArg ] repeatedGenerics

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

Lines changed: 68 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,6 @@ let getMemberArgsAndBody (com: IPythonCompiler) ctx kind hasSpread (args: Fable.
8282
Set.difference (Annotation.getGenericTypeParams [ thisArg.Type ]) ctx.ScopedTypeParams
8383

8484
let body =
85-
// TODO: If ident is not captured maybe we can just replace it with "this"
8685
if isIdentUsed thisArg.Name body then
8786
let thisKeyword = Fable.IdentExpr { thisArg with Name = "self" }
8887

@@ -365,14 +364,23 @@ let transformCast (com: IPythonCompiler) (ctx: Context) t e : Expression * State
365364

366365
// Wrap ResizeArray (Python list) when cast to IEnumerable
367366
// Python lists don't implement IEnumerable_1, so they need wrapping
367+
// Optimization: If ResizeArray was created via of_seq from IEnumerable, use the original arg
368368
| Types.ienumerableGeneric, _ when
369369
match e.Type with
370370
| Fable.Array(_, Fable.ArrayKind.ResizeArray) -> true
371371
| Fable.DeclaredType(entRef, _) when entRef.FullName = Types.resizeArray -> true
372372
| _ -> false
373373
->
374374
let listExpr, stmts = com.TransformAsExpr(ctx, e)
375-
libCall com ctx None "util" "to_enumerable" [ listExpr ], stmts
375+
// Check if the expression is of_seq(arg) - if so, use arg directly (already IEnumerable)
376+
match listExpr with
377+
| Expression.Call {
378+
Func = Expression.Name { Id = Identifier "of_seq" }
379+
Args = [ innerArg ]
380+
} ->
381+
// Skip both of_seq and to_enumerable - use original IEnumerable directly
382+
innerArg, stmts
383+
| _ -> libCall com ctx None "util" "to_enumerable" [ listExpr ], stmts
376384

377385
| _ -> com.TransformAsExpr(ctx, e)
378386
| Fable.Number(Float32, _), _ ->
@@ -601,7 +609,59 @@ let transformObjectExpr
601609
// A generic class nested in another generic class cannot use same type variables. (PEP-484)
602610
let ctx = { ctx with TypeParamsScope = ctx.TypeParamsScope + 1 }
603611

612+
// Check if any member body uses ThisValue from an outer scope (e.g., inside a constructor).
613+
// ThisValue specifically represents `self` in a constructor/method context.
614+
// Note: IsThisArgument identifiers are captured via default arguments (x: Any=x),
615+
// so we only need to handle explicit ThisValue here.
616+
let usesOuterThis =
617+
members
618+
|> List.exists (fun memb ->
619+
memb.Body
620+
|> deepExists (
621+
function
622+
| Fable.Value(Fable.ThisValue _, _) -> true
623+
| _ -> false
624+
)
625+
)
626+
627+
// Only generate capture statement if outer this is actually used.
628+
// This allows inner class methods to reference the outer instance via "_this"
629+
// while using standard "self" for the inner instance (satisfies Pylance).
630+
let thisCaptureStmts =
631+
if usesOuterThis then
632+
let anyType = stdlibModuleAnnotation com ctx "typing" "Any" []
633+
634+
[
635+
Statement.assign (Expression.name "_this", anyType, value = Expression.name "self")
636+
]
637+
else
638+
[]
639+
640+
// Replace ThisValue in the body with an identifier reference to "_this"
641+
// This ensures that outer self references correctly bind to the captured variable
642+
let replaceThisValue (body: Fable.Expr) =
643+
if usesOuterThis then
644+
body
645+
|> visitFromInsideOut (
646+
function
647+
| Fable.Value(Fable.ThisValue typ, r) ->
648+
Fable.IdentExpr
649+
{
650+
Name = "_this"
651+
Type = typ
652+
IsMutable = false
653+
IsThisArgument = false
654+
IsCompilerGenerated = true
655+
Range = r
656+
}
657+
| e -> e
658+
)
659+
else
660+
body
661+
604662
let makeMethod prop hasSpread (fableArgs: Fable.Ident list) (fableBody: Fable.Expr) decorators =
663+
let fableBody = replaceThisValue fableBody
664+
605665
let args, body, returnType =
606666
getMemberArgsAndBody com ctx (Attached(isStatic = false)) hasSpread fableArgs fableBody
607667

@@ -614,16 +674,7 @@ let transformObjectExpr
614674
com.GetIdentifier(ctx, Naming.toPythonNaming name)
615675

616676
let self = Arg.arg "self"
617-
618-
let args =
619-
match decorators with
620-
// Remove extra parameters from getters, i.e __unit=None
621-
| [ Expression.Name { Id = Identifier "property" } ] ->
622-
{ args with
623-
Args = [ self ]
624-
Defaults = []
625-
}
626-
| _ -> { args with Args = self :: args.Args }
677+
let args = { args with Args = self :: args.Args }
627678

628679
// Calculate type parameters for generic object expression methods
629680
let argTypes = fableArgs |> List.map _.Type
@@ -666,12 +717,16 @@ let transformObjectExpr
666717
| Fable.Lambda(arg, body, _) -> [ arg ], body
667718
| _ -> memb.Args, memb.Body
668719

720+
// Replace ThisValue with this_ identifier for outer self references
721+
let body = replaceThisValue body
722+
669723
let args', body', returnType =
670724
getMemberArgsAndBody com ctx (NonAttached memb.Name) false args body
671725

672726
let name = com.GetIdentifier(ctx, Naming.toPythonNaming memb.Name)
673727
let self = Arg.arg "self"
674728
let args' = { args' with Args = self :: args'.Args }
729+
675730
let argTypes = args |> List.map _.Type
676731
let typeParams = Annotation.calculateMethodTypeParams com ctx argTypes body.Type
677732

@@ -716,7 +771,7 @@ let transformObjectExpr
716771

717772
let stmt = Statement.classDef (name, body = classBody, bases = interfaces)
718773

719-
Expression.call (Expression.name name), [ stmt ] @ stmts
774+
Expression.call (Expression.name name), thisCaptureStmts @ [ stmt ] @ stmts
720775

721776

722777
let transformCallArgs

src/Fable.Transforms/Python/Replacements.fs

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1728,11 +1728,18 @@ let resizeArrays (com: ICompiler) (ctx: Context) r (t: Type) (i: CallInfo) (this
17281728
| ".ctor", _, [] -> makeResizeArray (getElementType t) [] |> Some
17291729
| ".ctor", _, [ ExprType(Number _) ] -> makeResizeArray (getElementType t) [] |> Some
17301730
| ".ctor", _, [ ArrayOrListLiteral(vals, _) ] -> makeResizeArray (getElementType t) vals |> Some
1731-
// Use Array constructor which accepts both Iterable and IEnumerable_1
1732-
| ".ctor", _, args ->
1733-
Helper.LibCall(com, "array", "Array", t, args, ?loc = r)
1734-
|> withTag "array"
1735-
|> Some
1731+
// When a ResizeArray is cast to IEnumerable and passed to ResizeArray constructor,
1732+
// unwrap the cast since list() can handle lists directly (avoids to_enumerable wrapper)
1733+
| ".ctor", _, [ TypeCast(innerExpr, DeclaredType(ent, _)) ] when
1734+
ent.FullName = Types.ienumerableGeneric
1735+
&& match innerExpr.Type with
1736+
| Array(_, ResizeArray) -> true
1737+
| DeclaredType(entRef, _) when entRef.FullName = Types.resizeArray -> true
1738+
| _ -> false
1739+
->
1740+
Helper.GlobalCall("list", t, [ innerExpr ], ?loc = r) |> Some
1741+
// Use resize_array.of_seq to create a list from IEnumerable_1 or any iterable
1742+
| ".ctor", _, args -> Helper.LibCall(com, "resize_array", "of_seq", t, args, ?loc = r) |> Some
17361743
| "get_Item", Some ar, [ idx ] -> getExpr r t ar idx |> Some
17371744
| "set_Item", Some ar, [ idx; value ] -> setExpr r ar idx value |> Some
17381745
| "Add", Some ar, [ arg ] ->

src/fable-library-py/fable_library/resize_array.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,11 @@
99
from .util import to_iterable
1010

1111

12+
def of_seq[T](items: Iterable[T] | IEnumerable_1[T]) -> list[T]:
13+
"""Create a ResizeArray (Python list) from an iterable or IEnumerable_1."""
14+
return list(to_iterable(items))
15+
16+
1217
def exists[T](predicate: Callable[[T], bool], xs: list[T]) -> bool:
1318
"""Test if a predicate is true for at least one element in a list."""
1419
return any(predicate(x) for x in xs)
@@ -146,6 +151,7 @@ def contains[T](value: T, xs: list[T], cons: Any | None = None) -> bool:
146151
"index_of",
147152
"insert_range_in_place",
148153
"iterate",
154+
"of_seq",
149155
"remove",
150156
"remove_all_in_place",
151157
"remove_range",

0 commit comments

Comments
 (0)