diff --git a/CHANGELOG.md b/CHANGELOG.md index faec265c9..594bf64cb 100644 --- a/CHANGELOG.md +++ b/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)) diff --git a/nf_core/subworkflows/lint/main_nf.py b/nf_core/subworkflows/lint/main_nf.py index f59e1e427..c73559502 100644 --- a/nf_core/subworkflows/lint/main_nf.py +++ b/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,7 +153,7 @@ 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 @@ -160,7 +161,7 @@ def check_subworkflow_section(self, lines): 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,7 +172,7 @@ 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) ) @@ -179,10 +180,17 @@ def check_subworkflow_section(self, lines): 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: diff --git a/tests/subworkflows/lint.py b/tests/subworkflows/lint.py index 8804f8bf6..b89b7b78c 100644 --- a/tests/subworkflows/lint.py +++ b/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) diff --git a/tests/test_subworkflows.py b/tests/test_subworkflows.py index 19872ee16..cd0bd2114 100644 --- a/tests/test_subworkflows.py +++ b/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,