Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: add builder for keeping track of references found #76

Merged
merged 11 commits into from
Nov 3, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 2 additions & 14 deletions comments/comments.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,9 +131,7 @@ func ProcessFlags(flagsRef lflags.FlagsRef, flags []ldapi.FeatureFlag, config *l
// sort keys so hashing can work for checking if comment already exists
sort.Strings(addedKeys)
for _, flagKey := range addedKeys {
// If flag is in both added and removed then it is being modified
delete(flagsRef.FlagsRemoved, flagKey)
Comment on lines -134 to -135
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

deduping "modified" flags now happens in Build()

flagAliases := uniqueAliases(flagsRef.FlagsAdded[flagKey])
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove duplicate or empty alias strings is now handled by Build()

flagAliases := flagsRef.FlagsAdded[flagKey]
idx, _ := find(flags, flagKey)
createComment, err := githubFlagComment(flags[idx], flagAliases, true, config)
buildComment.CommentsAdded = append(buildComment.CommentsAdded, createComment)
Expand All @@ -147,7 +145,7 @@ func ProcessFlags(flagsRef lflags.FlagsRef, flags []ldapi.FeatureFlag, config *l
}
sort.Strings(removedKeys)
for _, flagKey := range removedKeys {
flagAliases := uniqueAliases(flagsRef.FlagsRemoved[flagKey])
flagAliases := flagsRef.FlagsRemoved[flagKey]
idx, _ := find(flags, flagKey)
removedComment, err := githubFlagComment(flags[idx], flagAliases, false, config)
buildComment.CommentsRemoved = append(buildComment.CommentsRemoved, removedComment)
Expand Down Expand Up @@ -184,16 +182,6 @@ func uniqueFlagKeys(a, b lflags.FlagAliasMap) []string {
return allKeys
}

func uniqueAliases(aliases lflags.AliasSet) []string {
flagAliases := make([]string, 0, len(aliases))
for alias := range aliases {
if len(strings.TrimSpace(alias)) > 0 {
flagAliases = append(flagAliases, alias)
}
}
return flagAliases
}

func pluralize(str string, strLength int) string {
tmpl := "%d %s"
if strLength != 1 {
Expand Down
16 changes: 8 additions & 8 deletions comments/comments_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ func (e *testFlagEnv) ArchivedRemoved(t *testing.T) {
}

func (e *testCommentBuilder) AddedOnly(t *testing.T) {
e.FlagsRef.FlagsAdded["example-flag"] = lflags.AliasSet{}
e.FlagsRef.FlagsAdded["example-flag"] = []string{}
e.Comments.CommentsAdded = []string{"comment1", "comment2"}
comment := BuildFlagComment(e.Comments, e.FlagsRef, nil)

Expand All @@ -205,8 +205,8 @@ func (e *testCommentBuilder) AddedOnly(t *testing.T) {
}

func (e *testCommentBuilder) RemovedOnly(t *testing.T) {
e.FlagsRef.FlagsRemoved["example-flag"] = lflags.AliasSet{}
e.FlagsRef.FlagsRemoved["sample-flag"] = lflags.AliasSet{}
e.FlagsRef.FlagsRemoved["example-flag"] = []string{}
e.FlagsRef.FlagsRemoved["sample-flag"] = []string{}
e.Comments.CommentsRemoved = []string{"comment1", "comment2"}
comment := BuildFlagComment(e.Comments, e.FlagsRef, nil)

Expand All @@ -215,8 +215,8 @@ func (e *testCommentBuilder) RemovedOnly(t *testing.T) {
}

func (e *testCommentBuilder) AddedAndRemoved(t *testing.T) {
e.FlagsRef.FlagsAdded["example-flag"] = lflags.AliasSet{}
e.FlagsRef.FlagsRemoved["example-flag"] = lflags.AliasSet{}
e.FlagsRef.FlagsAdded["example-flag"] = []string{}
e.FlagsRef.FlagsRemoved["example-flag"] = []string{}
e.Comments.CommentsAdded = []string{"comment1", "comment2"}
e.Comments.CommentsRemoved = []string{"comment1", "comment2"}
comment := BuildFlagComment(e.Comments, e.FlagsRef, nil)
Expand All @@ -228,7 +228,7 @@ func (e *testCommentBuilder) AddedAndRemoved(t *testing.T) {
}

func (e *testProcessor) Basic(t *testing.T) {
e.FlagsRef.FlagsAdded["example-flag"] = lflags.AliasSet{"": true}
e.FlagsRef.FlagsAdded["example-flag"] = []string{}
processor := ProcessFlags(e.FlagsRef, e.Flags, &e.Config)
expected := FlagComments{
CommentsAdded: []string{"| [example flag](https://example.com/test) | `example-flag` | |"},
Expand All @@ -237,8 +237,8 @@ func (e *testProcessor) Basic(t *testing.T) {
}

func (e *testProcessor) Multi(t *testing.T) {
e.FlagsRef.FlagsAdded["example-flag"] = lflags.AliasSet{"": true}
e.FlagsRef.FlagsAdded["second-flag"] = lflags.AliasSet{"": true}
e.FlagsRef.FlagsAdded["example-flag"] = []string{}
e.FlagsRef.FlagsAdded["second-flag"] = []string{}
processor := ProcessFlags(e.FlagsRef, e.Flags, &e.Config)
expected := FlagComments{
CommentsAdded: []string{
Expand Down
56 changes: 9 additions & 47 deletions diff/diff.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (

lflags "github.com/launchdarkly/find-code-references-in-pull-request/flags"
"github.com/launchdarkly/find-code-references-in-pull-request/ignore"
diff_util "github.com/launchdarkly/find-code-references-in-pull-request/internal/util/diff_util"
lsearch "github.com/launchdarkly/ld-find-code-refs/v2/search"
"github.com/sourcegraph/go-diff/diff"
)
Expand Down Expand Up @@ -50,61 +51,22 @@ func CheckDiff(parsedDiff *diff.FileDiff, workspace string) *DiffPaths {
return &diffPaths
}

func ProcessDiffs(matcher lsearch.Matcher, hunk *diff.Hunk, flagsRef lflags.FlagsRef, maxFlags int) {
flagMap := map[Operation]lflags.FlagAliasMap{
Add: flagsRef.FlagsAdded,
Delete: flagsRef.FlagsRemoved,
}
func ProcessDiffs(matcher lsearch.Matcher, hunk *diff.Hunk, builder *lflags.ReferenceBuilder) {
diffLines := strings.Split(string(hunk.Body), "\n")
for _, line := range diffLines {
if flagsRef.Count() >= maxFlags {
break
}

op := operation(line)
if op == Equal {
op := diff_util.LineOperation(line)
if op == diff_util.OperationEqual {
continue
}

// only one for now
elementMatcher := matcher.Elements[0]
for _, flagKey := range elementMatcher.FindMatches(line) {
if _, ok := flagMap[op][flagKey]; !ok {
flagMap[op][flagKey] = make(lflags.AliasSet)
}
if aliasMatches := matcher.FindAliases(line, flagKey); len(aliasMatches) > 0 {
if _, ok := flagMap[op][flagKey]; !ok {
flagMap[op][flagKey] = make(lflags.AliasSet)
}
for _, alias := range aliasMatches {
flagMap[op][flagKey][alias] = true
}
}
aliasMatches := elementMatcher.FindAliases(line, flagKey)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was always supposed to be calling elementMatcher.FindAliases - since we don't have monorepo support, it hasn't mattered so far that it wasn't

builder.AddReference(flagKey, op, aliasMatches)
}
if builder.MaxReferences() {
break
}
}
flagsRef.FlagsAdded = flagMap[Add]
flagsRef.FlagsRemoved = flagMap[Delete]
}

// Operation defines the operation of a diff item.
type Operation int

const (
// Equal item represents an equals diff.
Equal Operation = iota
// Add item represents an insert diff.
Add
// Delete item represents a delete diff.
Delete
)

func operation(row string) Operation {
if strings.HasPrefix(row, "+") {
return Add
}
if strings.HasPrefix(row, "-") {
return Delete
}

return Equal
}
43 changes: 19 additions & 24 deletions diff/diff_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,9 @@ func createFlag(key string) ldapi.FeatureFlag {
}

type testProcessor struct {
Flags ldapi.FeatureFlags
FlagsRef lflags.FlagsRef
Config config.Config
Flags ldapi.FeatureFlags
Config config.Config
Builder *lflags.ReferenceBuilder
}

func (t testProcessor) flagKeys() []string {
Expand All @@ -55,21 +55,15 @@ func newProcessFlagAccEnv() *testProcessor {
flags := ldapi.FeatureFlags{}
flags.Items = append(flags.Items, flag)
flags.Items = append(flags.Items, flag2)
flagsAdded := make(lflags.FlagAliasMap)
flagsRemoved := make(lflags.FlagAliasMap)
flagsRef := lflags.FlagsRef{
FlagsAdded: flagsAdded,
FlagsRemoved: flagsRemoved,
}

builder := lflags.NewReferenceBuilder(5)
config := config.Config{
LdEnvironment: "production",
LdInstance: "https://example.com/",
}
return &testProcessor{
Flags: flags,
FlagsRef: flagsRef,
Config: config,
Flags: flags,
Config: config,
Builder: builder,
}
}

Expand Down Expand Up @@ -117,7 +111,7 @@ func TestCheckDiff(t *testing.T) {
}
}

func TestProcessDiffs(t *testing.T) {
func TestProcessDiffs_BuildReferences(t *testing.T) {
cases := []struct {
name string
sampleBody string
Expand All @@ -128,7 +122,7 @@ func TestProcessDiffs(t *testing.T) {
{
name: "add flag",
expected: lflags.FlagsRef{
FlagsAdded: lflags.FlagAliasMap{"example-flag": lflags.AliasSet{}},
FlagsAdded: lflags.FlagAliasMap{"example-flag": []string{}},
FlagsRemoved: lflags.FlagAliasMap{},
},
aliases: map[string][]string{},
Expand All @@ -145,7 +139,7 @@ func TestProcessDiffs(t *testing.T) {
name: "remove flag",
expected: lflags.FlagsRef{
FlagsAdded: lflags.FlagAliasMap{},
FlagsRemoved: lflags.FlagAliasMap{"example-flag": lflags.AliasSet{}},
FlagsRemoved: lflags.FlagAliasMap{"example-flag": []string{}},
},
aliases: map[string][]string{},
sampleBody: `
Expand All @@ -160,8 +154,8 @@ func TestProcessDiffs(t *testing.T) {
{
name: "add and remove flag",
expected: lflags.FlagsRef{
FlagsAdded: lflags.FlagAliasMap{"sample-flag": lflags.AliasSet{}},
FlagsRemoved: lflags.FlagAliasMap{"example-flag": lflags.AliasSet{}},
FlagsAdded: lflags.FlagAliasMap{"sample-flag": []string{}},
FlagsRemoved: lflags.FlagAliasMap{"example-flag": []string{}},
},
aliases: map[string][]string{},
sampleBody: `
Expand All @@ -177,8 +171,8 @@ func TestProcessDiffs(t *testing.T) {
{
name: "modified flag",
expected: lflags.FlagsRef{
FlagsAdded: lflags.FlagAliasMap{"example-flag": lflags.AliasSet{}},
FlagsRemoved: lflags.FlagAliasMap{"example-flag": lflags.AliasSet{}},
FlagsAdded: lflags.FlagAliasMap{"example-flag": []string{}},
FlagsRemoved: lflags.FlagAliasMap{},
},
aliases: map[string][]string{},
sampleBody: `
Expand All @@ -195,7 +189,7 @@ func TestProcessDiffs(t *testing.T) {
{
name: "alias flag",
expected: lflags.FlagsRef{
FlagsAdded: lflags.FlagAliasMap{"example-flag": lflags.AliasSet{"exampleFlag": true}},
FlagsAdded: lflags.FlagAliasMap{"example-flag": []string{"exampleFlag"}},
FlagsRemoved: lflags.FlagAliasMap{},
},
aliases: map[string][]string{"example-flag": {"exampleFlag"}},
Expand Down Expand Up @@ -225,7 +219,7 @@ func TestProcessDiffs(t *testing.T) {
{
name: "require delimiters - match",
expected: lflags.FlagsRef{
FlagsAdded: lflags.FlagAliasMap{"example-flag": lflags.AliasSet{}},
FlagsAdded: lflags.FlagAliasMap{"example-flag": []string{}},
FlagsRemoved: lflags.FlagAliasMap{},
},
delimiters: "'\"",
Expand Down Expand Up @@ -255,8 +249,9 @@ func TestProcessDiffs(t *testing.T) {
matcher := lsearch.Matcher{
Elements: elements,
}
ProcessDiffs(matcher, hunk, processor.FlagsRef, 5)
assert.Equal(t, tc.expected, processor.FlagsRef)
ProcessDiffs(matcher, hunk, processor.Builder)
flagsRef := processor.Builder.Build()
assert.Equal(t, tc.expected, flagsRef)
})
}
}
103 changes: 103 additions & 0 deletions flags/builder.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
package flags

import (
"fmt"
"sort"
"strings"

diff_util "github.com/launchdarkly/find-code-references-in-pull-request/internal/util/diff_util"
)

type ReferenceBuilder struct {
max int // maximum number of flags to find
flagsAdded map[string][]string
flagsRemoved map[string][]string
foundFlags map[string]struct{}
}

func NewReferenceBuilder(max int) *ReferenceBuilder {
return &ReferenceBuilder{
flagsAdded: make(map[string][]string),
flagsRemoved: make(map[string][]string),
foundFlags: make(map[string]struct{}),
max: max,
}
}

func (b *ReferenceBuilder) MaxReferences() bool {
return len(b.foundFlags) >= b.max
}

func (b *ReferenceBuilder) AddReference(flagKey string, op diff_util.Operation, aliases []string) error {
switch op {
case diff_util.OperationAdd:
b.AddedFlag(flagKey, aliases)
case diff_util.OperationDelete:
b.RemovedFlag(flagKey, aliases)
default:
return fmt.Errorf("invalid operation=%s", op.String())
}

return nil
}

func (b *ReferenceBuilder) AddedFlag(flagKey string, aliases []string) {
b.foundFlags[flagKey] = struct{}{}
if _, ok := b.flagsAdded[flagKey]; !ok {
b.flagsAdded[flagKey] = make([]string, 0, len(aliases))
}
b.flagsAdded[flagKey] = append(b.flagsAdded[flagKey], aliases...)
}

func (b *ReferenceBuilder) RemovedFlag(flagKey string, aliases []string) {
b.foundFlags[flagKey] = struct{}{}
if _, ok := b.flagsRemoved[flagKey]; !ok {
b.flagsRemoved[flagKey] = make([]string, 0, len(aliases))
}
b.flagsRemoved[flagKey] = append(b.flagsRemoved[flagKey], aliases...)
}

func (b *ReferenceBuilder) Build() FlagsRef {
added := make(map[string][]string, len(b.flagsAdded))
removed := make(map[string][]string, len(b.flagsRemoved))

for flagKey := range b.foundFlags {
if aliases, ok := b.flagsAdded[flagKey]; ok {
// if there are any removed aliases, we should add them
aliases := append(aliases, b.flagsRemoved[flagKey]...)
aliases = uniqueStrs(aliases)
sort.Strings(aliases)
added[flagKey] = aliases
} else if aliases, ok := b.flagsRemoved[flagKey]; ok {
// only add to removed if it wasn't also added
aliases := uniqueStrs(aliases)
sort.Strings(aliases)
removed[flagKey] = aliases
}
}

return FlagsRef{
FlagsAdded: added,
FlagsRemoved: removed,
}
}

// get slice with unique, non-empty strings
func uniqueStrs(s []string) []string {
if len(s) <= 1 {
return s
}
keys := make(map[string]struct{}, len(s))
ret := make([]string, 0, len(s))
for _, elem := range s {
trimmed := strings.TrimSpace(elem)
if len(trimmed) == 0 {
continue
}
if _, ok := keys[trimmed]; !ok {
keys[trimmed] = struct{}{}
ret = append(ret, trimmed)
}
}
return ret
}
4 changes: 1 addition & 3 deletions flags/types.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
package flags

type FlagAliasMap = map[string]AliasSet

type AliasSet = map[string]bool
type FlagAliasMap = map[string][]string

type FlagsRef struct {
FlagsAdded FlagAliasMap
Expand Down
Loading