Lint check for System.exit calls

[awgymer]

#1840 for more details

This implements a very simplistic check of all .nf and .groovy files in the pipeline repo for System.exit calls.

PR checklist

• This comment contains a description of changes (with reason)
• CHANGELOG.md is updated
• If you've fixed a bug or added code that should be tested, add tests!
• Documentation in docs is updated

[awgymer]

CI is proving this works 😂

╭─ [✗] 6 Pipeline Tests Failed ────────────────────────────────────────────────╮
│                                                                              │
│ system_exit: System.exit in WorkflowTestpipeline.groovy: System.exit(1)      │
│ system_exit: System.exit in WorkflowTestpipeline.groovy: System.exit(1)      │
│ system_exit: System.exit in NfcoreSchema.groovy: System.exit(1)              │
│ system_exit: System.exit in WorkflowMain.groovy: System.exit(0)              │
│ system_exit: System.exit in WorkflowMain.groovy: System.exit(0)              │
│ system_exit: System.exit in WorkflowMain.groovy: System.exit(1)              │
│                                                                              │
╰──────────────────────────────────────────────────────────────────────────────╯

[awgymer]

I think we should allow System.exit(0) - replacing it with an error doesn't really make sense and after discussion with @drpatelh it is only used for the help and version options, to exit cleanly without running.

cc @ewels as you created the parent issue

[codecov]

Codecov Report

Merging #2211 (9626ae5) into dev (f3b5d1e) will increase coverage by 0.02%.
The diff coverage is 86.36%.

@@            Coverage Diff             @@
##              dev    #2211      +/-   ##
==========================================
+ Coverage   71.95%   71.98%   +0.02%     
==========================================
  Files          77       78       +1     
  Lines        8363     8384      +21     
==========================================
+ Hits         6018     6035      +17     
- Misses       2345     2349       +4     

Impacted Files	Coverage Δ
nf_core/lint/system_exit.py	85.71% <85.71%> (ø)
nf_core/lint/__init__.py	74.28% <100.00%> (+0.10%)	⬆️

... and 3 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[awgymer]

Does this need an independent test? It is tested as part of the lint test pipeline?

[ewels]

I think we should allow System.exit(0) - replacing it with an error doesn't really make sense and after discussion with @drpatelh it is only used for the help and version options, to exit cleanly without running.

I'm still a bit nervous about this. @pditommaso / @bentsherman - are System.exit(0) calls when we want to exit cleanly without an error exit code? Or is there a good alternative to error() that we can use when there is no error?

[bentsherman]

I think the exit() method is the standard way to exit with a given exit code: https://github.com/nextflow-io/nextflow/blob/7ce05c15f253059f5981f29ba85d4e16ab0b7620/modules/nextflow/src/main/groovy/nextflow/Nextflow.groovy#L183-L220

But as you can see it just logs an optional message and then calls System.exit()

Are you looking for something that calls the usual shutdown logic in the event of a workflow terminate or finish? I don't think there is a method for that.