Skip to content

Commit 321bbdb

Browse files
MangelMaximedbrattliclaude
authored
[JS/TS/Python] Fix record/value type constructor including static backing fields as parameters (#4405)
Co-authored-by: Dag Brattli <dag.brattli@cognite.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent ab82e32 commit 321bbdb

8 files changed

Lines changed: 124 additions & 25 deletions

File tree

‎src/Fable.Cli/CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
99

1010
### Fixed
1111

12+
* [JS/TS/Python] Fix record/struct types augmented with `static let` or `static member val` generating extra constructor parameters for each static field, causing constructor arguments to be assigned to wrong slots (by @MangelMaxime)
1213
* [TS] Annotate `System.Collections.Generic.IList<T>` as `MutableArray<T>` (by @MangelMaxime)
1314
* [JS/TS] Fix `ResizeArray` index getter/setter not throwing `IndexOutOfRangeException` when index is out of bounds (fix #3812) (by @MangelMaxime)
1415
* [Beam] Fix unused term warning in try/catch when exception variable is not referenced (by @dbrattli)

‎src/Fable.Compiler/CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
99

1010
### Fixed
1111

12+
* [JS/TS/Python] Fix record/struct types augmented with `static let` or `static member val` generating extra constructor parameters for each static field, causing constructor arguments to be assigned to wrong slots (by @MangelMaxime)
1213
* [TS] Annotate `System.Collections.Generic.IList<T>` as `MutableArray<T>` (by @MangelMaxime)
1314
* [JS/TS] Fix `ResizeArray` index getter/setter not throwing `IndexOutOfRangeException` when index is out of bounds (fix #3812) (by @MangelMaxime)
1415
* [Beam] Fix unused term warning in try/catch when exception variable is not referenced (by @dbrattli)

‎src/Fable.Transforms/Fable2Babel.fs‎

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -176,12 +176,15 @@ module Reflection =
176176

177177
let fields =
178178
ent.FSharpFields
179-
|> List.map (fun fi ->
180-
let fieldName = sanitizeMemberName fi.Name |> Expression.stringLiteral
179+
|> List.choose (fun fi ->
180+
if fi.IsStatic then
181+
None
182+
else
183+
let fieldName = sanitizeMemberName fi.Name |> Expression.stringLiteral
181184

182-
let typeInfo = transformTypeInfoFor Reflection com ctx r genMap fi.FieldType
185+
let typeInfo = transformTypeInfoFor Reflection com ctx r genMap fi.FieldType
183186

184-
Expression.arrayExpression ([| fieldName; typeInfo |])
187+
Some(Expression.arrayExpression ([| fieldName; typeInfo |]))
185188
)
186189
|> List.toArray
187190

@@ -3410,10 +3413,13 @@ but thanks to the optimisation done below we get
34103413

34113414
let getEntityFieldsAsIdents (ent: Fable.Entity) =
34123415
ent.FSharpFields
3413-
|> List.map (fun field ->
3414-
let name = sanitizeName field.Name
3415-
let typ = field.FieldType
3416-
{ makeTypedIdent typ name with IsMutable = field.IsMutable }
3416+
|> List.choose (fun field ->
3417+
if field.IsStatic then
3418+
None
3419+
else
3420+
let name = sanitizeName field.Name
3421+
let typ = field.FieldType
3422+
Some { makeTypedIdent typ name with IsMutable = field.IsMutable }
34173423
)
34183424

34193425
let declareClassWithParams
@@ -3997,6 +4003,7 @@ but thanks to the optimisation done below we get
39974003
yield callSuperAsStatement []
39984004
yield!
39994005
ent.FSharpFields
4006+
|> List.filter (fun field -> not field.IsStatic)
40004007
|> List.mapi (fun i field ->
40014008
let left = get None thisExpr field.Name
40024009

‎src/Fable.Transforms/FableTransforms.fs‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -745,7 +745,11 @@ module private Transforms =
745745
Emit({ emitInfo with CallInfo = { callInfo with Args = args } }, t, r)
746746
// Uncurry also values in setters or new record/union/tuple
747747
| Value(NewRecord(args, ent, genArgs), r) ->
748-
let args = com.GetEntity(ent).FSharpFields |> uncurryConsArgs args
748+
let args =
749+
com.GetEntity(ent).FSharpFields
750+
|> Seq.filter (fun f -> not f.IsStatic)
751+
|> uncurryConsArgs args
752+
749753
Value(NewRecord(args, ent, genArgs), r)
750754
| Value(NewAnonymousRecord(args, fieldNames, genArgs, isStruct), r) ->
751755
let args = uncurryArgs com false genArgs args

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

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -32,16 +32,19 @@ let private transformRecordReflectionInfo com ctx r (ent: Fable.Entity) generics
3232

3333
let fields, stmts =
3434
ent.FSharpFields
35-
|> Seq.map (fun fi ->
36-
let typeInfo, stmts = transformTypeInfo com ctx r genMap fi.FieldType
35+
|> Seq.choose (fun fi ->
36+
if fi.IsStatic then
37+
None
38+
else
39+
let typeInfo, stmts = transformTypeInfo com ctx r genMap fi.FieldType
3740

38-
let name =
39-
if Util.shouldUseRecordFieldNaming ent then
40-
fi.Name |> Naming.toRecordFieldSnakeCase |> Helpers.clean
41-
else
42-
fi.Name |> Naming.toSnakeCase |> Helpers.clean
41+
let name =
42+
if Util.shouldUseRecordFieldNaming ent then
43+
fi.Name |> Naming.toRecordFieldSnakeCase |> Helpers.clean
44+
else
45+
fi.Name |> Naming.toSnakeCase |> Helpers.clean
4346

44-
Expression.tuple [ Expression.stringConstant name; typeInfo ], stmts
47+
Some(Expression.tuple [ Expression.stringConstant name; typeInfo ], stmts)
4548
)
4649
|> Seq.toList
4750
|> Helpers.unzipArgs

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

Lines changed: 38 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3197,14 +3197,17 @@ let getEntityFieldsAsIdents (com: IPythonCompiler) (ent: Fable.Entity) =
31973197
Naming.toPythonNaming
31983198

31993199
ent.FSharpFields
3200-
|> Seq.map (fun field ->
3201-
let name =
3202-
(entityNamingConvention field.Name, Naming.NoMemberPart)
3203-
||> Naming.sanitizeIdent (fun _ -> false)
3200+
|> Seq.choose (fun field ->
3201+
if field.IsStatic then
3202+
None
3203+
else
3204+
let name =
3205+
(entityNamingConvention field.Name, Naming.NoMemberPart)
3206+
||> Naming.sanitizeIdent (fun _ -> false)
32043207

3205-
let typ = field.FieldType
3208+
let typ = field.FieldType
32063209

3207-
{ makeTypedIdent typ name with IsMutable = field.IsMutable }
3210+
Some { makeTypedIdent typ name with IsMutable = field.IsMutable }
32083211
)
32093212
|> Seq.toArray
32103213

@@ -3248,6 +3251,7 @@ let declareDataClassType
32483251
// lambda types in the type annotations to match.
32493252
let props =
32503253
ent.FSharpFields
3254+
|> List.filter (fun field -> not field.IsStatic)
32513255
|> List.mapi (fun i field ->
32523256
// Get the argument name from consArgs (preserves the Python naming convention)
32533257
let argName =
@@ -3343,8 +3347,33 @@ let declareDataClassType
33433347
returns = Expression.name "int"
33443348
)
33453349

3350+
// Generate ClassVar annotations for static fields so that Pyright recognizes
3351+
// the class-level attributes assigned by the static constructor (_cctor).
3352+
// With slots=True, unannotated class attributes are not allowed.
3353+
let staticFieldAnnotations =
3354+
ent.FSharpFields
3355+
|> List.choose (fun field ->
3356+
if
3357+
field.IsStatic
3358+
&& not (field.Name.StartsWith("init@", System.StringComparison.Ordinal))
3359+
then
3360+
let fieldName = field.Name.TrimEnd('@') |> Naming.toPythonNaming
3361+
let ta, _ = Annotation.typeAnnotation com ctx None field.FieldType
3362+
let classVar = com.GetImportExpr(ctx, "typing", "ClassVar")
3363+
let classVarAnnotation = Expression.subscript (classVar, ta)
3364+
Some(Statement.annAssign (Expression.name fieldName, annotation = classVarAnnotation))
3365+
else
3366+
None
3367+
)
3368+
33463369
let classBody =
3347-
let body = [ yield! props; yield! classMembers; yield hashMethod ]
3370+
let body =
3371+
[
3372+
yield! staticFieldAnnotations
3373+
yield! props
3374+
yield! classMembers
3375+
yield hashMethod
3376+
]
33483377

33493378
match body with
33503379
| [] -> [ Statement.ellipsis ]
@@ -4142,6 +4171,7 @@ let transformClassWithCompilerGeneratedConstructor
41424171
if classAttributes.Style = ClassStyle.Attributes then
41434172
yield!
41444173
ent.FSharpFields
4174+
|> List.filter (fun field -> not field.IsStatic)
41454175
|> List.collect (fun field ->
41464176
let fieldName = field.Name |> Naming.toPropertyNaming
41474177
let left = get com ctx None thisExpr fieldName false
@@ -4155,6 +4185,7 @@ let transformClassWithCompilerGeneratedConstructor
41554185
else
41564186
yield!
41574187
ent.FSharpFields
4188+
|> List.filter (fun field -> not field.IsStatic)
41584189
|> List.collecti (fun i field ->
41594190
let fieldName =
41604191
if shouldUseRecordFieldNaming ent then

‎tests/Js/Main/RecordTypeTests.fs‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,17 @@ type Time =
4545
type CarInterior = { Seats: int }
4646
type Car = { Interior: CarInterior }
4747

48+
// Issue #3923 - static let on a record shifts constructor arg indices
49+
type RecordWithStaticLet =
50+
{ id: int }
51+
static let _a = 2
52+
static member a = _a
53+
54+
// Issue #4091 - static member val on a record causes shared mutable state
55+
type RecordWithStaticMemberVal =
56+
{ xs: ResizeArray<int> }
57+
static member val empty = { xs = ResizeArray() }
58+
4859
let tests =
4960
testList "RecordTypes" [
5061

@@ -160,4 +171,18 @@ let tests =
160171
{| car with Interior.Seats = 5 |}
161172
equal 5 car2.Interior.Seats
162173

174+
// Issue #3923
175+
testCase "Record with static let has correct constructor" <| fun () ->
176+
let x = { id = 1 }
177+
equal 1 x.id
178+
equal 2 RecordWithStaticLet.a
179+
180+
// Issue #4091
181+
testCase "Record with static member val does not share state across instances" <| fun () ->
182+
let a = RecordWithStaticMemberVal.empty
183+
let b = RecordWithStaticMemberVal.empty
184+
a.xs.Add 1
185+
equal 1 a.xs.Count
186+
equal 1 b.xs.Count
187+
163188
]

‎tests/Python/TestRecordType.fs‎

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,17 @@ type Time =
4242
type CarInterior = { Seats: int }
4343
type Car = { Interior: CarInterior }
4444

45+
// Issue #3923 - static let on a record shifts constructor arg indices
46+
type RecordWithStaticLet =
47+
{ id: int }
48+
static let _a = 2
49+
static member a = _a
50+
51+
// Issue #4091 - static member val on a record causes shared mutable state
52+
type RecordWithStaticMemberVal =
53+
{ xs: ResizeArray<int> }
54+
static member val empty = { xs = ResizeArray() }
55+
4556
type RecordA =
4657
{ OptionalField : string option }
4758

@@ -199,4 +210,20 @@ type RecordWithProperty =
199210
[<Fact>]
200211
let ``test Record property access uses correct naming`` () =
201212
let x = { items = ["Hello"; "World"] }
202-
equal "Hello - World" x.fullName
213+
equal "Hello - World" x.fullName
214+
215+
// Issue #3923
216+
[<Fact>]
217+
let ``test Record with static let has correct constructor`` () =
218+
let x = { id = 1 }
219+
equal 1 x.id
220+
equal 2 RecordWithStaticLet.a
221+
222+
// Issue #4091
223+
[<Fact>]
224+
let ``test Record with static member val does not share state across instances`` () =
225+
let a = RecordWithStaticMemberVal.empty
226+
let b = RecordWithStaticMemberVal.empty
227+
a.xs.Add 1
228+
equal 1 a.xs.Count
229+
equal 1 b.xs.Count

0 commit comments

Comments
 (0)