diff --git a/lib/galaxy/tool_util/lint.py b/lib/galaxy/tool_util/lint.py index bfde64c9d1bd..599006457349 100644 --- a/lib/galaxy/tool_util/lint.py +++ b/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 diff --git a/lib/galaxy/tool_util/linters/outputs.py b/lib/galaxy/tool_util/linters/output.py similarity index 100% rename from lib/galaxy/tool_util/linters/outputs.py rename to lib/galaxy/tool_util/linters/output.py diff --git a/test/unit/tool_util/test_tool_linters.py b/test/unit/tool_util/test_tool_linters.py index e553d2c16884..64d9478f1e7c 100644 --- a/test/unit/tool_util/test_tool_linters.py +++ b/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_module(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]