add file name to linter message

- add fname to LintMessage
- LintContext make lint messages persistent over multiple linter calls
  before the messages were reset before the call of the next linter
  which is "impractical" if we want to use the lintmessages eg in
  planemo
- add test for linting a tool + (internal and external) macros
  to ensure that the fname and line number is correct
- add a simple linter test for yaml and json tools

lib/galaxy/tool_util/lint.py

@@ -64,19 +64,26 @@ def lint_xml_with(lint_context, tool_xml, extra_modules=None):
 
 
 class LintMessage:
-    def __init__(self, level, message, line, xpath=None):
+    """
+    a message from the linter
+    """
+    def __init__(self, level, message, line, fname, xpath=None):
         self.level = level
         self.message = message
-        self.line = line
+        self.line = line  # 1 based
+        self.fname = fname
         self.xpath = xpath
 
     def __str__(self):
         rval = f".. {self.level.upper()}: {self.message}"
         if self.line is not None:
-            rval += f" (line {self.line})"
+            rval += " ("
+            if self.fname:
+                rval += f"{self.fname}:"
+            rval += str(self.line)
+            rval += ")"
         if self.xpath is not None:
             rval += f" [{self.xpath}]"
-
         return rval
 
 
@@ -91,12 +98,14 @@ class LintContext:
         self.object_name = object_name
         self.found_errors = False
         self.found_warns = False
+        self.message_list = []
 
     def lint(self, name, lint_func, lint_target):
         name = name.replace("tsts", "tests")[len("lint_"):]
         if name in self.skip_types:
             return
         self.printed_linter_info = False
+        tmp_message_list = list(self.message_list)
         self.message_list = []
 
         # call linter
@@ -141,6 +150,7 @@ class LintContext:
                     continue
                 print_linter_info()
                 print(f"{message}")
+        self.message_list = tmp_message_list + self.message_list
 
     @property
     def valid_messages(self):
@@ -158,22 +168,23 @@ class LintContext:
     def error_messages(self):
         return [x.message for x in self.message_list if x.level == "error"]
 
-    def __handle_message(self, level, message, line, xpath, *args):
+    def __handle_message(self, level, message, line, fname, xpath, *args):
         if args:
             message = message % args
-        self.message_list.append(LintMessage(level=level, message=message, line=line, xpath=xpath))
+        self.message_list.append(LintMessage(level=level, message=message,
+                                 line=line, fname=fname, xpath=xpath))
 
-    def valid(self, message, line=None, xpath=None, *args):
-        self.__handle_message("check", message, line, xpath, *args)
+    def valid(self, message, line=None, fname=None, xpath=None, *args):
+        self.__handle_message("check", message, line, fname, xpath, *args)
 
-    def info(self, message, line=None, xpath=None, *args):
-        self.__handle_message("info", message, line, xpath, *args)
+    def info(self, message, line=None, fname=None, xpath=None, *args):
+        self.__handle_message("info", message, line, fname, xpath, *args)
 
-    def error(self, message, line=None, xpath=None, *args):
-        self.__handle_message("error", message, line, xpath, *args)
+    def error(self, message, line=None, fname=None, xpath=None, *args):
+        self.__handle_message("error", message, line, fname, xpath, *args)
 
-    def warn(self, message, line=None, xpath=None, *args):
-        self.__handle_message("warning", message, line, xpath, *args)
+    def warn(self, message, line=None, fname=None, xpath=None, *args):
+        self.__handle_message("warning", message, line, fname, xpath, *args)
 
     def failed(self, fail_level):
         found_warns = self.found_warns

lib/galaxy/tool_util/linters/_util.py

@@ -1,6 +1,15 @@
 import re
 
 
+def node_props(node, tool_xml):
+    if node is None:
+        return {"line": 0, "fname": None, "xpath": None}
+    else:
+        return {"line": node.sourceline,
+                "fname": node.base,
+                "xpath": tool_xml.getpath(node)}
+
+
 def is_datasource(tool_xml):
     """Returns true if the tool is a datasource tool"""
     return tool_xml.getroot().attrib.get('tool_type', '') == 'data_source'

lib/galaxy/tool_util/linters/citations.py

@@ -3,41 +3,36 @@
 Citations describe references that should be used when consumers
 of the tool publish results.
 """
+from ._util import node_props
 
 
 def lint_citations(tool_xml, lint_ctx):
     """Ensure tool contains at least one valid citation."""
     root = tool_xml.getroot()
-    if root is not None:
-        root_line = root.sourceline
-        root_xpath = tool_xml.getpath(root)
-    else:
-        root_line = 1
-        root_xpath = None
     citations = root.findall("citations")
     if len(citations) > 1:
-        lint_ctx.error("More than one citation section found, behavior undefined.", line=citations[1].sourceline, xpath=tool_xml.getpath(citations[1]))
+        lint_ctx.error("More than one citation section found, behavior undefined.", **node_props(citations[1], tool_xml))
         return
 
     if len(citations) == 0:
-        lint_ctx.warn("No citations found, consider adding citations to your tool.", line=root_line, xpath=root_xpath)
+        lint_ctx.warn("No citations found, consider adding citations to your tool.", **node_props(root, tool_xml))
         return
 
     valid_citations = 0
     for citation in citations[0]:
         if citation.tag != "citation":
-            lint_ctx.warn(f"Unknown tag discovered in citations block [{citation.tag}], will be ignored.", line=citation.sourceline, xpath=tool_xml.getpath(citation))
+            lint_ctx.warn(f"Unknown tag discovered in citations block [{citation.tag}], will be ignored.", **node_props(citation, tool_xml))
             continue
         citation_type = citation.attrib.get("type")
         if citation_type not in ('bibtex', 'doi'):
-            lint_ctx.warn(f"Unknown citation type discovered [{citation_type}], will be ignored.", line=citation.sourceline, xpath=tool_xml.getpath(citation))
+            lint_ctx.warn(f"Unknown citation type discovered [{citation_type}], will be ignored.", **node_props(citation, tool_xml))
             continue
         if citation.text is None or not citation.text.strip():
-            lint_ctx.error(f'Empty {citation_type} citation.', line=citation.sourceline, xpath=tool_xml.getpath(citation))
+            lint_ctx.error(f'Empty {citation_type} citation.', **node_props(citation, tool_xml))
             continue
         valid_citations += 1
 
     if valid_citations > 0:
-        lint_ctx.valid(f"Found {valid_citations} likely valid citations.", line=root_line, xpath=root_xpath)
+        lint_ctx.valid(f"Found {valid_citations} likely valid citations.", **node_props(root, tool_xml))
     else:
-        lint_ctx.warn("Found no valid citations.", line=root_line, xpath=root_xpath)
+        lint_ctx.warn("Found no valid citations.", **node_props(root, tool_xml))

lib/galaxy/tool_util/linters/command.py

@@ -3,31 +3,26 @@
 A command description describes how to build the command-line to execute
 from supplied inputs.
 """
+from ._util import node_props
 
 
 def lint_command(tool_xml, lint_ctx):
     """Ensure tool contains exactly one command and check attributes."""
     root = tool_xml.getroot()
-    if root is not None:
-        root_line = root.sourceline
-        root_path = tool_xml.getpath(root)
-    else:
-        root_line = 1
-        root_path = None
     commands = root.findall("command")
     if len(commands) > 1:
-        lint_ctx.error("More than one command tag found, behavior undefined.", line=commands[1].sourceline, xpath=tool_xml.getpath(commands[1]))
+        lint_ctx.error("More than one command tag found, behavior undefined.", **node_props(commands[1], tool_xml))
         return
 
     if len(commands) == 0:
-        lint_ctx.error("No command tag found, must specify a command template to execute.", line=root_line, xpath=root_path)
+        lint_ctx.error("No command tag found, must specify a command template to execute.", **node_props(root, tool_xml))
         return
 
     command = get_command(tool_xml)
     if command.text is None:
-        lint_ctx.error("Command is empty.", line=root_line, xpath=root_path)
+        lint_ctx.error("Command is empty.", **node_props(root, tool_xml))
     elif "TODO" in command.text:
-        lint_ctx.warn("Command template contains TODO text.", line=command.sourceline, xpath=tool_xml.getpath(command))
+        lint_ctx.warn("Command template contains TODO text.", **node_props(command, tool_xml))
 
     command_attrib = command.attrib
     interpreter_type = None
@@ -37,14 +32,14 @@ def lint_command(tool_xml, lint_ctx):
         elif key == "detect_errors":
             detect_errors = value
             if detect_errors not in ["default", "exit_code", "aggressive"]:
-                lint_ctx.warn(f"Unknown detect_errors attribute [{detect_errors}]", line=command.sourceline, xpath=tool_xml.getpath(command))
+                lint_ctx.warn(f"Unknown detect_errors attribute [{detect_errors}]", **node_props(command, tool_xml))
 
     interpreter_info = ""
     if interpreter_type:
         interpreter_info = f" with interpreter of type [{interpreter_type}]"
     if interpreter_type:
-        lint_ctx.warn("Command uses deprecated 'interpreter' attribute.", line=command.sourceline, xpath=tool_xml.getpath(command))
-    lint_ctx.info(f"Tool contains a command{interpreter_info}.", line=command.sourceline, xpath=tool_xml.getpath(command))
+        lint_ctx.warn("Command uses deprecated 'interpreter' attribute.", **node_props(command, tool_xml))
+    lint_ctx.info(f"Tool contains a command{interpreter_info}.", **node_props(command, tool_xml))
 
 
 def get_command(tool_xml):

lib/galaxy/tool_util/linters/cwl.py

@@ -14,7 +14,7 @@ def lint_cwl_validation(tool_source, lint_ctx):
     except Exception as e:
         validation_exception = e
     if validation_exception:
-        lint_ctx.error("Failed to valdiate CWL artifact [%s]", validation_exception)
+        lint_ctx.error(f"Failed to valdiate CWL artifact: {validation_exception}")
     else:
         lint_ctx.info("CWL appears to be valid.")
 
@@ -28,7 +28,7 @@ def lint_new_draft(tool_source, lint_ctx):
     if cwl_version not in ["v1.0"]:
         lint_ctx.warn(f"CWL version [{cwl_version}] is unknown, we recommend the v1.0 the stable release.")
     else:
-        lint_ctx.info("Modern CWL version [%s]", cwl_version)
+        lint_ctx.info(f"Modern CWL version [{cwl_version}].")
 
 
 def lint_docker_image(tool_source, lint_ctx):

lib/galaxy/tool_util/linters/help.py

@@ -3,42 +3,37 @@ from galaxy.util import (
     rst_to_html,
     unicodify,
 )
+from ._util import node_props
 
 
 def lint_help(tool_xml, lint_ctx):
     """Ensure tool contains exactly one valid RST help block."""
-    # determine line to report for general problems with help
+    # determine node to report for general problems with help
     root = tool_xml.getroot()
-    if root is not None:
-        root_line = root.sourceline
-        root_path = tool_xml.getpath(root)
-    else:
-        root_line = 1
-        root_path = None
     helps = root.findall("help")
     if len(helps) > 1:
-        lint_ctx.error("More than one help section found, behavior undefined.", line=helps[1].sourceline, xpath=tool_xml.getpath(helps[1]))
+        lint_ctx.error("More than one help section found, behavior undefined.", **node_props(helps[1], tool_xml))
         return
 
     if len(helps) == 0:
-        lint_ctx.warn("No help section found, consider adding a help section to your tool.", line=root_line, xpath=root_path)
+        lint_ctx.warn("No help section found, consider adding a help section to your tool.", **node_props(root, tool_xml))
         return
 
     help = helps[0].text or ''
     if not help.strip():
-        lint_ctx.warn("Help section appears to be empty.", line=helps[0].sourceline, xpath=tool_xml.getpath(helps[0]))
+        lint_ctx.warn("Help section appears to be empty.", **node_props(helps[0], tool_xml))
         return
 
-    lint_ctx.valid("Tool contains help section.", line=helps[0].sourceline, xpath=tool_xml.getpath(helps[0]))
+    lint_ctx.valid("Tool contains help section.", **node_props(helps[0], tool_xml))
     invalid_rst = rst_invalid(help)
 
     if "TODO" in help:
-        lint_ctx.warn("Help contains TODO text.", line=helps[0].sourceline, xpath=tool_xml.getpath(helps[0]))
+        lint_ctx.warn("Help contains TODO text.", **node_props(helps[0], tool_xml))
 
     if invalid_rst:
-        lint_ctx.warn(f"Invalid reStructuredText found in help - [{invalid_rst}].", line=helps[0].sourceline, xpath=tool_xml.getpath(helps[0]))
+        lint_ctx.warn(f"Invalid reStructuredText found in help - [{invalid_rst}].", **node_props(helps[0], tool_xml))
     else:
-        lint_ctx.valid("Help contains valid reStructuredText.", line=helps[0].sourceline, xpath=tool_xml.getpath(helps[0]))
+        lint_ctx.valid("Help contains valid reStructuredText.", **node_props(helps[0], tool_xml))
 
 
 def rst_invalid(text):

lib/galaxy/tool_util/linters/inputs.py

@@ -1,6 +1,12 @@
 """This module contains a linting functions for tool inputs."""
+import re
+
 from galaxy.util import string_as_bool
-from ._util import is_datasource, is_valid_cheetah_placeholder
+from ._util import (
+    is_datasource,
+    is_valid_cheetah_placeholder,
+    node_props
+)
 from ..parser.util import _parse_name
 
 FILTER_TYPES = [
@@ -46,13 +52,8 @@ PARAMETER_VALIDATOR_TYPE_COMPATIBILITY = {
 
 def lint_inputs(tool_xml, lint_ctx):
     """Lint parameters in a tool's inputs block."""
-    # determine line to report for general problems with outputs
-    try:
-        tool_line = tool_xml.find("./tool").sourceline
-        tool_path = tool_xml.getpath(tool_xml.find("./tool"))
-    except AttributeError:
-        tool_line = 1
-        tool_path = None
+    # determine node to report for general problems with outputs
+    tool_node = tool_xml.find("./tool")
     datasource = is_datasource(tool_xml)
     inputs = tool_xml.findall("./inputs//param")
     num_inputs = 0
@@ -60,33 +61,32 @@ def lint_inputs(tool_xml, lint_ctx):
         num_inputs += 1
         param_attrib = param.attrib
         if "name" not in param_attrib and "argument" not in param_attrib:
-            lint_ctx.error("Found param input with no name specified.", line=param.sourceline, xpath=tool_xml.getpath(param))
+            lint_ctx.error("Found param input with no name specified.", **node_props(param, tool_xml))
             continue
         param_name = _parse_name(param_attrib.get("name"), param_attrib.get("argument"))
         if "name" in param_attrib and "argument" in param_attrib:
             if param_attrib.get("name") == _parse_name(None, param_attrib.get("argument")):
-                lint_ctx.warn(f"Param input [{param_name}] 'name' attribute is redundant if argument implies the same name.")
+                lint_ctx.warn(f"Param input [{param_name}] 'name' attribute is redundant if argument implies the same name.", **node_props(param, tool_xml))
         if param_name.strip() == "":
-            lint_ctx.error("Param input with empty name.", line=param.sourceline, xpath=tool_xml.getpath(param))
+            lint_ctx.error("Param input with empty name.", **node_props(param, tool_xml))
         elif not is_valid_cheetah_placeholder(param_name):
-            lint_ctx.warn(f"Param input [{param_name}] is not a valid Cheetah placeholder.", line=param.sourceline, xpath=tool_xml.getpath(param))
+            lint_ctx.warn(f"Param input [{param_name}] is not a valid Cheetah placeholder.", **node_props(param, tool_xml))
 
         # TODO lint for params with duplicated name (in inputs & outputs)
 
         if "type" not in param_attrib:
-            lint_ctx.error(f"Param input [{param_name}] input with no type specified.", line=param.sourceline, xpath=tool_xml.getpath(param))
+            lint_ctx.error(f"Param input [{param_name}] input with no type specified.", **node_props(param, tool_xml))
             continue
         elif param_attrib["type"].strip() == "":
-            lint_ctx.error(f"Param input [{param_name}] with empty type specified.", line=param.sourceline, xpath=tool_xml.getpath(param))
+            lint_ctx.error(f"Param input [{param_name}] with empty type specified.", **node_props(param, tool_xml))
             continue
         param_type = param_attrib["type"]
 
         # TODO lint for valid param type - attribute combinations
-        # TODO lint required attributes for valid each param type
 
         if param_type == "data":
             if "format" not in param_attrib:
-                lint_ctx.warn(f"Param input [{param_name}] with no format specified - 'data' format will be assumed.", line=param.sourceline, xpath=tool_xml.getpath(param))
+                lint_ctx.warn(f"Param input [{param_name}] with no format specified - 'data' format will be assumed.", **node_props(param, tool_xml))
         elif param_type == "select":
             # get dynamic/statically defined options
             dynamic_options = param.get("dynamic_options", None)
@@ -95,15 +95,15 @@ def lint_inputs(tool_xml, lint_ctx):
             select_options = param.findall('./option')
 
             if dynamic_options is not None:
-                lint_ctx.warn(f"Select parameter [{param_name}] uses deprecated 'dynamic_options' attribute.", line=param.sourceline, xpath=tool_xml.getpath(param))
+                lint_ctx.warn(f"Select parameter [{param_name}] uses deprecated 'dynamic_options' attribute.", **node_props(param, tool_xml))
 
             # check if options are defined by exactly one possibility
             if param.getparent().tag != "conditional":
                 if (dynamic_options is not None) + (len(options) > 0) + (len(select_options) > 0) != 1:
-                    lint_ctx.error(f"Select parameter [{param_name}] options have to be defined by either 'option' children elements, a 'options' element or the 'dynamic_options' attribute.", line=param.sourceline, xpath=tool_xml.getpath(param))
+                    lint_ctx.error(f"Select parameter [{param_name}] options have to be defined by either 'option' children elements, a 'options' element or the 'dynamic_options' attribute.", **node_props(param, tool_xml))
             else:
                 if len(select_options) == 0:
-                    lint_ctx.error(f"Select parameter of a conditional [{param_name}] options have to be defined by 'option' children elements.", line=param.sourceline, xpath=tool_xml.getpath(param))
+                    lint_ctx.error(f"Select parameter of a conditional [{param_name}] options have to be defined by 'option' children elements.", **node_props(param, tool_xml))
 
             # lint dynamic options
             if len(options) == 1:
@@ -114,10 +114,10 @@ def lint_inputs(tool_xml, lint_ctx):
                 for f in filters:
                     ftype = f.get("type", None)
                     if ftype is None:
-                        lint_ctx.error(f"Select parameter [{param_name}] contains filter without type.", line=f.sourceline, xpath=tool_xml.getpath(f))
+                        lint_ctx.error(f"Select parameter [{param_name}] contains filter without type.", **node_props(f, tool_xml))
                         continue
                     if ftype not in FILTER_TYPES:
-                        lint_ctx.error(f"Select parameter [{param_name}] contains filter with unknown type '{ftype}'.", line=f.sourceline, xpath=tool_xml.getpath(f))
+                        lint_ctx.error(f"Select parameter [{param_name}] contains filter with unknown type '{ftype}'.", **node_props(f, tool_xml))
                         continue
                     if ftype in ['add_value', 'data_meta']:
                         filter_adds_options = True
@@ -132,33 +132,26 @@ def lint_inputs(tool_xml, lint_ctx):
                 if (from_file is None and from_parameter is None
                         and from_dataset is None and from_data_table is None
                         and not filter_adds_options):
-                    lint_ctx.error(f"Select parameter [{param_name}] options tag defines no options. Use 'from_dataset', 'from_data_table', or a filter that adds values.", line=options[0].sourceline, xpath=tool_xml.getpath(options[0]))
+                    lint_ctx.error(f"Select parameter [{param_name}] options tag defines no options. Use 'from_dataset', 'from_data_table', or a filter that adds values.", **node_props(options[0], tool_xml))
 
-                if from_file is not None:
-                    lint_ctx.warn(f"Select parameter [{param_name}] options uses deprecated 'from_file' attribute.", line=options[0].sourceline, xpath=tool_xml.getpath(options[0]))
-                if from_parameter is not None:
-                    lint_ctx.warn(f"Select parameter [{param_name}] options uses deprecated 'from_parameter' attribute.", line=options[0].sourceline, xpath=tool_xml.getpath(options[0]))
+                for deprecated_attr in ["from_file", "from_parameter", "options_filter_attribute", "transform_lines"]:
+                    if options[0].get(deprecated_attr) is not None:
+                        lint_ctx.warn(f"Select parameter [{param_name}] options uses deprecated '{deprecated_attr}' attribute.", **node_props(options[0], tool_xml))
 
                 if from_dataset is not None and from_data_table is not None:
-                    lint_ctx.error(f"Select parameter [{param_name}] options uses 'from_dataset' and 'from_data_table' attribute.", line=options[0].sourceline, xpath=tool_xml.getpath(options[0]))
+                    lint_ctx.error(f"Select parameter [{param_name}] options uses 'from_dataset' and 'from_data_table' attribute.", **node_props(options[0], tool_xml))
 
                 if options[0].get("meta_file_key", None) is not None and from_dataset is None:
-                    lint_ctx.error(f"Select parameter [{param_name}] 'meta_file_key' is only compatible with 'from_dataset'.", line=options[0].sourceline, xpath=tool_xml.getpath(options[0]))
-
-                if options[0].get("options_filter_attribute", None) is not None:
-                    lint_ctx.warn(f"Select parameter [{param_name}] options uses deprecated 'options_filter_attribute' attribute.", line=options[0].sourceline, xpath=tool_xml.getpath(options[0]))
-
-                if options[0].get("transform_lines", None) is not None:
-                    lint_ctx.warn(f"Select parameter [{param_name}] options uses deprecated 'transform_lines' attribute.", line=options[0].sourceline, xpath=tool_xml.getpath(options[0]))
+                    lint_ctx.error(f"Select parameter [{param_name}] 'meta_file_key' is only compatible with 'from_dataset'.", **node_props(options[0], tool_xml))
 
             elif len(options) > 1:
-                lint_ctx.error(f"Select parameter [{param_name}] contains multiple options elements.", line=options[1].sourceline, xpath=tool_xml.getpath(options[1]))
+                lint_ctx.error(f"Select parameter [{param_name}] contains multiple options elements.", **node_props(options[1], tool_xml))
 
             # lint statically defined options
             if any('value' not in option.attrib for option in select_options):
-                lint_ctx.error(f"Select parameter [{param_name}] has option without value", line=param.sourceline, xpath=tool_xml.getpath(param))
+                lint_ctx.error(f"Select parameter [{param_name}] has option without value", **node_props(param, tool_xml))
             if any(option.text is None for option in select_options):
-                lint_ctx.warn(f"Select parameter [{param_name}] has option without text", line=param.sourceline, xpath=tool_xml.getpath(param))
+                lint_ctx.warn(f"Select parameter [{param_name}] has option without text", **node_props(param, tool_xml))
 
             select_options_texts = list()
             select_options_values = list()
@@ -171,22 +164,22 @@ def lint_inputs(tool_xml, lint_ctx):
                 select_options_texts.append((text, option.attrib.get("selected", "false")))
                 select_options_values.append((value, option.attrib.get("selected", "false")))
             if len(set(select_options_texts)) != len(select_options_texts):
-                lint_ctx.error(f"Select parameter [{param_name}] has multiple options with the same text content", line=param.sourceline, xpath=tool_xml.getpath(param))
+                lint_ctx.error(f"Select parameter [{param_name}] has multiple options with the same text content", **node_props(param, tool_xml))
             if len(set(select_options_values)) != len(select_options_values):
-                lint_ctx.error(f"Select parameter [{param_name}] has multiple options with the same value", line=param.sourceline, xpath=tool_xml.getpath(param))
+                lint_ctx.error(f"Select parameter [{param_name}] has multiple options with the same value", **node_props(param, tool_xml))
 
             multiple = string_as_bool(param_attrib.get("multiple", "false"))
             optional = string_as_bool(param_attrib.get("optional", multiple))
             if param_attrib.get("display") == "checkboxes":
                 if not multiple:
-                    lint_ctx.error(f'Select [{param_name}] `display="checkboxes"` is incompatible with `multiple="false"`, remove the `display` attribute', line=param.sourceline, xpath=tool_xml.getpath(param))
+                    lint_ctx.error(f'Select [{param_name}] `display="checkboxes"` is incompatible with `multiple="false"`, remove the `display` attribute', **node_props(param, tool_xml))
                 if not optional:
-                    lint_ctx.error(f'Select [{param_name}] `display="checkboxes"` is incompatible with `optional="false"`, remove the `display` attribute', line=param.sourceline, xpath=tool_xml.getpath(param))
+                    lint_ctx.error(f'Select [{param_name}] `display="checkboxes"` is incompatible with `optional="false"`, remove the `display` attribute', **node_props(param, tool_xml))
             if param_attrib.get("display") == "radio":
                 if multiple:
-                    lint_ctx.error(f'Select [{param_name}] display="radio" is incompatible with multiple="true"', line=param.sourceline, xpath=tool_xml.getpath(param))
+                    lint_ctx.error(f'Select [{param_name}] display="radio" is incompatible with multiple="true"', **node_props(param, tool_xml))
                 if optional:
-                    lint_ctx.error(f'Select [{param_name}] display="radio" is incompatible with optional="true"', line=param.sourceline, xpath=tool_xml.getpath(param))
+                    lint_ctx.error(f'Select [{param_name}] display="radio" is incompatible with optional="true"', **node_props(param, tool_xml))
         # TODO: Validate type, much more...
 
         # lint validators
@@ -196,40 +189,46 @@ def lint_inputs(tool_xml, lint_ctx):
             vtype = validator.attrib['type']
             if param_type in PARAMETER_VALIDATOR_TYPE_COMPATIBILITY:
                 if vtype not in PARAMETER_VALIDATOR_TYPE_COMPATIBILITY[param_type]:
-                    lint_ctx.error(f"Parameter [{param_name}]: validator with an incompatible type '{vtype}'", line=validator.sourceline, xpath=tool_xml.getpath(validator))
+                    lint_ctx.error(f"Parameter [{param_name}]: validator with an incompatible type '{vtype}'", **node_props(validator, tool_xml))
             for attrib in ATTRIB_VALIDATOR_COMPATIBILITY:
                 if attrib in validator.attrib and vtype not in ATTRIB_VALIDATOR_COMPATIBILITY[attrib]:
-                    lint_ctx.error(f"Parameter [{param_name}]: attribute '{attrib}' is incompatible with validator of type '{vtype}'", line=validator.sourceline, xpath=tool_xml.getpath(validator))
-            if vtype == "expression" and validator.text is None:
-                lint_ctx.error(f"Parameter [{param_name}]: expression validator without content", line=validator.sourceline, xpath=tool_xml.getpath(validator))
+                    lint_ctx.error(f"Parameter [{param_name}]: attribute '{attrib}' is incompatible with validator of type '{vtype}'", **node_props(validator, tool_xml))
+            if vtype == "expression":
+                if validator.text is None:
+                    lint_ctx.error(f"Parameter [{param_name}]: expression validators are expected to contain text", **node_props(validator, tool_xml))
+                else:
+                    try:
+                        re.compile(validator.text)
+                    except Exception as e:
+                        lint_ctx.error(f"Parameter [{param_name}]: '{validator.text}' is no valid regular expression: {str(e)}", **node_props(validator, tool_xml))
             if vtype not in ["expression", "regex"] and validator.text is not None:
-                lint_ctx.warn(f"Parameter [{param_name}]: '{vtype}' validators are not expected to contain text (found '{validator.text}')", line=validator.sourceline, xpath=tool_xml.getpath(validator))
+                lint_ctx.warn(f"Parameter [{param_name}]: '{vtype}' validators are not expected to contain text (found '{validator.text}')", **node_props(validator, tool_xml))
             if vtype in ["in_range", "length", "dataset_metadata_in_range"] and ("min" not in validator.attrib and "max" not in validator.attrib):
-                lint_ctx.error(f"Parameter [{param_name}]: '{vtype}' validators need to define the 'min' or 'max' attribute(s)", line=validator.sourceline, xpath=tool_xml.getpath(validator))
+                lint_ctx.error(f"Parameter [{param_name}]: '{vtype}' validators need to define the 'min' or 'max' attribute(s)", **node_props(validator, tool_xml))
             if vtype in ["metadata"] and ("check" not in validator.attrib and "skip" not in validator.attrib):
-                lint_ctx.error(f"Parameter [{param_name}]: '{vtype}' validators need to define the 'check' or 'skip' attribute(s)", line=validator.sourceline, xpath=tool_xml.getpath(validator))
+                lint_ctx.error(f"Parameter [{param_name}]: '{vtype}' validators need to define the 'check' or 'skip' attribute(s)", **node_props(validator, tool_xml))
             if vtype in ["value_in_data_table", "value_not_in_data_table", "dataset_metadata_in_data_table", "dataset_metadata_not_in_data_table"] and "table_name" not in validator.attrib:
-                lint_ctx.error(f"Parameter [{param_name}]: '{vtype}' validators need to define the 'table_name' attribute", line=validator.sourceline, xpath=tool_xml.getpath(validator))
+                lint_ctx.error(f"Parameter [{param_name}]: '{vtype}' validators need to define the 'table_name' attribute", **node_props(validator, tool_xml))
 
     conditional_selects = tool_xml.findall("./inputs//conditional")
     for conditional in conditional_selects:
         conditional_name = conditional.get('name')
         if not conditional_name:
-            lint_ctx.error("Conditional without a name", line=conditional.sourceline, xpath=tool_xml.getpath(conditional))
+            lint_ctx.error("Conditional without a name", **node_props(conditional, tool_xml))
         if conditional.get("value_from"):
             # Probably only the upload tool use this, no children elements
             continue
         first_param = conditional.findall("param")
         if len(first_param) != 1:
-            lint_ctx.error(f"Conditional [{conditional_name}] needs exactly one child <param> found {len(first_param)}", line=conditional.sourceline, xpath=tool_xml.getpath(conditional))
+            lint_ctx.error(f"Conditional [{conditional_name}] needs exactly one child <param> found {len(first_param)}", **node_props(conditional, tool_xml))
             continue
         first_param = first_param[0]
         first_param_type = first_param.get('type')
-        if first_param_type not in ['select', 'boolean']:
-            lint_ctx.error(f'Conditional [{conditional_name}] first param should have type="select" (or type="boolean" which is discouraged)', line=first_param.sourceline, xpath=tool_xml.getpath(first_param))
+        if first_param_type == 'boolean':
+            lint_ctx.warn(f'Conditional [{conditional_name}] first param of type="boolean" is discouraged, use a select', **node_props(first_param, tool_xml))
+        elif first_param_type != 'select':
+            lint_ctx.error(f'Conditional [{conditional_name}] first param should have type="select"', **node_props(first_param, tool_xml))
             continue
-        elif first_param_type == 'boolean':
-            lint_ctx.warn(f'Conditional [{conditional_name}] first param of type="boolean" is discouraged, use a select', line=first_param.sourceline, xpath=tool_xml.getpath(first_param))
 
         if first_param_type == 'select':
             select_options = _find_with_attribute(first_param, 'option', 'value')
@@ -242,38 +241,38 @@ def lint_inputs(tool_xml, lint_ctx):
 
         for incomp in ["optional", "multiple"]:
             if string_as_bool(first_param.get(incomp, False)):
-                lint_ctx.warn(f'Conditional [{conditional_name}] test parameter cannot be {incomp}="true"', line=first_param.sourceline, xpath=tool_xml.getpath(first_param))
+                lint_ctx.warn(f'Conditional [{conditional_name}] test parameter cannot be {incomp}="true"', **node_props(first_param, tool_xml))
 
         whens = conditional.findall('./when')
         if any('value' not in when.attrib for when in whens):
-            lint_ctx.error(f"Conditional [{conditional_name}] when without value", line=conditional.sourceline, xpath=tool_xml.getpath(conditional))
+            lint_ctx.error(f"Conditional [{conditional_name}] when without value", **node_props(conditional, tool_xml))
 
         when_ids = [w.get('value') for w in whens if w.get('value') is not None]
 
         for option_id in option_ids:
             if option_id not in when_ids:
-                lint_ctx.warn(f"Conditional [{conditional_name}] no <when /> block found for {first_param_type} option '{option_id}'", line=conditional.sourceline, xpath=tool_xml.getpath(conditional))
+                lint_ctx.warn(f"Conditional [{conditional_name}] no <when /> block found for {first_param_type} option '{option_id}'", **node_props(conditional, tool_xml))
 
         for when_id in when_ids:
             if when_id not in option_ids:
                 if first_param_type == 'select':
-                    lint_ctx.warn(f"Conditional [{conditional_name}] no <option /> found for when block '{when_id}'", line=conditional.sourceline, xpath=tool_xml.getpath(conditional))
+                    lint_ctx.warn(f"Conditional [{conditional_name}] no <option /> found for when block '{when_id}'", **node_props(conditional, tool_xml))
                 else:
-                    lint_ctx.warn(f"Conditional [{conditional_name}] no truevalue/falsevalue found for when block '{when_id}'", line=conditional.sourceline, xpath=tool_xml.getpath(conditional))
+                    lint_ctx.warn(f"Conditional [{conditional_name}] no truevalue/falsevalue found for when block '{when_id}'", **node_props(conditional, tool_xml))
 
     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", line=tool_line, xpath=tool_path)
+                lint_ctx.info(f"{datasource_tag} tag usually present in data sources", **node_props(tool_node, tool_xml))
 
     if num_inputs:
-        lint_ctx.info(f"Found {num_inputs} input parameters.", line=tool_line, xpath=tool_path)
+        lint_ctx.info(f"Found {num_inputs} input parameters.", **node_props(tool_node, tool_xml))
     else:
         if datasource:
-            lint_ctx.info("No input parameters, OK for data sources", line=tool_line, xpath=tool_path)
+            lint_ctx.info("No input parameters, OK for data sources", **node_props(tool_node, tool_xml))
         else:
-            lint_ctx.warn("Found no input parameters.", line=tool_line, xpath=tool_path)
+            lint_ctx.warn("Found no input parameters.", **node_props(tool_node, tool_xml))
 
 
 def lint_repeats(tool_xml, lint_ctx):
@@ -281,9 +280,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.", line=repeat.sourceline, xpath=tool_xml.getpath(repeat))
+            lint_ctx.error("Repeat does not specify name attribute.", **node_props(repeat, tool_xml))
         if "title" not in repeat.attrib:
-            lint_ctx.error("Repeat does not specify title attribute.", line=repeat.sourceline, xpath=tool_xml.getpath(repeat))
+            lint_ctx.error("Repeat does not specify title attribute.", **node_props(repeat, tool_xml))
 
 
 def _find_with_attribute(element, tag, attribute, test_value=None):

lib/galaxy/tool_util/linters/outputs.py

@@ -1,35 +1,30 @@
 """This module contains a linting functions for tool outputs."""
 from galaxy.util import string_as_bool
-from ._util import is_valid_cheetah_placeholder
+from ._util import is_valid_cheetah_placeholder, node_props
 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
+    # determine node to report for general problems with outputs
+    tool_node = tool_xml.find("./tool")
     outputs = tool_xml.findall("./outputs")
     if len(outputs) == 0:
-        lint_ctx.warn("Tool contains no outputs section, most tools should produce outputs.", line=tool_line, xpath=tool_path)
+        lint_ctx.warn("Tool contains no outputs section, most tools should produce outputs.", **node_props(tool_node, tool_xml))
         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]))
+        lint_ctx.warn("Tool contains multiple output sections, behavior undefined.", **node_props(outputs[1], tool_xml))
     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}]", line=output.sourceline, xpath=tool_xml.getpath(output))
+            lint_ctx.warn(f"Unknown element found in outputs [{output.tag}]", **node_props(output, tool_xml))
             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.", line=output.sourceline, xpath=tool_xml.getpath(output))
+            lint_ctx.warn("Tool output doesn't define a name - this is likely a problem.", **node_props(output, tool_xml))
             # 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))
+            lint_ctx.warn(f'Tool output name [{output.attrib["name"]}] is not a valid Cheetah placeholder.', **node_props(output, tool_xml))
 
         format_set = False
         if __check_format(tool_xml, output, lint_ctx):
@@ -40,7 +35,7 @@ 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.", line=output.sourceline, xpath=tool_xml.getpath(output))
+                lint_ctx.warn("Collection output with undefined 'type' found.", **node_props(output, tool_xml))
             if "structured_like" in output.attrib and "inherit_format" in output.attrib:
                 format_set = True
         for sub in output:
@@ -50,7 +45,7 @@ def lint_output(tool_xml, lint_ctx):
                 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.", line=output.sourceline, xpath=tool_xml.getpath(output))
+            lint_ctx.warn(f"Tool {output.tag} output {output.attrib.get('name', 'with missing name')} doesn't define an output format.", **node_props(output, tool_xml))
 
     # TODO: check for different labels in case of multiple outputs
     lint_ctx.info(f"{num_outputs} outputs found.", line=outputs[0].sourceline)
@@ -63,7 +58,7 @@ def __check_format(tool_xml, node, lint_ctx, allow_ext=False):
     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", line=node.sourceline, xpath=tool_xml.getpath(node))
+        lint_ctx.warn(f"Tool {node.tag} output '{node.attrib.get('name', 'with missing name')}' should use either format_source or format/ext", **node_props(node, tool_xml))
     if "format_source" in node.attrib:
         return True
     # if allowed (e.g. for discover_datasets), ext takes precedence over format
@@ -73,7 +68,7 @@ def __check_format(tool_xml, 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.", line=node.sourceline, xpath=tool_xml.getpath(node))
+        lint_ctx.warn(f"Using format='input' on {node.tag}, format_source attribute is less ambiguous and should be used instead.", **node_props(node, tool_xml))
     return fmt is not None
 
 

lib/galaxy/tool_util/linters/stdio.py

@@ -1,31 +1,27 @@
 """This module contains a linting functions for tool error detection."""
 import re
 
+from ._util import node_props
 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
+    # determine node to report for general problems with stdio
+    tool_node = tool_xml.find("./tool")
     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.", line=tool_line, xpath=tool_path)
+                lint_ctx.info("No stdio definition found, tool indicates error conditions with output written to stderr.", **node_props(tool_node, tool_xml))
             else:
-                lint_ctx.info("No stdio definition found, tool indicates error conditions with non-zero exit codes.", line=tool_line, xpath=tool_path)
+                lint_ctx.info("No stdio definition found, tool indicates error conditions with non-zero exit codes.", **node_props(tool_node, tool_xml))
         return
 
     if len(stdios) > 1:
-        lint_ctx.error("More than one stdio tag found, behavior undefined.", line=stdios[1].sourceline, xpath=tool_xml.getpath(stdios[1]))
+        lint_ctx.error("More than one stdio tag found, behavior undefined.", **node_props(stdios[1], tool_xml))
         return
 
     stdio = stdios[0]
@@ -37,22 +33,22 @@ def lint_stdio(tool_source, lint_ctx):
         else:
             message = "Unknown stdio child tag discovered [%s]. "
             message += "Valid options are exit_code and regex."
-            lint_ctx.warn(message % child.tag, line=child.sourceline, xpath=tool_xml.getpath(child))
+            lint_ctx.warn(message % child.tag, **node_props(child, tool_xml))
 
 
 def _lint_exit_code(tool_xml, child, lint_ctx):
     for key in child.attrib.keys():
         if key not in ["description", "level", "range"]:
-            lint_ctx.warn(f"Unknown attribute [{key}] encountered on exit_code tag.", line=child.sourceline, xpath=tool_xml.getpath(child))
+            lint_ctx.warn(f"Unknown attribute [{key}] encountered on exit_code tag.", **node_props(child, tool_xml))
 
 
 def _lint_regex(tool_xml, child, lint_ctx):
     for key in child.attrib.keys():
         if key not in ["description", "level", "match", "source"]:
-            lint_ctx.warn(f"Unknown attribute [{key}] encountered on regex tag.", line=child.sourceline, xpath=tool_xml.getpath(child))
+            lint_ctx.warn(f"Unknown attribute [{key}] encountered on regex tag.", **node_props(child, tool_xml))
     match = child.attrib.get("match")
     if match:
         try:
             re.compile(match)
         except Exception as e:
-            lint_ctx.error(f"Match '{match}' is no valid regular expression: {str(e)}", line=child.sourceline, xpath=tool_xml.getpath(child))
+            lint_ctx.error(f"Match '{match}' is no valid regular expression: {str(e)}", **node_props(child, tool_xml))

lib/galaxy/tool_util/linters/tests.py

@@ -1,31 +1,24 @@
 """This module contains a linting functions for tool tests."""
 from inspect import Parameter, signature
 
-from ._util import is_datasource
+from ._util import is_datasource, node_props
 from ..verify import asserts
 
 
 # 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
+    # determine node to report for general problems with tests
+    general_node = tool_xml.find("./tests")
+    if not general_node:
+        general_node = tool_xml.find("./tool")
+
     tests = tool_xml.findall("./tests/test")
     datasource = is_datasource(tool_xml)
     if not tests:
         if not datasource:
-            lint_ctx.warn("No tests found, most tools should define test cases.", line=tests_line, xpath=tests_path)
+            lint_ctx.warn("No tests found, most tools should define test cases.", **node_props(general_node, tool_xml))
         elif datasource:
-            lint_ctx.info("No tests found, that should be OK for data_sources.", line=tests_line, xpath=tests_path)
+            lint_ctx.info("No tests found, that should be OK for data_sources.", **node_props(general_node, tool_xml))
         return
 
     num_valid_tests = 0
@@ -51,7 +44,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.", line=param.sourceline, xpath=tool_xml.getpath(param))
+                lint_ctx.error(f"Test {test_idx}: Found test param tag without a name defined.", **node_props(param, tool_xml))
                 continue
             name = name.split("|")[-1]
             xpaths = [f"@name='{name}'",
@@ -69,7 +62,7 @@ def lint_tsts(tool_xml, lint_ctx):
                     found = True
                     break
             if not found:
-                lint_ctx.error(f"Test {test_idx}: Test param {name} not found in the inputs", line=param.sourceline, xpath=tool_xml.getpath(param))
+                lint_ctx.error(f"Test {test_idx}: Test param {name} not found in the inputs", **node_props(param, tool_xml))
 
         output_data_names, output_collection_names = _collect_output_names(tool_xml)
         found_output_test = False
@@ -81,25 +74,25 @@ def lint_tsts(tool_xml, lint_ctx):
             else:
                 valid_names = output_collection_names
             if not name:
-                lint_ctx.error(f"Test {test_idx}: Found {output.tag} tag without a name defined.", line=output.sourceline, xpath=tool_xml.getpath(output))
+                lint_ctx.error(f"Test {test_idx}: Found {output.tag} tag without a name defined.", **node_props(output, tool_xml))
             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))
+                    lint_ctx.error(f"Test {test_idx}: Found {output.tag} tag with unknown name [{name}], valid names [{valid_names}]", **node_props(output, tool_xml))
 
         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.")
+            lint_ctx.error(f"Test {test_idx}: Cannot specify outputs in a test expecting failure.", **node_props(test, tool_xml))
             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.", line=test.sourceline, xpath=tool_xml.getpath(test))
+            lint_ctx.warn(f"Test {test_idx}: No outputs or expectations defined for tests, this test is likely invalid.", **node_props(test, tool_xml))
         else:
             num_valid_tests += 1
 
     if num_valid_tests or datasource:
-        lint_ctx.valid(f"{num_valid_tests} test(s) found.", line=tests_line, xpath=tests_path)
+        lint_ctx.valid(f"{num_valid_tests} test(s) found.", **node_props(general_node, tool_xml))
     else:
-        lint_ctx.warn("No valid test(s) found.", line=tests_line, xpath=tests_path)
+        lint_ctx.warn("No valid test(s) found.", **node_props(general_node, tool_xml))
 
 
 def _check_asserts(test_idx, assertions, lint_ctx):

lib/galaxy/tool_util/linters/xml_order.py

@@ -3,6 +3,8 @@
 For more information on the IUC standard for XML block order see -
 https://github.com/galaxy-iuc/standards.
 """
+from ._util import node_props
+
 # https://github.com/galaxy-iuc/standards
 # https://github.com/galaxy-iuc/standards/pull/7/files
 TAG_ORDER = [
@@ -59,8 +61,8 @@ def lint_xml_order(tool_xml, lint_ctx):
             key = tag_ordering.index(tag)
             if last_key:
                 if last_key > key:
-                    lint_ctx.warn(f"Best practice violation [{tag}] elements should come before [{last_tag}]", line=elem.sourceline, xpath=tool_xml.getpath(elem))
+                    lint_ctx.warn(f"Best practice violation [{tag}] elements should come before [{last_tag}]", **node_props(elem, tool_xml))
             last_tag = tag
             last_key = key
         else:
-            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))
+            lint_ctx.info(f"Unknown tag [{tag}] encountered, this may result in a warning in the future.", **node_props(elem, tool_xml))

test/unit/tool_util/test_tool_linters.py

@@ -1,8 +1,10 @@
+import os
+import tempfile
 from inspect import getfullargspec
 
 import pytest
 
-from galaxy.tool_util.lint import LintContext
+from galaxy.tool_util.lint import lint_tool_source_with, LintContext
 from galaxy.tool_util.linters import (
     citations,
     command,
@@ -14,8 +16,9 @@ from galaxy.tool_util.linters import (
     tests,
     xml_order,
 )
+from galaxy.tool_util.loader_directory import load_tool_sources_from_path
 from galaxy.tool_util.parser.xml import XmlToolSource
-from galaxy.util import etree
+from galaxy.util.xml_macros import load_with_references
 
 # TODO tests tool xml for general linter
 # tests tool xml for citations linter
@@ -340,6 +343,7 @@ INPUTS_VALIDATOR_INCOMPATIBILITIES = """
             <validator type="in_range">TEXT</validator>
             <validator type="regex" filename="blah"/>
             <validator type="expression"/>
+            <validator type="expression">[</validator>
             <validator type="value_in_data_table"/>
         </param>
         <param name="another_param_name" type="data" format="bed">
@@ -797,7 +801,7 @@ TESTS = [
             and "Select parameter of a conditional [select] options have to be defined by 'option' children elements." in x.error_messages
             and 'Conditional [cond_wo_param] needs exactly one child <param> found 0' in x.error_messages
             and 'Conditional [cond_w_mult_param] needs exactly one child <param> found 2' in x.error_messages
-            and 'Conditional [cond_text] first param should have type="select" (or type="boolean" which is discouraged)' in x.error_messages
+            and 'Conditional [cond_text] first param should have type="select"' in x.error_messages
             and 'Conditional [cond_boolean] first param of type="boolean" is discouraged, use a select' in x.warn_messages
             and "Conditional [cond_boolean] no truevalue/falsevalue found for when block 'False'" in x.warn_messages
             and 'Conditional [cond_w_optional_select] test parameter cannot be optional="true"' in x.warn_messages
@@ -870,10 +874,11 @@ TESTS = [
             and "Parameter [param_name]: validator with an incompatible type 'in_range'" in x.error_messages
             and "Parameter [param_name]: 'in_range' validators need to define the 'min' or 'max' attribute(s)" in x.error_messages
             and "Parameter [param_name]: attribute 'filename' is incompatible with validator of type 'regex'" in x.error_messages
-            and "Parameter [param_name]: expression validator without content" in x.error_messages
+            and "Parameter [param_name]: expression validators are expected to contain text" in x.error_messages
+            and "Parameter [param_name]: '[' is no valid regular expression: unterminated character set at position 0" in x.error_messages
             and "Parameter [another_param_name]: 'metadata' validators need to define the 'check' or 'skip' attribute(s)" in x.error_messages
             and "Parameter [param_name]: 'value_in_data_table' validators need to define the 'table_name' attribute" in x.error_messages
-            and len(x.info_messages) == 1 and len(x.valid_messages) == 0 and len(x.warn_messages) == 1 and len(x.error_messages) == 6
+            and len(x.info_messages) == 1 and len(x.valid_messages) == 0 and len(x.warn_messages) == 1 and len(x.error_messages) == 7
     ),
     (
         INPUTS_VALIDATOR_CORRECT, inputs.lint_inputs,
@@ -1107,14 +1112,186 @@ TEST_IDS = [
 
 @pytest.mark.parametrize('tool_xml,lint_func,assert_func', TESTS, ids=TEST_IDS)
 def test_tool_xml(tool_xml, lint_func, assert_func):
+    """
+    test separate linting functions (lint_func) on tool_xml
+    checking assert_func on the resulting LinterContext
+
+    furthermore for all lint_func except for lint_general it is asserted
+    that each message has a complete lint context message (i.e. a line number)
+    """
     lint_ctx = LintContext('all')
     # the general linter gets XMLToolSource and all others
     # an ElementTree
     first_arg = getfullargspec(lint_func).args[0]
-    lint_target = etree.ElementTree(element=etree.fromstring(tool_xml))
+    with tempfile.TemporaryDirectory() as tmp:
+        tool_path = os.path.join(tmp, "tool.xml")
+        with open(tool_path, "w") as tmpf:
+            tmpf.write(tool_xml)
+        lint_target, _ = load_with_references(tool_path)
     if first_arg != "tool_xml":
         lint_target = XmlToolSource(lint_target)
     lint_ctx.lint(name="test_lint", lint_func=lint_func, lint_target=lint_target)
+    # TODO would be nice if lint_general would have full context as well
+    if lint_func != general.lint_general:
+        for message in lint_ctx.message_list:
+            assert message.line is not None, f"No context found for message: {message.message}"
+    assert assert_func(lint_ctx), (
+        f"Valid: {lint_ctx.valid_messages}\n"
+        f"Info: {lint_ctx.info_messages}\n"
+        f"Warnings: {lint_ctx.warn_messages}\n"
+        f"Errors: {lint_ctx.error_messages}"
+    )
+
+
+COMPLETE = """<tool>
+    <macros>
+        <import>macros.xml</import>
+        <xml name="test_macro">
+            <param name="select" type="select">
+                <option value="a">a</option>
+                <option value="a">a</option>
+            </param>
+        </xml>
+    </macros>
+    <inputs>
+        <expand macro="test_macro"/>
+        <param/>
+        <expand macro="test_macro2"/>
+    </inputs>
+</tool>
+"""
+
+COMPLETE_MACROS = """<macros>
+    <xml name="test_macro2">
+        <param name="No_type"/>
+    </xml>
+</macros>
+"""
+
+TOOL_AND_MACRO_XML_TESTS = [
+    (
+        COMPLETE, COMPLETE_MACROS,
+        [
+            ("Select parameter [select] has multiple options with the same value", "tool.xml", 5, "/tool/inputs/param[1]"),
+            ("Found param input with no name specified.", "tool.xml", 13, "/tool/inputs/param[2]"),
+            ("Param input [No_type] input with no type specified.", "macros.xml", 3, "/tool/inputs/param[3]")
+        ]
+    ),
+]
+TOOL_AND_MACRO_XML_IDS = [
+    "test tool xml and macros: context: line numbers, file paths, and xpaths"
+]
+
+
+@pytest.mark.parametrize('tool_xml,macros_xml,asserts', TOOL_AND_MACRO_XML_TESTS, ids=TOOL_AND_MACRO_XML_IDS)
+def test_tool_and_macro_xml(tool_xml, macros_xml, asserts):
+    """
+    test linters (all of them via lint_tool_source_with) on a tool and macro xml file
+    checking a list of asserts, where each assert is a 4-tuple:
+    - expected message
+    - filename suffix (tool.xml or macro.xml)
+    - line number (1 based)
+    - xpath
+    """
+    with tempfile.TemporaryDirectory() as tmp:
+        tool_path = os.path.join(tmp, "tool.xml")
+        macros_path = os.path.join(tmp, "macros.xml")
+        with open(tool_path, "w") as tmpf:
+            tmpf.write(tool_xml)
+        with open(macros_path, "w") as tmpf:
+            tmpf.write(macros_xml)
+        tool_xml, _ = load_with_references(tool_path)
+
+    tool_source = XmlToolSource(tool_xml)
+    lint_ctx = LintContext('all')
+    lint_tool_source_with(lint_ctx, tool_source)
+
+    for a in asserts:
+        message, fname, line, xpath = a
+        found = False
+        for lint_message in lint_ctx.message_list:
+            if lint_message.message != message:
+                continue
+            found = True
+            assert lint_message.line == line, f"Assumed line {line} found {lint_message.line} for: {message}"
+            assert lint_message.xpath == xpath, f"Assumed xpath {xpath} xpath {lint_message.xpath} for: {message}"
+            assert lint_message.fname.endswith(fname), f"Assumed file {fname} found {lint_message.fname} for: {message}"
+        assert found, f"Did not find {message}"
+
+
+# TODO COPIED from test/unit/app/tools/test_tool_deserialization.py
+CWL_TOOL = """
+cwlVersion: v1.0
+class: CommandLineTool
+baseCommand: echo
+inputs:
+  message:
+    type: string
+    inputBinding:
+      position: 1
+outputs: []
+"""
+
+YAML_TOOL = """
+class: GalaxyTool
+id: simple_constructs_y
+name: simple_constructs_y
+version: 1.0
+command:
+  >
+    echo "$booltest"  >> $out_file1;
+inputs:
+- name: booltest
+  type: boolean
+  truevalue: booltrue
+  falsevalue: boolfalse
+  checked: false
+outputs:
+  out_file1:
+    format: txt
+"""
+
+YML_CWL_TOOLS = [
+    (
+        CWL_TOOL, "cwl",
+        lambda x:
+            "Tool defines a version [0.0.1]." in x.valid_messages
+            and "Tool defines a name [tool]." in x.valid_messages
+            and "Tool defines an id [tool]." in x.valid_messages
+            and "Tool specifies profile version [16.04]." in x.valid_messages
+            and "CWL appears to be valid." in x.info_messages
+            and "Description of tool is empty or absent." in x.warn_messages
+            and "Tool does not specify a DockerPull source." in x.warn_messages
+            and "Modern CWL version [v1.0]." in x.info_messages
+            and len(x.info_messages) == 2 and len(x.valid_messages) == 4 and len(x.warn_messages) == 2 and len(x.error_messages) == 0
+    ),
+    (
+        YAML_TOOL, "yml",
+        lambda x:
+            "Tool defines a version [1.0]." in x.valid_messages
+            and "Tool defines a name [simple_constructs_y]." in x.valid_messages
+            and "Tool defines an id [simple_constructs_y]." in x.valid_messages
+            and "Tool specifies profile version [16.04]." in x.valid_messages
+            and len(x.info_messages) == 0 and len(x.valid_messages) == 4 and len(x.warn_messages) == 0 and len(x.error_messages) == 0
+    )
+]
+YML_CWL_TOOLS_IDS = [
+    "cwl tool",
+    "yaml tool"
+]
+
+
+@pytest.mark.parametrize('tool_raw,ext,assert_func', YML_CWL_TOOLS, ids=YML_CWL_TOOLS_IDS)
+def test_tool_yml_cwl(tool_raw, ext, assert_func):
+    with tempfile.TemporaryDirectory() as tmp:
+        tool_path = os.path.join(tmp, f"tool.{ext}")
+        with open(tool_path, "w") as tmpf:
+            tmpf.write(tool_raw)
+        tool_sources = load_tool_sources_from_path(tmp)
+        assert len(tool_sources) == 1, "Expected 1 tool source"
+        tool_source = tool_sources[0][1]
+        lint_ctx = LintContext('all')
+        lint_tool_source_with(lint_ctx, tool_source)
     assert assert_func(lint_ctx), (
         f"Valid: {lint_ctx.valid_messages}\n"
         f"Info: {lint_ctx.info_messages}\n"