From ec60b94e921ac672c152a94ffc275d4ee7f62a11 Mon Sep 17 00:00:00 2001
From: Matthias Bernt <m.bernt@ufz.de>
Date: Mon, 18 Oct 2021 12:40:52 +0200
Subject: [PATCH 1/6] linter: regex validator does not require expression

---
 lib/galaxy/tool_util/linters/inputs.py | 2 --
 1 file changed, 2 deletions(-)

diff --git a/lib/galaxy/tool_util/linters/inputs.py b/lib/galaxy/tool_util/linters/inputs.py
index 437efe5e76f..78de207632b 100644
--- a/lib/galaxy/tool_util/linters/inputs.py
+++ b/lib/galaxy/tool_util/linters/inputs.py
@@ -163,8 +163,6 @@ def lint_inputs(tool_xml, lint_ctx):
                 lint_ctx.error(f"Parameter [{param_name}]: expression validator without content")
             if vtype != "expression" and validator.text is not None:
                 lint_ctx.warn(f"Parameter [{param_name}]: '{vtype}' validators are not expected to contain text (found '{validator.text}')")
-            if vtype == "regex" and "expression" not in validator.attrib:
-                lint_ctx.error(f"Parameter [{param_name}]: '{vtype}' validators need to define an 'expression' attribute")
             if vtype in ["in_range", "length", "dataset_metadata_in_range"] and ("min" not in validator.attrib or "max" not in validator.attrib):
                 lint_ctx.error(f"Parameter [{param_name}]: '{vtype}' validators need to define the 'min' or 'max' attribute(s)")
             if vtype in ["metadata"] and ("check" not in validator.attrib or "skip" not in validator.attrib):

From f5ca471c8e8c6f3a843fdbff61e829691641138f Mon Sep 17 00:00:00 2001
From: Matthias Bernt <m.bernt@ufz.de>
Date: Mon, 18 Oct 2021 13:23:34 +0200
Subject: [PATCH 2/6] more fixes to validator linters

and test of most correct combinations
---
 lib/galaxy/tool_util/linters/inputs.py   | 14 +++---
 test/unit/tool_util/test_tool_linters.py | 57 +++++++++++++++++++++++-
 2 files changed, 62 insertions(+), 9 deletions(-)

diff --git a/lib/galaxy/tool_util/linters/inputs.py b/lib/galaxy/tool_util/linters/inputs.py
index 78de207632b..af378c8d780 100644
--- a/lib/galaxy/tool_util/linters/inputs.py
+++ b/lib/galaxy/tool_util/linters/inputs.py
@@ -19,16 +19,16 @@ FILTER_TYPES = [
 
 ATTRIB_VALIDATOR_COMPATIBILITY = {
     "check": ["metadata"],
-    "expression": ["regex", "substitute_value_in_message"],
+    "expression": ["substitute_value_in_message"],
     "table_name": ["dataset_metadata_in_data_table", "dataset_metadata_not_in_data_table", "value_in_data_table", "value_not_in_data_table"],
     "filename": ["dataset_metadata_in_file"],
     "metadata_name": ["dataset_metadata_in_data_table", "dataset_metadata_not_in_data_table", "dataset_metadata_in_file"],
-    "metadata_column": ["dataset_metadata_in_data_table", "dataset_metadata_not_in_data_table", "value_in_data_table", "value_not_in_data_table", "dataset_metadata_in_file options"],
+    "metadata_column": ["dataset_metadata_in_data_table", "dataset_metadata_not_in_data_table", "value_in_data_table", "value_not_in_data_table", "dataset_metadata_in_file"],
     "line_startswith": ["dataset_metadata_in_file"],
-    "min": ["in_range", "length"],
-    "max": ["in_range", "length"],
-    "exclude_min": ["in_range"],
-    "exclude_max": ["in_range"],
+    "min": ["in_range", "length", "dataset_metadata_in_range"],
+    "max": ["in_range", "length", "dataset_metadata_in_range"],
+    "exclude_min": ["in_range", "dataset_metadata_in_range"],
+    "exclude_max": ["in_range", "dataset_metadata_in_range"],
     "split": ["dataset_metadata_in_file"],
     "skip": ["metadata"]
 }
@@ -161,7 +161,7 @@ def lint_inputs(tool_xml, lint_ctx):
                     lint_ctx.error(f"Parameter [{param_name}]: attribute '{attrib}' is incompatible with validator of type '{vtype}'")
             if vtype == "expression" and validator.text is None:
                 lint_ctx.error(f"Parameter [{param_name}]: expression validator without content")
-            if vtype != "expression" and validator.text is not None:
+            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}')")
             if vtype in ["in_range", "length", "dataset_metadata_in_range"] and ("min" not in validator.attrib or "max" not in validator.attrib):
                 lint_ctx.error(f"Parameter [{param_name}]: '{vtype}' validators need to define the 'min' or 'max' attribute(s)")
diff --git a/test/unit/tool_util/test_tool_linters.py b/test/unit/tool_util/test_tool_linters.py
index 0dab32dba2e..c91547aa84d 100644
--- a/test/unit/tool_util/test_tool_linters.py
+++ b/test/unit/tool_util/test_tool_linters.py
@@ -41,6 +41,7 @@ RADIO_SELECT_INCOMPATIBILITIES = """
 <tool name="BWA Mapper" id="bwa" version="1.0.1" is_multi_byte="true" display_interface="true" require_login="true" hidden="true">
     <description>The BWA Mapper</description>
     <version_command interpreter="python">bwa.py --version</version_command>
+    <command>$radio_select$radio_checkboxes</command>
     <inputs>
         <param name="radio_select" type="select" display="radio" optional="true" multiple="true">
             <option value="1">1</option>
@@ -58,6 +59,7 @@ SELECT_DUPLICATED_OPTIONS = """
 <tool name="BWA Mapper" id="bwa" version="1.0.1" is_multi_byte="true" display_interface="true" require_login="true" hidden="true">
     <description>The BWA Mapper</description>
     <version_command interpreter="python">bwa.py --version</version_command>
+    <command>$select</command>
     <inputs>
         <param name="select" type="select" optional="true" multiple="true">
             <option value="v">x</option>
@@ -71,6 +73,7 @@ SELECT_DEPRECATIONS = """
 <tool name="BWA Mapper" id="bwa" version="1.0.1" is_multi_byte="true" display_interface="true" require_login="true" hidden="true">
     <description>The BWA Mapper</description>
     <version_command interpreter="python">bwa.py --version</version_command>
+    <command>$select_do$select_ff$select_fp</command>
     <inputs>
         <param name="select_do" type="select" dynamic_options="blah()"/>
         <param name="select_ff" type="select">
@@ -87,6 +90,7 @@ SELECT_OPTION_DEFINITIONS = """
 <tool name="BWA Mapper" id="bwa" version="1.0.1" is_multi_byte="true" display_interface="true" require_login="true" hidden="true">
     <description>The BWA Mapper</description>
     <version_command interpreter="python">bwa.py --version</version_command>
+    <command>$select_noopt$select_noopts$select_fd_op$select_fd_fdt</command>
     <inputs>
         <param name="select_noopt" type="select"/>
         <param name="select_noopts" type="select">
@@ -134,6 +138,51 @@ VALIDATOR_INCOMPATIBILITIES = """
 </tool>
 """
 
+VALIDATOR_CORRECT = """
+<tool name="BWA Mapper" id="bwa" version="1.0.1" is_multi_byte="true" display_interface="true" require_login="true" hidden="true">
+    <description>The BWA Mapper</description>
+    <version_command interpreter="python">bwa.py --version</version_command>
+    <inputs>
+        <param name="data_param" type="data" format="data">
+            <validator type="metadata" check="md1,md2" skip="md3,md4" message="cutom validation message" negate="true"/>
+            <validator type="unspecified_build" message="cutom validation message" negate="true"/>
+            <validator type="dataset_ok_validator" message="cutom validation message" negate="true"/>
+            <validator type="dataset_metadata_in_range" min="0" max="100" exclude_min="true" exclude_max="true" message="cutom validation message" negate="true"/>
+            <validator type="dataset_metadata_in_file" filename="file.tsv" metadata_column="3" split=","  message="cutom validation message" negate="true"/>
+            <validator type="dataset_metadata_in_data_table" table_name="datatable_name" metadata_column="3" message="cutom validation message" negate="true"/>
+        </param>
+        <param name="collection_param" type="collection">
+            <validator type="metadata" check="md1,md2" skip="md3,md4" message="cutom validation message"/>
+            <validator type="unspecified_build" message="cutom validation message"/>
+            <validator type="dataset_ok_validator" message="cutom validation message"/>
+            <validator type="dataset_metadata_in_range" min="0" max="100" exclude_min="true" exclude_max="true" message="cutom validation message"/>
+            <validator type="dataset_metadata_in_file" filename="file.tsv" metadata_column="3" split=","  message="cutom validation message"/>
+            <validator type="dataset_metadata_in_data_table" table_name="datatable_name" metadata_column="3" message="cutom validation message"/>
+        </param>
+        <param name="text_param" type="text">
+            <validator type="regex">reg.xp</validator>
+            <validator type="length" min="0" max="100" message="cutom validation message"/>
+            <validator type="empty_field" message="cutom validation message"/>
+            <validator type="value_in_data_table" table_name="datatable_name" metadata_column="3" message="cutom validation message"/>
+            <validator type="expression" message="cutom validation message">somepythonexpression</validator>
+        </param>
+        <param name="select_param" type="select">
+            <options from_data_table="bowtie2_indexes"/>
+            <validator type="no_options" negate="true"/>
+            <validator type="regex" negate="true">reg.xp</validator>
+            <validator type="length" min="0" max="100" message="cutom validation message" negate="true"/>
+            <validator type="empty_field" message="cutom validation message" negate="true"/>
+            <validator type="value_in_data_table" table_name="datatable_name" metadata_column="3" message="cutom validation message" negate="true"/>
+            <validator type="expression" message="cutom validation message" negate="true">somepythonexpression</validator>
+        </param>
+        <param name="int_param" type="integer">
+            <validator type="in_range" min="0" max="100" exclude_min="true" exclude_max="true" negate="true"/>
+            <validator type="expression" message="cutom validation message">somepythonexpression</validator>
+        </param>
+    </inputs>
+</tool>
+"""
+
 # check that linter accepts format source for collection elements as means to specify format
 # and that the linter warns if format and format_source are used
 OUTPUTS_COLLECTION_FORMAT_SOURCE = """
@@ -226,8 +275,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]: 'regex' validators need to define an 'expression' attribute" in x.error_messages
-            and len(x.warn_messages) == 1 and len(x.error_messages) == 4
+            and len(x.warn_messages) == 1 and len(x.error_messages) == 3
+    ),
+    (
+        VALIDATOR_CORRECT, inputs.lint_inputs,
+        lambda x: len(x.warn_messages) == 0 and len(x.error_messages) == 0
     ),
     (
         OUTPUTS_COLLECTION_FORMAT_SOURCE, outputs.lint_output,
@@ -251,6 +303,7 @@ TEST_IDS = [
     'select option definitions',
     'hazardous whitespace',
     'validator imcompatibilities',
+    'validator all correct',
     'outputs collection static elements with format_source',
     'outputs discover datatsets with tool provided metadata'
 ]

From 2849c813734e886df24f916cf098e43e5cb423eb Mon Sep 17 00:00:00 2001
From: Matthias Bernt <m.bernt@ufz.de>
Date: Mon, 18 Oct 2021 13:28:04 +0200
Subject: [PATCH 3/6] fix

---
 test/unit/tool_util/test_tool_linters.py | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/test/unit/tool_util/test_tool_linters.py b/test/unit/tool_util/test_tool_linters.py
index c91547aa84d..1c760c53ba9 100644
--- a/test/unit/tool_util/test_tool_linters.py
+++ b/test/unit/tool_util/test_tool_linters.py
@@ -41,7 +41,6 @@ RADIO_SELECT_INCOMPATIBILITIES = """
 <tool name="BWA Mapper" id="bwa" version="1.0.1" is_multi_byte="true" display_interface="true" require_login="true" hidden="true">
     <description>The BWA Mapper</description>
     <version_command interpreter="python">bwa.py --version</version_command>
-    <command>$radio_select$radio_checkboxes</command>
     <inputs>
         <param name="radio_select" type="select" display="radio" optional="true" multiple="true">
             <option value="1">1</option>
@@ -59,7 +58,6 @@ SELECT_DUPLICATED_OPTIONS = """
 <tool name="BWA Mapper" id="bwa" version="1.0.1" is_multi_byte="true" display_interface="true" require_login="true" hidden="true">
     <description>The BWA Mapper</description>
     <version_command interpreter="python">bwa.py --version</version_command>
-    <command>$select</command>
     <inputs>
         <param name="select" type="select" optional="true" multiple="true">
             <option value="v">x</option>
@@ -73,7 +71,6 @@ SELECT_DEPRECATIONS = """
 <tool name="BWA Mapper" id="bwa" version="1.0.1" is_multi_byte="true" display_interface="true" require_login="true" hidden="true">
     <description>The BWA Mapper</description>
     <version_command interpreter="python">bwa.py --version</version_command>
-    <command>$select_do$select_ff$select_fp</command>
     <inputs>
         <param name="select_do" type="select" dynamic_options="blah()"/>
         <param name="select_ff" type="select">
@@ -90,7 +87,6 @@ SELECT_OPTION_DEFINITIONS = """
 <tool name="BWA Mapper" id="bwa" version="1.0.1" is_multi_byte="true" display_interface="true" require_login="true" hidden="true">
     <description>The BWA Mapper</description>
     <version_command interpreter="python">bwa.py --version</version_command>
-    <command>$select_noopt$select_noopts$select_fd_op$select_fd_fdt</command>
     <inputs>
         <param name="select_noopt" type="select"/>
         <param name="select_noopts" type="select">

From fa4655ac538fc9a99d7ad2b972016a7ba967fb24 Mon Sep 17 00:00:00 2001
From: Matthias Bernt <m.bernt@ufz.de>
Date: Tue, 19 Oct 2021 13:03:54 +0200
Subject: [PATCH 4/6] fix for options without text

---
 lib/galaxy/tool_util/linters/inputs.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/galaxy/tool_util/linters/inputs.py b/lib/galaxy/tool_util/linters/inputs.py
index af378c8d780..bf780c94b43 100644
--- a/lib/galaxy/tool_util/linters/inputs.py
+++ b/lib/galaxy/tool_util/linters/inputs.py
@@ -132,7 +132,7 @@ def lint_inputs(tool_xml, lint_ctx):
             # 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")
-            if len(set([option.text.strip() for option in select_options])) != len(select_options):
+            if len(set([option.text.strip() for option in select_options if option.text is not None])) != len(select_options):
                 lint_ctx.error(f"Select parameter [{param_name}] has multiple options with the same text content")
             if len(set([option.attrib.get("value") for option in select_options])) != len(select_options):
                 lint_ctx.error(f"Select parameter [{param_name}] has multiple options with the same value")

From 990e60fc99ddc0c6529c314c7fd97bfcf6e991f4 Mon Sep 17 00:00:00 2001
From: Matthias Bernt <m.bernt@ufz.de>
Date: Tue, 19 Oct 2021 13:14:30 +0200
Subject: [PATCH 5/6] fix linting for metadata validators

---
 lib/galaxy/tool_util/linters/inputs.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/galaxy/tool_util/linters/inputs.py b/lib/galaxy/tool_util/linters/inputs.py
index bf780c94b43..33f7f0ed6bd 100644
--- a/lib/galaxy/tool_util/linters/inputs.py
+++ b/lib/galaxy/tool_util/linters/inputs.py
@@ -165,8 +165,8 @@ def lint_inputs(tool_xml, lint_ctx):
                 lint_ctx.warn(f"Parameter [{param_name}]: '{vtype}' validators are not expected to contain text (found '{validator.text}')")
             if vtype in ["in_range", "length", "dataset_metadata_in_range"] and ("min" not in validator.attrib or "max" not in validator.attrib):
                 lint_ctx.error(f"Parameter [{param_name}]: '{vtype}' validators need to define the 'min' or 'max' attribute(s)")
-            if vtype in ["metadata"] and ("check" not in validator.attrib or "skip" not in validator.attrib):
-                lint_ctx.error(f"Parameter [{param_name}]: '{vtype}' validators need to define the 'check' or 'skip' attribute(s)")
+            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) {validator.attrib}")
             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")
 

From 2c28cc85d6cd954151de745aa32a8b9d2adf86ab Mon Sep 17 00:00:00 2001
From: Matthias Bernt <m.bernt@ufz.de>
Date: Tue, 19 Oct 2021 14:18:32 +0200
Subject: [PATCH 6/6] fix length validator message

---
 lib/galaxy/tool_util/linters/inputs.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/galaxy/tool_util/linters/inputs.py b/lib/galaxy/tool_util/linters/inputs.py
index 33f7f0ed6bd..ea887af358b 100644
--- a/lib/galaxy/tool_util/linters/inputs.py
+++ b/lib/galaxy/tool_util/linters/inputs.py
@@ -163,7 +163,7 @@ def lint_inputs(tool_xml, lint_ctx):
                 lint_ctx.error(f"Parameter [{param_name}]: expression validator without content")
             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}')")
-            if vtype in ["in_range", "length", "dataset_metadata_in_range"] and ("min" not in validator.attrib or "max" not in validator.attrib):
+            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)")
             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) {validator.attrib}")