Skip to content
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@

### Linting

- Add linting for ifEmpty(null) ([#3411](https://github.com/nf-core/tools/pull/3411))

### Modules

### Subworkflows
Expand Down
1 change: 1 addition & 0 deletions docs/api/_src/pipeline_lint_tests/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
- [nfcore_yml](./nfcore_yml/)
- [pipeline_name_conventions](./pipeline_name_conventions/)
- [pipeline_todos](./pipeline_todos/)
- [pipeline_if_empty_null](./pipeline_if_empty_null/)
- [plugin_includes](./plugin_includes/)
- [readme](./readme/)
- [schema_description](./schema_description/)
Expand Down
5 changes: 5 additions & 0 deletions docs/api/_src/pipeline_lint_tests/pipeline_if_empty_null.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# pipeline_if_empty_null

```{eval-rst}
.. automethod:: nf_core.pipelines.lint.PipelineLint.pipeline_if_empty_null
```
1 change: 1 addition & 0 deletions docs/api/_src/subworkflow_lint_tests/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,5 @@
- [subworkflow_changes](./subworkflow_changes/)
- [subworkflow_tests](./subworkflow_tests/)
- [subworkflow_todos](./subworkflow_todos/)
- [subworkflow_if_empty_null](./subworkflow_if_empty_null/)
- [subworkflow_version](./subworkflow_version/)
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# subworkflow_if_empty_null

```{eval-rst}
.. automethod:: nf_core.subworkflows.lint.SubworkflowLint.subworkflow_if_empty_null
```
11 changes: 9 additions & 2 deletions nf_core/components/lint/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -181,9 +181,16 @@ def get_all_module_lint_tests(is_pipeline):
@staticmethod
def get_all_subworkflow_lint_tests(is_pipeline):
if is_pipeline:
return ["main_nf", "meta_yml", "subworkflow_changes", "subworkflow_todos", "subworkflow_version"]
return [
"main_nf",
"meta_yml",
"subworkflow_changes",
"subworkflow_todos",
"subworkflow_if_empty_null",
"subworkflow_version",
]
else:
return ["main_nf", "meta_yml", "subworkflow_todos", "subworkflow_tests"]
return ["main_nf", "meta_yml", "subworkflow_todos", "subworkflow_if_empty_null", "subworkflow_tests"]

def set_up_pipeline_files(self):
self.load_lint_config()
Expand Down
3 changes: 3 additions & 0 deletions nf_core/pipelines/lint/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
from .multiqc_config import multiqc_config
from .nextflow_config import nextflow_config
from .nfcore_yml import nfcore_yml
from .pipeline_if_empty_null import pipeline_if_empty_null
from .pipeline_name_conventions import pipeline_name_conventions
from .pipeline_todos import pipeline_todos
from .plugin_includes import plugin_includes
Expand Down Expand Up @@ -94,6 +95,7 @@ class PipelineLint(nf_core.utils.Pipeline):
nfcore_yml = nfcore_yml
pipeline_name_conventions = pipeline_name_conventions
pipeline_todos = pipeline_todos
pipeline_if_empty_null = pipeline_if_empty_null
plugin_includes = plugin_includes
readme = readme
schema_description = schema_description
Expand Down Expand Up @@ -139,6 +141,7 @@ def _get_all_lint_tests(release_mode):
"actions_awsfulltest",
"readme",
"pipeline_todos",
"pipeline_if_empty_null",
"plugin_includes",
"pipeline_name_conventions",
"template_strings",
Expand Down
44 changes: 44 additions & 0 deletions nf_core/pipelines/lint/pipeline_if_empty_null.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
import logging
import os
import re
from pathlib import Path

log = logging.getLogger(__name__)


def pipeline_if_empty_null(self, root_dir=None):
"""Check for ifEmpty(null)

There are two general cases for workflows to use the channel operator `ifEmpty`:
1. `ifEmpty( [ ] )` to ensure a process executes, for example when an input file is optional (although this can be replaced by `toList()`).
2. When a channel should not be empty and throws an error `ifEmpty { error ... }`, e.g. reading from an empty samplesheet.

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*\)")

# 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 root, dirs, files in os.walk(root_dir, topdown=True):
for fname in files:
try:
with open(Path(root, fname), encoding="latin1") as fh:
for line in fh:
if re.findall(pattern, line):
warned.append(f"`ifEmpty(null)` found in `{fname}`: _{line}_")
file_paths.append(Path(root, fname))
except FileNotFoundError:
log.debug(f"Could not open file {fname} in pipeline_if_empty_null lint test")

if len(warned) == 0:
passed.append("No `ifEmpty(null)` strings found")

# return file_paths for use in subworkflow lint
return {"passed": passed, "warned": warned, "file_paths": file_paths}
2 changes: 2 additions & 0 deletions nf_core/subworkflows/lint/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
from .main_nf import main_nf # type: ignore[misc]
from .meta_yml import meta_yml # type: ignore[misc]
from .subworkflow_changes import subworkflow_changes # type: ignore[misc]
from .subworkflow_if_empty_null import subworkflow_if_empty_null # type: ignore[misc]
from .subworkflow_tests import subworkflow_tests # type: ignore[misc]
from .subworkflow_todos import subworkflow_todos # type: ignore[misc]
from .subworkflow_version import subworkflow_version # type: ignore[misc]
Expand All @@ -40,6 +41,7 @@ class SubworkflowLint(ComponentLint):
subworkflow_changes = subworkflow_changes
subworkflow_tests = subworkflow_tests
subworkflow_todos = subworkflow_todos
subworkflow_if_empty_null = subworkflow_if_empty_null
subworkflow_version = subworkflow_version

def __init__(
Expand Down
24 changes: 24 additions & 0 deletions nf_core/subworkflows/lint/subworkflow_if_empty_null.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import logging

from nf_core.pipelines.lint.pipeline_if_empty_null import pipeline_if_empty_null

log = logging.getLogger(__name__)


def subworkflow_if_empty_null(_, subworkflow):
"""Check for ifEmpty(null)

There are two general cases for workflows to use the channel operator `ifEmpty`:
1. `ifEmpty( [ ] )` to ensure a process executes, for example when an input file is optional (although this can be replaced by `toList()`).
2. When a channel should not be empty and throws an error `ifEmpty { error ... }`, e.g. reading from an empty samplesheet.

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.
"""

# Main subworkflow directory
swf_results = pipeline_if_empty_null(None, root_dir=subworkflow.component_dir)
for i, warning in enumerate(swf_results["warned"]):
subworkflow.warned.append(("subworkflow_if_empty_null", warning, swf_results["file_paths"][i]))
for i, passed in enumerate(swf_results["passed"]):
subworkflow.passed.append(("subworkflow_if_empty_null", passed, subworkflow.component_dir))
2 changes: 2 additions & 0 deletions nf_core/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -1218,6 +1218,8 @@ class NFCoreYamlLintConfig(BaseModel):
""" Lint all required files to run full tests on AWS """
pipeline_todos: Optional[bool] = None
""" Lint for TODOs statements"""
pipeline_if_empty_null: Optional[bool] = None
""" Lint for ifEmpty(null) statements"""
plugin_includes: Optional[bool] = None
""" Lint for nextflow plugin """
pipeline_name_conventions: Optional[bool] = None
Expand Down
39 changes: 39 additions & 0 deletions tests/pipelines/lint/test_if_empty_null.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
from pathlib import Path

import yaml

import nf_core.pipelines.create
import nf_core.pipelines.lint

from ..test_lint import TestLint


class TestLintIfEmptyNull(TestLint):
def setUp(self) -> None:
super().setUp()
self.new_pipeline = self._make_pipeline_copy()
self.nf_core_yml_path = Path(self.new_pipeline) / ".nf-core.yml"
with open(self.nf_core_yml_path) as f:
self.nf_core_yml = yaml.safe_load(f)

def test_if_empty_null_throws_warn(self):
"""Tests finding ifEmpty(null) in file throws warn in linting"""
# Create a file and add examples that should fail linting
txt_file = Path(self.new_pipeline) / "docs" / "test.txt"
with open(txt_file, "w") as f:
f.writelines(
[
"ifEmpty(null)\n",
"ifEmpty (null)\n",
"ifEmpty( null )\n",
"ifEmpty ( null )\n",
".ifEmpty(null)\n",
". ifEmpty(null)\n",
"|ifEmpty(null)\n",
"| ifEmpty(null)\n",
]
)
lint_obj = nf_core.pipelines.lint.PipelineLint(self.new_pipeline)
lint_obj._load()
result = lint_obj.pipeline_if_empty_null()
assert len(result["warned"]) == 8
Loading