Tool linter: check for leaf nodes with unstripped text content

lib/galaxy/tool_util/linters/general.py

@@ -227,3 +227,20 @@ def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"):
                 lint_ctx.warn(
                     "Expressions in resource requirement not supported yet", linter=cls.name(), node=tool_node
                 )
+
+
+class TextSpaces(Linter):
+    @classmethod
+    def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"):
+        _, tool_node = _tool_xml_and_root(tool_source)
+        if not tool_node:
+            return
+        for node in tool_node.iter():
+            if len(node) > 0:
+                continue
+            if node.text and node.text != node.text.strip():
+                lint_ctx.warn(
+                    f"XML node '{node.tag}' has text with leading or trailing spaces ('{node.text}'!='{node.text.strip()}').",
+                    linter=cls.name(),
+                    node=node,
+                )

test/unit/tool_util/test_tool_linters.py

@@ -6,7 +6,6 @@
 from galaxy.tool_util.lint import (
     lint_tool_source_with,
     lint_tool_source_with_modules,
-    lint_xml_with,
     LintContext,
     Linter,
     XMLLintMessageLine,
@@ -26,10 +25,7 @@
 )
 from galaxy.tool_util.loader_directory import load_tool_sources_from_path
 from galaxy.tool_util.parser.xml import XmlToolSource
-from galaxy.util import (
-    ElementTree,
-    parse_xml,
-)
+from galaxy.util import ElementTree
 from galaxy.util.xml_macros import load_with_references
 
 # TODO tests tool xml for general linter
@@ -118,6 +114,16 @@
 </tool>
 """
 
+GENERAL_TEXT_SPACES = """
+<tool name="valid name" id="valid_id" version="1.0+galaxy1" profile="21.09">
+    <xrefs>
+        <xref type="bio.tools">
+            bwa
+        </xref>
+    </xrefs>
+</tool>
+"""
+
 # test tool xml for help linter
 HELP_MULTIPLE = """
 <tool id="id" name="name">
@@ -893,10 +899,12 @@
 TOOL_WITH_COMMENTS = """
 <tool>
     <stdio>
-    <!-- This is a comment -->
+        <!-- This is a comment -->
+        <regex match="low space" source="both" level="warning" description="Low space on device" />
     </stdio>
     <outputs>
-    <!-- This is a comment -->
+        <!-- This is a comment -->
+        <data format="pdf" name="out_file1" />
     </outputs>
 </tool>
 """
@@ -924,15 +932,6 @@ def get_xml_tool_source(xml_string: str) -> XmlToolSource:
     return XmlToolSource(get_xml_tree(xml_string))
 
 
-def get_tool_xml_exact(xml_string: str):
-    """Returns the tool XML as it is, without stripping comments or anything else."""
-    with tempfile.NamedTemporaryFile(mode="w", suffix="tool.xml") as tmp:
-        tmp.write(xml_string)
-        tmp.flush()
-        tool_path = tmp.name
-        return parse_xml(tool_path, strip_whitespace=False, remove_comments=False)
-
-
 def run_lint_module(lint_ctx, lint_module, lint_target):
     lint_tool_source_with_modules(lint_ctx, lint_target, list(set([lint_module, xsd])))
 
@@ -1081,6 +1080,25 @@ def test_general_valid_new_profile_fmt(lint_ctx):
     assert not lint_ctx.error_messages
 
 
+def test_general_text_spaces(lint_ctx):
+    tool_source = get_xml_tool_source(GENERAL_TEXT_SPACES)
+    run_lint_module(lint_ctx, general, tool_source)
+    assert "XML node 'xref' has text with leading or trailing spaces ('\n            bwa\n        '!='bwa')" in lint_ctx.warn_messages
+    assert not lint_ctx.info_messages
+    assert len(lint_ctx.valid_messages) == 1
+    assert len(lint_ctx.warn_messages) == 1
+    assert not lint_ctx.error_messages
+
+
+def test_general_text_spaces_comments(lint_ctx):
+    tool_source = get_xml_tool_source(TOOL_WITH_COMMENTS)
+    run_lint_module(lint_ctx, general, tool_source)
+    assert not lint_ctx.info_messages
+    assert len(lint_ctx.valid_messages) == 4
+    assert not lint_ctx.warn_messages
+    assert not lint_ctx.error_messages
+
+
 def test_help_multiple(lint_ctx):
     tool_source = get_xml_tool_source(HELP_MULTIPLE)
     run_lint_module(lint_ctx, help, tool_source)
@@ -2068,17 +2086,10 @@ def test_linting_cwl_tool(lint_ctx):
     assert not lint_ctx.error_messages
 
 
-def test_xml_comments_are_ignored(lint_ctx: LintContext):
-    tool_xml = get_tool_xml_exact(TOOL_WITH_COMMENTS)
-    lint_xml_with(lint_ctx, tool_xml)
-    for lint_message in lint_ctx.message_list:
-        assert "Comment" not in lint_message.message
-
-
 def test_list_linters():
     linter_names = Linter.list_listers()
     # make sure to add/remove a test for new/removed linters if this number changes
-    assert len(linter_names) == 130
+    assert len(linter_names) == 131
     assert "Linter" not in linter_names
     # make sure that linters from all modules are available
     for prefix in [