Add linting for ifEmpty(null)

CHANGELOG.md

@@ -6,6 +6,8 @@
 
 ### Linting
 
+- Add linting for ifEmpty(null) ([#3411](https://github.com/nf-core/tools/pull/3411))
+
 ### Modules
 
 ### Subworkflows

docs/api/_src/pipeline_lint_tests/index.md

@@ -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/)

docs/api/_src/pipeline_lint_tests/pipeline_if_empty_null.md

@@ -0,0 +1,5 @@
+# pipeline_if_empty_null
+
+```{eval-rst}
+.. automethod:: nf_core.pipelines.lint.PipelineLint.pipeline_if_empty_null
+```

docs/api/_src/subworkflow_lint_tests/index.md

@@ -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/)

docs/api/_src/subworkflow_lint_tests/subworkflow_if_empty_null.md

@@ -0,0 +1,5 @@
+# subworkflow_if_empty_null
+
+```{eval-rst}
+.. automethod:: nf_core.subworkflows.lint.SubworkflowLint.subworkflow_if_empty_null
+```

nf_core/components/lint/__init__.py

@@ -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()

nf_core/pipelines/lint/__init__.py

@@ -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
@@ -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
@@ -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",

nf_core/pipelines/lint/pipeline_if_empty_null.py

@@ -0,0 +1,50 @@
+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}

nf_core/subworkflows/lint/__init__.py

@@ -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]
@@ -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__(

nf_core/subworkflows/lint/subworkflow_if_empty_null.py

@@ -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))

nf_core/utils.py

@@ -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

tests/pipelines/lint/test_if_empty_null.py

@@ -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