From ccf562bfe9ef1f69035d3a305d6b8e6935438fdc Mon Sep 17 00:00:00 2001 From: Marko Gacesa Date: Thu, 18 May 2023 15:52:19 +0000 Subject: [PATCH] Merge branch 'mg/code-comments/fix-cc-migration' of _OKE5H2PQKOUfzFFDuD4FA/default/CODE/gitness (#85) --- internal/services/codecomments/migrator.go | 36 ++-- .../services/codecomments/migrator_test.go | 163 ++++++++++++++++++ 2 files changed, 185 insertions(+), 14 deletions(-) create mode 100644 internal/services/codecomments/migrator_test.go diff --git a/internal/services/codecomments/migrator.go b/internal/services/codecomments/migrator.go index cde858676..577901c40 100644 --- a/internal/services/codecomments/migrator.go +++ b/internal/services/codecomments/migrator.go @@ -153,22 +153,11 @@ func (migrator *Migrator) migrate( } for _, hunk := range file.HunkHeaders { - hunkStart := hunk.NewLine - hunkEnd := hunk.NewLine + hunk.NewSpan - 1 for _, cc := range codeComments { - commentStart, commentEnd := getCommentStartEnd(cc) - if commentEnd < hunkStart { - continue - } - - outdated := commentStart <= hunkEnd + ccStart, ccEnd := getCommentStartEnd(cc) + outdated, moveDelta := processCodeComment(ccStart, ccEnd, hunk) cc.Outdated = outdated - - if outdated { - continue - } - - updateCommentLine(cc, hunk.NewSpan-hunk.OldSpan) + updateCommentLine(cc, moveDelta) } } } @@ -208,3 +197,22 @@ func mapCodeComments( return commitMap } + +func processCodeComment(ccStart, ccEnd int, h gitrpc.HunkHeader) (outdated bool, moveDelta int) { + // it's outdated if code is changed (old code) inside the code comment or + // if there are added lines inside the code comment, don't care about how many (NewSpan). + outdated = (h.OldSpan > 0 && ccEnd >= h.OldLine && ccStart <= h.OldLine+h.OldSpan-1) || + (h.NewSpan > 0 && h.NewLine > ccStart && h.NewLine <= ccEnd) + + if outdated { + return // outdated comments aren't moved + } + + if ccEnd <= h.OldLine { + return // the change described by the hunk header is below the code comment, so it doesn't affect it + } + + moveDelta = h.NewSpan - h.OldSpan + + return +} diff --git a/internal/services/codecomments/migrator_test.go b/internal/services/codecomments/migrator_test.go new file mode 100644 index 000000000..96c667c18 --- /dev/null +++ b/internal/services/codecomments/migrator_test.go @@ -0,0 +1,163 @@ +// Copyright 2022 Harness Inc. All rights reserved. +// Use of this source code is governed by the Polyform Free Trial License +// that can be found in the LICENSE.md file for this repository. + +package codecomments + +import ( + "testing" + + "github.com/harness/gitness/gitrpc" +) + +func TestProcessCodeComment(t *testing.T) { + // the code comment tested in this unit test spans five lines, from line 20 to line 24 + const ccStart = 20 + const ccEnd = 24 + tests := []struct { + name string + hunk gitrpc.HunkHeader + expOutdated bool + expMoveDelta int + }{ + // only added lines + { + name: "three-lines-added-before-far", + hunk: gitrpc.HunkHeader{OldLine: 10, OldSpan: 0, NewLine: 11, NewSpan: 3}, + expOutdated: false, expMoveDelta: 3, + }, + { + name: "three-lines-added-before-but-touching", + hunk: gitrpc.HunkHeader{OldLine: 19, OldSpan: 0, NewLine: 20, NewSpan: 3}, + expOutdated: false, expMoveDelta: 3, + }, + { + name: "three-lines-added-overlap-at-start", + hunk: gitrpc.HunkHeader{OldLine: 20, OldSpan: 0, NewLine: 21, NewSpan: 3}, + expOutdated: true, expMoveDelta: 0, + }, + { + name: "three-lines-added-inside", + hunk: gitrpc.HunkHeader{OldLine: 21, OldSpan: 0, NewLine: 22, NewSpan: 3}, + expOutdated: true, expMoveDelta: 0, + }, + { + name: "three-lines-added-overlap-at-end", + hunk: gitrpc.HunkHeader{OldLine: 23, OldSpan: 0, NewLine: 24, NewSpan: 3}, + expOutdated: true, expMoveDelta: 0, + }, + { + name: "three-lines-added-after-but-touching", + hunk: gitrpc.HunkHeader{OldLine: 24, OldSpan: 0, NewLine: 25, NewSpan: 3}, + expOutdated: false, expMoveDelta: 0, + }, + { + name: "three-lines-added-after-far", + hunk: gitrpc.HunkHeader{OldLine: 30, OldSpan: 0, NewLine: 31, NewSpan: 3}, + expOutdated: false, expMoveDelta: 0, + }, + // only removed lines + { + name: "three-lines-removed-before-far", + hunk: gitrpc.HunkHeader{OldLine: 10, OldSpan: 3, NewLine: 9, NewSpan: 0}, + expOutdated: false, expMoveDelta: -3, + }, + { + name: "three-lines-removed-before-but-touching", + hunk: gitrpc.HunkHeader{OldLine: 17, OldSpan: 3, NewLine: 16, NewSpan: 0}, + expOutdated: false, expMoveDelta: -3, + }, + { + name: "three-lines-removed-overlap-at-start", + hunk: gitrpc.HunkHeader{OldLine: 18, OldSpan: 3, NewLine: 17, NewSpan: 0}, + expOutdated: true, expMoveDelta: 0, + }, + { + name: "three-lines-removed-inside", + hunk: gitrpc.HunkHeader{OldLine: 21, OldSpan: 3, NewLine: 20, NewSpan: 0}, + expOutdated: true, expMoveDelta: 0, + }, + { + name: "three-lines-removed-overlap-at-end", + hunk: gitrpc.HunkHeader{OldLine: 24, OldSpan: 3, NewLine: 23, NewSpan: 0}, + expOutdated: true, expMoveDelta: 0, + }, + { + name: "three-lines-removed-after-but-touching", + hunk: gitrpc.HunkHeader{OldLine: 25, OldSpan: 3, NewLine: 24, NewSpan: 0}, + expOutdated: false, expMoveDelta: 0, + }, + { + name: "three-lines-removed-after-far", + hunk: gitrpc.HunkHeader{OldLine: 30, OldSpan: 3, NewLine: 29, NewSpan: 0}, + expOutdated: false, expMoveDelta: 0, + }, + // only changed lines + { + name: "three-lines-changed-before-far", + hunk: gitrpc.HunkHeader{OldLine: 10, OldSpan: 3, NewLine: 10, NewSpan: 3}, + expOutdated: false, expMoveDelta: 0, + }, + { + name: "three-lines-changed-before-but-touching", + hunk: gitrpc.HunkHeader{OldLine: 17, OldSpan: 3, NewLine: 17, NewSpan: 3}, + expOutdated: false, expMoveDelta: 0, + }, + { + name: "three-lines-changed-overlap-at-start", + hunk: gitrpc.HunkHeader{OldLine: 18, OldSpan: 3, NewLine: 18, NewSpan: 3}, + expOutdated: true, expMoveDelta: 0, + }, + { + name: "three-lines-changed-inside", + hunk: gitrpc.HunkHeader{OldLine: 21, OldSpan: 3, NewLine: 21, NewSpan: 3}, + expOutdated: true, expMoveDelta: 0, + }, + { + name: "three-lines-changed-overlap-at-end", + hunk: gitrpc.HunkHeader{OldLine: 24, OldSpan: 3, NewLine: 24, NewSpan: 3}, + expOutdated: true, expMoveDelta: 0, + }, + { + name: "three-lines-changed-after-but-touching", + hunk: gitrpc.HunkHeader{OldLine: 25, OldSpan: 3, NewLine: 25, NewSpan: 3}, + expOutdated: false, expMoveDelta: 0, + }, + { + name: "three-lines-changed-after-far", + hunk: gitrpc.HunkHeader{OldLine: 30, OldSpan: 3, NewLine: 30, NewSpan: 3}, + expOutdated: false, expMoveDelta: 0, + }, + // mixed tests + { + name: "two-lines-added-one-changed-just-before", + hunk: gitrpc.HunkHeader{OldLine: 19, OldSpan: 1, NewLine: 19, NewSpan: 3}, + expOutdated: false, expMoveDelta: 2, + }, + { + name: "two-lines-removed-one-added-just-after", + hunk: gitrpc.HunkHeader{OldLine: 25, OldSpan: 2, NewLine: 25, NewSpan: 1}, + expOutdated: false, expMoveDelta: 0, + }, + { + name: "twenty-lines-added-at-line-15", + hunk: gitrpc.HunkHeader{OldLine: 14, OldSpan: 0, NewLine: 15, NewSpan: 20}, + expOutdated: false, expMoveDelta: 20, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + outdated, moveDelta := processCodeComment(ccStart, ccEnd, test.hunk) + + if want, got := test.expOutdated, outdated; want != got { + t.Errorf("outdated mismatch; want=%t got=%t", want, got) + return + } + + if want, got := test.expMoveDelta, moveDelta; want != got { + t.Errorf("moveDelta mismatch; want=%d got=%d", want, got) + } + }) + } +}