diff --git a/lib/galaxy/tool_util/linters/inputs.py b/lib/galaxy/tool_util/linters/inputs.py index d3426bb78596..0540856be567 100644 --- a/lib/galaxy/tool_util/linters/inputs.py +++ b/lib/galaxy/tool_util/linters/inputs.py @@ -558,8 +558,11 @@ def lint_inputs(tool_source: "ToolSource", lint_ctx: "LintContext"): ) -def lint_repeats(tool_xml, lint_ctx): +def lint_repeats(tool_source: "ToolSource", lint_ctx): """Lint repeat blocks in tool inputs.""" + tool_xml = getattr(tool_source, "xml_tree", None) + if tool_xml is None: + return repeats = tool_xml.findall("./inputs//repeat") for repeat in repeats: if "name" not in repeat.attrib: diff --git a/lib/galaxy/tool_util/linters/outputs.py b/lib/galaxy/tool_util/linters/outputs.py index e3de862b8836..e034b0fe606d 100644 --- a/lib/galaxy/tool_util/linters/outputs.py +++ b/lib/galaxy/tool_util/linters/outputs.py @@ -1,4 +1,8 @@ """This module contains a linting functions for tool outputs.""" +from typing import TYPE_CHECKING + +from packaging.version import Version + from galaxy.util import ( etree, string_as_bool, @@ -6,9 +10,18 @@ from ._util import is_valid_cheetah_placeholder from ..parser.output_collection_def import NAMED_PATTERNS +if TYPE_CHECKING: + from galaxy.tool_util.lint import LintContext + from galaxy.tool_util.parser import ToolSource -def lint_output(tool_xml, lint_ctx): + +def lint_output(tool_source: "ToolSource", lint_ctx: "LintContext"): """Check output elements, ensure there is at least one and check attributes.""" + tool_xml = getattr(tool_source, "xml_tree", None) + if tool_xml is None: + return + profile = tool_source.parse_profile() + outputs = tool_xml.findall("./outputs") # determine node to report for general problems with outputs tool_node = tool_xml.find("./outputs") @@ -56,7 +69,7 @@ def lint_output(tool_xml, lint_ctx): labels.add(label) format_set = False - if __check_format(output, lint_ctx): + if __check_format(output, lint_ctx, profile): format_set = True if output.tag == "data": if "auto_format" in output.attrib and output.attrib["auto_format"]: @@ -70,7 +83,7 @@ def lint_output(tool_xml, lint_ctx): for sub in output: if __check_pattern(sub): format_set = True - elif __check_format(sub, lint_ctx, allow_ext=True): + elif __check_format(sub, lint_ctx, profile, allow_ext=True): format_set = True if not format_set: @@ -83,7 +96,7 @@ def lint_output(tool_xml, lint_ctx): lint_ctx.info(f"{num_outputs} outputs found.", node=outputs[0]) -def __check_format(node, lint_ctx, allow_ext=False): +def __check_format(node, lint_ctx, profile: str, allow_ext=False): """ check if format/ext/format_source attribute is set in a given node issue a warning if the value is input @@ -105,10 +118,12 @@ def __check_format(node, lint_ctx, allow_ext=False): if fmt is None: fmt = node.attrib.get("format") if fmt == "input": - lint_ctx.error( - f"Using format='input' on {node.tag}, format_source attribute is less ambiguous and should be used instead.", - node=node, - ) + message = f"Using format='input' on {node.tag} is deprecated. Use the format_source attribute." + if Version(str(profile)) <= Version("16.01"): + lint_ctx.warn(message, node=node) + else: + lint_ctx.error(message, node=node) + return fmt is not None diff --git a/test/functional/tools/output_format_input.xml b/test/functional/tools/output_format_input.xml new file mode 100644 index 000000000000..c346b10dedf3 --- /dev/null +++ b/test/functional/tools/output_format_input.xml @@ -0,0 +1,26 @@ + + + + + cat '$input' > '$output' + + + + + + + + + + + + + + + + + + + + diff --git a/test/functional/tools/sample_tool_conf.xml b/test/functional/tools/sample_tool_conf.xml index 8e3b722954af..b1a809b7d230 100644 --- a/test/functional/tools/sample_tool_conf.xml +++ b/test/functional/tools/sample_tool_conf.xml @@ -8,8 +8,9 @@ - - + + + diff --git a/test/unit/tool_util/test_tool_linters.py b/test/unit/tool_util/test_tool_linters.py index da9fea004195..bd5f3a3160e6 100644 --- a/test/unit/tool_util/test_tool_linters.py +++ b/test/unit/tool_util/test_tool_linters.py @@ -538,13 +538,20 @@ """ -OUTPUTS_FORMAT_INPUT = """ +OUTPUTS_FORMAT_INPUT_LEGACY = """ """ +OUTPUTS_FORMAT_INPUT = """ + + + + + +""" # check that linter accepts format source for collection elements as means to specify format # and that the linter warns if format and format_source are used @@ -911,15 +918,6 @@ def get_tool_xml_exact(xml_string: str): return parse_xml(tool_path, strip_whitespace=False, remove_comments=False) -def failed_assert_print(lint_ctx): - return ( - f"Valid: {lint_ctx.valid_messages}\n" - f"Info: {lint_ctx.info_messages}\n" - f"Warnings: {lint_ctx.warn_messages}\n" - f"Errors: {lint_ctx.error_messages}" - ) - - def run_lint(lint_ctx, lint_func, lint_target): lint_ctx.lint(name="test_lint", lint_func=lint_func, lint_target=lint_target) # check if the lint messages have the line @@ -1457,8 +1455,8 @@ def test_inputs_duplicate_names(lint_ctx): def test_inputs_repeats(lint_ctx): - tool_xml_tree = get_xml_tree(REPEATS) - run_lint(lint_ctx, inputs.lint_repeats, tool_xml_tree) + tool_source = get_xml_tool_source(REPEATS) + run_lint(lint_ctx, inputs.lint_repeats, tool_source) assert "Repeat does not specify name attribute." in lint_ctx.error_messages assert "Repeat does not specify title attribute." in lint_ctx.error_messages assert not lint_ctx.info_messages @@ -1468,8 +1466,8 @@ def test_inputs_repeats(lint_ctx): def test_outputs_missing(lint_ctx): - tool_xml_tree = get_xml_tree(OUTPUTS_MISSING) - run_lint(lint_ctx, outputs.lint_output, tool_xml_tree) + tool_source = get_xml_tool_source(OUTPUTS_MISSING) + run_lint(lint_ctx, outputs.lint_output, tool_source) assert "Tool contains no outputs section, most tools should produce outputs." in lint_ctx.warn_messages assert not lint_ctx.info_messages assert not lint_ctx.valid_messages @@ -1478,8 +1476,8 @@ def test_outputs_missing(lint_ctx): def test_outputs_multiple(lint_ctx): - tool_xml_tree = get_xml_tree(OUTPUTS_MULTIPLE) - run_lint(lint_ctx, outputs.lint_output, tool_xml_tree) + tool_source = get_xml_tool_source(OUTPUTS_MULTIPLE) + run_lint(lint_ctx, outputs.lint_output, tool_source) assert "0 outputs found." in lint_ctx.info_messages assert "Tool contains multiple output sections, behavior undefined." in lint_ctx.warn_messages assert len(lint_ctx.info_messages) == 1 @@ -1489,8 +1487,8 @@ def test_outputs_multiple(lint_ctx): def test_outputs_unknown_tag(lint_ctx): - tool_xml_tree = get_xml_tree(OUTPUTS_UNKNOWN_TAG) - run_lint(lint_ctx, outputs.lint_output, tool_xml_tree) + tool_source = get_xml_tool_source(OUTPUTS_UNKNOWN_TAG) + run_lint(lint_ctx, outputs.lint_output, tool_source) assert "0 outputs found." in lint_ctx.info_messages assert "Unknown element found in outputs [output]" in lint_ctx.warn_messages assert len(lint_ctx.info_messages) == 1 @@ -1500,8 +1498,8 @@ def test_outputs_unknown_tag(lint_ctx): def test_outputs_unnamed_invalid_name(lint_ctx): - tool_xml_tree = get_xml_tree(OUTPUTS_UNNAMED_INVALID_NAME) - run_lint(lint_ctx, outputs.lint_output, tool_xml_tree) + tool_source = get_xml_tool_source(OUTPUTS_UNNAMED_INVALID_NAME) + run_lint(lint_ctx, outputs.lint_output, tool_source) assert "2 outputs found." in lint_ctx.info_messages assert "Tool output doesn't define a name - this is likely a problem." in lint_ctx.warn_messages assert "Tool data output with missing name doesn't define an output format." in lint_ctx.warn_messages @@ -1514,14 +1512,22 @@ def test_outputs_unnamed_invalid_name(lint_ctx): assert not lint_ctx.error_messages +def test_outputs_format_input_legacy(lint_ctx): + tool_source = get_xml_tool_source(OUTPUTS_FORMAT_INPUT_LEGACY) + run_lint(lint_ctx, outputs.lint_output, tool_source) + assert "1 outputs found." in lint_ctx.info_messages + assert "Using format='input' on data is deprecated. Use the format_source attribute." in lint_ctx.warn_messages + assert len(lint_ctx.info_messages) == 1 + assert not lint_ctx.valid_messages + assert len(lint_ctx.warn_messages) == 1 + assert not lint_ctx.error_messages + + def test_outputs_format_input(lint_ctx): - tool_xml_tree = get_xml_tree(OUTPUTS_FORMAT_INPUT) - run_lint(lint_ctx, outputs.lint_output, tool_xml_tree) + tool_source = get_xml_tool_source(OUTPUTS_FORMAT_INPUT) + run_lint(lint_ctx, outputs.lint_output, tool_source) assert "1 outputs found." in lint_ctx.info_messages - assert ( - "Using format='input' on data, format_source attribute is less ambiguous and should be used instead." - in lint_ctx.error_messages - ) + assert "Using format='input' on data is deprecated. Use the format_source attribute." in lint_ctx.error_messages assert len(lint_ctx.info_messages) == 1 assert not lint_ctx.valid_messages assert not lint_ctx.warn_messages @@ -1529,8 +1535,8 @@ def test_outputs_format_input(lint_ctx): def test_outputs_collection_format_source(lint_ctx): - tool_xml_tree = get_xml_tree(OUTPUTS_COLLECTION_FORMAT_SOURCE) - run_lint(lint_ctx, outputs.lint_output, tool_xml_tree) + tool_source = get_xml_tool_source(OUTPUTS_COLLECTION_FORMAT_SOURCE) + run_lint(lint_ctx, outputs.lint_output, tool_source) assert "Tool data output 'reverse' should use either format_source or format/ext" in lint_ctx.warn_messages assert len(lint_ctx.info_messages) == 1 assert not lint_ctx.valid_messages @@ -1539,8 +1545,8 @@ def test_outputs_collection_format_source(lint_ctx): def test_outputs_format_action(lint_ctx): - tool_xml_tree = get_xml_tree(OUTPUTS_FORMAT_ACTION) - run_lint(lint_ctx, outputs.lint_output, tool_xml_tree) + tool_source = get_xml_tool_source(OUTPUTS_FORMAT_ACTION) + run_lint(lint_ctx, outputs.lint_output, tool_source) assert len(lint_ctx.info_messages) == 1 assert not lint_ctx.valid_messages assert not lint_ctx.warn_messages @@ -1548,8 +1554,8 @@ def test_outputs_format_action(lint_ctx): def test_outputs_discover_tool_provided_metadata(lint_ctx): - tool_xml_tree = get_xml_tree(OUTPUTS_DISCOVER_TOOL_PROVIDED_METADATA) - run_lint(lint_ctx, outputs.lint_output, tool_xml_tree) + tool_source = get_xml_tool_source(OUTPUTS_DISCOVER_TOOL_PROVIDED_METADATA) + run_lint(lint_ctx, outputs.lint_output, tool_source) assert "1 outputs found." in lint_ctx.info_messages assert len(lint_ctx.info_messages) == 1 assert not lint_ctx.valid_messages @@ -1558,8 +1564,8 @@ def test_outputs_discover_tool_provided_metadata(lint_ctx): def test_outputs_duplicated_name_label(lint_ctx): - tool_xml_tree = get_xml_tree(OUTPUTS_DUPLICATED_NAME_LABEL) - run_lint(lint_ctx, outputs.lint_output, tool_xml_tree) + tool_source = get_xml_tool_source(OUTPUTS_DUPLICATED_NAME_LABEL) + run_lint(lint_ctx, outputs.lint_output, tool_source) assert "4 outputs found." in lint_ctx.info_messages assert len(lint_ctx.info_messages) == 1 assert not lint_ctx.valid_messages