diff --git a/lib/galaxy/tool_util/lint.py b/lib/galaxy/tool_util/lint.py index aac34bb1cae..91435adfac1 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 @@ class LintContext: 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 @@ class LintContext: 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 @@ class LintContext: 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 3224150c792..03e728b38ef 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 444fecf0650..6bab16702f3 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 231cd6a5a0c..fa76d487e3a 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 69ddd9efd8b..784961eef20 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 2667966856d..b62392d8bc3 100644 --- a/lib/galaxy/tool_util/linters/help.py +++ b/lib/galaxy/tool_util/linters/help.py @@ -3,42 +3,37 @@ from galaxy.util import ( 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 93557997224..a800dd58a0d 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 @@ PARAMETER_VALIDATOR_TYPE_COMPATIBILITY = { 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