From a8384a034314c9c319823a0c87ec413d54ba3f57 Mon Sep 17 00:00:00 2001 From: Arthur Gymer <24782660+awgymer@users.noreply.github.com> Date: Mon, 27 Mar 2023 13:14:04 +0100 Subject: [PATCH 1/4] Initial commit of system exit lint check --- nf_core/lint/__init__.py | 2 ++ nf_core/lint/system_exit.py | 26 ++++++++++++++++++++++++++ 2 files changed, 28 insertions(+) create mode 100644 nf_core/lint/system_exit.py diff --git a/nf_core/lint/__init__.py b/nf_core/lint/__init__.py index e014a933ea..a998c964a0 100644 --- a/nf_core/lint/__init__.py +++ b/nf_core/lint/__init__.py @@ -181,6 +181,7 @@ class PipelineLint(nf_core.utils.Pipeline): from .schema_description import schema_description from .schema_lint import schema_lint from .schema_params import schema_params + from .system_exit import system_exit from .template_strings import template_strings from .version_consistency import version_consistency @@ -223,6 +224,7 @@ def _get_all_lint_tests(release_mode): "template_strings", "schema_lint", "schema_params", + "system_exit", "schema_description", "actions_schema_validation", "merge_markers", diff --git a/nf_core/lint/system_exit.py b/nf_core/lint/system_exit.py new file mode 100644 index 0000000000..a6764a920f --- /dev/null +++ b/nf_core/lint/system_exit.py @@ -0,0 +1,26 @@ +import logging +from pathlib import Path + +log = logging.getLogger(__name__) + + +def system_exit(self): + passed = [] + warned = [] + + root_dir = Path(self.wf_path) + + # Get all groovy and nf files + groovy_files = [f for f in root_dir.rglob("*.groovy")] + nf_files = [f for f in root_dir.rglob("*.nf")] + to_check = nf_files + groovy_files + + for file in to_check: + with file.open() as fh: + for l in fh.readlines(): + if "System.exit" in l: + warned.append(f"`System.exit` in {file.name}: _{l.strip()}_") + if len(warned) == 0: + passed.append("No `System.exit` calls found") + + return {"passed": passed, "warned": warned} From 1422b3444e50c98aa30fc283b781c914658ead95 Mon Sep 17 00:00:00 2001 From: Arthur Gymer <24782660+awgymer@users.noreply.github.com> Date: Mon, 27 Mar 2023 14:38:19 +0100 Subject: [PATCH 2/4] Add a small docstring. Add line number to lint message --- nf_core/lint/system_exit.py | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/nf_core/lint/system_exit.py b/nf_core/lint/system_exit.py index a6764a920f..419f0cd63d 100644 --- a/nf_core/lint/system_exit.py +++ b/nf_core/lint/system_exit.py @@ -5,6 +5,13 @@ def system_exit(self): + """Check for System.exit calls in groovy/nextflow code + + Calls to System.exit(1) should be replaced by throwing errors + + This lint test looks for all calls to `System.exit` + in any file with the `.nf` or `.groovy` extension + """ passed = [] warned = [] @@ -16,10 +23,14 @@ def system_exit(self): to_check = nf_files + groovy_files for file in to_check: - with file.open() as fh: - for l in fh.readlines(): - if "System.exit" in l: - warned.append(f"`System.exit` in {file.name}: _{l.strip()}_") + try: + with file.open() as fh: + for i, l in enumerate(fh.readlines(), start=1): + if "System.exit" in l: + warned.append(f"`System.exit` in {file.name}: _{l.strip()}_ [line {i}]") + except FileNotFoundError: + log.debug(f"Could not open file {file.name} in system_exit lint test") + if len(warned) == 0: passed.append("No `System.exit` calls found") From 8d70a776e0f9d0522c066f97d49ef8b2eb79c1f0 Mon Sep 17 00:00:00 2001 From: Arthur Gymer <24782660+awgymer@users.noreply.github.com> Date: Mon, 27 Mar 2023 15:20:25 +0100 Subject: [PATCH 3/4] Add docs .md stub --- docs/api/_src/pipeline_lint_tests/system_exit.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 docs/api/_src/pipeline_lint_tests/system_exit.md diff --git a/docs/api/_src/pipeline_lint_tests/system_exit.md b/docs/api/_src/pipeline_lint_tests/system_exit.md new file mode 100644 index 0000000000..3d0ac20f8d --- /dev/null +++ b/docs/api/_src/pipeline_lint_tests/system_exit.md @@ -0,0 +1,5 @@ +# system_exit + +```{eval-rst} +.. automethod:: nf_core.lint.PipelineLint.system_exit +``` From 9626ae5f18d3660da28eca3cd3523ded1e9069fa Mon Sep 17 00:00:00 2001 From: Arthur Gymer <24782660+awgymer@users.noreply.github.com> Date: Mon, 27 Mar 2023 16:49:02 +0100 Subject: [PATCH 4/4] Allow System.exit(0) and update template --- nf_core/lint/system_exit.py | 2 +- nf_core/pipeline-template/lib/NfcoreSchema.groovy | 3 ++- nf_core/pipeline-template/lib/WorkflowMain.groovy | 5 +++-- nf_core/pipeline-template/lib/WorkflowPipeline.groovy | 8 ++++---- 4 files changed, 10 insertions(+), 8 deletions(-) diff --git a/nf_core/lint/system_exit.py b/nf_core/lint/system_exit.py index 419f0cd63d..56a526d97b 100644 --- a/nf_core/lint/system_exit.py +++ b/nf_core/lint/system_exit.py @@ -26,7 +26,7 @@ def system_exit(self): try: with file.open() as fh: for i, l in enumerate(fh.readlines(), start=1): - if "System.exit" in l: + if "System.exit" in l and not "System.exit(0)" in l: warned.append(f"`System.exit` in {file.name}: _{l.strip()}_ [line {i}]") except FileNotFoundError: log.debug(f"Could not open file {file.name} in system_exit lint test") diff --git a/nf_core/pipeline-template/lib/NfcoreSchema.groovy b/nf_core/pipeline-template/lib/NfcoreSchema.groovy index 33cd4f6e8d..4d29681431 100755 --- a/nf_core/pipeline-template/lib/NfcoreSchema.groovy +++ b/nf_core/pipeline-template/lib/NfcoreSchema.groovy @@ -2,6 +2,7 @@ // This file holds several functions used to perform JSON parameter validation, help and summary rendering for the nf-core pipeline template. // +import nextflow.Nextflow import org.everit.json.schema.Schema import org.everit.json.schema.loader.SchemaLoader import org.everit.json.schema.ValidationException @@ -177,7 +178,7 @@ class NfcoreSchema { } if (has_error) { - System.exit(1) + Nextflow.error('Exiting!') } } diff --git a/nf_core/pipeline-template/lib/WorkflowMain.groovy b/nf_core/pipeline-template/lib/WorkflowMain.groovy index 05db418b2d..2024e95c79 100755 --- a/nf_core/pipeline-template/lib/WorkflowMain.groovy +++ b/nf_core/pipeline-template/lib/WorkflowMain.groovy @@ -2,6 +2,8 @@ // This file holds several functions specific to the main.nf workflow in the {{ name }} pipeline // +import nextflow.Nextflow + class WorkflowMain { // @@ -85,8 +87,7 @@ class WorkflowMain { // Check input has been provided if (!params.input) { - log.error "Please provide an input samplesheet to the pipeline e.g. '--input samplesheet.csv'" - System.exit(1) + Nextflow.error("Please provide an input samplesheet to the pipeline e.g. '--input samplesheet.csv'") } } {% if igenomes -%} diff --git a/nf_core/pipeline-template/lib/WorkflowPipeline.groovy b/nf_core/pipeline-template/lib/WorkflowPipeline.groovy index 252f127d80..600a655439 100755 --- a/nf_core/pipeline-template/lib/WorkflowPipeline.groovy +++ b/nf_core/pipeline-template/lib/WorkflowPipeline.groovy @@ -2,6 +2,7 @@ // This file holds several functions specific to the workflow/{{ short_name }}.nf in the {{ name }} pipeline // +import nextflow.Nextflow import groovy.text.SimpleTemplateEngine class Workflow{{ short_name[0]|upper }}{{ short_name[1:] }} { @@ -15,8 +16,7 @@ class Workflow{{ short_name[0]|upper }}{{ short_name[1:] }} { {% endif %} if (!params.fasta) { - log.error "Genome fasta file not specified with e.g. '--fasta genome.fa' or via a detectable config file." - System.exit(1) + Nextflow.error "Genome fasta file not specified with e.g. '--fasta genome.fa' or via a detectable config file." } } @@ -70,12 +70,12 @@ class Workflow{{ short_name[0]|upper }}{{ short_name[1:] }} { // private static void genomeExistsError(params, log) { if (params.genomes && params.genome && !params.genomes.containsKey(params.genome)) { - log.error "~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\n" + + def error_string = "~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\n" + " Genome '${params.genome}' not found in any config files provided to the pipeline.\n" + " Currently, the available genome keys are:\n" + " ${params.genomes.keySet().join(", ")}\n" + "~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~" - System.exit(1) + Nextflow.error(error_string) } } {% endif -%}}