From 7fcc772a1c4a8b0654c0cf967930cb1fa3eabf51 Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Mon, 17 Jan 2022 18:13:02 +0100 Subject: [PATCH 01/18] small restructuring of linter unit tests --- test/unit/tool_util/test_tool_linters.py | 47 +++++++++++++++++++----- 1 file changed, 37 insertions(+), 10 deletions(-) diff --git a/test/unit/tool_util/test_tool_linters.py b/test/unit/tool_util/test_tool_linters.py index cee5d85f5d55..0bf314037b2c 100644 --- a/test/unit/tool_util/test_tool_linters.py +++ b/test/unit/tool_util/test_tool_linters.py @@ -17,8 +17,8 @@ from galaxy.tool_util.parser.xml import XmlToolSource from galaxy.util import etree +# TODO tests tool xml for general linter # tests tool xml for citations linter -# tests tool xml for general linter CITATIONS_MULTIPLE = """ @@ -586,6 +586,18 @@ """ +TESTS_VALID = """ + + + + + + + + + + +""" # tool xml for xml_order linter XML_ORDER = """ @@ -861,6 +873,13 @@ 'Found 5 input parameters.' in x.info_messages and len(x.info_messages) == 1 and len(x.valid_messages) == 0 and len(x.warn_messages) == 0 and len(x.error_messages) == 0 ), + ( + REPEATS, inputs.lint_repeats, + lambda x: + "Repeat does not specify name attribute." in x.error_messages + and "Repeat does not specify title attribute." in x.error_messages + and len(x.info_messages) == 0 and len(x.valid_messages) == 0 and len(x.warn_messages) == 0 and len(x.error_messages) == 2 + ), ( OUTPUTS_MISSING, outputs.lint_output, lambda x: @@ -974,13 +993,6 @@ 'No tests found, that should be OK for data_sources.' in x.info_messages and len(x.info_messages) == 1 and len(x.valid_messages) == 0 and len(x.warn_messages) == 0 and len(x.error_messages) == 0 ), - ( - TESTS_WO_EXPECTATIONS, tests.lint_tsts, - lambda x: - 'Test 1: No outputs or expectations defined for tests, this test is likely invalid.' in x.warn_messages - and 'No valid test(s) found.' in x.warn_messages - and len(x.info_messages) == 0 and len(x.valid_messages) == 0 and len(x.warn_messages) == 2 and len(x.error_messages) == 0 - ), ( TESTS_PARAM_OUTPUT_NAMES, tests.lint_tsts, lambda x: @@ -1000,6 +1012,19 @@ and "Test 1: Cannot specify outputs in a test expecting failure." in x.error_messages and len(x.info_messages) == 0 and len(x.valid_messages) == 0 and len(x.warn_messages) == 1 and len(x.error_messages) == 1 ), + ( + TESTS_WO_EXPECTATIONS, tests.lint_tsts, + lambda x: + 'Test 1: No outputs or expectations defined for tests, this test is likely invalid.' in x.warn_messages + and 'No valid test(s) found.' in x.warn_messages + and len(x.info_messages) == 0 and len(x.valid_messages) == 0 and len(x.warn_messages) == 2 and len(x.error_messages) == 0 + ), + ( + TESTS_VALID, tests.lint_tsts, + lambda x: + "1 test(s) found." in x.valid_messages + and len(x.info_messages) == 0 and len(x.valid_messages) == 1 and len(x.warn_messages) == 0 and len(x.error_messages) == 0 + ), ( XML_ORDER, xml_order.lint_xml_order, lambda x: @@ -1042,6 +1067,7 @@ 'inputs: select filter', 'inputs: validator incompatibilities', 'inputs: validator all correct', + 'repeats', 'outputs: missing outputs tag', 'outputs: multiple outputs tags', 'outputs: unknow tag in outputs', @@ -1057,10 +1083,11 @@ 'stdio: invalid tag or attribute', 'tests: absent', 'tests: absent data_source', - 'tests: without expectations', 'tests: param and output names', 'tests: expecting failure with outputs', - 'xml_order', + 'tests: without expectations', + 'tests: valid', + 'xml_order' ] From 4e237d84e22ecff6ddc9b8d136446edb5ed2f2f2 Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Mon, 17 Jan 2022 18:22:10 +0100 Subject: [PATCH 02/18] lint for invalid regex in stdio regex --- lib/galaxy/tool_util/linters/stdio.py | 12 ++++++++---- test/unit/tool_util/test_tool_linters.py | 20 +++++++++++++++++--- 2 files changed, 25 insertions(+), 7 deletions(-) diff --git a/lib/galaxy/tool_util/linters/stdio.py b/lib/galaxy/tool_util/linters/stdio.py index 0c40fbc04318..182255b1d37a 100644 --- a/lib/galaxy/tool_util/linters/stdio.py +++ b/lib/galaxy/tool_util/linters/stdio.py @@ -1,4 +1,6 @@ """This module contains a linting functions for tool error detection.""" +import re + from .command import get_command @@ -42,13 +44,15 @@ def _lint_exit_code(tool_xml, child, lint_ctx): for key in child.attrib.keys(): if key not in ["description", "level", "range"]: lint_ctx.warn(f"Unknown attribute [{key}] encountered on exit_code tag.", line=child.sourceline, xpath=tool_xml.getpath(child)) - # TODO lint range, or is regexp in xsd sufficient? - # TODO lint required attributes def _lint_regex(tool_xml, child, lint_ctx): for key in child.attrib.keys(): if key not in ["description", "level", "match", "source"]: lint_ctx.warn(f"Unknown attribute [{key}] encountered on regex tag.", line=child.sourceline, xpath=tool_xml.getpath(child)) - # TODO lint match (re.compile) - # TODO lint required attributes + match = child.attrib.get("match") + if match: + try: + re.compile(match) + except Exception as e: + lint_ctx.error(f"Match '{match}' is no valid regular expression: {str(e)}", line=child.sourceline, xpath=tool_xml.getpath(child)) diff --git a/test/unit/tool_util/test_tool_linters.py b/test/unit/tool_util/test_tool_linters.py index 0bf314037b2c..f82aa84d0dba 100644 --- a/test/unit/tool_util/test_tool_linters.py +++ b/test/unit/tool_util/test_tool_linters.py @@ -473,14 +473,14 @@ """ STDIO_MULTIPLE_STDIO = """ - + """ STDIO_INVALID_CHILD_OR_ATTRIB = """ - + @@ -489,6 +489,14 @@ """ +STDIO_INVALID_MATCH = """ + + + + + +""" + # check that linter does complain about tests wo assumptions TESTS_ABSENT = """ @@ -979,7 +987,12 @@ and "Unknown attribute [descriptio] encountered on exit_code tag." in x.warn_messages and "Unknown attribute [descriptio] encountered on regex tag." in x.warn_messages and len(x.info_messages) == 0 and len(x.valid_messages) == 0 and len(x.warn_messages) == 3 and len(x.error_messages) == 0 - + ), + ( + STDIO_INVALID_MATCH, stdio.lint_stdio, + lambda x: + "Match '[' is no valid regular expression: unterminated character set at position 0" in x.error_messages + and len(x.info_messages) == 0 and len(x.valid_messages) == 0 and len(x.warn_messages) == 0 and len(x.error_messages) == 1 ), ( TESTS_ABSENT, tests.lint_tsts, @@ -1081,6 +1094,7 @@ 'stdio: default for non-legacy profile', 'stdio: multiple stdio', 'stdio: invalid tag or attribute', + 'stdio: invalid regular expression', 'tests: absent', 'tests: absent data_source', 'tests: param and output names', From 5ea6253e653660b25a157801740b1e2a3bbe4b8a Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Tue, 18 Jan 2022 19:27:36 +0100 Subject: [PATCH 03/18] two minor improvements to cwl and yaml tools in preparation to get linter tests running for such tools cwl: - remove a print from SchemaLoader yaml: - define YamlToolSource.parse_tool_type otherwise yaml tools are parsed like xml tools - parse_version: return a string, otherwise 1.0 is treated as float which makes the linter (and probably more) fail --- lib/galaxy/tool_util/cwl/schema.py | 12 ------------ lib/galaxy/tool_util/parser/yaml.py | 5 ++++- 2 files changed, 4 insertions(+), 13 deletions(-) diff --git a/lib/galaxy/tool_util/cwl/schema.py b/lib/galaxy/tool_util/cwl/schema.py index 46e3759b2180..5356a62d982a 100644 --- a/lib/galaxy/tool_util/cwl/schema.py +++ b/lib/galaxy/tool_util/cwl/schema.py @@ -41,12 +41,6 @@ def raw_process_reference(self, path, loading_context=None): suffix = '' if '#' in path: path, suffix = path.split('#') - print(f""" ------- - -{open(path).read()} - - ------- - """) processed_path = os.path.join(output_dir, os.path.basename(path)) path = os.path.abspath(path) uri = f"file://{path}" @@ -56,12 +50,6 @@ def raw_process_reference(self, path, loading_context=None): exit_code = cwl_expression_refactor.main([output_dir, path, '--skip-some1', '--skip-some2']) if exit_code == 0: uri = f"file://{processed_path}" - print(f""" ------- - -{open(processed_path).read()} - - ------- - """) if suffix: uri = f"{uri}#{suffix}" loading_context, process_object, uri = load_tool.fetch_document(uri, loadingContext=loading_context) diff --git a/lib/galaxy/tool_util/parser/yaml.py b/lib/galaxy/tool_util/parser/yaml.py index f8a6e9acc64f..779d5a056e30 100644 --- a/lib/galaxy/tool_util/parser/yaml.py +++ b/lib/galaxy/tool_util/parser/yaml.py @@ -36,11 +36,14 @@ def __init__(self, root_dict, source_path=None): def source_path(self): return self._source_path + def parse_tool_type(self): + return 'yml' + def parse_id(self): return self.root_dict.get("id") def parse_version(self): - return self.root_dict.get("version") + return str(self.root_dict.get("version")) def parse_name(self): return self.root_dict.get("name") From b7ef626fdf2a5f398ca691fcea8d60ddc8faee6f Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Tue, 18 Jan 2022 19:30:38 +0100 Subject: [PATCH 04/18] fix node.base to macro file while parsing macros --- lib/galaxy/util/xml_macros.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/lib/galaxy/util/xml_macros.py b/lib/galaxy/util/xml_macros.py index 24883e96f2c0..2bf51ddf8612 100644 --- a/lib/galaxy/util/xml_macros.py +++ b/lib/galaxy/util/xml_macros.py @@ -283,6 +283,14 @@ def _imported_macro_paths_from_el(macros_el): def _load_macro_file(path, xml_base_dir): tree = parse_xml(path, strip_whitespace=False) + for node in tree.iter(): + # little hack: node.base contains that path to the file which is used in + # the linter. it is a property that apparently is determined from the + # xmltree, so if the macro is inserted into the main xml node.base will + # give the path of the main xml file. + # luckily lxml allows to set the property: + # https://github.com/lxml/lxml/blob/5a5c7fb01d15af58def4bab2ba7b15c937042835/src/lxml/etree.pyx#L1106 + node.base = node.base root = tree.getroot() return _load_macros(root, xml_base_dir) From 977f0fa88157a2e0fd394bd5e6055dd71fc32918 Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Tue, 18 Jan 2022 20:07:36 +0100 Subject: [PATCH 05/18] xsd: make name attribute required --- lib/galaxy/tool_util/xsd/galaxy.xsd | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/galaxy/tool_util/xsd/galaxy.xsd b/lib/galaxy/tool_util/xsd/galaxy.xsd index c82d22c9ce0d..7b61a48f8b57 100644 --- a/lib/galaxy/tool_util/xsd/galaxy.xsd +++ b/lib/galaxy/tool_util/xsd/galaxy.xsd @@ -2939,7 +2939,7 @@ demonstrates both testing strategies. - + Name for this element From 29f3f40296501d0637e919c62e6f19e2777fb09d Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Tue, 18 Jan 2022 20:08:41 +0100 Subject: [PATCH 06/18] add file name to linter message - add fname to LintMessage - LintContext make lint messages persistent over multiple linter calls before the messages were reset before the call of the next linter which is "impractical" if we want to use the lintmessages eg in planemo - add test for linting a tool + (internal and external) macros to ensure that the fname and line number is correct - add a simple linter test for yaml and json tools --- lib/galaxy/tool_util/lint.py | 39 +++-- lib/galaxy/tool_util/linters/_util.py | 9 ++ lib/galaxy/tool_util/linters/citations.py | 21 +-- lib/galaxy/tool_util/linters/command.py | 21 +-- lib/galaxy/tool_util/linters/cwl.py | 4 +- lib/galaxy/tool_util/linters/help.py | 23 ++- lib/galaxy/tool_util/linters/inputs.py | 135 ++++++++-------- lib/galaxy/tool_util/linters/outputs.py | 29 ++-- lib/galaxy/tool_util/linters/stdio.py | 24 ++- lib/galaxy/tool_util/linters/tests.py | 39 ++--- lib/galaxy/tool_util/linters/xml_order.py | 6 +- test/unit/tool_util/test_tool_linters.py | 189 +++++++++++++++++++++- 12 files changed, 353 insertions(+), 186 deletions(-) diff --git a/lib/galaxy/tool_util/lint.py b/lib/galaxy/tool_util/lint.py index aac34bb1cae7..91435adfac1d 100644 --- a/lib/galaxy/tool_util/lint.py +++ b/lib/galaxy/tool_util/lint.py @@ -64,19 +64,26 @@ def lint_xml_with(lint_context, tool_xml, extra_modules=None): class LintMessage: - def __init__(self, level, message, line, xpath=None): + """ + a message from the linter + """ + def __init__(self, level, message, line, fname, xpath=None): self.level = level self.message = message - self.line = line + self.line = line # 1 based + self.fname = fname self.xpath = xpath def __str__(self): rval = f".. {self.level.upper()}: {self.message}" if self.line is not None: - rval += f" (line {self.line})" + rval += " (" + if self.fname: + rval += f"{self.fname}:" + rval += str(self.line) + rval += ")" if self.xpath is not None: rval += f" [{self.xpath}]" - return rval @@ -91,12 +98,14 @@ def __init__(self, level, skip_types=None, object_name=None): self.object_name = object_name self.found_errors = False self.found_warns = False + self.message_list = [] def lint(self, name, lint_func, lint_target): name = name.replace("tsts", "tests")[len("lint_"):] if name in self.skip_types: return self.printed_linter_info = False + tmp_message_list = list(self.message_list) self.message_list = [] # call linter @@ -141,6 +150,7 @@ def print_linter_info(): continue print_linter_info() print(f"{message}") + self.message_list = tmp_message_list + self.message_list @property def valid_messages(self): @@ -158,22 +168,23 @@ def warn_messages(self): def error_messages(self): return [x.message for x in self.message_list if x.level == "error"] - def __handle_message(self, level, message, line, xpath, *args): + def __handle_message(self, level, message, line, fname, xpath, *args): if args: message = message % args - self.message_list.append(LintMessage(level=level, message=message, line=line, xpath=xpath)) + self.message_list.append(LintMessage(level=level, message=message, + line=line, fname=fname, xpath=xpath)) - def valid(self, message, line=None, xpath=None, *args): - self.__handle_message("check", message, line, xpath, *args) + def valid(self, message, line=None, fname=None, xpath=None, *args): + self.__handle_message("check", message, line, fname, xpath, *args) - def info(self, message, line=None, xpath=None, *args): - self.__handle_message("info", message, line, xpath, *args) + def info(self, message, line=None, fname=None, xpath=None, *args): + self.__handle_message("info", message, line, fname, xpath, *args) - def error(self, message, line=None, xpath=None, *args): - self.__handle_message("error", message, line, xpath, *args) + def error(self, message, line=None, fname=None, xpath=None, *args): + self.__handle_message("error", message, line, fname, xpath, *args) - def warn(self, message, line=None, xpath=None, *args): - self.__handle_message("warning", message, line, xpath, *args) + def warn(self, message, line=None, fname=None, xpath=None, *args): + self.__handle_message("warning", message, line, fname, xpath, *args) def failed(self, fail_level): found_warns = self.found_warns diff --git a/lib/galaxy/tool_util/linters/_util.py b/lib/galaxy/tool_util/linters/_util.py index 3224150c792b..03e728b38ef9 100644 --- a/lib/galaxy/tool_util/linters/_util.py +++ b/lib/galaxy/tool_util/linters/_util.py @@ -1,6 +1,15 @@ import re +def node_props(node, tool_xml): + if node is None: + return {"line": 0, "fname": None, "xpath": None} + else: + return {"line": node.sourceline, + "fname": node.base, + "xpath": tool_xml.getpath(node)} + + def is_datasource(tool_xml): """Returns true if the tool is a datasource tool""" return tool_xml.getroot().attrib.get('tool_type', '') == 'data_source' diff --git a/lib/galaxy/tool_util/linters/citations.py b/lib/galaxy/tool_util/linters/citations.py index 444fecf0650d..6bab16702f3d 100644 --- a/lib/galaxy/tool_util/linters/citations.py +++ b/lib/galaxy/tool_util/linters/citations.py @@ -3,41 +3,36 @@ Citations describe references that should be used when consumers of the tool publish results. """ +from ._util import node_props def lint_citations(tool_xml, lint_ctx): """Ensure tool contains at least one valid citation.""" root = tool_xml.getroot() - if root is not None: - root_line = root.sourceline - root_xpath = tool_xml.getpath(root) - else: - root_line = 1 - root_xpath = None citations = root.findall("citations") if len(citations) > 1: - lint_ctx.error("More than one citation section found, behavior undefined.", line=citations[1].sourceline, xpath=tool_xml.getpath(citations[1])) + lint_ctx.error("More than one citation section found, behavior undefined.", **node_props(citations[1], tool_xml)) return if len(citations) == 0: - lint_ctx.warn("No citations found, consider adding citations to your tool.", line=root_line, xpath=root_xpath) + lint_ctx.warn("No citations found, consider adding citations to your tool.", **node_props(root, tool_xml)) return valid_citations = 0 for citation in citations[0]: if citation.tag != "citation": - lint_ctx.warn(f"Unknown tag discovered in citations block [{citation.tag}], will be ignored.", line=citation.sourceline, xpath=tool_xml.getpath(citation)) + lint_ctx.warn(f"Unknown tag discovered in citations block [{citation.tag}], will be ignored.", **node_props(citation, tool_xml)) continue citation_type = citation.attrib.get("type") if citation_type not in ('bibtex', 'doi'): - lint_ctx.warn(f"Unknown citation type discovered [{citation_type}], will be ignored.", line=citation.sourceline, xpath=tool_xml.getpath(citation)) + lint_ctx.warn(f"Unknown citation type discovered [{citation_type}], will be ignored.", **node_props(citation, tool_xml)) continue if citation.text is None or not citation.text.strip(): - lint_ctx.error(f'Empty {citation_type} citation.', line=citation.sourceline, xpath=tool_xml.getpath(citation)) + lint_ctx.error(f'Empty {citation_type} citation.', **node_props(citation, tool_xml)) continue valid_citations += 1 if valid_citations > 0: - lint_ctx.valid(f"Found {valid_citations} likely valid citations.", line=root_line, xpath=root_xpath) + lint_ctx.valid(f"Found {valid_citations} likely valid citations.", **node_props(root, tool_xml)) else: - lint_ctx.warn("Found no valid citations.", line=root_line, xpath=root_xpath) + lint_ctx.warn("Found no valid citations.", **node_props(root, tool_xml)) diff --git a/lib/galaxy/tool_util/linters/command.py b/lib/galaxy/tool_util/linters/command.py index 231cd6a5a0c1..fa76d487e3a4 100644 --- a/lib/galaxy/tool_util/linters/command.py +++ b/lib/galaxy/tool_util/linters/command.py @@ -3,31 +3,26 @@ A command description describes how to build the command-line to execute from supplied inputs. """ +from ._util import node_props def lint_command(tool_xml, lint_ctx): """Ensure tool contains exactly one command and check attributes.""" root = tool_xml.getroot() - if root is not None: - root_line = root.sourceline - root_path = tool_xml.getpath(root) - else: - root_line = 1 - root_path = None commands = root.findall("command") if len(commands) > 1: - lint_ctx.error("More than one command tag found, behavior undefined.", line=commands[1].sourceline, xpath=tool_xml.getpath(commands[1])) + lint_ctx.error("More than one command tag found, behavior undefined.", **node_props(commands[1], tool_xml)) return if len(commands) == 0: - lint_ctx.error("No command tag found, must specify a command template to execute.", line=root_line, xpath=root_path) + lint_ctx.error("No command tag found, must specify a command template to execute.", **node_props(root, tool_xml)) return command = get_command(tool_xml) if command.text is None: - lint_ctx.error("Command is empty.", line=root_line, xpath=root_path) + lint_ctx.error("Command is empty.", **node_props(root, tool_xml)) elif "TODO" in command.text: - lint_ctx.warn("Command template contains TODO text.", line=command.sourceline, xpath=tool_xml.getpath(command)) + lint_ctx.warn("Command template contains TODO text.", **node_props(command, tool_xml)) command_attrib = command.attrib interpreter_type = None @@ -37,14 +32,14 @@ def lint_command(tool_xml, lint_ctx): elif key == "detect_errors": detect_errors = value if detect_errors not in ["default", "exit_code", "aggressive"]: - lint_ctx.warn(f"Unknown detect_errors attribute [{detect_errors}]", line=command.sourceline, xpath=tool_xml.getpath(command)) + lint_ctx.warn(f"Unknown detect_errors attribute [{detect_errors}]", **node_props(command, tool_xml)) interpreter_info = "" if interpreter_type: interpreter_info = f" with interpreter of type [{interpreter_type}]" if interpreter_type: - lint_ctx.warn("Command uses deprecated 'interpreter' attribute.", line=command.sourceline, xpath=tool_xml.getpath(command)) - lint_ctx.info(f"Tool contains a command{interpreter_info}.", line=command.sourceline, xpath=tool_xml.getpath(command)) + lint_ctx.warn("Command uses deprecated 'interpreter' attribute.", **node_props(command, tool_xml)) + lint_ctx.info(f"Tool contains a command{interpreter_info}.", **node_props(command, tool_xml)) def get_command(tool_xml): diff --git a/lib/galaxy/tool_util/linters/cwl.py b/lib/galaxy/tool_util/linters/cwl.py index 69ddd9efd8be..784961eef20e 100644 --- a/lib/galaxy/tool_util/linters/cwl.py +++ b/lib/galaxy/tool_util/linters/cwl.py @@ -14,7 +14,7 @@ def lint_cwl_validation(tool_source, lint_ctx): except Exception as e: validation_exception = e if validation_exception: - lint_ctx.error("Failed to valdiate CWL artifact [%s]", validation_exception) + lint_ctx.error(f"Failed to valdiate CWL artifact: {validation_exception}") else: lint_ctx.info("CWL appears to be valid.") @@ -28,7 +28,7 @@ def lint_new_draft(tool_source, lint_ctx): if cwl_version not in ["v1.0"]: lint_ctx.warn(f"CWL version [{cwl_version}] is unknown, we recommend the v1.0 the stable release.") else: - lint_ctx.info("Modern CWL version [%s]", cwl_version) + lint_ctx.info(f"Modern CWL version [{cwl_version}].") def lint_docker_image(tool_source, lint_ctx): diff --git a/lib/galaxy/tool_util/linters/help.py b/lib/galaxy/tool_util/linters/help.py index 2667966856d9..b62392d8bc3e 100644 --- a/lib/galaxy/tool_util/linters/help.py +++ b/lib/galaxy/tool_util/linters/help.py @@ -3,42 +3,37 @@ rst_to_html, unicodify, ) +from ._util import node_props def lint_help(tool_xml, lint_ctx): """Ensure tool contains exactly one valid RST help block.""" - # determine line to report for general problems with help + # determine node to report for general problems with help root = tool_xml.getroot() - if root is not None: - root_line = root.sourceline - root_path = tool_xml.getpath(root) - else: - root_line = 1 - root_path = None helps = root.findall("help") if len(helps) > 1: - lint_ctx.error("More than one help section found, behavior undefined.", line=helps[1].sourceline, xpath=tool_xml.getpath(helps[1])) + lint_ctx.error("More than one help section found, behavior undefined.", **node_props(helps[1], tool_xml)) return if len(helps) == 0: - lint_ctx.warn("No help section found, consider adding a help section to your tool.", line=root_line, xpath=root_path) + lint_ctx.warn("No help section found, consider adding a help section to your tool.", **node_props(root, tool_xml)) return help = helps[0].text or '' if not help.strip(): - lint_ctx.warn("Help section appears to be empty.", line=helps[0].sourceline, xpath=tool_xml.getpath(helps[0])) + lint_ctx.warn("Help section appears to be empty.", **node_props(helps[0], tool_xml)) return - lint_ctx.valid("Tool contains help section.", line=helps[0].sourceline, xpath=tool_xml.getpath(helps[0])) + lint_ctx.valid("Tool contains help section.", **node_props(helps[0], tool_xml)) invalid_rst = rst_invalid(help) if "TODO" in help: - lint_ctx.warn("Help contains TODO text.", line=helps[0].sourceline, xpath=tool_xml.getpath(helps[0])) + lint_ctx.warn("Help contains TODO text.", **node_props(helps[0], tool_xml)) if invalid_rst: - lint_ctx.warn(f"Invalid reStructuredText found in help - [{invalid_rst}].", line=helps[0].sourceline, xpath=tool_xml.getpath(helps[0])) + lint_ctx.warn(f"Invalid reStructuredText found in help - [{invalid_rst}].", **node_props(helps[0], tool_xml)) else: - lint_ctx.valid("Help contains valid reStructuredText.", line=helps[0].sourceline, xpath=tool_xml.getpath(helps[0])) + lint_ctx.valid("Help contains valid reStructuredText.", **node_props(helps[0], tool_xml)) def rst_invalid(text): diff --git a/lib/galaxy/tool_util/linters/inputs.py b/lib/galaxy/tool_util/linters/inputs.py index 93557997224d..a800dd58a0d2 100644 --- a/lib/galaxy/tool_util/linters/inputs.py +++ b/lib/galaxy/tool_util/linters/inputs.py @@ -1,6 +1,12 @@ """This module contains a linting functions for tool inputs.""" +import re + from galaxy.util import string_as_bool -from ._util import is_datasource, is_valid_cheetah_placeholder +from ._util import ( + is_datasource, + is_valid_cheetah_placeholder, + node_props +) from ..parser.util import _parse_name FILTER_TYPES = [ @@ -46,13 +52,8 @@ def lint_inputs(tool_xml, lint_ctx): """Lint parameters in a tool's inputs block.""" - # determine line to report for general problems with outputs - try: - tool_line = tool_xml.find("./tool").sourceline - tool_path = tool_xml.getpath(tool_xml.find("./tool")) - except AttributeError: - tool_line = 1 - tool_path = None + # determine node to report for general problems with outputs + tool_node = tool_xml.find("./tool") datasource = is_datasource(tool_xml) inputs = tool_xml.findall("./inputs//param") num_inputs = 0 @@ -60,33 +61,32 @@ def lint_inputs(tool_xml, lint_ctx): num_inputs += 1 param_attrib = param.attrib if "name" not in param_attrib and "argument" not in param_attrib: - lint_ctx.error("Found param input with no name specified.", line=param.sourceline, xpath=tool_xml.getpath(param)) + lint_ctx.error("Found param input with no name specified.", **node_props(param, tool_xml)) continue param_name = _parse_name(param_attrib.get("name"), param_attrib.get("argument")) if "name" in param_attrib and "argument" in param_attrib: if param_attrib.get("name") == _parse_name(None, param_attrib.get("argument")): - lint_ctx.warn(f"Param input [{param_name}] 'name' attribute is redundant if argument implies the same name.") + lint_ctx.warn(f"Param input [{param_name}] 'name' attribute is redundant if argument implies the same name.", **node_props(param, tool_xml)) if param_name.strip() == "": - lint_ctx.error("Param input with empty name.", line=param.sourceline, xpath=tool_xml.getpath(param)) + lint_ctx.error("Param input with empty name.", **node_props(param, tool_xml)) elif not is_valid_cheetah_placeholder(param_name): - lint_ctx.warn(f"Param input [{param_name}] is not a valid Cheetah placeholder.", line=param.sourceline, xpath=tool_xml.getpath(param)) + lint_ctx.warn(f"Param input [{param_name}] is not a valid Cheetah placeholder.", **node_props(param, tool_xml)) # TODO lint for params with duplicated name (in inputs & outputs) if "type" not in param_attrib: - lint_ctx.error(f"Param input [{param_name}] input with no type specified.", line=param.sourceline, xpath=tool_xml.getpath(param)) + lint_ctx.error(f"Param input [{param_name}] input with no type specified.", **node_props(param, tool_xml)) continue elif param_attrib["type"].strip() == "": - lint_ctx.error(f"Param input [{param_name}] with empty type specified.", line=param.sourceline, xpath=tool_xml.getpath(param)) + lint_ctx.error(f"Param input [{param_name}] with empty type specified.", **node_props(param, tool_xml)) continue param_type = param_attrib["type"] # TODO lint for valid param type - attribute combinations - # TODO lint required attributes for valid each param type if param_type == "data": if "format" not in param_attrib: - lint_ctx.warn(f"Param input [{param_name}] with no format specified - 'data' format will be assumed.", line=param.sourceline, xpath=tool_xml.getpath(param)) + lint_ctx.warn(f"Param input [{param_name}] with no format specified - 'data' format will be assumed.", **node_props(param, tool_xml)) elif param_type == "select": # get dynamic/statically defined options dynamic_options = param.get("dynamic_options", None) @@ -95,15 +95,15 @@ def lint_inputs(tool_xml, lint_ctx): select_options = param.findall('./option') if dynamic_options is not None: - lint_ctx.warn(f"Select parameter [{param_name}] uses deprecated 'dynamic_options' attribute.", line=param.sourceline, xpath=tool_xml.getpath(param)) + lint_ctx.warn(f"Select parameter [{param_name}] uses deprecated 'dynamic_options' attribute.", **node_props(param, tool_xml)) # check if options are defined by exactly one possibility if param.getparent().tag != "conditional": if (dynamic_options is not None) + (len(options) > 0) + (len(select_options) > 0) != 1: - lint_ctx.error(f"Select parameter [{param_name}] options have to be defined by either 'option' children elements, a 'options' element or the 'dynamic_options' attribute.", line=param.sourceline, xpath=tool_xml.getpath(param)) + lint_ctx.error(f"Select parameter [{param_name}] options have to be defined by either 'option' children elements, a 'options' element or the 'dynamic_options' attribute.", **node_props(param, tool_xml)) else: if len(select_options) == 0: - lint_ctx.error(f"Select parameter of a conditional [{param_name}] options have to be defined by 'option' children elements.", line=param.sourceline, xpath=tool_xml.getpath(param)) + lint_ctx.error(f"Select parameter of a conditional [{param_name}] options have to be defined by 'option' children elements.", **node_props(param, tool_xml)) # lint dynamic options if len(options) == 1: @@ -114,10 +114,10 @@ def lint_inputs(tool_xml, lint_ctx): for f in filters: ftype = f.get("type", None) if ftype is None: - lint_ctx.error(f"Select parameter [{param_name}] contains filter without type.", line=f.sourceline, xpath=tool_xml.getpath(f)) + lint_ctx.error(f"Select parameter [{param_name}] contains filter without type.", **node_props(f, tool_xml)) continue if ftype not in FILTER_TYPES: - lint_ctx.error(f"Select parameter [{param_name}] contains filter with unknown type '{ftype}'.", line=f.sourceline, xpath=tool_xml.getpath(f)) + lint_ctx.error(f"Select parameter [{param_name}] contains filter with unknown type '{ftype}'.", **node_props(f, tool_xml)) continue if ftype in ['add_value', 'data_meta']: filter_adds_options = True @@ -132,33 +132,26 @@ def lint_inputs(tool_xml, lint_ctx): if (from_file is None and from_parameter is None and from_dataset is None and from_data_table is None and not filter_adds_options): - lint_ctx.error(f"Select parameter [{param_name}] options tag defines no options. Use 'from_dataset', 'from_data_table', or a filter that adds values.", line=options[0].sourceline, xpath=tool_xml.getpath(options[0])) + lint_ctx.error(f"Select parameter [{param_name}] options tag defines no options. Use 'from_dataset', 'from_data_table', or a filter that adds values.", **node_props(options[0], tool_xml)) - if from_file is not None: - lint_ctx.warn(f"Select parameter [{param_name}] options uses deprecated 'from_file' attribute.", line=options[0].sourceline, xpath=tool_xml.getpath(options[0])) - if from_parameter is not None: - lint_ctx.warn(f"Select parameter [{param_name}] options uses deprecated 'from_parameter' attribute.", line=options[0].sourceline, xpath=tool_xml.getpath(options[0])) + for deprecated_attr in ["from_file", "from_parameter", "options_filter_attribute", "transform_lines"]: + if options[0].get(deprecated_attr) is not None: + lint_ctx.warn(f"Select parameter [{param_name}] options uses deprecated '{deprecated_attr}' attribute.", **node_props(options[0], tool_xml)) if from_dataset is not None and from_data_table is not None: - lint_ctx.error(f"Select parameter [{param_name}] options uses 'from_dataset' and 'from_data_table' attribute.", line=options[0].sourceline, xpath=tool_xml.getpath(options[0])) + lint_ctx.error(f"Select parameter [{param_name}] options uses 'from_dataset' and 'from_data_table' attribute.", **node_props(options[0], tool_xml)) if options[0].get("meta_file_key", None) is not None and from_dataset is None: - lint_ctx.error(f"Select parameter [{param_name}] 'meta_file_key' is only compatible with 'from_dataset'.", line=options[0].sourceline, xpath=tool_xml.getpath(options[0])) - - if options[0].get("options_filter_attribute", None) is not None: - lint_ctx.warn(f"Select parameter [{param_name}] options uses deprecated 'options_filter_attribute' attribute.", line=options[0].sourceline, xpath=tool_xml.getpath(options[0])) - - if options[0].get("transform_lines", None) is not None: - lint_ctx.warn(f"Select parameter [{param_name}] options uses deprecated 'transform_lines' attribute.", line=options[0].sourceline, xpath=tool_xml.getpath(options[0])) + lint_ctx.error(f"Select parameter [{param_name}] 'meta_file_key' is only compatible with 'from_dataset'.", **node_props(options[0], tool_xml)) elif len(options) > 1: - lint_ctx.error(f"Select parameter [{param_name}] contains multiple options elements.", line=options[1].sourceline, xpath=tool_xml.getpath(options[1])) + lint_ctx.error(f"Select parameter [{param_name}] contains multiple options elements.", **node_props(options[1], tool_xml)) # lint statically defined options if any('value' not in option.attrib for option in select_options): - lint_ctx.error(f"Select parameter [{param_name}] has option without value", line=param.sourceline, xpath=tool_xml.getpath(param)) + lint_ctx.error(f"Select parameter [{param_name}] has option without value", **node_props(param, tool_xml)) if any(option.text is None for option in select_options): - lint_ctx.warn(f"Select parameter [{param_name}] has option without text", line=param.sourceline, xpath=tool_xml.getpath(param)) + lint_ctx.warn(f"Select parameter [{param_name}] has option without text", **node_props(param, tool_xml)) select_options_texts = list() select_options_values = list() @@ -171,22 +164,22 @@ def lint_inputs(tool_xml, lint_ctx): select_options_texts.append((text, option.attrib.get("selected", "false"))) select_options_values.append((value, option.attrib.get("selected", "false"))) if len(set(select_options_texts)) != len(select_options_texts): - lint_ctx.error(f"Select parameter [{param_name}] has multiple options with the same text content", line=param.sourceline, xpath=tool_xml.getpath(param)) + lint_ctx.error(f"Select parameter [{param_name}] has multiple options with the same text content", **node_props(param, tool_xml)) if len(set(select_options_values)) != len(select_options_values): - lint_ctx.error(f"Select parameter [{param_name}] has multiple options with the same value", line=param.sourceline, xpath=tool_xml.getpath(param)) + lint_ctx.error(f"Select parameter [{param_name}] has multiple options with the same value", **node_props(param, tool_xml)) multiple = string_as_bool(param_attrib.get("multiple", "false")) optional = string_as_bool(param_attrib.get("optional", multiple)) if param_attrib.get("display") == "checkboxes": if not multiple: - lint_ctx.error(f'Select [{param_name}] `display="checkboxes"` is incompatible with `multiple="false"`, remove the `display` attribute', line=param.sourceline, xpath=tool_xml.getpath(param)) + lint_ctx.error(f'Select [{param_name}] `display="checkboxes"` is incompatible with `multiple="false"`, remove the `display` attribute', **node_props(param, tool_xml)) if not optional: - lint_ctx.error(f'Select [{param_name}] `display="checkboxes"` is incompatible with `optional="false"`, remove the `display` attribute', line=param.sourceline, xpath=tool_xml.getpath(param)) + lint_ctx.error(f'Select [{param_name}] `display="checkboxes"` is incompatible with `optional="false"`, remove the `display` attribute', **node_props(param, tool_xml)) if param_attrib.get("display") == "radio": if multiple: - lint_ctx.error(f'Select [{param_name}] display="radio" is incompatible with multiple="true"', line=param.sourceline, xpath=tool_xml.getpath(param)) + lint_ctx.error(f'Select [{param_name}] display="radio" is incompatible with multiple="true"', **node_props(param, tool_xml)) if optional: - lint_ctx.error(f'Select [{param_name}] display="radio" is incompatible with optional="true"', line=param.sourceline, xpath=tool_xml.getpath(param)) + lint_ctx.error(f'Select [{param_name}] display="radio" is incompatible with optional="true"', **node_props(param, tool_xml)) # TODO: Validate type, much more... # lint validators @@ -196,40 +189,46 @@ def lint_inputs(tool_xml, lint_ctx): vtype = validator.attrib['type'] if param_type in PARAMETER_VALIDATOR_TYPE_COMPATIBILITY: if vtype not in PARAMETER_VALIDATOR_TYPE_COMPATIBILITY[param_type]: - lint_ctx.error(f"Parameter [{param_name}]: validator with an incompatible type '{vtype}'", line=validator.sourceline, xpath=tool_xml.getpath(validator)) + lint_ctx.error(f"Parameter [{param_name}]: validator with an incompatible type '{vtype}'", **node_props(validator, tool_xml)) for attrib in ATTRIB_VALIDATOR_COMPATIBILITY: if attrib in validator.attrib and vtype not in ATTRIB_VALIDATOR_COMPATIBILITY[attrib]: - lint_ctx.error(f"Parameter [{param_name}]: attribute '{attrib}' is incompatible with validator of type '{vtype}'", line=validator.sourceline, xpath=tool_xml.getpath(validator)) - if vtype == "expression" and validator.text is None: - lint_ctx.error(f"Parameter [{param_name}]: expression validator without content", line=validator.sourceline, xpath=tool_xml.getpath(validator)) + lint_ctx.error(f"Parameter [{param_name}]: attribute '{attrib}' is incompatible with validator of type '{vtype}'", **node_props(validator, tool_xml)) + if vtype == "expression": + if validator.text is None: + lint_ctx.error(f"Parameter [{param_name}]: expression validators are expected to contain text", **node_props(validator, tool_xml)) + else: + try: + re.compile(validator.text) + except Exception as e: + lint_ctx.error(f"Parameter [{param_name}]: '{validator.text}' is no valid regular expression: {str(e)}", **node_props(validator, tool_xml)) if vtype not in ["expression", "regex"] and validator.text is not None: - lint_ctx.warn(f"Parameter [{param_name}]: '{vtype}' validators are not expected to contain text (found '{validator.text}')", line=validator.sourceline, xpath=tool_xml.getpath(validator)) + lint_ctx.warn(f"Parameter [{param_name}]: '{vtype}' validators are not expected to contain text (found '{validator.text}')", **node_props(validator, tool_xml)) if vtype in ["in_range", "length", "dataset_metadata_in_range"] and ("min" not in validator.attrib and "max" not in validator.attrib): - lint_ctx.error(f"Parameter [{param_name}]: '{vtype}' validators need to define the 'min' or 'max' attribute(s)", line=validator.sourceline, xpath=tool_xml.getpath(validator)) + lint_ctx.error(f"Parameter [{param_name}]: '{vtype}' validators need to define the 'min' or 'max' attribute(s)", **node_props(validator, tool_xml)) if vtype in ["metadata"] and ("check" not in validator.attrib and "skip" not in validator.attrib): - lint_ctx.error(f"Parameter [{param_name}]: '{vtype}' validators need to define the 'check' or 'skip' attribute(s)", line=validator.sourceline, xpath=tool_xml.getpath(validator)) + lint_ctx.error(f"Parameter [{param_name}]: '{vtype}' validators need to define the 'check' or 'skip' attribute(s)", **node_props(validator, tool_xml)) if vtype in ["value_in_data_table", "value_not_in_data_table", "dataset_metadata_in_data_table", "dataset_metadata_not_in_data_table"] and "table_name" not in validator.attrib: - lint_ctx.error(f"Parameter [{param_name}]: '{vtype}' validators need to define the 'table_name' attribute", line=validator.sourceline, xpath=tool_xml.getpath(validator)) + lint_ctx.error(f"Parameter [{param_name}]: '{vtype}' validators need to define the 'table_name' attribute", **node_props(validator, tool_xml)) conditional_selects = tool_xml.findall("./inputs//conditional") for conditional in conditional_selects: conditional_name = conditional.get('name') if not conditional_name: - lint_ctx.error("Conditional without a name", line=conditional.sourceline, xpath=tool_xml.getpath(conditional)) + lint_ctx.error("Conditional without a name", **node_props(conditional, tool_xml)) 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)}", line=conditional.sourceline, xpath=tool_xml.getpath(conditional)) + lint_ctx.error(f"Conditional [{conditional_name}] needs exactly one child found {len(first_param)}", **node_props(conditional, tool_xml)) continue first_param = first_param[0] first_param_type = first_param.get('type') - if first_param_type not in ['select', 'boolean']: - lint_ctx.error(f'Conditional [{conditional_name}] first param should have type="select" (or type="boolean" which is discouraged)', line=first_param.sourceline, xpath=tool_xml.getpath(first_param)) + if first_param_type == 'boolean': + lint_ctx.warn(f'Conditional [{conditional_name}] first param of type="boolean" is discouraged, use a select', **node_props(first_param, tool_xml)) + elif first_param_type != 'select': + lint_ctx.error(f'Conditional [{conditional_name}] first param should have type="select"', **node_props(first_param, tool_xml)) continue - elif first_param_type == 'boolean': - lint_ctx.warn(f'Conditional [{conditional_name}] first param of type="boolean" is discouraged, use a select', line=first_param.sourceline, xpath=tool_xml.getpath(first_param)) if first_param_type == 'select': select_options = _find_with_attribute(first_param, 'option', 'value') @@ -242,38 +241,38 @@ def lint_inputs(tool_xml, lint_ctx): for incomp in ["optional", "multiple"]: if string_as_bool(first_param.get(incomp, False)): - lint_ctx.warn(f'Conditional [{conditional_name}] test parameter cannot be {incomp}="true"', line=first_param.sourceline, xpath=tool_xml.getpath(first_param)) + lint_ctx.warn(f'Conditional [{conditional_name}] test parameter cannot be {incomp}="true"', **node_props(first_param, tool_xml)) 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", line=conditional.sourceline, xpath=tool_xml.getpath(conditional)) + lint_ctx.error(f"Conditional [{conditional_name}] when without value", **node_props(conditional, tool_xml)) when_ids = [w.get('value') for w in whens if w.get('value') is not None] for option_id in option_ids: if option_id not in when_ids: - lint_ctx.warn(f"Conditional [{conditional_name}] no block found for {first_param_type} option '{option_id}'", line=conditional.sourceline, xpath=tool_xml.getpath(conditional)) + lint_ctx.warn(f"Conditional [{conditional_name}] no block found for {first_param_type} option '{option_id}'", **node_props(conditional, tool_xml)) for when_id in when_ids: if when_id not in option_ids: if first_param_type == 'select': - lint_ctx.warn(f"Conditional [{conditional_name}] no """ -TESTS = [ - ( - CITATIONS_MULTIPLE, citations.lint_citations, - lambda x: - "More than one citation section found, behavior undefined." in x.error_messages - and len(x.info_messages) == 0 and len(x.valid_messages) == 0 and len(x.warn_messages) == 0 and len(x.error_messages) == 1 - ), - ( - CITATIONS_ABSENT, citations.lint_citations, - lambda x: - "No citations found, consider adding citations to your tool." in x.warn_messages - and len(x.info_messages) == 0 and len(x.valid_messages) == 0 and len(x.warn_messages) == 1 and len(x.error_messages) == 0 - ), - ( - CITATIONS_ERRORS, citations.lint_citations, - lambda x: - "Unknown tag discovered in citations block [nonsense], will be ignored." in x.warn_messages - and "Unknown citation type discovered [hoerensagen], will be ignored." in x.warn_messages - and 'Empty doi citation.' in x.error_messages - and 'Found no valid citations.' in x.warn_messages - and len(x.info_messages) == 0 and len(x.valid_messages) == 0 and len(x.warn_messages) == 3 and len(x.error_messages) == 1 - ), - ( - CITATIONS_VALID, citations.lint_citations, - lambda x: - 'Found 1 likely valid citations.' in x.valid_messages - and len(x.info_messages) == 0 and len(x.valid_messages) == 1 and len(x.warn_messages) == 0 and len(x.error_messages) == 0 - ), - ( - COMMAND_MULTIPLE, command.lint_command, - lambda x: - 'More than one command tag found, behavior undefined.' in x.error_messages - and len(x.info_messages) == 0 and len(x.valid_messages) == 0 and len(x.warn_messages) == 0 and len(x.error_messages) == 1 - ), - ( - COMMAND_MISSING, command.lint_command, - lambda x: - 'No command tag found, must specify a command template to execute.' in x.error_messages - and len(x.info_messages) == 0 and len(x.valid_messages) == 0 and len(x.warn_messages) == 0 and len(x.error_messages) == 1 - ), - ( - COMMAND_TODO, command.lint_command, - lambda x: - 'Tool contains a command.' in x.info_messages - and 'Command template contains TODO text.' in x.warn_messages - and len(x.info_messages) == 1 and len(x.valid_messages) == 0 and len(x.warn_messages) == 1 and len(x.error_messages) == 0 - ), - ( - COMMAND_DETECT_ERRORS_INTERPRETER, command.lint_command, - lambda x: - "Command uses deprecated 'interpreter' attribute." in x.warn_messages - and 'Tool contains a command with interpreter of type [python].' in x.info_messages - and 'Unknown detect_errors attribute [nonsense]' in x.warn_messages - and 'Command is empty.' in x.error_messages - and len(x.info_messages) == 1 and len(x.valid_messages) == 0 and len(x.warn_messages) == 2 and len(x.error_messages) == 1 - ), - ( - GENERAL_MISSING_TOOL_ID_NAME_VERSION, general.lint_general, - lambda x: - 'Tool version is missing or empty.' in x.error_messages - and 'Tool name is missing or empty.' in x.error_messages - and 'Tool does not define an id attribute.' in x.error_messages - and 'Tool specifies an invalid profile version [2109].' in x.error_messages - and len(x.info_messages) == 0 and len(x.valid_messages) == 0 and len(x.warn_messages) == 0 and len(x.error_messages) == 4 - ), - ( - GENERAL_WHITESPACE_IN_VERSIONS_AND_NAMES, general.lint_general, - lambda x: - "Tool version is pre/suffixed by whitespace, this may cause errors: [ 1.0.1 ]." in x.warn_messages - and "Tool name is pre/suffixed by whitespace, this may cause errors: [ BWA Mapper ]." in x.warn_messages - and "Requirement version contains whitespace, this may cause errors: [ 1.2.5 ]." in x.warn_messages - and "Tool ID contains whitespace - this is discouraged: [bwa tool]." in x.warn_messages - and "Tool targets 16.01 Galaxy profile." in x.valid_messages - and len(x.info_messages) == 0 and len(x.valid_messages) == 1 and len(x.warn_messages) == 4 and len(x.error_messages) == 0 - ), - ( - GENERAL_REQUIREMENT_WO_VERSION, general.lint_general, - lambda x: - 'Tool version [1.0.1blah] is not compliant with PEP 440.' in x.warn_messages - and "Requirement bwa defines no version" in x.warn_messages - and "Requirement without name found" in x.error_messages - and "Tool specifies profile version [20.09]." in x.valid_messages - and "Tool defines an id [bwa_tool]." in x.valid_messages - and "Tool defines a name [BWA Mapper]." in x.valid_messages - and len(x.info_messages) == 0 and len(x.valid_messages) == 3 and len(x.warn_messages) == 2 and len(x.error_messages) == 1 - ), - ( - GENERAL_VALID, general.lint_general, - lambda x: - 'Tool defines a version [1.0+galaxy1].' in x.valid_messages - and "Tool specifies profile version [21.09]." in x.valid_messages - and "Tool defines an id [valid_id]." in x.valid_messages - and "Tool defines a name [valid name]." in x.valid_messages - and len(x.info_messages) == 0 and len(x.valid_messages) == 4 and len(x.warn_messages) == 0 and len(x.error_messages) == 0 - ), - ( - HELP_MULTIPLE, help.lint_help, - lambda x: - 'More than one help section found, behavior undefined.' in x.error_messages - and len(x.info_messages) == 0 and len(x.valid_messages) == 0 and len(x.warn_messages) == 0 and len(x.error_messages) == 1 - ), - ( - HELP_ABSENT, help.lint_help, - lambda x: - 'No help section found, consider adding a help section to your tool.' in x.warn_messages - and len(x.info_messages) == 0 and len(x.valid_messages) == 0 and len(x.warn_messages) == 1 and len(x.error_messages) == 0 - ), - ( - HELP_EMPTY, help.lint_help, - lambda x: - 'Help section appears to be empty.' in x.warn_messages - and len(x.info_messages) == 0 and len(x.valid_messages) == 0 and len(x.warn_messages) == 1 and len(x.error_messages) == 0 - ), - ( - HELP_TODO, help.lint_help, - lambda x: - 'Tool contains help section.' in x.valid_messages - and 'Help contains valid reStructuredText.' in x.valid_messages - and "Help contains TODO text." in x.warn_messages - and len(x.info_messages) == 0 and len(x.valid_messages) == 2 and len(x.warn_messages) == 1 and len(x.error_messages) == 0 - ), - ( - HELP_INVALID_RST, help.lint_help, - lambda x: - 'Tool contains help section.' in x.valid_messages - and "Invalid reStructuredText found in help - [:2: (WARNING/2) Inline strong start-string without end-string.\n]." in x.warn_messages - and len(x.info_messages) == 0 and len(x.valid_messages) == 1 and len(x.warn_messages) == 1 and len(x.error_messages) == 0 - ), - ( - INPUTS_NO_INPUTS, inputs.lint_inputs, - lambda x: - 'Found no input parameters.' in x.warn_messages - and len(x.info_messages) == 0 and len(x.valid_messages) == 0 and len(x.warn_messages) == 1 and len(x.error_messages) == 0 - ), - ( - INPUTS_NO_INPUTS_DATASOURCE, inputs.lint_inputs, - lambda x: - 'No input parameters, OK for data sources' in x.info_messages - and 'display tag usually present in data sources' in x.info_messages - and 'uihints tag usually present in data sources' in x.info_messages - and len(x.info_messages) == 3 and len(x.valid_messages) == 0 and len(x.warn_messages) == 0 and len(x.error_messages) == 0 - ), - ( - INPUTS_VALID, inputs.lint_inputs, - lambda x: - "Found 2 input parameters." in x.info_messages - and len(x.info_messages) == 1 and len(x.valid_messages) == 0 and len(x.warn_messages) == 0 and len(x.error_messages) == 0 - ), - ( - INPUTS_PARAM_NAME, inputs.lint_inputs, - lambda x: - "Found 5 input parameters." in x.info_messages - and 'Param input [2] is not a valid Cheetah placeholder.' in x.warn_messages - and 'Found param input with no name specified.' in x.error_messages - and 'Param input with empty name.' in x.error_messages - and "Param input [param_name] 'name' attribute is redundant if argument implies the same name." in x.warn_messages - and len(x.info_messages) == 1 and len(x.valid_messages) == 0 and len(x.warn_messages) == 2 and len(x.error_messages) == 2 - ), - ( - INPUTS_PARAM_TYPE, inputs.lint_inputs, - lambda x: - "Found 2 input parameters." in x.info_messages - and 'Param input [valid_name] input with no type specified.' in x.error_messages - and 'Param input [another_valid_name] with empty type specified.' in x.error_messages - and len(x.info_messages) == 1 and len(x.valid_messages) == 0 and len(x.warn_messages) == 0 and len(x.error_messages) == 2 - ), - ( - INPUTS_DATA_PARAM, inputs.lint_inputs, - lambda x: - "Found 1 input parameters." in x.info_messages - and "Param input [valid_name] with no format specified - 'data' format will be assumed." in x.warn_messages - and len(x.info_messages) == 1 and len(x.valid_messages) == 0 and len(x.warn_messages) == 1 and len(x.error_messages) == 0 - ), - ( - INPUTS_CONDITIONAL, inputs.lint_inputs, - lambda x: - 'Found 10 input parameters.' in x.info_messages - and "Conditional without a name" in x.error_messages - and "Select parameter of a conditional [select] options have to be defined by 'option' children elements." in x.error_messages - and 'Conditional [cond_wo_param] needs exactly one child found 0' in x.error_messages - and 'Conditional [cond_w_mult_param] needs exactly one child found 2' in x.error_messages - and 'Conditional [cond_text] first param should have type="select"' in x.error_messages - and 'Conditional [cond_boolean] first param of type="boolean" is discouraged, use a select' in x.warn_messages - and "Conditional [cond_boolean] no truevalue/falsevalue found for when block 'False'" in x.warn_messages - and 'Conditional [cond_w_optional_select] test parameter cannot be optional="true"' in x.warn_messages - and 'Conditional [cond_w_multiple_select] test parameter cannot be multiple="true"' in x.warn_messages - and "Conditional [when_wo_value] when without value" in x.error_messages - and "Conditional [missing_when] no block found for select option 'none'" in x.warn_messages - and len(x.info_messages) == 1 and len(x.valid_messages) == 0 and len(x.warn_messages) == 6 and len(x.error_messages) == 6 - ), - ( - INPUTS_SELECT_INCOMPATIBLE_DISPLAY, inputs.lint_inputs, - lambda x: - 'Found 3 input parameters.' in x.info_messages - and 'Select [radio_select] display="radio" is incompatible with optional="true"' in x.error_messages - and 'Select [radio_select] display="radio" is incompatible with multiple="true"' in x.error_messages - and 'Select [checkboxes_select] `display="checkboxes"` is incompatible with `optional="false"`, remove the `display` attribute' in x.error_messages - and 'Select [checkboxes_select] `display="checkboxes"` is incompatible with `multiple="false"`, remove the `display` attribute' in x.error_messages - and len(x.info_messages) == 1 and len(x.valid_messages) == 0 and len(x.warn_messages) == 0 and len(x.error_messages) == 4 - ), - ( - INPUTS_SELECT_DUPLICATED_OPTIONS, inputs.lint_inputs, - lambda x: - 'Found 1 input parameters.' in x.info_messages - and 'Select parameter [select] has multiple options with the same text content' in x.error_messages - and 'Select parameter [select] has multiple options with the same value' in x.error_messages - and len(x.info_messages) == 1 and len(x.valid_messages) == 0 and len(x.warn_messages) == 0 and len(x.error_messages) == 2 - ), - ( - SELECT_DUPLICATED_OPTIONS_WITH_DIFF_SELECTED, inputs.lint_inputs, - lambda x: - len(x.warn_messages) == 0 and len(x.error_messages) == 0 - ), - ( - INPUTS_SELECT_DEPRECATIONS, inputs.lint_inputs, - lambda x: - 'Found 3 input parameters.' in x.info_messages - and "Select parameter [select_do] uses deprecated 'dynamic_options' attribute." in x.warn_messages - and "Select parameter [select_ff] options uses deprecated 'from_file' attribute." in x.warn_messages - and "Select parameter [select_fp] options uses deprecated 'from_parameter' attribute." in x.warn_messages - and "Select parameter [select_ff] options uses deprecated 'transform_lines' attribute." in x.warn_messages - and "Select parameter [select_fp] options uses deprecated 'options_filter_attribute' attribute." in x.warn_messages - and len(x.info_messages) == 1 and len(x.valid_messages) == 0 and len(x.warn_messages) == 5 and len(x.error_messages) == 0 - ), - ( - INPUTS_SELECT_OPTION_DEFINITIONS, inputs.lint_inputs, - lambda x: - 'Found 6 input parameters.' in x.info_messages - and "Select parameter [select_noopt] options have to be defined by either 'option' children elements, a 'options' element or the 'dynamic_options' attribute." in x.error_messages - and "Select parameter [select_noopts] options tag defines no options. Use 'from_dataset', 'from_data_table', or a filter that adds values." in x.error_messages - and "Select parameter [select_fd_op] options have to be defined by either 'option' children elements, a 'options' element or the 'dynamic_options' attribute." in x.error_messages - and "Select parameter [select_fd_op] contains multiple options elements." in x.error_messages - and "Select parameter [select_fd_fdt] options uses 'from_dataset' and 'from_data_table' attribute." in x.error_messages - and "Select parameter [select_noval_notext] has option without value" in x.error_messages - and "Select parameter [select_noval_notext] has option without text" in x.warn_messages - and "Select parameter [select_meta_file_key_incomp] 'meta_file_key' is only compatible with 'from_dataset'." in x.error_messages - and len(x.info_messages) == 1 and len(x.valid_messages) == 0 and len(x.warn_messages) == 1 and len(x.error_messages) == 7 - ), - ( - INPUTS_SELECT_FILTER, inputs.lint_inputs, - lambda x: - 'Found 1 input parameters.' in x.info_messages - and "Select parameter [select_filter_types] contains filter without type." in x.error_messages - and "Select parameter [select_filter_types] contains filter with unknown type 'unknown_filter_type'." in x.error_messages - and len(x.info_messages) == 1 and len(x.valid_messages) == 0 and len(x.warn_messages) == 0 and len(x.error_messages) == 2 - ), - ( - INPUTS_VALIDATOR_INCOMPATIBILITIES, inputs.lint_inputs, - lambda x: - 'Found 2 input parameters.' in x.info_messages - and "Parameter [param_name]: 'in_range' validators are not expected to contain text (found 'TEXT')" in x.warn_messages - and "Parameter [param_name]: validator with an incompatible type 'in_range'" in x.error_messages - and "Parameter [param_name]: 'in_range' validators need to define the 'min' or 'max' attribute(s)" in x.error_messages - and "Parameter [param_name]: attribute 'filename' is incompatible with validator of type 'regex'" in x.error_messages - and "Parameter [param_name]: expression validators are expected to contain text" in x.error_messages - and "Parameter [param_name]: '[' is no valid regular expression: unterminated character set at position 0" in x.error_messages - and "Parameter [another_param_name]: 'metadata' validators need to define the 'check' or 'skip' attribute(s)" in x.error_messages - and "Parameter [param_name]: 'value_in_data_table' validators need to define the 'table_name' attribute" in x.error_messages - and len(x.info_messages) == 1 and len(x.valid_messages) == 0 and len(x.warn_messages) == 1 and len(x.error_messages) == 7 - ), - ( - INPUTS_VALIDATOR_CORRECT, inputs.lint_inputs, - lambda x: - 'Found 5 input parameters.' in x.info_messages - and len(x.info_messages) == 1 and len(x.valid_messages) == 0 and len(x.warn_messages) == 0 and len(x.error_messages) == 0 - ), - ( - REPEATS, inputs.lint_repeats, - lambda x: - "Repeat does not specify name attribute." in x.error_messages - and "Repeat does not specify title attribute." in x.error_messages - and len(x.info_messages) == 0 and len(x.valid_messages) == 0 and len(x.warn_messages) == 0 and len(x.error_messages) == 2 - ), - ( - OUTPUTS_MISSING, outputs.lint_output, - lambda x: - 'Tool contains no outputs section, most tools should produce outputs.' in x.warn_messages - and len(x.info_messages) == 0 and len(x.valid_messages) == 0 and len(x.warn_messages) == 1 and len(x.error_messages) == 0 - ), - ( - OUTPUTS_MULTIPLE, outputs.lint_output, - lambda x: - '0 outputs found.' in x.info_messages - and 'Tool contains multiple output sections, behavior undefined.' in x.warn_messages - and len(x.info_messages) == 1 and len(x.valid_messages) == 0 and len(x.warn_messages) == 1 and len(x.error_messages) == 0 - ), - ( - OUTPUTS_UNKNOWN_TAG, outputs.lint_output, - lambda x: - '0 outputs found.' in x.info_messages - and 'Unknown element found in outputs [output]' in x.warn_messages - and len(x.info_messages) == 1 and len(x.valid_messages) == 0 and len(x.warn_messages) == 1 and len(x.error_messages) == 0 - ), - ( - OUTPUTS_UNNAMED_INVALID_NAME, outputs.lint_output, - lambda x: - '2 outputs found.' in x.info_messages - and "Tool output doesn't define a name - this is likely a problem." in x.warn_messages - and "Tool data output with missing name doesn't define an output format." in x.warn_messages - and 'Tool output name [2output] is not a valid Cheetah placeholder.' in x.warn_messages - and "Collection output with undefined 'type' found." in x.warn_messages - and "Tool collection output 2output doesn't define an output format." in x.warn_messages - and len(x.info_messages) == 1 and len(x.valid_messages) == 0 and len(x.warn_messages) == 5 and len(x.error_messages) == 0 - ), - ( - OUTPUTS_FORMAT_INPUT, outputs.lint_output, - lambda x: - '1 outputs found.' in x.info_messages - and "Using format='input' on data, format_source attribute is less ambiguous and should be used instead." in x.warn_messages - and len(x.info_messages) == 1 and len(x.valid_messages) == 0 and len(x.warn_messages) == 1 and len(x.error_messages) == 0 - ), - ( - OUTPUTS_COLLECTION_FORMAT_SOURCE, outputs.lint_output, - lambda x: - "Tool data output 'reverse' should use either format_source or format/ext" in x.warn_messages - and len(x.info_messages) == 1 and len(x.valid_messages) == 0 and len(x.warn_messages) == 1 and len(x.error_messages) == 0 - ), - ( - OUTPUTS_DISCOVER_TOOL_PROVIDED_METADATA, outputs.lint_output, - lambda x: - '1 outputs found.' in x.info_messages - and len(x.info_messages) == 1 and len(x.valid_messages) == 0 and len(x.warn_messages) == 0 and len(x.error_messages) == 0 - ), - ( - ASSERTS, tests.lint_tsts, - lambda x: - 'Test 1: unknown assertion invalid' in x.error_messages - and 'Test 1: unknown attribute invalid_attrib for has_text' in x.error_messages - and 'Test 1: missing attribute text for has_text' in x.error_messages - and 'Test 1: attribute value for has_size needs to be int got 500k' in x.error_messages - and 'Test 1: attribute delta for has_size needs to be int got 1O' in x.error_messages - and 'Test 1: unknown attribute invalid_attrib_also_checked_in_nested_asserts for not_has_text' in x.error_messages - and "Test 1: has_size needs to specify 'n', 'min', or 'max'" in x.error_messages - and "Test 1: has_n_columns needs to specify 'n', 'min', or 'max'" in x.error_messages - and "Test 1: has_n_lines needs to specify 'n', 'min', or 'max'" in x.error_messages - and len(x.warn_messages) == 0 and len(x.error_messages) == 9 - ), - ( - REPEATS, inputs.lint_repeats, - lambda x: - "Repeat does not specify name attribute." in x.error_messages - and "Repeat does not specify title attribute." in x.error_messages - and len(x.info_messages) == 0 and len(x.valid_messages) == 0 and len(x.warn_messages) == 0 and len(x.error_messages) == 2 - ), - ( - STDIO_DEFAULT_FOR_DEFAULT_PROFILE, stdio.lint_stdio, - lambda x: - "No stdio definition found, tool indicates error conditions with output written to stderr." in x.info_messages - and len(x.info_messages) == 1 and len(x.valid_messages) == 0 and len(x.warn_messages) == 0 and len(x.error_messages) == 0 - - ), - ( - STDIO_DEFAULT_FOR_NONLEGACY_PROFILE, stdio.lint_stdio, - lambda x: - "No stdio definition found, tool indicates error conditions with non-zero exit codes." in x.info_messages - and len(x.info_messages) == 1 and len(x.valid_messages) == 0 and len(x.warn_messages) == 0 and len(x.error_messages) == 0 - - ), - ( - STDIO_MULTIPLE_STDIO, stdio.lint_stdio, - lambda x: - "More than one stdio tag found, behavior undefined." in x.error_messages - and len(x.info_messages) == 0 and len(x.valid_messages) == 0 and len(x.warn_messages) == 0 and len(x.error_messages) == 1 - - ), - ( - STDIO_INVALID_CHILD_OR_ATTRIB, stdio.lint_stdio, - lambda x: - "Unknown stdio child tag discovered [reqex]. Valid options are exit_code and regex." in x.warn_messages - and "Unknown attribute [descriptio] encountered on exit_code tag." in x.warn_messages - and "Unknown attribute [descriptio] encountered on regex tag." in x.warn_messages - and len(x.info_messages) == 0 and len(x.valid_messages) == 0 and len(x.warn_messages) == 3 and len(x.error_messages) == 0 - ), - ( - STDIO_INVALID_MATCH, stdio.lint_stdio, - lambda x: - "Match '[' is no valid regular expression: unterminated character set at position 0" in x.error_messages - and len(x.info_messages) == 0 and len(x.valid_messages) == 0 and len(x.warn_messages) == 0 and len(x.error_messages) == 1 - ), - ( - TESTS_ABSENT, tests.lint_tsts, - lambda x: - 'No tests found, most tools should define test cases.' in x.warn_messages - and len(x.info_messages) == 0 and len(x.valid_messages) == 0 and len(x.warn_messages) == 1 and len(x.error_messages) == 0 - ), - ( - TESTS_ABSENT_DATA_SOURCE, tests.lint_tsts, - lambda x: - 'No tests found, that should be OK for data_sources.' in x.info_messages - and len(x.info_messages) == 1 and len(x.valid_messages) == 0 and len(x.warn_messages) == 0 and len(x.error_messages) == 0 - ), - ( - TESTS_PARAM_OUTPUT_NAMES, tests.lint_tsts, - lambda x: - '1 test(s) found.' in x.valid_messages - and "Test 1: Found test param tag without a name defined." in x.error_messages - and "Test 1: Test param non_existent_test_name not found in the inputs" in x.error_messages - and "Test 1: Found output tag without a name defined." in x.error_messages - and "Test 1: Found output tag with unknown name [nonexistent_output], valid names [['existent_output']]" in x.error_messages - and "Test 1: Found output_collection tag without a name defined." in x.error_messages - and "Test 1: Found output_collection tag with unknown name [nonexistent_collection], valid names [['existent_collection']]" in x.error_messages - and len(x.info_messages) == 0 and len(x.valid_messages) == 1 and len(x.warn_messages) == 0 and len(x.error_messages) == 6 - ), - ( - TESTS_EXPECT_FAILURE_OUTPUT, tests.lint_tsts, - lambda x: - 'No valid test(s) found.' in x.warn_messages - and "Test 1: Cannot specify outputs in a test expecting failure." in x.error_messages - and len(x.info_messages) == 0 and len(x.valid_messages) == 0 and len(x.warn_messages) == 1 and len(x.error_messages) == 1 - ), - ( - TESTS_WO_EXPECTATIONS, tests.lint_tsts, - lambda x: - 'Test 1: No outputs or expectations defined for tests, this test is likely invalid.' in x.warn_messages - and 'No valid test(s) found.' in x.warn_messages - and len(x.info_messages) == 0 and len(x.valid_messages) == 0 and len(x.warn_messages) == 2 and len(x.error_messages) == 0 - ), - ( - TESTS_VALID, tests.lint_tsts, - lambda x: - "1 test(s) found." in x.valid_messages - and len(x.info_messages) == 0 and len(x.valid_messages) == 1 and len(x.warn_messages) == 0 and len(x.error_messages) == 0 - ), - ( - XML_ORDER, xml_order.lint_xml_order, - lambda x: - 'Unknown tag [wrong_tag] encountered, this may result in a warning in the future.' in x.info_messages - and 'Best practice violation [stdio] elements should come before [command]' in x.warn_messages - and len(x.info_messages) == 1 and len(x.valid_messages) == 0 and len(x.warn_messages) == 1 and len(x.error_messages) == 0 - ), -] - -TEST_IDS = [ - 'citations: multiple', - 'citations: absent', - 'citations: errors', - 'citations: valid', - 'command: multiple', - 'command: missing', - 'command: todo', - 'command: detect_errors and interpreter', - 'general: missing tool id, name, version; invalid profile', - 'general: whitespace in version, id, name', - 'general: requirement without version', - 'general: valid name, id, profile', - 'help: multiple', - 'help: absent', - 'help: empty', - 'help: with todo', - 'help: with invalid restructured text', - 'inputs: no inputs sections', - 'inputs: no inputs sections for datasource', - 'inputs: valid', - 'inputs: param name', - 'inputs: param type', - 'inputs: data param', - 'inputs: conditional', - 'inputs: select with incompatible display', - 'inputs: select duplicated options', - 'inputs: select duplicated options with different selected', - 'inputs: select deprecations', - 'inputs: select option definitions', - 'inputs: select filter', - 'inputs: validator incompatibilities', - 'inputs: validator all correct', - 'repeats', - 'outputs: missing outputs tag', - 'outputs: multiple outputs tags', - 'outputs: unknow tag in outputs', - 'outputs: unnamed or invalid output', - 'outputs: format="input"', - 'outputs collection static elements with format_source', - 'outputs discover datatsets with tool provided metadata', - 'outputs: asserts', - 'repeats', - 'stdio: default for default profile', - 'stdio: default for non-legacy profile', - 'stdio: multiple stdio', - 'stdio: invalid tag or attribute', - 'stdio: invalid regular expression', - 'tests: absent', - 'tests: absent data_source', - 'tests: param and output names', - 'tests: expecting failure with outputs', - 'tests: without expectations', - 'tests: valid', - 'xml_order' -] - - -@pytest.mark.parametrize('tool_xml,lint_func,assert_func', TESTS, ids=TEST_IDS) -def test_tool_xml(tool_xml, lint_func, assert_func): - """ - test separate linting functions (lint_func) on tool_xml - checking assert_func on the resulting LinterContext - furthermore for all lint_func except for lint_general it is asserted - that each message has a complete lint context message (i.e. a line number) - """ - lint_ctx = LintContext('all') - # the general linter gets XMLToolSource and all others - # an ElementTree - first_arg = getfullargspec(lint_func).args[0] - with tempfile.TemporaryDirectory() as tmp: - tool_path = os.path.join(tmp, "tool.xml") - with open(tool_path, "w") as tmpf: - tmpf.write(tool_xml) - lint_target, _ = load_with_references(tool_path) - if first_arg != "tool_xml": - lint_target = XmlToolSource(lint_target) +@pytest.fixture() +def lint_ctx(): + return LintContext('all') + + +def get_xml_tool_source(xml_string): + with tempfile.NamedTemporaryFile(mode="w", suffix="tool.xml") as tmp: + tmp.write(xml_string) + tmp.flush() + tool_path = tmp.name + return load_with_references(tool_path)[0] + + +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 can be pretty printed + # and if there are line numbers (except for the general linter + # which can not generate line numbers) for message in lint_ctx.message_list: message.pretty_print(level=True, fname=True, xpath=True) # TODO would be nice if lint_general would have full context as well if lint_func != general.lint_general: assert message.line is not None, f"No context found for message: {message.message}" - assert assert_func(lint_ctx), ( - 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 test_citations_multiple(lint_ctx): + tool_source = get_xml_tool_source(CITATIONS_MULTIPLE) + run_lint(lint_ctx, citations.lint_citations, tool_source) + assert "More than one citation section found, behavior undefined." in lint_ctx.error_messages + assert not lint_ctx.info_messages + assert not lint_ctx.valid_messages + assert not lint_ctx.warn_messages + assert len(lint_ctx.error_messages) == 1 + + +def test_citations_absent(lint_ctx): + tool_source = get_xml_tool_source(CITATIONS_ABSENT) + run_lint(lint_ctx, citations.lint_citations, tool_source) + assert lint_ctx.warn_messages == ["No citations found, consider adding citations to your tool."] + assert not lint_ctx.info_messages + assert not lint_ctx.valid_messages + assert not lint_ctx.error_messages + + +def test_citations_errors(lint_ctx): + tool_source = get_xml_tool_source(CITATIONS_ERRORS) + run_lint(lint_ctx, citations.lint_citations, tool_source) + assert "Unknown tag discovered in citations block [nonsense], will be ignored." in lint_ctx.warn_messages + assert "Unknown citation type discovered [hoerensagen], will be ignored." in lint_ctx.warn_messages + assert 'Empty doi citation.' in lint_ctx.error_messages + assert 'Found no valid citations.' in lint_ctx.warn_messages + assert len(lint_ctx.warn_messages) == 3 + assert not lint_ctx.info_messages + assert not lint_ctx.valid_messages + + +def test_citations_valid(lint_ctx): + tool_source = get_xml_tool_source(CITATIONS_VALID) + run_lint(lint_ctx, citations.lint_citations, tool_source) + assert 'Found 1 likely valid citations.' in lint_ctx.valid_messages + assert len(lint_ctx.valid_messages) == 1 + assert not lint_ctx.info_messages + assert not lint_ctx.error_messages + + +def test_command_multiple(lint_ctx): + tool_source = get_xml_tool_source(COMMAND_MULTIPLE) + run_lint(lint_ctx, command.lint_command, tool_source) + assert 'More than one command tag found, behavior undefined.' in lint_ctx.error_messages + assert len(lint_ctx.error_messages) == 1 + assert not lint_ctx.info_messages + assert not lint_ctx.valid_messages + assert not lint_ctx.warn_messages + + +def test_command_missing(lint_ctx): + tool_source = get_xml_tool_source(COMMAND_MISSING) + run_lint(lint_ctx, command.lint_command, tool_source) + assert 'No command tag found, must specify a command template to execute.' in lint_ctx.error_messages + + +def test_command_todo(lint_ctx): + tool_source = get_xml_tool_source(COMMAND_TODO) + run_lint(lint_ctx, command.lint_command, tool_source) + assert 'Tool contains a command.' in lint_ctx.info_messages + assert 'Command template contains TODO text.' in lint_ctx.warn_messages + + +def test_command_detect_errors_interpreter(lint_ctx): + tool_source = get_xml_tool_source(COMMAND_DETECT_ERRORS_INTERPRETER) + run_lint(lint_ctx, command.lint_command, tool_source) + assert "Command uses deprecated 'interpreter' attribute." in lint_ctx.warn_messages + assert 'Tool contains a command with interpreter of type [python].' in lint_ctx.info_messages + assert 'Unknown detect_errors attribute [nonsense]' in lint_ctx.warn_messages + assert 'Command is empty.' in lint_ctx.error_messages + + +def test_general_missing_tool_id_name_version(lint_ctx): + tool_source = get_xml_tool_source(GENERAL_MISSING_TOOL_ID_NAME_VERSION) + run_lint(lint_ctx, general.lint_general, XmlToolSource(tool_source)) + assert 'Tool version is missing or empty.' in lint_ctx.error_messages + assert 'Tool name is missing or empty.' in lint_ctx.error_messages + assert 'Tool does not define an id attribute.' in lint_ctx.error_messages + assert 'Tool specifies an invalid profile version [2109].' in lint_ctx.error_messages + + +def test_general_whitespace_in_versions_and_names(lint_ctx): + tool_source = get_xml_tool_source(GENERAL_WHITESPACE_IN_VERSIONS_AND_NAMES) + run_lint(lint_ctx, general.lint_general, XmlToolSource(tool_source)) + assert "Tool version is pre/suffixed by whitespace, this may cause errors: [ 1.0.1 ]." in lint_ctx.warn_messages + assert "Tool name is pre/suffixed by whitespace, this may cause errors: [ BWA Mapper ]." in lint_ctx.warn_messages + assert "Requirement version contains whitespace, this may cause errors: [ 1.2.5 ]." in lint_ctx.warn_messages + assert "Tool ID contains whitespace - this is discouraged: [bwa tool]." in lint_ctx.warn_messages + assert "Tool targets 16.01 Galaxy profile." in lint_ctx.valid_messages + + +def test_general_requirement_without_version(lint_ctx): + tool_source = get_xml_tool_source(GENERAL_REQUIREMENT_WO_VERSION) + run_lint(lint_ctx, general.lint_general, XmlToolSource(tool_source)) + assert 'Tool version [1.0.1blah] is not compliant with PEP 440.' in lint_ctx.warn_messages + assert "Requirement bwa defines no version" in lint_ctx.warn_messages + assert "Requirement without name found" in lint_ctx.error_messages + assert "Tool specifies profile version [20.09]." in lint_ctx.valid_messages + assert "Tool defines an id [bwa_tool]." in lint_ctx.valid_messages + assert "Tool defines a name [BWA Mapper]." in lint_ctx.valid_messages + assert not lint_ctx.info_messages + assert len(lint_ctx.valid_messages) == 3 + assert len(lint_ctx.warn_messages) == 2 + assert len(lint_ctx.error_messages) == 1 + + +def test_general_valid(lint_ctx): + tool_source = get_xml_tool_source(GENERAL_VALID) + run_lint(lint_ctx, general.lint_general, XmlToolSource(tool_source)) + assert 'Tool defines a version [1.0+galaxy1].' in lint_ctx.valid_messages + assert "Tool specifies profile version [21.09]." in lint_ctx.valid_messages + assert "Tool defines an id [valid_id]." in lint_ctx.valid_messages + assert "Tool defines a name [valid name]." in lint_ctx.valid_messages + assert not lint_ctx.info_messages + assert len(lint_ctx.valid_messages) == 4 + assert not lint_ctx.warn_messages + assert not lint_ctx.error_messages + + +def test_help_multiple(lint_ctx): + tool_source = get_xml_tool_source(HELP_MULTIPLE) + run_lint(lint_ctx, help.lint_help, tool_source) + assert 'More than one help section found, behavior undefined.' in lint_ctx.error_messages + assert not lint_ctx.info_messages + assert not lint_ctx.valid_messages + assert not lint_ctx.warn_messages + assert len(lint_ctx.error_messages) == 1 + + +def test_help_absent(lint_ctx): + tool_source = get_xml_tool_source(HELP_ABSENT) + run_lint(lint_ctx, help.lint_help, tool_source) + assert 'No help section found, consider adding a help section to your tool.' in lint_ctx.warn_messages + assert not lint_ctx.info_messages + assert not lint_ctx.valid_messages + assert len(lint_ctx.warn_messages) == 1 + assert not lint_ctx.error_messages + + +def test_help_empty(lint_ctx): + tool_source = get_xml_tool_source(HELP_EMPTY) + run_lint(lint_ctx, help.lint_help, tool_source) + assert 'Help section appears to be empty.' in lint_ctx.warn_messages + assert not lint_ctx.info_messages + assert not lint_ctx.valid_messages + assert len(lint_ctx.warn_messages) == 1 + assert not lint_ctx.error_messages + + +def test_help_todo(lint_ctx): + tool_source = get_xml_tool_source(HELP_TODO) + run_lint(lint_ctx, help.lint_help, tool_source) + assert 'Tool contains help section.' in lint_ctx.valid_messages + assert 'Help contains valid reStructuredText.' in lint_ctx.valid_messages + assert "Help contains TODO text." in lint_ctx.warn_messages + assert not lint_ctx.info_messages + assert len(lint_ctx.valid_messages) == 2 + assert len(lint_ctx.warn_messages) == 1 + assert not lint_ctx.error_messages + + +def test_help_invalid_rst(lint_ctx): + tool_source = get_xml_tool_source(HELP_INVALID_RST) + run_lint(lint_ctx, help.lint_help, tool_source) + assert 'Tool contains help section.' in lint_ctx.valid_messages + assert "Invalid reStructuredText found in help - [:2: (WARNING/2) Inline strong start-string without end-string.\n]." in lint_ctx.warn_messages + assert not lint_ctx.info_messages + assert len(lint_ctx.valid_messages) == 1 + assert len(lint_ctx.warn_messages) == 1 + assert not lint_ctx.error_messages + + +def test_inputs_no_inputs(lint_ctx): + tool_source = get_xml_tool_source(INPUTS_NO_INPUTS) + run_lint(lint_ctx, inputs.lint_inputs, tool_source) + assert 'Found no input parameters.' in lint_ctx.warn_messages + assert not lint_ctx.info_messages + assert not lint_ctx.valid_messages + assert len(lint_ctx.warn_messages) == 1 + assert not lint_ctx.error_messages + + +def test_inputs_no_inputs_datasource(lint_ctx): + tool_source = get_xml_tool_source(INPUTS_NO_INPUTS_DATASOURCE) + run_lint(lint_ctx, inputs.lint_inputs, tool_source) + assert 'No input parameters, OK for data sources' in lint_ctx.info_messages + assert 'display tag usually present in data sources' in lint_ctx.info_messages + assert 'uihints tag usually present in data sources' in lint_ctx.info_messages + assert len(lint_ctx.info_messages) == 3 + assert not lint_ctx.valid_messages + assert not lint_ctx.warn_messages + assert not lint_ctx.error_messages + + +def test_inputs_valid(lint_ctx): + tool_source = get_xml_tool_source(INPUTS_VALID) + run_lint(lint_ctx, inputs.lint_inputs, tool_source) + assert "Found 2 input parameters." in lint_ctx.info_messages + assert len(lint_ctx.info_messages) == 1 + assert not lint_ctx.valid_messages + assert not lint_ctx.warn_messages + assert not lint_ctx.error_messages + + +def test_inputs_param_name(lint_ctx): + tool_source = get_xml_tool_source(INPUTS_PARAM_NAME) + run_lint(lint_ctx, inputs.lint_inputs, tool_source) + assert "Found 5 input parameters." in lint_ctx.info_messages + assert 'Param input [2] is not a valid Cheetah placeholder.' in lint_ctx.warn_messages + assert 'Found param input with no name specified.' in lint_ctx.error_messages + assert 'Param input with empty name.' in lint_ctx.error_messages + assert "Param input [param_name] 'name' attribute is redundant if argument implies the same name." in lint_ctx.warn_messages + assert len(lint_ctx.info_messages) == 1 + assert not lint_ctx.valid_messages + assert len(lint_ctx.warn_messages) == 2 + assert len(lint_ctx.error_messages) == 2 + + +def test_inputs_param_type(lint_ctx): + tool_source = get_xml_tool_source(INPUTS_PARAM_TYPE) + run_lint(lint_ctx, inputs.lint_inputs, tool_source) + assert "Found 2 input parameters." in lint_ctx.info_messages + assert 'Param input [valid_name] input with no type specified.' in lint_ctx.error_messages + assert 'Param input [another_valid_name] with empty type specified.' in lint_ctx.error_messages + assert len(lint_ctx.info_messages) == 1 + assert not lint_ctx.valid_messages + assert not lint_ctx.warn_messages + assert len(lint_ctx.error_messages) == 2 + + +def test_inputs_data_param(lint_ctx): + tool_source = get_xml_tool_source(INPUTS_DATA_PARAM) + run_lint(lint_ctx, inputs.lint_inputs, tool_source) + assert "Found 1 input parameters." in lint_ctx.info_messages + assert "Param input [valid_name] with no format specified - 'data' format will be assumed." 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_inputs_conditional(lint_ctx): + tool_source = get_xml_tool_source(INPUTS_CONDITIONAL) + run_lint(lint_ctx, inputs.lint_inputs, tool_source) + assert 'Found 10 input parameters.' in lint_ctx.info_messages + assert "Conditional without a name" in lint_ctx.error_messages + assert "Select parameter of a conditional [select] options have to be defined by 'option' children elements." in lint_ctx.error_messages + assert 'Conditional [cond_wo_param] needs exactly one child found 0' in lint_ctx.error_messages + assert 'Conditional [cond_w_mult_param] needs exactly one child found 2' in lint_ctx.error_messages + assert 'Conditional [cond_text] first param should have type="select"' in lint_ctx.error_messages + assert 'Conditional [cond_boolean] first param of type="boolean" is discouraged, use a select' in lint_ctx.warn_messages + assert "Conditional [cond_boolean] no truevalue/falsevalue found for when block 'False'" in lint_ctx.warn_messages + assert 'Conditional [cond_w_optional_select] test parameter cannot be optional="true"' in lint_ctx.warn_messages + assert 'Conditional [cond_w_multiple_select] test parameter cannot be multiple="true"' in lint_ctx.warn_messages + assert "Conditional [when_wo_value] when without value" in lint_ctx.error_messages + assert "Conditional [missing_when] no block found for select option 'none'" in lint_ctx.warn_messages + assert len(lint_ctx.info_messages) == 1 + assert not lint_ctx.valid_messages + assert len(lint_ctx.warn_messages) == 6 + assert len(lint_ctx.error_messages) == 6 + + +def test_inputs_select_incompatible_display(lint_ctx): + tool_source = get_xml_tool_source(INPUTS_SELECT_INCOMPATIBLE_DISPLAY) + run_lint(lint_ctx, inputs.lint_inputs, tool_source) + assert 'Found 3 input parameters.' in lint_ctx.info_messages + assert 'Select [radio_select] display="radio" is incompatible with optional="true"' in lint_ctx.error_messages + assert 'Select [radio_select] display="radio" is incompatible with multiple="true"' in lint_ctx.error_messages + assert 'Select [checkboxes_select] `display="checkboxes"` is incompatible with `optional="false"`, remove the `display` attribute' in lint_ctx.error_messages + assert 'Select [checkboxes_select] `display="checkboxes"` is incompatible with `multiple="false"`, remove the `display` 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 + assert len(lint_ctx.error_messages) == 4 + + +def test_inputs_duplicated_options(lint_ctx): + tool_source = get_xml_tool_source(INPUTS_SELECT_DUPLICATED_OPTIONS) + run_lint(lint_ctx, inputs.lint_inputs, tool_source) + assert 'Found 1 input parameters.' in lint_ctx.info_messages + assert 'Select parameter [select] has multiple options with the same text content' in lint_ctx.error_messages + assert 'Select parameter [select] has multiple options with the same value' in lint_ctx.error_messages + assert len(lint_ctx.info_messages) == 1 + assert not lint_ctx.valid_messages + assert not lint_ctx.warn_messages + assert len(lint_ctx.error_messages) == 2 + + +def test_inputs_duplicated_options_with_different_select(lint_ctx): + tool_source = get_xml_tool_source(SELECT_DUPLICATED_OPTIONS_WITH_DIFF_SELECTED) + run_lint(lint_ctx, inputs.lint_inputs, tool_source) + assert not lint_ctx.warn_messages + assert not lint_ctx.error_messages + + +def test_inputs_select_deprections(lint_ctx): + tool_source = get_xml_tool_source(INPUTS_SELECT_DEPRECATIONS) + run_lint(lint_ctx, inputs.lint_inputs, tool_source) + assert 'Found 3 input parameters.' in lint_ctx.info_messages + assert "Select parameter [select_do] uses deprecated 'dynamic_options' attribute." in lint_ctx.warn_messages + assert "Select parameter [select_ff] options uses deprecated 'from_file' attribute." in lint_ctx.warn_messages + assert "Select parameter [select_fp] options uses deprecated 'from_parameter' attribute." in lint_ctx.warn_messages + assert "Select parameter [select_ff] options uses deprecated 'transform_lines' attribute." in lint_ctx.warn_messages + assert "Select parameter [select_fp] options uses deprecated 'options_filter_attribute' 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) == 5 + assert not lint_ctx.error_messages + + +def test_inputs_select_option_definitions(lint_ctx): + tool_source = get_xml_tool_source(INPUTS_SELECT_OPTION_DEFINITIONS) + run_lint(lint_ctx, inputs.lint_inputs, tool_source) + assert 'Found 6 input parameters.' in lint_ctx.info_messages + assert "Select parameter [select_noopt] options have to be defined by either 'option' children elements, a 'options' element or the 'dynamic_options' attribute." in lint_ctx.error_messages + assert "Select parameter [select_noopts] options tag defines no options. Use 'from_dataset', 'from_data_table', or a filter that adds values." in lint_ctx.error_messages + assert "Select parameter [select_fd_op] options have to be defined by either 'option' children elements, a 'options' element or the 'dynamic_options' attribute." in lint_ctx.error_messages + assert "Select parameter [select_fd_op] contains multiple options elements." in lint_ctx.error_messages + assert "Select parameter [select_fd_fdt] options uses 'from_dataset' and 'from_data_table' attribute." in lint_ctx.error_messages + assert "Select parameter [select_noval_notext] has option without value" in lint_ctx.error_messages + assert "Select parameter [select_noval_notext] has option without text" in lint_ctx.warn_messages + assert "Select parameter [select_meta_file_key_incomp] 'meta_file_key' is only compatible with 'from_dataset'." in lint_ctx.error_messages + assert len(lint_ctx.info_messages) == 1 + assert not lint_ctx.valid_messages + assert len(lint_ctx.warn_messages) == 1 + assert len(lint_ctx.error_messages) == 7 + + +def test_inputs_select_filter(lint_ctx): + tool_source = get_xml_tool_source(INPUTS_SELECT_FILTER) + run_lint(lint_ctx, inputs.lint_inputs, tool_source) + assert 'Found 1 input parameters.' in lint_ctx.info_messages + assert "Select parameter [select_filter_types] contains filter without type." in lint_ctx.error_messages + assert "Select parameter [select_filter_types] contains filter with unknown type 'unknown_filter_type'." in lint_ctx.error_messages + assert len(lint_ctx.info_messages) == 1 + assert not lint_ctx.valid_messages + assert not lint_ctx.warn_messages + assert len(lint_ctx.error_messages) == 2 + + +def test_inputs_validator_incompatibilities(lint_ctx): + tool_source = get_xml_tool_source(INPUTS_VALIDATOR_INCOMPATIBILITIES) + run_lint(lint_ctx, inputs.lint_inputs, tool_source) + assert 'Found 2 input parameters.' in lint_ctx.info_messages + assert "Parameter [param_name]: 'in_range' validators are not expected to contain text (found 'TEXT')" in lint_ctx.warn_messages + assert "Parameter [param_name]: validator with an incompatible type 'in_range'" in lint_ctx.error_messages + assert "Parameter [param_name]: 'in_range' validators need to define the 'min' or 'max' attribute(s)" in lint_ctx.error_messages + assert "Parameter [param_name]: attribute 'filename' is incompatible with validator of type 'regex'" in lint_ctx.error_messages + assert "Parameter [param_name]: expression validators are expected to contain text" in lint_ctx.error_messages + assert "Parameter [param_name]: '[' is no valid regular expression: unterminated character set at position 0" in lint_ctx.error_messages + assert "Parameter [another_param_name]: 'metadata' validators need to define the 'check' or 'skip' attribute(s)" in lint_ctx.error_messages + assert "Parameter [param_name]: 'value_in_data_table' validators need to define the 'table_name' attribute" in lint_ctx.error_messages + assert len(lint_ctx.info_messages) == 1 + assert not lint_ctx.valid_messages + assert len(lint_ctx.warn_messages) == 1 + assert len(lint_ctx.error_messages) == 7 + + +def test_inputs_validator_correct(lint_ctx): + tool_source = get_xml_tool_source(INPUTS_VALIDATOR_CORRECT) + run_lint(lint_ctx, inputs.lint_inputs, tool_source) + assert 'Found 5 input parameters.' in lint_ctx.info_messages + assert len(lint_ctx.info_messages) == 1 + assert not lint_ctx.valid_messages + assert not lint_ctx.warn_messages + assert not lint_ctx.error_messages + + +def test_inputs_repeats(lint_ctx): + 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 + assert not lint_ctx.valid_messages + assert not lint_ctx.warn_messages + assert len(lint_ctx.error_messages) == 2 + + +def test_outputs_missing(lint_ctx): + 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 + assert len(lint_ctx.warn_messages) == 1 + assert not lint_ctx.error_messages + + +def test_outputs_multiple(lint_ctx): + 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 + assert not lint_ctx.valid_messages + assert len(lint_ctx.warn_messages) == 1 + assert not lint_ctx.error_messages + + +def test_outputs_unknown_tag(lint_ctx): + 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 + assert not lint_ctx.valid_messages + assert len(lint_ctx.warn_messages) == 1 + assert not lint_ctx.error_messages + + +def test_outputs_unnamed_invalid_name(lint_ctx): + 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 + assert 'Tool output name [2output] is not a valid Cheetah placeholder.' in lint_ctx.warn_messages + assert "Collection output with undefined 'type' found." in lint_ctx.warn_messages + assert "Tool collection output 2output doesn't define an output format." in lint_ctx.warn_messages + assert len(lint_ctx.info_messages) == 1 + assert not lint_ctx.valid_messages + assert len(lint_ctx.warn_messages) == 5 + assert not lint_ctx.error_messages + + +def test_outputs_format_input(lint_ctx): + 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.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_collection_format_source(lint_ctx): + 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 + assert len(lint_ctx.warn_messages) == 1 + assert not lint_ctx.error_messages + + +def test_outputs_discover_tool_provided_metadata(lint_ctx): + 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 + assert not lint_ctx.warn_messages + assert not lint_ctx.error_messages + + +def test_stdio_default_for_default_profile(lint_ctx): + tool_source = get_xml_tool_source(STDIO_DEFAULT_FOR_DEFAULT_PROFILE) + run_lint(lint_ctx, stdio.lint_stdio, XmlToolSource(tool_source)) + assert "No stdio definition found, tool indicates error conditions with output written to stderr." in lint_ctx.info_messages + assert len(lint_ctx.info_messages) == 1 + assert not lint_ctx.valid_messages + assert not lint_ctx.warn_messages + assert not lint_ctx.error_messages + + +def test_stdio_default_for_nonlegacy_profile(lint_ctx): + tool_source = get_xml_tool_source(STDIO_DEFAULT_FOR_NONLEGACY_PROFILE) + run_lint(lint_ctx, stdio.lint_stdio, XmlToolSource(tool_source)) + assert "No stdio definition found, tool indicates error conditions with non-zero exit codes." in lint_ctx.info_messages + assert len(lint_ctx.info_messages) == 1 + assert not lint_ctx.valid_messages + assert not lint_ctx.warn_messages + assert not lint_ctx.error_messages + + +def test_stdio_multiple_stdio(lint_ctx): + tool_source = get_xml_tool_source(STDIO_MULTIPLE_STDIO) + run_lint(lint_ctx, stdio.lint_stdio, XmlToolSource(tool_source)) + assert "More than one stdio tag found, behavior undefined." in lint_ctx.error_messages + assert not lint_ctx.info_messages + assert not lint_ctx.valid_messages + assert not lint_ctx.warn_messages + assert len(lint_ctx.error_messages) == 1 + + +def test_stdio_invalid_child_or_attrib(lint_ctx): + tool_source = get_xml_tool_source(STDIO_INVALID_CHILD_OR_ATTRIB) + run_lint(lint_ctx, stdio.lint_stdio, XmlToolSource(tool_source)) + assert "Unknown stdio child tag discovered [reqex]. Valid options are exit_code and regex." in lint_ctx.warn_messages + assert "Unknown attribute [descriptio] encountered on exit_code tag." in lint_ctx.warn_messages + assert "Unknown attribute [descriptio] encountered on regex tag." in lint_ctx.warn_messages + assert not lint_ctx.info_messages + assert not lint_ctx.valid_messages + assert len(lint_ctx.warn_messages) == 3 + assert not lint_ctx.error_messages + + +def test_stdio_invalid_match(lint_ctx): + tool_source = get_xml_tool_source(STDIO_INVALID_MATCH) + run_lint(lint_ctx, stdio.lint_stdio, XmlToolSource(tool_source)) + assert "Match '[' is no valid regular expression: unterminated character set at position 0" in lint_ctx.error_messages + assert not lint_ctx.info_messages + assert not lint_ctx.valid_messages + assert not lint_ctx.warn_messages + assert len(lint_ctx.error_messages) == 1 + + +def test_tests_absent(lint_ctx): + tool_source = get_xml_tool_source(TESTS_ABSENT) + run_lint(lint_ctx, tests.lint_tsts, tool_source) + assert 'No tests found, most tools should define test cases.' in lint_ctx.warn_messages + assert not lint_ctx.info_messages + assert not lint_ctx.valid_messages + assert len(lint_ctx.warn_messages) == 1 + assert not lint_ctx.error_messages + + +def test_tests_data_source(lint_ctx): + tool_source = get_xml_tool_source(TESTS_ABSENT_DATA_SOURCE) + run_lint(lint_ctx, tests.lint_tsts, tool_source) + assert 'No tests found, that should be OK for data_sources.' in lint_ctx.info_messages + assert len(lint_ctx.info_messages) == 1 + assert not lint_ctx.valid_messages + assert not lint_ctx.warn_messages + assert not lint_ctx.error_messages + + +def test_tests_param_output_names(lint_ctx): + tool_source = get_xml_tool_source(TESTS_PARAM_OUTPUT_NAMES) + run_lint(lint_ctx, tests.lint_tsts, tool_source) + assert '1 test(s) found.' in lint_ctx.valid_messages + assert "Test 1: Found test param tag without a name defined." in lint_ctx.error_messages + assert "Test 1: Test param non_existent_test_name not found in the inputs" in lint_ctx.error_messages + assert "Test 1: Found output tag without a name defined." in lint_ctx.error_messages + assert "Test 1: Found output tag with unknown name [nonexistent_output], valid names [['existent_output']]" in lint_ctx.error_messages + assert "Test 1: Found output_collection tag without a name defined." in lint_ctx.error_messages + assert "Test 1: Found output_collection tag with unknown name [nonexistent_collection], valid names [['existent_collection']]" in lint_ctx.error_messages + assert not lint_ctx.info_messages + assert len(lint_ctx.valid_messages) == 1 + assert not lint_ctx.warn_messages + assert len(lint_ctx.error_messages) == 6 + + +def test_tests_expect_failure_output(lint_ctx): + tool_source = get_xml_tool_source(TESTS_EXPECT_FAILURE_OUTPUT) + run_lint(lint_ctx, tests.lint_tsts, tool_source) + assert 'No valid test(s) found.' in lint_ctx.warn_messages + assert "Test 1: Cannot specify outputs in a test expecting failure." in lint_ctx.error_messages + assert not lint_ctx.info_messages + assert not lint_ctx.valid_messages + assert len(lint_ctx.warn_messages) == 1 + assert len(lint_ctx.error_messages) == 1 + + +def test_tests_without_expectations(lint_ctx): + tool_source = get_xml_tool_source(TESTS_WO_EXPECTATIONS) + run_lint(lint_ctx, tests.lint_tsts, tool_source) + assert 'Test 1: No outputs or expectations defined for tests, this test is likely invalid.' in lint_ctx.warn_messages + assert 'No valid test(s) found.' in lint_ctx.warn_messages + assert not lint_ctx.info_messages + assert not lint_ctx.valid_messages + assert len(lint_ctx.warn_messages) == 2 + assert not lint_ctx.error_messages + + +def test_tests_valid(lint_ctx): + tool_source = get_xml_tool_source(TESTS_VALID) + run_lint(lint_ctx, tests.lint_tsts, tool_source) + assert "1 test(s) found." in lint_ctx.valid_messages + assert not lint_ctx.info_messages + assert len(lint_ctx.valid_messages) == 1 + assert not lint_ctx.warn_messages + assert not lint_ctx.error_messages + + +def test_tests_asserts(lint_ctx): + tool_source = get_xml_tool_source(ASSERTS) + run_lint(lint_ctx, tests.lint_tsts, tool_source) + assert "Test 1: unknown assertion 'invalid'" in lint_ctx.error_messages + assert "Test 1: unknown attribute 'invalid_attrib' for 'has_text'" in lint_ctx.error_messages + assert "Test 1: missing attribute 'text' for 'has_text'" in lint_ctx.error_messages + assert "Test 1: attribute 'value' for 'has_size' needs to be 'int' got '500k'" in lint_ctx.error_messages + assert "Test 1: attribute 'delta' for 'has_size' needs to be 'int' got '1O'" in lint_ctx.error_messages + assert "Test 1: unknown attribute 'invalid_attrib_also_checked_in_nested_asserts' for 'not_has_text'" in lint_ctx.error_messages + assert "Test 1: 'has_size' needs to specify 'n', 'min', or 'max'" in lint_ctx.error_messages + assert "Test 1: 'has_n_columns' needs to specify 'n', 'min', or 'max'" in lint_ctx.error_messages + assert "Test 1: 'has_n_lines' needs to specify 'n', 'min', or 'max'" in lint_ctx.error_messages + assert not lint_ctx.warn_messages + assert len(lint_ctx.error_messages) == 9 + + +def test_xml_order(lint_ctx): + tool_source = get_xml_tool_source(XML_ORDER) + run_lint(lint_ctx, xml_order.lint_xml_order, tool_source) + assert 'Unknown tag [wrong_tag] encountered, this may result in a warning in the future.' in lint_ctx.info_messages + assert 'Best practice violation [stdio] elements should come before [command]' 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 COMPLETE = """ @@ -1169,23 +1283,8 @@ def test_tool_xml(tool_xml, lint_func, assert_func): """ -TOOL_AND_MACRO_XML_TESTS = [ - ( - COMPLETE, COMPLETE_MACROS, - [ - ("Select parameter [select] has multiple options with the same value", "tool.xml", 5, "/tool/inputs/param[1]"), - ("Found param input with no name specified.", "tool.xml", 13, "/tool/inputs/param[2]"), - ("Param input [No_type] input with no type specified.", "macros.xml", 3, "/tool/inputs/param[3]") - ] - ), -] -TOOL_AND_MACRO_XML_IDS = [ - "test tool xml and macros: context: line numbers, file paths, and xpaths" -] - - -@pytest.mark.parametrize('tool_xml,macros_xml,asserts', TOOL_AND_MACRO_XML_TESTS, ids=TOOL_AND_MACRO_XML_IDS) -def test_tool_and_macro_xml(tool_xml, macros_xml, asserts): + +def test_tool_and_macro_xml(lint_ctx): """ test linters (all of them via lint_tool_source_with) on a tool and macro xml file checking a list of asserts, where each assert is a 4-tuple: @@ -1198,15 +1297,19 @@ def test_tool_and_macro_xml(tool_xml, macros_xml, asserts): tool_path = os.path.join(tmp, "tool.xml") macros_path = os.path.join(tmp, "macros.xml") with open(tool_path, "w") as tmpf: - tmpf.write(tool_xml) + tmpf.write(COMPLETE) with open(macros_path, "w") as tmpf: - tmpf.write(macros_xml) + tmpf.write(COMPLETE_MACROS) tool_xml, _ = load_with_references(tool_path) tool_source = XmlToolSource(tool_xml) - lint_ctx = LintContext('all') lint_tool_source_with(lint_ctx, tool_source) + asserts = ( + ("Select parameter [select] has multiple options with the same value", "tool.xml", 5, "/tool/inputs/param[1]"), + ("Found param input with no name specified.", "tool.xml", 13, "/tool/inputs/param[2]"), + ("Param input [No_type] input with no type specified.", "macros.xml", 3, "/tool/inputs/param[3]") + ) for a in asserts: message, fname, line, xpath = a found = False @@ -1252,50 +1355,44 @@ def test_tool_and_macro_xml(tool_xml, macros_xml, asserts): format: txt """ -YML_CWL_TOOLS = [ - ( - CWL_TOOL, "cwl", - lambda x: - "Tool defines a version [0.0.1]." in x.valid_messages - and "Tool defines a name [tool]." in x.valid_messages - and "Tool defines an id [tool]." in x.valid_messages - and "Tool specifies profile version [16.04]." in x.valid_messages - and "CWL appears to be valid." in x.info_messages - and "Description of tool is empty or absent." in x.warn_messages - and "Tool does not specify a DockerPull source." in x.warn_messages - and "Modern CWL version [v1.0]." in x.info_messages - and len(x.info_messages) == 2 and len(x.valid_messages) == 4 and len(x.warn_messages) == 2 and len(x.error_messages) == 0 - ), - ( - YAML_TOOL, "yml", - lambda x: - "Tool defines a version [1.0]." in x.valid_messages - and "Tool defines a name [simple_constructs_y]." in x.valid_messages - and "Tool defines an id [simple_constructs_y]." in x.valid_messages - and "Tool specifies profile version [16.04]." in x.valid_messages - and len(x.info_messages) == 0 and len(x.valid_messages) == 4 and len(x.warn_messages) == 0 and len(x.error_messages) == 0 - ) -] -YML_CWL_TOOLS_IDS = [ - "cwl tool", - "yaml tool" -] +def test_linting_yml_tool(lint_ctx): + with tempfile.TemporaryDirectory() as tmp: + tool_path = os.path.join(tmp, "tool.yml") + with open(tool_path, "w") as tmpf: + tmpf.write(YAML_TOOL) + tool_sources = load_tool_sources_from_path(tmp) + assert len(tool_sources) == 1, "Expected 1 tool source" + tool_source = tool_sources[0][1] + lint_tool_source_with(lint_ctx, tool_source) + assert "Tool defines a version [1.0]." in lint_ctx.valid_messages + assert "Tool defines a name [simple_constructs_y]." in lint_ctx.valid_messages + assert "Tool defines an id [simple_constructs_y]." in lint_ctx.valid_messages + assert "Tool specifies profile version [16.04]." in lint_ctx.valid_messages + assert not lint_ctx.info_messages + assert len(lint_ctx.valid_messages) == 4 + assert not lint_ctx.warn_messages + assert not lint_ctx.error_messages -@pytest.mark.parametrize('tool_raw,ext,assert_func', YML_CWL_TOOLS, ids=YML_CWL_TOOLS_IDS) -def test_tool_yml_cwl(tool_raw, ext, assert_func): + +def test_linting_cwl_tool(lint_ctx): with tempfile.TemporaryDirectory() as tmp: - tool_path = os.path.join(tmp, f"tool.{ext}") + tool_path = os.path.join(tmp, "tool.cwl") with open(tool_path, "w") as tmpf: - tmpf.write(tool_raw) + tmpf.write(CWL_TOOL) tool_sources = load_tool_sources_from_path(tmp) assert len(tool_sources) == 1, "Expected 1 tool source" tool_source = tool_sources[0][1] - lint_ctx = LintContext('all') lint_tool_source_with(lint_ctx, tool_source) - assert assert_func(lint_ctx), ( - 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}" - ) + assert "Tool defines a version [0.0.1]." in lint_ctx.valid_messages + assert "Tool defines a name [tool]." in lint_ctx.valid_messages + assert "Tool defines an id [tool]." in lint_ctx.valid_messages + assert "Tool specifies profile version [16.04]." in lint_ctx.valid_messages + assert "CWL appears to be valid." in lint_ctx.info_messages + assert "Description of tool is empty or absent." in lint_ctx.warn_messages + assert "Tool does not specify a DockerPull source." in lint_ctx.warn_messages + assert "Modern CWL version [v1.0]." in lint_ctx.info_messages + assert len(lint_ctx.info_messages) == 2 + assert len(lint_ctx.valid_messages) == 4 + assert len(lint_ctx.warn_messages) == 2 + assert not lint_ctx.error_messages From 1d1bae8453ef30dae5a70d3b5e3a8855c3ec9a28 Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Wed, 26 Jan 2022 20:16:02 +0100 Subject: [PATCH 12/18] add function to return lint context this adds functionality to the linter functions to return the lint context. without breaking previous behavior. Planemo seems to use - lint_tool_source (which returned the fail state) and - lint_tool_source_with (which returned nothing) For the later a parameter has been added to make the linter silent and the lint context is now returned. For the former a variant has been added which returns the lint context. --- lib/galaxy/tool_util/lint.py | 127 ++++++++++++++--------- test/unit/tool_util/test_tool_linters.py | 2 +- 2 files changed, 78 insertions(+), 51 deletions(-) diff --git a/lib/galaxy/tool_util/lint.py b/lib/galaxy/tool_util/lint.py index 8bf081e3c56a..c2f72e09f428 100644 --- a/lib/galaxy/tool_util/lint.py +++ b/lib/galaxy/tool_util/lint.py @@ -12,15 +12,36 @@ def lint_tool_source(tool_source, level=LEVEL_ALL, fail_level=LEVEL_WARN, extra_modules=None, skip_types=None, name=None): + """ + apply all (applicable) linters from the linters submodule + and the ones in extramodules + + immediately print linter messages (wrt level) and return if linting failed (wrt fail_level) + """ extra_modules = extra_modules or [] skip_types = skip_types or [] lint_context = LintContext(level=level, skip_types=skip_types, object_name=name) lint_tool_source_with(lint_context, tool_source, extra_modules) - return not lint_context.failed(fail_level) +def lint_context_for_tool_source(tool_source, extra_modules=None, skip_types=None, name=None): + """ + this is the silent variant of lint_tool_source + it returns the LintContext from which all linter messages + and the status can be obtained + """ + extra_modules = extra_modules or [] + skip_types = skip_types or [] + lint_context = LintContext(level="all", skip_types=skip_types, object_name=name) + lint_tool_source_with(lint_context, tool_source, extra_modules, silent=True) + return lint_context + + def lint_xml(tool_xml, level=LEVEL_ALL, fail_level=LEVEL_WARN, extra_modules=None, skip_types=None, name=None): + """ + silently lint an xml tool + """ extra_modules = extra_modules or [] skip_types = skip_types or [] lint_context = LintContext(level=level, skip_types=skip_types, object_name=name) @@ -29,7 +50,7 @@ def lint_xml(tool_xml, level=LEVEL_ALL, fail_level=LEVEL_WARN, extra_modules=Non return not lint_context.failed(fail_level) -def lint_tool_source_with(lint_context, tool_source, extra_modules=None): +def lint_tool_source_with(lint_context, tool_source, extra_modules=None, silent=False): extra_modules = extra_modules or [] import galaxy.tool_util.linters tool_xml = getattr(tool_source, "xml_tree", None) @@ -52,15 +73,16 @@ def lint_tool_source_with(lint_context, tool_source, extra_modules=None): # XML linter and non-XML tool, skip for now continue else: - lint_context.lint(name, value, tool_xml) + lint_context.lint(name, value, tool_xml, silent) else: - lint_context.lint(name, value, tool_source) + lint_context.lint(name, value, tool_source, silent) + return lint_context def lint_xml_with(lint_context, tool_xml, extra_modules=None): extra_modules = extra_modules or [] tool_source = get_tool_source(xml_tree=tool_xml) - return lint_tool_source_with(lint_context, tool_source, extra_modules=extra_modules) + lint_tool_source_with(lint_context, tool_source, extra_modules=extra_modules) class LintMessage: @@ -102,61 +124,66 @@ def __init__(self, level, skip_types=None, object_name=None): self.skip_types = skip_types or [] self.level = level self.object_name = object_name - self.found_errors = False - self.found_warns = False self.message_list = [] - def lint(self, name, lint_func, lint_target): + @property + def found_errors(self): + return len(self.error_messages) > 0 + + @property + def found_warns(self): + return len(self.warn_messages) > 0 + + def lint(self, name, lint_func, lint_target, silent=False): name = name.replace("tsts", "tests")[len("lint_"):] if name in self.skip_types: return - self.printed_linter_info = False - tmp_message_list = list(self.message_list) - self.message_list = [] + + if not silent: + # this is a relict from the past where the lint context + # was reset when called with a new lint_func, as workaround + # we safe the message list, apply the lint_func (which then + # adds to the message_list) and restore the message list + # at the end (+ append the new messages) + tmp_message_list = list(self.message_list) + self.message_list = [] # call linter lint_func(lint_target, self) - # TODO: colorful emoji if in click CLI. - if self.error_messages: - status = "FAIL" - elif self.warn_messages: - status = "WARNING" - else: - status = "CHECK" - - def print_linter_info(): - if self.printed_linter_info: - return - self.printed_linter_info = True - print(f"Applying linter {name}... {status}") - - for message in self.message_list: - if message.level != "error": - continue - self.found_errors = True - print_linter_info() - print(f"{message}") - - if self.level != LEVEL_ERROR: - for message in self.message_list: - if message.level != "warning": - continue - self.found_warns = True - print_linter_info() - print(f"{message}") - if self.level == LEVEL_ALL: - for message in self.message_list: - if message.level != "info": - continue - print_linter_info() + if not silent: + # TODO: colorful emoji if in click CLI. + if self.error_messages: + status = "FAIL" + elif self.warn_messages: + status = "WARNING" + else: + status = "CHECK" + + def print_linter_info(printed_linter_info): + if printed_linter_info: + return True + print(f"Applying linter {name}... {status}") + return True + + plf = False + for message in self.error_messages: + plf = print_linter_info(plf) print(f"{message}") - for message in self.message_list: - if message.level != "check": - continue - print_linter_info() - print(f"{message}") - self.message_list = tmp_message_list + self.message_list + + if self.level != LEVEL_ERROR: + for message in self.warn_messages: + plf = print_linter_info(plf) + print(f"{message}") + + if self.level == LEVEL_ALL: + for message in self.info_messages: + plf = print_linter_info(plf) + print(f"{message}") + for message in self.valid_messages: + plf = print_linter_info(plf) + print(f"{message}") + self.message_list = tmp_message_list + self.message_list @property def valid_messages(self): diff --git a/test/unit/tool_util/test_tool_linters.py b/test/unit/tool_util/test_tool_linters.py index 36339abc40a1..27147820ee95 100644 --- a/test/unit/tool_util/test_tool_linters.py +++ b/test/unit/tool_util/test_tool_linters.py @@ -1303,7 +1303,7 @@ def test_tool_and_macro_xml(lint_ctx): tool_xml, _ = load_with_references(tool_path) tool_source = XmlToolSource(tool_xml) - lint_tool_source_with(lint_ctx, tool_source) + lint_tool_source_with(lint_ctx, tool_source, silent=True) asserts = ( ("Select parameter [select] has multiple options with the same value", "tool.xml", 5, "/tool/inputs/param[1]"), From 9d6bfad2553fc15da5a1b764066ebcbc1f3bb233 Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Wed, 26 Jan 2022 20:51:39 +0100 Subject: [PATCH 13/18] adapt tool loader test when loading macros from imported files the base property of the node is overwritten. this adds a xml:base property to the node which is printed in the test --- lib/galaxy/util/xml_macros.py | 3 ++- test/unit/tool_util/test_tool_loader.py | 12 ++++++++---- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/lib/galaxy/util/xml_macros.py b/lib/galaxy/util/xml_macros.py index 2bf51ddf8612..2b662981db8b 100644 --- a/lib/galaxy/util/xml_macros.py +++ b/lib/galaxy/util/xml_macros.py @@ -288,7 +288,8 @@ def _load_macro_file(path, xml_base_dir): # the linter. it is a property that apparently is determined from the # xmltree, so if the macro is inserted into the main xml node.base will # give the path of the main xml file. - # luckily lxml allows to set the property: + # luckily lxml allows to set the property by adding xml:base to the node + # which will be returned if the property is read # https://github.com/lxml/lxml/blob/5a5c7fb01d15af58def4bab2ba7b15c937042835/src/lxml/etree.pyx#L1106 node.base = node.base root = tree.getroot() diff --git a/test/unit/tool_util/test_tool_loader.py b/test/unit/tool_util/test_tool_loader.py index 6233fcec7422..b1cce14ce4ec 100644 --- a/test/unit/tool_util/test_tool_loader.py +++ b/test/unit/tool_util/test_tool_loader.py @@ -1,4 +1,5 @@ import os +import re from shutil import rmtree from tempfile import mkdtemp @@ -638,12 +639,15 @@ def test_loader_specify_nested_macro_by_token(): ''', name="external.xml") xml = tool_dir.load() - assert xml_to_string(xml, pretty=True) == ''' + # test with re because loading from external macros + # adds a xml:base property (containing the source path) + # to the node which is printed + assert re.match(r"""<\?xml version="1\.0" \?> - - -''' + + +""", xml_to_string(xml, pretty=True), re.MULTILINE) def test_loader_circular_macros(): From 33807be8c7133724ab4233de901ad0017e38e0e2 Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Thu, 27 Jan 2022 12:14:18 +0100 Subject: [PATCH 14/18] add type annotations --- lib/galaxy/tool_util/lint.py | 227 ++++++++++++++++++++--------------- 1 file changed, 128 insertions(+), 99 deletions(-) diff --git a/lib/galaxy/tool_util/lint.py b/lib/galaxy/tool_util/lint.py index c2f72e09f428..0f1a25178e27 100644 --- a/lib/galaxy/tool_util/lint.py +++ b/lib/galaxy/tool_util/lint.py @@ -1,6 +1,14 @@ """This modules contains the functions that drive the tool linting framework.""" import inspect +from typing import ( + Callable, + List, + NoReturn, + Optional, + TypeVar, + Union +) from galaxy.tool_util.parser import get_tool_source from galaxy.util import submodules @@ -11,95 +19,35 @@ LEVEL_ERROR = "error" -def lint_tool_source(tool_source, level=LEVEL_ALL, fail_level=LEVEL_WARN, extra_modules=None, skip_types=None, name=None): - """ - apply all (applicable) linters from the linters submodule - and the ones in extramodules - - immediately print linter messages (wrt level) and return if linting failed (wrt fail_level) - """ - extra_modules = extra_modules or [] - skip_types = skip_types or [] - lint_context = LintContext(level=level, skip_types=skip_types, object_name=name) - lint_tool_source_with(lint_context, tool_source, extra_modules) - return not lint_context.failed(fail_level) - - -def lint_context_for_tool_source(tool_source, extra_modules=None, skip_types=None, name=None): - """ - this is the silent variant of lint_tool_source - it returns the LintContext from which all linter messages - and the status can be obtained - """ - extra_modules = extra_modules or [] - skip_types = skip_types or [] - lint_context = LintContext(level="all", skip_types=skip_types, object_name=name) - lint_tool_source_with(lint_context, tool_source, extra_modules, silent=True) - return lint_context - - -def lint_xml(tool_xml, level=LEVEL_ALL, fail_level=LEVEL_WARN, extra_modules=None, skip_types=None, name=None): - """ - silently lint an xml tool - """ - extra_modules = extra_modules or [] - skip_types = skip_types or [] - lint_context = LintContext(level=level, skip_types=skip_types, object_name=name) - lint_xml_with(lint_context, tool_xml, extra_modules) - - return not lint_context.failed(fail_level) - - -def lint_tool_source_with(lint_context, tool_source, extra_modules=None, silent=False): - extra_modules = extra_modules or [] - import galaxy.tool_util.linters - tool_xml = getattr(tool_source, "xml_tree", None) - linter_modules = submodules.import_submodules(galaxy.tool_util.linters, ordered=True) - linter_modules.extend(extra_modules) - for module in linter_modules: - tool_type = tool_source.parse_tool_type() or "default" - lint_tool_types = getattr(module, "lint_tool_types", ["default"]) - if not ("*" in lint_tool_types or tool_type in lint_tool_types): - continue - - for (name, value) in inspect.getmembers(module): - if callable(value) and name.startswith("lint_"): - # Look at the first argument to the linter to decide - # if we should lint the XML description or the abstract - # tool parser object. - first_arg = inspect.getfullargspec(value).args[0] - if first_arg == "tool_xml": - if tool_xml is None: - # XML linter and non-XML tool, skip for now - continue - else: - lint_context.lint(name, value, tool_xml, silent) - else: - lint_context.lint(name, value, tool_source, silent) - return lint_context - - -def lint_xml_with(lint_context, tool_xml, extra_modules=None): - extra_modules = extra_modules or [] - tool_source = get_tool_source(xml_tree=tool_xml) - lint_tool_source_with(lint_context, tool_source, extra_modules=extra_modules) - - class LintMessage: """ a message from the linter """ - def __init__(self, level, message, line, fname, xpath=None): + def __init__(self, level: str, message: str, line, fname, xpath=None): self.level = level self.message = message self.line = line # 1 based self.fname = fname self.xpath = xpath - def __str__(self): + def __eq__(self, other: Union[str, 'LintMessage']) -> bool: + """ + add equal operator to easy lookup of a message in a + List[LintMessage] which is usefull in tests + """ + if isinstance(other, str): + return self.message == other + if isinstance(other, LintMessage): + return self.message == other.message + return False + + def __str__(self) -> str: return f".. {self.level.upper()}: {self.message}" - def pretty_print(self, level=True, fname=True, xpath=False): + def __repr__(self) -> str: + return f"LintMessage({self.message})" + + def pretty_print(self, level: bool = True, fname: bool = True, xpath: bool = False) -> str: rval = "" if level: rval += f".. {self.level.upper()}: " @@ -115,26 +63,33 @@ def pretty_print(self, level=True, fname=True, xpath=False): return rval +LintTargetType = TypeVar('LintTarget') + + # TODO: Nothing inherently tool-y about LintContext and in fact # it is reused for repositories in planemo. Therefore, it should probably # be moved to galaxy.util.lint. class LintContext: - def __init__(self, level, skip_types=None, object_name=None): - self.skip_types = skip_types or [] - self.level = level - self.object_name = object_name - self.message_list = [] + def __init__(self, level: str, skip_types: Optional[List[str]] = None, object_name: Optional[str] = None): + self.skip_types: List[str] = skip_types or [] + self.level: str = level + self.object_name: str = object_name + self.message_list: List[LintMessage] = [] @property - def found_errors(self): + def found_errors(self) -> bool: return len(self.error_messages) > 0 @property - def found_warns(self): + def found_warns(self) -> bool: return len(self.warn_messages) > 0 - def lint(self, name, lint_func, lint_target, silent=False): + def lint(self, + name: str, + lint_func: Callable[['LintContext', LintTargetType], NoReturn], + lint_target: LintTargetType, + silent: bool = False): name = name.replace("tsts", "tests")[len("lint_"):] if name in self.skip_types: return @@ -186,40 +141,40 @@ def print_linter_info(printed_linter_info): self.message_list = tmp_message_list + self.message_list @property - def valid_messages(self): - return [x.message for x in self.message_list if x.level == "check"] + def valid_messages(self) -> List[LintMessage]: + return [x for x in self.message_list if x.level == "check"] @property - def info_messages(self): - return [x.message for x in self.message_list if x.level == "info"] + def info_messages(self) -> List[LintMessage]: + return [x for x in self.message_list if x.level == "info"] @property - def warn_messages(self): - return [x.message for x in self.message_list if x.level == "warning"] + def warn_messages(self) -> List[LintMessage]: + return [x for x in self.message_list if x.level == "warning"] @property - def error_messages(self): - return [x.message for x in self.message_list if x.level == "error"] + def error_messages(self) -> List[LintMessage]: + return [x for x in self.message_list if x.level == "error"] - def __handle_message(self, level, message, line, fname, xpath, *args): + def __handle_message(self, level: str, message: str, line: int, fname: str, xpath: str, *args) -> NoReturn: if args: message = message % args self.message_list.append(LintMessage(level=level, message=message, line=line, fname=fname, xpath=xpath)) - def valid(self, message, line=None, fname=None, xpath=None, *args): + def valid(self, message: str, line: Optional[str] = None, fname: Optional[str] = None, xpath: Optional[str] = None, *args) -> NoReturn: self.__handle_message("check", message, line, fname, xpath, *args) - def info(self, message, line=None, fname=None, xpath=None, *args): + def info(self, message: str, line: Optional[str] = None, fname: Optional[str] = None, xpath: Optional[str] = None, *args) -> NoReturn: self.__handle_message("info", message, line, fname, xpath, *args) - def error(self, message, line=None, fname=None, xpath=None, *args): + def error(self, message: str, line: Optional[str] = None, fname: Optional[str] = None, xpath: Optional[str] = None, *args) -> NoReturn: self.__handle_message("error", message, line, fname, xpath, *args) - def warn(self, message, line=None, fname=None, xpath=None, *args): + def warn(self, message: str, line: Optional[str] = None, fname: Optional[str] = None, xpath: Optional[str] = None, *args) -> NoReturn: self.__handle_message("warning", message, line, fname, xpath, *args) - def failed(self, fail_level): + def failed(self, fail_level: str) -> bool: found_warns = self.found_warns found_errors = self.found_errors if fail_level == LEVEL_WARN: @@ -227,3 +182,77 @@ def failed(self, fail_level): else: lint_fail = found_errors return lint_fail + + +def lint_tool_source(tool_source, level=LEVEL_ALL, fail_level=LEVEL_WARN, extra_modules=None, skip_types=None, name=None) -> bool: + """ + apply all (applicable) linters from the linters submodule + and the ones in extramodules + + immediately print linter messages (wrt level) and return if linting failed (wrt fail_level) + """ + extra_modules = extra_modules or [] + skip_types = skip_types or [] + lint_context = LintContext(level=level, skip_types=skip_types, object_name=name) + lint_tool_source_with(lint_context, tool_source, extra_modules) + return not lint_context.failed(fail_level) + + +def get_lint_context_for_tool_source(tool_source, extra_modules=None, skip_types=None, name=None) -> LintContext: + """ + this is the silent variant of lint_tool_source + it returns the LintContext from which all linter messages + and the status can be obtained + """ + extra_modules = extra_modules or [] + skip_types = skip_types or [] + lint_context = LintContext(level="all", skip_types=skip_types, object_name=name) + lint_tool_source_with(lint_context, tool_source, extra_modules, silent=True) + return lint_context + + +def lint_xml(tool_xml, level=LEVEL_ALL, fail_level=LEVEL_WARN, extra_modules=None, skip_types=None, name=None) -> bool: + """ + silently lint an xml tool + """ + extra_modules = extra_modules or [] + skip_types = skip_types or [] + lint_context = LintContext(level=level, skip_types=skip_types, object_name=name) + lint_xml_with(lint_context, tool_xml, extra_modules) + + return not lint_context.failed(fail_level) + + +def lint_tool_source_with(lint_context, tool_source, extra_modules=None, silent=False) -> LintContext: + extra_modules = extra_modules or [] + import galaxy.tool_util.linters + tool_xml = getattr(tool_source, "xml_tree", None) + linter_modules = submodules.import_submodules(galaxy.tool_util.linters, ordered=True) + linter_modules.extend(extra_modules) + for module in linter_modules: + tool_type = tool_source.parse_tool_type() or "default" + lint_tool_types = getattr(module, "lint_tool_types", ["default"]) + if not ("*" in lint_tool_types or tool_type in lint_tool_types): + continue + + for (name, value) in inspect.getmembers(module): + if callable(value) and name.startswith("lint_"): + # Look at the first argument to the linter to decide + # if we should lint the XML description or the abstract + # tool parser object. + first_arg = inspect.getfullargspec(value).args[0] + if first_arg == "tool_xml": + if tool_xml is None: + # XML linter and non-XML tool, skip for now + continue + else: + lint_context.lint(name, value, tool_xml, silent) + else: + lint_context.lint(name, value, tool_source, silent) + return lint_context + + +def lint_xml_with(lint_context, tool_xml, extra_modules=None) -> LintContext: + extra_modules = extra_modules or [] + tool_source = get_tool_source(xml_tree=tool_xml) + return lint_tool_source_with(lint_context, tool_source, extra_modules=extra_modules) From f3ba64e9775ecfb37647aa026de730f3c61f9c92 Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Thu, 27 Jan 2022 13:48:15 +0100 Subject: [PATCH 15/18] refactoring of LintMessage subclassed XMLLintMessage --- lib/galaxy/tool_util/lint.py | 73 +++++++++++++++--------- lib/galaxy/tool_util/linters/general.py | 37 ++++++------ test/unit/tool_util/test_tool_linters.py | 4 +- 3 files changed, 67 insertions(+), 47 deletions(-) diff --git a/lib/galaxy/tool_util/lint.py b/lib/galaxy/tool_util/lint.py index 0f1a25178e27..8eedf01b1631 100644 --- a/lib/galaxy/tool_util/lint.py +++ b/lib/galaxy/tool_util/lint.py @@ -4,10 +4,9 @@ from typing import ( Callable, List, - NoReturn, Optional, - TypeVar, - Union + Type, + TypeVar ) from galaxy.tool_util.parser import get_tool_source @@ -23,14 +22,11 @@ class LintMessage: """ a message from the linter """ - def __init__(self, level: str, message: str, line, fname, xpath=None): + def __init__(self, level: str, message: str, **kwargs): self.level = level self.message = message - self.line = line # 1 based - self.fname = fname - self.xpath = xpath - def __eq__(self, other: Union[str, 'LintMessage']) -> bool: + def __eq__(self, other) -> bool: """ add equal operator to easy lookup of a message in a List[LintMessage] which is usefull in tests @@ -47,23 +43,40 @@ def __str__(self) -> str: def __repr__(self) -> str: return f"LintMessage({self.message})" - def pretty_print(self, level: bool = True, fname: bool = True, xpath: bool = False) -> str: + def pretty_print(self, level: bool = True, **kwargs) -> str: rval = "" if level: rval += f".. {self.level.upper()}: " rval += "{self.message}" - if fname and self.line is not None: + return rval + + +class XMLLintMessage(LintMessage): + def __init__(self, level: str, message: str, line="", fname="", xpath=None): + super().__init__(level, message) + self.line = line # 1 based + self.fname = fname + self.xpath = xpath + + def pretty_print(self, level: bool = True, **kwargs) -> str: + """ + returns the string produced by LintMessage.pretty_print + plus filename/line and xpath information if kwargs['fname'] or + kwargs['xpath'] is True, respectively + """ + rval = super().pretty_print(level) + if kwargs.get("fname", False) and self.line is not None: rval += " (" if self.fname: rval += f"{self.fname}:" rval += str(self.line) rval += ")" - if xpath and self.xpath is not None: + if kwargs.get("xpath", False) and self.xpath is not None: rval += f" [{self.xpath}]" return rval -LintTargetType = TypeVar('LintTarget') +LintTargetType = TypeVar('LintTargetType') # TODO: Nothing inherently tool-y about LintContext and in fact @@ -71,10 +84,15 @@ def pretty_print(self, level: bool = True, fname: bool = True, xpath: bool = Fal # be moved to galaxy.util.lint. class LintContext: - def __init__(self, level: str, skip_types: Optional[List[str]] = None, object_name: Optional[str] = None): + def __init__(self, + level: str, + lint_message_class: Type[LintMessage] = LintMessage, + skip_types: Optional[List[str]] = None, + object_name: Optional[str] = None): self.skip_types: List[str] = skip_types or [] self.level: str = level - self.object_name: str = object_name + self.lint_message_class = lint_message_class + self.object_name: Optional[str] = object_name self.message_list: List[LintMessage] = [] @property @@ -87,7 +105,7 @@ def found_warns(self) -> bool: def lint(self, name: str, - lint_func: Callable[['LintContext', LintTargetType], NoReturn], + lint_func: Callable[[LintTargetType, 'LintContext'], None], lint_target: LintTargetType, silent: bool = False): name = name.replace("tsts", "tests")[len("lint_"):] @@ -156,23 +174,22 @@ def warn_messages(self) -> List[LintMessage]: def error_messages(self) -> List[LintMessage]: return [x for x in self.message_list if x.level == "error"] - def __handle_message(self, level: str, message: str, line: int, fname: str, xpath: str, *args) -> NoReturn: + def __handle_message(self, level: str, message: str, *args, **kwargs) -> None: if args: message = message % args - self.message_list.append(LintMessage(level=level, message=message, - line=line, fname=fname, xpath=xpath)) + self.message_list.append(self.lint_message_class(level=level, message=message, **kwargs)) - def valid(self, message: str, line: Optional[str] = None, fname: Optional[str] = None, xpath: Optional[str] = None, *args) -> NoReturn: - self.__handle_message("check", message, line, fname, xpath, *args) + def valid(self, message: str, *args, **kwargs) -> None: + self.__handle_message("check", message, *args, **kwargs) - def info(self, message: str, line: Optional[str] = None, fname: Optional[str] = None, xpath: Optional[str] = None, *args) -> NoReturn: - self.__handle_message("info", message, line, fname, xpath, *args) + def info(self, message: str, *args, **kwargs) -> None: + self.__handle_message("info", message, *args, **kwargs) - def error(self, message: str, line: Optional[str] = None, fname: Optional[str] = None, xpath: Optional[str] = None, *args) -> NoReturn: - self.__handle_message("error", message, line, fname, xpath, *args) + def error(self, message: str, *args, **kwargs) -> None: + self.__handle_message("error", message, *args, **kwargs) - def warn(self, message: str, line: Optional[str] = None, fname: Optional[str] = None, xpath: Optional[str] = None, *args) -> NoReturn: - self.__handle_message("warning", message, line, fname, xpath, *args) + def warn(self, message: str, *args, **kwargs) -> None: + self.__handle_message("warning", message, *args, **kwargs) def failed(self, fail_level: str) -> bool: found_warns = self.found_warns @@ -217,7 +234,7 @@ def lint_xml(tool_xml, level=LEVEL_ALL, fail_level=LEVEL_WARN, extra_modules=Non """ extra_modules = extra_modules or [] skip_types = skip_types or [] - lint_context = LintContext(level=level, skip_types=skip_types, object_name=name) + lint_context = LintContext(level=level, lint_message_class=XMLLintMessage, skip_types=skip_types, object_name=name) lint_xml_with(lint_context, tool_xml, extra_modules) return not lint_context.failed(fail_level) @@ -227,10 +244,10 @@ def lint_tool_source_with(lint_context, tool_source, extra_modules=None, silent= extra_modules = extra_modules or [] import galaxy.tool_util.linters tool_xml = getattr(tool_source, "xml_tree", None) + tool_type = tool_source.parse_tool_type() or "default" linter_modules = submodules.import_submodules(galaxy.tool_util.linters, ordered=True) linter_modules.extend(extra_modules) for module in linter_modules: - tool_type = tool_source.parse_tool_type() or "default" lint_tool_types = getattr(module, "lint_tool_types", ["default"]) if not ("*" in lint_tool_types or tool_type in lint_tool_types): continue diff --git a/lib/galaxy/tool_util/linters/general.py b/lib/galaxy/tool_util/linters/general.py index 878f7e9d736c..943b51df605e 100644 --- a/lib/galaxy/tool_util/linters/general.py +++ b/lib/galaxy/tool_util/linters/general.py @@ -3,6 +3,8 @@ import packaging.version +from ._util import node_props_factory + ERROR_VERSION_MSG = "Tool version is missing or empty." WARN_VERSION_MSG = "Tool version [%s] is not compliant with PEP 440." VALID_VERSION_MSG = "Tool defines a version [%s]." @@ -30,45 +32,46 @@ def lint_general(tool_source, lint_ctx): """Check tool version, name, and id.""" # determine line to report for general problems with outputs tool_xml = getattr(tool_source, "xml_tree", None) - try: - tool_line = tool_xml.find("./tool").sourceline - except AttributeError: - tool_line = 0 + node_props = node_props_factory(tool_xml) + if tool_xml: + tool_node = tool_xml.find("./tool") + else: + tool_node = None version = tool_source.parse_version() or '' parsed_version = packaging.version.parse(version) if not version: - lint_ctx.error(ERROR_VERSION_MSG, line=tool_line) + lint_ctx.error(ERROR_VERSION_MSG, **node_props(tool_node)) elif isinstance(parsed_version, packaging.version.LegacyVersion): - lint_ctx.warn(WARN_VERSION_MSG % version, line=tool_line) + lint_ctx.warn(WARN_VERSION_MSG % version, **node_props(tool_node)) elif version != version.strip(): - lint_ctx.warn(WARN_WHITESPACE_PRESUFFIX % ('Tool version', version), line=tool_line) + lint_ctx.warn(WARN_WHITESPACE_PRESUFFIX % ('Tool version', version), **node_props(tool_node)) else: - lint_ctx.valid(VALID_VERSION_MSG % version, line=tool_line) + lint_ctx.valid(VALID_VERSION_MSG % version, **node_props(tool_node)) name = tool_source.parse_name() if not name: - lint_ctx.error(ERROR_NAME_MSG, line=tool_line) + lint_ctx.error(ERROR_NAME_MSG, **node_props(tool_node)) elif name != name.strip(): - lint_ctx.warn(WARN_WHITESPACE_PRESUFFIX % ('Tool name', name), line=tool_line) + lint_ctx.warn(WARN_WHITESPACE_PRESUFFIX % ('Tool name', name), **node_props(tool_node)) else: - lint_ctx.valid(VALID_NAME_MSG % name, line=tool_line) + lint_ctx.valid(VALID_NAME_MSG % name, **node_props(tool_node)) tool_id = tool_source.parse_id() if not tool_id: - lint_ctx.error(ERROR_ID_MSG, line=tool_line) + lint_ctx.error(ERROR_ID_MSG, **node_props(tool_node)) elif re.search(r"\s", tool_id): - lint_ctx.warn(WARN_ID_WHITESPACE_MSG % tool_id, line=tool_line) + lint_ctx.warn(WARN_ID_WHITESPACE_MSG % tool_id, **node_props(tool_node)) else: - lint_ctx.valid(VALID_ID_MSG % tool_id, line=tool_line) + lint_ctx.valid(VALID_ID_MSG % tool_id, **node_props(tool_node)) profile = tool_source.parse_profile() profile_valid = PROFILE_PATTERN.match(profile) is not None if not profile_valid: - lint_ctx.error(PROFILE_INVALID_MSG % profile, line=tool_line) + lint_ctx.error(PROFILE_INVALID_MSG % profile, **node_props(tool_node)) elif profile == "16.01": - lint_ctx.valid(PROFILE_INFO_DEFAULT_MSG, line=tool_line) + lint_ctx.valid(PROFILE_INFO_DEFAULT_MSG, **node_props(tool_node)) else: - lint_ctx.valid(PROFILE_INFO_SPECIFIED_MSG % profile, line=tool_line) + lint_ctx.valid(PROFILE_INFO_SPECIFIED_MSG % profile, **node_props(tool_node)) requirements, containers = tool_source.parse_requirements_and_containers() for r in requirements: diff --git a/test/unit/tool_util/test_tool_linters.py b/test/unit/tool_util/test_tool_linters.py index 27147820ee95..d101d258d680 100644 --- a/test/unit/tool_util/test_tool_linters.py +++ b/test/unit/tool_util/test_tool_linters.py @@ -3,7 +3,7 @@ import pytest -from galaxy.tool_util.lint import lint_tool_source_with, LintContext +from galaxy.tool_util.lint import lint_tool_source_with, LintContext, XMLLintMessage from galaxy.tool_util.linters import ( citations, command, @@ -622,7 +622,7 @@ @pytest.fixture() def lint_ctx(): - return LintContext('all') + return LintContext('all', lint_message_class=XMLLintMessage) def get_xml_tool_source(xml_string): From 665d83e5ef7bfce0efca35d6de6e27a65cc24efb Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Thu, 27 Jan 2022 16:54:39 +0100 Subject: [PATCH 16/18] eliminate node_props --- lib/galaxy/tool_util/lint.py | 18 ++-- lib/galaxy/tool_util/linters/_util.py | 13 --- lib/galaxy/tool_util/linters/citations.py | 23 ++--- lib/galaxy/tool_util/linters/command.py | 23 ++--- lib/galaxy/tool_util/linters/general.py | 30 +++--- lib/galaxy/tool_util/linters/help.py | 22 ++--- lib/galaxy/tool_util/linters/inputs.py | 107 +++++++++++----------- lib/galaxy/tool_util/linters/outputs.py | 35 +++---- lib/galaxy/tool_util/linters/stdio.py | 30 +++--- lib/galaxy/tool_util/linters/tests.py | 46 +++++----- lib/galaxy/tool_util/linters/xml_order.py | 6 +- 11 files changed, 170 insertions(+), 183 deletions(-) diff --git a/lib/galaxy/tool_util/lint.py b/lib/galaxy/tool_util/lint.py index 8eedf01b1631..1ab357dc062f 100644 --- a/lib/galaxy/tool_util/lint.py +++ b/lib/galaxy/tool_util/lint.py @@ -10,8 +10,7 @@ ) from galaxy.tool_util.parser import get_tool_source -from galaxy.util import submodules - +from galaxy.util import etree, submodules LEVEL_ALL = "all" LEVEL_WARN = "warn" @@ -52,11 +51,16 @@ def pretty_print(self, level: bool = True, **kwargs) -> str: class XMLLintMessage(LintMessage): - def __init__(self, level: str, message: str, line="", fname="", xpath=None): + def __init__(self, level: str, message: str, node: Optional[etree.Element] = None): super().__init__(level, message) - self.line = line # 1 based - self.fname = fname - self.xpath = xpath + self.line = None + self.fname = None + self.xpath = None + if node is not None: + self.line = node.sourceline + self.fname = node.base + tool_xml = node.getroottree() + self.xpath = tool_xml.getpath(node) def pretty_print(self, level: bool = True, **kwargs) -> str: """ @@ -65,7 +69,7 @@ def pretty_print(self, level: bool = True, **kwargs) -> str: kwargs['xpath'] is True, respectively """ rval = super().pretty_print(level) - if kwargs.get("fname", False) and self.line is not None: + if kwargs.get("fname", True) and self.line is not None: rval += " (" if self.fname: rval += f"{self.fname}:" diff --git a/lib/galaxy/tool_util/linters/_util.py b/lib/galaxy/tool_util/linters/_util.py index a654527dfed8..3224150c792b 100644 --- a/lib/galaxy/tool_util/linters/_util.py +++ b/lib/galaxy/tool_util/linters/_util.py @@ -1,19 +1,6 @@ import re -def node_props_factory(tool_xml): - - def node_props(node): - if node is None: - return {"line": 0, "fname": None, "xpath": None} - else: - return {"line": node.sourceline, - "fname": node.base, - "xpath": tool_xml.getpath(node)} - - return node_props - - def is_datasource(tool_xml): """Returns true if the tool is a datasource tool""" return tool_xml.getroot().attrib.get('tool_type', '') == 'data_source' diff --git a/lib/galaxy/tool_util/linters/citations.py b/lib/galaxy/tool_util/linters/citations.py index ae55b3993a42..b3940272ef95 100644 --- a/lib/galaxy/tool_util/linters/citations.py +++ b/lib/galaxy/tool_util/linters/citations.py @@ -3,37 +3,38 @@ Citations describe references that should be used when consumers of the tool publish results. """ -from ._util import node_props_factory def lint_citations(tool_xml, lint_ctx): """Ensure tool contains at least one valid citation.""" - root = tool_xml.getroot() - node_props = node_props_factory(tool_xml) - citations = root.findall("citations") + root = tool_xml.find("./citations") + if root is None: + root = tool_xml.getroot() + + citations = tool_xml.findall("citations") if len(citations) > 1: - lint_ctx.error("More than one citation section found, behavior undefined.", **node_props(citations[1])) + lint_ctx.error("More than one citation section found, behavior undefined.", node=citations[1]) return if len(citations) == 0: - lint_ctx.warn("No citations found, consider adding citations to your tool.", **node_props(root)) + lint_ctx.warn("No citations found, consider adding citations to your tool.", node=root) return valid_citations = 0 for citation in citations[0]: if citation.tag != "citation": - lint_ctx.warn(f"Unknown tag discovered in citations block [{citation.tag}], will be ignored.", **node_props(citation)) + lint_ctx.warn(f"Unknown tag discovered in citations block [{citation.tag}], will be ignored.", node=citation) continue citation_type = citation.attrib.get("type") if citation_type not in ('bibtex', 'doi'): - lint_ctx.warn(f"Unknown citation type discovered [{citation_type}], will be ignored.", **node_props(citation)) + lint_ctx.warn(f"Unknown citation type discovered [{citation_type}], will be ignored.", node=citation) continue if citation.text is None or not citation.text.strip(): - lint_ctx.error(f'Empty {citation_type} citation.', **node_props(citation)) + lint_ctx.error(f'Empty {citation_type} citation.', node=citation) continue valid_citations += 1 if valid_citations > 0: - lint_ctx.valid(f"Found {valid_citations} likely valid citations.", **node_props(root)) + lint_ctx.valid(f"Found {valid_citations} likely valid citations.", node=root) else: - lint_ctx.warn("Found no valid citations.", **node_props(root)) + lint_ctx.warn("Found no valid citations.", node=root) diff --git a/lib/galaxy/tool_util/linters/command.py b/lib/galaxy/tool_util/linters/command.py index d5f428bc3e38..9d2dadd1b844 100644 --- a/lib/galaxy/tool_util/linters/command.py +++ b/lib/galaxy/tool_util/linters/command.py @@ -3,27 +3,28 @@ A command description describes how to build the command-line to execute from supplied inputs. """ -from ._util import node_props_factory def lint_command(tool_xml, lint_ctx): """Ensure tool contains exactly one command and check attributes.""" - root = tool_xml.getroot() - node_props = node_props_factory(tool_xml) - commands = root.findall("command") + root = tool_xml.find("./command") + if root is None: + root = tool_xml.getroot() + + commands = tool_xml.findall("./command") if len(commands) > 1: - lint_ctx.error("More than one command tag found, behavior undefined.", **node_props(commands[1])) + lint_ctx.error("More than one command tag found, behavior undefined.", node=commands[1]) return if len(commands) == 0: - lint_ctx.error("No command tag found, must specify a command template to execute.", **node_props(root)) + lint_ctx.error("No command tag found, must specify a command template to execute.", node=root) return command = get_command(tool_xml) if command.text is None: - lint_ctx.error("Command is empty.", **node_props(root)) + lint_ctx.error("Command is empty.", node=root) elif "TODO" in command.text: - lint_ctx.warn("Command template contains TODO text.", **node_props(command)) + lint_ctx.warn("Command template contains TODO text.", node=command) command_attrib = command.attrib interpreter_type = None @@ -33,14 +34,14 @@ def lint_command(tool_xml, lint_ctx): elif key == "detect_errors": detect_errors = value if detect_errors not in ["default", "exit_code", "aggressive"]: - lint_ctx.warn(f"Unknown detect_errors attribute [{detect_errors}]", **node_props(command)) + lint_ctx.warn(f"Unknown detect_errors attribute [{detect_errors}]", node=command) interpreter_info = "" if interpreter_type: interpreter_info = f" with interpreter of type [{interpreter_type}]" if interpreter_type: - lint_ctx.warn("Command uses deprecated 'interpreter' attribute.", **node_props(command)) - lint_ctx.info(f"Tool contains a command{interpreter_info}.", **node_props(command)) + lint_ctx.warn("Command uses deprecated 'interpreter' attribute.", node=command) + lint_ctx.info(f"Tool contains a command{interpreter_info}.", node=command) def get_command(tool_xml): diff --git a/lib/galaxy/tool_util/linters/general.py b/lib/galaxy/tool_util/linters/general.py index 943b51df605e..a7e472544d95 100644 --- a/lib/galaxy/tool_util/linters/general.py +++ b/lib/galaxy/tool_util/linters/general.py @@ -3,7 +3,6 @@ import packaging.version -from ._util import node_props_factory ERROR_VERSION_MSG = "Tool version is missing or empty." WARN_VERSION_MSG = "Tool version [%s] is not compliant with PEP 440." @@ -32,46 +31,45 @@ def lint_general(tool_source, lint_ctx): """Check tool version, name, and id.""" # determine line to report for general problems with outputs tool_xml = getattr(tool_source, "xml_tree", None) - node_props = node_props_factory(tool_xml) if tool_xml: - tool_node = tool_xml.find("./tool") + tool_node = tool_xml.getroot() else: tool_node = None version = tool_source.parse_version() or '' parsed_version = packaging.version.parse(version) if not version: - lint_ctx.error(ERROR_VERSION_MSG, **node_props(tool_node)) + lint_ctx.error(ERROR_VERSION_MSG, node=tool_node) elif isinstance(parsed_version, packaging.version.LegacyVersion): - lint_ctx.warn(WARN_VERSION_MSG % version, **node_props(tool_node)) + lint_ctx.warn(WARN_VERSION_MSG % version, node=tool_node) elif version != version.strip(): - lint_ctx.warn(WARN_WHITESPACE_PRESUFFIX % ('Tool version', version), **node_props(tool_node)) + lint_ctx.warn(WARN_WHITESPACE_PRESUFFIX % ('Tool version', version), node=tool_node) else: - lint_ctx.valid(VALID_VERSION_MSG % version, **node_props(tool_node)) + lint_ctx.valid(VALID_VERSION_MSG % version, node=tool_node) name = tool_source.parse_name() if not name: - lint_ctx.error(ERROR_NAME_MSG, **node_props(tool_node)) + lint_ctx.error(ERROR_NAME_MSG, node=tool_node) elif name != name.strip(): - lint_ctx.warn(WARN_WHITESPACE_PRESUFFIX % ('Tool name', name), **node_props(tool_node)) + lint_ctx.warn(WARN_WHITESPACE_PRESUFFIX % ('Tool name', name), node=tool_node) else: - lint_ctx.valid(VALID_NAME_MSG % name, **node_props(tool_node)) + lint_ctx.valid(VALID_NAME_MSG % name, node=tool_node) tool_id = tool_source.parse_id() if not tool_id: - lint_ctx.error(ERROR_ID_MSG, **node_props(tool_node)) + lint_ctx.error(ERROR_ID_MSG, node=tool_node) elif re.search(r"\s", tool_id): - lint_ctx.warn(WARN_ID_WHITESPACE_MSG % tool_id, **node_props(tool_node)) + lint_ctx.warn(WARN_ID_WHITESPACE_MSG % tool_id, node=tool_node) else: - lint_ctx.valid(VALID_ID_MSG % tool_id, **node_props(tool_node)) + lint_ctx.valid(VALID_ID_MSG % tool_id, node=tool_node) profile = tool_source.parse_profile() profile_valid = PROFILE_PATTERN.match(profile) is not None if not profile_valid: - lint_ctx.error(PROFILE_INVALID_MSG % profile, **node_props(tool_node)) + lint_ctx.error(PROFILE_INVALID_MSG % profile, node=tool_node) elif profile == "16.01": - lint_ctx.valid(PROFILE_INFO_DEFAULT_MSG, **node_props(tool_node)) + lint_ctx.valid(PROFILE_INFO_DEFAULT_MSG, node=tool_node) else: - lint_ctx.valid(PROFILE_INFO_SPECIFIED_MSG % profile, **node_props(tool_node)) + lint_ctx.valid(PROFILE_INFO_SPECIFIED_MSG % profile, node=tool_node) requirements, containers = tool_source.parse_requirements_and_containers() for r in requirements: diff --git a/lib/galaxy/tool_util/linters/help.py b/lib/galaxy/tool_util/linters/help.py index 4e10fed384b5..3d22accabdce 100644 --- a/lib/galaxy/tool_util/linters/help.py +++ b/lib/galaxy/tool_util/linters/help.py @@ -3,38 +3,38 @@ rst_to_html, unicodify, ) -from ._util import node_props_factory def lint_help(tool_xml, lint_ctx): """Ensure tool contains exactly one valid RST help block.""" # determine node to report for general problems with help - root = tool_xml.getroot() - node_props = node_props_factory(tool_xml) - helps = root.findall("help") + root = tool_xml.find("./help") + if root is None: + root = tool_xml.getroot() + helps = tool_xml.findall("./help") if len(helps) > 1: - lint_ctx.error("More than one help section found, behavior undefined.", **node_props(helps[1])) + lint_ctx.error("More than one help section found, behavior undefined.", node=helps[1]) return if len(helps) == 0: - lint_ctx.warn("No help section found, consider adding a help section to your tool.", **node_props(root)) + lint_ctx.warn("No help section found, consider adding a help section to your tool.", node=root) return help = helps[0].text or '' if not help.strip(): - lint_ctx.warn("Help section appears to be empty.", **node_props(helps[0])) + lint_ctx.warn("Help section appears to be empty.", node=helps[0]) return - lint_ctx.valid("Tool contains help section.", **node_props(helps[0])) + lint_ctx.valid("Tool contains help section.", node=helps[0]) invalid_rst = rst_invalid(help) if "TODO" in help: - lint_ctx.warn("Help contains TODO text.", **node_props(helps[0])) + lint_ctx.warn("Help contains TODO text.", node=helps[0]) if invalid_rst: - lint_ctx.warn(f"Invalid reStructuredText found in help - [{invalid_rst}].", **node_props(helps[0])) + lint_ctx.warn(f"Invalid reStructuredText found in help - [{invalid_rst}].", node=helps[0]) else: - lint_ctx.valid("Help contains valid reStructuredText.", **node_props(helps[0])) + lint_ctx.valid("Help contains valid reStructuredText.", node=helps[0]) def rst_invalid(text): diff --git a/lib/galaxy/tool_util/linters/inputs.py b/lib/galaxy/tool_util/linters/inputs.py index 5277b9507c5d..39afa939ed82 100644 --- a/lib/galaxy/tool_util/linters/inputs.py +++ b/lib/galaxy/tool_util/linters/inputs.py @@ -4,8 +4,7 @@ from galaxy.util import string_as_bool from ._util import ( is_datasource, - is_valid_cheetah_placeholder, - node_props_factory + is_valid_cheetah_placeholder ) from ..parser.util import _parse_name @@ -52,34 +51,35 @@ def lint_inputs(tool_xml, lint_ctx): """Lint parameters in a tool's inputs block.""" - # determine node to report for general problems with outputs - node_props = node_props_factory(tool_xml) - tool_node = tool_xml.find("./tool") datasource = is_datasource(tool_xml) inputs = tool_xml.findall("./inputs//param") + # determine node to report for general problems with outputs + tool_node = tool_xml.find("./inputs") + if tool_node is None: + tool_node = tool_xml.getroot() num_inputs = 0 for param in inputs: num_inputs += 1 param_attrib = param.attrib if "name" not in param_attrib and "argument" not in param_attrib: - lint_ctx.error("Found param input with no name specified.", **node_props(param)) + lint_ctx.error("Found param input with no name specified.", node=param) continue param_name = _parse_name(param_attrib.get("name"), param_attrib.get("argument")) if "name" in param_attrib and "argument" in param_attrib: if param_attrib.get("name") == _parse_name(None, param_attrib.get("argument")): - lint_ctx.warn(f"Param input [{param_name}] 'name' attribute is redundant if argument implies the same name.", **node_props(param)) + lint_ctx.warn(f"Param input [{param_name}] 'name' attribute is redundant if argument implies the same name.", node=param) if param_name.strip() == "": - lint_ctx.error("Param input with empty name.", **node_props(param)) + lint_ctx.error("Param input with empty name.", node=param) elif not is_valid_cheetah_placeholder(param_name): - lint_ctx.warn(f"Param input [{param_name}] is not a valid Cheetah placeholder.", **node_props(param)) + lint_ctx.warn(f"Param input [{param_name}] is not a valid Cheetah placeholder.", node=param) # TODO lint for params with duplicated name (in inputs & outputs) if "type" not in param_attrib: - lint_ctx.error(f"Param input [{param_name}] input with no type specified.", **node_props(param)) + lint_ctx.error(f"Param input [{param_name}] input with no type specified.", node=param) continue elif param_attrib["type"].strip() == "": - lint_ctx.error(f"Param input [{param_name}] with empty type specified.", **node_props(param)) + lint_ctx.error(f"Param input [{param_name}] with empty type specified.", node=param) continue param_type = param_attrib["type"] @@ -87,7 +87,7 @@ def lint_inputs(tool_xml, lint_ctx): if param_type == "data": if "format" not in param_attrib: - lint_ctx.warn(f"Param input [{param_name}] with no format specified - 'data' format will be assumed.", **node_props(param)) + lint_ctx.warn(f"Param input [{param_name}] with no format specified - 'data' format will be assumed.", node=param) elif param_type == "select": # get dynamic/statically defined options dynamic_options = param.get("dynamic_options", None) @@ -96,15 +96,15 @@ def lint_inputs(tool_xml, lint_ctx): select_options = param.findall('./option') if dynamic_options is not None: - lint_ctx.warn(f"Select parameter [{param_name}] uses deprecated 'dynamic_options' attribute.", **node_props(param)) + lint_ctx.warn(f"Select parameter [{param_name}] uses deprecated 'dynamic_options' attribute.", node=param) # check if options are defined by exactly one possibility if param.getparent().tag != "conditional": if (dynamic_options is not None) + (len(options) > 0) + (len(select_options) > 0) != 1: - lint_ctx.error(f"Select parameter [{param_name}] options have to be defined by either 'option' children elements, a 'options' element or the 'dynamic_options' attribute.", **node_props(param)) + lint_ctx.error(f"Select parameter [{param_name}] options have to be defined by either 'option' children elements, a 'options' element or the 'dynamic_options' attribute.", node=param) else: if len(select_options) == 0: - lint_ctx.error(f"Select parameter of a conditional [{param_name}] options have to be defined by 'option' children elements.", **node_props(param)) + lint_ctx.error(f"Select parameter of a conditional [{param_name}] options have to be defined by 'option' children elements.", node=param) # lint dynamic options if len(options) == 1: @@ -115,10 +115,10 @@ def lint_inputs(tool_xml, lint_ctx): for f in filters: ftype = f.get("type", None) if ftype is None: - lint_ctx.error(f"Select parameter [{param_name}] contains filter without type.", **node_props(f)) + lint_ctx.error(f"Select parameter [{param_name}] contains filter without type.", node=f) continue if ftype not in FILTER_TYPES: - lint_ctx.error(f"Select parameter [{param_name}] contains filter with unknown type '{ftype}'.", **node_props(f)) + lint_ctx.error(f"Select parameter [{param_name}] contains filter with unknown type '{ftype}'.", node=f) continue if ftype in ['add_value', 'data_meta']: filter_adds_options = True @@ -133,26 +133,26 @@ def lint_inputs(tool_xml, lint_ctx): if (from_file is None and from_parameter is None and from_dataset is None and from_data_table is None and not filter_adds_options): - lint_ctx.error(f"Select parameter [{param_name}] options tag defines no options. Use 'from_dataset', 'from_data_table', or a filter that adds values.", **node_props(options[0])) + lint_ctx.error(f"Select parameter [{param_name}] options tag defines no options. Use 'from_dataset', 'from_data_table', or a filter that adds values.", node=options[0]) for deprecated_attr in ["from_file", "from_parameter", "options_filter_attribute", "transform_lines"]: if options[0].get(deprecated_attr) is not None: - lint_ctx.warn(f"Select parameter [{param_name}] options uses deprecated '{deprecated_attr}' attribute.", **node_props(options[0])) + lint_ctx.warn(f"Select parameter [{param_name}] options uses deprecated '{deprecated_attr}' attribute.", node=options[0]) if from_dataset is not None and from_data_table is not None: - lint_ctx.error(f"Select parameter [{param_name}] options uses 'from_dataset' and 'from_data_table' attribute.", **node_props(options[0])) + lint_ctx.error(f"Select parameter [{param_name}] options uses 'from_dataset' and 'from_data_table' attribute.", node=options[0]) if options[0].get("meta_file_key", None) is not None and from_dataset is None: - lint_ctx.error(f"Select parameter [{param_name}] 'meta_file_key' is only compatible with 'from_dataset'.", **node_props(options[0])) + lint_ctx.error(f"Select parameter [{param_name}] 'meta_file_key' is only compatible with 'from_dataset'.", node=options[0]) elif len(options) > 1: - lint_ctx.error(f"Select parameter [{param_name}] contains multiple options elements.", **node_props(options[1])) + lint_ctx.error(f"Select parameter [{param_name}] contains multiple options elements.", node=options[1]) # lint statically defined options if any('value' not in option.attrib for option in select_options): - lint_ctx.error(f"Select parameter [{param_name}] has option without value", **node_props(param)) + lint_ctx.error(f"Select parameter [{param_name}] has option without value", node=param) if any(option.text is None for option in select_options): - lint_ctx.warn(f"Select parameter [{param_name}] has option without text", **node_props(param)) + lint_ctx.warn(f"Select parameter [{param_name}] has option without text", node=param) select_options_texts = list() select_options_values = list() @@ -165,22 +165,22 @@ def lint_inputs(tool_xml, lint_ctx): select_options_texts.append((text, option.attrib.get("selected", "false"))) select_options_values.append((value, option.attrib.get("selected", "false"))) if len(set(select_options_texts)) != len(select_options_texts): - lint_ctx.error(f"Select parameter [{param_name}] has multiple options with the same text content", **node_props(param)) + lint_ctx.error(f"Select parameter [{param_name}] has multiple options with the same text content", node=param) if len(set(select_options_values)) != len(select_options_values): - lint_ctx.error(f"Select parameter [{param_name}] has multiple options with the same value", **node_props(param)) + lint_ctx.error(f"Select parameter [{param_name}] has multiple options with the same value", node=param) multiple = string_as_bool(param_attrib.get("multiple", "false")) optional = string_as_bool(param_attrib.get("optional", multiple)) if param_attrib.get("display") == "checkboxes": if not multiple: - lint_ctx.error(f'Select [{param_name}] `display="checkboxes"` is incompatible with `multiple="false"`, remove the `display` attribute', **node_props(param)) + lint_ctx.error(f'Select [{param_name}] `display="checkboxes"` is incompatible with `multiple="false"`, remove the `display` attribute', node=param) if not optional: - lint_ctx.error(f'Select [{param_name}] `display="checkboxes"` is incompatible with `optional="false"`, remove the `display` attribute', **node_props(param)) + lint_ctx.error(f'Select [{param_name}] `display="checkboxes"` is incompatible with `optional="false"`, remove the `display` attribute', node=param) if param_attrib.get("display") == "radio": if multiple: - lint_ctx.error(f'Select [{param_name}] display="radio" is incompatible with multiple="true"', **node_props(param)) + lint_ctx.error(f'Select [{param_name}] display="radio" is incompatible with multiple="true"', node=param) if optional: - lint_ctx.error(f'Select [{param_name}] display="radio" is incompatible with optional="true"', **node_props(param)) + lint_ctx.error(f'Select [{param_name}] display="radio" is incompatible with optional="true"', node=param) # TODO: Validate type, much more... # lint validators @@ -190,45 +190,45 @@ def lint_inputs(tool_xml, lint_ctx): vtype = validator.attrib['type'] if param_type in PARAMETER_VALIDATOR_TYPE_COMPATIBILITY: if vtype not in PARAMETER_VALIDATOR_TYPE_COMPATIBILITY[param_type]: - lint_ctx.error(f"Parameter [{param_name}]: validator with an incompatible type '{vtype}'", **node_props(validator)) + lint_ctx.error(f"Parameter [{param_name}]: validator with an incompatible type '{vtype}'", node=validator) for attrib in ATTRIB_VALIDATOR_COMPATIBILITY: if attrib in validator.attrib and vtype not in ATTRIB_VALIDATOR_COMPATIBILITY[attrib]: - lint_ctx.error(f"Parameter [{param_name}]: attribute '{attrib}' is incompatible with validator of type '{vtype}'", **node_props(validator)) + lint_ctx.error(f"Parameter [{param_name}]: attribute '{attrib}' is incompatible with validator of type '{vtype}'", node=validator) if vtype == "expression": if validator.text is None: - lint_ctx.error(f"Parameter [{param_name}]: expression validators are expected to contain text", **node_props(validator)) + lint_ctx.error(f"Parameter [{param_name}]: expression validators are expected to contain text", node=validator) else: try: re.compile(validator.text) except Exception as e: - lint_ctx.error(f"Parameter [{param_name}]: '{validator.text}' is no valid regular expression: {str(e)}", **node_props(validator)) + lint_ctx.error(f"Parameter [{param_name}]: '{validator.text}' is no valid regular expression: {str(e)}", node=validator) if vtype not in ["expression", "regex"] and validator.text is not None: - lint_ctx.warn(f"Parameter [{param_name}]: '{vtype}' validators are not expected to contain text (found '{validator.text}')", **node_props(validator)) + lint_ctx.warn(f"Parameter [{param_name}]: '{vtype}' validators are not expected to contain text (found '{validator.text}')", node=validator) if vtype in ["in_range", "length", "dataset_metadata_in_range"] and ("min" not in validator.attrib and "max" not in validator.attrib): - lint_ctx.error(f"Parameter [{param_name}]: '{vtype}' validators need to define the 'min' or 'max' attribute(s)", **node_props(validator)) + lint_ctx.error(f"Parameter [{param_name}]: '{vtype}' validators need to define the 'min' or 'max' attribute(s)", node=validator) if vtype in ["metadata"] and ("check" not in validator.attrib and "skip" not in validator.attrib): - lint_ctx.error(f"Parameter [{param_name}]: '{vtype}' validators need to define the 'check' or 'skip' attribute(s)", **node_props(validator)) + lint_ctx.error(f"Parameter [{param_name}]: '{vtype}' validators need to define the 'check' or 'skip' attribute(s)", node=validator) if vtype in ["value_in_data_table", "value_not_in_data_table", "dataset_metadata_in_data_table", "dataset_metadata_not_in_data_table"] and "table_name" not in validator.attrib: - lint_ctx.error(f"Parameter [{param_name}]: '{vtype}' validators need to define the 'table_name' attribute", **node_props(validator)) + lint_ctx.error(f"Parameter [{param_name}]: '{vtype}' validators need to define the 'table_name' attribute", node=validator) conditional_selects = tool_xml.findall("./inputs//conditional") for conditional in conditional_selects: conditional_name = conditional.get('name') if not conditional_name: - lint_ctx.error("Conditional without a name", **node_props(conditional)) + lint_ctx.error("Conditional without a name", node=conditional) 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_props(conditional)) + lint_ctx.error(f"Conditional [{conditional_name}] needs exactly one child found {len(first_param)}", node=conditional) continue first_param = first_param[0] first_param_type = first_param.get('type') if first_param_type == 'boolean': - lint_ctx.warn(f'Conditional [{conditional_name}] first param of type="boolean" is discouraged, use a select', **node_props(first_param)) + lint_ctx.warn(f'Conditional [{conditional_name}] first param of type="boolean" is discouraged, use a select', node=first_param) elif first_param_type != 'select': - lint_ctx.error(f'Conditional [{conditional_name}] first param should have type="select"', **node_props(first_param)) + lint_ctx.error(f'Conditional [{conditional_name}] first param should have type="select"', node=first_param) continue if first_param_type == 'select': @@ -242,49 +242,48 @@ def lint_inputs(tool_xml, lint_ctx): for incomp in ["optional", "multiple"]: if string_as_bool(first_param.get(incomp, False)): - lint_ctx.warn(f'Conditional [{conditional_name}] test parameter cannot be {incomp}="true"', **node_props(first_param)) + lint_ctx.warn(f'Conditional [{conditional_name}] test parameter cannot be {incomp}="true"', node=first_param) 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_props(conditional)) + lint_ctx.error(f"Conditional [{conditional_name}] when without value", node=conditional) when_ids = [w.get('value') for w in whens if w.get('value') is not None] for option_id in option_ids: if option_id not in when_ids: - lint_ctx.warn(f"Conditional [{conditional_name}] no block found for {first_param_type} option '{option_id}'", **node_props(conditional)) + lint_ctx.warn(f"Conditional [{conditional_name}] no block found for {first_param_type} option '{option_id}'", node=conditional) for when_id in when_ids: if when_id not in option_ids: if first_param_type == 'select': - lint_ctx.warn(f"Conditional [{conditional_name}] no