Repository: twisted/towncrier

Issue:
Check for invalid filenames in `changes.d/`
Although `towncrier create` enforces valid fragment file names, creating a fragment file manually can sometimes lead to mistakes in the file name. It would be useful to include in our CI a check that all files in `changes.d/` have valid fragment file names (excluding `changelog-template.jinja` of course).

Note we don't use `towncrier check` as we don't create fragments for small or non-user-facing changes.

Discussion:
MetRonnie: I think all feedback has been addressed now. Let me know if there is anything else
adiroiban: > Maybe a warning if towncrier build encounters invalid filenames but build_ignore_filenames has not been configured?

That would be nice. We can also have this in a separate PR.
This should not be a blocker.

-------------

> Do you envisage deprecating the --strict option and making the fail behaviour the default in a future version of Towncrier?

I hope that we don't have to add the `--strict` option... so that we don't have to deprecate it.

It would be nice to have a warinig raised when `build_ignore_filenames` is not configured and make a release that raised the warning.

In a followup release, we can then change the default value of  `build_ignore_filenames` so thta it will alwasy do a strict check by default... while still allowing users to configure it with ` `build_ignore_filenames = None`
adiroiban: I have updated the PR description to note that it will also fix #426  ... the #619 might be a duplicate 
adiroiban: Thanks for the update. I have enabled auto-merge.

I hope this will be merged soon.

I plan to do a new release soon... I hope #627  will also be merge soon and then we can have a release.


Patch:
diff --git a/docs/configuration.rst b/docs/configuration.rst
index aa4edfc..955f62e 100644
--- a/docs/configuration.rst
+++ b/docs/configuration.rst
@@ -130,6 +130,15 @@ Top level keys
 
     ``true`` by default.
 
+``ignore``
+    A case-insensitive list of filenames in the news fragments directory to ignore.
+
+    ``towncrier check`` will fail if there are any news fragment files that have invalid filenames, except for those in the list. ``towncrier build`` will likewise fail, but only if this list has been configured (set to an empty list if there are no files to ignore).
+
+    Some filenames such as ``.gitignore`` and ``README`` are automatically ignored. However, if a custom template is stored in the news fragment directory, you should add it to this list.
+
+    ``None`` by default.
+
 Extra top level keys for Python projects
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
diff --git a/docs/tutorial.rst b/docs/tutorial.rst
index 70b9c41..a126eff 100644
--- a/docs/tutorial.rst
+++ b/docs/tutorial.rst
@@ -45,8 +45,6 @@ Create this folder::
    # This makes sure that Git will never delete the empty folder
    $ echo '!.gitignore' > src/myproject/newsfragments/.gitignore
 
-The ``.gitignore`` will remain and keep Git from not tracking the directory.
-
 
 Detecting Version
 -----------------
diff --git a/src/towncrier/_builder.py b/src/towncrier/_builder.py
index e7fbf12..62a7d3f 100644
--- a/src/towncrier/_builder.py
+++ b/src/towncrier/_builder.py
@@ -12,6 +12,7 @@ from collections import defaultdict
 from pathlib import Path
 from typing import Any, DefaultDict, Iterable, Iterator, Mapping, NamedTuple, Sequence
 
+from click import ClickException
 from jinja2 import Template
 
 from towncrier._settings.load import Config
@@ -106,10 +107,17 @@ class FragmentsPath:
 def find_fragments(
     base_directory: str,
     config: Config,
+    strict: bool,
 ) -> tuple[Mapping[str, Mapping[tuple[str, str, int], str]], list[tuple[str, str]]]:
     """
     Sections are a dictonary of section names to paths.
+
+    If strict, raise ClickException if any fragments have an invalid name.
     """
+    ignored_files = {".gitignore", ".keep", "readme", "readme.md", "readme.rst"}
+    if config.ignore:
+        ignored_files.update(filename.lower() for filename in config.ignore)
+
     get_section_path = FragmentsPath(base_directory, config)
 
     content = {}
@@ -129,10 +137,19 @@ def find_fragments(
         file_content = {}
 
         for basename in files:
+            if basename.lower() in ignored_files:
+                continue
+
             issue, category, counter = parse_newfragment_basename(
                 basename, config.types
             )
             if category is None:
+                if strict and issue is None:
+                    raise ClickException(
+                        f"Invalid news fragment name: {basename}\n"
+                        "If this filename is deliberate, add it to "
+                        "'ignore' in your configuration."
+                    )
                 continue
             assert issue is not None
             assert counter is not None
diff --git a/src/towncrier/_settings/load.py b/src/towncrier/_settings/load.py
index 724a776..e282a39 100644
--- a/src/towncrier/_settings/load.py
+++ b/src/towncrier/_settings/load.py
@@ -54,6 +54,7 @@ class Config:
     orphan_prefix: str = "+"
     create_eof_newline: bool = True
     create_add_extension: bool = True
+    ignore: list[str] | None = None
 
 
 class ConfigError(ClickException):
diff --git a/src/towncrier/build.py b/src/towncrier/build.py
index bf7cd35..546ef11 100644
--- a/src/towncrier/build.py
+++ b/src/towncrier/build.py
@@ -178,7 +178,13 @@ def __main(
 
     click.echo("Finding news fragments...", err=to_err)
 
-    fragment_contents, fragment_files = find_fragments(base_directory, config)
+    fragment_contents, fragment_files = find_fragments(
+        base_directory,
+        config,
+        # Fail if any fragment filenames are invalid only if ignore list is set
+        # (this maintains backward compatibility):
+        strict=(config.ignore is not None),
+    )
     fragment_filenames = [filename for (filename, _category) in fragment_files]
 
     click.echo("Rendering news fragments...", err=to_err)
diff --git a/src/towncrier/check.py b/src/towncrier/check.py
index a519f9a..8b05754 100644
--- a/src/towncrier/check.py
+++ b/src/towncrier/check.py
@@ -101,12 +101,14 @@ def __main(
         click.echo(f"{n}. {change}")
     click.echo("----")
 
+    # This will fail if any fragment files have an invalid name:
+    all_fragment_files = find_fragments(base_directory, config, strict=True)[1]
+
     news_file = os.path.normpath(os.path.join(base_directory, config.filename))
     if news_file in files:
         click.echo("Checks SKIPPED: news file changes detected.")
         sys.exit(0)
 
-    all_fragment_files = find_fragments(base_directory, config)[1]
     fragments = set()  # will only include fragments of types that are checked
     unchecked_fragments = set()  # will include fragments of types that are not checked
     for fragment_filename, category in all_fragment_files:
diff --git a/src/towncrier/newsfragments/622.feature.rst b/src/towncrier/newsfragments/622.feature.rst
new file mode 100644
index 0000000..6ca64a7
--- /dev/null
+++ b/src/towncrier/newsfragments/622.feature.rst
@@ -0,0 +1,3 @@
+``towncrier check`` will now fail if any news fragments have invalid filenames.
+
+Added a new configuration option called ``ignore`` that allows you to specify a list of filenames that should be ignored. If this is set, ``towncrier build`` will also fail if any filenames are invalid, except for those in the list.


Test patch:
diff --git a/src/towncrier/test/test_build.py b/src/towncrier/test/test_build.py
index 5141fe3..c14fd58 100644
--- a/src/towncrier/test/test_build.py
+++ b/src/towncrier/test/test_build.py
@@ -1583,3 +1583,66 @@ class TestCli(TestCase):
                 """
             ),
         )
+
+    @with_project(
+        config="""
+        [tool.towncrier]
+        package = "foo"
+        ignore = ["template.jinja", "CAPYBARAS.md"]
+        """
+    )
+    def test_ignored_files(self, runner):
+        """
+        When `ignore` is set in config, files with those names are ignored.
+        """
+        with open("foo/newsfragments/123.feature", "w") as f:
+            f.write("This has valid filename (control case)")
+        with open("foo/newsfragments/template.jinja", "w") as f:
+            f.write("This template has been manually ignored")
+        with open("foo/newsfragments/CAPYBARAS.md", "w") as f:
+            f.write("This markdown file has been manually ignored")
+        with open("foo/newsfragments/.gitignore", "w") as f:
+            f.write("gitignore is automatically ignored")
+
+        result = runner.invoke(
+            _main, ["--draft", "--date", "01-01-2001", "--version", "1.0.0"]
+        )
+        self.assertEqual(0, result.exit_code, result.output)
+
+    @with_project(
+        config="""
+        [tool.towncrier]
+        package = "foo"
+        ignore = []
+        """
+    )
+    def test_invalid_fragment_name(self, runner):
+        """
+        When `ignore` is set in config, invalid filenames cause failure.
+        """
+        with open("foo/newsfragments/123.feature", "w") as f:
+            f.write("This has valid filename (control case)")
+        with open("foo/newsfragments/feature.124", "w") as f:
+            f.write("This has the issue and category the wrong way round")
+
+        result = runner.invoke(
+            _main, ["--draft", "--date", "01-01-2001", "--version", "1.0.0"]
+        )
+        self.assertEqual(1, result.exit_code, result.output)
+        self.assertIn("Invalid news fragment name: feature.124", result.output)
+
+    @with_project()
+    def test_no_ignore_configured(self, runner):
+        """
+        When `ignore` is not set in config, invalid filenames are skipped.
+
+        This maintains backward compatibility with before we added `ignore`
+        to the configuration spec.
+        """
+        with open("foo/newsfragments/feature.124", "w") as f:
+            f.write("This has the issue and category the wrong way round")
+
+        result = runner.invoke(
+            _main, ["--draft", "--date", "01-01-2001", "--version", "1.0.0"]
+        )
+        self.assertEqual(0, result.exit_code, result.output)
diff --git a/src/towncrier/test/test_builder.py b/src/towncrier/test/test_builder.py
index 1213e1a..31108fd 100644
--- a/src/towncrier/test/test_builder.py
+++ b/src/towncrier/test/test_builder.py
@@ -22,6 +22,10 @@ class TestParseNewsfragmentBasename(TestCase):
             parse_newfragment_basename("README.ext", ["feature"]),
             (None, None, None),
         )
+        self.assertEqual(
+            parse_newfragment_basename("README", ["feature"]),
+            (None, None, None),
+        )
 
     def test_counter(self):
         """<number>.<category>.<counter> generates a custom counter value."""
diff --git a/src/towncrier/test/test_check.py b/src/towncrier/test/test_check.py
index 23db326..7305b11 100644
--- a/src/towncrier/test/test_check.py
+++ b/src/towncrier/test/test_check.py
@@ -470,3 +470,47 @@ class TestChecker(TestCase):
         self.assertTrue(
             result.output.endswith("No new newsfragments found on this branch.\n")
         )
+
+    @with_isolated_runner
+    def test_ignored_files(self, runner):
+        """
+        When `ignore` is set in config, files with those names are ignored.
+        """
+        create_project("pyproject.toml", extra_config='ignore = ["template.jinja"]')
+
+        write(
+            "foo/newsfragments/124.feature",
+            "This fragment has valid name (control case)",
+        )
+        write("foo/newsfragments/template.jinja", "This is manually ignored")
+        write("foo/newsfragments/.gitignore", "gitignore is automatically ignored")
+        commit("add stuff")
+
+        result = runner.invoke(towncrier_check, ["--compare-with", "main"])
+        self.assertEqual(0, result.exit_code, result.output)
+
+    @with_isolated_runner
+    def test_invalid_fragment_name(self, runner):
+        """
+        Fails if a news fragment has an invalid name, even if `ignore` is not set in
+        the config.
+        """
+        create_project("pyproject.toml")
+
+        write(
+            "foo/newsfragments/124.feature",
+            "This fragment has valid name (control case)",
+        )
+        write(
+            "foo/newsfragments/feature.125",
+            "This has issue and category wrong way round",
+        )
+        write(
+            "NEWS.rst",
+            "Modification of news file should not skip check of invalid names",
+        )
+        commit("add stuff")
+
+        result = runner.invoke(towncrier_check, ["--compare-with", "main"])
+        self.assertEqual(1, result.exit_code, result.output)
+        self.assertIn("Invalid news fragment name: feature.125", result.output)