mirror of
https://github.com/galaxyproject/galaxy.git
synced 2026-09-24 16:30:27 +08:00
Merge pull request #12978 from bernt-matthias/topic/lint-context
provide more context for lint context
This commit is contained in:
Marius van den Beek
@@ -63,6 +63,23 @@ def lint_xml_with(lint_context, tool_xml, extra_modules=None):
|
||||
return lint_tool_source_with(lint_context, tool_source, extra_modules=extra_modules)
|
||||
|
||||
|
||||
class LintMessage:
|
||||
def __init__(self, level, message, line, xpath=None):
|
||||
self.level = level
|
||||
self.message = message
|
||||
self.line = line
|
||||
self.xpath = xpath
|
||||
|
||||
def __str__(self):
|
||||
rval = f".. {self.level.upper()}: {self.message}"
|
||||
if self.line is not None:
|
||||
rval += f" (line {self.line})"
|
||||
if self.xpath is not None:
|
||||
rval += f" [{self.xpath}]"
|
||||
|
||||
return rval
|
||||
|
||||
|
||||
# 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.
|
||||
@@ -80,10 +97,9 @@ class LintContext:
|
||||
if name in self.skip_types:
|
||||
return
|
||||
self.printed_linter_info = False
|
||||
self.valid_messages = []
|
||||
self.info_messages = []
|
||||
self.warn_messages = []
|
||||
self.error_messages = []
|
||||
self.message_list = []
|
||||
|
||||
# call linter
|
||||
lint_func(lint_target, self)
|
||||
# TODO: colorful emoji if in click CLI.
|
||||
if self.error_messages:
|
||||
@@ -99,41 +115,65 @@ class LintContext:
|
||||
self.printed_linter_info = True
|
||||
print(f"Applying linter {name}... {status}")
|
||||
|
||||
for message in self.error_messages:
|
||||
for message in self.message_list:
|
||||
if message.level != "error":
|
||||
continue
|
||||
self.found_errors = True
|
||||
print_linter_info()
|
||||
print(f".. ERROR: {message}")
|
||||
print(f"{message}")
|
||||
|
||||
if self.level != LEVEL_ERROR:
|
||||
for message in self.warn_messages:
|
||||
for message in self.message_list:
|
||||
if message.level != "warning":
|
||||
continue
|
||||
self.found_warns = True
|
||||
print_linter_info()
|
||||
print(f".. WARNING: {message}")
|
||||
print(f"{message}")
|
||||
|
||||
if self.level == LEVEL_ALL:
|
||||
for message in self.info_messages:
|
||||
for message in self.message_list:
|
||||
if message.level != "info":
|
||||
continue
|
||||
print_linter_info()
|
||||
print(f".. INFO: {message}")
|
||||
for message in self.valid_messages:
|
||||
print(f"{message}")
|
||||
for message in self.message_list:
|
||||
if message.level != "check":
|
||||
continue
|
||||
print_linter_info()
|
||||
print(f".. CHECK: {message}")
|
||||
print(f"{message}")
|
||||
|
||||
def __handle_message(self, message_list, message, *args):
|
||||
@property
|
||||
def valid_messages(self):
|
||||
return [x.message 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"]
|
||||
|
||||
@property
|
||||
def warn_messages(self):
|
||||
return [x.message 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 __handle_message(self, level, message, line, xpath, *args):
|
||||
if args:
|
||||
message = message % args
|
||||
message_list.append(message)
|
||||
self.message_list.append(LintMessage(level=level, message=message, line=line, xpath=xpath))
|
||||
|
||||
def valid(self, message, *args):
|
||||
self.__handle_message(self.valid_messages, message, *args)
|
||||
def valid(self, message, line=None, xpath=None, *args):
|
||||
self.__handle_message("check", message, line, xpath, *args)
|
||||
|
||||
def info(self, message, *args):
|
||||
self.__handle_message(self.info_messages, message, *args)
|
||||
def info(self, message, line=None, xpath=None, *args):
|
||||
self.__handle_message("info", message, line, xpath, *args)
|
||||
|
||||
def error(self, message, *args):
|
||||
self.__handle_message(self.error_messages, message, *args)
|
||||
def error(self, message, line=None, xpath=None, *args):
|
||||
self.__handle_message("error", message, line, xpath, *args)
|
||||
|
||||
def warn(self, message, *args):
|
||||
self.__handle_message(self.warn_messages, message, *args)
|
||||
def warn(self, message, line=None, xpath=None, *args):
|
||||
self.__handle_message("warning", message, line, xpath, *args)
|
||||
|
||||
def failed(self, fail_level):
|
||||
found_warns = self.found_warns
|
||||
|
||||
@@ -8,28 +8,36 @@ of the tool publish results.
|
||||
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.")
|
||||
lint_ctx.error("More than one citation section found, behavior undefined.", line=citations[1].sourceline, xpath=tool_xml.getpath(citations[1]))
|
||||
return
|
||||
|
||||
if len(citations) == 0:
|
||||
lint_ctx.warn("No citations found, consider adding citations to your tool.")
|
||||
lint_ctx.warn("No citations found, consider adding citations to your tool.", line=root_line, xpath=root_xpath)
|
||||
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.")
|
||||
lint_ctx.warn(f"Unknown tag discovered in citations block [{citation.tag}], will be ignored.", line=citation.sourceline, xpath=tool_xml.getpath(citation))
|
||||
continue
|
||||
citation_type = citation.attrib.get("type")
|
||||
if citation_type not in ('bibtex', 'doi'):
|
||||
lint_ctx.warn("Unknown citation type discovered [%s], will be ignored.", citation_type)
|
||||
lint_ctx.warn(f"Unknown citation type discovered [{citation_type}], will be ignored.", line=citation.sourceline, xpath=tool_xml.getpath(citation))
|
||||
continue
|
||||
if citation.text is None or not citation.text.strip():
|
||||
lint_ctx.error(f'Empty {citation_type} citation.')
|
||||
lint_ctx.error(f'Empty {citation_type} citation.', line=citation.sourceline, xpath=tool_xml.getpath(citation))
|
||||
continue
|
||||
valid_citations += 1
|
||||
|
||||
if valid_citations > 0:
|
||||
lint_ctx.valid("Found %d likely valid citations.", valid_citations)
|
||||
lint_ctx.valid(f"Found {valid_citations} likely valid citations.", line=root_line, xpath=root_xpath)
|
||||
else:
|
||||
lint_ctx.warn("Found no valid citations.", line=root_line, xpath=root_xpath)
|
||||
|
||||
@@ -8,18 +8,26 @@ from supplied inputs.
|
||||
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.")
|
||||
lint_ctx.error("More than one command tag found, behavior undefined.", line=commands[1].sourceline, xpath=tool_xml.getpath(commands[1]))
|
||||
return
|
||||
|
||||
if len(commands) == 0:
|
||||
lint_ctx.error("No command tag found, must specify a command template to execute.")
|
||||
lint_ctx.error("No command tag found, must specify a command template to execute.", line=root_line, xpath=root_path)
|
||||
return
|
||||
|
||||
command = get_command(tool_xml)
|
||||
if "TODO" in command:
|
||||
lint_ctx.warn("Command template contains TODO text.")
|
||||
if command.text is None:
|
||||
lint_ctx.error("Command is empty.", line=root_line, xpath=root_path)
|
||||
elif "TODO" in command.text:
|
||||
lint_ctx.warn("Command template contains TODO text.", line=command.sourceline, xpath=tool_xml.getpath(command))
|
||||
|
||||
command_attrib = command.attrib
|
||||
interpreter_type = None
|
||||
@@ -29,14 +37,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}]")
|
||||
lint_ctx.warn(f"Unknown detect_errors attribute [{detect_errors}]", line=command.sourceline, xpath=tool_xml.getpath(command))
|
||||
|
||||
interpreter_info = ""
|
||||
if interpreter_type:
|
||||
interpreter_info = f" with interpreter of type [{interpreter_type}]"
|
||||
if interpreter_type:
|
||||
lint_ctx.info("Command uses deprecated 'interpreter' attribute.")
|
||||
lint_ctx.info(f"Tool contains a command{interpreter_info}.")
|
||||
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))
|
||||
|
||||
|
||||
def get_command(tool_xml):
|
||||
|
||||
@@ -19,6 +19,7 @@ PROFILE_INFO_SPECIFIED_MSG = "Tool specifies profile version [%s]."
|
||||
PROFILE_INVALID_MSG = "Tool specifies an invalid profile version [%s]."
|
||||
|
||||
WARN_WHITESPACE_MSG = "%s contains whitespace, this may cause errors: [%s]."
|
||||
WARN_WHITESPACE_PRESUFFIX = "%s is pre/suffixed by whitespace, this may cause errors: [%s]."
|
||||
WARN_ID_WHITESPACE_MSG = (
|
||||
"Tool ID contains whitespace - this is discouraged: [%s].")
|
||||
|
||||
@@ -27,39 +28,47 @@ lint_tool_types = ["*"]
|
||||
|
||||
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
|
||||
version = tool_source.parse_version() or ''
|
||||
parsed_version = packaging.version.parse(version)
|
||||
if not version:
|
||||
lint_ctx.error(ERROR_VERSION_MSG)
|
||||
lint_ctx.error(ERROR_VERSION_MSG, line=tool_line)
|
||||
elif isinstance(parsed_version, packaging.version.LegacyVersion):
|
||||
lint_ctx.warn(WARN_VERSION_MSG % version)
|
||||
lint_ctx.warn(WARN_VERSION_MSG % version, line=tool_line)
|
||||
elif version != version.strip():
|
||||
lint_ctx.warn(WARN_WHITESPACE_MSG % ('Tool version', version))
|
||||
lint_ctx.warn(WARN_WHITESPACE_PRESUFFIX % ('Tool version', version), line=tool_line)
|
||||
else:
|
||||
lint_ctx.valid(VALID_VERSION_MSG % version)
|
||||
lint_ctx.valid(VALID_VERSION_MSG % version, line=tool_line)
|
||||
|
||||
name = tool_source.parse_name()
|
||||
if not name:
|
||||
lint_ctx.error(ERROR_NAME_MSG)
|
||||
lint_ctx.error(ERROR_NAME_MSG, line=tool_line)
|
||||
elif name != name.strip():
|
||||
lint_ctx.warn(WARN_WHITESPACE_MSG % ('Tool name', name))
|
||||
lint_ctx.warn(WARN_WHITESPACE_PRESUFFIX % ('Tool name', name), line=tool_line)
|
||||
else:
|
||||
lint_ctx.valid(VALID_NAME_MSG % name)
|
||||
lint_ctx.valid(VALID_NAME_MSG % name, line=tool_line)
|
||||
|
||||
tool_id = tool_source.parse_id()
|
||||
if not tool_id:
|
||||
lint_ctx.error(ERROR_ID_MSG)
|
||||
lint_ctx.error(ERROR_ID_MSG, line=tool_line)
|
||||
elif re.search(r"\s", tool_id):
|
||||
lint_ctx.warn(WARN_ID_WHITESPACE_MSG % tool_id, line=tool_line)
|
||||
else:
|
||||
lint_ctx.valid(VALID_ID_MSG % tool_id)
|
||||
lint_ctx.valid(VALID_ID_MSG % tool_id, line=tool_line)
|
||||
|
||||
profile = tool_source.parse_profile()
|
||||
profile_valid = PROFILE_PATTERN.match(profile) is not None
|
||||
if not profile_valid:
|
||||
lint_ctx.error(PROFILE_INVALID_MSG)
|
||||
lint_ctx.error(PROFILE_INVALID_MSG % profile, line=tool_line)
|
||||
elif profile == "16.01":
|
||||
lint_ctx.valid(PROFILE_INFO_DEFAULT_MSG)
|
||||
lint_ctx.valid(PROFILE_INFO_DEFAULT_MSG, line=tool_line)
|
||||
else:
|
||||
lint_ctx.valid(PROFILE_INFO_SPECIFIED_MSG % profile)
|
||||
lint_ctx.valid(PROFILE_INFO_SPECIFIED_MSG % profile, line=tool_line)
|
||||
|
||||
requirements, containers = tool_source.parse_requirements_and_containers()
|
||||
for r in requirements:
|
||||
@@ -72,6 +81,3 @@ def lint_general(tool_source, lint_ctx):
|
||||
elif r.version != r.version.strip():
|
||||
lint_ctx.warn(
|
||||
WARN_WHITESPACE_MSG % ('Requirement version', r.version))
|
||||
|
||||
if re.search(r"\s", tool_id):
|
||||
lint_ctx.warn(WARN_ID_WHITESPACE_MSG % tool_id)
|
||||
|
||||
@@ -7,31 +7,38 @@ from galaxy.util import (
|
||||
|
||||
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
|
||||
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.")
|
||||
lint_ctx.error("More than one help section found, behavior undefined.", line=helps[1].sourceline, xpath=tool_xml.getpath(helps[1]))
|
||||
return
|
||||
|
||||
if len(helps) == 0:
|
||||
lint_ctx.warn("No help section found, consider adding a help section to your tool.")
|
||||
lint_ctx.warn("No help section found, consider adding a help section to your tool.", line=root_line, xpath=root_path)
|
||||
return
|
||||
|
||||
help = helps[0].text or ''
|
||||
if not help.strip():
|
||||
lint_ctx.warn("Help section appears to be empty.")
|
||||
lint_ctx.warn("Help section appears to be empty.", line=helps[0].sourceline, xpath=tool_xml.getpath(helps[0]))
|
||||
return
|
||||
|
||||
lint_ctx.valid("Tool contains help section.")
|
||||
lint_ctx.valid("Tool contains help section.", line=helps[0].sourceline, xpath=tool_xml.getpath(helps[0]))
|
||||
invalid_rst = rst_invalid(help)
|
||||
|
||||
if "TODO" in help:
|
||||
lint_ctx.warn("Help contains TODO text.")
|
||||
lint_ctx.warn("Help contains TODO text.", line=helps[0].sourceline, xpath=tool_xml.getpath(helps[0]))
|
||||
|
||||
if invalid_rst:
|
||||
lint_ctx.warn(f"Invalid reStructuredText found in help - [{invalid_rst}].")
|
||||
lint_ctx.warn(f"Invalid reStructuredText found in help - [{invalid_rst}].", line=helps[0].sourceline, xpath=tool_xml.getpath(helps[0]))
|
||||
else:
|
||||
lint_ctx.valid("Help contains valid reStructuredText.")
|
||||
lint_ctx.valid("Help contains valid reStructuredText.", line=helps[0].sourceline, xpath=tool_xml.getpath(helps[0]))
|
||||
|
||||
|
||||
def rst_invalid(text):
|
||||
|
||||
@@ -46,6 +46,13 @@ 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
|
||||
datasource = is_datasource(tool_xml)
|
||||
inputs = tool_xml.findall("./inputs//param")
|
||||
num_inputs = 0
|
||||
@@ -53,24 +60,33 @@ 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.")
|
||||
lint_ctx.error("Found param input with no name specified.", line=param.sourceline, xpath=tool_xml.getpath(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.")
|
||||
if param_name.strip() == "":
|
||||
lint_ctx.error("Param input with empty name.", line=param.sourceline, xpath=tool_xml.getpath(param))
|
||||
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))
|
||||
|
||||
# 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.")
|
||||
lint_ctx.error(f"Param input [{param_name}] input with no type specified.", line=param.sourceline, xpath=tool_xml.getpath(param))
|
||||
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))
|
||||
continue
|
||||
param_type = param_attrib["type"]
|
||||
|
||||
if not is_valid_cheetah_placeholder(param_name):
|
||||
lint_ctx.warn(f"Param input [{param_name}] is not a valid Cheetah placeholder.")
|
||||
# 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.")
|
||||
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))
|
||||
elif param_type == "select":
|
||||
# get dynamic/statically defined options
|
||||
dynamic_options = param.get("dynamic_options", None)
|
||||
@@ -79,24 +95,29 @@ 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.")
|
||||
lint_ctx.warn(f"Select parameter [{param_name}] uses deprecated 'dynamic_options' attribute.", line=param.sourceline, xpath=tool_xml.getpath(param))
|
||||
|
||||
# check if options are defined by exactly one possibility
|
||||
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.")
|
||||
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))
|
||||
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 dynamic options
|
||||
if len(options) == 1:
|
||||
filters = options[0].findall("./filter")
|
||||
# lint filters
|
||||
# TODO check if dataset is available for filters referring other datasets
|
||||
filter_adds_options = False
|
||||
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.")
|
||||
lint_ctx.error(f"Select parameter [{param_name}] contains filter without type.", line=f.sourceline, xpath=tool_xml.getpath(f))
|
||||
continue
|
||||
if ftype not in FILTER_TYPES:
|
||||
lint_ctx.error(f"Select parameter [{param_name}] contains filter with unknown type '{ftype}'.")
|
||||
lint_ctx.error(f"Select parameter [{param_name}] contains filter with unknown type '{ftype}'.", line=f.sourceline, xpath=tool_xml.getpath(f))
|
||||
continue
|
||||
if ftype in ['add_value', 'data_meta']:
|
||||
filter_adds_options = True
|
||||
@@ -106,36 +127,38 @@ def lint_inputs(tool_xml, lint_ctx):
|
||||
from_parameter = options[0].get("from_parameter", None)
|
||||
from_dataset = options[0].get("from_dataset", None)
|
||||
from_data_table = options[0].get("from_data_table", None)
|
||||
# TODO check if input param is present for from_dataset
|
||||
|
||||
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.")
|
||||
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]))
|
||||
|
||||
if from_file is not None:
|
||||
lint_ctx.warn(f"Select parameter [{param_name}] options uses deprecated 'from_file' attribute.")
|
||||
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.")
|
||||
lint_ctx.warn(f"Select parameter [{param_name}] options uses deprecated 'from_parameter' attribute.", line=options[0].sourceline, xpath=tool_xml.getpath(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.")
|
||||
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]))
|
||||
|
||||
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'.")
|
||||
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.")
|
||||
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.")
|
||||
lint_ctx.warn(f"Select parameter [{param_name}] options uses deprecated 'transform_lines' attribute.", line=options[0].sourceline, xpath=tool_xml.getpath(options[0]))
|
||||
|
||||
elif len(options) > 1:
|
||||
lint_ctx.error(f"Select parameter [{param_name}] contains multiple options elements")
|
||||
lint_ctx.error(f"Select parameter [{param_name}] contains multiple options elements.", line=options[1].sourceline, xpath=tool_xml.getpath(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")
|
||||
lint_ctx.error(f"Select parameter [{param_name}] has option without value", line=param.sourceline, xpath=tool_xml.getpath(param))
|
||||
if any(option.text is None for option in select_options):
|
||||
lint_ctx.warn(f"Select parameter [{param_name}] has option without text")
|
||||
lint_ctx.warn(f"Select parameter [{param_name}] has option without text", line=param.sourceline, xpath=tool_xml.getpath(param))
|
||||
|
||||
select_options_texts = list()
|
||||
select_options_values = list()
|
||||
@@ -148,61 +171,65 @@ 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")
|
||||
lint_ctx.error(f"Select parameter [{param_name}] has multiple options with the same text content", line=param.sourceline, xpath=tool_xml.getpath(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")
|
||||
lint_ctx.error(f"Select parameter [{param_name}] has multiple options with the same value", line=param.sourceline, xpath=tool_xml.getpath(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')
|
||||
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))
|
||||
if not optional:
|
||||
lint_ctx.error(f'Select [{param_name}] `display="checkboxes"` is incompatible with `optional="false"`, remove the `display` attribute')
|
||||
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))
|
||||
if param_attrib.get("display") == "radio":
|
||||
if multiple:
|
||||
lint_ctx.error(f'Select [{param_name}] display="radio" is incompatible with multiple="true"')
|
||||
lint_ctx.error(f'Select [{param_name}] display="radio" is incompatible with multiple="true"', line=param.sourceline, xpath=tool_xml.getpath(param))
|
||||
if optional:
|
||||
lint_ctx.error(f'Select [{param_name}] display="radio" is incompatible with optional="true"')
|
||||
lint_ctx.error(f'Select [{param_name}] display="radio" is incompatible with optional="true"', line=param.sourceline, xpath=tool_xml.getpath(param))
|
||||
# TODO: Validate type, much more...
|
||||
|
||||
# lint validators
|
||||
# TODO check if dataset is available for validators referring other datasets
|
||||
validators = param.findall("./validator")
|
||||
for validator in validators:
|
||||
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}'")
|
||||
lint_ctx.error(f"Parameter [{param_name}]: validator with an incompatible type '{vtype}'", line=validator.sourceline, xpath=tool_xml.getpath(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}'")
|
||||
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")
|
||||
lint_ctx.error(f"Parameter [{param_name}]: expression validator without content", line=validator.sourceline, xpath=tool_xml.getpath(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}')")
|
||||
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))
|
||||
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)")
|
||||
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))
|
||||
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) {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))
|
||||
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")
|
||||
lint_ctx.error(f"Parameter [{param_name}]: '{vtype}' validators need to define the 'table_name' attribute", line=validator.sourceline, xpath=tool_xml.getpath(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")
|
||||
lint_ctx.error("Conditional without a name", line=conditional.sourceline, xpath=tool_xml.getpath(conditional))
|
||||
if conditional.get("value_from"):
|
||||
# Probably only the upload tool use this, no children elements
|
||||
continue
|
||||
first_param = conditional.find("param")
|
||||
if first_param is None:
|
||||
lint_ctx.error(f"Conditional [{conditional_name}] has no child <param>")
|
||||
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)}", line=conditional.sourceline, xpath=tool_xml.getpath(conditional))
|
||||
continue
|
||||
first_param = first_param[0]
|
||||
first_param_type = first_param.get('type')
|
||||
if first_param_type not in ['select', 'boolean']:
|
||||
lint_ctx.warn(f'Conditional [{conditional_name}] first param should have type="select" /> or type="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))
|
||||
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')
|
||||
@@ -213,38 +240,40 @@ def lint_inputs(tool_xml, lint_ctx):
|
||||
first_param.get('falsevalue', 'false')
|
||||
]
|
||||
|
||||
if string_as_bool(first_param.get('optional', False)):
|
||||
lint_ctx.warn(f"Conditional [{conditional_name}] test parameter cannot be optional")
|
||||
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))
|
||||
|
||||
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")
|
||||
lint_ctx.error(f"Conditional [{conditional_name}] when without value", line=conditional.sourceline, xpath=tool_xml.getpath(conditional))
|
||||
|
||||
when_ids = [w.get('value') for w in whens]
|
||||
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}'")
|
||||
lint_ctx.warn(f"Conditional [{conditional_name}] no <when /> block found for {first_param_type} option '{option_id}'", line=conditional.sourceline, xpath=tool_xml.getpath(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}'")
|
||||
lint_ctx.warn(f"Conditional [{conditional_name}] no <option /> found for when block '{when_id}'", line=conditional.sourceline, xpath=tool_xml.getpath(conditional))
|
||||
else:
|
||||
lint_ctx.warn(f"Conditional [{conditional_name}] no truevalue/falsevalue found for when block '{when_id}'")
|
||||
lint_ctx.warn(f"Conditional [{conditional_name}] no truevalue/falsevalue found for when block '{when_id}'", line=conditional.sourceline, xpath=tool_xml.getpath(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")
|
||||
lint_ctx.info(f"{datasource_tag} tag usually present in data sources", line=tool_line, xpath=tool_path)
|
||||
|
||||
if num_inputs:
|
||||
lint_ctx.info(f"Found {num_inputs} input parameters.")
|
||||
lint_ctx.info(f"Found {num_inputs} input parameters.", line=tool_line, xpath=tool_path)
|
||||
else:
|
||||
if datasource:
|
||||
lint_ctx.info("No input parameters, OK for data sources")
|
||||
lint_ctx.info("No input parameters, OK for data sources", line=tool_line, xpath=tool_path)
|
||||
else:
|
||||
lint_ctx.warn("Found no input parameters.")
|
||||
lint_ctx.warn("Found no input parameters.", line=tool_line, xpath=tool_path)
|
||||
|
||||
|
||||
def lint_repeats(tool_xml, lint_ctx):
|
||||
@@ -252,9 +281,9 @@ def lint_repeats(tool_xml, lint_ctx):
|
||||
repeats = tool_xml.findall("./inputs//repeat")
|
||||
for repeat in repeats:
|
||||
if "name" not in repeat.attrib:
|
||||
lint_ctx.error("Repeat does not specify name attribute.")
|
||||
lint_ctx.error("Repeat does not specify name attribute.", line=repeat.sourceline, xpath=tool_xml.getpath(repeat))
|
||||
if "title" not in repeat.attrib:
|
||||
lint_ctx.error("Repeat does not specify title attribute.")
|
||||
lint_ctx.error("Repeat does not specify title attribute.", line=repeat.sourceline, xpath=tool_xml.getpath(repeat))
|
||||
|
||||
|
||||
def _find_with_attribute(element, tag, attribute, test_value=None):
|
||||
|
||||
@@ -6,30 +6,33 @@ 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 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 = 0
|
||||
tool_path = None
|
||||
outputs = tool_xml.findall("./outputs")
|
||||
if len(outputs) == 0:
|
||||
lint_ctx.warn("Tool contains no outputs section, most tools should produce outputs.")
|
||||
if len(outputs) > 1:
|
||||
lint_ctx.warn("Tool contains multiple output sections, behavior undefined.")
|
||||
|
||||
num_outputs = 0
|
||||
if len(outputs) == 0:
|
||||
lint_ctx.warn("No outputs found")
|
||||
lint_ctx.warn("Tool contains no outputs section, most tools should produce outputs.", line=tool_line, xpath=tool_path)
|
||||
return
|
||||
|
||||
if len(outputs) > 1:
|
||||
lint_ctx.warn("Tool contains multiple output sections, behavior undefined.", line=outputs[1].sourceline, xpath=tool_xml.getpath(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}]")
|
||||
lint_ctx.warn(f"Unknown element found in outputs [{output.tag}]", line=output.sourceline, xpath=tool_xml.getpath(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.")
|
||||
else:
|
||||
if not is_valid_cheetah_placeholder(output.attrib["name"]):
|
||||
lint_ctx.warn("Tool output name [%s] is not a valid Cheetah placeholder.", output.attrib["name"])
|
||||
lint_ctx.warn("Tool output doesn't define a name - this is likely a problem.", line=output.sourceline, xpath=tool_xml.getpath(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.', line=output.sourceline, xpath=tool_xml.getpath(output))
|
||||
|
||||
format_set = False
|
||||
if __check_format(output, lint_ctx):
|
||||
if __check_format(tool_xml, output, lint_ctx):
|
||||
format_set = True
|
||||
if output.tag == "data":
|
||||
if "auto_format" in output.attrib and output.attrib["auto_format"]:
|
||||
@@ -37,29 +40,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.")
|
||||
lint_ctx.warn("Collection output with undefined 'type' found.", line=output.sourceline, xpath=tool_xml.getpath(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, allow_ext=True):
|
||||
elif __check_format(tool_xml, 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.")
|
||||
lint_ctx.warn(f"Tool {output.tag} output {output.attrib.get('name', 'with missing name')} doesn't define an output format.", line=output.sourceline, xpath=tool_xml.getpath(output))
|
||||
|
||||
lint_ctx.info("%d outputs found.", num_outputs)
|
||||
# TODO: check for different labels in case of multiple outputs
|
||||
lint_ctx.info(f"{num_outputs} outputs found.", line=outputs[0].sourceline)
|
||||
|
||||
|
||||
def __check_format(node, lint_ctx, allow_ext=False):
|
||||
def __check_format(tool_xml, 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")
|
||||
lint_ctx.warn(f"Tool {node.tag} output '{node.attrib.get('name', 'with missing name')}' should use either format_source or format/ext", line=node.sourceline, xpath=tool_xml.getpath(node))
|
||||
if "format_source" in node.attrib:
|
||||
return True
|
||||
# if allowed (e.g. for discover_datasets), ext takes precedence over format
|
||||
@@ -69,7 +73,7 @@ def __check_format(node, lint_ctx, 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.")
|
||||
lint_ctx.warn(f"Using format='input' on {node.tag}, format_source attribute is less ambiguous and should be used instead.", line=node.sourceline, xpath=tool_xml.getpath(node))
|
||||
return fmt is not None
|
||||
|
||||
|
||||
@@ -87,5 +91,6 @@ def __check_pattern(node):
|
||||
return False
|
||||
pattern = node.attrib["pattern"]
|
||||
regex_pattern = NAMED_PATTERNS.get(pattern, pattern)
|
||||
# TODO error on wrong pattern or non-regexp
|
||||
if "(?P<ext>" in regex_pattern:
|
||||
return True
|
||||
|
||||
@@ -4,40 +4,51 @@ from .command import get_command
|
||||
|
||||
def lint_stdio(tool_source, lint_ctx):
|
||||
tool_xml = getattr(tool_source, "xml_tree", None)
|
||||
# determine line to report for general problems with stdio
|
||||
try:
|
||||
tool_line = tool_xml.find("./tool").sourceline
|
||||
tool_path = tool_xml.getpath(tool_xml.find("./tool"))
|
||||
except AttributeError:
|
||||
tool_line = 0
|
||||
tool_path = None
|
||||
stdios = tool_xml.findall("./stdio") if tool_xml else []
|
||||
|
||||
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.")
|
||||
lint_ctx.info("No stdio definition found, tool indicates error conditions with output written to stderr.", line=tool_line, xpath=tool_path)
|
||||
else:
|
||||
lint_ctx.info("No stdio definition found, tool indicates error conditions with non-zero exit codes.")
|
||||
lint_ctx.info("No stdio definition found, tool indicates error conditions with non-zero exit codes.", line=tool_line, xpath=tool_path)
|
||||
return
|
||||
|
||||
if len(stdios) > 1:
|
||||
lint_ctx.error("More than one stdio tag found, behavior undefined.")
|
||||
lint_ctx.error("More than one stdio tag found, behavior undefined.", line=stdios[1].sourceline, xpath=tool_xml.getpath(stdios[1]))
|
||||
return
|
||||
|
||||
stdio = stdios[0]
|
||||
for child in list(stdio):
|
||||
if child.tag == "regex":
|
||||
_lint_regex(child, lint_ctx)
|
||||
_lint_regex(tool_xml, child, lint_ctx)
|
||||
elif child.tag == "exit_code":
|
||||
_lint_exit_code(child, lint_ctx)
|
||||
_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)
|
||||
lint_ctx.warn(message % child.tag, line=child.sourceline, xpath=tool_xml.getpath(child))
|
||||
|
||||
|
||||
def _lint_exit_code(child, lint_ctx):
|
||||
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.")
|
||||
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(child, lint_ctx):
|
||||
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.")
|
||||
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
|
||||
|
||||
@@ -4,13 +4,26 @@ from ._util import is_datasource
|
||||
|
||||
# Misspelled so as not be picked up by nosetests.
|
||||
def lint_tsts(tool_xml, lint_ctx):
|
||||
# determine line to report for general problems with tests
|
||||
try:
|
||||
tests_line = tool_xml.find("./tests").sourceline
|
||||
tests_path = tool_xml.getpath(tool_xml.find("./tests"))
|
||||
except AttributeError:
|
||||
tests_line = 1
|
||||
tests_path = None
|
||||
try:
|
||||
tests_line = tool_xml.find("./tool").sourceline
|
||||
tests_path = tool_xml.getpath(tool_xml.find("./tool"))
|
||||
except AttributeError:
|
||||
pass
|
||||
tests = tool_xml.findall("./tests/test")
|
||||
datasource = is_datasource(tool_xml)
|
||||
|
||||
if not tests and not datasource:
|
||||
lint_ctx.warn("No tests found, most tools should define test cases.")
|
||||
elif datasource:
|
||||
lint_ctx.info("No tests found, that should be OK for data_sources.")
|
||||
if not tests:
|
||||
if not datasource:
|
||||
lint_ctx.warn("No tests found, most tools should define test cases.", line=tests_line, xpath=tests_path)
|
||||
elif datasource:
|
||||
lint_ctx.info("No tests found, that should be OK for data_sources.", line=tests_line, xpath=tests_path)
|
||||
return
|
||||
|
||||
num_valid_tests = 0
|
||||
for test_idx, test in enumerate(tests, start=1):
|
||||
@@ -30,7 +43,7 @@ def lint_tsts(tool_xml, lint_ctx):
|
||||
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.")
|
||||
lint_ctx.error(f"Test {test_idx}: Found test param tag without a name defined.", line=param.sourceline, xpath=tool_xml.getpath(param))
|
||||
continue
|
||||
name = name.split("|")[-1]
|
||||
xpaths = [f"@name='{name}'",
|
||||
@@ -48,41 +61,37 @@ 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")
|
||||
lint_ctx.error(f"Test {test_idx}: Test param {name} not found in the inputs", line=param.sourceline, xpath=tool_xml.getpath(param))
|
||||
|
||||
output_data_names, output_collection_names = _collect_output_names(tool_xml)
|
||||
found_output_test = False
|
||||
for output in test.findall("output"):
|
||||
for output in test.findall("output") + test.findall("output_collection"):
|
||||
found_output_test = True
|
||||
name = output.attrib.get("name", None)
|
||||
if not name:
|
||||
lint_ctx.warn(f"Test {test_idx}: Found output tag without a name defined.")
|
||||
if output.tag == "output":
|
||||
valid_names = output_data_names
|
||||
else:
|
||||
if name not in output_data_names:
|
||||
lint_ctx.error(f"Test {test_idx}: Found output tag with unknown name [{name}], valid names [{output_data_names}]")
|
||||
valid_names = output_collection_names
|
||||
if not name:
|
||||
lint_ctx.error(f"Test {test_idx}: Found {output.tag} tag without a name defined.", line=output.sourceline, xpath=tool_xml.getpath(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}]", line=output.sourceline, xpath=tool_xml.getpath(output))
|
||||
|
||||
for output_collection in test.findall("output_collection"):
|
||||
found_output_test = True
|
||||
name = output_collection.attrib.get("name", None)
|
||||
if not name:
|
||||
lint_ctx.warn(f"Test {test_idx}: Found output_collection tag without a name defined.")
|
||||
else:
|
||||
if name not in output_collection_names:
|
||||
lint_ctx.warn(f"Test {test_idx}: Found output_collection tag with unknown name [{name}], valid names [{output_collection_names}]")
|
||||
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.")
|
||||
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.")
|
||||
lint_ctx.warn(f"Test {test_idx}: No outputs or expectations defined for tests, this test is likely invalid.", line=test.sourceline, xpath=tool_xml.getpath(test))
|
||||
else:
|
||||
num_valid_tests += 1
|
||||
|
||||
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.")
|
||||
|
||||
if num_valid_tests or datasource:
|
||||
lint_ctx.valid("%d test(s) found.", num_valid_tests)
|
||||
lint_ctx.valid(f"{num_valid_tests} test(s) found.", line=tests_line, xpath=tests_path)
|
||||
else:
|
||||
lint_ctx.warn("No valid test(s) found.")
|
||||
lint_ctx.warn("No valid test(s) found.", line=tests_line, xpath=tests_path)
|
||||
|
||||
|
||||
def _collect_output_names(tool_xml):
|
||||
|
||||
@@ -47,22 +47,20 @@ def lint_xml_order(tool_xml, lint_ctx):
|
||||
tool_root = tool_xml.getroot()
|
||||
|
||||
if tool_root.attrib.get('tool_type', '') == 'data_source':
|
||||
_validate_for_tags(tool_root, lint_ctx, DATASOURCE_TAG_ORDER)
|
||||
tag_ordering = DATASOURCE_TAG_ORDER
|
||||
else:
|
||||
_validate_for_tags(tool_root, lint_ctx, TAG_ORDER)
|
||||
tag_ordering = TAG_ORDER
|
||||
|
||||
|
||||
def _validate_for_tags(root, lint_ctx, tag_ordering):
|
||||
last_tag = None
|
||||
last_key = None
|
||||
for elem in root:
|
||||
for elem in tool_root:
|
||||
tag = elem.tag
|
||||
if tag in tag_ordering:
|
||||
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}]")
|
||||
lint_ctx.warn(f"Best practice violation [{tag}] elements should come before [{last_tag}]", line=elem.sourceline, xpath=tool_xml.getpath(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.")
|
||||
lint_ctx.info(f"Unknown tag [{tag}] encountered, this may result in a warning in the future.", line=elem.sourceline, xpath=tool_xml.getpath(elem))
|
||||
|
||||
File diff suppressed because it is too large
Load Diff
Reference in New Issue
Block a user