Linting: fix ignoring files_unchanged

.github/workflows/fix-linting.yml

@@ -10,7 +10,7 @@ jobs:
       contains(github.event.comment.html_url, '/pull/') &&
       contains(github.event.comment.body, '@nf-core-bot fix linting') &&
       github.repository == 'nf-core/tools'
-    runs-on: self-hosted
+    runs-on: ubuntu-latest
     steps:
       # Use the @nf-core-bot token to check out so we can push later
       - uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4

CHANGELOG.md

@@ -7,6 +7,7 @@
 ### Linting
 
 - Handle default values of type number from nextflow schema ([#2703](https://github.com/nf-core/tools/pull/2703))
+- fix ignoring files_unchanged ([#2707](https://github.com/nf-core/tools/pull/2707))
 
 ### Modules
 

nf_core/lint/__init__.py

@@ -8,6 +8,8 @@
 import json
 import logging
 import os
+from pathlib import Path
+from typing import List, Union
 
 import git
 import rich
@@ -621,7 +623,7 @@ def _save_json_results(self, json_fn):
         with open(json_fn, "w") as fh:
             json.dump(results, fh, indent=4)
 
-    def _wrap_quotes(self, files):
+    def _wrap_quotes(self, files: Union[List[str], List[Path], Path]) -> str:
         """Helper function to take a list of filenames and format with markdown.
 
         Args:
@@ -636,5 +638,5 @@ def _wrap_quotes(self, files):
         """
         if not isinstance(files, list):
             files = [files]
-        bfiles = [f"`{f}`" for f in files]
+        bfiles = [f"`{str(f)}`" for f in files]
         return " or ".join(bfiles)

nf_core/lint/files_exist.py

@@ -1,11 +1,11 @@
 import logging
 from pathlib import Path
-from typing import Union
+from typing import Dict, List, Tuple, Union
 
 log = logging.getLogger(__name__)
 
 
-def files_exist(self):
+def files_exist(self) -> dict[str, Union[List[str], bool]]:
     """Checks a given pipeline directory for required files.
 
     Iterates through the pipeline's directory content and checks that specified
@@ -129,19 +129,19 @@ def files_exist(self):
         short_name = self.nf_config["manifest.name"].strip("\"'").split("/")
 
     files_fail = [
-        [".gitattributes"],
-        [".gitignore"],
-        [".nf-core.yml"],
-        [".editorconfig"],
-        [".prettierignore"],
-        [".prettierrc.yml"],
-        ["CHANGELOG.md"],
-        ["CITATIONS.md"],
-        ["CODE_OF_CONDUCT.md"],
-        ["LICENSE", "LICENSE.md", "LICENCE", "LICENCE.md"],  # NB: British / American spelling
-        ["nextflow_schema.json"],
-        ["nextflow.config"],
-        ["README.md"],
+        [Path(".gitattributes")],
+        [Path(".gitignore")],
+        [Path(".nf-core.yml")],
+        [Path(".editorconfig")],
+        [Path(".prettierignore")],
+        [Path(".prettierrc.yml")],
+        [Path("CHANGELOG.md")],
+        [Path("CITATIONS.md")],
+        [Path("CODE_OF_CONDUCT.md")],
+        [Path("LICENSE"), Path("LICENSE.md"), Path("LICENCE"), Path("LICENCE.md")],  # NB: British / American spelling
+        [Path("nextflow_schema.json")],
+        [Path("nextflow.config")],
+        [Path("README.md")],
         [Path(".github", ".dockstore.yml")],
         [Path(".github", "CONTRIBUTING.md")],
         [Path(".github", "ISSUE_TEMPLATE", "bug_report.yml")],
@@ -171,37 +171,39 @@ def files_exist(self):
     ]
 
     files_warn = [
-        ["main.nf"],
+        [Path("main.nf")],
         [Path("assets", "multiqc_config.yml")],
         [Path("conf", "base.config")],
         [Path("conf", "igenomes.config")],
         [Path(".github", "workflows", "awstest.yml")],
         [Path(".github", "workflows", "awsfulltest.yml")],
         [Path("lib", f"Workflow{short_name[0].upper()}{short_name[1:]}.groovy")],
-        ["modules.json"],
-        ["pyproject.toml"],
+        [Path("modules.json")],
+        [Path("pyproject.toml")],
     ]
 
     # List of strings. Fails / warns if any of the strings exist.
     files_fail_ifexists = [
-        "Singularity",
-        "parameters.settings.json",
-        "pipeline_template.yml",  # saving information in .nf-core.yml
-        ".nf-core.yaml",  # yml not yaml
+        Path("Singularity"),
+        Path("parameters.settings.json"),
+        Path("pipeline_template.yml"),  # saving information in .nf-core.yml
+        Path(".nf-core.yaml"),  # yml not yaml
         Path("bin", "markdown_to_html.r"),
         Path("conf", "aws.config"),
         Path(".github", "workflows", "push_dockerhub.yml"),
         Path(".github", "ISSUE_TEMPLATE", "bug_report.md"),
         Path(".github", "ISSUE_TEMPLATE", "feature_request.md"),
         Path("docs", "images", f"nf-core-{short_name}_logo.png"),
-        ".markdownlint.yml",
-        ".yamllint.yml",
+        Path(".markdownlint.yml"),
+        Path(".yamllint.yml"),
         Path("lib", "Checks.groovy"),
         Path("lib", "Completion.groovy"),
         Path("lib", "Workflow.groovy"),
     ]
-    files_warn_ifexists = [".travis.yml"]
-    files_fail_ifinconfig = [[Path("lib", "nfcore_external_java_deps.jar"), "nf-validation"]]
+    files_warn_ifexists = [Path(".travis.yml")]
+    files_fail_ifinconfig: List[Tuple[Path, Dict[str, str]]] = [
+        (Path("lib", "nfcore_external_java_deps.jar"), {"plugins": "nf-validation"}),
+    ]
 
     # Remove files that should be ignored according to the linting config
     ignore_files = self.lint_config.get("files_exist", [])
@@ -242,22 +244,24 @@ def pf(file_path: Union[str, Path]) -> Path:
         else:
             passed.append(f"File not found check: {self._wrap_quotes(file)}")
     # Files that cause an error if they exists together with a certain entry in nextflow.config
-    for file in files_fail_ifinconfig:
-        if str(file[0]) in ignore_files:
+    for file_cond in files_fail_ifinconfig:
+        log.error(f"{self.nf_config=}")
+        if str(file_cond[0]) in ignore_files:
             continue
-        nextflow_config = pf("nextflow.config")
         in_config = False
-        with open(nextflow_config) as f:
-            if file[1] in f.read():
-                in_config = True
-        if pf(file[0]).is_file() and in_config:
-            failed.append(f"File must be removed: {self._wrap_quotes(file[0])}")
-        elif pf(file[0]).is_file() and not in_config:
-            passed.append(f"File found check: {self._wrap_quotes(file[0])}")
-        elif not pf(file[0]).is_file() and not in_config:
-            failed.append(f"File not found check: {self._wrap_quotes(file[0])}")
-        elif not pf(file[0]).is_file() and in_config:
-            passed.append(f"File not found check: {self._wrap_quotes(file[0])}")
+        config_key, config_value = list(file_cond[1].items())[0]
+        if config_key in self.nf_config and config_value in self.nf_config[config_key]:
+            in_config = True
+            log.debug(f"Found {config_key} in nextflow.config with value {config_value}")
+            in_config = True
+        if pf(file_cond[0]).is_file() and in_config:
+            failed.append(f"File must be removed: {self._wrap_quotes(file_cond[0])}")
+        elif pf(file_cond[0]).is_file() and not in_config:
+            passed.append(f"File found check: {self._wrap_quotes(file_cond[0])}")
+        elif not pf(file_cond[0]).is_file() and not in_config:
+            failed.append(f"File not found check: {self._wrap_quotes(file_cond[0])}")
+        elif not pf(file_cond[0]).is_file() and in_config:
+            passed.append(f"File not found check: {self._wrap_quotes(file_cond[0])}")
     # Files that cause a warning if they exist
     for file in files_warn_ifexists:
         if file in ignore_files:

nf_core/lint/files_unchanged.py

@@ -3,7 +3,7 @@
 import shutil
 import tempfile
 from pathlib import Path
-from typing import Union
+from typing import Dict, List, Tuple, Union
 
 import yaml
 
@@ -12,7 +12,7 @@
 log = logging.getLogger(__name__)
 
 
-def files_unchanged(self):
+def files_unchanged(self) -> dict[str, Union[List[str], bool]]:
     """Checks that certain pipeline files are not modified from template output.
 
     Iterates through the pipeline's directory content and compares specified files
@@ -64,11 +64,11 @@ def files_unchanged(self):
 
     """
 
-    passed = []
-    failed = []
-    ignored = []
-    fixed = []
-    could_fix = False
+    passed: List[str] = []
+    failed: List[str] = []
+    ignored: List[str] = []
+    fixed: List[str] = []
+    could_fix: bool = False
 
     # Check that we have the minimum required config
     required_pipeline_config = {"manifest.name", "manifest.description", "manifest.author"}
@@ -87,10 +87,10 @@ def files_unchanged(self):
     # NB: Should all be files, not directories
     # List of lists. Passes if any of the files in the sublist are found.
     files_exact = [
-        [".gitattributes"],
-        [".prettierrc.yml"],
-        ["CODE_OF_CONDUCT.md"],
-        ["LICENSE", "LICENSE.md", "LICENCE", "LICENCE.md"],  # NB: British / American spelling
+        [Path(".gitattributes")],
+        [Path(".prettierrc.yml")],
+        [Path("CODE_OF_CONDUCT.md")],
+        [Path("LICENSE", "LICENSE.md", "LICENCE", "LICENCE.md")],  # NB: British / American spelling
         [Path(".github", ".dockstore.yml")],
         [Path(".github", "CONTRIBUTING.md")],
         [Path(".github", "ISSUE_TEMPLATE", "bug_report.yml")],
@@ -110,10 +110,10 @@ def files_unchanged(self):
         [Path("lib", "NfcoreTemplate.groovy")],
     ]
     files_partial = [
-        [".gitignore", ".prettierignore", "pyproject.toml"],
+        [Path(".gitignore"), Path(".prettierignore"), Path("pyproject.toml")],
     ]
-    files_conditional = [
-        [Path("lib", "nfcore_external_java_deps.jar"), {"plugins": "nf_validation"}],
+    files_conditional: List[Tuple[List[Path], Dict[str, str]]] = [
+        ([Path("lib", "nfcore_external_java_deps.jar")], {"plugins": "nf-validation"}),
     ]
 
     # Only show error messages from pipeline creation
@@ -153,7 +153,7 @@ def _tf(file_path: Union[str, Path]) -> Path:
     for files in files_exact:
         # Ignore if file specified in linting config
         ignore_files = self.lint_config.get("files_unchanged", [])
-        if any([f in ignore_files for f in files]):
+        if any([str(f) in ignore_files for f in files]):
             ignored.append(f"File ignored due to lint config: {self._wrap_quotes(files)}")
 
         # Ignore if we can't find the file
@@ -215,37 +215,38 @@ def _tf(file_path: Union[str, Path]) -> Path:
                     pass
 
     # Files that should be there only if an entry in nextflow config is not set
-    for files in files_conditional:
+    for file_conditional in files_conditional:
         # Ignore if file specified in linting config
         ignore_files = self.lint_config.get("files_unchanged", [])
-        if files[0] in ignore_files:
-            ignored.append(f"File ignored due to lint config: {self._wrap_quotes(files)}")
+        if any(str(f) in ignore_files for f in files_conditional[0]):
+            ignored.append(f"File ignored due to lint config: {self._wrap_quotes(file_conditional[0])}")
 
         # Ignore if we can't find the file
-        elif _pf(files[0]).is_file():
-            ignored.append(f"File does not exist: {self._wrap_quotes(files[0])}")
+        elif not any([_pf(f).is_file() for f in file_conditional[0]]):
+            ignored.append(f"File does not exist: {self._wrap_quotes(file_conditional[0])}")
 
         # Check that the file has an identical match
         else:
-            config_key, config_value = list(files[1].items())[0]
-            if config_key in self.nf_config and self.nf_config[config_key] == config_value:
-                # Ignore if the config key is set to the expected value
-                ignored.append(f"File ignored due to config: {self._wrap_quotes(files)}")
-            else:
-                try:
-                    if filecmp.cmp(_pf(files[0]), _tf(files[0]), shallow=True):
-                        passed.append(f"`{files[0]}` matches the template")
-                    else:
-                        if "files_unchanged" in self.fix:
-                            # Try to fix the problem by overwriting the pipeline file
-                            shutil.copy(_tf(files[0]), _pf(files[0]))
-                            passed.append(f"`{files[0]}` matches the template")
-                            fixed.append(f"`{files[0]}` overwritten with template file")
+            config_key, config_value = list(file_conditional[1].items())[0]
+            for f in file_conditional[0]:
+                if config_key in self.nf_config and config_value in self.nf_config[config_key]:
+                    # Ignore if the config key is set to the expected value
+                    ignored.append(f"File ignored due to config: {self._wrap_quotes(file_conditional[0])}")
+                else:
+                    try:
+                        if filecmp.cmp(_pf(f), _tf(f), shallow=True):
+                            passed.append(f"`{f}` matches the template")
                         else:
-                            failed.append(f"`{files[0]}` does not match the template")
-                            could_fix = True
-                except FileNotFoundError:
-                    pass
+                            if "files_unchanged" in self.fix:
+                                # Try to fix the problem by overwriting the pipeline file
+                                shutil.copy(_tf(f), _pf(f))
+                                passed.append(f"`{f}` matches the template")
+                                fixed.append(f"`{f}` overwritten with template file")
+                            else:
+                                failed.append(f"`{f}` does not match the template")
+                                could_fix = True
+                    except FileNotFoundError:
+                        pass
 
     # cleaning up temporary dir
     shutil.rmtree(tmp_dir)