CM-53811: unified commit range parse

Author: gotbadger

cycode/cli/apps/scan/commit_range_scanner.py

@@ -26,8 +26,7 @@
     get_diff_file_path,
     get_pre_commit_modified_documents,
     get_safe_head_reference_for_diff,
-    parse_commit_range_sast,
-    parse_commit_range_sca,
+    parse_commit_range,
 )
 from cycode.cli.files_collector.documents_walk_ignore import filter_documents_with_cycodeignore
 from cycode.cli.files_collector.file_excluder import excluder
@@ -187,7 +186,7 @@ def _scan_commit_range_documents(
 def _scan_sca_commit_range(ctx: typer.Context, repo_path: str, commit_range: str, **_) -> None:
     scan_parameters = get_scan_parameters(ctx, (repo_path,))
 
-    from_commit_rev, to_commit_rev = parse_commit_range_sca(commit_range, repo_path)
+    from_commit_rev, to_commit_rev = parse_commit_range(commit_range, repo_path)
     from_commit_documents, to_commit_documents, _ = get_commit_range_modified_documents(
         ctx.obj['progress_bar'], ScanProgressBarSection.PREPARE_LOCAL_FILES, repo_path, from_commit_rev, to_commit_rev
     )
@@ -228,7 +227,7 @@ def _scan_secret_commit_range(
 def _scan_sast_commit_range(ctx: typer.Context, repo_path: str, commit_range: str, **_) -> None:
     scan_parameters = get_scan_parameters(ctx, (repo_path,))
 
-    from_commit_rev, to_commit_rev = parse_commit_range_sast(commit_range, repo_path)
+    from_commit_rev, to_commit_rev = parse_commit_range(commit_range, repo_path)
     _, commit_documents, diff_documents = get_commit_range_modified_documents(
         ctx.obj['progress_bar'],
         ScanProgressBarSection.PREPARE_LOCAL_FILES,

cycode/cli/files_collector/commit_range_documents.py

@@ -408,22 +408,7 @@ def get_pre_commit_modified_documents(
     return git_head_documents, pre_committed_documents, diff_documents
 
 
-def parse_commit_range_sca(commit_range: str, path: str) -> tuple[Optional[str], Optional[str]]:
-    # FIXME(MarshalX): i truly believe that this function does NOT work as expected
-    #  it does not handle cases like 'A..B' correctly
-    #  i leave it as it for SCA to not break anything
-    #  the more correct approach is implemented for SAST
-    from_commit_rev = to_commit_rev = None
-
-    for commit in git_proxy.get_repo(path).iter_commits(rev=commit_range):
-        if not to_commit_rev:
-            to_commit_rev = commit.hexsha
-        from_commit_rev = commit.hexsha
-
-    return from_commit_rev, to_commit_rev
-
-
-def parse_commit_range_sast(commit_range: str, path: str) -> tuple[Optional[str], Optional[str]]:
+def parse_commit_range(commit_range: str, path: str) -> tuple[Optional[str], Optional[str]]:
     """Parses a git commit range string and returns the full SHAs for the 'from' and 'to' commits.
 
     Supports:

tests/cli/files_collector/test_commit_range_documents.py

@@ -14,6 +14,7 @@
     calculate_pre_push_commit_range,
     get_diff_file_path,
     get_safe_head_reference_for_diff,
+    parse_commit_range,
     parse_pre_push_input,
 )
 from cycode.cli.utils.path_utils import get_path_by_os
@@ -22,7 +23,8 @@
 @contextmanager
 def git_repository(path: str) -> Generator[Repo, None, None]:
     """Context manager for Git repositories that ensures proper cleanup on Windows."""
-    repo = Repo.init(path)
+    # Ensure the initialized repository uses 'main' as the default branch
+    repo = Repo.init(path, b='main')
     try:
         yield repo
     finally:
@@ -539,8 +541,8 @@ def test_calculate_range_for_new_branch_with_merge_base(self) -> None:
             repo.index.add(['feature.py'])
             feature_commit = repo.index.commit('Add feature')
 
-            # Switch back to master to simulate we're pushing a feature branch
-            repo.heads.master.checkout()
+            # Switch back to the default branch to simulate pushing the feature branch
+            repo.heads.main.checkout()
 
             # Test new branch push
             push_details = f'refs/heads/feature {feature_commit.hexsha} refs/heads/feature {consts.EMPTY_COMMIT_SHA}'
@@ -805,3 +807,67 @@ def test_simulate_pre_push_hook_input_format(self) -> None:
                     parts = push_input.split()
                     expected_range = f'{parts[3]}..{parts[1]}'
                     assert commit_range == expected_range
+
+
+class TestParseCommitRange:
+    """Tests to validate unified parse_commit_range behavior matches git semantics."""
+
+    def _make_linear_history(self, repo: Repo, base_dir: str) -> tuple[str, str, str]:
+        """Create three linear commits A -> B -> C and return their SHAs."""
+        a_file = os.path.join(base_dir, 'a.txt')
+        with open(a_file, 'w') as f:
+            f.write('A')
+        repo.index.add(['a.txt'])
+        a = repo.index.commit('A')
+
+        with open(a_file, 'a') as f:
+            f.write('B')
+        repo.index.add(['a.txt'])
+        b = repo.index.commit('B')
+
+        with open(a_file, 'a') as f:
+            f.write('C')
+        repo.index.add(['a.txt'])
+        c = repo.index.commit('C')
+
+        return a.hexsha, b.hexsha, c.hexsha
+
+    def test_two_dot_linear_history(self) -> None:
+        """For 'A..C', expect (A,C) in linear history."""
+        with temporary_git_repository() as (temp_dir, repo):
+            a, b, c = self._make_linear_history(repo, temp_dir)
+
+            parsed_from, parsed_to = parse_commit_range(f'{a}..{c}', temp_dir)
+            assert (parsed_from, parsed_to) == (a, c)
+
+    def test_three_dot_linear_history(self) -> None:
+        """For 'A...C' in linear history, expect (A,C)."""
+        with temporary_git_repository() as (temp_dir, repo):
+            a, b, c = self._make_linear_history(repo, temp_dir)
+
+            parsed_from, parsed_to = parse_commit_range(f'{a}...{c}', temp_dir)
+            assert (parsed_from, parsed_to) == (a, c)
+
+    def test_open_right_linear_history(self) -> None:
+        """For 'A..', expect (A,HEAD=C)."""
+        with temporary_git_repository() as (temp_dir, repo):
+            a, b, c = self._make_linear_history(repo, temp_dir)
+
+            parsed_from, parsed_to = parse_commit_range(f'{a}..', temp_dir)
+            assert (parsed_from, parsed_to) == (a, c)
+
+    def test_open_left_linear_history(self) -> None:
+        """For '..C' where HEAD==C, expect (HEAD=C,C)."""
+        with temporary_git_repository() as (temp_dir, repo):
+            a, b, c = self._make_linear_history(repo, temp_dir)
+
+            parsed_from, parsed_to = parse_commit_range(f'..{c}', temp_dir)
+            assert (parsed_from, parsed_to) == (c, c)
+
+    def test_single_commit_spec(self) -> None:
+        """For 'A', expect (A,HEAD=C)."""
+        with temporary_git_repository() as (temp_dir, repo):
+            a, b, c = self._make_linear_history(repo, temp_dir)
+
+            parsed_from, parsed_to = parse_commit_range(a, temp_dir)
+            assert (parsed_from, parsed_to) == (a, c)