Skip to content
Closed
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
### Linting

- accept `process_low_memory` as a standard module label ([#4264](https://github.com/nf-core/tools/pull/4264))
- `pipeline_if_empty_null`, `system_exit` and `pipeline_todos` (incl. module/subworkflow todos) now match structurally with ast-grep and the tree-sitter-nextflow grammar when available (skips comments/strings), falling back to regex otherwise — per file, whenever a file has parse errors, so recall never drops below regex level. Match logic lives in ast-grep rule files under `nf_core/pipelines/lint/rules/` with snippet tests in `tests/pipelines/lint/rules/`
- improve linting for `modules.json` to add support for only installing subworkflows from a repository and provide more explicit error messages ([#4287](https://github.com/nf-core/tools/pull/4287))
- improve singularity_tag linting check (add skip, handle exceptions correctly, check against oras) ([#4358](https://github.com/nf-core/tools/pull/4358))

Expand Down
1 change: 1 addition & 0 deletions MANIFEST.in
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,6 @@ include nf_core/assets/logo/nf-core-repo-logo-base-lightbg.png
include nf_core/assets/logo/nf-core-repo-logo-base-darkbg.png
include nf_core/assets/logo/placeholder_logo.svg
include nf_core/assets/logo/MavenPro-Bold.ttf
graft nf_core/pipelines/lint/rules
include nf_core/pipelines/create/create.tcss
include nf_core/pipelines/create/template_features.yml
185 changes: 185 additions & 0 deletions nf_core/astgrep.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,185 @@
"""Structural (AST-based) matching of Nextflow code via ast-grep.

Uses the tree-sitter-nextflow grammar (https://github.com/nextflow-io/tree-sitter-nextflow)
registered as an ast-grep custom language. The compiled parser library is discovered from:

1. The ``NFCORE_TREE_SITTER_NEXTFLOW_LIB`` environment variable (path to
``libnextflow.so`` / ``libnextflow.dylib``),
2. An installed ``tree-sitter-nextflow`` Python package (its compiled binding
exports the ``tree_sitter_nextflow`` symbol ast-grep needs); until it is on
PyPI, install with
``pip install git+https://github.com/nextflow-io/tree-sitter-nextflow.git``, or
3. A global ast-grep install (``~/.config/ast-grep/sgconfig.yml``), as set up by
tree-sitter-nextflow's ``install-ast-grep.sh --global``.

If none is available, :func:`nextflow_available` returns ``False`` and callers
should fall back to text-based matching.
"""

import logging
import os
import platform
import re
import sys
from collections.abc import Iterable
from functools import cache
from pathlib import Path

import yaml

log = logging.getLogger(__name__)

#: File extensions that the Nextflow grammar can parse (error recovery makes
#: it usable for plain Groovy lib files too)
NEXTFLOW_EXTENSIONS = {".nf", ".config", ".groovy"}

GLOBAL_SGCONFIG = Path.home() / ".config" / "ast-grep" / "sgconfig.yml"


def _platform_triple() -> str | None:
"""Rust-style target triple used as libraryPath key in sgconfig.yml."""
arch = {"arm64": "aarch64", "aarch64": "aarch64", "x86_64": "x86_64", "amd64": "x86_64"}.get(
platform.machine().lower()
)
os_part = {"darwin": "apple-darwin", "linux": "unknown-linux-gnu", "win32": "pc-windows-msvc"}.get(sys.platform)
if arch and os_part:
return f"{arch}-{os_part}"
return None


def _find_library() -> Path | None:
"""Locate the compiled tree-sitter-nextflow parser library."""
env_path = os.environ.get("NFCORE_TREE_SITTER_NEXTFLOW_LIB")
if env_path:
if Path(env_path).is_file():
return Path(env_path)
log.debug(f"NFCORE_TREE_SITTER_NEXTFLOW_LIB set but not a file: {env_path}")

try:
import tree_sitter_nextflow

lib = next(Path(tree_sitter_nextflow.__file__).parent.glob("_binding*"), None)
if lib is not None:
return lib
except ImportError:
pass

if GLOBAL_SGCONFIG.is_file():
try:
with open(GLOBAL_SGCONFIG) as fh:
config = yaml.safe_load(fh)
library_path = config["customLanguages"]["nextflow"]["libraryPath"]
if isinstance(library_path, dict):
library_path = library_path.get(_platform_triple())
if library_path:
lib = GLOBAL_SGCONFIG.parent / library_path
if lib.is_file():
return lib
except (KeyError, TypeError, yaml.YAMLError) as e:
log.debug(f"Could not read nextflow language from {GLOBAL_SGCONFIG}: {e}")

return None


@cache
def nextflow_available() -> bool:
"""Register the Nextflow custom language with ast-grep (once per process).

Returns False if ast-grep-py or the compiled parser library is unavailable.
"""
try:
from ast_grep_py import register_dynamic_language
except ImportError:
log.debug("ast-grep-py not installed, structural Nextflow linting unavailable")
return False

lib = _find_library()
if lib is None:
log.debug("tree-sitter-nextflow parser library not found, structural Nextflow linting unavailable")
return False

try:
register_dynamic_language(
{
"nextflow": {
"library_path": str(lib),
"language_symbol": "tree_sitter_nextflow",
"expando_char": "_",
# required at runtime but missing from the CustomLang type stub
"extensions": ["nf", "config"], # type: ignore[typeddict-unknown-key]
}
}
)
except RuntimeError as e:
# e.g. an outdated library that does not export the parser symbol
log.debug(f"Could not register Nextflow ast-grep language from {lib}: {e}")
return False
log.debug(f"Registered Nextflow ast-grep language from {lib}")
return True


def find_all(source: str, rule: dict):
"""Find all nodes in Nextflow source matching an ast-grep rule.

Callers must check :func:`nextflow_available` first.
"""
from ast_grep_py import SgRoot

return SgRoot(source, "nextflow").root().find_all(**rule)


@cache
def load_rule(path: Path) -> dict:
"""Load an ast-grep lint rule file (id, severity, message, rule, ...)."""
with open(path) as fh:
return yaml.safe_load(fh)


def scan(source: str, rule_config: dict):
"""Find all nodes matching a full ast-grep rule config (rule/constraints/utils)."""
from ast_grep_py import SgRoot

config = {key: rule_config[key] for key in ("rule", "constraints", "utils") if key in rule_config}
return SgRoot(source, "nextflow").root().find_all(config=config)


def find_matches(rule_config: dict, files: Iterable[Path | str], regex_unparseable: bool = False):
"""Yield ``(file, line_number, snippet)`` for every rule match across files.

A Nextflow/Groovy file is matched structurally only when the parser is
available AND the file parses without ERROR nodes. A broken parse can
swallow the rest of a file (e.g. process directives the grammar does not
know yet), so any parse error means that file falls back to line-by-line
matching with the rule's ``metadata.fallback-regex`` — recall never drops
below regex level, and precision upgrades as the grammar matures.

``regex_unparseable`` controls files the Nextflow grammar cannot parse at
all (e.g. markdown, yaml) when the parser is available: skipped by
default, regex-scanned if True (for rules that target all file types,
like TODOs).
"""
structural = nextflow_available()
pattern = re.compile(rule_config["metadata"]["fallback-regex"])
for file in map(Path, files):
try:
text = file.read_text(encoding="latin1")
except (FileNotFoundError, IsADirectoryError, PermissionError) as e:
log.debug(f"Could not open file {file} while scanning for {rule_config.get('id')}: {e}")
continue

if structural and file.suffix in NEXTFLOW_EXTENSIONS:
from ast_grep_py import SgRoot

root = SgRoot(text, "nextflow").root()
if root.find(kind="ERROR") is None:
config = {key: rule_config[key] for key in ("rule", "constraints", "utils") if key in rule_config}
for match in root.find_all(config=config):
yield file, match.range().start.line + 1, match.text()
continue
log.debug(f"{file} has parse errors, falling back to regex for {rule_config.get('id')}")
elif structural and not regex_unparseable:
continue

for line_number, line in enumerate(text.splitlines(), start=1):
if pattern.search(line):
yield file, line_number, line.strip()
32 changes: 14 additions & 18 deletions nf_core/pipelines/lint/pipeline_if_empty_null.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
import logging
import re
from pathlib import Path

from nf_core import astgrep
from nf_core.utils import get_wf_files

log = logging.getLogger(__name__)

RULE_FILE = Path(__file__).parent / "rules" / "pipeline_if_empty_null.yml"


def pipeline_if_empty_null(self, root_dir=None):
"""Check for ifEmpty(null)
Expand All @@ -16,29 +18,23 @@ def pipeline_if_empty_null(self, root_dir=None):

There are multiple examples of workflows that inject null objects into channels using `ifEmpty(null)`, which can cause unhandled null pointer exceptions.
This lint test throws warnings for those instances.
"""
passed = []
warned = []
file_paths = []
pattern = re.compile(r"ifEmpty\s*\(\s*null\s*\)")

The match logic lives in the ast-grep rule file ``rules/pipeline_if_empty_null.yml``.
See ``nf_core.astgrep.find_matches`` for the structural vs regex-fallback behaviour.
"""
# Pipelines don't provide a path, so use the workflow path.
# Modules run this function twice and provide a string path
if root_dir is None:
root_dir = self.wf_path

for file in get_wf_files(root_dir):
try:
with open(Path(file), encoding="latin1") as fh:
for line in fh:
if re.findall(pattern, line):
warned.append(f"`ifEmpty(null)` found in `{file}`: _{line}_")
file_paths.append(Path(file))
except FileNotFoundError:
log.debug(f"Could not open file {file} in pipeline_if_empty_null lint test")

if len(warned) == 0:
passed.append("No `ifEmpty(null)` strings found")
rule = astgrep.load_rule(RULE_FILE)
warned = []
file_paths = []
for file, line_num, text in astgrep.find_matches(rule, get_wf_files(root_dir)):
warned.append(f"{rule['message']} in `{file}` [line {line_num}]: _{text}_")
file_paths.append(file)

passed = [] if warned else ["No `ifEmpty(null)` strings found"]

# return file_paths for use in subworkflow lint
return {"passed": passed, "warned": warned, "file_paths": file_paths}
54 changes: 35 additions & 19 deletions nf_core/pipelines/lint/pipeline_todos.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,31 @@
import os
from pathlib import Path

from nf_core import astgrep

log = logging.getLogger(__name__)

RULE_FILE = Path(__file__).parent / "rules" / "pipeline_todos.yml"


def _clean_todo(text: str) -> str:
"""Reduce a matched comment to the TODO text itself."""
for line in text.splitlines():
if "TODO nf-core" in line:
text = line
break
return (
text.replace("<!--", "")
.replace("-->", "")
.replace("/*", "")
.replace("*/", "")
.replace("*", "")
.replace("# TODO nf-core: ", "")
.replace("// TODO nf-core: ", "")
.replace("TODO nf-core: ", "")
.strip()
)


def pipeline_todos(self, root_dir=None):
"""Check for nf-core *TODO* lines.
Expand All @@ -26,6 +49,10 @@ def pipeline_todos(self, root_dir=None):
This lint test runs through all files in the pipeline and searches for these lines.
If any are found they will throw a warning.

Nextflow/Groovy files are matched structurally via the ast-grep rule file
``rules/pipeline_todos.yml`` when the parser is available (a "TODO nf-core"
inside a string is not flagged); all other files are searched line by line.

.. tip:: Note that many GUI code editors have plugins to list all instances of *TODO*
in a given project directory. This is a very quick and convenient way to get
started on your pipeline!
Expand All @@ -45,33 +72,22 @@ def pipeline_todos(self, root_dir=None):
with open(Path(root_dir, ".gitignore"), encoding="latin1") as fh:
for line in fh:
ignore.append(Path(line.strip().rstrip("/")).name)

to_check = []
for root, dirs, files in os.walk(root_dir, topdown=True):
# Ignore files
for i_base in ignore:
i = str(Path(root, i_base))
dirs[:] = [d for d in dirs if not fnmatch.fnmatch(str(Path(root, d)), i)]
files[:] = [f for f in files if not fnmatch.fnmatch(str(Path(root, f)), i)]
for fname in files:
try:
with open(Path(root, fname), encoding="latin1") as fh:
for line in fh:
if "TODO nf-core" in line:
line = (
line.replace("<!--", "")
.replace("-->", "")
.replace("# TODO nf-core: ", "")
.replace("// TODO nf-core: ", "")
.replace("TODO nf-core: ", "")
.strip()
)
warned.append(f"TODO string in `{fname}`: _{line}_")
file_paths.append(Path(root, fname))
except FileNotFoundError:
log.debug(f"Could not open file {fname} in pipeline_todos lint test")
to_check.extend(Path(root, fname) for fname in files)

rule = astgrep.load_rule(RULE_FILE)
for file, _line_num, text in astgrep.find_matches(rule, to_check, regex_unparseable=True):
warned.append(f"TODO string in `{file.name}`: _{_clean_todo(text)}_")
file_paths.append(file)

if len(warned) == 0:
passed.append("No TODO strings found")

# HACK file paths are returned to allow usage of this function in modules/lint.py
# Needs to be refactored!
return {"passed": passed, "warned": warned, "file_paths": file_paths}
28 changes: 28 additions & 0 deletions nf_core/pipelines/lint/rules/pipeline_if_empty_null.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
# ast-grep lint rule (https://ast-grep.github.io/guide/project/lint-rule.html)
# Also usable directly with `ast-grep scan` via a sgconfig.yml ruleDirs entry.
id: pipeline_if_empty_null
language: nextflow
severity: warning
message: "`ifEmpty(null)` found"
note: |
Instead of `ifEmpty(null)`, use `ifEmpty([])` to keep a process executing on
optional input, or `ifEmpty { error ... }` when the channel must not be empty.
Injecting null objects into channels causes unhandled null pointer exceptions.
metadata:
fallback-regex: ifEmpty\s*\(\s*null\s*\)
rule:
any:
# method call form: ch.ifEmpty(null)
- pattern: $CH.ifEmpty(null)
# bare / piped form: ch | ifEmpty(null)
- pattern: ifEmpty(null)
# method calls whose receiver the grammar could not fully parse; only
# requires direct children `ifEmpty` and `null`
- all:
- kind: method_call
- has:
kind: identifier
regex: ^ifEmpty$
- has:
kind: simple_expression
regex: ^null$
16 changes: 16 additions & 0 deletions nf_core/pipelines/lint/rules/pipeline_todos.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# ast-grep lint rule (https://ast-grep.github.io/guide/project/lint-rule.html)
id: pipeline_todos
language: nextflow
severity: warning
message: "TODO string"
note: |
The nf-core templates contain `TODO nf-core:` comments marking places that
developers need to edit. They should be removed once addressed.
metadata:
fallback-regex: TODO nf-core
# a comment node containing "TODO nf-core" (strings/code are not flagged)
rule:
regex: TODO nf-core
any:
- kind: line_comment
- kind: block_comment
Loading
Loading