opensource/galaxy
3
0
Fork 0
mirror of https://github.com/galaxyproject/galaxy.git synced 2026-09-24 16:30:27 +08:00
Code Issues Packages Projects Releases Wiki Activity

eliminate node_props

Browse Source
This commit is contained in:
Matthias Bernt
2022-01-27 17:14:46 +01:00
parent f3ba64e977
commit 665d83e5ef
11 changed files with 170 additions and 183 deletions
Download Patch File Download Diff File Expand all files Collapse all files
lib/galaxy/tool_util/lint.py
+11 -7
View File
@@ -10,8 +10,7 @@ from typing import (
)
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 @@ class LintMessage:
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 @@ class XMLLintMessage(LintMessage):
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}:"
lib/galaxy/tool_util/linters/_util.py
-13
View File
@@ -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'
lib/galaxy/tool_util/linters/citations.py
+12 -11
View File
@@ -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)
lib/galaxy/tool_util/linters/command.py
+12 -11
View File
@@ -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):
lib/galaxy/tool_util/linters/general.py
+14 -16
View File
@@ -3,7 +3,6 @@ import re
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:
lib/galaxy/tool_util/linters/help.py
+11 -11
View File
@@ -3,38 +3,38 @@ from galaxy.util import (
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):
lib/galaxy/tool_util/linters/inputs.py
+53 -54
View File
@@ -4,8 +4,7 @@ import re
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 @@ PARAMETER_VALIDATOR_TYPE_COMPATIBILITY = {
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 <param> found {len(first_param)}", **node_props(conditional))
lint_ctx.error(f"Conditional [{conditional_name}] needs exactly one child <param> 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 <when /> block found for {first_param_type} option '{option_id}'", **node_props(conditional))
lint_ctx.warn(f"Conditional [{conditional_name}] no <when /> 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 <option /> found for when block '{when_id}'", **node_props(conditional))
lint_ctx.warn(f"Conditional [{conditional_name}] no <option /> found for when block '{when_id}'", node=conditional)
else:
lint_ctx.warn(f"Conditional [{conditional_name}] no truevalue/falsevalue found for when block '{when_id}'", **node_props(conditional))
lint_ctx.warn(f"Conditional [{conditional_name}] no truevalue/falsevalue found for when block '{when_id}'", node=conditional)
if datasource:
# TODO only display is subtag of inputs, uihints is a separate top level tag (supporting only attrib minwidth)
for datasource_tag in ('display', 'uihints'):
if not any(param.tag == datasource_tag for param in inputs):
lint_ctx.info(f"{datasource_tag} tag usually present in data sources", **node_props(tool_node))
lint_ctx.info(f"{datasource_tag} tag usually present in data sources", node=tool_node)
if num_inputs:
lint_ctx.info(f"Found {num_inputs} input parameters.", **node_props(tool_node))
lint_ctx.info(f"Found {num_inputs} input parameters.", node=tool_node)
else:
if datasource:
lint_ctx.info("No input parameters, OK for data sources", **node_props(tool_node))
lint_ctx.info("No input parameters, OK for data sources", node=tool_node)
else:
lint_ctx.warn("Found no input parameters.", **node_props(tool_node))
lint_ctx.warn("Found no input parameters.", node=tool_node)
def lint_repeats(tool_xml, lint_ctx):
"""Lint repeat blocks in tool inputs."""
repeats = tool_xml.findall("./inputs//repeat")
node_props = node_props_factory(tool_xml)
for repeat in repeats:
if "name" not in repeat.attrib:
lint_ctx.error("Repeat does not specify name attribute.", **node_props(repeat))
lint_ctx.error("Repeat does not specify name attribute.", node=repeat)
if "title" not in repeat.attrib:
lint_ctx.error("Repeat does not specify title attribute.", **node_props(repeat))
lint_ctx.error("Repeat does not specify title attribute.", node=repeat)
def _find_with_attribute(element, tag, attribute, test_value=None):
lib/galaxy/tool_util/linters/outputs.py
+18 -17
View File
@@ -1,34 +1,35 @@
"""This module contains a linting functions for tool outputs."""
from galaxy.util import string_as_bool
from ._util import is_valid_cheetah_placeholder, node_props_factory
from ._util import is_valid_cheetah_placeholder
from ..parser.output_collection_def import NAMED_PATTERNS
def lint_output(tool_xml, lint_ctx):
"""Check output elements, ensure there is at least one and check attributes."""
# determine node to report for general problems with outputs
node_props = node_props_factory(tool_xml)
tool_node = tool_xml.find("./tool")
outputs = tool_xml.findall("./outputs")
# determine node to report for general problems with outputs
tool_node = tool_xml.find("./outputs")
if tool_node is None:
tool_node = tool_xml.getroot()
if len(outputs) == 0:
lint_ctx.warn("Tool contains no outputs section, most tools should produce outputs.", **node_props(tool_node))
lint_ctx.warn("Tool contains no outputs section, most tools should produce outputs.", node=tool_node)
return
if len(outputs) > 1:
lint_ctx.warn("Tool contains multiple output sections, behavior undefined.", **node_props(outputs[1]))
lint_ctx.warn("Tool contains multiple output sections, behavior undefined.", node=outputs[1])
num_outputs = 0
for output in list(outputs[0]):
if output.tag not in ["data", "collection"]:
lint_ctx.warn(f"Unknown element found in outputs [{output.tag}]", **node_props(output))
lint_ctx.warn(f"Unknown element found in outputs [{output.tag}]", node=output)
continue
num_outputs += 1
if "name" not in output.attrib:
lint_ctx.warn("Tool output doesn't define a name - this is likely a problem.", **node_props(output))
lint_ctx.warn("Tool output doesn't define a name - this is likely a problem.", node=output)
# TODO make this an error if there is no discover_datasets / from_work_dir (is this then still a problem)
elif not is_valid_cheetah_placeholder(output.attrib["name"]):
lint_ctx.warn(f'Tool output name [{output.attrib["name"]}] is not a valid Cheetah placeholder.', **node_props(output))
lint_ctx.warn(f'Tool output name [{output.attrib["name"]}] is not a valid Cheetah placeholder.', node=output)
format_set = False
if __check_format(output, lint_ctx, node_props):
if __check_format(output, lint_ctx):
format_set = True
if output.tag == "data":
if "auto_format" in output.attrib and output.attrib["auto_format"]:
@@ -36,30 +37,30 @@ def lint_output(tool_xml, lint_ctx):
elif output.tag == "collection":
if "type" not in output.attrib:
lint_ctx.warn("Collection output with undefined 'type' found.", **node_props(output))
lint_ctx.warn("Collection output with undefined 'type' found.", node=output)
if "structured_like" in output.attrib and "inherit_format" in output.attrib:
format_set = True
for sub in output:
if __check_pattern(sub):
format_set = True
elif __check_format(sub, lint_ctx, node_props, allow_ext=True):
elif __check_format(sub, lint_ctx, allow_ext=True):
format_set = True
if not format_set:
lint_ctx.warn(f"Tool {output.tag} output {output.attrib.get('name', 'with missing name')} doesn't define an output format.", **node_props(output))
lint_ctx.warn(f"Tool {output.tag} output {output.attrib.get('name', 'with missing name')} doesn't define an output format.", node=output)
# TODO: check for different labels in case of multiple outputs
lint_ctx.info(f"{num_outputs} outputs found.", line=outputs[0].sourceline)
lint_ctx.info(f"{num_outputs} outputs found.", node=outputs[0])
def __check_format(node, lint_ctx, node_props, allow_ext=False):
def __check_format(node, lint_ctx, allow_ext=False):
"""
check if format/ext/format_source attribute is set in a given node
issue a warning if the value is input
return true (node defines format/ext) / false (else)
"""
if "format_source" in node.attrib and ("ext" in node.attrib or "format" in node.attrib):
lint_ctx.warn(f"Tool {node.tag} output '{node.attrib.get('name', 'with missing name')}' should use either format_source or format/ext", **node_props(node))
lint_ctx.warn(f"Tool {node.tag} output '{node.attrib.get('name', 'with missing name')}' should use either format_source or format/ext", node=node)
if "format_source" in node.attrib:
return True
# if allowed (e.g. for discover_datasets), ext takes precedence over format
@@ -69,7 +70,7 @@ def __check_format(node, lint_ctx, node_props, allow_ext=False):
if fmt is None:
fmt = node.attrib.get("format")
if fmt == "input":
lint_ctx.warn(f"Using format='input' on {node.tag}, format_source attribute is less ambiguous and should be used instead.", **node_props(node))
lint_ctx.warn(f"Using format='input' on {node.tag}, format_source attribute is less ambiguous and should be used instead.", node=node)
return fmt is not None
lib/galaxy/tool_util/linters/stdio.py
+15 -15
View File
@@ -1,7 +1,6 @@
"""This module contains a linting functions for tool error detection."""
import re
from ._util import node_props_factory
from .command import get_command
@@ -11,49 +10,50 @@ def lint_stdio(tool_source, lint_ctx):
# Can only lint XML tools at this point.
# Should probably use tool_source.parse_stdio() to abstract away XML details
return
# determine node to report for general problems with stdio
node_props = node_props_factory(tool_xml)
tool_node = tool_xml.find("./tool")
stdios = tool_xml.findall("./stdio") if tool_xml else []
# determine node to report for general problems with stdio
tool_node = tool_xml.find("./stdio")
if tool_node is None:
tool_node = tool_xml.getroot()
if not stdios:
command = get_command(tool_xml) if tool_xml else None
if command is None or not command.get("detect_errors"):
if tool_source.parse_profile() <= "16.01":
lint_ctx.info("No stdio definition found, tool indicates error conditions with output written to stderr.", **node_props(tool_node))
lint_ctx.info("No stdio definition found, tool indicates error conditions with output written to stderr.", node=tool_node)
else:
lint_ctx.info("No stdio definition found, tool indicates error conditions with non-zero exit codes.", **node_props(tool_node))
lint_ctx.info("No stdio definition found, tool indicates error conditions with non-zero exit codes.", node=tool_node)
return
if len(stdios) > 1:
lint_ctx.error("More than one stdio tag found, behavior undefined.", **node_props(stdios[1]))
lint_ctx.error("More than one stdio tag found, behavior undefined.", node=stdios[1])
return
stdio = stdios[0]
for child in list(stdio):
if child.tag == "regex":
_lint_regex(tool_xml, child, lint_ctx, node_props)
_lint_regex(tool_xml, child, lint_ctx)
elif child.tag == "exit_code":
_lint_exit_code(tool_xml, child, lint_ctx, node_props)
_lint_exit_code(tool_xml, child, lint_ctx)
else:
message = "Unknown stdio child tag discovered [%s]. "
message += "Valid options are exit_code and regex."
lint_ctx.warn(message % child.tag, **node_props(child))
lint_ctx.warn(message % child.tag, node=child)
def _lint_exit_code(tool_xml, child, lint_ctx, node_props):
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.", **node_props(child))
lint_ctx.warn(f"Unknown attribute [{key}] encountered on exit_code tag.", node=child)
def _lint_regex(tool_xml, child, lint_ctx, node_props):
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.", **node_props(child))
lint_ctx.warn(f"Unknown attribute [{key}] encountered on regex tag.", node=child)
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)}", **node_props(child))
lint_ctx.error(f"Match '{match}' is no valid regular expression: {str(e)}", node=child)
lib/galaxy/tool_util/linters/tests.py
+22 -24
View File
@@ -1,25 +1,23 @@
"""This module contains a linting functions for tool tests."""
from inspect import Parameter, signature
from ._util import is_datasource, node_props_factory
from ._util import is_datasource
from ..verify import asserts
# Misspelled so as not be picked up by nosetests.
def lint_tsts(tool_xml, lint_ctx):
node_props = node_props_factory(tool_xml)
# determine node to report for general problems with tests
tests = tool_xml.findall("./tests/test")
general_node = tool_xml.find("./tests")
if general_node is None:
general_node = tool_xml.find("./tool")
tests = tool_xml.findall("./tests/test")
general_node = tool_xml.getroot()
datasource = is_datasource(tool_xml)
if not tests:
if not datasource:
lint_ctx.warn("No tests found, most tools should define test cases.", **node_props(general_node))
lint_ctx.warn("No tests found, most tools should define test cases.", node=general_node)
elif datasource:
lint_ctx.info("No tests found, that should be OK for data_sources.", **node_props(general_node))
lint_ctx.info("No tests found, that should be OK for data_sources.", node=general_node)
return
num_valid_tests = 0
@@ -38,14 +36,14 @@ def lint_tsts(tool_xml, lint_ctx):
if len(assertions) > 1:
lint_ctx.error("Test {test_idx}: More than one {ta} found. Only the first is considered.")
has_test = True
_check_asserts(test_idx, assertions, lint_ctx, node_props)
_check_asserts(test_idx, test.findall(".//assert_contents"), lint_ctx, node_props)
_check_asserts(test_idx, assertions, lint_ctx)
_check_asserts(test_idx, test.findall(".//assert_contents"), lint_ctx)
# really simple test that test parameters are also present in the inputs
for param in test.findall("param"):
name = param.attrib.get("name", None)
if not name:
lint_ctx.error(f"Test {test_idx}: Found test param tag without a name defined.", **node_props(param))
lint_ctx.error(f"Test {test_idx}: Found test param tag without a name defined.", node=param)
continue
name = name.split("|")[-1]
xpaths = [f"@name='{name}'",
@@ -63,7 +61,7 @@ def lint_tsts(tool_xml, lint_ctx):
found = True
break
if not found:
lint_ctx.error(f"Test {test_idx}: Test param {name} not found in the inputs", **node_props(param))
lint_ctx.error(f"Test {test_idx}: Test param {name} not found in the inputs", node=param)
output_data_names, output_collection_names = _collect_output_names(tool_xml)
found_output_test = False
@@ -75,28 +73,28 @@ def lint_tsts(tool_xml, lint_ctx):
else:
valid_names = output_collection_names
if not name:
lint_ctx.error(f"Test {test_idx}: Found {output.tag} tag without a name defined.", **node_props(output))
lint_ctx.error(f"Test {test_idx}: Found {output.tag} tag without a name defined.", node=output)
else:
if name not in valid_names:
lint_ctx.error(f"Test {test_idx}: Found {output.tag} tag with unknown name [{name}], valid names [{valid_names}]", **node_props(output))
lint_ctx.error(f"Test {test_idx}: Found {output.tag} tag with unknown name [{name}], valid names [{valid_names}]", node=output)
if "expect_failure" in test.attrib and found_output_test:
lint_ctx.error(f"Test {test_idx}: Cannot specify outputs in a test expecting failure.", **node_props(test))
lint_ctx.error(f"Test {test_idx}: Cannot specify outputs in a test expecting failure.", node=test)
continue
has_test = has_test or found_output_test
if not has_test:
lint_ctx.warn(f"Test {test_idx}: No outputs or expectations defined for tests, this test is likely invalid.", **node_props(test))
lint_ctx.warn(f"Test {test_idx}: No outputs or expectations defined for tests, this test is likely invalid.", node=test)
else:
num_valid_tests += 1
if num_valid_tests or datasource:
lint_ctx.valid(f"{num_valid_tests} test(s) found.", **node_props(general_node))
lint_ctx.valid(f"{num_valid_tests} test(s) found.", node=general_node)
else:
lint_ctx.warn("No valid test(s) found.", **node_props(general_node))
lint_ctx.warn("No valid test(s) found.", node=general_node)
def _check_asserts(test_idx, assertions, lint_ctx, node_props):
def _check_asserts(test_idx, assertions, lint_ctx):
"""
assertions is a list of assert_contents, assert_stdout, assert_stderr, assert_command
in practice only for the first case the list may be longer than one
@@ -107,13 +105,13 @@ def _check_asserts(test_idx, assertions, lint_ctx, node_props):
continue
assert_function_name = "assert_" + a.tag
if assert_function_name not in asserts.assertion_functions:
lint_ctx.error(f"Test {test_idx}: unknown assertion '{a.tag}'", **node_props(a))
lint_ctx.error(f"Test {test_idx}: unknown assertion '{a.tag}'", node=a)
continue
assert_function_sig = signature(asserts.assertion_functions[assert_function_name])
# check type of the attributes (int, float ...)
for attrib in a.attrib:
if attrib not in assert_function_sig.parameters:
lint_ctx.error(f"Test {test_idx}: unknown attribute '{attrib}' for '{a.tag}'", **node_props(a))
lint_ctx.error(f"Test {test_idx}: unknown attribute '{attrib}' for '{a.tag}'", node=a)
continue
if assert_function_sig.parameters[attrib].annotation is not Parameter.empty:
try:
@@ -121,20 +119,20 @@ def _check_asserts(test_idx, assertions, lint_ctx, node_props):
except ValueError:
lint_ctx.error(
f"Test {test_idx}: attribute '{attrib}' for '{a.tag}' needs to be '{assert_function_sig.parameters[attrib].annotation.__name__}' got '{a.attrib[attrib]}'",
**node_props(a))
node=a)
# check missing required attributes
for p in assert_function_sig.parameters:
if p in ["output", "output_bytes", "verify_assertions_function", "children"]:
continue
if assert_function_sig.parameters[p].default is Parameter.empty and p not in a.attrib:
lint_ctx.error(f"Test {test_idx}: missing attribute '{p}' for '{a.tag}'", **node_props(a))
lint_ctx.error(f"Test {test_idx}: missing attribute '{p}' for '{a.tag}'", node=a)
# has_n_lines, has_n_columns, and has_size need to specify n/value, min, or max
if a.tag in ["has_n_lines", "has_n_columns"]:
if "n" not in a.attrib and "min" not in a.attrib and "max" not in a.attrib:
lint_ctx.error(f"Test {test_idx}: '{a.tag}' needs to specify 'n', 'min', or 'max'", **node_props(a))
lint_ctx.error(f"Test {test_idx}: '{a.tag}' needs to specify 'n', 'min', or 'max'", node=a)
if a.tag == "has_size":
if "value" not in a.attrib and "min" not in a.attrib and "max" not in a.attrib:
lint_ctx.error(f"Test {test_idx}: '{a.tag}' needs to specify 'n', 'min', or 'max'", **node_props(a))
lint_ctx.error(f"Test {test_idx}: '{a.tag}' needs to specify 'n', 'min', or 'max'", node=a)
def _collect_output_names(tool_xml):
lib/galaxy/tool_util/linters/xml_order.py
+2 -4
View File
@@ -3,7 +3,6 @@
For more information on the IUC standard for XML block order see -
https://github.com/galaxy-iuc/standards.
"""
from ._util import node_props_factory
# https://github.com/galaxy-iuc/standards
# https://github.com/galaxy-iuc/standards/pull/7/files
@@ -47,7 +46,6 @@ DATASOURCE_TAG_ORDER = [
# by the tool author best practices.
def lint_xml_order(tool_xml, lint_ctx):
tool_root = tool_xml.getroot()
node_props = node_props_factory(tool_xml)
if tool_root.attrib.get('tool_type', '') == 'data_source':
tag_ordering = DATASOURCE_TAG_ORDER
@@ -62,8 +60,8 @@ def lint_xml_order(tool_xml, lint_ctx):
key = tag_ordering.index(tag)
if last_key:
if last_key > key:
lint_ctx.warn(f"Best practice violation [{tag}] elements should come before [{last_tag}]", **node_props(elem))
lint_ctx.warn(f"Best practice violation [{tag}] elements should come before [{last_tag}]", node=elem)
last_tag = tag
last_key = key
else:
lint_ctx.info(f"Unknown tag [{tag}] encountered, this may result in a warning in the future.", **node_props(elem))
lint_ctx.info(f"Unknown tag [{tag}] encountered, this may result in a warning in the future.", node=elem)