From f9944c0e6929f1c314d0a6e8d38a5592ba6eee2c Mon Sep 17 00:00:00 2001 From: guillep2k <18600385+guillep2k@users.noreply.github.com> Date: Wed, 30 Oct 2019 09:43:59 -0300 Subject: [PATCH] Configurable close and reopen keywords for PRs (#8120) * Add settings for CloseKeywords and ReopenKeywords * Fix and improve tests * Use sync.Once() for initialization * Fix unintended exported function --- custom/conf/app.ini.sample | 4 + .../doc/advanced/config-cheat-sheet.en-us.md | 4 + modules/references/references.go | 62 ++++-- modules/references/references_test.go | 198 ++++++++++++------ modules/setting/repository.go | 8 + 5 files changed, 199 insertions(+), 77 deletions(-) diff --git a/custom/conf/app.ini.sample b/custom/conf/app.ini.sample index f0204bb06..e6ccab95d 100644 --- a/custom/conf/app.ini.sample +++ b/custom/conf/app.ini.sample @@ -69,6 +69,10 @@ MAX_FILES = 5 [repository.pull-request] ; List of prefixes used in Pull Request title to mark them as Work In Progress WORK_IN_PROGRESS_PREFIXES=WIP:,[WIP] +; List of keywords used in Pull Request comments to automatically close a related issue +CLOSE_KEYWORDS=close,closes,closed,fix,fixes,fixed,resolve,resolves,resolved +; List of keywords used in Pull Request comments to automatically reopen a related issue +REOPEN_KEYWORDS=reopen,reopens,reopened [repository.issue] ; List of reasons why a Pull Request or Issue can be locked diff --git a/docs/content/doc/advanced/config-cheat-sheet.en-us.md b/docs/content/doc/advanced/config-cheat-sheet.en-us.md index c2744b295..bcf871a3a 100644 --- a/docs/content/doc/advanced/config-cheat-sheet.en-us.md +++ b/docs/content/doc/advanced/config-cheat-sheet.en-us.md @@ -71,6 +71,10 @@ Values containing `#` or `;` must be quoted using `` ` `` or `"""`. - `WORK_IN_PROGRESS_PREFIXES`: **WIP:,\[WIP\]**: List of prefixes used in Pull Request title to mark them as Work In Progress +- `CLOSE_KEYWORDS`: **close**, **closes**, **closed**, **fix**, **fixes**, **fixed**, **resolve**, **resolves**, **resolved**: List of + keywords used in Pull Request comments to automatically close a related issue +- `REOPEN_KEYWORDS`: **reopen**, **reopens**, **reopened**: List of keywords used in Pull Request comments to automatically reopen + a related issue ### Repository - Issue (`repository.issue`) diff --git a/modules/references/references.go b/modules/references/references.go index 9c74d0d08..58a8da289 100644 --- a/modules/references/references.go +++ b/modules/references/references.go @@ -11,6 +11,7 @@ import ( "strings" "sync" + "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/markup/mdstripper" "code.gitea.io/gitea/modules/setting" ) @@ -35,12 +36,8 @@ var ( // e.g. gogits/gogs#12345 crossReferenceIssueNumericPattern = regexp.MustCompile(`(?:\s|^|\(|\[)([0-9a-zA-Z-_\.]+/[0-9a-zA-Z-_\.]+#[0-9]+)(?:\s|$|\)|\]|\.(\s|$))`) - // Same as GitHub. See - // https://help.github.com/articles/closing-issues-via-commit-messages - issueCloseKeywords = []string{"close", "closes", "closed", "fix", "fixes", "fixed", "resolve", "resolves", "resolved"} - issueReopenKeywords = []string{"reopen", "reopens", "reopened"} - issueCloseKeywordsPat, issueReopenKeywordsPat *regexp.Regexp + issueKeywordsOnce sync.Once giteaHostInit sync.Once giteaHost string @@ -107,13 +104,40 @@ type RefSpan struct { End int } -func makeKeywordsPat(keywords []string) *regexp.Regexp { - return regexp.MustCompile(`(?i)(?:\s|^|\(|\[)(` + strings.Join(keywords, `|`) + `):? $`) +func makeKeywordsPat(words []string) *regexp.Regexp { + acceptedWords := parseKeywords(words) + if len(acceptedWords) == 0 { + // Never match + return nil + } + return regexp.MustCompile(`(?i)(?:\s|^|\(|\[)(` + strings.Join(acceptedWords, `|`) + `):? $`) } -func init() { - issueCloseKeywordsPat = makeKeywordsPat(issueCloseKeywords) - issueReopenKeywordsPat = makeKeywordsPat(issueReopenKeywords) +func parseKeywords(words []string) []string { + acceptedWords := make([]string, 0, 5) + wordPat := regexp.MustCompile(`^[\pL]+$`) + for _, word := range words { + word = strings.ToLower(strings.TrimSpace(word)) + // Accept Unicode letter class runes (a-z, á, à, ä, ) + if wordPat.MatchString(word) { + acceptedWords = append(acceptedWords, word) + } else { + log.Info("Invalid keyword: %s", word) + } + } + return acceptedWords +} + +func newKeywords() { + issueKeywordsOnce.Do(func() { + // Delay initialization until after the settings module is initialized + doNewKeywords(setting.Repository.PullRequest.CloseKeywords, setting.Repository.PullRequest.ReopenKeywords) + }) +} + +func doNewKeywords(close []string, reopen []string) { + issueCloseKeywordsPat = makeKeywordsPat(close) + issueReopenKeywordsPat = makeKeywordsPat(reopen) } // getGiteaHostName returns a normalized string with the local host name, with no scheme or port information @@ -310,13 +334,19 @@ func getCrossReference(content []byte, start, end int, fromLink bool) *rawRefere } func findActionKeywords(content []byte, start int) (XRefAction, *RefSpan) { - m := issueCloseKeywordsPat.FindSubmatchIndex(content[:start]) - if m != nil { - return XRefActionCloses, &RefSpan{Start: m[2], End: m[3]} + newKeywords() + var m []int + if issueCloseKeywordsPat != nil { + m = issueCloseKeywordsPat.FindSubmatchIndex(content[:start]) + if m != nil { + return XRefActionCloses, &RefSpan{Start: m[2], End: m[3]} + } } - m = issueReopenKeywordsPat.FindSubmatchIndex(content[:start]) - if m != nil { - return XRefActionReopens, &RefSpan{Start: m[2], End: m[3]} + if issueReopenKeywordsPat != nil { + m = issueReopenKeywordsPat.FindSubmatchIndex(content[:start]) + if m != nil { + return XRefActionReopens, &RefSpan{Start: m[2], End: m[3]} + } } return XRefActionNone, nil } diff --git a/modules/references/references_test.go b/modules/references/references_test.go index f8153ffe3..52e9b4ff5 100644 --- a/modules/references/references_test.go +++ b/modules/references/references_test.go @@ -12,161 +12,136 @@ import ( "github.com/stretchr/testify/assert" ) +type testFixture struct { + input string + expected []testResult +} + +type testResult struct { + Index int64 + Owner string + Name string + Issue string + Action XRefAction + RefLocation *RefSpan + ActionLocation *RefSpan +} + func TestFindAllIssueReferences(t *testing.T) { - type result struct { - Index int64 - Owner string - Name string - Issue string - Action XRefAction - RefLocation *RefSpan - ActionLocation *RefSpan - } - - type testFixture struct { - input string - expected []result - } - fixtures := []testFixture{ { "Simply closes: #29 yes", - []result{ + []testResult{ {29, "", "", "29", XRefActionCloses, &RefSpan{Start: 15, End: 18}, &RefSpan{Start: 7, End: 13}}, }, }, { "#123 no, this is a title.", - []result{}, + []testResult{}, }, { " #124 yes, this is a reference.", - []result{ + []testResult{ {124, "", "", "124", XRefActionNone, &RefSpan{Start: 0, End: 4}, nil}, }, }, { "```\nThis is a code block.\n#723 no, it's a code block.```", - []result{}, + []testResult{}, }, { "This `#724` no, it's inline code.", - []result{}, + []testResult{}, }, { "This user3/repo4#200 yes.", - []result{ + []testResult{ {200, "user3", "repo4", "200", XRefActionNone, &RefSpan{Start: 5, End: 20}, nil}, }, }, { "This [one](#919) no, this is a URL fragment.", - []result{}, + []testResult{}, }, { "This [two](/user2/repo1/issues/921) yes.", - []result{ + []testResult{ {921, "user2", "repo1", "921", XRefActionNone, nil, nil}, }, }, { "This [three](/user2/repo1/pulls/922) yes.", - []result{ + []testResult{ {922, "user2", "repo1", "922", XRefActionNone, nil, nil}, }, }, { "This [four](http://gitea.com:3000/user3/repo4/issues/203) yes.", - []result{ + []testResult{ {203, "user3", "repo4", "203", XRefActionNone, nil, nil}, }, }, { "This [five](http://github.com/user3/repo4/issues/204) no.", - []result{}, + []testResult{}, }, { "This http://gitea.com:3000/user4/repo5/201 no, bad URL.", - []result{}, + []testResult{}, }, { "This http://gitea.com:3000/user4/repo5/pulls/202 yes.", - []result{ + []testResult{ {202, "user4", "repo5", "202", XRefActionNone, nil, nil}, }, }, { "This http://GiTeA.COM:3000/user4/repo6/pulls/205 yes.", - []result{ + []testResult{ {205, "user4", "repo6", "205", XRefActionNone, nil, nil}, }, }, { "Reopens #15 yes", - []result{ + []testResult{ {15, "", "", "15", XRefActionReopens, &RefSpan{Start: 8, End: 11}, &RefSpan{Start: 0, End: 7}}, }, }, { "This closes #20 for you yes", - []result{ + []testResult{ {20, "", "", "20", XRefActionCloses, &RefSpan{Start: 12, End: 15}, &RefSpan{Start: 5, End: 11}}, }, }, { "Do you fix user6/repo6#300 ? yes", - []result{ + []testResult{ {300, "user6", "repo6", "300", XRefActionCloses, &RefSpan{Start: 11, End: 26}, &RefSpan{Start: 7, End: 10}}, }, }, { "For 999 #1235 no keyword, but yes", - []result{ + []testResult{ {1235, "", "", "1235", XRefActionNone, &RefSpan{Start: 8, End: 13}, nil}, }, }, { "Which abc. #9434 same as above", - []result{ + []testResult{ {9434, "", "", "9434", XRefActionNone, &RefSpan{Start: 11, End: 16}, nil}, }, }, { "This closes #600 and reopens #599", - []result{ + []testResult{ {600, "", "", "600", XRefActionCloses, &RefSpan{Start: 12, End: 16}, &RefSpan{Start: 5, End: 11}}, {599, "", "", "599", XRefActionReopens, &RefSpan{Start: 29, End: 33}, &RefSpan{Start: 21, End: 28}}, }, }, } - // Save original value for other tests that may rely on it - prevURL := setting.AppURL - setting.AppURL = "https://gitea.com:3000/" - - for _, fixture := range fixtures { - expraw := make([]*rawReference, len(fixture.expected)) - for i, e := range fixture.expected { - expraw[i] = &rawReference{ - index: e.Index, - owner: e.Owner, - name: e.Name, - action: e.Action, - issue: e.Issue, - refLocation: e.RefLocation, - actionLocation: e.ActionLocation, - } - } - expref := rawToIssueReferenceList(expraw) - refs := FindAllIssueReferencesMarkdown(fixture.input) - assert.EqualValues(t, expref, refs, "Failed to parse: {%s}", fixture.input) - rawrefs := findAllIssueReferencesMarkdown(fixture.input) - assert.EqualValues(t, expraw, rawrefs, "Failed to parse: {%s}", fixture.input) - } - - // Restore for other tests that may rely on the original value - setting.AppURL = prevURL + testFixtures(t, fixtures, "default") type alnumFixture struct { input string @@ -203,6 +178,35 @@ func TestFindAllIssueReferences(t *testing.T) { } } +func testFixtures(t *testing.T, fixtures []testFixture, context string) { + // Save original value for other tests that may rely on it + prevURL := setting.AppURL + setting.AppURL = "https://gitea.com:3000/" + + for _, fixture := range fixtures { + expraw := make([]*rawReference, len(fixture.expected)) + for i, e := range fixture.expected { + expraw[i] = &rawReference{ + index: e.Index, + owner: e.Owner, + name: e.Name, + action: e.Action, + issue: e.Issue, + refLocation: e.RefLocation, + actionLocation: e.ActionLocation, + } + } + expref := rawToIssueReferenceList(expraw) + refs := FindAllIssueReferencesMarkdown(fixture.input) + assert.EqualValues(t, expref, refs, "[%s] Failed to parse: {%s}", context, fixture.input) + rawrefs := findAllIssueReferencesMarkdown(fixture.input) + assert.EqualValues(t, expraw, rawrefs, "[%s] Failed to parse: {%s}", context, fixture.input) + } + + // Restore for other tests that may rely on the original value + setting.AppURL = prevURL +} + func TestRegExp_mentionPattern(t *testing.T) { trueTestCases := []string{ "@Unknwon", @@ -294,3 +298,75 @@ func TestRegExp_issueAlphanumericPattern(t *testing.T) { assert.False(t, issueAlphanumericPattern.MatchString(testCase)) } } + +func TestCustomizeCloseKeywords(t *testing.T) { + fixtures := []testFixture{ + { + "Simplemente cierra: #29 yes", + []testResult{ + {29, "", "", "29", XRefActionCloses, &RefSpan{Start: 20, End: 23}, &RefSpan{Start: 12, End: 18}}, + }, + }, + { + "Closes: #123 no, this English.", + []testResult{ + {123, "", "", "123", XRefActionNone, &RefSpan{Start: 8, End: 12}, nil}, + }, + }, + { + "Cerró user6/repo6#300 yes", + []testResult{ + {300, "user6", "repo6", "300", XRefActionCloses, &RefSpan{Start: 7, End: 22}, &RefSpan{Start: 0, End: 6}}, + }, + }, + { + "Reabre user3/repo4#200 yes", + []testResult{ + {200, "user3", "repo4", "200", XRefActionReopens, &RefSpan{Start: 7, End: 22}, &RefSpan{Start: 0, End: 6}}, + }, + }, + } + + issueKeywordsOnce.Do(func() {}) + + doNewKeywords([]string{"cierra", "cerró"}, []string{"reabre"}) + testFixtures(t, fixtures, "spanish") + + // Restore default settings + doNewKeywords(setting.Repository.PullRequest.CloseKeywords, setting.Repository.PullRequest.ReopenKeywords) +} + +func TestParseCloseKeywords(t *testing.T) { + // Test parsing of CloseKeywords and ReopenKeywords + assert.Len(t, parseKeywords([]string{""}), 0) + assert.Len(t, parseKeywords([]string{" aa ", " bb ", "99", "#", "", "this is", "cc"}), 3) + + for _, test := range []struct { + pattern string + match string + expected string + }{ + {"close", "This PR will close ", "close"}, + {"cerró", "cerró ", "cerró"}, + {"cerró", "AQUÍ SE CERRÓ: ", "CERRÓ"}, + {"закрывается", "закрывается ", "закрывается"}, + {"κλείνει", "κλείνει: ", "κλείνει"}, + {"关闭", "关闭 ", "关闭"}, + {"閉じます", "閉じます ", "閉じます"}, + {",$!", "", ""}, + {"1234", "", ""}, + } { + // The patern only needs to match the part that precedes the reference. + // getCrossReference() takes care of finding the reference itself. + pat := makeKeywordsPat([]string{test.pattern}) + if test.expected == "" { + assert.Nil(t, pat) + } else { + assert.NotNil(t, pat) + res := pat.FindAllStringSubmatch(test.match, -1) + assert.Len(t, res, 1) + assert.Len(t, res[0], 2) + assert.EqualValues(t, test.expected, res[0][1]) + } + } +} diff --git a/modules/setting/repository.go b/modules/setting/repository.go index 19c68d003..3e183b6c9 100644 --- a/modules/setting/repository.go +++ b/modules/setting/repository.go @@ -59,6 +59,8 @@ var ( // Pull request settings PullRequest struct { WorkInProgressPrefixes []string + CloseKeywords []string + ReopenKeywords []string } `ini:"repository.pull-request"` // Issue Setting @@ -122,8 +124,14 @@ var ( // Pull request settings PullRequest: struct { WorkInProgressPrefixes []string + CloseKeywords []string + ReopenKeywords []string }{ WorkInProgressPrefixes: []string{"WIP:", "[WIP]"}, + // Same as GitHub. See + // https://help.github.com/articles/closing-issues-via-commit-messages + CloseKeywords: strings.Split("close,closes,closed,fix,fixes,fixed,resolve,resolves,resolved", ","), + ReopenKeywords: strings.Split("reopen,reopens,reopened", ","), }, // Issue settings