Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Linting: allow ignoring specific files when linting template_strings #2686

Merged
merged 9 commits into from
Jan 29, 2024
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
- Fix linting of a pipeline with patched custom module ([#2669](https://github.com/nf-core/tools/pull/2669))
- linting a pipeline also lints the installed subworkflows ([#2677](https://github.com/nf-core/tools/pull/2677))
- environment.yml name must be lowercase ([#2676](https://github.com/nf-core/tools/pull/2676))
- allow ignoring specific files when template_strings ([#2686](https://github.com/nf-core/tools/pull/2686))
- lint `nextflow.config` default values match the ones specified in `nextflow_schema.json` ([#2684](https://github.com/nf-core/tools/pull/2684))

### Modules
Expand Down
11 changes: 11 additions & 0 deletions nf_core/lint/merge_markers.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,17 @@ def merge_markers(self):
This test looks for remaining merge markers in the code, e.g.:
>>>>>>> or <<<<<<<

.. tip:: You can choose to ignore this lint tests by editing the file called
``.nf-core.yml`` in the root of your pipeline and setting the test to false:
.. code-block:: yaml
lint:
merge_markers: False
To disable this test only for specific files, you can specify a list of file paths to ignore.
For example, to ignore a pdf you added to the docs:
.. code-block:: yaml
lint:
merge_markers:
- docs/my_pdf.pdf
"""
passed = []
failed = []
Expand Down
25 changes: 24 additions & 1 deletion nf_core/lint/template_strings.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,36 @@ def template_strings(self):

This test ignores any double-brackets prefixed with a dollar sign, such as
``${{ secrets.AWS_ACCESS_KEY_ID }}`` as these placeholders are used in GitHub Actions workflows.

.. tip:: You can choose to ignore lint test tests by editing the file called
``.nf-core.yml`` in the root of your pipeline and setting the test to false:

.. code-block:: yaml

lint:
template_strings: False

To disable this test only for specific files, you can specify a list of file paths to ignore.
For example, to ignore a pdf you added to the docs:

.. code-block:: yaml

lint:
template_strings:
- docs/my_pdf.pdf
"""
passed = []
failed = []
ignored = []
# Files that should be ignored according to the linting config
ignore_files = self.lint_config.get("template_strings", [])

# Loop through files, searching for string
num_matches = 0
for fn in self.files:
if str(fn.relative_to(self.wf_path)) in ignore_files:
ignored.append(f"Ignoring Jinja template strings in file `{fn}`")
continue
# Skip binary files
binary_ftypes = ["image", "application/java-archive"]
(ftype, encoding) = mimetypes.guess_type(fn)
Expand All @@ -41,4 +64,4 @@ def template_strings(self):
if num_matches == 0:
passed.append(f"Did not find any Jinja template strings ({len(self.files)} files)")

return {"passed": passed, "failed": failed}
return {"passed": passed, "failed": failed, "ignored": ignored}
6 changes: 3 additions & 3 deletions nf_core/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -159,8 +159,8 @@ def _list_files(self):
git_ls_files = subprocess.check_output(["git", "ls-files"], cwd=self.wf_path).splitlines()
self.files = []
for fn in git_ls_files:
full_fn = os.path.join(self.wf_path, fn.decode("utf-8"))
if os.path.isfile(full_fn):
full_fn = Path(self.wf_path) / fn.decode("utf-8")
if full_fn.is_file():
self.files.append(full_fn)
else:
log.debug(f"`git ls-files` returned '{full_fn}' but could not open it!")
Expand All @@ -170,7 +170,7 @@ def _list_files(self):
self.files = []
for subdir, _, files in os.walk(self.wf_path):
for fn in files:
self.files.append(os.path.join(subdir, fn))
self.files.append(Path(subdir) / fn)

def _load_pipeline_config(self):
"""Get the nextflow config for this pipeline
Expand Down
54 changes: 54 additions & 0 deletions tests/lint/template_strings.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
import subprocess
from pathlib import Path

import nf_core.create
import nf_core.lint


def test_template_strings(self):
"""Tests finding a template string in a file fails linting."""
new_pipeline = self._make_pipeline_copy()
# Add template string to a file
txt_file = Path(new_pipeline) / "docs" / "test.txt"
with open(txt_file, "w") as f:
f.write("my {{ template_string }}")
subprocess.check_output(["git", "add", "docs"], cwd=new_pipeline)
lint_obj = nf_core.lint.PipelineLint(new_pipeline)
lint_obj._load()
result = lint_obj.template_strings()
print(result["failed"])
assert len(result["failed"]) == 1
assert len(result["ignored"]) == 0


def test_template_strings_ignored(self):
"""Tests ignoring template_strings"""
new_pipeline = self._make_pipeline_copy()
# Ignore template_strings test
nf_core_yml = Path(new_pipeline) / ".nf-core.yml"
with open(nf_core_yml, "w") as f:
f.write("repository_type: pipeline\nlint:\n template_strings: False")
lint_obj = nf_core.lint.PipelineLint(new_pipeline)
lint_obj._load()
lint_obj._lint_pipeline()
assert len(lint_obj.failed) == 0
assert len(lint_obj.ignored) == 1


def test_template_strings_ignore_file(self):
"""Tests ignoring template_strings file"""
new_pipeline = self._make_pipeline_copy()
# Add template string to a file
txt_file = Path(new_pipeline) / "docs" / "test.txt"
with open(txt_file, "w") as f:
f.write("my {{ template_string }}")
subprocess.check_output(["git", "add", "docs"], cwd=new_pipeline)
# Ignore template_strings test
nf_core_yml = Path(new_pipeline) / ".nf-core.yml"
with open(nf_core_yml, "w") as f:
f.write("repository_type: pipeline\nlint:\n template_strings:\n - docs/test.txt")
lint_obj = nf_core.lint.PipelineLint(new_pipeline)
lint_obj._load()
result = lint_obj.template_strings()
assert len(result["failed"]) == 0
assert len(result["ignored"]) == 1
5 changes: 5 additions & 0 deletions tests/test_lint.py
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,11 @@ def test_sphinx_md_files(self):
test_nextflow_config_example_pass,
test_nextflow_config_missing_test_profile_failed,
)
from .lint.template_strings import ( # type: ignore[misc]
test_template_strings,
test_template_strings_ignore_file,
test_template_strings_ignored,
)
from .lint.version_consistency import test_version_consistency # type: ignore[misc]


Expand Down
6 changes: 3 additions & 3 deletions tests/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -112,14 +112,14 @@ def test_load_pipeline_config(self):
def test_list_files_git(self):
"""Test listing pipeline files using `git ls`"""
self.pipeline_obj._list_files()
assert os.path.join(self.test_pipeline_dir, "main.nf") in self.pipeline_obj.files
assert Path(self.test_pipeline_dir, "main.nf") in self.pipeline_obj.files

@with_temporary_folder
def test_list_files_no_git(self, tmpdir):
"""Test listing pipeline files without `git-ls`"""
# Create a test file in a temporary directory
tmp_fn = os.path.join(tmpdir, "testfile")
Path(tmp_fn).touch()
tmp_fn = Path(tmpdir, "testfile")
tmp_fn.touch()
pipeline_obj = nf_core.utils.Pipeline(tmpdir)
pipeline_obj._list_files()
assert tmp_fn in pipeline_obj.files
Expand Down
Loading