diff --git a/cwl_utils/parser/cwl_v1_2_utils.py b/cwl_utils/parser/cwl_v1_2_utils.py index e40f5877..2eb45685 100644 --- a/cwl_utils/parser/cwl_v1_2_utils.py +++ b/cwl_utils/parser/cwl_v1_2_utils.py @@ -210,6 +210,7 @@ def check_all_types( extra_message = ( "pickValue is %s" % sink.pickValue if sink.pickValue is not None else None ) + sink_type = type_dict[sink.id] match sink: case cwl.WorkflowOutputParameter(): sourceName = "outputSource" @@ -220,7 +221,7 @@ def check_all_types( case _: continue if sourceField is not None: - if isinstance(sourceField, MutableSequence): + if isinstance(sourceField, MutableSequence) and len(sourceField) > 1: linkMerge: str | None = sink.linkMerge or ( "merge_nested" if len(sourceField) > 1 else None ) @@ -231,23 +232,34 @@ def check_all_types( srcs_of_sink += [src_dict[parm_id]] if ( _is_conditional_step(param_to_step, parm_id) - and sink.pickValue is not None + and sink.pickValue is None ): - validation["warning"].append( - SrcSink( - src_dict[parm_id], - sink, - linkMerge, - message="Source is from conditional step, but pickValue is not used", + src_typ = aslist(type_dict[src_dict[parm_id].id]) + if "null" not in src_typ: + src_typ = ["null"] + cast(list[Any], src_typ) + if ( + not isinstance(sink_type, MutableSequence) + or "null" not in sink_type + ): + validation["warning"].append( + SrcSink( + src_dict[parm_id], + sink, + linkMerge, + message="Source is from conditional step, but pickValue is not used", + ) ) - ) + type_dict[src_dict[parm_id].id] = src_typ if _is_all_output_method_loop_step(param_to_step, parm_id): src_typ = type_dict[src_dict[parm_id].id] type_dict[src_dict[parm_id].id] = cwl.ArraySchema( items=src_typ, type_="array" ) else: - parm_id = cast(str, sourceField) + if isinstance(sourceField, MutableSequence): + parm_id = cast(str, sourceField[0]) + else: + parm_id = cast(str, sourceField) if parm_id not in src_dict: raise SourceLine(sink, sourceName, ValidationException).makeError( f"{sourceName} not found: {parm_id}" @@ -289,7 +301,7 @@ def check_all_types( for src in srcs_of_sink: check_result = check_types( type_dict[cast(str, src.id)], - type_dict[sink.id], + sink_type, linkMerge, getattr(sink, "valueFrom", None), ) @@ -313,19 +325,24 @@ def check_types( """ if valueFrom is not None: return "pass" - if linkMerge is None: - if can_assign_src_to_sink(srctype, sinktype, strict=True): - return "pass" - if can_assign_src_to_sink(srctype, sinktype, strict=False): - return "warning" - return "exception" - if linkMerge == "merge_nested": - return check_types( - cwl.ArraySchema(items=srctype, type_="array"), sinktype, None, None - ) - if linkMerge == "merge_flattened": - return check_types(merge_flatten_type(srctype), sinktype, None, None) - raise ValidationException(f"Invalid value {linkMerge} for linkMerge field.") + match linkMerge: + case None: + if can_assign_src_to_sink(srctype, sinktype, strict=True): + return "pass" + if can_assign_src_to_sink(srctype, sinktype, strict=False): + return "warning" + return "exception" + case "merge_nested": + return check_types( + cwl.ArraySchema(items=srctype, type_="array"), + sinktype, + None, + None, + ) + case "merge_flattened": + return check_types(merge_flatten_type(srctype), sinktype, None, None) + case _: + raise ValidationException(f"Invalid value {linkMerge} for linkMerge field.") def content_limit_respected_read_bytes(f: IO[bytes]) -> bytes: diff --git a/cwl_utils/testdata/checker_wf/no-warning-wf.cwl b/cwl_utils/testdata/checker_wf/no-warning-wf.cwl new file mode 100755 index 00000000..7b1e7a25 --- /dev/null +++ b/cwl_utils/testdata/checker_wf/no-warning-wf.cwl @@ -0,0 +1,44 @@ +#!/usr/bin/env cwl-runner +class: Workflow +cwlVersion: v1.2 +requirements: + ScatterFeatureRequirement: {} + MultipleInputFeatureRequirement: {} + StepInputExpressionRequirement: {} +inputs: + letters0: + type: [string, int] + default: "a0" + letters1: + type: string[] + default: ["a1", "b1"] + letters2: + type: [string, int] + default: "a2" + letters3: + type: string[] + default: ["a3", "b3"] + letters4: + type: string + default: "a4" + letters5: + type: string[] + default: ["a5", "b5", "c5"] + exec: + type: bool + default: False + +outputs: + single: + type: File? + outputSource: + - step1/txt + +steps: + - id: step1 + run: echo.cwl + in: + exec: exec + out: [txt] + when: $(inputs.exec) + diff --git a/cwl_utils/testdata/checker_wf/warning-wf.cwl b/cwl_utils/testdata/checker_wf/warning-wf.cwl new file mode 100755 index 00000000..1afb3e26 --- /dev/null +++ b/cwl_utils/testdata/checker_wf/warning-wf.cwl @@ -0,0 +1,44 @@ +#!/usr/bin/env cwl-runner +class: Workflow +cwlVersion: v1.2 +requirements: + ScatterFeatureRequirement: {} + MultipleInputFeatureRequirement: {} + StepInputExpressionRequirement: {} +inputs: + letters0: + type: [string, int] + default: "a0" + letters1: + type: string[] + default: ["a1", "b1"] + letters2: + type: [string, int] + default: "a2" + letters3: + type: string[] + default: ["a3", "b3"] + letters4: + type: string + default: "a4" + letters5: + type: string[] + default: ["a5", "b5", "c5"] + exec: + type: bool + default: False + +outputs: + single: + type: File + outputSource: + - step1/txt + +steps: + - id: step1 + run: echo.cwl + in: + exec: exec + out: [txt] + when: $(inputs.exec) + diff --git a/cwl_utils/testdata/checker_wf/warning-wf2.cwl b/cwl_utils/testdata/checker_wf/warning-wf2.cwl new file mode 100755 index 00000000..10ef7f3a --- /dev/null +++ b/cwl_utils/testdata/checker_wf/warning-wf2.cwl @@ -0,0 +1,43 @@ +#!/usr/bin/env cwl-runner +class: Workflow +cwlVersion: v1.2 +requirements: + ScatterFeatureRequirement: {} + MultipleInputFeatureRequirement: {} + StepInputExpressionRequirement: {} +inputs: + letters0: + type: [string, int] + default: "a0" + letters1: + type: string[] + default: ["a1", "b1"] + letters2: + type: [string, int] + default: "a2" + letters3: + type: string[] + default: ["a3", "b3"] + letters4: + type: string + default: "a4" + letters5: + type: string[] + default: ["a5", "b5", "c5"] + exec: + type: bool + default: False + +outputs: + single_a: + type: File + outputSource: step1/txt + +steps: + - id: step1 + run: echo.cwl + in: + exec: exec + out: [txt] + when: $(inputs.exec) + diff --git a/cwl_utils/testdata/checker_wf/warning-wf3.cwl b/cwl_utils/testdata/checker_wf/warning-wf3.cwl new file mode 100755 index 00000000..c2132166 --- /dev/null +++ b/cwl_utils/testdata/checker_wf/warning-wf3.cwl @@ -0,0 +1,50 @@ +#!/usr/bin/env cwl-runner +class: Workflow +cwlVersion: v1.2 +requirements: + ScatterFeatureRequirement: {} + MultipleInputFeatureRequirement: {} + StepInputExpressionRequirement: {} +inputs: + letters0: + type: [string, int] + default: "a0" + letters1: + type: string[] + default: ["a1", "b1"] + letters2: + type: [string, int] + default: "a2" + letters3: + type: string[] + default: ["a3", "b3"] + letters4: + type: string + default: "a4" + letters5: + type: string[] + default: ["a5", "b5", "c5"] + exec: + type: bool + default: False + +outputs: + pair: + type: File[] + outputSource: + - step1/txt + - step2/txt + +steps: + - id: step1 + run: echo.cwl + in: + exec: exec + out: [txt] + when: $(inputs.exec) + - id: step2 + run: echo.cwl + in: [] + out: [txt] + when: $(inputs.exec) + diff --git a/cwl_utils/tests/test_parser_utils.py b/cwl_utils/tests/test_parser_utils.py index d4d24fa7..44c2d023 100644 --- a/cwl_utils/tests/test_parser_utils.py +++ b/cwl_utils/tests/test_parser_utils.py @@ -1,11 +1,13 @@ # SPDX-License-Identifier: Apache-2.0 """Test the CWL parsers utility functions.""" +import logging +import re import tempfile from collections.abc import MutableSequence from typing import cast import pytest -from pytest import raises +from pytest import raises, LogCaptureFixture from schema_salad.exceptions import ValidationException import cwl_utils.parser.cwl_v1_0 @@ -71,6 +73,39 @@ def test_static_checker_fail(cwlVersion: str) -> None: cwl_utils.parser.utils.static_checker(cwl_obj7) +def test_static_checker_warning(caplog: LogCaptureFixture) -> None: + uri1 = get_path("testdata/checker_wf/warning-wf.cwl").as_uri() + cwl_obj1 = load_document_by_uri(uri1) + with caplog.at_level(logging.WARNING): + cwl_utils.parser.utils.static_checker(cwl_obj1) + assert "Source is from conditional step and may produce `null`" in caplog.text + caplog.clear() + + uri2 = get_path("testdata/checker_wf/no-warning-wf.cwl").as_uri() + cwl_obj2 = load_document_by_uri(uri2) + with caplog.at_level(logging.WARNING): + cwl_utils.parser.utils.static_checker(cwl_obj2) + assert not caplog.text + caplog.clear() + + uri3 = get_path("testdata/checker_wf/warning-wf2.cwl").as_uri() + cwl_obj3 = load_document_by_uri(uri3) + with caplog.at_level(logging.WARNING): + cwl_utils.parser.utils.static_checker(cwl_obj3) + assert "Source is from conditional step and may produce `null`" in caplog.text + caplog.clear() + + uri4 = get_path("testdata/checker_wf/warning-wf3.cwl").as_uri() + cwl_obj4 = load_document_by_uri(uri4) + with caplog.at_level(logging.WARNING): + cwl_utils.parser.utils.static_checker(cwl_obj4) + assert re.search( + 'with sink \'pair\' of type {"name":.* "items": "File", "type": "array"}', + caplog.text, + ) + assert "Source is from conditional step, but pickValue is not used" in caplog.text + + @pytest.mark.parametrize("cwlVersion", ["v1_0", "v1_1", "v1_2"]) def test_static_checker_success(cwlVersion: str) -> None: """Confirm that static type checker correctly validates workflows."""