Skip to content

Commit 3afed51

Browse files
authored
[Python] More typing fixes (#4334)
1 parent 283f0cd commit 3afed51

3 files changed

Lines changed: 81 additions & 16 deletions

File tree

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

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -306,11 +306,16 @@ let stdlibModuleAnnotation (com: IPythonCompiler) ctx moduleName memberName args
306306
match args with
307307
| Expression.Name { Id = Identifier Ellipsis } :: _xs -> Expression.ellipsis
308308
| _ ->
309-
args
310-
|> List.removeAt (args.Length - 1)
309+
let argsWithoutReturn = args |> List.removeAt (args.Length - 1)
310+
311+
argsWithoutReturn
311312
|> List.choose (
312313
function
313-
| Expression.Name { Id = Identifier "None" } when args.Length = 2 -> None
314+
// Filter out None (unit) only when it's the sole argument.
315+
// F# `unit -> T` means "takes no args" in Python: Callable[[], T]
316+
// But `unit -> 'a -> T` uncurried to `(unit, 'a) -> T` must keep
317+
// None to match the actual function signature with unit parameter.
318+
| Expression.Name { Id = Identifier "None" } when argsWithoutReturn.Length = 1 -> None
314319
| x -> Some x
315320
)
316321
|> Expression.list
@@ -441,7 +446,7 @@ let rec typeAnnotation
441446
fableModuleAnnotation com ctx "option" "Option" [ Expression.none ], []
442447
| Fable.Option(genArg, _) ->
443448
// Must match mustWrapOption logic in Transforms.Util.fs
444-
// Wrap when: Any, Unit, GenericParam, or nested Option
449+
// Wrap when: Any, Unit, GenericParam, nested Option, or callable with generic params
445450
match genArg with
446451
| Fable.Option _
447452
| Fable.Any
@@ -450,8 +455,15 @@ let rec typeAnnotation
450455
// Use full Option type annotation (code will use SomeWrapper)
451456
let resolved, stmts = resolveGenerics com ctx [ genArg ] repeatedGenerics
452457
fableModuleAnnotation com ctx "option" "Option" resolved, stmts
458+
| Fable.LambdaType _
459+
| Fable.DelegateType _ when containsGenericParams genArg ->
460+
// Callable types with generic parameters (e.g., Callable[[_A], _B])
461+
// Must use Option[T] form because runtime wraps with SomeWrapper
462+
let resolved, stmts = resolveGenerics com ctx [ genArg ] repeatedGenerics
463+
fableModuleAnnotation com ctx "option" "Option" resolved, stmts
453464
| _ ->
454-
// For concrete types, erase to T | None (simpler, no wrapper needed)
465+
// For concrete types (including DeclaredTypes with generics like FSharpList[T]),
466+
// erase to T | None (simpler, no wrapper needed)
455467
let resolved, stmts = typeAnnotation com ctx repeatedGenerics genArg
456468
Expression.binOp (resolved, BitOr, Expression.none), stmts
457469
| Fable.Tuple(genArgs, _) -> makeGenericTypeAnnotation com ctx "tuple" genArgs repeatedGenerics, []

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

Lines changed: 38 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1109,19 +1109,36 @@ let transformCall (com: IPythonCompiler) ctx range callee (callInfo: Fable.CallI
11091109
let transformCurriedApply com ctx range (TransformExpr com ctx (applied, stmts)) args =
11101110
((applied, stmts), args)
11111111
||> List.fold (fun (applied, stmts) arg ->
1112-
let args, stmts' =
1112+
let args, argStmts =
11131113
match arg with
11141114
// TODO: If arg type is unit but it's an expression with potential
11151115
// side-effects, we need to extract it and execute it before the call
11161116

11171117
// TODO: discardUnitArg may still be needed in some cases
11181118
| Fable.Value(Fable.UnitConstant, _) -> [], []
11191119
| Fable.IdentExpr ident when ident.Type = Fable.Unit -> [], []
1120-
| TransformExpr com ctx (arg, stmts') -> [ arg ], stmts'
1120+
| arg ->
1121+
let argExpr, transformStmts = com.TransformAsExpr(ctx, arg)
1122+
// When a Call or CurriedApply result is passed as an argument,
1123+
// check if we need to erase Option[T] to T | None
1124+
let argExpr =
1125+
if needsOptionEraseForBinding arg arg.Type then
1126+
wrapInOptionErase com ctx argExpr
1127+
else
1128+
argExpr
1129+
1130+
[ argExpr ], transformStmts
11211131

1122-
callFunction range applied args [], stmts @ stmts'
1132+
callFunction range applied args [], stmts @ argStmts
11231133
)
11241134

1135+
/// Extract the expected return type from a return strategy, unwrapping ResourceManager if needed
1136+
let rec private getExpectedReturnType (strategy: ReturnStrategy option) =
1137+
match strategy with
1138+
| Some(Return(Some expectedType)) -> Some expectedType
1139+
| Some(ResourceManager inner) -> getExpectedReturnType inner
1140+
| _ -> None
1141+
11251142
let transformCallAsStatements com ctx range t returnStrategy callee callInfo =
11261143
let argsLen (i: Fable.CallInfo) =
11271144
List.length i.Args
@@ -1141,17 +1158,18 @@ let transformCallAsStatements com ctx range t returnStrategy callee callInfo =
11411158
| _ ->
11421159
let expr, stmts = transformCall com ctx range callee callInfo
11431160
// Check if we need to cast Option[T] to T | None for return statements
1161+
// Also handles ResourceManager-wrapped return strategies (e.g., inside `with` blocks)
11441162
let expr =
1145-
match returnStrategy with
1146-
| Some(Return(Some expectedType)) ->
1163+
match getExpectedReturnType returnStrategy with
1164+
| Some expectedType ->
11471165
// Create a temporary Fable.Call to check if erase is needed
11481166
let callExpr = Fable.Call(callee, callInfo, t, range)
11491167

11501168
if needsOptionEraseForReturn callExpr expectedType then
11511169
wrapInOptionErase com ctx expr
11521170
else
11531171
expr
1154-
| _ -> expr
1172+
| None -> expr
11551173

11561174
stmts @ (expr |> resolveExpr ctx t returnStrategy)
11571175

@@ -3996,7 +4014,14 @@ let transformUnion (com: IPythonCompiler) ctx (ent: Fable.Entity) (entName: stri
39964014
// Convert to snake_case and clean to remove invalid characters like apostrophes
39974015
// Handles: "Item" -> "item", "Item1" -> "item1", "MyField" -> "my_field"
39984016
let fieldName = field.Name |> Naming.toSnakeCase |> Helpers.clean
3999-
let ta, _ = Annotation.typeAnnotation com caseCtx None field.FieldType
4017+
// Uncurry lambda types for field annotations since union case fields
4018+
// store uncurried functions at runtime (same as record fields)
4019+
let fieldType =
4020+
match field.FieldType with
4021+
| Fable.LambdaType _ -> FableTransforms.uncurryType field.FieldType
4022+
| _ -> field.FieldType
4023+
4024+
let ta, _ = Annotation.typeAnnotation com caseCtx None fieldType
40004025
let target = Expression.name (fieldName, Store)
40014026
// Use annAssign to generate: field: type (not field = type)
40024027
Statement.annAssign (target, annotation = ta, simple = true)
@@ -4763,6 +4788,12 @@ let rec transformDeclaration (com: IPythonCompiler) ctx (decl: Fable.Declaration
47634788
let value, stmts = transformAsExpr com ctx decl.Body
47644789
let name = com.GetIdentifier(ctx, Naming.toPythonNaming decl.Name)
47654790
let ta, _ = Annotation.typeAnnotation com ctx None decl.Body.Type
4791+
// Erase Option wrapper if needed (Call/CurriedApply returning Option[T] to T | None)
4792+
let value =
4793+
if needsOptionEraseForBinding decl.Body decl.Body.Type then
4794+
wrapInOptionErase com ctx value
4795+
else
4796+
value
47664797

47674798
stmts @ declareModuleMember com ctx info.IsPublic name (Some ta) value
47684799
else

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

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -100,34 +100,56 @@ module Util =
100100
|| hasAttribute Atts.emitIndexer atts
101101
|| hasAttribute Atts.emitProperty atts
102102

103+
/// Check if a type contains any generic parameters (recursively).
104+
/// Used to determine if Option<T> should use Option[T] annotation vs T | None.
105+
/// Note: This is duplicated from Annotation.fs due to compilation order (Util.fs compiles first).
106+
let private containsGenericParams (t: Fable.Type) =
107+
FSharp2Fable.Util.getGenParamNames [ t ] |> List.isEmpty |> not
108+
103109
/// Check if an Option type has a concrete inner type (erases to T | None).
104110
/// Returns true when inner type is concrete, meaning Option[T] -> T | None.
105111
let hasConcreteOptionInner (typ: Fable.Type) =
106112
match typ with
107113
| Fable.Option(innerType, _) -> not (mustWrapOption innerType)
108114
| _ -> false
109115

116+
/// Check if inner type of Option is a callable with generic params (needs wrapping)
117+
let private isCallableWithGenerics (t: Fable.Type) =
118+
match t with
119+
| Fable.LambdaType _
120+
| Fable.DelegateType _ -> containsGenericParams t
121+
| _ -> false
122+
110123
/// Check if a call expression needs Option erase from Option[T] to T | None.
111124
/// When target type is Option<ConcreteType> (erased to T | None), we need to erase
112125
/// because library functions use Option[T] (wrapped form) in their signatures,
113126
/// but actual runtime values are not wrapped for concrete types.
114-
let private needsOptionEraseForCall (targetType: Fable.Type) =
127+
/// Also handles function types (LambdaType) where the return type contains a concrete Option,
128+
/// using the erase() overload for Callable[..., Option[T]] -> Callable[..., T | None].
129+
let rec private needsOptionEraseForCall (targetType: Fable.Type) =
115130
match targetType with
116-
| Fable.Option(tgtInner, _) when not (mustWrapOption tgtInner) -> true
131+
// Don't erase if inner type is callable with generic params - both annotation and runtime use wrapped form
132+
| Fable.Option(tgtInner, _) when not (mustWrapOption tgtInner) && not (isCallableWithGenerics tgtInner) -> true
133+
// Handle function types where return type is a concrete Option
134+
// erase() has an overload: Callable[..., Option[T]] -> Callable[..., T | None]
135+
| Fable.LambdaType(_, returnType) -> needsOptionEraseForCall returnType
136+
| Fable.DelegateType(_, returnType) -> needsOptionEraseForCall returnType
117137
| _ -> false
118138

119139
/// Check if binding needs Option erase from Option[T] to T | None.
120140
let needsOptionEraseForBinding (value: Fable.Expr) (targetType: Fable.Type) =
121141
match value with
122-
| Fable.Call _ -> needsOptionEraseForCall targetType
142+
| Fable.Call _
143+
| Fable.CurriedApply _ -> needsOptionEraseForCall targetType
123144
| _ -> false
124145

125146
/// Check if return expression needs Option erase from Option[T] to T | None.
126147
/// Used when returning from a function where the expected return type is T | None
127148
/// but the actual expression returns Option[T] (wrapped form).
128149
let needsOptionEraseForReturn (value: Fable.Expr) (expectedReturnType: Fable.Type) =
129150
match value with
130-
| Fable.Call _ -> needsOptionEraseForCall expectedReturnType
151+
| Fable.Call _
152+
| Fable.CurriedApply _ -> needsOptionEraseForCall expectedReturnType
131153
| _ -> false
132154

133155
/// Recursively check if a type contains Option (wrapped or erased).

0 commit comments

Comments
 (0)