diff --git a/.ci/validate_test_tools.sh b/.ci/validate_test_tools.sh index 258799dc95d4..45a32d3f6898 100755 --- a/.ci/validate_test_tools.sh +++ b/.ci/validate_test_tools.sh @@ -7,5 +7,7 @@ xsd_path="lib/galaxy/tools/xsd/galaxy.xsd" xmllint --noout "$xsd_path" test_tools_path='test/functional/tools' -tool_files_list=$(ls "$test_tools_path"/*.xml | grep -v '_conf.xml$') +# test all test tools except upload.xml which uses a non-standard conditional +# (without param) which does not survive xsd validation +tool_files_list=$(ls "$test_tools_path"/*.xml | grep -v '_conf.xml$' | grep -v upload.xml) sh scripts/validate_tools.sh $tool_files_list diff --git a/lib/galaxy/tool_util/lint.py b/lib/galaxy/tool_util/lint.py index 54390031a232..bfde64c9d1bd 100644 --- a/lib/galaxy/tool_util/lint.py +++ b/lib/galaxy/tool_util/lint.py @@ -45,22 +45,31 @@ """ import inspect +from abc import ( + ABC, + abstractmethod, +) from enum import IntEnum from typing import ( Callable, List, Optional, Type, + TYPE_CHECKING, TypeVar, Union, ) +import galaxy.tool_util.linters from galaxy.tool_util.parser import get_tool_source from galaxy.util import ( Element, submodules, ) +if TYPE_CHECKING: + from galaxy.tool_util.parser.interface import ToolSource + class LintLevel(IntEnum): SILENT = 5 @@ -71,14 +80,44 @@ class LintLevel(IntEnum): ALL = 0 +class Linter(ABC): + """ + a linter. needs to define a lint method and the code property. + optionally a fix method can be given + """ + + @classmethod + @abstractmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + """ + should add at most one message to the lint context + """ + pass + + @classmethod + def name(cls) -> str: + """ + get the linter name + """ + return cls.__name__ + + @classmethod + def list_listers(cls) -> List[str]: + """ + list the names of all linter derived from Linter + """ + return [s.__name__ for s in cls.__subclasses__()] + + class LintMessage: """ a message from the linter """ - def __init__(self, level: str, message: str, **kwargs): + def __init__(self, level: str, message: str, linter: Optional[str] = None, **kwargs): self.level = level self.message = message + self.linter = linter def __eq__(self, other) -> bool: """ @@ -95,15 +134,19 @@ def __eq__(self, other) -> bool: return False def __str__(self) -> str: - return f".. {self.level.upper()}: {self.message}" + if self.linter: + linter = f" ({self.linter})" + else: + linter = "" + return f".. {self.level.upper()}{linter}: {self.message}" def __repr__(self) -> str: return f"LintMessage({self.message})" class XMLLintMessageLine(LintMessage): - def __init__(self, level: str, message: str, node: Optional[Element] = None): - super().__init__(level, message) + def __init__(self, level: str, message: str, linter: Optional[str] = None, node: Optional[Element] = None): + super().__init__(level, message, linter) self.line = None if node is not None: self.line = node.sourceline @@ -118,8 +161,8 @@ def __str__(self) -> str: class XMLLintMessageXPath(LintMessage): - def __init__(self, level: str, message: str, node: Optional[Element] = None): - super().__init__(level, message) + def __init__(self, level: str, message: str, linter: Optional[str] = None, node: Optional[Element] = None): + super().__init__(level, message, linter) self.xpath = None if node is not None: tool_xml = node.getroottree() @@ -170,7 +213,8 @@ def found_warns(self) -> bool: return len(self.warn_messages) > 0 def lint(self, name: str, lint_func: Callable[[LintTargetType, "LintContext"], None], lint_target: LintTargetType): - name = name[len("lint_") :] + if name.startswith("lint_"): + name = name[len("lint_") :] if name in self.skip_types: return @@ -187,37 +231,18 @@ def lint(self, name: str, lint_func: Callable[[LintTargetType, "LintContext"], N lint_func(lint_target, self) if self.level < LintLevel.SILENT: - # TODO: colorful emoji if in click CLI. - if self.error_messages: - status = "FAIL" - elif self.warn_messages: - status = "WARNING" - else: - status = "CHECK" - - def print_linter_info(printed_linter_info): - if printed_linter_info: - return True - print(f"Applying linter {name}... {status}") - return True - - plf = False for message in self.error_messages: - plf = print_linter_info(plf) print(f"{message}") if self.level <= LintLevel.WARN: for message in self.warn_messages: - plf = print_linter_info(plf) print(f"{message}") if self.level <= LintLevel.INFO: for message in self.info_messages: - plf = print_linter_info(plf) print(f"{message}") if self.level <= LintLevel.VALID: for message in self.valid_messages: - plf = print_linter_info(plf) print(f"{message}") self.message_list = tmp_message_list + self.message_list @@ -237,22 +262,22 @@ def warn_messages(self) -> List[LintMessage]: def error_messages(self) -> List[LintMessage]: return [x for x in self.message_list if x.level == "error"] - def __handle_message(self, level: str, message: str, *args, **kwargs) -> None: + def __handle_message(self, level: str, message: str, linter: Optional[str] = None, *args, **kwargs) -> None: if args: message = message % args - self.message_list.append(self.lint_message_class(level=level, message=message, **kwargs)) + self.message_list.append(self.lint_message_class(level=level, message=message, linter=linter, **kwargs)) - def valid(self, message: str, *args, **kwargs) -> None: - self.__handle_message("check", message, *args, **kwargs) + def valid(self, message: str, linter: Optional[str] = None, *args, **kwargs) -> None: + self.__handle_message("check", message, linter, *args, **kwargs) - def info(self, message: str, *args, **kwargs) -> None: - self.__handle_message("info", message, *args, **kwargs) + def info(self, message: str, linter: Optional[str] = None, *args, **kwargs) -> None: + self.__handle_message("info", message, linter, *args, **kwargs) - def error(self, message: str, *args, **kwargs) -> None: - self.__handle_message("error", message, *args, **kwargs) + def error(self, message: str, linter: Optional[str] = None, *args, **kwargs) -> None: + self.__handle_message("error", message, linter, *args, **kwargs) - def warn(self, message: str, *args, **kwargs) -> None: - self.__handle_message("warning", message, *args, **kwargs) + def warn(self, message: str, linter: Optional[str] = None, *args, **kwargs) -> None: + self.__handle_message("warning", message, linter, *args, **kwargs) def failed(self, fail_level: Union[LintLevel, str]) -> bool: if isinstance(fail_level, str): @@ -319,12 +344,16 @@ def lint_xml( def lint_tool_source_with(lint_context, tool_source, extra_modules=None) -> LintContext: extra_modules = extra_modules or [] - import galaxy.tool_util.linters - tool_xml = getattr(tool_source, "xml_tree", None) - tool_type = tool_source.parse_tool_type() or "default" linter_modules = submodules.import_submodules(galaxy.tool_util.linters) linter_modules.extend(extra_modules) + return lint_tool_source_with_modules(lint_context, tool_source, linter_modules) + + +def lint_tool_source_with_modules(lint_context: LintContext, tool_source, linter_modules) -> LintContext: + tool_xml = getattr(tool_source, "xml_tree", None) + tool_type = tool_source.parse_tool_type() or "default" + for module in linter_modules: lint_tool_types = getattr(module, "lint_tool_types", ["default", "manage_data"]) if not ("*" in lint_tool_types or tool_type in lint_tool_types): @@ -344,6 +373,8 @@ def lint_tool_source_with(lint_context, tool_source, extra_modules=None) -> Lint lint_context.lint(name, value, tool_xml) else: lint_context.lint(name, value, tool_source) + elif inspect.isclass(value) and issubclass(value, Linter) and not inspect.isabstract(value): + lint_context.lint(name, value.lint, tool_source) return lint_context diff --git a/lib/galaxy/tool_util/linters/citations.py b/lib/galaxy/tool_util/linters/citations.py index 562691c15213..79e5b9cf5c80 100644 --- a/lib/galaxy/tool_util/linters/citations.py +++ b/lib/galaxy/tool_util/linters/citations.py @@ -1,42 +1,73 @@ -"""This module contains a citation lint function. +"""This module contains citation linters. Citations describe references that should be used when consumers of the tool publish results. """ +from typing import TYPE_CHECKING -def lint_citations(tool_xml, lint_ctx): - """Ensure tool contains at least one valid citation.""" - 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=citations[1]) - return - - if len(citations) == 0: - 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=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=citation) - continue - if citation.text is None or not citation.text.strip(): - 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=root) - else: - lint_ctx.warn("Found no valid citations.", node=root) +from galaxy.tool_util.lint import Linter + +if TYPE_CHECKING: + from galaxy.tool_util.lint import LintContext + from galaxy.tool_util.parser.interface import ToolSource + + +class CitationsMissing(Linter): + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + tool_xml = getattr(tool_source, "xml_tree", None) + if not tool_xml: + return + root = tool_xml.find("./citations") + if root is None: + root = tool_xml.getroot() + citations = tool_xml.findall("citations") + if len(citations) == 0: + lint_ctx.warn("No citations found, consider adding citations to your tool.", linter=cls.name(), node=root) + + +class CitationsNoText(Linter): + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + tool_xml = getattr(tool_source, "xml_tree", None) + if not tool_xml: + return + citations = tool_xml.find("citations") + if citations is None: + return + for citation in citations: + citation_type = citation.attrib.get("type") + if citation_type in ["doi", "bibtex"] and (citation.text is None or not citation.text.strip()): + lint_ctx.error(f"Empty {citation_type} citation.", linter=cls.name(), node=citation) + + +class CitationsFound(Linter): + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + tool_xml = getattr(tool_source, "xml_tree", None) + if not tool_xml: + return + root = tool_xml.find("./citations") + if root is None: + root = tool_xml.getroot() + citations = tool_xml.find("citations") + + if citations is not None and len(citations) > 0: + lint_ctx.valid(f"Found {len(citations)} citations.", linter=cls.name(), node=root) + + +class CitationsNoValid(Linter): + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + tool_xml = getattr(tool_source, "xml_tree", None) + if not tool_xml: + return + root = tool_xml.find("./citations") + if root is None: + root = tool_xml.getroot() + citations = tool_xml.findall("citations") + if len(citations) != 1: + return + if len(citations[0]) == 0: + lint_ctx.warn("Found no valid citations.", linter=cls.name(), node=root) diff --git a/lib/galaxy/tool_util/linters/command.py b/lib/galaxy/tool_util/linters/command.py index 9d2dadd1b844..eff900341cb1 100644 --- a/lib/galaxy/tool_util/linters/command.py +++ b/lib/galaxy/tool_util/linters/command.py @@ -1,54 +1,84 @@ -"""This module contains a linting function for a tool's command description. +"""This module contains linters for a tool's command description. A command description describes how to build the command-line to execute from supplied inputs. """ +from typing import TYPE_CHECKING -def lint_command(tool_xml, lint_ctx): - """Ensure tool contains exactly one command and check attributes.""" - 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=commands[1]) - return - - if len(commands) == 0: - 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=root) - elif "TODO" in command.text: - lint_ctx.warn("Command template contains TODO text.", node=command) - - command_attrib = command.attrib - interpreter_type = None - for key, value in command_attrib.items(): - if key == "interpreter": - interpreter_type = value - 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=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=command) - lint_ctx.info(f"Tool contains a command{interpreter_info}.", node=command) - - -def get_command(tool_xml): - """Get command XML element from supplied XML root.""" - root = tool_xml.getroot() - commands = root.findall("command") - command = None - if len(commands) == 1: - command = commands[0] - return command +from galaxy.tool_util.lint import Linter + +if TYPE_CHECKING: + from galaxy.tool_util.lint import LintContext + from galaxy.tool_util.parser.interface import ToolSource + + +class CommandMissing(Linter): + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + tool_xml = getattr(tool_source, "xml_tree", None) + if not tool_xml: + return + root = tool_xml.find("./command") + if root is None: + root = tool_xml.getroot() + command = tool_xml.find("./command") + if command is None: + lint_ctx.error( + "No command tag found, must specify a command template to execute.", linter=cls.name(), node=root + ) + + +class CommandEmpty(Linter): + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + tool_xml = getattr(tool_source, "xml_tree", None) + if not tool_xml: + return + root = tool_xml.find("./command") + if root is None: + root = tool_xml.getroot() + command = tool_xml.find("./command") + if command is not None and command.text is None: + lint_ctx.error("Command is empty.", linter=cls.name(), node=root) + + +class CommandTODO(Linter): + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + tool_xml = getattr(tool_source, "xml_tree", None) + if not tool_xml: + return + command = tool_xml.find("./command") + if command is not None and command.text is not None and "TODO" in command.text: + lint_ctx.warn("Command template contains TODO text.", linter=cls.name(), node=command) + + +class CommandInterpreterDeprecated(Linter): + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + tool_xml = getattr(tool_source, "xml_tree", None) + if not tool_xml: + return + command = tool_xml.find("./command") + if command is None: + return + interpreter_type = command.attrib.get("interpreter", None) + if interpreter_type is not None: + lint_ctx.warn("Command uses deprecated 'interpreter' attribute.", linter=cls.name(), node=command) + + +class CommandInfo(Linter): + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + tool_xml = getattr(tool_source, "xml_tree", None) + if not tool_xml: + return + command = tool_xml.find("./command") + if command is None: + return + interpreter_type = command.attrib.get("interpreter", None) + interpreter_info = "" + if interpreter_type: + interpreter_info = f" with interpreter of type [{interpreter_type}]" + lint_ctx.info(f"Tool contains a command{interpreter_info}.", linter=cls.name(), node=command) diff --git a/lib/galaxy/tool_util/linters/cwl.py b/lib/galaxy/tool_util/linters/cwl.py index edfc992f8887..f4e7f367ba56 100644 --- a/lib/galaxy/tool_util/linters/cwl.py +++ b/lib/galaxy/tool_util/linters/cwl.py @@ -1,48 +1,93 @@ """Linter for CWL tools.""" +from typing import TYPE_CHECKING + lint_tool_types = ["cwl"] from galaxy.tool_util.cwl.schema import schema_loader +from galaxy.tool_util.lint import Linter + +if TYPE_CHECKING: + from galaxy.tool_util.lint import LintContext + from galaxy.tool_util.parser import ToolSource -def lint_cwl_validation(tool_source, lint_ctx): - """Determine in CWL tool validates against spec.""" - raw_reference = schema_loader.raw_process_reference(tool_source._source_path) - validation_exception = None - try: - schema_loader.process_definition(raw_reference) - except Exception as e: - validation_exception = e - if validation_exception: - lint_ctx.error(f"Failed to valdiate CWL artifact: {validation_exception}") - else: +class CWLValid(Linter): + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + raw_reference = schema_loader.raw_process_reference(tool_source.source_path) + try: + schema_loader.process_definition(raw_reference) + except Exception: + return lint_ctx.info("CWL appears to be valid.") -def lint_new_draft(tool_source, lint_ctx): - """Determine in CWL tool is valid, modern draft.""" - raw_reference = schema_loader.raw_process_reference(tool_source._source_path) - cwl_version = raw_reference.process_object.get("cwlVersion", None) - if cwl_version is None: - lint_ctx.error("CWL file does not contain a 'cwlVersion'") - if cwl_version not in ["v1.0"]: - lint_ctx.warn(f"CWL version [{cwl_version}] is unknown, we recommend the v1.0 the stable release.") - else: - lint_ctx.info(f"Modern CWL version [{cwl_version}].") - - -def lint_docker_image(tool_source, lint_ctx): - _, containers, *_ = tool_source.parse_requirements_and_containers() - if len(containers) == 0: - lint_ctx.warn("Tool does not specify a DockerPull source.") - else: - identifier = containers[0].identifier - lint_ctx.info(f"Tool will run in Docker image [{identifier}].") - - -def lint_description(tool_source, lint_ctx): - help = tool_source.parse_help() - if not help: - lint_ctx.warn("Description of tool is empty or absent.") - elif "TODO" in help: - lint_ctx.warn("Help contains TODO text.") +class CWLInValid(Linter): + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + raw_reference = schema_loader.raw_process_reference(tool_source.source_path) + try: + schema_loader.process_definition(raw_reference) + except Exception as e: + lint_ctx.error(f"Failed to valdiate CWL artifact: {e}") + + +class CWLVersionMissing(Linter): + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + raw_reference = schema_loader.raw_process_reference(tool_source.source_path) + cwl_version = raw_reference.process_object.get("cwlVersion", None) + if cwl_version is None: + lint_ctx.error("CWL file does not contain a 'cwlVersion'") + + +class CWLVersionUnknown(Linter): + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + raw_reference = schema_loader.raw_process_reference(tool_source.source_path) + cwl_version = raw_reference.process_object.get("cwlVersion", None) + if cwl_version not in ["v1.0"]: + lint_ctx.warn(f"CWL version [{cwl_version}] is unknown, we recommend the v1.0 the stable release.") + + +class CWLVersionGood(Linter): + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + raw_reference = schema_loader.raw_process_reference(tool_source.source_path) + cwl_version = raw_reference.process_object.get("cwlVersion", None) + if cwl_version in ["v1.0"]: + lint_ctx.info(f"Modern CWL version [{cwl_version}].") + + +class CWLDockerMissing(Linter): + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + _, containers, *_ = tool_source.parse_requirements_and_containers() + if len(containers) == 0: + lint_ctx.warn("Tool does not specify a DockerPull source.") + + +class CWLDockerGood(Linter): + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + _, containers, *_ = tool_source.parse_requirements_and_containers() + if len(containers) > 0: + identifier = containers[0].identifier + lint_ctx.info(f"Tool will run in Docker image [{identifier}].") + + +class CWLDescriptionMissing(Linter): + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + help = tool_source.parse_help() + if not help: + lint_ctx.warn("Description of tool is empty or absent.") + + +class CWLHelpTODO(Linter): + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + help = tool_source.parse_help() + if help and "TODO" in help: + lint_ctx.warn("Help contains TODO text.") diff --git a/lib/galaxy/tool_util/linters/general.py b/lib/galaxy/tool_util/linters/general.py index 8ca73c76ff76..72e2ad7b4861 100644 --- a/lib/galaxy/tool_util/linters/general.py +++ b/lib/galaxy/tool_util/linters/general.py @@ -1,88 +1,227 @@ """This module contains linting functions for general aspects of the tool.""" import re +from typing import ( + Tuple, + TYPE_CHECKING, +) +from galaxy.tool_util.lint import Linter from galaxy.tool_util.version import ( LegacyVersion, parse_version, ) -ERROR_VERSION_MSG = "Tool version is missing or empty." -WARN_VERSION_MSG = "Tool version [%s] is not compliant with PEP 440." -VALID_VERSION_MSG = "Tool defines a version [%s]." - -ERROR_NAME_MSG = "Tool name is missing or empty." -VALID_NAME_MSG = "Tool defines a name [%s]." - -ERROR_ID_MSG = "Tool does not define an id attribute." -VALID_ID_MSG = "Tool defines an id [%s]." +if TYPE_CHECKING: + from galaxy.tool_util.lint import LintContext + from galaxy.tool_util.parser.interface import ToolSource + from galaxy.util.etree import ( + Element, + ElementTree, + ) PROFILE_PATTERN = re.compile(r"^[12]\d\.\d{1,2}$") -PROFILE_INFO_DEFAULT_MSG = "Tool targets 16.01 Galaxy profile." -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]." 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 +def _tool_xml_and_root(tool_source: "ToolSource") -> Tuple["ElementTree", "Element"]: tool_xml = getattr(tool_source, "xml_tree", None) if tool_xml: tool_node = tool_xml.getroot() else: tool_node = None - version = tool_source.parse_version() or "" - parsed_version = parse_version(version) - if not version: - lint_ctx.error(ERROR_VERSION_MSG, node=tool_node) - elif isinstance(parsed_version, LegacyVersion): - lint_ctx.warn(WARN_VERSION_MSG % version, node=tool_node) - elif version != version.strip(): - lint_ctx.warn(WARN_WHITESPACE_PRESUFFIX % ("Tool version", version), node=tool_node) - else: - lint_ctx.valid(VALID_VERSION_MSG % version, node=tool_node) + return tool_xml, tool_node - name = tool_source.parse_name() - if not name: - lint_ctx.error(ERROR_NAME_MSG, node=tool_node) - elif name != name.strip(): - lint_ctx.warn(WARN_WHITESPACE_PRESUFFIX % ("Tool name", name), node=tool_node) - else: - 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=tool_node) - elif re.search(r"\s", tool_id): - lint_ctx.warn(WARN_ID_WHITESPACE_MSG % tool_id, node=tool_node) - else: - 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=tool_node) - elif profile == "16.01": - lint_ctx.valid(PROFILE_INFO_DEFAULT_MSG, node=tool_node) - else: - lint_ctx.valid(PROFILE_INFO_SPECIFIED_MSG % profile, node=tool_node) +class ToolVersionMissing(Linter): + """ + Tools must have a version + """ + + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + tool_xml, tool_node = _tool_xml_and_root(tool_source) + version = tool_source.parse_version() or "" + if not version: + lint_ctx.error("Tool version is missing or empty.", linter=cls.name(), node=tool_node) + + +class ToolVersionPEP404(Linter): + """ + Tools should have a PEP404 compliant version. + """ + + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + tool_xml, tool_node = _tool_xml_and_root(tool_source) + version = tool_source.parse_version() or "" + parsed_version = parse_version(version) + if version and isinstance(parsed_version, LegacyVersion): + lint_ctx.warn(f"Tool version [{version}] is not compliant with PEP 440.", linter=cls.name(), node=tool_node) + + +class ToolVersionWhitespace(Linter): + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + tool_xml, tool_node = _tool_xml_and_root(tool_source) + version = tool_source.parse_version() or "" + if version != version.strip(): + lint_ctx.warn( + f"Tool version is pre/suffixed by whitespace, this may cause errors: [{version}].", + linter=cls.name(), + node=tool_node, + ) + + +class ToolVersionValid(Linter): + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + tool_xml, tool_node = _tool_xml_and_root(tool_source) + version = tool_source.parse_version() or "" + parsed_version = parse_version(version) + if version and not isinstance(parsed_version, LegacyVersion) and version == version.strip(): + lint_ctx.valid(f"Tool defines a version [{version}].", linter=cls.name(), node=tool_node) + + +class ToolNameMissing(Linter): + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + _, tool_node = _tool_xml_and_root(tool_source) + name = tool_source.parse_name() + if not name: + lint_ctx.error("Tool name is missing or empty.", linter=cls.name(), node=tool_node) + + +class ToolNameWhitespace(Linter): + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + _, tool_node = _tool_xml_and_root(tool_source) + name = tool_source.parse_name() + if name and name != name.strip(): + lint_ctx.warn( + f"Tool name is pre/suffixed by whitespace, this may cause errors: [{name}].", + linter=cls.name(), + node=tool_node, + ) + - requirements, containers, resource_requirements = tool_source.parse_requirements_and_containers() - for r in requirements: - if r.type == "package": +class ToolNameValid(Linter): + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + _, tool_node = _tool_xml_and_root(tool_source) + name = tool_source.parse_name() + if name and name == name.strip(): + lint_ctx.valid(f"Tool defines a name [{name}].", linter=cls.name(), node=tool_node) + + +class ToolIDMissing(Linter): + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + _, tool_node = _tool_xml_and_root(tool_source) + tool_id = tool_source.parse_id() + if not tool_id: + lint_ctx.error("Tool does not define an id attribute.", linter=cls.name(), node=tool_node) + + +class ToolIDWhitespace(Linter): + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + _, tool_node = _tool_xml_and_root(tool_source) + tool_id = tool_source.parse_id() + if tool_id and re.search(r"\s", tool_id): + lint_ctx.warn( + f"Tool ID contains whitespace - this is discouraged: [{tool_id}].", linter=cls.name(), node=tool_node + ) + + +class ToolIDValid(Linter): + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + _, tool_node = _tool_xml_and_root(tool_source) + tool_id = tool_source.parse_id() + if tool_id and not re.search(r"\s", tool_id): + lint_ctx.valid(f"Tool defines an id [{tool_id}].", linter=cls.name(), node=tool_node) + + +class ToolProfileInvalid(Linter): + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + _, tool_node = _tool_xml_and_root(tool_source) + profile = tool_source.parse_profile() + profile_valid = PROFILE_PATTERN.match(profile) is not None + if not profile_valid: + lint_ctx.error(f"Tool specifies an invalid profile version [{profile}].", linter=cls.name(), node=tool_node) + + +class ToolProfileLegacy(Linter): + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + _, tool_node = _tool_xml_and_root(tool_source) + profile = tool_source.parse_profile() + profile_valid = PROFILE_PATTERN.match(profile) is not None + if profile_valid and profile == "16.01": + lint_ctx.valid("Tool targets 16.01 Galaxy profile.", linter=cls.name(), node=tool_node) + + +class ToolProfileValid(Linter): + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + _, tool_node = _tool_xml_and_root(tool_source) + profile = tool_source.parse_profile() + profile_valid = PROFILE_PATTERN.match(profile) is not None + if profile_valid and profile != "16.01": + lint_ctx.valid(f"Tool specifies profile version [{profile}].", linter=cls.name(), node=tool_node) + + +class RequirementNameMissing(Linter): + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + _, tool_node = _tool_xml_and_root(tool_source) + requirements, containers, resource_requirements = tool_source.parse_requirements_and_containers() + for r in requirements: + if r.type != "package": + continue if not r.name: - lint_ctx.error("Requirement without name found") + lint_ctx.error("Requirement without name found", linter=cls.name(), node=tool_node) + + +class RequirementVersionMissing(Linter): + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + _, tool_node = _tool_xml_and_root(tool_source) + requirements, containers, resource_requirements = tool_source.parse_requirements_and_containers() + for r in requirements: + if r.type != "package": + continue if not r.version: - lint_ctx.warn(f"Requirement {r.name} defines no version") - # Warn requirement attributes with leading/trailing whitespace: - elif r.version != r.version.strip(): - lint_ctx.warn(WARN_WHITESPACE_MSG % ("Requirement version", r.version)) - for rr in resource_requirements: - if rr.runtime_required: - lint_ctx.warn("Expressions in resource requirement not supported yet") + lint_ctx.warn(f"Requirement {r.name} defines no version", linter=cls.name(), node=tool_node) + + +class RequirementVersionWhitespace(Linter): + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + _, tool_node = _tool_xml_and_root(tool_source) + requirements, containers, resource_requirements = tool_source.parse_requirements_and_containers() + for r in requirements: + if r.type != "package": + continue + if r.version and r.version != r.version.strip(): + lint_ctx.warn( + f"Requirement version contains whitespace, this may cause errors: [{r.version}].", + linter=cls.name(), + node=tool_node, + ) + + +class ResourceRequirementExpression(Linter): + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + _, tool_node = _tool_xml_and_root(tool_source) + requirements, containers, resource_requirements = tool_source.parse_requirements_and_containers() + for rr in resource_requirements: + if rr.runtime_required: + lint_ctx.warn( + "Expressions in resource requirement not supported yet", linter=cls.name(), node=tool_node + ) diff --git a/lib/galaxy/tool_util/linters/help.py b/lib/galaxy/tool_util/linters/help.py index 0008d465662e..38f7be7c10d1 100644 --- a/lib/galaxy/tool_util/linters/help.py +++ b/lib/galaxy/tool_util/linters/help.py @@ -1,43 +1,111 @@ """This module contains a linting function for a tool's help.""" -from typing import Union +from typing import ( + TYPE_CHECKING, + Union, +) +from galaxy.tool_util.lint import Linter from galaxy.util import ( rst_to_html, unicodify, ) +if TYPE_CHECKING: + from galaxy.tool_util.lint import LintContext + from galaxy.tool_util.parser.interface import ToolSource + + +class HelpMissing(Linter): + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + tool_xml = getattr(tool_source, "xml_tree", None) + if not tool_xml: + return + root = tool_xml.find("./help") + if root is None: + root = tool_xml.getroot() + help = tool_xml.find("./help") + if help is None: + lint_ctx.warn( + "No help section found, consider adding a help section to your tool.", linter=cls.name(), node=root + ) + + +class HelpEmpty(Linter): + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + tool_xml = getattr(tool_source, "xml_tree", None) + if not tool_xml: + return + help = tool_xml.find("./help") + if help is None: + return + help_text = help.text or "" + if not help_text.strip(): + lint_ctx.warn("Help section appears to be empty.", linter=cls.name(), node=help) + + +class HelpPresent(Linter): + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + tool_xml = getattr(tool_source, "xml_tree", None) + if not tool_xml: + return + help = tool_xml.find("./help") + if help is None: + return + help_text = help.text or "" + if help_text.strip(): + lint_ctx.valid("Tool contains help section.", linter=cls.name(), node=help) + + +class HelpTODO(Linter): + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + tool_xml = getattr(tool_source, "xml_tree", None) + if not tool_xml: + return + help = tool_xml.find("./help") + if help is None: + return + help_text = help.text or "" + if "TODO" in help_text: + lint_ctx.warn("Help contains TODO text.", linter=cls.name(), node=help) + + +class HelpInvalidRST(Linter): + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + tool_xml = getattr(tool_source, "xml_tree", None) + if not tool_xml: + return + help = tool_xml.find("./help") + if help is None: + return + help_text = help.text or "" + if not help_text.strip(): + return + invalid_rst = rst_invalid(help_text) + if invalid_rst: + lint_ctx.warn(f"Invalid reStructuredText found in help - [{invalid_rst}].", linter=cls.name(), node=help) + -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.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=helps[1]) - return - - if len(helps) == 0: - lint_ctx.warn("No help section found, consider adding a help section to your tool.", node=root) - return - - help_text = helps[0].text or "" - if not help_text.strip(): - lint_ctx.warn("Help section appears to be empty.", node=helps[0]) - return - - lint_ctx.valid("Tool contains help section.", node=helps[0]) - - if "TODO" in help_text: - lint_ctx.warn("Help contains TODO text.", node=helps[0]) - - invalid_rst = rst_invalid(help_text) - if invalid_rst: - lint_ctx.warn(f"Invalid reStructuredText found in help - [{invalid_rst}].", node=helps[0]) - else: - lint_ctx.valid("Help contains valid reStructuredText.", node=helps[0]) +class HelpValidRST(Linter): + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + tool_xml = getattr(tool_source, "xml_tree", None) + if not tool_xml: + return + help = tool_xml.find("./help") + if help is None: + return + help_text = help.text or "" + if not help_text.strip(): + return + invalid_rst = rst_invalid(help_text) + if not invalid_rst: + lint_ctx.valid("Help contains valid reStructuredText.", linter=cls.name(), node=help) def rst_invalid(text: str) -> Union[bool, str]: diff --git a/lib/galaxy/tool_util/linters/inputs.py b/lib/galaxy/tool_util/linters/inputs.py index 4fdfb89ea070..ae6c8fb71954 100644 --- a/lib/galaxy/tool_util/linters/inputs.py +++ b/lib/galaxy/tool_util/linters/inputs.py @@ -2,8 +2,15 @@ import ast import re -from typing import TYPE_CHECKING +import warnings +from typing import ( + Iterator, + Optional, + Tuple, + TYPE_CHECKING, +) +from galaxy.tool_util.lint import Linter from galaxy.util import string_as_bool from ._util import ( is_datasource, @@ -14,19 +21,12 @@ if TYPE_CHECKING: from galaxy.tool_util.lint import LintContext from galaxy.tool_util.parser import ToolSource + from galaxy.util.etree import ( + Element, + ElementTree, + ) -FILTER_TYPES = [ - "data_meta", - "param_value", - "static_value", - "regexp", - "unique_value", - "multiple_splitter", - "attribute_value_splitter", - "add_value", - "remove_value", - "sort_by", -] +lint_tool_types = ["*"] ATTRIB_VALIDATOR_COMPATIBILITY = { "check": ["metadata"], @@ -123,308 +123,1016 @@ PARAM_TYPE_CHILD_COMBINATIONS = [ ("./options", ["data", "select", "drill_down"]), ("./options/option", ["drill_down"]), - ("./column", ["data_column"]), + ("./options/column", ["data", "select"]), ] +# TODO lint for valid param type - attribute combinations +# TODO check if dataset is available for filters referring other datasets +# TODO check if ref input param is present for from_dataset + + +class InputsNum(Linter): + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + tool_xml = getattr(tool_source, "xml_tree", None) + if not tool_xml: + return + tool_node = tool_xml.find("./inputs") + if tool_node is None: + tool_node = tool_xml.getroot() + num_inputs = len(tool_xml.findall("./inputs//param")) + if num_inputs: + lint_ctx.info(f"Found {num_inputs} input parameters.", linter=cls.name(), node=tool_node) + + +class InputsMissing(Linter): + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + tool_xml = getattr(tool_source, "xml_tree", None) + if not tool_xml: + return + tool_node = tool_xml.find("./inputs") + if tool_node is None: + tool_node = tool_xml.getroot() + num_inputs = len(tool_xml.findall("./inputs//param")) + if num_inputs == 0 and not is_datasource(tool_xml): + lint_ctx.warn("Found no input parameters.", linter=cls.name(), node=tool_node) + + +class InputsMissingDataSource(Linter): + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + tool_xml = getattr(tool_source, "xml_tree", None) + if not tool_xml: + return + tool_node = tool_xml.find("./inputs") + if tool_node is None: + tool_node = tool_xml.getroot() + num_inputs = len(tool_xml.findall("./inputs//param")) + if num_inputs == 0 and is_datasource(tool_xml): + lint_ctx.info("No input parameters, OK for data sources", linter=cls.name(), node=tool_node) + -def lint_inputs(tool_source: "ToolSource", lint_ctx: "LintContext"): - """Lint parameters in a tool's inputs block.""" - tool_xml = getattr(tool_source, "xml_tree", None) - if tool_xml is None: - return - profile = tool_source.parse_profile() - datasource = is_datasource(tool_xml) - input_names = set() - 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=param) +class InputsDatasourceTags(Linter): + """ + Lint that datasource tools have display and uihints tags + """ + + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + tool_xml = getattr(tool_source, "xml_tree", None) + if not tool_xml: + return + tool_node = tool_xml.find("./inputs") + if tool_node is None: + tool_node = tool_xml.getroot() + inputs = tool_xml.findall("./inputs//param") + if is_datasource(tool_xml): + # 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", linter=cls.name(), node=tool_node + ) + + +def _iter_param(tool_xml: "ElementTree") -> Iterator[Tuple["Element", str]]: + for param in tool_xml.findall("./inputs//param"): + if "name" not in param.attrib and "argument" not in param.attrib: 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=param, - ) - if param_name.strip() == "": - 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=param) - - # check for parameters with duplicate names - path = [param_name] - parent = param - while True: - parent = parent.getparent() - if parent.tag == "inputs": - break - # parameters of the same name in different when branches are allowed - # just add the value of the when branch to the path (this also allows - # that the conditional select has the same name as params in the whens) - if parent.tag == "when": - path.append(str(parent.attrib.get("value"))) - else: - path.append(str(parent.attrib.get("name"))) - path_str = ".".join(reversed(path)) - if path_str in input_names: - lint_ctx.error(f"Tool defines multiple parameters with the same name: '{path_str}'", node=param) - input_names.add(path_str) - - if "type" not in param_attrib: - lint_ctx.error(f"Param input [{param_name}] input with no type specified.", node=param) + param_name = _parse_name(param.attrib.get("name"), param.attrib.get("argument")) + yield param, param_name + + +def _iter_param_type(tool_xml: "ElementTree") -> Iterator[Tuple["Element", str, str]]: + for param, param_name in _iter_param(tool_xml): + if "type" not in param.attrib: continue - elif param_attrib["type"].strip() == "": - lint_ctx.error(f"Param input [{param_name}] with empty type specified.", node=param) + param_type = param.attrib["type"] + if param_type == "": continue - param_type = param_attrib["type"] + yield param, param_name, param_type + + +class InputsName(Linter): + """ + Lint params define a name + """ + + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + tool_xml = getattr(tool_source, "xml_tree", None) + if not tool_xml: + return + for param in tool_xml.findall("./inputs//param"): + if "name" not in param.attrib and "argument" not in param.attrib: + lint_ctx.error("Found param input with no name specified.", linter=cls.name(), node=param) + + +class InputsNameRedundantArgument(Linter): + """ + Lint params define a name + """ + + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + tool_xml = getattr(tool_source, "xml_tree", None) + if not tool_xml: + return + for param in tool_xml.findall("./inputs//param"): + if "name" in param.attrib and "argument" in param.attrib: + param_name = _parse_name(param.attrib.get("name"), param.attrib.get("argument")) + 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.", + linter=cls.name(), + node=param, + ) + + +# TODO redundant with InputsNameValid +class InputsNameEmpty(Linter): + """ + Lint params define a non-empty name + """ + + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + tool_xml = getattr(tool_source, "xml_tree", None) + if not tool_xml: + return + for param, param_name in _iter_param(tool_xml): + if param_name.strip() == "": + lint_ctx.error("Param input with empty name.", linter=cls.name(), node=param) + + +class InputsNameValid(Linter): + """ + Lint params define valid cheetah placeholder + """ + + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + tool_xml = getattr(tool_source, "xml_tree", None) + if not tool_xml: + return + for param, param_name in _iter_param(tool_xml): + if param_name != "" and not is_valid_cheetah_placeholder(param_name): + lint_ctx.warn( + f"Param input [{param_name}] is not a valid Cheetah placeholder.", linter=cls.name(), node=param + ) - # TODO lint for valid param type - attribute combinations - # lint for valid param type - child node combinations - for ptcc in PARAM_TYPE_CHILD_COMBINATIONS: - if param.find(ptcc[0]) is not None and param_type not in ptcc[1]: +def _param_path(param: "Element", param_name: str) -> str: + path = [param_name] + parent = param + while True: + parent = parent.getparent() + if parent.tag == "inputs": + break + # parameters of the same name in different when branches are allowed + # just add the value of the when branch to the path (this also allows + # that the conditional select has the same name as params in the whens) + if parent.tag == "when": + path.append(str(parent.attrib.get("value"))) + else: + path.append(str(parent.attrib.get("name"))) + return ".".join(reversed(path)) + + +class InputsNameDuplicate(Linter): + """ + Lint params with duplicate names + """ + + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + tool_xml = getattr(tool_source, "xml_tree", None) + if not tool_xml: + return + input_names = set() + for param, param_name in _iter_param(tool_xml): + # check for parameters with duplicate names + path = _param_path(param, param_name) + if path in input_names: lint_ctx.error( - f"Parameter [{param_name}] '{ptcc[0]}' tags are only allowed for parameters of type {ptcc[1]}", - node=param, + f"Tool defines multiple parameters with the same name: '{path}'", linter=cls.name(), node=param ) + input_names.add(path) + + +class InputsNameDuplicateOutput(Linter): + """ + Lint params names that are also used in outputs + """ + + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + tool_xml = getattr(tool_source, "xml_tree", None) + if not tool_xml: + return + input_names = set() + for param, param_name in _iter_param(tool_xml): + input_names.add(_param_path(param, param_name)) + + # check if there is an output with the same name as an input + outputs = tool_xml.findall("./outputs/*") + for output in outputs: + if output.get("name") in input_names: + lint_ctx.error( + f'Tool defines an output with a name equal to the name of an input: \'{output.get("name")}\'', + linter=cls.name(), + node=output, + ) + + +class InputsTypeChildCombination(Linter): + """ + Lint for invalid parameter type child combinations + """ - # param type specific linting - if param_type == "data": - if "format" not in param_attrib: + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + tool_xml = getattr(tool_source, "xml_tree", None) + if not tool_xml: + return + for param, param_name, param_type in _iter_param_type(tool_xml): + # lint for valid param type - child node combinations + for ptcc in PARAM_TYPE_CHILD_COMBINATIONS: + if param.find(ptcc[0]) is not None and param_type not in ptcc[1]: + lint_ctx.error( + f"Parameter [{param_name}] '{ptcc[0]}' tags are only allowed for parameters of type {ptcc[1]}", + linter=cls.name(), + node=param, + ) + + +class InputsDataFormat(Linter): + """ + Lint for data params wo format + """ + + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + tool_xml = getattr(tool_source, "xml_tree", None) + if not tool_xml: + return + for param, param_name, param_type in _iter_param_type(tool_xml): + if param_type != "data": + continue + if "format" not in param.attrib: lint_ctx.warn( - f"Param input [{param_name}] with no format specified - 'data' format will be assumed.", node=param + f"Param input [{param_name}] with no format specified - 'data' format will be assumed.", + linter=cls.name(), + node=param, ) + + +class InputsDataOptionsMultiple(Linter): + """ + Lint for data params with multiple options tags + """ + + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + tool_xml = getattr(tool_source, "xml_tree", None) + if not tool_xml: + return + for param, param_name, param_type in _iter_param_type(tool_xml): + if param_type != "data": + continue options = param.findall("./options") - has_options_filter_attribute = False - if len(options) == 1: - for oa in options[0].attrib: - if oa == "options_filter_attribute": - has_options_filter_attribute = True - else: - lint_ctx.error(f"Data parameter [{param_name}] uses invalid attribute: {oa}", node=param) - elif len(options) > 1: - lint_ctx.error(f"Data parameter [{param_name}] contains multiple options elements.", node=options[1]) - # for data params only filters with key='build' of type='data_meta' are allowed - filters = param.findall("./options/filter") - for f in filters: - if not f.get("ref"): + if len(options) > 1: + lint_ctx.error( + f"Data parameter [{param_name}] contains multiple options elements.", + linter=cls.name(), + node=options[1], + ) + + +class InputsDataOptionsAttrib(Linter): + """ + Lint for data params with options that have invalid attributes + """ + + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + tool_xml = getattr(tool_source, "xml_tree", None) + if not tool_xml: + return + for param, param_name, param_type in _iter_param_type(tool_xml): + if param_type != "data": + continue + options = param.find("./options") + if options is None: + continue + for oa in options.attrib: + if oa != "options_filter_attribute": lint_ctx.error( - f"Data parameter [{param_name}] filter needs to define a ref attribute", - node=f, + f"Data parameter [{param_name}] uses invalid attribute: {oa}", linter=cls.name(), node=param ) - if has_options_filter_attribute: - if f.get("type") != "data_meta": + + +class InputsDataOptionsFilterAttribFiltersType(Linter): + """ + Lint for valid filter types for data parameters + if options set options_filter_attribute + """ + + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + tool_xml = getattr(tool_source, "xml_tree", None) + if not tool_xml: + return + for param, param_name, param_type in _iter_param_type(tool_xml): + if param_type != "data": + continue + options = param.find("./options") + if options is None: + continue + for filter in param.findall("./options/filter"): + if "options_filter_attribute" in options.attrib: + if filter.get("type") != "data_meta": lint_ctx.error( - f'Data parameter [{param_name}] for filters only type="data_meta" is allowed, found type="{f.get("type")}"', - node=f, + f'Data parameter [{param_name}] for filters only type="data_meta" is allowed, found type="{filter.get("type")}"', + linter=cls.name(), + node=filter, ) - else: - if f.get("key") != "dbkey" or f.get("type") != "data_meta": + + +class InputsDataOptionsFiltersType(Linter): + """ + Lint for valid filter types for data parameters + if options do not set options_filter_attribute + """ + + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + tool_xml = getattr(tool_source, "xml_tree", None) + if not tool_xml: + return + for param, param_name, param_type in _iter_param_type(tool_xml): + if param_type != "data": + continue + options = param.find("./options") + if options is None: + continue + for filter in param.findall("./options/filter"): + if "options_filter_attribute" not in options.attrib: + if filter.get("key") != "dbkey" or filter.get("type") != "data_meta": lint_ctx.error( - f'Data parameter [{param_name}] for filters only type="data_meta" and key="dbkey" are allowed, found type="{f.get("type")}" and key="{f.get("key")}"', - node=f, + f'Data parameter [{param_name}] for filters only type="data_meta" and key="dbkey" are allowed, found type="{filter.get("type")}" and key="{filter.get("key")}"', + linter=cls.name(), + node=filter, ) - elif param_type == "select": - # get dynamic/statically defined options - dynamic_options = param.get("dynamic_options", None) - options = param.findall("./options") - filters = param.findall("./options/filter") - select_options = param.findall("./option") +class InputsDataOptionsFiltersRef(Linter): + """ + Lint for set ref for filters of data parameters + """ + + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + tool_xml = getattr(tool_source, "xml_tree", None) + if not tool_xml: + return + for param, param_name, param_type in _iter_param_type(tool_xml): + if param_type != "data": + continue + options = param.find("./options") + if options is None: + continue + for filter in param.findall("./options/filter"): + if not filter.get("ref"): + lint_ctx.error( + f"Data parameter [{param_name}] filter needs to define a ref attribute", + linter=cls.name(), + node=filter, + ) + + +class InputsSelectDynamicOptions(Linter): + """ + Lint for select with deprecated dynamic_options attribute + """ + + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + tool_xml = getattr(tool_source, "xml_tree", None) + if not tool_xml: + return + for param, param_name, param_type in _iter_param_type(tool_xml): + if param_type != "select": + continue + dynamic_options = param.get("dynamic_options", None) if dynamic_options is not None: lint_ctx.warn( - f"Select parameter [{param_name}] uses deprecated 'dynamic_options' attribute.", node=param + f"Select parameter [{param_name}] uses deprecated 'dynamic_options' attribute.", + linter=cls.name(), + node=param, ) - # check if options are defined by exactly one possibility + +class InputsSelectOptionsDef(Linter): + """ + Lint for valid ways to define select options + """ + + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + tool_xml = getattr(tool_source, "xml_tree", None) + if not tool_xml: + return + for param, param_name, param_type in _iter_param_type(tool_xml): + if param_type != "select": + continue + dynamic_options = param.get("dynamic_options", None) + options = param.findall("./options") + select_options = param.findall("./option") 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.", + linter=cls.name(), node=param, ) - else: - if len(select_options) == 0: + + +class InputsSelectOptionsDefConditional(Linter): + """ + Lint for valid ways to define select options + (for a select in a conditional) + """ + + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + tool_xml = getattr(tool_source, "xml_tree", None) + if not tool_xml: + return + for param, param_name, param_type in _iter_param_type(tool_xml): + if param_type != "select": + continue + dynamic_options = param.get("dynamic_options", None) + options = param.findall("./options") + select_options = param.findall("./option") + if param.getparent().tag == "conditional": + if len(select_options) == 0 or dynamic_options is not None or len(options) > 0: lint_ctx.error( f"Select parameter of a conditional [{param_name}] options have to be defined by 'option' children elements.", + linter=cls.name(), node=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.", node=f) - continue - if ftype not in FILTER_TYPES: - 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 - # TODO more linting of filters - from_file = options[0].get("from_file", None) - 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 +# TODO xsd +class InputsSelectOptionValueMissing(Linter): + """ + Lint for select option tags without value + """ - 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=options[0], - ) + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + tool_xml = getattr(tool_source, "xml_tree", None) + if not tool_xml: + return + for param, param_name, param_type in _iter_param_type(tool_xml): + if param_type != "select": + continue + select_options = param.findall("./option") + if any("value" not in option.attrib for option in select_options): + lint_ctx.error( + f"Select parameter [{param_name}] has option without value", linter=cls.name(), node=param + ) - 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=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=options[0], - ) +class InputsSelectOptionDuplicateValue(Linter): + """ + Lint for select option with same value + """ - 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=options[0], - ) + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + tool_xml = getattr(tool_source, "xml_tree", None) + if not tool_xml: + return + for param, param_name, param_type in _iter_param_type(tool_xml): + if param_type != "select": + continue + select_options = param.findall("./option") + select_options_values = list() + for option in select_options: + value = option.attrib.get("value", "") + select_options_values.append((value, option.attrib.get("selected", "false"))) + 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", + linter=cls.name(), + node=param, + ) - elif len(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=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=param) +class InputsSelectOptionDuplicateText(Linter): + """ + Lint for select option with same text + """ + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + tool_xml = getattr(tool_source, "xml_tree", None) + if not tool_xml: + return + for param, param_name, param_type in _iter_param_type(tool_xml): + if param_type != "select": + continue + select_options = param.findall("./option") select_options_texts = list() - select_options_values = list() for option in select_options: - value = option.attrib.get("value", "") if option.text is None: - text = value.capitalize() + text = option.attrib.get("value", "").capitalize() else: text = option.text 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=param + f"Select parameter [{param_name}] has multiple options with the same text content", + linter=cls.name(), + 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=param) - if param_type == "boolean": - truevalue = param_attrib.get("truevalue", "true") - falsevalue = param_attrib.get("falsevalue", "false") + +class InputsSelectOptionsMultiple(Linter): + """ + Lint dynamic options select for multiple options tags + """ + + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + tool_xml = getattr(tool_source, "xml_tree", None) + if not tool_xml: + return + for param, param_name, param_type in _iter_param_type(tool_xml): + if param_type != "select": + continue + options = param.findall("./options") + if len(options) > 1: + lint_ctx.error( + f"Select parameter [{param_name}] contains multiple options elements.", + linter=cls.name(), + node=options[1], + ) + + +class InputsSelectOptionsDefinesOptions(Linter): + """ + Lint dynamic options select for the potential to define options + """ + + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + tool_xml = getattr(tool_source, "xml_tree", None) + if not tool_xml: + return + for param, param_name, param_type in _iter_param_type(tool_xml): + if param_type != "select": + continue + options = param.find("./options") + if options is None: + continue + filter_adds_options = any( + [f.get("type", None) in ["add_value", "data_meta"] for f in param.findall("./options/filter")] + ) + from_file = options.get("from_file", None) + from_parameter = options.get("from_parameter", None) + # TODO check if input param is present for from_dataset + from_dataset = options.get("from_dataset", None) + from_data_table = options.get("from_data_table", None) + + 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.", + linter=cls.name(), + node=options, + ) + + +class InputsSelectOptionsDeprecatedAttr(Linter): + """ + Lint dynamic options select deprecated attributes + """ + + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + tool_xml = getattr(tool_source, "xml_tree", None) + if not tool_xml: + return + for param, param_name, param_type in _iter_param_type(tool_xml): + if param_type != "select": + continue + options = param.find("./options") + if options is None: + continue + for deprecated_attr in ["from_file", "from_parameter", "options_filter_attribute", "transform_lines"]: + if options.get(deprecated_attr) is not None: + lint_ctx.warn( + f"Select parameter [{param_name}] options uses deprecated '{deprecated_attr}' attribute.", + linter=cls.name(), + node=options, + ) + + +class InputsSelectOptionsFromDatasetAndDatatable(Linter): + """ + Lint dynamic options select for from_dataset and from_data_table + """ + + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + tool_xml = getattr(tool_source, "xml_tree", None) + if not tool_xml: + return + for param, param_name, param_type in _iter_param_type(tool_xml): + if param_type != "select": + continue + options = param.find("./options") + if options is None: + continue + from_dataset = options.get("from_dataset", None) + from_data_table = options.get("from_data_table", None) + 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.", + linter=cls.name(), + node=options, + ) + + +class InputsSelectOptionsMetaFileKey(Linter): + """ + Lint dynamic options select: meta_file_key attribute can only be used with from_dataset + """ + + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + tool_xml = getattr(tool_source, "xml_tree", None) + if not tool_xml: + return + for param, param_name, param_type in _iter_param_type(tool_xml): + if param_type != "select": + continue + options = param.find("./options") + if options is None: + continue + from_dataset = options.get("from_dataset", None) + if options.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'.", + linter=cls.name(), + node=options, + ) + + +class InputsBoolDistinctValues(Linter): + """ + Lint booleans for distinct true/false value + """ + + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + tool_xml = getattr(tool_source, "xml_tree", None) + if not tool_xml: + return + for param, param_name, param_type in _iter_param_type(tool_xml): + if param_type != "boolean": + continue + profile = tool_source.parse_profile() + truevalue = param.attrib.get("truevalue", "true") + falsevalue = param.attrib.get("falsevalue", "false") problematic_booleans_allowed = profile < "23.1" lint_level = lint_ctx.warn if problematic_booleans_allowed else lint_ctx.error if truevalue == falsevalue: lint_level( - f"Boolean parameter [{param_name}] needs distinct 'truevalue' and 'falsevalue' values.", node=param + f"Boolean parameter [{param_name}] needs distinct 'truevalue' and 'falsevalue' values.", + linter=cls.name(), + node=param, ) + + +class InputsBoolProblematic(Linter): + """ + Lint booleans for problematic values, i.e. truevalue being false and vice versa + """ + + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + tool_xml = getattr(tool_source, "xml_tree", None) + if not tool_xml: + return + for param, param_name, param_type in _iter_param_type(tool_xml): + if param_type != "boolean": + continue + profile = tool_source.parse_profile() + truevalue = param.attrib.get("truevalue", "true") + falsevalue = param.attrib.get("falsevalue", "false") + problematic_booleans_allowed = profile < "23.1" + lint_level = lint_ctx.warn if problematic_booleans_allowed else lint_ctx.error if truevalue.lower() == "false": - lint_level(f"Boolean parameter [{param_name}] has invalid truevalue [{truevalue}].", node=param) + lint_level( + f"Boolean parameter [{param_name}] has invalid truevalue [{truevalue}].", + linter=cls.name(), + node=param, + ) if falsevalue.lower() == "true": - lint_level(f"Boolean parameter [{param_name}] has invalid falsevalue [{falsevalue}].", node=param) + lint_level( + f"Boolean parameter [{param_name}] has invalid falsevalue [{falsevalue}].", + linter=cls.name(), + node=param, + ) + + +class InputsSelectSingleCheckboxes(Linter): + """ + Lint selects that allow only a single selection but display as checkboxes + """ - if param_type in ["select", "data_column", "drill_down"]: - multiple = string_as_bool(param_attrib.get("multiple", "false")) - optional = string_as_bool(param_attrib.get("optional", multiple)) - if param_attrib.get("display") == "checkboxes": + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + tool_xml = getattr(tool_source, "xml_tree", None) + if not tool_xml: + return + for param, param_name, param_type in _iter_param_type(tool_xml): + if param_type not in ["select", "data_column", "drill_down"]: + continue + multiple = string_as_bool(param.attrib.get("multiple", "false")) + 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', + linter=cls.name(), node=param, ) + + +class InputsSelectMandatoryCheckboxes(Linter): + """ + Lint selects that are mandatory but display as checkboxes + """ + + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + tool_xml = getattr(tool_source, "xml_tree", None) + if not tool_xml: + return + for param, param_name, param_type in _iter_param_type(tool_xml): + if param_type not in ["select", "data_column", "drill_down"]: + continue + 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 optional: lint_ctx.error( f'Select [{param_name}] `display="checkboxes"` is incompatible with `optional="false"`, remove the `display` attribute', + linter=cls.name(), node=param, ) - if param_attrib.get("display") == "radio": + + +class InputsSelectMultipleRadio(Linter): + """ + Lint selects that allow only multiple selections but display as radio + """ + + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + tool_xml = getattr(tool_source, "xml_tree", None) + if not tool_xml: + return + for param, param_name, param_type in _iter_param_type(tool_xml): + if param_type not in ["select", "data_column", "drill_down"]: + continue + multiple = string_as_bool(param.attrib.get("multiple", "false")) + if param.attrib.get("display") == "radio": if multiple: lint_ctx.error( - f'Select [{param_name}] display="radio" is incompatible with multiple="true"', node=param + f'Select [{param_name}] display="radio" is incompatible with multiple="true"', + linter=cls.name(), + node=param, ) + + +class InputsSelectOptionalRadio(Linter): + """ + Lint selects that are optional but display as radio + """ + + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + tool_xml = getattr(tool_source, "xml_tree", None) + if not tool_xml: + return + for param, param_name, param_type in _iter_param_type(tool_xml): + if param_type not in ["select", "data_column", "drill_down"]: + continue + multiple = string_as_bool(param.attrib.get("multiple", "false")) + optional = string_as_bool(param.attrib.get("optional", multiple)) + if param.attrib.get("display") == "radio": if optional: lint_ctx.error( - f'Select [{param_name}] display="radio" is incompatible with optional="true"', node=param + f'Select [{param_name}] display="radio" is incompatible with optional="true"', + linter=cls.name(), + node=param, ) - # TODO: Validate type, much more... - # lint validators - # TODO check if dataset is available for validators referring other datasets - validators = param.findall("./validator") + +def _iter_param_validator(tool_xml: "ElementTree") -> Iterator[Tuple[str, str, "Element", str]]: + input_params = tool_xml.findall("./inputs//param[@type]") + for param in input_params: + try: + param_name = _parse_name(param.attrib.get("name"), param.attrib.get("argument")) + except ValueError: + continue + param_type = param.attrib["type"] + validators = param.findall("./validator[@type]") for validator in validators: vtype = validator.attrib["type"] + yield (param_name, param_type, validator, vtype) + + +class ValidatorParamIncompatible(Linter): + """ + Lint for validator type - parameter type incompatibilities + """ + + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + tool_xml = getattr(tool_source, "xml_tree", None) + if not tool_xml: + return + for param_name, param_type, validator, vtype in _iter_param_validator(tool_xml): 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=validator + f"Parameter [{param_name}]: validator with an incompatible type '{vtype}'", + linter=cls.name(), + node=validator, ) + + +class ValidatorAttribIncompatible(Linter): + """ + Lint for incompatibilities between validator type and given attributes + """ + + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + tool_xml = getattr(tool_source, "xml_tree", None) + if not tool_xml: + return + for param_name, _, validator, vtype in _iter_param_validator(tool_xml): for attrib in ATTRIB_VALIDATOR_COMPATIBILITY: if attrib in validator.attrib and vtype not in ATTRIB_VALIDATOR_COMPATIBILITY[attrib]: lint_ctx.error( f"Parameter [{param_name}]: attribute '{attrib}' is incompatible with validator of type '{vtype}'", + linter=cls.name(), node=validator, ) + + +class ValidatorHasText(Linter): + """ + Lint that parameters that need text have text + """ + + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + tool_xml = getattr(tool_source, "xml_tree", None) + if not tool_xml: + return + for param_name, _, validator, vtype in _iter_param_validator(tool_xml): if vtype in ["expression", "regex"]: if validator.text is None: lint_ctx.error( - f"Parameter [{param_name}]: {vtype} validators are expected to contain text", node=validator + f"Parameter [{param_name}]: {vtype} validators are expected to contain text", + linter=cls.name(), + node=validator, ) - else: + + +class ValidatorHasNoText(Linter): + """ + Lint that parameters that need no text have no text + """ + + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + tool_xml = getattr(tool_source, "xml_tree", None) + if not tool_xml: + return + for param_name, _, validator, vtype in _iter_param_validator(tool_xml): + if vtype not in ["expression", "regex"] and validator.text is not None: + lint_ctx.warn( + f"Parameter [{param_name}]: '{vtype}' validators are not expected to contain text (found '{validator.text}')", + linter=cls.name(), + node=validator, + ) + + +class ValidatorExpression(Linter): + """ + Lint that checks expressions / regexp (ignoring FutureWarning) + """ + + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + tool_xml = getattr(tool_source, "xml_tree", None) + if not tool_xml: + return + with warnings.catch_warnings(): + warnings.simplefilter("error", FutureWarning) + for param_name, _, validator, vtype in _iter_param_validator(tool_xml): + if vtype in ["expression", "regex"] and validator.text is not None: try: if vtype == "regex": re.compile(validator.text) else: ast.parse(validator.text, mode="eval") + except FutureWarning: + pass except Exception as e: lint_ctx.error( - f"Parameter [{param_name}]: '{validator.text}' is no valid regular expression: {str(e)}", + f"Parameter [{param_name}]: '{validator.text}' is no valid {vtype}: {str(e)}", + linter=cls.name(), 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=validator, - ) + + +class ValidatorExpressionFuture(Linter): + """ + Lint that checks expressions / regexp FutureWarnings + """ + + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + tool_xml = getattr(tool_source, "xml_tree", None) + if not tool_xml: + return + with warnings.catch_warnings(): + warnings.simplefilter("error", FutureWarning) + for param_name, _, validator, vtype in _iter_param_validator(tool_xml): + if vtype in ["expression", "regex"] and validator.text is not None: + try: + if vtype == "regex": + re.compile(validator.text) + else: + ast.parse(validator.text, mode="eval") + except FutureWarning as e: + lint_ctx.warn( + f"Parameter [{param_name}]: '{validator.text}' is marked as deprecated {vtype}: {str(e)}", + linter=cls.name(), + node=validator, + ) + except Exception: + pass + + +class ValidatorMinMax(Linter): + """ + Lint for min/max for validator that need it + """ + + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + tool_xml = getattr(tool_source, "xml_tree", None) + if not tool_xml: + return + for param_name, _, validator, vtype in _iter_param_validator(tool_xml): if vtype in ["in_range", "length", "dataset_metadata_in_range"] and ( "min" not in validator.attrib and "max" not in validator.attrib ): lint_ctx.error( f"Parameter [{param_name}]: '{vtype}' validators need to define the 'min' or 'max' attribute(s)", + linter=cls.name(), node=validator, ) + + +class ValidatorDatasetMetadataEqualValue(Linter): + """ + Lint dataset_metadata_equal needs value or value_json + """ + + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + tool_xml = getattr(tool_source, "xml_tree", None) + if not tool_xml: + return + for param_name, _, validator, vtype in _iter_param_validator(tool_xml): if vtype in ["dataset_metadata_equal"]: if ( not ("value" in validator.attrib or "value_json" in validator.attrib) @@ -432,19 +1140,61 @@ def lint_inputs(tool_source: "ToolSource", lint_ctx: "LintContext"): ): lint_ctx.error( f"Parameter [{param_name}]: '{vtype}' validators need to define the 'value'/'value_json' and 'metadata_name' attributes", + linter=cls.name(), node=validator, ) + + +class ValidatorDatasetMetadataEqualValueOrJson(Linter): + """ + Lint dataset_metadata_equal needs either value or value_json + """ + + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + tool_xml = getattr(tool_source, "xml_tree", None) + if not tool_xml: + return + for param_name, _, validator, vtype in _iter_param_validator(tool_xml): + if vtype in ["dataset_metadata_equal"]: if "value" in validator.attrib and "value_json" in validator.attrib: lint_ctx.error( f"Parameter [{param_name}]: '{vtype}' validators must not define the 'value' and the 'value_json' attributes", + linter=cls.name(), node=validator, ) + +class ValidatorMetadataCheckSkip(Linter): + """ + Lint metadata needs check or skip + """ + + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + tool_xml = getattr(tool_source, "xml_tree", None) + if not tool_xml: + return + for param_name, _, validator, vtype in _iter_param_validator(tool_xml): if vtype in ["metadata"] and ("check" not in validator.attrib and "skip" not in validator.attrib): lint_ctx.error( f"Parameter [{param_name}]: '{vtype}' validators need to define the 'check' or 'skip' attribute(s)", + linter=cls.name(), node=validator, ) + + +class ValidatorTableName(Linter): + """ + Lint table_name is present if needed + """ + + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + tool_xml = getattr(tool_source, "xml_tree", None) + if not tool_xml: + return + for param_name, _, validator, vtype in _iter_param_validator(tool_xml): if ( vtype in [ @@ -457,8 +1207,22 @@ def lint_inputs(tool_source: "ToolSource", lint_ctx: "LintContext"): ): lint_ctx.error( f"Parameter [{param_name}]: '{vtype}' validators need to define the 'table_name' attribute", + linter=cls.name(), node=validator, ) + + +class ValidatorMetadataName(Linter): + """ + Lint metadata_name is present if needed + """ + + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + tool_xml = getattr(tool_source, "xml_tree", None) + if not tool_xml: + return + for param_name, _, validator, vtype in _iter_param_validator(tool_xml): if ( vtype in [ @@ -471,120 +1235,132 @@ def lint_inputs(tool_source: "ToolSource", lint_ctx: "LintContext"): ): lint_ctx.error( f"Parameter [{param_name}]: '{vtype}' validators need to define the 'metadata_name' attribute", + linter=cls.name(), node=validator, ) - conditional_selects = tool_xml.findall("./inputs//conditional") - for conditional in conditional_selects: + +def _iter_conditional(tool_xml: "ElementTree") -> Iterator[Tuple["Element", Optional[str], "Element", Optional[str]]]: + conditionals = tool_xml.findall("./inputs//conditional") + for conditional in conditionals: conditional_name = conditional.get("name") - if not conditional_name: - lint_ctx.error("Conditional without a name", node=conditional) - if conditional.get("value_from"): - # Probably only the upload tool use this, no children elements + if conditional.get("value_from"): # Probably only the upload tool use this, no children elements continue - first_param = conditional.findall("param") - if len(first_param) != 1: - lint_ctx.error( - f"Conditional [{conditional_name}] needs exactly one child found {len(first_param)}", - node=conditional, - ) + first_param = conditional.find("param") + if first_param is None: 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=first_param, - ) - elif first_param_type != "select": - lint_ctx.error(f'Conditional [{conditional_name}] first param should have type="select"', node=first_param) - continue + yield conditional, conditional_name, first_param, first_param_type - if first_param_type == "select": - select_options = _find_with_attribute(first_param, "option", "value") - option_ids = [option.get("value") for option in select_options] - else: # boolean - option_ids = [first_param.get("truevalue", "true"), first_param.get("falsevalue", "false")] - for incomp in ["optional", "multiple"]: - if string_as_bool(first_param.get(incomp, False)): +class ConditionalParamTypeBool(Linter): + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + tool_xml = getattr(tool_source, "xml_tree", None) + if not tool_xml: + return + for _, conditional_name, first_param, first_param_type in _iter_conditional(tool_xml): + if first_param_type == "boolean": lint_ctx.warn( - f'Conditional [{conditional_name}] test parameter cannot be {incomp}="true"', node=first_param + f'Conditional [{conditional_name}] first param of type="boolean" is discouraged, use a select', + linter=cls.name(), + 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=conditional) - - when_ids = [w.get("value") for w in whens if w.get("value") is not None] - for option_id in option_ids: - if option_id not in when_ids: - lint_ctx.warn( - f"Conditional [{conditional_name}] no block found for {first_param_type} option '{option_id}'", - node=conditional, +class ConditionalParamType(Linter): + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + tool_xml = getattr(tool_source, "xml_tree", None) + if not tool_xml: + return + for _, conditional_name, first_param, first_param_type in _iter_conditional(tool_xml): + if first_param_type not in ["boolean", "select"]: + lint_ctx.error( + f'Conditional [{conditional_name}] first param should have type="select"', + linter=cls.name(), + node=first_param, ) - 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