[taiyangc]

Fixes #6759.

gnd codegen generates nullable accessors for nullable fields whose AssemblyScript type is a primitive: Int, Boolean, Int8 and Timestamp, and references to entities with id: Int8!, which map to i64. The setter then starts with if (!value) { this.unset(...) }, so setting such a field to 0 or false unsets it, and the store holds null. graph-cli stores the value.

graph-cli takes an accessor's nullability from the AssemblyScript type (returnType instanceof tsCodegen.NullableType), and _typeFromGraphQl never makes a primitive nullable. gnd's type_from_field applies the same rule, but generate_field_getter and generate_field_setter used field.is_nullable. This PR takes nullability from the type expression in both, so for these fields gnd now generates what graph-cli does: the getter returns the type's default and the setter always stores the value. Nullable fields whose AssemblyScript type is not a primitive keep the nullable accessors.

For Int, Int8, Timestamp and the Int8-id references, the fix is the setter change: on master the getter's return null already compiles to 0 for i32 and i64. For Boolean the getter change matters as well: on master a mapping that reads a nullable Boolean doesn't compile under asc 0.19.23 (ERROR AS200: Conversion from type 'usize' to 'bool' requires an explicit cast at the getter's return null).

Testing

• test_nullable_primitive_fields in gnd/src/codegen/schema.rs covers Int, Boolean, Int8, Timestamp and a nullable reference to an Int8-id entity. A nullable String is the control and keeps the nullable accessors.
• I didn't build the whole workspace locally. Instead I compiled gnd's codegen/schema.rs, codegen/types.rs, codegen/typescript.rs and shared/sanitize.rs in a standalone crate (graphql-tools 0.5.1, Rust 1.94.0 from rust-toolchain.toml) and ran their unit tests:
  • with this change, all 18 schema.rs tests pass, and so do the types.rs, typescript.rs and sanitize.rs tests;
  • on master with only the new test added, the 17 existing tests pass and the new one fails at its first field, side, whose getter returns null;
  • with only the setter reverted, the new test fails at the setter (this.unset('side')).
• cargo clippy --all-targets with RUSTFLAGS="-D warnings" and the workspace's clippy lint settings is clean on those files.
• I haven't run cargo test -p gnd or the codegen_verification golden tests. None of their fixture schemas has a field of type Int, Boolean, Int8 or Timestamp, or an entity with an Int8 id, so their expected output doesn't change.

[dimitrovmaksim]

Thanks for the clear report and the fix.

A few things before merge:

1. Scope. The change also covers nullable references to entities with id: Int8!. type_from_field maps them to i64, so they get the non-nullable accessors too, as in graph-cli.
   That means "Every other field's accessors are unchanged" isn't accurate. Please mention this case in the description and commit message. The inline comment on the test adds it to
   test_nullable_primitive_fields.
2. Boolean build failure. On master, a mapping that reads a nullable Boolean doesn't compile under asc 0.19.23: ERROR AS200: Conversion from type 'usize' to 'bool' requires an explicit cast at the getter's return null. For i32/i64 that return null compiles to 0, so the getter change only matters at runtime for Boolean. For Int, Int8 and
   Timestamp the fix is the setter change. Worth a sentence in the description.
3. Commit trailers. Could you drop the Co-Authored-By: and Claude-Session: trailers from the commit message? I'd like to keep AI attribution out of the commit history, and reviewers can't open the session link

[dimitrovmaksim]

nullable is now true only when param_type is Nullable, so the other => other.to_string() arm can't be reached. It only existed for the old case of a primitive with a nullable GraphQL field. Binding the pattern directly removes the dead arm and the duplicated comment:

Suggested change

	let set_code = if let TypeExpr::Nullable(n) = &param_type {
	let inner_type = n.inner.to_string();

[dimitrovmaksim]

Nit: one line is enough. With the setter change below, this is the only copy.

Suggested change

	// graph-cli generates it.
	// AS primitives can't be nullable, so take nullability from the AS type.

[dimitrovmaksim]

This misses nullable references to Int8-id entities, which this PR also changes: they map to i64, so they now get the non-nullable setter (assigning id 0 stores it instead of unsetting the field). The getter changes only in source; master's return null already compiled to 0. Here's a version that covers them and trims the comment:

-        // AssemblyScript primitives cannot be nullable, so a nullable field of a
-        // primitive type gets the non-nullable accessors, as in graph-cli: the
-        // getter returns the type's default and the setter always stores the
-        // value. A nullable setter would unset the field when given 0 or false.
+        // Nullable primitives, incl. Int8-id entity refs, get non-nullable accessors.
         let schema = r#"
@@
                 at: Timestamp
+                counter: Counter
                 name: String
             }
+
+            type Counter @entity(immutable: true) {
+                id: Int8!
+            }
         "#;
@@
-        assert_eq!(classes.len(), 1);
+        assert_eq!(classes.len(), 2);
@@
-        for (field, default) in [("side", "0"), ("flag", "false"), ("big", "0"), ("at", "0")] {
+        for (field, default) in [
+            ("side", "0"),
+            ("flag", "false"),
+            ("big", "0"),
+            ("at", "0"),
+            ("counter", "0"),
+        ] {

[taiyangc]

Thanks for the review. All of it is in the amended commit:

• Applied all three inline suggestions: the one-line comment in the getter, if let TypeExpr::Nullable(n) in the setter, and the test extended with an Int8-id entity reference.
• The description and the commit message now cover nullable references to entities with id: Int8!. The description also notes that for Boolean the getter change fixes the compile error on master, while for the other types the setter change is the fix.
• Removed the commit trailers.