Make lists that represent repeated fields non-nullable in model class #33

jingtang10 · 2025-07-08T09:08:42Z

Fixes #28

Make lists in model classes non-nullable whilst maintaining nullability in the surrogate classes. Null values are needed for primitive types: https://www.hl7.org/fhir/R5/json.html#primitive
Add exceptions in test cases for example FHIR resources that are missing mandatory fields (e.g status)
Explicitly define the source directory src/commonMain/kotlin in order to omit build/generated which would generate "redeclaration" error
Minor refactoring of the SurrogateTypeSpecGenerator class, making functions in the file member functions of the class so there's no need to keep passing the valueset map
Fix variable naming in surrogate classes

Copilot

Pull Request Overview

This PR makes lists representing repeated fields non-nullable in model classes while maintaining nullability in surrogate classes. The changes address primitive type handling per FHIR JSON specification requirements and include necessary build configuration updates.

Key changes:

Changed list field types from List<T?>? to MutableList<T> with default empty list initialization
Updated build configuration to exclude generated code from compilation
Modified function visibility in surrogate classes from public to internal

Reviewed Changes

Copilot reviewed 112 out of 1219 changed files in this pull request and generated 8 comments.

File	Description
Multiple model files	Changed nullable lists to non-nullable MutableList with default empty initialization
Primitive type classes	Updated logic for element creation and factory methods to handle non-nullable lists
Various sealed interfaces	Changed companion function visibility from public to internal

Copilot · 2025-07-18T14:39:42Z

fhir-model/src/commonMain/kotlin/com/google/fhir/model/r4/Measure.kt

@@ -197,7 +197,7 @@ public data class Measure(
   *
   * Allows filtering of measures that are appropriate for use versus not.
   */
-  public var status: Enumeration<PublicationStatus>? = null,
+  public var status: Enumeration<PublicationStatus>,


Making the 'status' field non-nullable without providing a default value creates a breaking change. Constructors will now require this parameter, which could break existing code that relied on the nullable version.

Suggested change

public var status: Enumeration<PublicationStatus>,

public var status: Enumeration<PublicationStatus> = Enumeration(PublicationStatus.DRAFT),

Copilot · 2025-07-18T14:39:43Z

fhir-model/src/commonMain/kotlin/com/google/fhir/model/r4/Measure.kt

@@ -529,7 +529,7 @@ public data class Measure(
       * observation resources included in the evaluatedResources bundle for each patient. See the
       * MeasureReport resource or the Quality Reporting topic for more information.
       */
-      public var criteria: Expression? = null,
+      public var criteria: Expression,


Making the 'criteria' field non-nullable without providing a default value creates a breaking change. This field was previously nullable and now requires a value during construction.

Suggested change

public var criteria: Expression,

public var criteria: Expression?,

Copilot · 2025-07-18T14:39:43Z

fhir-model/src/commonMain/kotlin/com/google/fhir/model/r4/MarketingStatus.kt

  /**
   * The country in which the marketing authorisation has been granted shall be specified It should
   * be specified using the ISO 3166 ‑ 1 alpha-2 code elements.
   */
-  public var country: CodeableConcept? = null,
+  public var country: CodeableConcept,


Making the 'country' field non-nullable without providing a default value creates a breaking change. Existing code that relied on the nullable version will need to be updated.

Suggested change

public var country: CodeableConcept,

public var country: CodeableConcept = CodeableConcept(),

Copilot · 2025-07-18T14:39:43Z

fhir-model/src/commonMain/kotlin/com/google/fhir/model/r4/Markdown.kt

      return Element(id, extension)
    }
    return null
  }

  public companion object {
    public fun of(`value`: kotlin.String?, element: Element?): Markdown? =
-      if (value == null && element == null) {
-        null
+      if (value != null || element?.id != null || element?.extension?.isEmpty() == false) {


The condition logic appears inverted. The condition element?.extension?.isEmpty() == false should likely be element?.extension?.isNotEmpty() == true or !element?.extension.isNullOrEmpty() for better readability and consistency.

Suggested change

if (value != null || element?.id != null || element?.extension?.isEmpty() == false) {

if (value != null || element?.id != null || !element?.extension.isNullOrEmpty()) {

Copilot · 2025-07-18T14:39:44Z

fhir-model/src/commonMain/kotlin/com/google/fhir/model/r4/Integer.kt

      return Element(id, extension)
    }
    return null
  }

  public companion object {
    public fun of(`value`: Int?, element: Element?): Integer? =
-      if (value == null && element == null) {
-        null
+      if (value != null || element?.id != null || element?.extension?.isEmpty() == false) {


The condition logic appears inverted. The condition element?.extension?.isEmpty() == false should likely be element?.extension?.isNotEmpty() == true for better readability and consistency.

Suggested change

if (value != null || element?.id != null || element?.extension?.isEmpty() == false) {

if (value != null || element?.id != null || element?.extension?.isNotEmpty() == true) {

Copilot · 2025-07-18T14:39:44Z

fhir-model/src/commonMain/kotlin/com/google/fhir/model/r4/List.kt

  /**
   * Indicates the current state of this list.
   *
   * This element is labeled as a modifier because the status contains codes that mark the resource
   * as not currently valid.
   */
-  public var status: Enumeration<ListStatus>? = null,
+  public var status: Enumeration<ListStatus>,


Making the 'status' field non-nullable without providing a default value creates a breaking change. This mandatory field now requires a value during construction.

Suggested change

public var status: Enumeration<ListStatus>,

public var status: Enumeration<ListStatus> = Enumeration(ListStatus.DRAFT),

Copilot · 2025-07-18T14:39:44Z

fhir-model/src/commonMain/kotlin/com/google/fhir/model/r4/HealthcareService.kt

    /** The reason that can be presented to the user as to why this time is not available. */
-    public var description: String? = null,
+    public var description: String,


Making the 'description' field non-nullable without providing a default value creates a breaking change. This field was previously nullable and now requires a value during construction.

Suggested change

public var description: String,

public var description: String = "",

Copilot · 2025-07-18T14:39:45Z

fhir-model/src/commonMain/kotlin/com/google/fhir/model/r4/Expression.kt

@@ -59,7 +59,7 @@ public data class Expression(
   */
  public var name: Id? = null,
  /** The media type of the language for the expression. */
-  public var language: Enumeration<ExpressionLanguage>? = null,
+  public var language: Enumeration<ExpressionLanguage>,


Making the 'language' field non-nullable without providing a default value creates a breaking change. This field was previously nullable and now requires a value during construction.

Suggested change

public var language: Enumeration<ExpressionLanguage>,

public var language: Enumeration<ExpressionLanguage> = Enumeration(ExpressionLanguage.Text_Cql),

fhir-codegen/gradle-plugin/src/main/kotlin/com/google/fhir/codegen/schema/MoreElements.kt

...le-plugin/src/main/kotlin/com/google/fhir/codegen/primitives/EnumerationFileSpecGenerator.kt

jingtang10 force-pushed the nullability branch from da87ab6 to 4694119 Compare July 14, 2025 15:54

jingtang10 changed the title ~~Nullability~~ Make repeated fields lists in model class Jul 14, 2025

jingtang10 changed the title ~~Make repeated fields lists in model class~~ Make lists that represent repeated fields non-nullable in model class Jul 14, 2025

jingtang10 force-pushed the nullability branch 2 times, most recently from 28dbeff to 99aabda Compare July 18, 2025 09:45

Make lists that represent repeated fields non-nullable in model class

bef9795

jingtang10 force-pushed the nullability branch from 99aabda to bef9795 Compare July 18, 2025 14:37

jingtang10 marked this pull request as ready for review July 18, 2025 14:37

jingtang10 requested review from ellykits and Copilot and removed request for ellykits July 18, 2025 14:38

Copilot AI reviewed Jul 18, 2025

View reviewed changes

ellykits requested changes Jul 22, 2025

View reviewed changes

fhir-codegen/gradle-plugin/src/main/kotlin/com/google/fhir/codegen/schema/MoreElements.kt Show resolved Hide resolved

...le-plugin/src/main/kotlin/com/google/fhir/codegen/primitives/EnumerationFileSpecGenerator.kt Show resolved Hide resolved

jingtang10 mentioned this pull request Jul 22, 2025

Make model classes immutable #27

Open

jingtang10 requested a review from ellykits July 22, 2025 12:54

ellykits approved these changes Jul 22, 2025

View reviewed changes

jingtang10 merged commit f1bee4e into google:main Jul 22, 2025
1 of 5 checks passed

jingtang10 deleted the nullability branch July 22, 2025 15:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Make lists that represent repeated fields non-nullable in model class #33

Make lists that represent repeated fields non-nullable in model class #33

Uh oh!

jingtang10 commented Jul 8, 2025 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jul 18, 2025

Uh oh!

Copilot AI Jul 18, 2025

Uh oh!

Copilot AI Jul 18, 2025

Uh oh!

Copilot AI Jul 18, 2025

Uh oh!

Copilot AI Jul 18, 2025

Uh oh!

Copilot AI Jul 18, 2025

Uh oh!

Copilot AI Jul 18, 2025

Uh oh!

Copilot AI Jul 18, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

	public var status: Enumeration<PublicationStatus>,
	public var status: Enumeration<PublicationStatus> = Enumeration(PublicationStatus.DRAFT),

	public var criteria: Expression,
	public var criteria: Expression?,

	public var country: CodeableConcept,
	public var country: CodeableConcept = CodeableConcept(),

	if (value != null \|\| element?.id != null \|\| element?.extension?.isEmpty() == false) {
	if (value != null \|\| element?.id != null \|\| !element?.extension.isNullOrEmpty()) {

	public var status: Enumeration<ListStatus>,
	public var status: Enumeration<ListStatus> = Enumeration(ListStatus.DRAFT),

	public var description: String,
	public var description: String = "",

	public var language: Enumeration<ExpressionLanguage>,
	public var language: Enumeration<ExpressionLanguage> = Enumeration(ExpressionLanguage.Text_Cql),

Make lists that represent repeated fields non-nullable in model class #33

Make lists that represent repeated fields non-nullable in model class #33

Uh oh!

Conversation

jingtang10 commented Jul 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Jul 18, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 18, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 18, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 18, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 18, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 18, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 18, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 18, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jingtang10 commented Jul 8, 2025 •

edited

Loading