fix: comment creation should not be possible for renamed and moved files

cmd/app/comment_helpers.go

@@ -23,6 +23,7 @@ type LineRange struct {
 /* PositionData represents the position of a comment or note (relative to a file diff) */
 type PositionData struct {
 	FileName       string     `json:"file_name"`
+	OldFileName    string     `json:"old_file_name"`
 	NewLine        *int       `json:"new_line,omitempty"`
 	OldLine        *int       `json:"old_line,omitempty"`
 	HeadCommitSHA  string     `json:"head_commit_sha"`
@@ -41,13 +42,19 @@ type RequestWithPosition interface {
 func buildCommentPosition(commentWithPositionData RequestWithPosition) *gitlab.PositionOptions {
 	positionData := commentWithPositionData.GetPositionData()
 
+	// If the file has been renamed, then this is a relevant part of the payload
+	oldFileName := positionData.OldFileName
+	if oldFileName == "" {
+		oldFileName = positionData.FileName
+	}
+
 	opt := &gitlab.PositionOptions{
 		PositionType: &positionData.Type,
 		StartSHA:     &positionData.StartCommitSHA,
 		HeadSHA:      &positionData.HeadCommitSHA,
 		BaseSHA:      &positionData.BaseCommitSHA,
 		NewPath:      &positionData.FileName,
-		OldPath:      &positionData.FileName,
+		OldPath:      &oldFileName,
 		NewLine:      positionData.NewLine,
 		OldLine:      positionData.OldLine,
 	}

lua/gitlab/actions/comment.lua

@@ -80,6 +80,7 @@ local confirm_create_comment = function(text, visual_range, unlinked, discussion
   local revision = state.MR_REVISIONS[1]
   local position_data = {
     file_name = reviewer_data.file_name,
+    old_file_name = reviewer_data.old_file_name,
     base_commit_sha = revision.base_commit_sha,
     start_commit_sha = revision.start_commit_sha,
     head_commit_sha = revision.head_commit_sha,
@@ -179,6 +180,12 @@ M.create_comment_layout = function(opts)
       return
     end
 
+    -- Check that the file has not been renamed
+    if reviewer.is_file_renamed() and not reviewer.does_file_have_changes() then
+      u.notify("Commenting on (unchanged) renamed or moved files is not supported", vim.log.levels.ERROR)
+      return
+    end
+
     -- Check that we are hovering over the code
     local filetype = vim.bo[0].filetype
     if not opts.reply and (filetype == "DiffviewFiles" or filetype == "gitlab") then
@@ -249,7 +256,6 @@ end
 --- This function will open a comment popup in order to create a comment on the changed/updated
 --- line in the current MR
 M.create_comment = function()
-  vim.print("Creating comment...")
   local has_clean_tree, err = git.has_clean_tree()
   if err ~= nil then
     return
@@ -293,7 +299,6 @@ end
 --- This function will open a a popup to create a "note" (e.g. unlinked comment)
 --- on the changed/updated line in the current MR
 M.create_note = function()
-  vim.print("Creating note...")
   local layout = M.create_comment_layout({ ranged = false, unlinked = true })
   if layout ~= nil then
     layout:mount()

lua/gitlab/annotations.lua

@@ -114,6 +114,7 @@
 ---@class DiffviewInfo
 ---@field modification_type string
 ---@field file_name string
+---@field old_file_name string -- Relevant for renamed files
 ---@field current_bufnr integer
 ---@field new_sha_win_id integer
 ---@field old_sha_win_id integer

lua/gitlab/hunks.lua

@@ -90,18 +90,28 @@ local line_was_added = function(linnr, hunk, all_diff_output)
 end
 
 ---Parse git diff hunks.
----@param file_path string Path to file.
 ---@param base_sha string Git base SHA of merge request.
 ---@return HunksAndDiff
-local parse_hunks_and_diff = function(file_path, base_sha)
+local parse_hunks_and_diff = function(base_sha)
   local hunks = {}
   local all_diff_output = {}
 
-  local Job = require("plenary.job")
+  local reviewer = require("gitlab.reviewer")
+  local cmd = {
+    "diff",
+    "--minimal",
+    "--unified=0",
+    "--no-color",
+    base_sha,
+    "--",
+    reviewer.get_current_file_oldpath(),
+    reviewer.get_current_file_path(),
+  }
 
+  local Job = require("plenary.job")
   local diff_job = Job:new({
     command = "git",
-    args = { "diff", "--minimal", "--unified=0", "--no-color", base_sha, "--", file_path },
+    args = cmd,
     on_exit = function(j, return_code)
       if return_code == 0 then
         all_diff_output = j:result()
@@ -139,8 +149,7 @@ end
 
 --- Processes the number of changes until the target is reached. This returns
 --- a negative or positive number indicating the number of lines in the hunk
---that have been added or removed prior to the target line
----comment
+--- that have been added or removed prior to the target line
 ---@param line_number number
 ---@param hunk Hunk
 ---@param lines table
@@ -221,11 +230,10 @@ end
 ---This is in order to build the payload for Gitlab correctly by setting the old line and new line.
 ---@param old_line number|nil
 ---@param new_line number|nil
----@param current_file string
 ---@param is_current_sha_focused boolean
 ---@return string|nil
-function M.get_modification_type(old_line, new_line, current_file, is_current_sha_focused)
-  local hunk_and_diff_data = parse_hunks_and_diff(current_file, state.INFO.diff_refs.base_sha)
+function M.get_modification_type(old_line, new_line, is_current_sha_focused)
+  local hunk_and_diff_data = parse_hunks_and_diff(state.INFO.diff_refs.base_sha)
   if hunk_and_diff_data.hunks == nil then
     return
   end
@@ -240,11 +248,19 @@ end
 ---@param old_sha string
 ---@param new_sha string
 ---@param file_path string
+---@param old_file_path string
 ---@param line_number number
 ---@return number|nil
-M.calculate_matching_line_new = function(old_sha, new_sha, file_path, line_number)
+M.calculate_matching_line_new = function(old_sha, new_sha, file_path, old_file_path, line_number)
   local net_change = 0
-  local diff_cmd = string.format("git diff --minimal --unified=0 --no-color %s %s -- %s", old_sha, new_sha, file_path)
+  local diff_cmd = string.format(
+    "git diff --minimal --unified=0 --no-color %s %s -- %s %s",
+    old_sha,
+    new_sha,
+    old_file_path,
+    file_path
+  )
+
   local handle = io.popen(diff_cmd)
   if handle == nil then
     u.notify(string.format("Error running git diff command for %s", file_path), vim.log.levels.ERROR)

lua/gitlab/indicators/common.lua

@@ -41,7 +41,7 @@ M.filter_placeable_discussions = function()
     draft_notes = {}
   end
 
-  local file = reviewer.get_current_file()
+  local file = reviewer.get_current_file_path()
   if not file then
     return {}
   end

lua/gitlab/reviewer/init.lua

@@ -152,7 +152,7 @@ M.get_reviewer_data = function()
     return
   end
 
-  local current_file = M.get_current_file()
+  local current_file = M.get_current_file_path()
   if current_file == nil then
     u.notify("Error getting current file from Diffview", vim.log.levels.ERROR)
     return
@@ -163,7 +163,7 @@ M.get_reviewer_data = function()
 
   local is_current_sha_focused = M.is_current_sha_focused()
 
-  local modification_type = hunks.get_modification_type(old_line, new_line, current_file, is_current_sha_focused)
+  local modification_type = hunks.get_modification_type(old_line, new_line, is_current_sha_focused)
   if modification_type == nil then
     u.notify("Error getting modification type", vim.log.levels.ERROR)
     return
@@ -180,6 +180,7 @@ M.get_reviewer_data = function()
 
   return {
     file_name = layout.a.file.path,
+    old_file_name = M.is_file_renamed() and layout.b.file.path or "",
     old_line_from_buf = old_line,
     new_line_from_buf = new_line,
     modification_type = modification_type,
@@ -205,14 +206,38 @@ M.is_current_sha_focused = function()
   return current_win == b_win
 end
 
----Get currently shown file
----@return string|nil
-M.get_current_file = function()
+---Get currently shown file data
+M.get_current_file_data = function()
   local view = diffview_lib.get_current_view()
-  if not view or not view.panel or not view.panel.cur_file then
-    return
-  end
-  return view.panel.cur_file.path
+  return view and view.panel and view.panel.cur_file
+end
+
+---Get currently shown file path
+---@return string|nil
+M.get_current_file_path = function()
+  local file_data = M.get_current_file_data()
+  return file_data and file_data.path
+end
+
+---Get currently shown file's old path
+---@return string|nil
+M.get_current_file_oldpath = function()
+  local file_data = M.get_current_file_data()
+  return file_data and file_data.oldpath
+end
+
+---Tell whether current file is renamed or not
+---@return boolean|nil
+M.is_file_renamed = function()
+  local file_data = M.get_current_file_data()
+  return file_data and file_data.status == "R"
+end
+
+---Tell whether current file has changes or not
+---@return boolean|nil
+M.does_file_have_changes = function()
+  local file_data = M.get_current_file_data()
+  return file_data.stats.additions > 0 or file_data.stats.deletions > 0
 end
 
 ---Diffview exposes events which can be used to setup autocommands.

lua/gitlab/reviewer/location.lua

@@ -96,7 +96,13 @@ function Location:get_line_number_from_new_sha(line)
     return line
   end
   -- Otherwise we want to get the matching line in the opposite buffer
-  return hunks.calculate_matching_line_new(self.base_sha, self.head_sha, self.reviewer_data.file_name, line)
+  return hunks.calculate_matching_line_new(
+    self.base_sha,
+    self.head_sha,
+    self.reviewer_data.file_name,
+    self.reviewer_data.old_file_name,
+    line
+  )
 end
 
 -- Returns the matching line from the old SHA.
@@ -112,7 +118,13 @@ function Location:get_line_number_from_old_sha(line)
   end
 
   -- Otherwise we want to get the matching line in the opposite buffer
-  return hunks.calculate_matching_line_new(self.head_sha, self.base_sha, self.reviewer_data.file_name, line)
+  return hunks.calculate_matching_line_new(
+    self.head_sha,
+    self.base_sha,
+    self.reviewer_data.file_name,
+    self.reviewer_data.old_file_name,
+    line
+  )
 end
 
 -- Returns the current line number from whatever SHA (new or old)
@@ -135,7 +147,7 @@ end
 ---@param visual_range LineRange
 ---@return ReviewerLineInfo|nil
 function Location:set_start_range(visual_range)
-  local current_file = require("gitlab.reviewer").get_current_file()
+  local current_file = require("gitlab.reviewer").get_current_file_path()
   if current_file == nil then
     u.notify("Error getting current file from Diffview", vim.log.levels.ERROR)
     return
@@ -165,7 +177,7 @@ function Location:set_start_range(visual_range)
     return
   end
 
-  local modification_type = hunks.get_modification_type(old_line, new_line, current_file, is_current_sha_focused)
+  local modification_type = hunks.get_modification_type(old_line, new_line, is_current_sha_focused)
   if modification_type == nil then
     u.notify("Error getting modification type for start of range", vim.log.levels.ERROR)
     return
@@ -182,7 +194,7 @@ end
 -- for the Gitlab payload
 ---@param visual_range LineRange
 function Location:set_end_range(visual_range)
-  local current_file = require("gitlab.reviewer").get_current_file()
+  local current_file = require("gitlab.reviewer").get_current_file_path()
   if current_file == nil then
     u.notify("Error getting current file from Diffview", vim.log.levels.ERROR)
     return
@@ -207,7 +219,7 @@ function Location:set_end_range(visual_range)
 
   local reviewer = require("gitlab.reviewer")
   local is_current_sha_focused = reviewer.is_current_sha_focused()
-  local modification_type = hunks.get_modification_type(old_line, new_line, current_file, is_current_sha_focused)
+  local modification_type = hunks.get_modification_type(old_line, new_line, is_current_sha_focused)
   if modification_type == nil then
     u.notify("Error getting modification type for end of range", vim.log.levels.ERROR)
     return