Skip to content

Commit fbec276

Browse files
coolreader18bfops
andauthored
[TS] Fix access before initialization in codegen (#4709)
# Description of Changes Fixes #4584. Now we check, fully recursively, whether compound `AlgebraicTypeUse`s contain a ref. # Expected complexity level and risk 1 # Testing - [x] Added a test to ensure correct codegen --------- Co-authored-by: Zeke Foppa <196249+bfops@users.noreply.github.com>
1 parent 44ef7f4 commit fbec276

File tree

7 files changed

+43
-18
lines changed

7 files changed

+43
-18
lines changed

crates/codegen/src/typescript.rs

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -733,6 +733,16 @@ fn write_object_type_builder_fields(
733733
Ok(())
734734
}
735735

736+
/// Returns whether `ty` recursively contains an `AlgebraicTypeUse::Ref`
737+
fn type_contains_ref(ty: &AlgebraicTypeUse) -> bool {
738+
match ty {
739+
AlgebraicTypeUse::Ref(_) => true,
740+
AlgebraicTypeUse::Option(inner) | AlgebraicTypeUse::Array(inner) => type_contains_ref(inner),
741+
AlgebraicTypeUse::Result { ok_ty, err_ty } => type_contains_ref(ok_ty) || type_contains_ref(err_ty),
742+
_ => false,
743+
}
744+
}
745+
736746
fn write_type_builder_field(
737747
module: &ModuleDef,
738748
out: &mut Indenter,
@@ -741,17 +751,8 @@ fn write_type_builder_field(
741751
ty: &AlgebraicTypeUse,
742752
is_primary_key: bool,
743753
) -> fmt::Result {
744-
// Do we need a getter? (Option/Array only if their inner is a Ref)
745-
let needs_getter = match ty {
746-
AlgebraicTypeUse::Ref(_) => true,
747-
AlgebraicTypeUse::Option(inner) | AlgebraicTypeUse::Array(inner) => {
748-
matches!(inner.as_ref(), AlgebraicTypeUse::Ref(_))
749-
}
750-
AlgebraicTypeUse::Result { ok_ty, err_ty } => {
751-
matches!(ok_ty.as_ref(), AlgebraicTypeUse::Ref(_)) || matches!(err_ty.as_ref(), AlgebraicTypeUse::Ref(_))
752-
}
753-
_ => false,
754-
};
754+
// If the type contains a ref, we need to use a getter to prevent access-before-initialization.
755+
let needs_getter = type_contains_ref(ty);
755756

756757
if needs_getter {
757758
writeln!(out, "get {name}() {{");

crates/codegen/tests/snapshots/codegen__codegen_csharp.snap

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2036,10 +2036,12 @@ namespace SpacetimeDB
20362036
public sealed class TestDCols
20372037
{
20382038
public global::SpacetimeDB.Col<TestD, NamespaceTestC> TestC { get; }
2039+
public global::SpacetimeDB.Col<TestD, System.Collections.Generic.List<NamespaceTestC>> TestCNested { get; }
20392040

20402041
public TestDCols(string tableName)
20412042
{
20422043
TestC = new global::SpacetimeDB.Col<TestD, NamespaceTestC>(tableName, "test_c");
2044+
TestCNested = new global::SpacetimeDB.Col<TestD, System.Collections.Generic.List<NamespaceTestC>>(tableName, "test_c_nested");
20432045
}
20442046
}
20452047

@@ -2566,10 +2568,16 @@ namespace SpacetimeDB
25662568
{
25672569
[DataMember(Name = "test_c")]
25682570
public NamespaceTestC? TestC;
2571+
[DataMember(Name = "test_c_nested")]
2572+
public System.Collections.Generic.List<NamespaceTestC>? TestCNested;
25692573

2570-
public TestD(NamespaceTestC? TestC)
2574+
public TestD(
2575+
NamespaceTestC? TestC,
2576+
System.Collections.Generic.List<NamespaceTestC>? TestCNested
2577+
)
25712578
{
25722579
this.TestC = TestC;
2580+
this.TestCNested = TestCNested;
25732581
}
25742582

25752583
public TestD()

crates/codegen/tests/snapshots/codegen__codegen_rust.snap

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3593,6 +3593,7 @@ use super::namespace_test_c_type::NamespaceTestC;
35933593
#[sats(crate = __lib)]
35943594
pub struct TestD {
35953595
pub test_c: Option::<NamespaceTestC>,
3596+
pub test_c_nested: Option::<Vec::<NamespaceTestC>>,
35963597
}
35973598

35983599

@@ -3606,13 +3607,15 @@ impl __sdk::InModule for TestD {
36063607
/// Provides typed access to columns for query building.
36073608
pub struct TestDCols {
36083609
pub test_c: __sdk::__query_builder::Col<TestD, Option::<NamespaceTestC>>,
3610+
pub test_c_nested: __sdk::__query_builder::Col<TestD, Option::<Vec::<NamespaceTestC>>>,
36093611
}
36103612

36113613
impl __sdk::__query_builder::HasCols for TestD {
36123614
type Cols = TestDCols;
36133615
fn cols(table_name: &'static str) -> Self::Cols {
36143616
TestDCols {
36153617
test_c: __sdk::__query_builder::Col::new(table_name, "test_c"),
3618+
test_c_nested: __sdk::__query_builder::Col::new(table_name, "test_c_nested"),
36163619

36173620
}
36183621
}

crates/codegen/tests/snapshots/codegen__codegen_typescript.snap

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -548,6 +548,9 @@ export default __t.row({
548548
get testC() {
549549
return __t.option(NamespaceTestC).name("test_c");
550550
},
551+
get testCNested() {
552+
return __t.option(__t.array(NamespaceTestC)).name("test_c_nested");
553+
},
551554
});
552555
'''
553556
"test_f_table.ts" = '''
@@ -701,6 +704,9 @@ export const TestD = __t.object("TestD", {
701704
get testC() {
702705
return __t.option(NamespaceTestC);
703706
},
707+
get testCNested() {
708+
return __t.option(__t.array(NamespaceTestC));
709+
},
704710
});
705711
export type TestD = __Infer<typeof TestD>;
706712

modules/module-test-cs/Lib.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ public partial struct TestD
5353
{
5454
// In Rust this was an Option<TestC>; in C# we use a nullable enum.
5555
public TestC? test_c;
56+
public TestC[]? test_c_nested;
5657
}
5758

5859
[Table(Accessor = "test_e")]

modules/module-test-ts/src/index.ts

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,7 @@ type TestA = Infer<typeof testA>;
9292
// NOTE: If your Option default requires wrapping, adjust to your bindings’ Option encoding.
9393
const testDRow = {
9494
test_c: t.option(testC).default(DEFAULT_TEST_C as unknown as any),
95+
test_c_nested: t.option(t.array(testC)),
9596
};
9697
type TestD = InferTypeOfRow<typeof testDRow>;
9798

@@ -152,16 +153,16 @@ const spacetimedb = schema({
152153
person: table(
153154
{
154155
public: true,
155-
indexes: [{ accessor: "age", algorithm: 'btree', columns: ['age'] }],
156+
indexes: [{ accessor: 'age', algorithm: 'btree', columns: ['age'] }],
156157
},
157158
personRow
158159
),
159160

160161
// test_a with index foo on x
161162
testATable: table(
162163
{
163-
name: "test_a",
164-
indexes: [{ accessor: "foo", algorithm: 'btree', columns: ['x'] }],
164+
name: 'test_a',
165+
indexes: [{ accessor: 'foo', algorithm: 'btree', columns: ['x'] }],
165166
},
166167
testA
167168
),
@@ -174,7 +175,7 @@ const spacetimedb = schema({
174175
{
175176
name: 'test_e',
176177
public: false,
177-
indexes: [{ accessor:"name", algorithm: 'btree', columns: ['name'] }],
178+
indexes: [{ accessor: 'name', algorithm: 'btree', columns: ['name'] }],
178179
},
179180
testERow
180181
),
@@ -194,7 +195,11 @@ const spacetimedb = schema({
194195
name: 'points',
195196
public: false,
196197
indexes: [
197-
{ accessor: 'multi_column_index', algorithm: 'btree', columns: ['x', 'y'] },
198+
{
199+
accessor: 'multi_column_index',
200+
algorithm: 'btree',
201+
columns: ['x', 'y'],
202+
},
198203
],
199204
},
200205
pointsRow
@@ -207,7 +212,7 @@ const spacetimedb = schema({
207212
repeatingTestArg: table(
208213
{
209214
name: 'repeating_test_arg',
210-
scheduled: (): any => repeatingTest
215+
scheduled: (): any => repeatingTest,
211216
},
212217
repeatingTestArg
213218
),

modules/module-test/src/lib.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ const DEFAULT_TEST_C: TestC = TestC::Foo;
6767
pub struct TestD {
6868
#[default(Some(DEFAULT_TEST_C))]
6969
test_c: Option<TestC>,
70+
test_c_nested: Option<Vec<TestC>>,
7071
}
7172

7273
// uses internal apis that should not be used by user code

0 commit comments

Comments
 (0)