From 09935346023a6eb86e1b6f9e5963e346d6fb4d2e Mon Sep 17 00:00:00 2001 From: Ronnie Dutta <61982285+MetRonnie@users.noreply.github.com> Date: Fri, 12 Jul 2024 12:06:10 +0100 Subject: [PATCH] Check for invalid fragment names --- docs/cli.rst | 5 ++ docs/configuration.rst | 7 +++ docs/tutorial.rst | 5 ++ src/towncrier/_builder.py | 23 +++++++++ src/towncrier/_settings/load.py | 1 + src/towncrier/build.py | 16 +++++- src/towncrier/check.py | 4 +- src/towncrier/newsfragments/622.feature.rst | 3 ++ src/towncrier/test/test_build.py | 57 +++++++++++++++++++++ src/towncrier/test/test_check.py | 20 ++++++++ 10 files changed, 139 insertions(+), 2 deletions(-) create mode 100644 src/towncrier/newsfragments/622.feature.rst diff --git a/docs/cli.rst b/docs/cli.rst index 6adbb94d..8faf0551 100644 --- a/docs/cli.rst +++ b/docs/cli.rst @@ -64,6 +64,11 @@ also remove the fragments directory if now empty. Don't delete news fragments after the build and don't ask for confirmation whether to delete or keep the fragments. +.. option:: --strict + + Fail if there are any news fragments that have invalid filenames. + This is automatically turned on if ``build_ignore_filenames`` has been set in the configuration. + ``towncrier create`` -------------------- diff --git a/docs/configuration.rst b/docs/configuration.rst index aa4edfca..c3749738 100644 --- a/docs/configuration.rst +++ b/docs/configuration.rst @@ -130,6 +130,13 @@ Top level keys ``true`` by default. +``build_ignore_filenames`` + A list of filenames in the news fragments directory to ignore when building the news file. + + If this is configured, it turns on the ``--strict`` build mode which will fail if there are any news fragment files not in this list that have invalid filenames. Note that if a template is used, it should also be included here. + + ``None`` by default. + Extra top level keys for Python projects ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/docs/tutorial.rst b/docs/tutorial.rst index 70b9c410..88a49db5 100644 --- a/docs/tutorial.rst +++ b/docs/tutorial.rst @@ -47,6 +47,11 @@ Create this folder:: The ``.gitignore`` will remain and keep Git from not tracking the directory. +If needed, you can also specify a list of filenames for ``towncrier`` to ignore in the news fragments directory:: + + [tool.towncrier] + build_ignore_filenames = ["README.rst"] + Detecting Version ----------------- diff --git a/src/towncrier/_builder.py b/src/towncrier/_builder.py index e7fbf12b..1c075478 100644 --- a/src/towncrier/_builder.py +++ b/src/towncrier/_builder.py @@ -12,6 +12,7 @@ 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,22 @@ def __call__(self, section_directory: str = "") -> str: def find_fragments( base_directory: str, config: Config, + strict: bool | None, ) -> tuple[Mapping[str, Mapping[tuple[str, str, int], str]], list[tuple[str, str]]]: """ Sections are a dictonary of section names to paths. + + In strict mode, raise ClickException if any fragments have an invalid name. """ + if strict is None: + # If strict mode is not set, turn it on only if build_ignore_filenames is set + # (this maintains backward compatibility). + strict = config.build_ignore_filenames is not None + + ignored_files = {".gitignore"} + if config.build_ignore_filenames: + ignored_files.update(config.build_ignore_filenames) + get_section_path = FragmentsPath(base_directory, config) content = {} @@ -129,10 +142,20 @@ def find_fragments( file_content = {} for basename in files: + # Skip files that are deliberately ignored + if basename 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 " + "'build_ignore_filenames' 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 724a7768..9ca2a593 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 + build_ignore_filenames: list[str] | None = None class ConfigError(ClickException): diff --git a/src/towncrier/build.py b/src/towncrier/build.py index bf7cd350..139072e3 100644 --- a/src/towncrier/build.py +++ b/src/towncrier/build.py @@ -106,6 +106,17 @@ def _validate_answer(ctx: Context, param: Option, value: bool) -> bool: help="Do not ask for confirmations. But keep news fragments.", callback=_validate_answer, ) +@click.option( + "--strict", + "strict", + default=None, + flag_value=True, + help=( + "Fail if there are any news fragments that have invalid filenames (this is " + "automatically turned on if 'build_ignore_filenames' has been set in the " + "configuration)." + ), +) def _main( draft: bool, directory: str | None, @@ -115,6 +126,7 @@ def _main( project_date: str | None, answer_yes: bool, answer_keep: bool, + strict: bool | None, ) -> None: """ Build a combined news file from news fragment. @@ -129,6 +141,7 @@ def _main( project_date, answer_yes, answer_keep, + strict, ) except ConfigError as e: print(e, file=sys.stderr) @@ -144,6 +157,7 @@ def __main( project_date: str | None, answer_yes: bool, answer_keep: bool, + strict: bool | None, ) -> None: """ The main entry point. @@ -178,7 +192,7 @@ 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, strict) 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 a519f9ae..8b057545 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 00000000..6845c471 --- /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 ``build_ignore_filenames`` 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. diff --git a/src/towncrier/test/test_build.py b/src/towncrier/test/test_build.py index 5da7d828..1efa7495 100644 --- a/src/towncrier/test/test_build.py +++ b/src/towncrier/test/test_build.py @@ -1530,3 +1530,60 @@ def test_orphans_in_non_showcontent_markdown(self, runner): self.assertEqual(0, result.exit_code, result.output) self.assertEqual(expected_output, result.output) + + @with_project( + config=""" + [tool.towncrier] + package = "foo" + build_ignore_filenames = ["template.jinja", "CAPYBARAS.md"] + """ + ) + def test_invalid_fragment_names(self, runner): + """ + When build_ignore_filenames is set, files with those names are ignored. + """ + opts = ["--draft", "--date", "01-01-2001", "--version", "1.0.0"] + # Valid filename: + with open("foo/newsfragments/123.feature", "w") as f: + f.write("Adds levitation") + # Files that should be ignored: + with open("foo/newsfragments/template.jinja", "w") as f: + f.write("Jinja template") + with open("foo/newsfragments/CAPYBARAS.md", "w") as f: + f.write("Peanut butter") + # Automatically ignored: + with open("foo/newsfragments/.gitignore", "w") as f: + f.write("!.gitignore") + + result = runner.invoke(_main, opts) + # Should succeed + self.assertEqual(0, result.exit_code, result.output) + + # Invalid filename: + with open("foo/newsfragments/feature.124", "w") as f: + f.write("Extends levitation") + + result = runner.invoke(_main, opts) + # Should now fail + self.assertEqual(1, result.exit_code, result.output) + self.assertIn("Invalid news fragment name: feature.124", result.output) + + @with_project() + def test_invalid_fragment_names_strict(self, runner): + """ + When using --strict, any invalid filenames will cause an error even if + build_ignore_filenames is NOT set. + """ + opts = ["--draft", "--date", "01-01-2001", "--version", "1.0.0"] + # Invalid filename: + with open("foo/newsfragments/feature.124", "w") as f: + f.write("Extends levitation") + + result = runner.invoke(_main, opts) + # Should succeed in normal mode + self.assertEqual(0, result.exit_code, result.output) + + result = runner.invoke(_main, [*opts, "--strict"]) + # Should now fail + self.assertEqual(1, result.exit_code, result.output) + self.assertIn("Invalid news fragment name: feature.124", result.output) diff --git a/src/towncrier/test/test_check.py b/src/towncrier/test/test_check.py index 23db3264..d73966ee 100644 --- a/src/towncrier/test/test_check.py +++ b/src/towncrier/test/test_check.py @@ -470,3 +470,23 @@ def test_in_different_dir_with_nondefault_newsfragments_directory(self, runner): self.assertTrue( result.output.endswith("No new newsfragments found on this branch.\n") ) + + @with_isolated_runner + def test_invalid_fragment_name(self, runner): + create_project("pyproject.toml") + opts = ["--compare-with", "main"] + + write("foo/bar.py", "# Scorpions!") + write("foo/newsfragments/123.feature", "Adds scorpions") + write("foo/newsfragments/.gitignore", "!.gitignore") + commit("add stuff") + + result = runner.invoke(towncrier_check, opts) + self.assertEqual(0, result.exit_code, result.output) + + # Make invalid filename: + os.rename("foo/newsfragments/123.feature", "foo/newsfragments/feature.123") + + result = runner.invoke(towncrier_check, opts) + self.assertEqual(1, result.exit_code, result.output) + self.assertIn("Invalid news fragment name: feature.123", result.output)