From 94d1390dd4009fe479bc5c18d79445710e5920a2 Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Thu, 7 Nov 2024 13:27:29 +0100 Subject: [PATCH 1/3] Add linter to check validity of output filters --- lib/galaxy/tool_util/linters/output.py | 19 +++++++++++ test/unit/tool_util/test_tool_linters.py | 41 ++++++++++++++++++++++-- 2 files changed, 57 insertions(+), 3 deletions(-) diff --git a/lib/galaxy/tool_util/linters/output.py b/lib/galaxy/tool_util/linters/output.py index 37ebab7343d4..4068bf4d9d42 100644 --- a/lib/galaxy/tool_util/linters/output.py +++ b/lib/galaxy/tool_util/linters/output.py @@ -1,5 +1,6 @@ """This module contains a linting functions for tool outputs.""" +import ast from typing import TYPE_CHECKING from packaging.version import Version @@ -76,6 +77,24 @@ def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): names.add(name) +class OutputsFilterExpression(Linter): + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + tool_xml = getattr(tool_source, "xml_tree", None) + if not tool_xml: + return + labels = set() + for filter in tool_xml.findall("./outputs//filter"): + try: + ast.parse(filter.text, mode="eval") + except Exception as e: + lint_ctx.error( + f"Filter '{filter.text}' is no valid expression: {str(e)}", + linter=cls.name(), + node=filter, + ) + + class OutputsLabelDuplicatedFilter(Linter): @classmethod def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): diff --git a/test/unit/tool_util/test_tool_linters.py b/test/unit/tool_util/test_tool_linters.py index 7e6d3b62ec93..c0aad892546b 100644 --- a/test/unit/tool_util/test_tool_linters.py +++ b/test/unit/tool_util/test_tool_linters.py @@ -638,15 +638,31 @@ - a condition + a and condition - another condition + another or condition """ +# check if filters are valid python expressions +OUTPUTS_FILTER_EXPRESSION = """ + + + + an invalid condition + an and condition + + + another invalid condition + another or condition + + + +""" + # tool xml for repeats linter REPEATS = """ @@ -1678,6 +1694,25 @@ def test_outputs_duplicated_name_label(lint_ctx): assert len(lint_ctx.error_messages) == 1 +def test_outputs_filter_expression(lint_ctx): + """ """ + tool_source = get_xml_tool_source(OUTPUTS_FILTER_EXPRESSION) + run_lint_module(lint_ctx, output, tool_source) + assert "2 outputs found." in lint_ctx.info_messages + assert len(lint_ctx.info_messages) == 1 + assert not lint_ctx.valid_messages + assert not lint_ctx.warn_messages + assert ( + "Filter 'another invalid condition' is no valid expression: invalid syntax (, line 1)" + in lint_ctx.error_messages + ) + assert ( + "Filter 'another invalid condition' is no valid expression: invalid syntax (, line 1)" + in lint_ctx.error_messages + ) + assert len(lint_ctx.error_messages) == 2 + + def test_stdio_default_for_default_profile(lint_ctx): tool_source = get_xml_tool_source(STDIO_DEFAULT_FOR_DEFAULT_PROFILE) run_lint_module(lint_ctx, stdio, tool_source) @@ -2145,7 +2180,7 @@ def test_skip_by_module(lint_ctx): 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) == 132 + assert len(linter_names) == 133 assert "Linter" not in linter_names # make sure that linters from all modules are available for prefix in [ From 66e85f45ce6887a0374b1b4b8ab82db9dddf6f5b Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Thu, 7 Nov 2024 13:32:10 +0100 Subject: [PATCH 2/3] remove copy pasted code --- lib/galaxy/tool_util/linters/output.py | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/galaxy/tool_util/linters/output.py b/lib/galaxy/tool_util/linters/output.py index 4068bf4d9d42..6323865db868 100644 --- a/lib/galaxy/tool_util/linters/output.py +++ b/lib/galaxy/tool_util/linters/output.py @@ -83,7 +83,6 @@ def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): tool_xml = getattr(tool_source, "xml_tree", None) if not tool_xml: return - labels = set() for filter in tool_xml.findall("./outputs//filter"): try: ast.parse(filter.text, mode="eval") From 5ec37325a7ad117aace17e516b333634cf6d4481 Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Fri, 8 Nov 2024 10:29:09 +0100 Subject: [PATCH 3/3] change to warn --- lib/galaxy/tool_util/linters/output.py | 2 +- test/unit/tool_util/test_tool_linters.py | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/galaxy/tool_util/linters/output.py b/lib/galaxy/tool_util/linters/output.py index 6323865db868..e9ab7c4aaf4e 100644 --- a/lib/galaxy/tool_util/linters/output.py +++ b/lib/galaxy/tool_util/linters/output.py @@ -87,7 +87,7 @@ def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): try: ast.parse(filter.text, mode="eval") except Exception as e: - lint_ctx.error( + lint_ctx.warn( f"Filter '{filter.text}' is no valid expression: {str(e)}", linter=cls.name(), node=filter, diff --git a/test/unit/tool_util/test_tool_linters.py b/test/unit/tool_util/test_tool_linters.py index 2db8cca0abfd..fc99e74ed963 100644 --- a/test/unit/tool_util/test_tool_linters.py +++ b/test/unit/tool_util/test_tool_linters.py @@ -1753,16 +1753,16 @@ def test_outputs_filter_expression(lint_ctx): assert "2 outputs found." in lint_ctx.info_messages assert len(lint_ctx.info_messages) == 1 assert not lint_ctx.valid_messages - assert not lint_ctx.warn_messages assert ( "Filter 'another invalid condition' is no valid expression: invalid syntax (, line 1)" - in lint_ctx.error_messages + in lint_ctx.warn_messages ) assert ( "Filter 'another invalid condition' is no valid expression: invalid syntax (, line 1)" - in lint_ctx.error_messages + in lint_ctx.warn_messages ) - assert len(lint_ctx.error_messages) == 2 + assert len(lint_ctx.warn_messages) == 2 + assert not lint_ctx.error_messages def test_stdio_default_for_default_profile(lint_ctx):