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

@@ -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

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`)
 

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 {
+func makeKeywordsPat(words []string) *regexp.Regexp {
-	return regexp.MustCompile(`(?i)(?:\s|^|\(|\[)(` + strings.Join(keywords, `|`) + `):? $`)
+	acceptedWords := parseKeywords(words)
+	if len(acceptedWords) == 0 {
+		// Never match
+		return nil
+	}
+	return regexp.MustCompile(`(?i)(?:\s|^|\(|\[)(` + strings.Join(acceptedWords, `|`) + `):? $`)
 }
 
-func init() {
+func parseKeywords(words []string) []string {
-	issueCloseKeywordsPat = makeKeywordsPat(issueCloseKeywords)
+	acceptedWords := make([]string, 0, 5)
-	issueReopenKeywordsPat = makeKeywordsPat(issueReopenKeywords)
+	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])
+	newKeywords()
+	var m []int
+	if issueCloseKeywordsPat != nil {
+		m = issueCloseKeywordsPat.FindSubmatchIndex(content[:start])
 		if m != nil {
 			return XRefActionCloses, &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
 }

modules/references/references_test.go

@@ -12,9 +12,12 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
-func TestFindAllIssueReferences(t *testing.T) {
+type testFixture struct {
+	input    string
+	expected []testResult
+}
 
-	type result struct {
+type testResult struct {
 	Index          int64
 	Owner          string
 	Name           string
@@ -22,151 +25,123 @@ func TestFindAllIssueReferences(t *testing.T) {
 	Action         XRefAction
 	RefLocation    *RefSpan
 	ActionLocation *RefSpan
-	}
+}
 
-	type testFixture struct {
+func TestFindAllIssueReferences(t *testing.T) {
-		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
+	testFixtures(t, fixtures, "default")
-	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
 
 	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])
+		}
+	}
+}

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