diff --git a/lib/galaxy/tool_util/linters/citations.py b/lib/galaxy/tool_util/linters/citations.py index e799e7773528..3e57cf836927 100644 --- a/lib/galaxy/tool_util/linters/citations.py +++ b/lib/galaxy/tool_util/linters/citations.py @@ -26,17 +26,6 @@ def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): lint_ctx.warn("No citations found, consider adding citations to your tool.", node=root) -class CitationsMultiple(Linter): - @classmethod - def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): - tool_xml = getattr(tool_source, "xml_tree", None) - if not tool_xml: - return - citations = tool_xml.findall("citations") - if len(citations) > 1: - lint_ctx.error("More than one citation section found, behavior undefined.", node=citations[1]) - - class CitationsNoText(Linter): @classmethod def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): diff --git a/lib/galaxy/tool_util/linters/command.py b/lib/galaxy/tool_util/linters/command.py index be2c0338a5ed..f0b7f5bb6b80 100644 --- a/lib/galaxy/tool_util/linters/command.py +++ b/lib/galaxy/tool_util/linters/command.py @@ -12,17 +12,6 @@ from galaxy.tool_util.parser.interface import ToolSource -class CommandMultiple(Linter): - @classmethod - def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): - tool_xml = getattr(tool_source, "xml_tree", None) - if not tool_xml: - return - commands = tool_xml.findall("./command") - if len(commands) > 1: - lint_ctx.error("More than one command tag found, behavior undefined.", node=commands[1]) - - class CommandMissing(Linter): @classmethod def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): diff --git a/lib/galaxy/tool_util/linters/help.py b/lib/galaxy/tool_util/linters/help.py index 8c2260bc9925..9b439fd53ad1 100644 --- a/lib/galaxy/tool_util/linters/help.py +++ b/lib/galaxy/tool_util/linters/help.py @@ -15,17 +15,6 @@ from galaxy.tool_util.parser.interface import ToolSource -class HelpMultiple(Linter): - @classmethod - def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): - tool_xml = getattr(tool_source, "xml_tree", None) - if not tool_xml: - return - helps = tool_xml.findall("./help") - if len(helps) > 1: - lint_ctx.error("More than one help section found, behavior undefined.", node=helps[1]) - - class HelpMissing(Linter): @classmethod def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): diff --git a/lib/galaxy/tool_util/linters/inputs.py b/lib/galaxy/tool_util/linters/inputs.py index 55febd6269d9..b8b5ec273eef 100644 --- a/lib/galaxy/tool_util/linters/inputs.py +++ b/lib/galaxy/tool_util/linters/inputs.py @@ -121,7 +121,7 @@ PARAM_TYPE_CHILD_COMBINATIONS = [ ("./options", ["data", "select", "drill_down"]), ("./options/option", ["drill_down"]), - ("./column", ["data_column"]), + ("./options/column", ["select"]), ] # TODO lint for valid param type - attribute combinations @@ -1167,38 +1167,6 @@ def _iter_conditional(tool_xml: "ElementTree") -> Iterator[Tuple["Element", Opti yield conditional, conditional_name, first_param, first_param_type -class ConditionalName(Linter): - @classmethod - def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): - tool_xml = getattr(tool_source, "xml_tree", None) - if not tool_xml: - return - conditionals = tool_xml.findall("./inputs//conditional") - for conditional in conditionals: - conditional_name = conditional.get("name") - if not conditional_name: - lint_ctx.error("Conditional without a name", node=conditional) - - -class ConditionalParamNumber(Linter): - @classmethod - def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): - tool_xml = getattr(tool_source, "xml_tree", None) - if not tool_xml: - return - conditionals = tool_xml.findall("./inputs//conditional") - for conditional in conditionals: - conditional_name = conditional.get("name") - if conditional.get("value_from"): # Probably only the upload tool use this, no children elements - continue - first_param = conditional.findall("param") - if len(first_param) != 1: - lint_ctx.error( - f"Conditional [{conditional_name}] needs exactly one child found {len(first_param)}", - node=conditional, - ) - - class ConditionalParamTypeBool(Linter): @classmethod def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): @@ -1242,20 +1210,6 @@ def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): ) -class ConditionalWhenValue(Linter): - @classmethod - def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): - tool_xml = getattr(tool_source, "xml_tree", None) - if not tool_xml: - return - for conditional, conditional_name, _, first_param_type in _iter_conditional(tool_xml): - if first_param_type not in ["boolean", "select"]: - continue - whens = conditional.findall("./when") - if any("value" not in when.attrib for when in whens): - lint_ctx.error(f"Conditional [{conditional_name}] when without value", node=conditional) - - class ConditionalWhenMissing(Linter): @classmethod def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): diff --git a/lib/galaxy/tool_util/linters/outputs.py b/lib/galaxy/tool_util/linters/outputs.py index 7395e1aed152..db8068f050d6 100644 --- a/lib/galaxy/tool_util/linters/outputs.py +++ b/lib/galaxy/tool_util/linters/outputs.py @@ -4,14 +4,13 @@ from packaging.version import Version from galaxy.tool_util.lint import Linter -from galaxy.util import string_as_bool 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 - from galaxy.util.etree import Element + from galaxy.util.etree import Element, ElementTree class OutputsMissing(Linter): @@ -27,17 +26,6 @@ def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): lint_ctx.warn("Tool contains no outputs section, most tools should produce outputs.", node=tool_node) -class OutputsMultiple(Linter): - @classmethod - def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): - tool_xml = getattr(tool_source, "xml_tree", None) - if not tool_xml: - return - outputs = tool_xml.findall("./outputs") - if len(outputs) > 1: - lint_ctx.warn("Tool contains multiple output sections, behavior undefined.", node=outputs[1]) - - class OutputsOutput(Linter): @classmethod def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): @@ -49,18 +37,6 @@ def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): lint_ctx.warn("Avoid the use of 'output' and replace by 'data' or 'collection'", node=output) -class OutputsNameMissing(Linter): - @classmethod - def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): - tool_xml = getattr(tool_source, "xml_tree", None) - if not tool_xml: - return - for output in tool_xml.findall("./outputs/data") + tool_xml.findall("./outputs/collection"): - if "name" not in output.attrib: - # TODO make this an error if there is no discover_datasets / from_work_dir (is this then still a problem) - lint_ctx.warn("Tool output doesn't define a name - this is likely a problem.", node=output) - - class OutputsNameInvalidCheetah(Linter): @classmethod def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): @@ -187,7 +163,7 @@ def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): if "structured_like" in output.attrib and "inherit_format" in output.attrib: format_set = True for sub in output: - if _check_pattern(sub): + if _check_pattern(sub) or _has_tool_provided_metadata(tool_xml): format_set = True elif _check_format(sub): format_set = True @@ -241,10 +217,6 @@ def _check_pattern(node): """ if node.tag != "discover_datasets": return False - if "from_tool_provided_metadata" in node.attrib and string_as_bool( - node.attrib.get("from_tool_provided_metadata", "false") - ): - return True if "pattern" not in node.attrib: return False pattern = node.attrib["pattern"] @@ -252,3 +224,18 @@ def _check_pattern(node): # TODO error on wrong pattern or non-regexp if "(?P" in regex_pattern: return True + + +def _has_tool_provided_metadata(tool_xml: "ElementTree") -> bool: + outputs = tool_xml.find("./outputs") + if outputs is not None: + if "provided_metadata_file" in outputs.attrib or "provided_metadata_style" in outputs.attrib: + return True + command = tool_xml.find("./command") + if command is not None: + if "galaxy.json" in command.text: + return True + config = tool_xml.find("./configfiles/configfile[@filename='galaxy.json']") + if config is not None: + return True + return False diff --git a/lib/galaxy/tool_util/linters/stdio.py b/lib/galaxy/tool_util/linters/stdio.py index 3e1bf619b279..988d66965f9c 100644 --- a/lib/galaxy/tool_util/linters/stdio.py +++ b/lib/galaxy/tool_util/linters/stdio.py @@ -53,21 +53,6 @@ def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): ) -class StdIOMultiple(Linter): - @classmethod - def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): - tool_xml = getattr(tool_source, "xml_tree", None) - if not tool_xml: - # Can only lint XML tools at this point. - # Should probably use tool_source.parse_stdio() to abstract away XML details - return - stdios = tool_xml.findall("./stdio") if tool_xml else [] - - if len(stdios) > 1: - lint_ctx.error("More than one stdio tag found, behavior undefined.", node=stdios[1]) - return - - class StdIORegex(Linter): @classmethod def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): diff --git a/lib/galaxy/tool_util/linters/tests.py b/lib/galaxy/tool_util/linters/tests.py index 0d7f83fd0292..08232bc9daf7 100644 --- a/lib/galaxy/tool_util/linters/tests.py +++ b/lib/galaxy/tool_util/linters/tests.py @@ -1,8 +1,4 @@ """This module contains a linting functions for tool tests.""" -from inspect import ( - Parameter, - signature, -) from typing import ( List, Optional, @@ -12,7 +8,6 @@ from galaxy.tool_util.lint import Linter from galaxy.util import asbool from ._util import is_datasource -from ..verify import asserts if TYPE_CHECKING: from galaxy.tool_util.lint import LintContext @@ -66,66 +61,6 @@ def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): ) -class TestsAssertsUnknown(Linter): - @classmethod - def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): - tool_xml = getattr(tool_source, "xml_tree", None) - if not tool_xml: - return - tests = tool_xml.findall("./tests/test") - for test_idx, test in enumerate(tests, start=1): - for a in test.xpath( - ".//*[self::assert_contents or self::assert_stdout or self::assert_stderr or self::assert_command]//*" - ): - assert_function_name = "assert_" + a.tag - if assert_function_name not in asserts.assertion_functions: - lint_ctx.error(f"Test {test_idx}: unknown assertion '{a.tag}'", node=a) - - -class TestsAssertsUnknownAttrib(Linter): - @classmethod - def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): - tool_xml = getattr(tool_source, "xml_tree", None) - if not tool_xml: - return - tests = tool_xml.findall("./tests/test") - for test_idx, test in enumerate(tests, start=1): - for a in test.xpath( - ".//*[self::assert_contents or self::assert_stdout or self::assert_stderr or self::assert_command]//*" - ): - assert_function_name = "assert_" + a.tag - if assert_function_name not in asserts.assertion_functions: - continue - assert_function_sig = signature(asserts.assertion_functions[assert_function_name]) - # check of the attributes - for attrib in a.attrib: - if attrib not in assert_function_sig.parameters: - lint_ctx.error(f"Test {test_idx}: unknown attribute '{attrib}' for '{a.tag}'", node=a) - - -class TestsAssertsMissingAttrib(Linter): - @classmethod - def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): - tool_xml = getattr(tool_source, "xml_tree", None) - if not tool_xml: - return - tests = tool_xml.findall("./tests/test") - for test_idx, test in enumerate(tests, start=1): - for a in test.xpath( - ".//*[self::assert_contents or self::assert_stdout or self::assert_stderr or self::assert_command]//*" - ): - assert_function_name = "assert_" + a.tag - if assert_function_name not in asserts.assertion_functions: - continue - assert_function_sig = signature(asserts.assertion_functions[assert_function_name]) - # check missing required attributes - for p in assert_function_sig.parameters: - if p in ["output", "output_bytes", "verify_assertions_function", "children"]: - continue - if assert_function_sig.parameters[p].default is Parameter.empty and p not in a.attrib: - lint_ctx.error(f"Test {test_idx}: missing attribute '{p}' for '{a.tag}'", node=a) - - class TestsAssertsHasNQuant(Linter): @classmethod def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): @@ -181,19 +116,6 @@ def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): ) -class TestsParamName(Linter): - @classmethod - def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): - tool_xml = getattr(tool_source, "xml_tree", None) - if not tool_xml: - return - tests = tool_xml.findall("./tests/test") - for test_idx, test in enumerate(tests, start=1): - for param in test.findall("param"): - if not param.attrib.get("name", None): - lint_ctx.error(f"Test {test_idx}: Found test param tag without a name defined.", node=param) - - class TestsParamInInputs(Linter): """ really simple linter that test parameters are also present in the inputs @@ -233,7 +155,8 @@ def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): return tests = tool_xml.findall("./tests/test") for test_idx, test in enumerate(tests, start=1): - for output in test.findall("output") + test.findall("output_collection"): + # note output_collections are covered by xsd, but output is not required to have one by xsd + for output in test.findall("output"): if not output.attrib.get("name", None): lint_ctx.error(f"Test {test_idx}: Found {output.tag} tag without a name defined.", node=output) diff --git a/test/unit/tool_util/test_tool_linters.py b/test/unit/tool_util/test_tool_linters.py index 3412f938059f..f8f7f6fb16e1 100644 --- a/test/unit/tool_util/test_tool_linters.py +++ b/test/unit/tool_util/test_tool_linters.py @@ -34,18 +34,18 @@ # TODO tests tool xml for general linter # tests tool xml for citations linter CITATIONS_MULTIPLE = """ - + """ CITATIONS_ABSENT = """ - + """ CITATIONS_ERRORS = """ - + @@ -53,7 +53,7 @@ """ CITATIONS_VALID = """ - + DOI @@ -62,23 +62,23 @@ # tests tool xml for command linter COMMAND_MULTIPLE = """ - + ls df """ COMMAND_MISSING = """ - + """ COMMAND_TODO = """ - + ## TODO """ COMMAND_DETECT_ERRORS_INTERPRETER = """ - + """ @@ -91,7 +91,7 @@ """ GENERAL_WHITESPACE_IN_VERSIONS_AND_NAMES = """ -