Repository: callowayproject/bump-my-version (python)

Issue:
Constant strings interpreted as regex
* bump-my-version version: 0.9.3
* Python version: 3.11.15
* Operating System: macOS

### Description

In some circumstances, while I deactivated regex mode everywhere (both in config file and on CLI invokation), the search string gets interpreted as a regex and has unwanted side-effects.

### What I Did

Starting from a `changelog.md` file containing:

```md
# Changelog

## [0.0.1 (unreleased)](https://cool.url)

- Test unreleased package.
```

With a `pyproject.toml` file alongside containing:
```toml
[tool.bumpversion]
current_version = "0.0.1"
allow_dirty = true

[[tool.bumpversion.files]]
filename = "./changelog.md"
no_regex = true
search = "(unreleased)"
replace = "({now:%Y-%m-%d})"
```

Now let's invoke this CLI:
```shell-session
$ bump-my-version replace --verbose --dry-run --no-regex --no-configured-files --search "(unreleased)" --replace "({now:%Y-%m-%d})" ./changelog.md
Starting BumpVersion 0.9.3                                                                                                                                                                                          
Reading config file pyproject.toml:                                                                                                                                                                                 
Parsing version '0.0.1' using regexp '(?P<major>\d+)\.(?P<minor>\d+)\.(?P<patch>\d+)'                                                                                                                               
Parsed the following values: major=0, minor=0, patch=1                                                                                                                                                              
Asserting files ./changelog.md contain the version string...                                                                                                                                                        
Found 're.compile('(unreleased)', re.MULTILINE|re.DOTALL)' in ./changelog.md at line 3: unreleased                                                                                                                  
Would change file ./changelog.md:                                                                                                                                                                                   
*** before ./changelog.md                                                                                                                                                                                           
--- after ./changelog.md                                                                                                                                                                                            
***************                                                                                                                                                                                                     
*** 1,5 ****                                                                                                                                                                                                        
  # Changelog                                                                                                                                                                                                       
                                                                                                                                                                                                                    
! ## [0.0.1 (unreleased)](https://cool.url)                                                                                                                                                                         
                                                                                                                                                                                                                    
! - Test unreleased package.                                                                                                                                                                                        
--- 1,5 ----                                                                                                                                                                                                        
  # Changelog                                                                                                                                                                                                       
                                                                                                                                                                                                                    
! ## [0.0.1 ((2023-08-29))](https://cool.url)                                                                                                                                                                       
                                                                                                                                                                                                                    
! - Test (2023-08-29) package.     
```

You can see here how the `(unreleased)` search string is interpreted as a regex and has unwanted side effect in `changelog.md` file.

I expect here to have the final result of that file to be:
```shell-session
# Changelog                                                                                                                                                                                                       
                                                                                                                                                                                                                    
## [0.0.1 (2023-08-29)](https://cool.url)                                                                                                                                                                       
                                                                                                                                                                                                                    
- Test unreleased package.     
```
Instead of:
```shell-session
# Changelog                                                                                                                                                                                                       
                                                                                                                                                                                                                    
## [0.0.1 ((2023-08-29))](https://cool.url)                                                                                                                                                                       
                                                                                                                                                                                                                    
- Test (2023-08-29) package.     
```

I have a hard time understanding what's happening.

Maybe there is a difference between in the way strings passed in the CLI are not parsed like those in the config file.

Maybe duplicating the replace action in both the config file and the CLI is messing with some internals.

Or maybe the `--no-configured-files` is not well-named, as I expect it to ignore the directives from any configuration file found. In which case what I am looking for is probably a `--no-config` option.

Pull request description:
Refactored file resolution, inclusion, and exclusion
- Fixes #61
- Config now includes `resolved_filemap` property
- resolved filemap exapands all globs
- Config now includes `files_to_modify` property
- files to modify resolves inclusions and exclutions
- Improved Config.add_files property

Patch:
diff --git a/bumpversion/bump.py b/bumpversion/bump.py
index 62359afd..7ad24af3 100644
--- a/bumpversion/bump.py
+++ b/bumpversion/bump.py
@@ -79,7 +79,7 @@ def do_bump(
 
     ctx = get_context(config, version, next_version)
 
-    configured_files = resolve_file_config(config.files, config.version_config)
+    configured_files = resolve_file_config(config.files_to_modify, config.version_config)
     modify_files(configured_files, version, next_version, ctx, dry_run)
     update_config_file(config_file, config.current_version, next_version_str, dry_run)
 
diff --git a/bumpversion/cli.py b/bumpversion/cli.py
index deb6e50c..b746b406 100644
--- a/bumpversion/cli.py
+++ b/bumpversion/cli.py
@@ -9,7 +9,7 @@ from bumpversion import __version__
 from bumpversion.aliases import AliasedGroup
 from bumpversion.bump import do_bump
 from bumpversion.config import find_config_file, get_configuration
-from bumpversion.files import modify_files, resolve_file_config
+from bumpversion.files import ConfiguredFile, modify_files
 from bumpversion.logging import setup_logging
 from bumpversion.show import do_show, log_list
 from bumpversion.ui import print_warning
@@ -300,10 +300,11 @@ def bump(
         config.scm_info.tool.assert_nondirty()
 
     if no_configured_files:
-        config.files = []
+        config.excluded_paths = list(config.resolved_filemap.keys())
 
     if files:
         config.add_files(files)
+        config.included_paths = files
 
     do_bump(version_part, new_version, config, found_config_file, dry_run)
 
@@ -495,13 +496,17 @@ def replace(
         config.scm_info.tool.assert_nondirty()
 
     if no_configured_files:
-        config.files = []
+        config.excluded_paths = list(config.resolved_filemap.keys())
 
     if files:
         config.add_files(files)
+        config.included_paths = files
 
-    version = config.version_config.parse(config.current_version)
+    configured_files = [
+        ConfiguredFile(file_cfg, config.version_config, search, replace) for file_cfg in config.files_to_modify
+    ]
 
+    version = config.version_config.parse(config.current_version)
     if new_version:
         next_version = config.version_config.parse(new_version)
     else:
@@ -509,6 +514,4 @@ def replace(
 
     ctx = get_context(config, version, next_version)
 
-    configured_files = resolve_file_config(config.files, config.version_config, search, replace)
-
     modify_files(configured_files, version, next_version, ctx, dry_run)
diff --git a/bumpversion/config.py b/bumpversion/config.py
index fc346e72..5e8daa2f 100644
--- a/bumpversion/config.py
+++ b/bumpversion/config.py
@@ -1,6 +1,7 @@
 """Configuration management."""
 from __future__ import annotations
 
+import glob
 import itertools
 import logging
 import re
@@ -66,6 +67,8 @@ class Config(BaseSettings):
     scm_info: Optional["SCMInfo"]
     parts: Dict[str, VersionPartConfig]
     files: List[FileConfig]
+    included_paths: List[str] = []
+    excluded_paths: List[str] = []
 
     class Config:
         env_prefix = "bumpversion_"
@@ -73,7 +76,42 @@ class Config(BaseSettings):
     def add_files(self, filename: Union[str, List[str]]) -> None:
         """Add a filename to the list of files."""
         filenames = [filename] if isinstance(filename, str) else filename
-        self.files.extend([FileConfig(filename=name) for name in filenames])  # type: ignore[call-arg]
+        for name in filenames:
+            if name in self.resolved_filemap:
+                continue
+            self.files.append(
+                FileConfig(
+                    filename=name,
+                    glob=None,
+                    parse=self.parse,
+                    serialize=self.serialize,
+                    search=self.search,
+                    replace=self.replace,
+                    no_regex=self.no_regex,
+                    ignore_missing_version=self.ignore_missing_version,
+                )
+            )
+
+    @property
+    def resolved_filemap(self) -> Dict[str, FileConfig]:
+        """Return a map of filenames to file configs, expanding any globs."""
+        new_files = []
+        for file_cfg in self.files:
+            if file_cfg.glob:
+                new_files.extend(get_glob_files(file_cfg))
+            else:
+                new_files.append(file_cfg)
+
+        return {file_cfg.filename: file_cfg for file_cfg in new_files}
+
+    @property
+    def files_to_modify(self) -> List[FileConfig]:
+        """Return a list of files to modify."""
+        files_not_excluded = [
+            file_cfg.filename for file_cfg in self.files if file_cfg.filename not in self.excluded_paths
+        ]
+        inclusion_set = set(self.included_paths) | set(files_not_excluded)
+        return [file_cfg for file_cfg in self.files if file_cfg.filename in inclusion_set]
 
     @property
     def version_config(self) -> "VersionConfig":
@@ -410,3 +448,22 @@ def update_config_file(
 
     if not dry_run:
         config_path.write_text(new_config)
+
+
+def get_glob_files(file_cfg: FileConfig) -> List[FileConfig]:
+    """
+    Return a list of files that match the glob pattern.
+
+    Args:
+        file_cfg: The file configuration containing the glob pattern
+
+    Returns:
+        A list of resolved file configurations according to the pattern.
+    """
+    files = []
+    for filename_glob in glob.glob(file_cfg.glob, recursive=True):
+        new_file_cfg = file_cfg.copy()
+        new_file_cfg.filename = filename_glob
+        new_file_cfg.glob = None
+        files.append(new_file_cfg)
+    return files
diff --git a/bumpversion/files.py b/bumpversion/files.py
index 254bdfe9..546a07f9 100644
--- a/bumpversion/files.py
+++ b/bumpversion/files.py
@@ -1,5 +1,4 @@
 """Methods for changing files."""
-import glob
 import logging
 import re
 from copy import deepcopy
@@ -190,14 +189,7 @@ def resolve_file_config(
     Returns:
         A list of ConfiguredFiles
     """
-    configured_files = []
-    for file_cfg in files:
-        if file_cfg.glob:
-            configured_files.extend(get_glob_files(file_cfg, version_config))
-        else:
-            configured_files.append(ConfiguredFile(file_cfg, version_config, search, replace))
-
-    return configured_files
+    return [ConfiguredFile(file_cfg, version_config, search, replace) for file_cfg in files]
 
 
 def modify_files(
@@ -222,29 +214,6 @@ def modify_files(
         f.replace_version(current_version, new_version, context, dry_run)
 
 
-def get_glob_files(
-    file_cfg: FileConfig, version_config: VersionConfig, search: Optional[str] = None, replace: Optional[str] = None
-) -> List[ConfiguredFile]:
-    """
-    Return a list of files that match the glob pattern.
-
-    Args:
-        file_cfg: The file configuration containing the glob pattern
-        version_config: The version configuration
-        search: The search pattern to use instead of any configured search pattern
-        replace: The replace pattern to use instead of any configured replace pattern
-
-    Returns:
-        A list of resolved files according to the pattern.
-    """
-    files = []
-    for filename_glob in glob.glob(file_cfg.glob, recursive=True):
-        new_file_cfg = file_cfg.copy()
-        new_file_cfg.filename = filename_glob
-        files.append(ConfiguredFile(new_file_cfg, version_config, search, replace))
-    return files
-
-
 def _check_files_contain_version(
     files: List[ConfiguredFile], current_version: Version, context: MutableMapping
 ) -> None:


Test patch:
diff --git a/tests/fixtures/basic_cfg_expected.txt b/tests/fixtures/basic_cfg_expected.txt
index 97d0203a..771b0735 100644
--- a/tests/fixtures/basic_cfg_expected.txt
+++ b/tests/fixtures/basic_cfg_expected.txt
@@ -2,6 +2,7 @@
  'commit': True,
  'commit_args': None,
  'current_version': '1.0.0',
+ 'excluded_paths': [],
  'files': [{'filename': 'setup.py',
             'glob': None,
             'ignore_missing_version': False,
@@ -30,6 +31,7 @@
             'serialize': ['{major}.{minor}.{patch}-{release}',
                           '{major}.{minor}.{patch}']}],
  'ignore_missing_version': False,
+ 'included_paths': [],
  'message': 'Bump version: {current_version} → {new_version}',
  'no_regex': False,
  'parse': '(?P<major>\\d+)\\.(?P<minor>\\d+)\\.(?P<patch>\\d+)(\\-(?P<release>[a-z]+))?',
diff --git a/tests/fixtures/basic_cfg_expected.yaml b/tests/fixtures/basic_cfg_expected.yaml
index fd991f24..99f0b64d 100644
--- a/tests/fixtures/basic_cfg_expected.yaml
+++ b/tests/fixtures/basic_cfg_expected.yaml
@@ -2,6 +2,8 @@ allow_dirty: false
 commit: true
 commit_args: null
 current_version: "1.0.0"
+excluded_paths:
+
 files:
   - filename: "setup.py"
     glob: null
@@ -34,6 +36,8 @@ files:
       - "{major}.{minor}.{patch}-{release}"
       - "{major}.{minor}.{patch}"
 ignore_missing_version: false
+included_paths:
+
 message: "Bump version: {current_version} → {new_version}"
 no_regex: false
 parse: "(?P<major>\\d+)\\.(?P<minor>\\d+)\\.(?P<patch>\\d+)(\\-(?P<release>[a-z]+))?"
diff --git a/tests/fixtures/basic_cfg_expected_full.json b/tests/fixtures/basic_cfg_expected_full.json
index 91421eef..0e0ba289 100644
--- a/tests/fixtures/basic_cfg_expected_full.json
+++ b/tests/fixtures/basic_cfg_expected_full.json
@@ -3,6 +3,7 @@
   "commit": true,
   "commit_args": null,
   "current_version": "1.0.0",
+  "excluded_paths": [],
   "files": [
     {
       "filename": "setup.py",
@@ -45,6 +46,7 @@
     }
   ],
   "ignore_missing_version": false,
+  "included_paths": [],
   "message": "Bump version: {current_version} \u2192 {new_version}",
   "no_regex": false,
   "parse": "(?P<major>\\d+)\\.(?P<minor>\\d+)\\.(?P<patch>\\d+)(\\-(?P<release>[a-z]+))?",
diff --git a/tests/test_cli.py b/tests/test_cli.py
index 3e1cd122..951654c4 100644
--- a/tests/test_cli.py
+++ b/tests/test_cli.py
@@ -201,12 +201,14 @@ def test_listing_with_version_part(tmp_path: Path, fixtures_path: Path):
         "DEPRECATED: The --list option is deprecated and will be removed in a future version.",
         "new_version=1.0.1-dev",
         "current_version=1.0.0",
+        "excluded_paths=[]",
         "parse=(?P<major>\\d+)\\.(?P<minor>\\d+)\\.(?P<patch>\\d+)(\\-(?P<release>[a-z]+))?",
         "serialize=['{major}.{minor}.{patch}-{release}', '{major}.{minor}.{patch}']",
         "search={current_version}",
         "replace={new_version}",
         "no_regex=False",
         "ignore_missing_version=False",
+        "included_paths=[]",
         "tag=True",
         "sign_tags=False",
         "tag_name=v{new_version}",
@@ -254,12 +256,14 @@ def test_listing_without_version_part(tmp_path: Path, fixtures_path: Path):
         "",
         "DEPRECATED: The --list option is deprecated and will be removed in a future version.",
         "current_version=1.0.0",
+        "excluded_paths=[]",
         "parse=(?P<major>\\d+)\\.(?P<minor>\\d+)\\.(?P<patch>\\d+)(\\-(?P<release>[a-z]+))?",
         "serialize=['{major}.{minor}.{patch}-{release}', '{major}.{minor}.{patch}']",
         "search={current_version}",
         "replace={new_version}",
         "no_regex=False",
         "ignore_missing_version=False",
+        "included_paths=[]",
         "tag=True",
         "sign_tags=False",
         "tag_name=v{new_version}",
@@ -517,3 +521,52 @@ def test_replace_search_with_plain_string(tmp_path, fixtures_path):
         print(traceback.print_exception(result.exc_info[1]))
 
     assert result.exit_code == 0
+
+
+def test_valid_regex_not_ignoring_regex(tmp_path: Path, caplog) -> None:
+    """A search string not meant to be a regex (but is) is still found and replaced correctly."""
+    # Arrange
+    search = "(unreleased)"
+    replace = "(2023-01-01)"
+
+    version_path = tmp_path / "VERSION"
+    version_path.write_text("# Changelog\n\n## [0.0.1 (unreleased)](https://cool.url)\n\n- Test unreleased package.\n")
+    config_file = tmp_path / ".bumpversion.toml"
+    config_file.write_text(
+        "[tool.bumpversion]\n"
+        'current_version = "0.0.1"\n'
+        "allow_dirty = true\n\n"
+        "[[tool.bumpversion.files]]\n"
+        'filename = "VERSION"\n'
+        "no_regex = true\n"
+        f'search = "{search}"\n'
+        f'replace = "{replace}"\n'
+    )
+
+    # Act
+    runner: CliRunner = CliRunner()
+    with inside_dir(tmp_path):
+        result: Result = runner.invoke(
+            cli.cli,
+            [
+                "replace",
+                "--verbose",
+                "--no-regex",
+                "--no-configured-files",
+                "--search",
+                search,
+                "--replace",
+                replace,
+                "VERSION",
+            ],
+        )
+
+    # Assert
+    if result.exit_code != 0:
+        print(result.output)
+
+    assert result.exit_code == 0
+    assert (
+        version_path.read_text()
+        == "# Changelog\n\n## [0.0.1 (2023-01-01)](https://cool.url)\n\n- Test unreleased package.\n"
+    )
diff --git a/tests/test_config.py b/tests/test_config.py
index 950d0c78..8e926baf 100644
--- a/tests/test_config.py
+++ b/tests/test_config.py
@@ -9,7 +9,7 @@ from click.testing import CliRunner, Result
 from pytest import param
 
 from bumpversion import config
-from tests.conftest import inside_dir
+from tests.conftest import inside_dir, get_config_data
 
 
 @pytest.fixture(params=[".bumpversion.cfg", "setup.cfg"])
@@ -246,3 +246,31 @@ def test_pep440_config(git_repo: Path, fixtures_path: Path):
         # assert result.exit_code == 0
         # cfg = config.get_configuration(cfg_path)
         # assert cfg.current_version == "1.0.0.dev1+myreallylongbranchna"
+
+
+@pytest.mark.parametrize(
+    ["glob_pattern", "file_list"],
+    [
+        param("*.txt", {Path("file1.txt"), Path("file2.txt")}, id="simple-glob"),
+        param("**/*.txt", {Path("file1.txt"), Path("file2.txt"), Path("directory/file3.txt")}, id="recursive-glob"),
+    ],
+)
+def test_get_glob_files(glob_pattern: str, file_list: set, fixtures_path: Path):
+    """Get glob files should return all the globbed files and nothing else."""
+    overrides = {
+        "current_version": "1.0.0",
+        "parse": r"(?P<major>\d+)\.(?P<minor>\d+)(\.(?P<release>[a-z]+))?",
+        "serialize": ["{major}.{minor}.{release}", "{major}.{minor}"],
+        "files": [
+            {
+                "glob": glob_pattern,
+            }
+        ],
+    }
+    conf, version_config, current_version = get_config_data(overrides)
+    with inside_dir(fixtures_path.joinpath("glob")):
+        result = config.get_glob_files(conf.files[0])
+
+    assert len(result) == len(file_list)
+    for f in result:
+        assert Path(f.filename) in file_list
diff --git a/tests/test_files.py b/tests/test_files.py
index 96fb6a03..ff2b716b 100644
--- a/tests/test_files.py
+++ b/tests/test_files.py
@@ -15,34 +15,6 @@ from bumpversion.version_part import VersionConfig
 from tests.conftest import get_config_data, inside_dir
 
 
-@pytest.mark.parametrize(
-    ["glob_pattern", "file_list"],
-    [
-        param("*.txt", {Path("file1.txt"), Path("file2.txt")}, id="simple-glob"),
-        param("**/*.txt", {Path("file1.txt"), Path("file2.txt"), Path("directory/file3.txt")}, id="recursive-glob"),
-    ],
-)
-def test_get_glob_files(glob_pattern: str, file_list: set, fixtures_path: Path):
-    """Get glob files should return all the globbed files and nothing else."""
-    overrides = {
-        "current_version": "1.0.0",
-        "parse": r"(?P<major>\d+)\.(?P<minor>\d+)(\.(?P<release>[a-z]+))?",
-        "serialize": ["{major}.{minor}.{release}", "{major}.{minor}"],
-        "files": [
-            {
-                "glob": glob_pattern,
-            }
-        ],
-    }
-    conf, version_config, current_version = get_config_data(overrides)
-    with inside_dir(fixtures_path.joinpath("glob")):
-        result = files.get_glob_files(conf.files[0], version_config)
-
-    assert len(result) == len(file_list)
-    for f in result:
-        assert Path(f.path) in file_list
-
-
 def test_single_file_processed_twice(tmp_path: Path):
     """
     Verify that a single file "file2" can be processed twice.
@@ -264,7 +236,6 @@ def test_search_replace_to_avoid_updating_unconcerned_lines(tmp_path: Path, capl
       standardized open source project CHANGELOG.
     """
     )
-    print(caplog.text)
     assert req_path.read_text() == "Django>=1.5.6,<1.6\nMyProject==1.6.0"
     assert changelog_path.read_text() == expected_chglog