linting: restore possibiity to skip old linter function names

until 23.2 we had linter functions lint_xyz in linter modules xyz.
with 24.0 we splited the functions linter classes.

lint contexts allow to skip linters by name (xyz for the old functions
now the class name), i.e. we can not skip by the old names anymore.
this may break planemo command lines xref
galaxyproject/planemo#1441 (comment).

here I suggest to add a check for the module name against the names
listed in the skip list. this restores the possibility to skip by old
linter names and adds the possibility to skip whole linter modules
(which might be handy anyway).

tried to add some tests that hopefully help to maintain this
functionality.

lib/galaxy/tool_util/lint.py

@@ -212,11 +212,19 @@ def found_errors(self) -> bool:
     def found_warns(self) -> bool:
         return len(self.warn_messages) > 0
 
-    def lint(self, name: str, lint_func: Callable[[LintTargetType, "LintContext"], None], lint_target: LintTargetType):
+    def lint(
+        self,
+        name: str,
+        lint_func: Callable[[LintTargetType, "LintContext"], None],
+        lint_target: LintTargetType,
+        module_name: Optional[str] = None,
+    ):
         if name.startswith("lint_"):
             name = name[len("lint_") :]
         if name in self.skip_types:
             return
+        if module_name and module_name in self.skip_types:
+            return
 
         if self.level < LintLevel.SILENT:
             # this is a relict from the past where the lint context
@@ -355,6 +363,7 @@ def lint_tool_source_with_modules(lint_context: LintContext, tool_source, linter
     tool_type = tool_source.parse_tool_type() or "default"
 
     for module in linter_modules:
+        module_name = module.__name__.split(".")[-1]
         lint_tool_types = getattr(module, "lint_tool_types", ["default", "manage_data"])
         if not ("*" in lint_tool_types or tool_type in lint_tool_types):
             continue
@@ -374,7 +383,7 @@ def lint_tool_source_with_modules(lint_context: LintContext, tool_source, linter
                 else:
                     lint_context.lint(name, value, tool_source)
             elif inspect.isclass(value) and issubclass(value, Linter) and not inspect.isabstract(value):
-                lint_context.lint(name, value.lint, tool_source)
+                lint_context.lint(name, value.lint, tool_source, module_name=module_name)
     return lint_context
 
 

test/unit/tool_util/test_tool_linters.py

@@ -1,8 +1,10 @@
+import inspect
 import os
 import tempfile
 
 import pytest
 
+import galaxy.tool_util.linters
 from galaxy.tool_util.lint import (
     lint_tool_source_with,
     lint_tool_source_with_modules,
@@ -18,7 +20,7 @@
     general,
     help,
     inputs,
-    outputs,
+    output,
     stdio,
     tests,
     xml_order,
@@ -29,6 +31,7 @@
 from galaxy.util import (
     ElementTree,
     parse_xml,
+    submodules,
 )
 from galaxy.util.xml_macros import load_with_references
 
@@ -1508,7 +1511,7 @@ def test_inputs_repeats(lint_ctx):
 
 def test_outputs_missing(lint_ctx):
     tool_source = get_xml_tool_source(OUTPUTS_MISSING)
-    run_lint_module(lint_ctx, outputs, tool_source)
+    run_lint_module(lint_ctx, output, tool_source)
     assert "Tool contains no outputs section, most tools should produce outputs." in lint_ctx.warn_messages
     assert not lint_ctx.info_messages
     assert not lint_ctx.valid_messages
@@ -1518,7 +1521,7 @@ def test_outputs_missing(lint_ctx):
 
 def test_outputs_multiple(lint_ctx):
     tool_source = get_xml_tool_source(OUTPUTS_MULTIPLE)
-    run_lint_module(lint_ctx, outputs, tool_source)
+    run_lint_module(lint_ctx, output, tool_source)
     assert "0 outputs found." in lint_ctx.info_messages
     assert "Invalid XML: Element 'outputs': This element is not expected." in lint_ctx.error_messages
     assert len(lint_ctx.info_messages) == 1
@@ -1535,7 +1538,7 @@ def test_outputs_unknown_tag(lint_ctx):
     probably be avoided.
     """
     tool_source = get_xml_tool_source(OUTPUTS_UNKNOWN_TAG)
-    lint_tool_source_with_modules(lint_ctx, tool_source, [outputs, xsd])
+    lint_tool_source_with_modules(lint_ctx, tool_source, [output, xsd])
     assert "0 outputs found." in lint_ctx.info_messages
     assert "Avoid the use of 'output' and replace by 'data' or 'collection'" in lint_ctx.warn_messages
     assert len(lint_ctx.info_messages) == 1
@@ -1546,7 +1549,7 @@ def test_outputs_unknown_tag(lint_ctx):
 
 def test_outputs_unnamed_invalid_name(lint_ctx):
     tool_source = get_xml_tool_source(OUTPUTS_UNNAMED_INVALID_NAME)
-    run_lint_module(lint_ctx, outputs, tool_source)
+    run_lint_module(lint_ctx, output, tool_source)
     assert "3 outputs found." in lint_ctx.info_messages
     assert "Invalid XML: Element 'data': The attribute 'name' is required but missing." in lint_ctx.error_messages
     assert "Invalid XML: Element 'collection': The attribute 'name' is required but missing." in lint_ctx.error_messages
@@ -1562,7 +1565,7 @@ def test_outputs_unnamed_invalid_name(lint_ctx):
 
 def test_outputs_format_input_legacy(lint_ctx):
     tool_source = get_xml_tool_source(OUTPUTS_FORMAT_INPUT_LEGACY)
-    run_lint_module(lint_ctx, outputs, tool_source)
+    run_lint_module(lint_ctx, output, tool_source)
     assert "1 outputs found." in lint_ctx.info_messages
     assert "Using format='input' on data is deprecated. Use the format_source attribute." in lint_ctx.warn_messages
     assert len(lint_ctx.info_messages) == 1
@@ -1573,7 +1576,7 @@ def test_outputs_format_input_legacy(lint_ctx):
 
 def test_outputs_format_input(lint_ctx):
     tool_source = get_xml_tool_source(OUTPUTS_FORMAT_INPUT)
-    run_lint_module(lint_ctx, outputs, tool_source)
+    run_lint_module(lint_ctx, output, tool_source)
     assert "1 outputs found." in lint_ctx.info_messages
     assert "Using format='input' on data is deprecated. Use the format_source attribute." in lint_ctx.error_messages
     assert len(lint_ctx.info_messages) == 1
@@ -1584,7 +1587,7 @@ def test_outputs_format_input(lint_ctx):
 
 def test_outputs_collection_format_source(lint_ctx):
     tool_source = get_xml_tool_source(OUTPUTS_COLLECTION_FORMAT_SOURCE)
-    run_lint_module(lint_ctx, outputs, tool_source)
+    run_lint_module(lint_ctx, output, tool_source)
     assert "Tool data output 'reverse' should use either format_source or format/ext" in lint_ctx.warn_messages
     assert len(lint_ctx.info_messages) == 1
     assert not lint_ctx.valid_messages
@@ -1594,7 +1597,7 @@ def test_outputs_collection_format_source(lint_ctx):
 
 def test_outputs_format_action(lint_ctx):
     tool_source = get_xml_tool_source(OUTPUTS_FORMAT_ACTION)
-    run_lint_module(lint_ctx, outputs, tool_source)
+    run_lint_module(lint_ctx, output, tool_source)
     assert len(lint_ctx.info_messages) == 1
     assert not lint_ctx.valid_messages
     assert not lint_ctx.warn_messages
@@ -1603,7 +1606,7 @@ def test_outputs_format_action(lint_ctx):
 
 def test_outputs_discover_tool_provided_metadata(lint_ctx):
     tool_source = get_xml_tool_source(OUTPUTS_DISCOVER_TOOL_PROVIDED_METADATA)
-    run_lint_module(lint_ctx, outputs, tool_source)
+    run_lint_module(lint_ctx, output, tool_source)
     assert "1 outputs found." in lint_ctx.info_messages
     assert len(lint_ctx.info_messages) == 1
     assert not lint_ctx.valid_messages
@@ -1614,7 +1617,7 @@ def test_outputs_discover_tool_provided_metadata(lint_ctx):
 def test_outputs_duplicated_name_label(lint_ctx):
     """ """
     tool_source = get_xml_tool_source(OUTPUTS_DUPLICATED_NAME_LABEL)
-    run_lint_module(lint_ctx, outputs, tool_source)
+    run_lint_module(lint_ctx, output, tool_source)
     assert "4 outputs found." in lint_ctx.info_messages
     assert len(lint_ctx.info_messages) == 1
     assert not lint_ctx.valid_messages
@@ -2075,6 +2078,30 @@ def test_xml_comments_are_ignored(lint_ctx: LintContext):
         assert "Comment" not in lint_message.message
 
 
+def test_skip_by_name(lint_ctx):
+    # add a linter class name to the skip list
+    lint_ctx.skip_types = ["CitationsMissing"]
+
+    tool_source = get_xml_tool_source(CITATIONS_ABSENT)
+    run_lint_module(lint_ctx, citations, tool_source)
+    assert not lint_ctx.warn_messages
+    assert not lint_ctx.info_messages
+    assert not lint_ctx.valid_messages
+    assert not lint_ctx.error_messages
+
+
+def test_skip_by_name(lint_ctx):
+    # add a module name to the skip list -> all linters in this module are skipped
+    lint_ctx.skip_types = ["citations"]
+
+    tool_source = get_xml_tool_source(CITATIONS_ABSENT)
+    run_lint_module(lint_ctx, citations, tool_source)
+    assert not lint_ctx.warn_messages
+    assert not lint_ctx.info_messages
+    assert not lint_ctx.valid_messages
+    assert not lint_ctx.error_messages
+
+
 def test_list_linters():
     linter_names = Linter.list_listers()
     # make sure to add/remove a test for new/removed linters if this number changes
@@ -2095,3 +2122,45 @@ def test_list_linters():
         "XSD",
     ]:
         assert len([x for x in linter_names if x.startswith(prefix)])
+
+
+def test_linter_module_list():
+    linter_modules = submodules.import_submodules(galaxy.tool_util.linters)
+    linter_module_names = [m.__name__.split(".")[-1] for m in linter_modules]
+    linter_module_names = [n for n in linter_module_names if not n.startswith("_")]
+    assert len(linter_module_names) >= 11
+
+    # until 23.2 the linters were implemented as functions lint_xyz contained in a module named xyz
+    # with 24.0 the functions were split in multiple classes (1 per linter message)
+    # in order to keep the skip functionality of lint contexts working (which is used eg in planemo)
+    # with the old names, we now also check for module name if a linter should be skipped
+    # therefore we test here that the module names are not changed
+    # the keys of the following dict represent the linter names before the switch and the values give
+    # the number of linter classes when we switched
+    # so adding new functionality to a linter module is fine. But splitting one or removing functionality
+    # should raise an error here to point to the possible consequences of renaming
+    old_linters = {
+        "citations": 4,
+        "command": 5,
+        "cwl": 9,
+        "general": 17,
+        "help": 6,
+        "inputs": 52,
+        "output": 11,
+        "stdio": 3,
+        "tests": 21,
+        "xml_order": 1,
+    }
+    assert len(set(linter_module_names).intersection(set(old_linters.keys()))) == len(old_linters.keys())
+
+    for module in linter_modules:
+        module_name = module.__name__.split(".")[-1]
+        if module_name not in old_linters:
+            continue
+        linter_cnt = 0
+        for name, value in inspect.getmembers(module):
+            if callable(value) and name.startswith("lint_"):
+                continue
+            elif inspect.isclass(value) and issubclass(value, Linter) and not inspect.isabstract(value):
+                linter_cnt += 1
+        assert linter_cnt >= old_linters[module_name]