Merge pull request #2762 from mashehu/fix-lint-multiple-module-import

Linting: Handle multiple aliases in module imports correctly during linting

CHANGELOG.md

@@ -12,6 +12,9 @@
 
 ### Linting
 
+- Make creat-lint-wf composable ([#2733](https://github.com/nf-core/tools/pull/2733))
+- Add looser comparison when pipeline logos ([#2744](https://github.com/nf-core/tools/pull/2744))
+- Handle multiple aliases in module imports correctly during linting ([#2762](https://github.com/nf-core/tools/pull/2762))
 - make creat-lint-wf composable ([#2733](https://github.com/nf-core/tools/pull/2733))
 - add looser comparison when pipeline logos ([#2744](https://github.com/nf-core/tools/pull/2744))
 - Switch to markdown based API and error docs ([#2758](https://github.com/nf-core/tools/pull/2758))

nf_core/subworkflows/lint/main_nf.py

@@ -4,6 +4,7 @@
 
 import logging
 import re
+from typing import List
 
 log = logging.getLogger(__name__)
 
@@ -120,7 +121,7 @@ def check_main_section(self, lines, included_components):
 
     # Check that all included components are used
     # Check that all included component versions are used
-    if included_components is not None:
+    if included_components:
         for component in included_components:
             if component in script:
                 self.passed.append(
@@ -152,15 +153,15 @@ def check_main_section(self, lines, included_components):
                 )
 
 
-def check_subworkflow_section(self, lines):
+def check_subworkflow_section(self, lines: List[str]) -> List[str]:
     """Lint the section of a subworkflow before the workflow definition
     Specifically checks if the subworkflow includes at least two modules or subworkflows
 
     Args:
         lines (List[str]): Content of subworkflow.
 
     Returns:
-        List: List of included component names. If subworkflow doesn't contain any lines, return None.
+        List[str]: List of included components.
     """
     # Check that we have subworkflow content
     if len(lines) == 0:
@@ -171,18 +172,25 @@ def check_subworkflow_section(self, lines):
                 self.main_nf,
             )
         )
-        return
+        return []
     self.passed.append(
         ("subworkflow_include", "Subworkflow does include modules before the workflow definition", self.main_nf)
     )
 
     includes = []
     for line in lines:
         if line.strip().startswith("include"):
-            component_name = line.split("{")[1].split("}")[0].strip()
-            if " as " in component_name:
-                component_name = component_name.split(" as ")[1].strip()
-            includes.append(component_name)
+            component_name = [line.split("{")[1].split("}")[0].strip()]
+            # check if multiple components are included
+            if ";" in component_name[0]:
+                component_name = component_name[0].split(";")
+            for comp in component_name:
+                if " as " in comp:
+                    comp = comp.split(" as ")[1].strip()
+                includes.append(comp)
+            continue
+    # remove duplicated components
+    includes = list(set(includes))
     if len(includes) >= 2:
         self.passed.append(("main_nf_include", "Subworkflow includes two or more modules", self.main_nf))
     else:

tests/subworkflows/lint.py

@@ -104,3 +104,80 @@ def test_subworkflows_lint_snapshot_file_not_needed(self):
     assert len(subworkflow_lint.failed) == 0, f"Linting failed with {[x.__dict__ for x in subworkflow_lint.failed]}"
     assert len(subworkflow_lint.passed) > 0
     assert len(subworkflow_lint.warned) >= 0
+
+
+def test_subworkflows_lint_less_than_two_modules_warning(self):
+    """Test linting a subworkflow with less than two modules"""
+    self.subworkflow_install.install("bam_stats_samtools")
+    # Remove two modules
+    with open(Path(self.pipeline_dir, "subworkflows", "nf-core", "bam_stats_samtools", "main.nf")) as fh:
+        content = fh.read()
+        new_content = content.replace(
+            "include { SAMTOOLS_IDXSTATS } from '../../../modules/nf-core/samtools/idxstats/main'", ""
+        )
+        new_content = new_content.replace(
+            "include { SAMTOOLS_FLAGSTAT } from '../../../modules/nf-core/samtools/flagstat/main'", ""
+        )
+    with open(Path(self.pipeline_dir, "subworkflows", "nf-core", "bam_stats_samtools", "main.nf"), "w") as fh:
+        fh.write(new_content)
+    subworkflow_lint = nf_core.subworkflows.SubworkflowLint(dir=self.pipeline_dir)
+    subworkflow_lint.lint(print_results=False, subworkflow="bam_stats_samtools")
+    assert len(subworkflow_lint.failed) >= 0, f"Linting failed with {[x.__dict__ for x in subworkflow_lint.failed]}"
+    assert len(subworkflow_lint.passed) > 0
+    assert len(subworkflow_lint.warned) > 0
+    assert subworkflow_lint.warned[0].lint_test == "main_nf_include"
+    # cleanup
+    self.subworkflow_remove.remove("bam_stats_samtools", force=True)
+
+
+def test_subworkflows_lint_include_multiple_alias(self):
+    """Test linting a subworkflow with multiple include methods"""
+    self.subworkflow_install.install("bam_stats_samtools")
+    with open(Path(self.pipeline_dir, "subworkflows", "nf-core", "bam_stats_samtools", "main.nf")) as fh:
+        content = fh.read()
+        new_content = content.replace("SAMTOOLS_STATS", "SAMTOOLS_STATS_1")
+        new_content = new_content.replace(
+            "include { SAMTOOLS_STATS_1 ",
+            "include { SAMTOOLS_STATS as SAMTOOLS_STATS_1; SAMTOOLS_STATS as SAMTOOLS_STATS_2 ",
+        )
+    with open(Path(self.pipeline_dir, "subworkflows", "nf-core", "bam_stats_samtools", "main.nf"), "w") as fh:
+        fh.write(new_content)
+
+    subworkflow_lint = nf_core.subworkflows.SubworkflowLint(dir=self.pipeline_dir)
+    subworkflow_lint.lint(print_results=False, subworkflow="bam_stats_samtools")
+    assert len(subworkflow_lint.failed) >= 0, f"Linting failed with {[x.__dict__ for x in subworkflow_lint.failed]}"
+    assert len(subworkflow_lint.passed) > 0
+    assert len(subworkflow_lint.warned) == 2
+    assert any(
+        [
+            x.message == "Included component 'SAMTOOLS_STATS_1' versions are added in main.nf"
+            for x in subworkflow_lint.passed
+        ]
+    )
+    assert any([x.message == "Included component 'SAMTOOLS_STATS_1' used in main.nf" for x in subworkflow_lint.passed])
+    assert any(
+        [x.message == "Included component 'SAMTOOLS_STATS_2' not used in main.nf" for x in subworkflow_lint.warned]
+    )
+
+    # cleanup
+    self.subworkflow_remove.remove("bam_stats_samtools", force=True)
+
+
+def test_subworkflows_lint_capitalization_fail(self):
+    """Test linting a subworkflow with a capitalization fail"""
+    self.subworkflow_install.install("bam_stats_samtools")
+    # change workflow name to lowercase
+    with open(Path(self.pipeline_dir, "subworkflows", "nf-core", "bam_stats_samtools", "main.nf")) as fh:
+        content = fh.read()
+        new_content = content.replace("workflow BAM_STATS_SAMTOOLS {", "workflow bam_stats_samtools {")
+    with open(Path(self.pipeline_dir, "subworkflows", "nf-core", "bam_stats_samtools", "main.nf"), "w") as fh:
+        fh.write(new_content)
+    subworkflow_lint = nf_core.subworkflows.SubworkflowLint(dir=self.pipeline_dir)
+    subworkflow_lint.lint(print_results=False, subworkflow="bam_stats_samtools")
+    assert len(subworkflow_lint.failed) >= 1, f"Linting failed with {[x.__dict__ for x in subworkflow_lint.failed]}"
+    assert len(subworkflow_lint.passed) > 0
+    assert len(subworkflow_lint.warned) >= 0
+    assert any([x.lint_test == "workflow_capitals" for x in subworkflow_lint.failed])
+
+    # cleanup
+    self.subworkflow_remove.remove("bam_stats_samtools", force=True)

tests/test_subworkflows.py

@@ -121,8 +121,11 @@ def tearDown(self):
     )
     from .subworkflows.lint import (  # type: ignore[misc]
         test_subworkflows_lint,
+        test_subworkflows_lint_capitalization_fail,
         test_subworkflows_lint_empty,
         test_subworkflows_lint_gitlab_subworkflows,
+        test_subworkflows_lint_include_multiple_alias,
+        test_subworkflows_lint_less_than_two_modules_warning,
         test_subworkflows_lint_multiple_remotes,
         test_subworkflows_lint_new_subworkflow,
         test_subworkflows_lint_no_gitlab,