Fix patch generation for module() calls missing version or using 0.0.0

.gitignore

@@ -11,5 +11,6 @@ user.bazelrc
 
 # Python
 .venv
+venv/
 __pycache__/
 /.coverage

src/registry_manager/bazel_wrapper.py

@@ -127,15 +127,21 @@ def try_parse_metadata_json(metadata_json: Path) -> BazelModuleInfo | None:
 def parse_MODULE_file_content(content: str) -> ModuleFileContent:  # noqa: N802
     """Parse the content of a MODULE.bazel file.
 
-    Extracts version and compatibility_level using regex patterns.
+    Extracts version and compatibility_level using regex patterns,
+    scoped to the module() call only to avoid false matches in
+    bazel_dep() or other function calls.
     """
     comp_level = None
-    if m_cl := re.search(r"compatibility_level\s*=\s*(\d+)", content):
-        comp_level = int(m_cl.group(1))
-
     version = None
-    if m_ver := re.search(r"version\s*=\s*['\"]([^'\"]+)['\"]", content):
-        version = str(m_ver.group(1))
+
+    # Only extract version and compatibility_level from within the module() call.
+    # Using DOTALL so the pattern matches even if the module() call spans multiple lines.
+    if module_match := re.search(r"\bmodule\s*\(([^)]*)\)", content, re.DOTALL):
+        module_args = module_match.group(1)
+        if m_cl := re.search(r"\bcompatibility_level\s*=\s*(\d+)", module_args):
+            comp_level = int(m_cl.group(1))
+        if m_ver := re.search(r"\bversion\s*=\s*['\"]([^'\"]+)['\"]", module_args):
+            version = str(m_ver.group(1))
 
     return ModuleFileContent(
         content=content,
@@ -275,6 +281,11 @@ def _create_patch_for_module_version_if_mismatch(self) -> str | None:
         compatibility_level than the release, a patch is created to stamp
         the correct version.
 
+        Handles two special cases:
+        - No version in module() call: the version attribute is added.
+        - Version "0.0.0" / no compatibility_level: compatibility_level defaults
+          to 0 in Bazel, so a missing compatibility_level is treated as 0.
+
         Note: this is based on rather fragile regex replacements and may need
         adjustments for more complex MODULE.bazel files.
         Example that would fail:
@@ -284,39 +295,85 @@ def _create_patch_for_module_version_if_mismatch(self) -> str | None:
         if not self.info.mod_file:
             raise ValueError("Module file content not available")
 
-        # Check if no patch is needed
-        if (
-            self.info.mod_file.version == self.info.release.version
-            and self.info.mod_file.major_version == self.info.mod_file.comp_level
-        ):
+        # Expected compatibility_level based on release version's major.
+        # None when the release version is not a valid semver.
+        expected_comp_level: int | None = None
+        if self.info.release.version.semver:
+            expected_comp_level = self.info.release.version.semver.major
+
+        # Compatibility_level is considered correct when:
+        # - It explicitly matches the expected value, OR
+        # - It is absent (None) and the expected value is also None (non-semver
+        #   release) or 0 (Bazel's default compatibility_level is 0).
+        comp_level_ok = self.info.mod_file.comp_level == expected_comp_level or (
+            self.info.mod_file.comp_level is None
+            and (expected_comp_level is None or expected_comp_level == 0)
+        )
+
+        # Version is considered correct when it is present and matches the release.
+        version_ok = (
+            self.info.mod_file.version is not None
+            and self.info.mod_file.version == self.info.release.version
+        )
+
+        if version_ok and comp_level_ok:
             log.debug("MODULE.bazel version matches release version; no patch needed.")
             return None  # No patch needed
 
         # Build metadata strings for logging
         file_meta = f"(version={self.info.mod_file.version}, comp_level={self.info.mod_file.comp_level})"
-        release_meta = f"(version={self.info.release.version}, comp_level={self.info.mod_file.major_version})"
+        release_meta = f"(version={self.info.release.version}, comp_level={expected_comp_level})"
         log.info(
             f"MODULE.bazel {file_meta} doesn't match release {release_meta}; creating patch"
         )
 
-        # Create patched content by replacing version
-        stamped_content = re.sub(
-            r"(version\s*=\s*['\"])([^'\"]+)(['\"])",
-            lambda m: f"{m.group(1)}{self.info.release.version}{m.group(3)}",
-            self.info.mod_file.content,
-            count=1,
-        )
-
-        if self.info.release.version.semver:
-            major_version = self.info.release.version.semver.major
+        release_version_str = str(self.info.release.version)
+        stamped_content = self.info.mod_file.content
 
-            # Replace compatibility_level with major version
+        # Stamp version inside the module() call.
+        if self.info.mod_file.version is None:
+            # version= is absent from module(): add it before the closing paren.
+            # The pattern strips any optional trailing comma + whitespace before ')'
+            # so we always get a clean ", version = '...'" insertion.
             stamped_content = re.sub(
-                r"(compatibility_level\s*=\s*)(\d+)",
-                lambda m: f"{m.group(1)}{major_version}",
+                r"(\bmodule\s*\([^)]*?),?\s*\)",
+                lambda m: f'{m.group(1)}, version = "{release_version_str}")',
                 stamped_content,
                 count=1,
+                flags=re.DOTALL,
             )
+        else:
+            # version= is present: replace its value, scoped to the module() call.
+            stamped_content = re.sub(
+                r"(\bmodule\s*\([^)]*?\bversion\s*=\s*['\"])([^'\"]+)(['\"])",
+                lambda m: f"{m.group(1)}{release_version_str}{m.group(3)}",
+                stamped_content,
+                count=1,
+                flags=re.DOTALL,
+            )
+
+        # Stamp compatibility_level inside the module() call (only for semver releases).
+        if expected_comp_level is not None and not comp_level_ok:
+            if self.info.mod_file.comp_level is not None:
+                # compatibility_level= is present: replace its value, scoped to
+                # the module() call.
+                stamped_content = re.sub(
+                    r"(\bmodule\s*\([^)]*?\bcompatibility_level\s*=\s*)(\d+)",
+                    lambda m: f"{m.group(1)}{expected_comp_level}",
+                    stamped_content,
+                    count=1,
+                    flags=re.DOTALL,
+                )
+            elif expected_comp_level > 0:
+                # compatibility_level= is absent and the expected value is non-zero
+                # (0 is Bazel's default so we do not need to add it explicitly).
+                stamped_content = re.sub(
+                    r"(\bmodule\s*\([^)]*?),?\s*\)",
+                    lambda m: f"{m.group(1)}, compatibility_level = {expected_comp_level})",
+                    stamped_content,
+                    count=1,
+                    flags=re.DOTALL,
+                )
 
         # Generate unified diff patch
         patch_text = "".join(

src/registry_manager/version.py

@@ -26,7 +26,8 @@ def __lt__(self, other: "Version") -> bool:
             return self._raw < other._raw
 
     def __eq__(self, other: object) -> bool:
-        assert isinstance(other, Version)
+        if not isinstance(other, Version):
+            return NotImplemented
         # Note: this intentionally compares the raw strings, not the semver objects.
         # e.g. technically "1.0.0" and "001.00.0000" are equivalent semver versions,
         # but we want to treat them as different versions.

tests/conftest.py

@@ -149,14 +149,17 @@ def make_update_info(
     comp_level: int | None = None,
     module_name: str = "score_demo",
     existing_versions: list[str] | None = None,
+    module_content: ModuleFileContent | None = None,
 ) -> ModuleUpdateInfo:
-    if module_version is None:
-        module_version = version
+    if module_content is None:
+        if module_version is None:
+            module_version = version
+        module_content = make_module_content(version=module_version, comp_level=comp_level)
 
     return ModuleUpdateInfo(
         module=make_module_info(name=module_name, versions=existing_versions or []),
         release=make_release_info(version=version),
-        mod_file=make_module_content(version=module_version, comp_level=comp_level),
+        mod_file=module_content,
     )
 
 

tests/test_file_generation.py

@@ -18,7 +18,7 @@
 from unittest.mock import patch
 
 import pytest
-from src.registry_manager.bazel_wrapper import ModuleUpdateRunner
+from src.registry_manager.bazel_wrapper import ModuleUpdateRunner, parse_MODULE_file_content
 
 from tests.conftest import make_update_info
 
@@ -212,3 +212,118 @@ def test_appends_version_with_semver_sorting(
         generated_versions = metadata["versions"]
         expected_versions = ["1.0.11", "1.0.10", "1.0.9"]
         assert generated_versions == expected_versions
+
+    def test_no_patch_when_zero_version_matches_and_no_comp_level(
+        self,
+        basic_registry_setup: Callable[..., None],
+    ) -> None:
+        """Release 0.0.0 with no compatibility_level in module() must not generate a patch.
+
+        Bazel's default for compatibility_level is 0, so a missing
+        compatibility_level is equivalent to compatibility_level=0.  When the
+        release version is 0.0.0 (major=0), no patching is required.
+        """
+        basic_registry_setup()
+        os.chdir("/")
+
+        # MODULE.bazel has version="0.0.0" but no compatibility_level attribute
+        mod_content = parse_MODULE_file_content('module(name="score_demo", version="0.0.0")')
+        update_info = make_update_info(version="0.0.0", module_content=mod_content)
+
+        runner = ModuleUpdateRunner(update_info)
+        with patch(
+            "src.registry_manager.bazel_wrapper.sha256_from_url",
+            return_value="sha256-test",
+        ):
+            runner.generate_files()
+
+        # No patches section should appear in source.json
+        source_path = Path("/modules/score_demo/0.0.0/source.json")
+        source = json.loads(source_path.read_text())
+        assert "patches" not in source
+
+    def test_creates_patch_when_module_has_no_version(
+        self,
+        basic_registry_setup: Callable[..., None],
+    ) -> None:
+        """When module() has no version= attribute the patch must add it.
+
+        This reproduces the bug from PR #326 where the regex was matching
+        version= inside a bazel_dep() call instead of the module() call,
+        producing a wrong patch.
+        """
+        basic_registry_setup()
+        os.chdir("/")
+
+        # MODULE.bazel like the one in PR #326: module() has no version,
+        # but a bazel_dep() further down does.
+        raw_content = (
+            'module(name = "score_demo")\n'
+            'bazel_dep(name = "platforms", version = "1.0.0")\n'
+        )
+        mod_content = parse_MODULE_file_content(raw_content)
+        # Sanity check: the parse must NOT pick up the bazel_dep version
+        assert mod_content.version is None
+
+        update_info = make_update_info(version="0.1.4", module_content=mod_content)
+
+        runner = ModuleUpdateRunner(update_info)
+        with patch(
+            "src.registry_manager.bazel_wrapper.sha256_from_url",
+            return_value="sha256-test",
+        ):
+            runner.generate_files()
+
+        patches_dir = Path("/modules/score_demo/0.1.4/patches")
+        assert patches_dir.exists()
+        patch_file = patches_dir / "module_dot_bazel_version.patch"
+        assert patch_file.exists()
+
+        patch_content = patch_file.read_text()
+
+        # The patch must add version="0.1.4" to the module() call
+        assert 'version = "0.1.4"' in patch_content
+
+        # The bazel_dep version line must appear only as context (space-prefixed),
+        # not as a removal (-) or addition (+)
+        for line in patch_content.splitlines():
+            if "bazel_dep" in line:
+                assert line.startswith(" "), (
+                    f"bazel_dep line must not be modified, got: {line!r}"
+                )
+
+        # The patched MODULE.bazel must have the version inside module()
+        mod = Path("/modules/score_demo/0.1.4/MODULE.bazel").read_text()
+        assert 'version = "0.1.4"' in mod
+        # The bazel_dep line must be unchanged
+        assert 'bazel_dep(name = "platforms", version = "1.0.0")' in mod
+
+    def test_creates_patch_when_module_has_no_version_and_major_nonzero(
+        self,
+        basic_registry_setup: Callable[..., None],
+    ) -> None:
+        """When module() has no version= and the release major version is > 0,
+        the patch must add both version and compatibility_level."""
+        basic_registry_setup()
+        os.chdir("/")
+
+        raw_content = 'module(name = "score_demo")\n'
+        mod_content = parse_MODULE_file_content(raw_content)
+
+        update_info = make_update_info(version="1.0.0", module_content=mod_content)
+
+        runner = ModuleUpdateRunner(update_info)
+        with patch(
+            "src.registry_manager.bazel_wrapper.sha256_from_url",
+            return_value="sha256-test",
+        ):
+            runner.generate_files()
+
+        patches_dir = Path("/modules/score_demo/1.0.0/patches")
+        assert patches_dir.exists()
+        patch_file = patches_dir / "module_dot_bazel_version.patch"
+        assert patch_file.exists()
+
+        patch_content = patch_file.read_text()
+        assert 'version = "1.0.0"' in patch_content
+        assert "compatibility_level = 1" in patch_content

tests/test_module_parsing.py

@@ -35,3 +35,39 @@ def test_parse_module_file_content_preserved(self):
         content = 'module(name="test", version="1.0.0")\nbazel_dep(...)'
         parsed = parse_MODULE_file_content(content)
         assert parsed.content == content
+
+    def test_parse_ignores_bazel_dep_version(self):
+        """version= in bazel_dep() must not be confused with the module version."""
+        content = 'module(name = "score_demo")\nbazel_dep(name = "platforms", version = "1.0.0")'
+        parsed = parse_MODULE_file_content(content)
+        assert parsed.version is None
+        assert parsed.comp_level is None
+
+    def test_parse_multiline_module_call(self):
+        """Multiline module() call is parsed correctly."""
+        content = (
+            'module(\n'
+            '    name = "score_demo",\n'
+            '    version = "2.0.0",\n'
+            '    compatibility_level = 2,\n'
+            ')\n'
+            'bazel_dep(name = "platforms", version = "0.1.0")\n'
+        )
+        parsed = parse_MODULE_file_content(content)
+        assert str(parsed.version) == "2.0.0"
+        assert parsed.comp_level == 2
+
+    def test_parse_zero_version(self):
+        """Version 0.0.0 and compatibility_level 0 are parsed correctly."""
+        content = 'module(name="score_demo", version="0.0.0", compatibility_level=0)'
+        parsed = parse_MODULE_file_content(content)
+        assert str(parsed.version) == "0.0.0"
+        assert parsed.comp_level == 0
+        assert parsed.major_version == 0
+
+    def test_parse_version_without_comp_level(self):
+        """version= present but no compatibility_level= → comp_level is None."""
+        content = 'module(name="score_demo", version="0.0.0")'
+        parsed = parse_MODULE_file_content(content)
+        assert str(parsed.version) == "0.0.0"
+        assert parsed.comp_level is None

tests/test_version.py

@@ -20,3 +20,19 @@ def test_version_smaller():
     assert Version("1.0.0") < Version("2.0.0")
     assert Version("1.0.0-alpha") < Version("1.0.0")
     assert Version("A") < Version("B")
+
+
+def test_version_equality():
+    assert Version("1.0.0") == Version("1.0.0")
+    assert not (Version("1.0.0") == Version("2.0.0"))
+
+
+def test_version_equality_with_non_version():
+    """Version.__eq__ must not crash when compared to non-Version objects."""
+    # Python evaluates to False (via NotImplemented fallback) for mismatched types
+    assert (Version("1.0.0") == None) is False  # noqa: E711
+    assert (Version("0.0.0") == None) is False  # noqa: E711
+    assert (Version("1.0.0") == 1) is False
+    assert (Version("1.0.0") == "1.0.0") is False
+    # Comparison must also work in the reverse direction
+    assert (None == Version("1.0.0")) is False  # noqa: E711