diff --git a/README.md b/README.md index ac48b132..f42ba205 100644 --- a/README.md +++ b/README.md @@ -101,6 +101,7 @@ This action does not support monorepos or searching for flags across LaunchDarkl | include-archived-flags | Scan for archived flags | `false` | true | | max-flags | Maximum number of flags to find per PR | `false` | 5 | | base-uri | The base URI for the LaunchDarkly server. Most users should use the default value. | `false` | https://app.launchdarkly.com | +| check-extinctions | Check if removed flags still exist in codebase | `false` | true | @@ -117,4 +118,7 @@ This action does not support monorepos or searching for flags across LaunchDarkl | any-changed | Returns true if any flags have been changed in PR | | changed-flags | Space-separated list of flags changed in PR | | changed-flags-count | Number of flags changed in PR | +| any-extinct | Returns true if any flags have been removed in PR and no longer exist in codebase. Only returned if `check-extinctions` is true. | +| extinct-flags | Space-separated list of flags removed in PR and no longer exist in codebase. Only returned if `check-extinctions` is true. | +| extinct-flags-count | Number of flags removed in PR and no longer exist in codebase. Only returned if `check-extinctions` is true. | diff --git a/action.yml b/action.yml index df578690..9bd4bda2 100644 --- a/action.yml +++ b/action.yml @@ -41,6 +41,10 @@ inputs: description: The base URI for the LaunchDarkly server. Most users should use the default value. required: false default: 'https://app.launchdarkly.com' + check-extinctions: + description: Check if removed flags still exist in codebase + required: false + default: 'true' outputs: any-modified: @@ -61,3 +65,9 @@ outputs: description: Space-separated list of flags changed in PR changed-flags-count: description: Number of flags changed in PR + any-extinct: + description: Returns true if any flags have been removed in PR and no longer exist in codebase. Only returned if `check-extinctions` is true. + extinct-flags: + description: Space-separated list of flags removed in PR and no longer exist in codebase. Only returned if `check-extinctions` is true. + extinct-flags-count: + description: Number of flags removed in PR and no longer exist in codebase. Only returned if `check-extinctions` is true. diff --git a/comments/comments.go b/comments/comments.go index 2cef2107..51d3bdfd 100644 --- a/comments/comments.go +++ b/comments/comments.go @@ -17,15 +17,15 @@ import ( "github.com/google/go-github/github" ldapi "github.com/launchdarkly/api-client-go/v13" - "github.com/launchdarkly/find-code-references-in-pull-request/config" lcr "github.com/launchdarkly/find-code-references-in-pull-request/config" - lflags "github.com/launchdarkly/find-code-references-in-pull-request/flags" + refs "github.com/launchdarkly/find-code-references-in-pull-request/internal/references" ) type Comment struct { Flag ldapi.FeatureFlag ArchivedAt time.Time Added bool + Extinct bool Aliases []string ChangeType string Primary ldapi.FeatureFlagConfig @@ -37,10 +37,11 @@ func isNil(a interface{}) bool { return a == nil || reflect.ValueOf(a).IsNil() } -func githubFlagComment(flag ldapi.FeatureFlag, aliases []string, added bool, config *config.Config) (string, error) { +func githubFlagComment(flag ldapi.FeatureFlag, aliases []string, added, extinct bool, config *lcr.Config) (string, error) { commentTemplate := Comment{ Flag: flag, Added: added, + Extinct: config.CheckExtinctions && extinct, Aliases: aliases, Primary: flag.Environments[config.LdEnvironment], LDInstance: config.LdInstance, @@ -50,15 +51,13 @@ func githubFlagComment(flag ldapi.FeatureFlag, aliases []string, added bool, con commentTemplate.ArchivedAt = time.UnixMilli(*flag.ArchivedDate) } // All whitespace for template is required to be there or it will not render properly nested. - tmplSetup := `| {{- if eq .Flag.Archived true}}{{- if eq .Added true}} :warning:{{- end}}{{- end}}` + - ` [{{.Flag.Name}}]({{.LDInstance}}{{.Primary.Site.Href}})` + - `{{- if eq .Flag.Archived true}}` + - ` (archived on {{.ArchivedAt | date "2006-01-02"}})` + - `{{- end}} | ` + + tmplSetup := `| [{{.Flag.Name}}]({{.LDInstance}}{{.Primary.Site.Href}}) | ` + "`" + `{{.Flag.Key}}` + "` |" + `{{- if ne (len .Aliases) 0}}` + `{{range $i, $e := .Aliases }}` + `{{if $i}},{{end}}` + " `" + `{{$e}}` + "`" + `{{end}}` + - `{{- end}} |` + `{{- end}} | ` + + `{{- if eq .Extinct true}} :white_check_mark: all references removed{{- end}} ` + + `{{- if eq .Flag.Archived true}}{{- if eq .Extinct true}}
{{end}}{{- if eq .Added true}} :warning:{{else}} :information_source:{{- end}} archived on {{.ArchivedAt | date "2006-01-02"}}{{- end}} |` tmpl := template.Must(template.New("comment").Funcs(template.FuncMap{"trim": strings.TrimSpace, "isNil": isNil}).Funcs(sprig.FuncMap()).Parse(tmplSetup)) err := tmpl.Execute(&commentBody, commentTemplate) @@ -85,8 +84,8 @@ type FlagComments struct { CommentsRemoved []string } -func BuildFlagComment(buildComment FlagComments, flagsRef lflags.FlagsRef, existingComment *github.IssueComment) string { - tableHeader := "| Name | Key | Aliases found |\n| --- | --- | --- |" +func BuildFlagComment(buildComment FlagComments, flagsRef refs.ReferenceSummary, existingComment *github.IssueComment) string { + tableHeader := "| Name | Key | Aliases found | Info |\n| --- | --- | --- | --- |" var commentStr []string commentStr = append(commentStr, "## LaunchDarkly flag references") @@ -122,36 +121,32 @@ func BuildFlagComment(buildComment FlagComments, flagsRef lflags.FlagsRef, exist return postedComments } -func ProcessFlags(flagsRef lflags.FlagsRef, flags []ldapi.FeatureFlag, config *lcr.Config) FlagComments { +func ProcessFlags(flagsRef refs.ReferenceSummary, flags []ldapi.FeatureFlag, config *lcr.Config) FlagComments { buildComment := FlagComments{} - addedKeys := make([]string, 0, len(flagsRef.FlagsAdded)) - for key := range flagsRef.FlagsAdded { - addedKeys = append(addedKeys, key) - } - // sort keys so hashing can work for checking if comment already exists - sort.Strings(addedKeys) - for _, flagKey := range addedKeys { + + for _, flagKey := range flagsRef.AddedKeys() { flagAliases := flagsRef.FlagsAdded[flagKey] idx, _ := find(flags, flagKey) - createComment, err := githubFlagComment(flags[idx], flagAliases, true, config) - buildComment.CommentsAdded = append(buildComment.CommentsAdded, createComment) + createComment, err := githubFlagComment(flags[idx], flagAliases, true, false, config) if err != nil { log.Println(err) } + buildComment.CommentsAdded = append(buildComment.CommentsAdded, createComment) } - removedKeys := make([]string, 0, len(flagsRef.FlagsRemoved)) - for key := range flagsRef.FlagsRemoved { - removedKeys = append(removedKeys, key) - } - sort.Strings(removedKeys) - for _, flagKey := range removedKeys { + + for _, flagKey := range flagsRef.RemovedKeys() { flagAliases := flagsRef.FlagsRemoved[flagKey] idx, _ := find(flags, flagKey) - removedComment, err := githubFlagComment(flags[idx], flagAliases, false, config) - buildComment.CommentsRemoved = append(buildComment.CommentsRemoved, removedComment) + extinct := false + if flagsRef.ExtinctFlags != nil { + _, e := flagsRef.ExtinctFlags[flagKey] + extinct = e + } + removedComment, err := githubFlagComment(flags[idx], flagAliases, false, extinct, config) if err != nil { log.Println(err) } + buildComment.CommentsRemoved = append(buildComment.CommentsRemoved, removedComment) } return buildComment @@ -166,7 +161,7 @@ func find(slice []ldapi.FeatureFlag, val string) (int, bool) { return -1, false } -func uniqueFlagKeys(a, b lflags.FlagAliasMap) []string { +func uniqueFlagKeys(a, b refs.FlagAliasMap) []string { maxKeys := len(a) + len(b) allKeys := make([]string, 0, maxKeys) for k := range a { diff --git a/comments/comments_test.go b/comments/comments_test.go index 999ab094..08fd830d 100644 --- a/comments/comments_test.go +++ b/comments/comments_test.go @@ -6,7 +6,7 @@ import ( ldapi "github.com/launchdarkly/api-client-go/v13" "github.com/launchdarkly/find-code-references-in-pull-request/config" - lflags "github.com/launchdarkly/find-code-references-in-pull-request/flags" + refs "github.com/launchdarkly/find-code-references-in-pull-request/internal/references" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -22,8 +22,9 @@ type testFlagEnv struct { func newTestAccEnv() *testFlagEnv { flag := createFlag("example-flag") config := config.Config{ - LdEnvironment: "production", - LdInstance: "https://example.com/", + LdEnvironment: "production", + LdInstance: "https://example.com/", + CheckExtinctions: true, } archivedFlag := createFlag("archived-flag") @@ -61,7 +62,7 @@ func createFlag(key string) ldapi.FeatureFlag { type testCommentBuilder struct { Comments FlagComments - FlagsRef lflags.FlagsRef + FlagsRef refs.ReferenceSummary } func newCommentBuilderAccEnv() *testCommentBuilder { @@ -69,9 +70,9 @@ func newCommentBuilderAccEnv() *testCommentBuilder { CommentsAdded: []string{}, CommentsRemoved: []string{}, } - flagsAdded := make(lflags.FlagAliasMap) - flagsRemoved := make(lflags.FlagAliasMap) - flagsRef := lflags.FlagsRef{ + flagsAdded := make(refs.FlagAliasMap) + flagsRemoved := make(refs.FlagAliasMap) + flagsRef := refs.ReferenceSummary{ FlagsAdded: flagsAdded, FlagsRemoved: flagsRemoved, } @@ -84,16 +85,16 @@ func newCommentBuilderAccEnv() *testCommentBuilder { type testProcessor struct { Flags []ldapi.FeatureFlag - FlagsRef lflags.FlagsRef + FlagsRef refs.ReferenceSummary Config config.Config } func newProcessFlagAccEnv() *testProcessor { flag := createFlag("example-flag") flags := []ldapi.FeatureFlag{flag} - flagsAdded := make(lflags.FlagAliasMap) - flagsRemoved := make(lflags.FlagAliasMap) - flagsRef := lflags.FlagsRef{ + flagsAdded := make(refs.FlagAliasMap) + flagsRemoved := make(refs.FlagAliasMap) + flagsRef := refs.ReferenceSummary{ FlagsAdded: flagsAdded, FlagsRemoved: flagsRemoved, } @@ -113,9 +114,9 @@ func newProcessMultipleFlagsFlagAccEnv() *testProcessor { flag := createFlag("example-flag") flag2 := createFlag("second-flag") flags := []ldapi.FeatureFlag{flag, flag2} - flagsAdded := make(lflags.FlagAliasMap) - flagsRemoved := make(lflags.FlagAliasMap) - flagsRef := lflags.FlagsRef{ + flagsAdded := make(refs.FlagAliasMap) + flagsRemoved := make(refs.FlagAliasMap) + flagsRef := refs.ReferenceSummary{ FlagsAdded: flagsAdded, FlagsRemoved: flagsRemoved, } @@ -137,6 +138,8 @@ func TestGithubFlagComment(t *testing.T) { t.Run("Flag with alias", acceptanceTestEnv.Alias) t.Run("Archived flag added", acceptanceTestEnv.ArchivedAdded) t.Run("Archived flag removed", acceptanceTestEnv.ArchivedRemoved) + t.Run("Extinct flag", acceptanceTestEnv.ExtinctFlag) + t.Run("Extinct and Archived flag", acceptanceTestEnv.ExtinctAndArchivedFlag) } func TestProcessFlags(t *testing.T) { @@ -164,34 +167,50 @@ func TestBuildFlagComment(t *testing.T) { } func (e *testFlagEnv) NoAliases(t *testing.T) { - comment, err := githubFlagComment(e.Flag, []string{}, true, &e.Config) + comment, err := githubFlagComment(e.Flag, []string{}, true, false, &e.Config) require.NoError(t, err) - expected := "| [example flag](https://example.com/test) | `example-flag` | |" + expected := "| [example flag](https://example.com/test) | `example-flag` | | |" assert.Equal(t, expected, comment) } func (e *testFlagEnv) Alias(t *testing.T) { - comment, err := githubFlagComment(e.Flag, []string{"exampleFlag", "ExampleFlag"}, true, &e.Config) + comment, err := githubFlagComment(e.Flag, []string{"exampleFlag", "ExampleFlag"}, true, false, &e.Config) require.NoError(t, err) - expected := "| [example flag](https://example.com/test) | `example-flag` | `exampleFlag`, `ExampleFlag` |" + expected := "| [example flag](https://example.com/test) | `example-flag` | `exampleFlag`, `ExampleFlag` | |" assert.Equal(t, expected, comment) } func (e *testFlagEnv) ArchivedAdded(t *testing.T) { - comment, err := githubFlagComment(e.ArchivedFlag, []string{}, true, &e.Config) + comment, err := githubFlagComment(e.ArchivedFlag, []string{}, true, false, &e.Config) require.NoError(t, err) - expected := "| :warning: [archived flag](https://example.com/test) (archived on 2023-08-03) | `archived-flag` | |" + expected := "| [archived flag](https://example.com/test) | `archived-flag` | | :warning: archived on 2023-08-03 |" assert.Equal(t, expected, comment) } func (e *testFlagEnv) ArchivedRemoved(t *testing.T) { - comment, err := githubFlagComment(e.ArchivedFlag, []string{}, false, &e.Config) + comment, err := githubFlagComment(e.ArchivedFlag, []string{}, false, false, &e.Config) + require.NoError(t, err) + + expected := "| [archived flag](https://example.com/test) | `archived-flag` | | :information_source: archived on 2023-08-03 |" + assert.Equal(t, expected, comment) +} + +func (e *testFlagEnv) ExtinctFlag(t *testing.T) { + comment, err := githubFlagComment(e.Flag, []string{}, false, true, &e.Config) + require.NoError(t, err) + + expected := "| [example flag](https://example.com/test) | `example-flag` | | :white_check_mark: all references removed |" + assert.Equal(t, expected, comment) +} + +func (e *testFlagEnv) ExtinctAndArchivedFlag(t *testing.T) { + comment, err := githubFlagComment(e.ArchivedFlag, []string{}, false, true, &e.Config) require.NoError(t, err) - expected := "| [archived flag](https://example.com/test) (archived on 2023-08-03) | `archived-flag` | |" + expected := "| [archived flag](https://example.com/test) | `archived-flag` | | :white_check_mark: all references removed
:information_source: archived on 2023-08-03 |" assert.Equal(t, expected, comment) } @@ -200,7 +219,7 @@ func (e *testCommentBuilder) AddedOnly(t *testing.T) { e.Comments.CommentsAdded = []string{"comment1", "comment2"} comment := BuildFlagComment(e.Comments, e.FlagsRef, nil) - expected := "## LaunchDarkly flag references\n### :mag: 1 flag added or modified\n\n| Name | Key | Aliases found |\n| --- | --- | --- |\ncomment1\ncomment2\n\n\n \n " + expected := "## LaunchDarkly flag references\n### :mag: 1 flag added or modified\n\n| Name | Key | Aliases found | Info |\n| --- | --- | --- | --- |\ncomment1\ncomment2\n\n\n \n " assert.Equal(t, expected, comment) } @@ -210,7 +229,7 @@ func (e *testCommentBuilder) RemovedOnly(t *testing.T) { e.Comments.CommentsRemoved = []string{"comment1", "comment2"} comment := BuildFlagComment(e.Comments, e.FlagsRef, nil) - expected := "## LaunchDarkly flag references\n### :x: 2 flags removed\n\n| Name | Key | Aliases found |\n| --- | --- | --- |\ncomment1\ncomment2\n \n " + expected := "## LaunchDarkly flag references\n### :x: 2 flags removed\n\n| Name | Key | Aliases found | Info |\n| --- | --- | --- | --- |\ncomment1\ncomment2\n \n " assert.Equal(t, expected, comment) } @@ -221,7 +240,7 @@ func (e *testCommentBuilder) AddedAndRemoved(t *testing.T) { e.Comments.CommentsRemoved = []string{"comment1", "comment2"} comment := BuildFlagComment(e.Comments, e.FlagsRef, nil) - expected := "## LaunchDarkly flag references\n### :mag: 1 flag added or modified\n\n| Name | Key | Aliases found |\n| --- | --- | --- |\ncomment1\ncomment2\n\n\n### :x: 1 flag removed\n\n| Name | Key | Aliases found |\n| --- | --- | --- |\ncomment1\ncomment2\n \n " + expected := "## LaunchDarkly flag references\n### :mag: 1 flag added or modified\n\n| Name | Key | Aliases found | Info |\n| --- | --- | --- | --- |\ncomment1\ncomment2\n\n\n### :x: 1 flag removed\n\n| Name | Key | Aliases found | Info |\n| --- | --- | --- | --- |\ncomment1\ncomment2\n \n " assert.Equal(t, expected, comment) @@ -231,7 +250,7 @@ func (e *testProcessor) Basic(t *testing.T) { 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` | |"}, + CommentsAdded: []string{"| [example flag](https://example.com/test) | `example-flag` | | |"}, } assert.Equal(t, expected, processor) } @@ -242,8 +261,8 @@ func (e *testProcessor) Multi(t *testing.T) { processor := ProcessFlags(e.FlagsRef, e.Flags, &e.Config) expected := FlagComments{ CommentsAdded: []string{ - "| [example flag](https://example.com/test) | `example-flag` | |", - "| [second flag](https://example.com/test) | `second-flag` | |", + "| [example flag](https://example.com/test) | `example-flag` | | |", + "| [second flag](https://example.com/test) | `second-flag` | | |", }, } assert.Equal(t, expected, processor) diff --git a/config/config.go b/config/config.go index 8f045860..510e19ba 100644 --- a/config/config.go +++ b/config/config.go @@ -24,6 +24,7 @@ type Config struct { MaxFlags int PlaceholderComment bool IncludeArchivedFlags bool + CheckExtinctions bool } func ValidateInputandParse(ctx context.Context) (*Config, error) { @@ -35,8 +36,13 @@ func ValidateInputandParse(ctx context.Context) (*Config, error) { fmt.Printf("::add-mask::%s\n", repoToken) } - // set config - var config Config + // init config with defaults + config := Config{ + MaxFlags: 5, + IncludeArchivedFlags: true, + CheckExtinctions: true, + } + config.LdProject = os.Getenv("INPUT_PROJECT-KEY") if config.LdProject == "" { return nil, errors.New("`project-key` is required") @@ -74,12 +80,16 @@ func ValidateInputandParse(ctx context.Context) (*Config, error) { config.PlaceholderComment = placholderComment } - config.IncludeArchivedFlags = true if includeArchivedFlags, err := strconv.ParseBool(os.Getenv("INPUT_INCLUDE-ARCHIVED-FLAGS")); err == nil { // ignore error - default is true config.IncludeArchivedFlags = includeArchivedFlags } + if checkExtinctions, err := strconv.ParseBool(os.Getenv("INPUT_CHECK-EXTINCTIONS")); err == nil { + // ignore error - default is true + config.CheckExtinctions = checkExtinctions + } + config.GHClient = getGithubClient(ctx) return &config, nil } diff --git a/diff/diff.go b/diff/diff.go index 6b596ffd..11848569 100644 --- a/diff/diff.go +++ b/diff/diff.go @@ -5,8 +5,8 @@ import ( "os" "strings" - lflags "github.com/launchdarkly/find-code-references-in-pull-request/flags" i "github.com/launchdarkly/find-code-references-in-pull-request/ignore" + refs "github.com/launchdarkly/find-code-references-in-pull-request/internal/references" diff_util "github.com/launchdarkly/find-code-references-in-pull-request/internal/utils/diff_util" "github.com/launchdarkly/ld-find-code-refs/v2/aliases" lsearch "github.com/launchdarkly/ld-find-code-refs/v2/search" @@ -67,7 +67,7 @@ func checkDiffFile(parsedDiff *diff.FileDiff, workspace string) (filePath string return filePath, false } -func ProcessDiffs(matcher lsearch.Matcher, contents []byte, builder *lflags.ReferenceBuilder) { +func ProcessDiffs(matcher lsearch.Matcher, contents []byte, builder *refs.ReferenceSummaryBuilder) { diffLines := strings.Split(string(contents), "\n") for _, line := range diffLines { op := diff_util.LineOperation(line) diff --git a/diff/diff_test.go b/diff/diff_test.go index aa89d4b2..6f8f141d 100644 --- a/diff/diff_test.go +++ b/diff/diff_test.go @@ -5,7 +5,7 @@ import ( ldapi "github.com/launchdarkly/api-client-go/v13" "github.com/launchdarkly/find-code-references-in-pull-request/config" - lflags "github.com/launchdarkly/find-code-references-in-pull-request/flags" + refs "github.com/launchdarkly/find-code-references-in-pull-request/internal/references" lsearch "github.com/launchdarkly/ld-find-code-refs/v2/search" "github.com/sourcegraph/go-diff/diff" "github.com/stretchr/testify/assert" @@ -38,7 +38,7 @@ func createFlag(key string) ldapi.FeatureFlag { type testProcessor struct { Flags ldapi.FeatureFlags Config config.Config - Builder *lflags.ReferenceBuilder + Builder *refs.ReferenceSummaryBuilder } func (t testProcessor) flagKeys() []string { @@ -55,7 +55,7 @@ func newProcessFlagAccEnv() *testProcessor { flags := ldapi.FeatureFlags{} flags.Items = append(flags.Items, flag) flags.Items = append(flags.Items, flag2) - builder := lflags.NewReferenceBuilder(5) + builder := refs.NewReferenceSummaryBuilder(5, false) config := config.Config{ LdEnvironment: "production", LdInstance: "https://example.com/", @@ -117,15 +117,15 @@ func TestProcessDiffs_BuildReferences(t *testing.T) { cases := []struct { name string sampleBody string - expected lflags.FlagsRef + expected refs.ReferenceSummary aliases map[string][]string delimiters string }{ { name: "add flag", - expected: lflags.FlagsRef{ - FlagsAdded: lflags.FlagAliasMap{"example-flag": []string{}}, - FlagsRemoved: lflags.FlagAliasMap{}, + expected: refs.ReferenceSummary{ + FlagsAdded: refs.FlagAliasMap{"example-flag": []string{}}, + FlagsRemoved: refs.FlagAliasMap{}, }, aliases: map[string][]string{}, sampleBody: ` @@ -139,9 +139,9 @@ func TestProcessDiffs_BuildReferences(t *testing.T) { }, { name: "remove flag", - expected: lflags.FlagsRef{ - FlagsAdded: lflags.FlagAliasMap{}, - FlagsRemoved: lflags.FlagAliasMap{"example-flag": []string{}}, + expected: refs.ReferenceSummary{ + FlagsAdded: refs.FlagAliasMap{}, + FlagsRemoved: refs.FlagAliasMap{"example-flag": []string{}}, }, aliases: map[string][]string{}, sampleBody: ` @@ -155,9 +155,9 @@ func TestProcessDiffs_BuildReferences(t *testing.T) { }, { name: "add and remove flag", - expected: lflags.FlagsRef{ - FlagsAdded: lflags.FlagAliasMap{"sample-flag": []string{}}, - FlagsRemoved: lflags.FlagAliasMap{"example-flag": []string{}}, + expected: refs.ReferenceSummary{ + FlagsAdded: refs.FlagAliasMap{"sample-flag": []string{}}, + FlagsRemoved: refs.FlagAliasMap{"example-flag": []string{}}, }, aliases: map[string][]string{}, sampleBody: ` @@ -172,9 +172,9 @@ func TestProcessDiffs_BuildReferences(t *testing.T) { }, { name: "modified flag", - expected: lflags.FlagsRef{ - FlagsAdded: lflags.FlagAliasMap{"example-flag": []string{}}, - FlagsRemoved: lflags.FlagAliasMap{}, + expected: refs.ReferenceSummary{ + FlagsAdded: refs.FlagAliasMap{"example-flag": []string{}}, + FlagsRemoved: refs.FlagAliasMap{}, }, aliases: map[string][]string{}, sampleBody: ` @@ -190,9 +190,9 @@ func TestProcessDiffs_BuildReferences(t *testing.T) { }, { name: "alias flag", - expected: lflags.FlagsRef{ - FlagsAdded: lflags.FlagAliasMap{"example-flag": []string{"exampleFlag"}}, - FlagsRemoved: lflags.FlagAliasMap{}, + expected: refs.ReferenceSummary{ + FlagsAdded: refs.FlagAliasMap{"example-flag": []string{"exampleFlag"}}, + FlagsRemoved: refs.FlagAliasMap{}, }, aliases: map[string][]string{"example-flag": {"exampleFlag"}}, sampleBody: ` @@ -205,9 +205,9 @@ func TestProcessDiffs_BuildReferences(t *testing.T) { }, { name: "require delimiters - no matches", - expected: lflags.FlagsRef{ - FlagsAdded: lflags.FlagAliasMap{}, - FlagsRemoved: lflags.FlagAliasMap{}, + expected: refs.ReferenceSummary{ + FlagsAdded: refs.FlagAliasMap{}, + FlagsRemoved: refs.FlagAliasMap{}, }, delimiters: "'\"", aliases: map[string][]string{}, @@ -220,9 +220,9 @@ func TestProcessDiffs_BuildReferences(t *testing.T) { }, { name: "require delimiters - match", - expected: lflags.FlagsRef{ - FlagsAdded: lflags.FlagAliasMap{"example-flag": []string{}}, - FlagsRemoved: lflags.FlagAliasMap{}, + expected: refs.ReferenceSummary{ + FlagsAdded: refs.FlagAliasMap{"example-flag": []string{}}, + FlagsRemoved: refs.FlagAliasMap{}, }, delimiters: "'\"", aliases: map[string][]string{}, diff --git a/flags/builder.go b/flags/builder.go deleted file mode 100644 index 91cf0a66..00000000 --- a/flags/builder.go +++ /dev/null @@ -1,103 +0,0 @@ -package flags - -import ( - "fmt" - "sort" - "strings" - - diff_util "github.com/launchdarkly/find-code-references-in-pull-request/internal/utils/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 -} diff --git a/flags/types.go b/flags/types.go deleted file mode 100644 index 1a6bf28e..00000000 --- a/flags/types.go +++ /dev/null @@ -1,16 +0,0 @@ -package flags - -type FlagAliasMap = map[string][]string - -type FlagsRef struct { - FlagsAdded FlagAliasMap - FlagsRemoved FlagAliasMap -} - -func (fr FlagsRef) Found() bool { - return fr.Count() > 0 -} - -func (fr FlagsRef) Count() int { - return len(fr.FlagsAdded) + len(fr.FlagsRemoved) -} diff --git a/internal/extinctions/extinctions.go b/internal/extinctions/extinctions.go new file mode 100644 index 00000000..75e35b52 --- /dev/null +++ b/internal/extinctions/extinctions.go @@ -0,0 +1,30 @@ +package extinctions + +import ( + refs "github.com/launchdarkly/find-code-references-in-pull-request/internal/references" + "github.com/launchdarkly/find-code-references-in-pull-request/search" + "github.com/launchdarkly/ld-find-code-refs/v2/options" + ld_search "github.com/launchdarkly/ld-find-code-refs/v2/search" +) + +func CheckExtinctions(opts options.Options, builder *refs.ReferenceSummaryBuilder) error { + flagKeys := make([]string, 0, len(builder.RemovedFlagKeys())) + + matcher, err := search.GetMatcher(opts, flagKeys, nil) + if err != nil { + return err + } + + references, err := ld_search.SearchForRefs(opts.Dir, matcher) + if err != nil { + return err + } + + for _, ref := range references { + for _, hunk := range ref.Hunks { + builder.AddHeadFlag(hunk.FlagKey) + } + } + + return nil +} diff --git a/internal/references/builder.go b/internal/references/builder.go new file mode 100644 index 00000000..a44f5873 --- /dev/null +++ b/internal/references/builder.go @@ -0,0 +1,142 @@ +package flags + +import ( + "fmt" + "sort" + "strings" + + "github.com/launchdarkly/find-code-references-in-pull-request/internal/utils/diff_util" +) + +type ReferenceSummaryBuilder struct { + max int // maximum number of flags to find + includeExtinctions bool // include extinctions in summary + flagsAdded map[string][]string + flagsRemoved map[string][]string + flagsFoundAtHead map[string]struct{} + foundFlags map[string]struct{} +} + +func NewReferenceSummaryBuilder(max int, includeExtinctions bool) *ReferenceSummaryBuilder { + return &ReferenceSummaryBuilder{ + flagsAdded: make(map[string][]string), + flagsRemoved: make(map[string][]string), + foundFlags: make(map[string]struct{}), + flagsFoundAtHead: make(map[string]struct{}), + max: max, + includeExtinctions: includeExtinctions, + } +} + +func (b *ReferenceSummaryBuilder) MaxReferences() bool { + return len(b.foundFlags) >= b.max +} + +// Add a found flag in diff by operation +func (b *ReferenceSummaryBuilder) 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 +} + +// Flag found in HEAD ref +func (b *ReferenceSummaryBuilder) AddHeadFlag(flagKey string) { + if _, ok := b.flagsFoundAtHead[flagKey]; !ok { + b.flagsFoundAtHead[flagKey] = struct{}{} + } +} + +func (b *ReferenceSummaryBuilder) foundFlag(flagKey string) { + if _, ok := b.foundFlags[flagKey]; !ok { + b.foundFlags[flagKey] = struct{}{} + } +} + +// Flag and aliases found in added diff +func (b *ReferenceSummaryBuilder) addedFlag(flagKey string, aliases []string) { + b.foundFlag(flagKey) + if _, ok := b.flagsAdded[flagKey]; !ok { + b.flagsAdded[flagKey] = make([]string, 0, len(aliases)) + } + b.flagsAdded[flagKey] = append(b.flagsAdded[flagKey], aliases...) +} + +// Flag and aliases found in removed diff +func (b *ReferenceSummaryBuilder) removedFlag(flagKey string, aliases []string) { + b.foundFlag(flagKey) + if _, ok := b.flagsRemoved[flagKey]; !ok { + b.flagsRemoved[flagKey] = make([]string, 0, len(aliases)) + } + b.flagsRemoved[flagKey] = append(b.flagsRemoved[flagKey], aliases...) +} + +// Returns a list of removed flag keys +func (b *ReferenceSummaryBuilder) RemovedFlagKeys() []string { + keys := make([]string, 0, len(b.flagsRemoved)) + for k := range b.flagsRemoved { + keys = append(keys, k) + } + return keys +} + +func (b *ReferenceSummaryBuilder) Build() ReferenceSummary { + added := make(map[string][]string, len(b.flagsAdded)) + removed := make(map[string][]string, len(b.flagsRemoved)) + extinctions := make(map[string]struct{}, 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 + if _, ok := b.flagsFoundAtHead[flagKey]; !ok { + extinctions[flagKey] = struct{}{} + } + } + } + + summary := ReferenceSummary{ + FlagsAdded: added, + FlagsRemoved: removed, + } + + if b.includeExtinctions { + summary.ExtinctFlags = extinctions + } + + return summary +} + +// 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 +} diff --git a/internal/references/types.go b/internal/references/types.go new file mode 100644 index 00000000..8686b5d1 --- /dev/null +++ b/internal/references/types.go @@ -0,0 +1,47 @@ +package flags + +import "sort" + +type FlagAliasMap = map[string][]string + +type ReferenceSummary struct { + FlagsAdded FlagAliasMap + FlagsRemoved FlagAliasMap + ExtinctFlags map[string]struct{} +} + +func (fr ReferenceSummary) AnyFound() bool { + return len(fr.FlagsAdded)+len(fr.FlagsRemoved) > 0 +} + +// returns a sorted list of all added flag keys +func (fr ReferenceSummary) AddedKeys() []string { + return fr.sortedKeys(fr.FlagsAdded) +} + +// returns a sorted list of all removed flag keys +func (fr ReferenceSummary) RemovedKeys() []string { + return fr.sortedKeys(fr.FlagsRemoved) +} + +// returns a sorted list of all extinct flag keys +func (fr ReferenceSummary) ExtinctKeys() []string { + if fr.ExtinctFlags == nil { + return nil + } + keys := make([]string, 0, len(fr.ExtinctFlags)) + for k := range fr.ExtinctFlags { + keys = append(keys, k) + } + sort.Strings(keys) + return keys +} + +func (fr ReferenceSummary) sortedKeys(keys map[string][]string) []string { + sortedKeys := make([]string, 0, len(keys)) + for k := range keys { + sortedKeys = append(sortedKeys, k) + } + sort.Strings(sortedKeys) + return sortedKeys +} diff --git a/main.go b/main.go index 65dee39c..d6f81411 100644 --- a/main.go +++ b/main.go @@ -16,9 +16,10 @@ import ( lcr "github.com/launchdarkly/find-code-references-in-pull-request/config" ldiff "github.com/launchdarkly/find-code-references-in-pull-request/diff" e "github.com/launchdarkly/find-code-references-in-pull-request/errors" - lflags "github.com/launchdarkly/find-code-references-in-pull-request/flags" + "github.com/launchdarkly/find-code-references-in-pull-request/internal/extinctions" gha "github.com/launchdarkly/find-code-references-in-pull-request/internal/github_actions" ldclient "github.com/launchdarkly/find-code-references-in-pull-request/internal/ldclient" + references "github.com/launchdarkly/find-code-references-in-pull-request/internal/references" "github.com/launchdarkly/find-code-references-in-pull-request/search" "github.com/launchdarkly/ld-find-code-refs/v2/options" "github.com/sourcegraph/go-diff/diff" @@ -48,18 +49,30 @@ func main() { opts, err := getOptions(config) failExit(err) + flagKeys := make([]string, 0, len(flags)) + for _, flag := range flags { + flagKeys = append(flagKeys, flag.Key) + } + multiFiles, err := getDiffs(ctx, config, *event.PullRequest.Number) failExit(err) diffMap := ldiff.PreprocessDiffs(opts.Dir, multiFiles) - matcher, err := search.GetMatcher(opts, flags, diffMap) + matcher, err := search.GetMatcher(opts, flagKeys, diffMap) failExit(err) - builder := lflags.NewReferenceBuilder(config.MaxFlags) + builder := references.NewReferenceSummaryBuilder(config.MaxFlags, config.CheckExtinctions) for _, contents := range diffMap { ldiff.ProcessDiffs(matcher, contents, builder) } + + if config.CheckExtinctions { + if err := extinctions.CheckExtinctions(opts, builder); err != nil { + gha.LogWarning("Error checking for extinct flags") + log.Println(err) + } + } flagsRef := builder.Build() // Add comment @@ -75,7 +88,7 @@ func main() { } // Set outputs - setOutputs(flagsRef) + setOutputs(config, flagsRef) failExit(err) } @@ -95,13 +108,13 @@ func checkExistingComments(event *github.PullRequestEvent, config *lcr.Config, c return nil } -func postGithubComment(ctx context.Context, flagsRef lflags.FlagsRef, config *lcr.Config, existingComment *github.IssueComment, prNumber int, comment github.IssueComment) error { +func postGithubComment(ctx context.Context, flagsRef references.ReferenceSummary, config *lcr.Config, existingComment *github.IssueComment, prNumber int, comment github.IssueComment) error { var existingCommentId int64 if existingComment != nil { existingCommentId = existingComment.GetID() } - if flagsRef.Found() { + if flagsRef.AnyFound() { if existingCommentId > 0 { _, _, err := config.GHClient.Issues.EditComment(ctx, config.Owner, config.Repo, existingCommentId, &comment) return err @@ -161,22 +174,21 @@ func getOptions(config *lcr.Config) (options.Options, error) { return options.GetOptions() } -func setOutputs(flagsRef lflags.FlagsRef) { - flagsModified := make([]string, 0, len(flagsRef.FlagsAdded)) - for k := range flagsRef.FlagsAdded { - flagsModified = append(flagsModified, k) - } +func setOutputs(config *lcr.Config, flagsRef references.ReferenceSummary) { + flagsModified := flagsRef.AddedKeys() setOutputsForChangedFlags("modified", flagsModified) - flagsRemoved := make([]string, 0, len(flagsRef.FlagsRemoved)) - for k := range flagsRef.FlagsRemoved { - flagsRemoved = append(flagsRemoved, k) + flagsRemoved := flagsRef.RemovedKeys() + setOutputsForChangedFlags("removed", flagsRemoved) + + if config.CheckExtinctions { + setOutputsForChangedFlags("extinct", flagsRef.ExtinctKeys()) } - setOutputsForChangedFlags("removed", flagsModified) allChangedFlags := make([]string, 0, len(flagsModified)+len(flagsRemoved)) allChangedFlags = append(allChangedFlags, flagsModified...) allChangedFlags = append(allChangedFlags, flagsRemoved...) + sort.Strings(allChangedFlags) setOutputsForChangedFlags("changed", allChangedFlags) } diff --git a/search/search.go b/search/search.go index 6e944a26..a4e6f2f7 100644 --- a/search/search.go +++ b/search/search.go @@ -3,7 +3,6 @@ package search import ( "strings" - ldapi "github.com/launchdarkly/api-client-go/v13" laliases "github.com/launchdarkly/ld-find-code-refs/v2/aliases" "github.com/launchdarkly/ld-find-code-refs/v2/options" lsearch "github.com/launchdarkly/ld-find-code-refs/v2/search" @@ -11,12 +10,7 @@ import ( "github.com/launchdarkly/find-code-references-in-pull-request/internal/aliases" ) -func GetMatcher(opts options.Options, flags []ldapi.FeatureFlag, diffContents laliases.FileContentsMap) (matcher lsearch.Matcher, err error) { - flagKeys := make([]string, 0, len(flags)) - for _, flag := range flags { - flagKeys = append(flagKeys, flag.Key) - } - +func GetMatcher(opts options.Options, flagKeys []string, diffContents laliases.FileContentsMap) (matcher lsearch.Matcher, err error) { aliasesByFlagKey, err := aliases.GenerateAliases(opts, flagKeys, diffContents) if err != nil { return lsearch.Matcher{}, err @@ -24,8 +18,7 @@ func GetMatcher(opts options.Options, flags []ldapi.FeatureFlag, diffContents la delimiters := strings.Join(lsearch.GetDelimiters(opts), "") elements := make([]lsearch.ElementMatcher, 0, 1) - elements = append(elements, lsearch.NewElementMatcher(opts.ProjKey, "", delimiters, flagKeys, aliasesByFlagKey)) - + elements = append(elements, lsearch.NewElementMatcher(opts.ProjKey, opts.Dir, delimiters, flagKeys, aliasesByFlagKey)) matcher = lsearch.Matcher{ Elements: elements, } diff --git a/testdata/test b/testdata/test index 2ea99814..30fd7a89 100644 --- a/testdata/test +++ b/testdata/test @@ -1,11 +1,9 @@ show-widgets showWidgets -betaUi -betaUi show_widgets -beta_ui show-widgets old-pricing-banner -betaUi oldPricingBanner +show_widgets oldPricingBanner +mobile-app-promo-ios