Skip to content

Support validation of collection params with record types#7025

Open
bentsherman wants to merge 3 commits intomasterfrom
record-collection-params
Open

Support validation of collection params with record types#7025
bentsherman wants to merge 3 commits intomasterfrom
record-collection-params

Conversation

@bentsherman
Copy link
Copy Markdown
Member

This PR extends the support for loading a collection-type param from a CSV/JSON/YAML file to also validate and convert against a record type when used.

Notable changes:

  • Add ADR for workflow params (with appendix on runtime type analysis)
  • Compile params block as a hidden class to preserve runtime type information
  • Validate and convert collection elements against record type when specified
  • Add custom @Nullable annotation to model nullable record fields at runtime
  • Extend Types helper class to render parameterized types

Signed-off-by: Ben Sherman <bentshermann@gmail.com>
@bentsherman bentsherman requested a review from jorgee April 10, 2026 21:39
@bentsherman bentsherman requested a review from a team as a code owner April 10, 2026 21:39
@netlify
Copy link
Copy Markdown

netlify bot commented Apr 10, 2026

Deploy Preview for nextflow-docs-staging canceled.

Name Link
🔨 Latest commit 6438612
🔍 Latest deploy log https://app.netlify.com/projects/nextflow-docs-staging/deploys/69da88adb9842a0008e2c08f

Signed-off-by: Ben Sherman <bentshermann@gmail.com>
Signed-off-by: Ben Sherman <bentshermann@gmail.com>
@pditommaso pditommaso self-requested a review April 14, 2026 09:44
@jorgee
Copy link
Copy Markdown
Contributor

jorgee commented Apr 14, 2026

Suggestion from AI review:

1. Null-safety bug in TypeHelper.asType for nullable Path fields

In modules/nf-commons/src/main/nextflow/util/TypeHelper.groovy, if a record has a nullable Path field (e.g., fastq_2: Path?) and the input value is null, then value.toString() at line ~99 will throw a
NullPointerException. The validation loop in asRecordType skips nullable fields, but the conversion loop still calls asType(entry.value, ...) when entry.value is null. Needs a null guard at the top of asType.

2. No unit tests for TypeHelper conversion methods

getRawType, isCollectionType, asType, asCollectionType, and asRecordType have no dedicated unit tests — only tested indirectly through ParamsDslTest. Given these methods handle type reflection and conversion (prone
to subtle bugs), they should have their own tests covering edge cases: null values, nested parameterized types, unknown types, etc.

3. No test for @nullable annotation propagation

There's no test verifying that a ?-suffixed record field actually gets the @nullable annotation at runtime, or that asRecordType correctly allows missing values for nullable fields. Only the rejection path
(non-nullable missing field) is partially covered.


- Replacing `nextflow_schema.json` -- while the `params` block addresses many of the same needs, existing pipelines that use a JSON Schema should not be required to migrate. A native integration with `nextflow_schema.json` can be explored in the future.

- Supporting nested params -- the `params` block only supports a flat list of params. Nested params can still be used in the config, but they do not have first-class support at this time.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Record types are considered nested params, right? I mean , can I define a param as a Record type such as the following?:

params {
    meta : Metadata
}
record Metadata {
    id: String
    ....
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants