Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add linter to check validity of output filters #19114

Merged
merged 4 commits into from
Nov 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions lib/galaxy/tool_util/linters/output.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -76,6 +77,23 @@ 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
for filter in tool_xml.findall("./outputs//filter"):
try:
ast.parse(filter.text, mode="eval")
except Exception as e:
lint_ctx.warn(
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"):
Expand Down
41 changes: 38 additions & 3 deletions test/unit/tool_util/test_tool_linters.py
Original file line number Diff line number Diff line change
Expand Up @@ -640,15 +640,31 @@
<data name="valid_name" format="fasta"/>
<data name="valid_name" format="fasta"/>
<data name="another_valid_name" format="fasta" label="same label may be OK if there is a filter">
<filter>a condition</filter>
<filter>a and condition</filter>
</data>
<data name="yet_another_valid_name" format="fasta" label="same label may be OK if there is a filter">
<filter>another condition</filter>
<filter>another or condition</filter>
</data>
</outputs>
</tool>
"""

# check if filters are valid python expressions
OUTPUTS_FILTER_EXPRESSION = """
<tool id="id" name="name">
<outputs>
<data name="another_valid_name" format="fasta" label="a label">
<filter>an invalid condition</filter>
<filter>an and condition</filter>
</data>
<collection name="yet_another_valid_name" type="list" format="fasta" label="another label">
<filter>another invalid condition</filter>
<filter>another or condition</filter>
</collection>
</outputs>
</tool>
"""

# tool xml for repeats linter
REPEATS = """
<tool id="id" name="name">
Expand Down Expand Up @@ -1730,6 +1746,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 (
"Filter 'another invalid condition' is no valid expression: invalid syntax (<unknown>, line 1)"
in lint_ctx.warn_messages
)
assert (
"Filter 'another invalid condition' is no valid expression: invalid syntax (<unknown>, line 1)"
in lint_ctx.warn_messages
)
assert len(lint_ctx.warn_messages) == 2
assert not lint_ctx.error_messages


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)
Expand Down Expand Up @@ -2202,7 +2237,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) == 134
assert len(linter_names) == 135
assert "Linter" not in linter_names
# make sure that linters from all modules are available
for prefix in [
Expand Down
Loading