From 082d4656c8c72a192cf2cfdd78ee7161f90052ef Mon Sep 17 00:00:00 2001
From: Matthias Bernt <m.bernt@ufz.de>
Date: Mon, 11 Mar 2024 12:57:19 +0100
Subject: [PATCH] tool linter: check for lead nodes with unstripped text
 content

---
 lib/galaxy/tool_util/linters/general.py  | 15 +++++++
 test/unit/tool_util/test_tool_linters.py | 53 +++++++++++++++---------
 2 files changed, 49 insertions(+), 19 deletions(-)

diff --git a/lib/galaxy/tool_util/linters/general.py b/lib/galaxy/tool_util/linters/general.py
index f395e3350904..a19cb6e175e1 100644
--- a/lib/galaxy/tool_util/linters/general.py
+++ b/lib/galaxy/tool_util/linters/general.py
@@ -227,3 +227,18 @@ 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
+                )
diff --git a/test/unit/tool_util/test_tool_linters.py b/test/unit/tool_util/test_tool_linters.py
index e553d2c16884..dd1fa868e1b9 100644
--- a/test/unit/tool_util/test_tool_linters.py
+++ b/test/unit/tool_util/test_tool_linters.py
@@ -118,6 +118,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 +903,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 +936,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 +1084,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 +2090,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 [