From ec04666cc1ed504f6a7caf3b831777bda87d0c17 Mon Sep 17 00:00:00 2001
From: Matthias Bernt <m.bernt@ufz.de>
Date: Mon, 12 Sep 2022 15:14:34 +0200
Subject: [PATCH] linter: check for inputs with duplicated names

and outputs
---
 lib/galaxy/tool_util/linters/inputs.py   | 30 ++++++++++++++++-
 test/unit/tool_util/test_tool_linters.py | 41 ++++++++++++++++++++++++
 2 files changed, 70 insertions(+), 1 deletion(-)

diff --git a/lib/galaxy/tool_util/linters/inputs.py b/lib/galaxy/tool_util/linters/inputs.py
index 6a7e602f7c0..25414233834 100644
--- a/lib/galaxy/tool_util/linters/inputs.py
+++ b/lib/galaxy/tool_util/linters/inputs.py
@@ -117,6 +117,7 @@ PARAM_TYPE_CHILD_COMBINATIONS = [
 def lint_inputs(tool_xml, lint_ctx):
     """Lint parameters in a tool's inputs block."""
     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")
@@ -141,7 +142,24 @@ def lint_inputs(tool_xml, lint_ctx):
         elif not is_valid_cheetah_placeholder(param_name):
             lint_ctx.warn(f"Param input [{param_name}] is not a valid Cheetah placeholder.", node=param)
 
-        # TODO lint for params with duplicated name (in inputs & outputs)
+        # 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 = ".".join(reversed(path))
+        if path in input_names:
+            lint_ctx.error(f"Tool defines multiple parameters with the same name: '{path}'", node=param)
+        input_names.add(path)
 
         if "type" not in param_attrib:
             lint_ctx.error(f"Param input [{param_name}] input with no type specified.", node=param)
@@ -440,6 +458,16 @@ def lint_inputs(tool_xml, lint_ctx):
         else:
             lint_ctx.warn("Found no input parameters.", node=tool_node)
 
+    # check if there is an output with the same name as an input
+    outputs = tool_xml.find("./outputs")
+    if 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")}\'',
+                    node=output,
+                )
+
 
 def lint_repeats(tool_xml, lint_ctx):
     """Lint repeat blocks in tool inputs."""
diff --git a/test/unit/tool_util/test_tool_linters.py b/test/unit/tool_util/test_tool_linters.py
index 7bb4f7becbb..8219da313b8 100644
--- a/test/unit/tool_util/test_tool_linters.py
+++ b/test/unit/tool_util/test_tool_linters.py
@@ -418,6 +418,34 @@ INPUTS_TYPE_CHILD_COMBINATIONS = """
 </tool>
 """
 
+INPUTS_DUPLICATE_NAMES = """
+<tool>
+    <inputs>
+        <param name="dup" type="text"/>
+        <param name="dup" type="text"/>
+        <param name="dup_in_section" type="text"/>
+        <section name="sec">
+            <param name="dup_in_section" type="text"/>
+        </section>
+        <conditional name="cond">
+            <param name="dup_in_cond" type="select">
+                <option value="a">a</option>
+                <option value="b">b</option>
+            </param>
+            <when value="a">
+                <param name="dup_in_cond" type="text"/>
+            </when>
+            <when value="b">
+                <param name="dup_in_cond" type="text"/>
+            </when>
+        </conditional>
+        <param name="dup_in_output" type="text"/>
+    </inputs>
+    <outputs>
+        <data name="dup_in_output"/>
+    </outputs>
+</tool>
+"""
 
 # test tool xml for outputs linter
 OUTPUTS_MISSING = """
@@ -1211,6 +1239,19 @@ def test_inputs_type_child_combinations(lint_ctx):
     assert len(lint_ctx.error_messages) == 3
 
 
+def test_inputs_duplicate_names(lint_ctx):
+    tool_source = get_xml_tool_source(INPUTS_DUPLICATE_NAMES)
+    run_lint(lint_ctx, inputs.lint_inputs, tool_source)
+    assert len(lint_ctx.info_messages) == 1
+    assert not lint_ctx.valid_messages
+    assert not lint_ctx.warn_messages
+    assert "Tool defines multiple parameters with the same name: 'dup'" in lint_ctx.error_messages
+    assert (
+        "Tool defines an output with a name equal to the name of an input: 'dup_in_output'" in lint_ctx.error_messages
+    )
+    assert len(lint_ctx.error_messages) == 2
+
+
 def test_inputs_repeats(lint_ctx):
     tool_source = get_xml_tool_source(REPEATS)
     run_lint(lint_ctx, inputs.lint_repeats, tool_source)