Support commitfooter formatting

We want to enforce the existence of certain commit message footers.

We do this by introducing a formatter called

  fmt:commitfooter-FOOTER-NAME.DISAMBIGUATION-SUFFIX

Since commit message footers commonly have dashes (Signed-off-by,
Change-Id), we can't use '-' as separator between the
DISAMBIGUATION-SUFFIX and the language.

This is solved by changing the separator to '.'. This will invalidate
existing formatter configs.

Change-Id: Ibd6d878d3944746c723fc4d1f8699fe4fbc862f1
diff --git a/cmd/checker/checker.go b/cmd/checker/checker.go
index 6473174..418e4dd 100644
--- a/cmd/checker/checker.go
+++ b/cmd/checker/checker.go
@@ -73,14 +73,18 @@
 	hash := sha1.New()
 	hash.Write([]byte(repo))
 
-	uuid := fmt.Sprintf("%s:%s-%x", checkerScheme, language, hash.Sum(nil))
+	uuid := fmt.Sprintf("%s:%s.%x", checkerScheme, language, hash.Sum(nil))
+	cfg, ok := linter.GetFormatter(language)
+	if !ok {
+		return nil, fmt.Errorf("no checker for language %q", language)
+	}
 	in := gerrit.CheckerInput{
 		UUID:        uuid,
 		Name:        language + " formatting",
 		Repository:  repo,
 		Description: "check source code formatting.",
 		Status:      "ENABLED",
-		Query:       linter.Formatters[language].Query,
+		Query:       cfg.Query,
 	}
 
 	body, err := json.Marshal(&in)
@@ -108,7 +112,7 @@
 // checkerLanguage extracts the language to check for from a checker UUID.
 func checkerLanguage(uuid string) (string, bool) {
 	uuid = strings.TrimPrefix(uuid, checkerScheme+":")
-	fields := strings.Split(uuid, "-")
+	fields := strings.Split(uuid, ".")
 	if len(fields) != 2 {
 		return "", false
 	}
@@ -131,8 +135,9 @@
 // errIrrelevant is a marker error value used for checks that don't apply for a change.
 var errIrrelevant = errors.New("irrelevant")
 
-// checkChange checks a (change, patchset) for correct formatting in the given language. It returns
-// a list of complaints, or the errIrrelevant error if there is nothing to do.
+// checkChange checks a (change, patchset) for correct formatting in
+// the given language. It returns a list of complaints, or the
+// errIrrelevant error if there is nothing to do.
 func (c *gerritChecker) checkChange(changeID string, psID int, language string) ([]string, error) {
 	ch, err := c.server.GetChange(changeID, strconv.Itoa(psID))
 	if err != nil {
@@ -140,8 +145,8 @@
 	}
 	req := linter.FormatRequest{}
 	for n, f := range ch.Files {
-		cfg := linter.Formatters[language]
-		if cfg == nil {
+		cfg, ok := linter.GetFormatter(language)
+		if !ok {
 			return nil, fmt.Errorf("language %q not configured", language)
 		}
 		if !cfg.Regex.MatchString(n) {
@@ -191,13 +196,13 @@
 
 func (c *gerritChecker) Serve() {
 	for {
-		// TODO: real rate limiting.
 		wait, err := c.processPendingChecks()
 		if err != nil {
 			log.Printf("checkAllChecks: %v", err)
 		}
 		if wait {
-			time.Sleep(10 * time.Second)
+			// TODO: real rate limiting?
+			time.Sleep(c.delay)
 		}
 	}
 }
diff --git a/cmd/checker/checker_test.go b/cmd/checker/checker_test.go
index 9d2614e..511643f 100644
--- a/cmd/checker/checker_test.go
+++ b/cmd/checker/checker_test.go
@@ -25,6 +25,16 @@
 	"github.com/google/gerrit-linter/gerrit"
 )
 
+func TestSchemeLanguage(t *testing.T) {
+	lang, ok := checkerLanguage("fmt:commitfooter-Change-Id.1234")
+	if !ok {
+		t.Fatalf("checkerLanguage failed")
+	}
+	if want := "commitfooter-Change-Id"; lang != want {
+		t.Errorf("got %q, want %q", lang, want)
+	}
+}
+
 func urlParse(s string) url.URL {
 	u, err := url.Parse(s)
 	if err != nil {
@@ -34,12 +44,34 @@
 	return *u
 }
 
-type changeInfo struct {
+type ChangeInfo struct {
 	ChangeId string `json:"change_id"`
 	Number   int    `json:"_number"`
 }
 
-func TestGerrit(t *testing.T) {
+func createUpdateChecker(t *testing.T, gc *gerritChecker, formatter string) *gerrit.CheckerInfo {
+	checker, err := gc.PostChecker("gerrit-linter-test", "commitmsg", true)
+	if err != nil {
+		// create
+		checker, err = gc.PostChecker("gerrit-linter-test", "commitmsg", false)
+		if err != nil {
+			t.Fatalf("create PostChecker: %v", err)
+		}
+	}
+	return checker
+}
+
+type ChangeInput struct {
+	Project string `json:"project"`
+	Subject string `json:"subject"`
+	Branch  string `json:"branch"`
+}
+
+type EditMessageInput struct {
+	Message string `json:"message"`
+}
+
+func TestBasic(t *testing.T) {
 	g := gerrit.New(urlParse("http://localhost:8080/"))
 	g.Authenticator = gerrit.NewBasicAuth("admin:secret")
 	g.Debug = true
@@ -54,29 +86,29 @@
 	if _, err := g.GetPath("/projects/gerrit-linter-test/"); err != nil {
 		t.Fatalf("GetPath: %v", err)
 	}
-	msgChecker, err := gc.PostChecker("gerrit-linter-test", "commitmsg", true)
-	if err != nil {
-		// create
-		msgChecker, err = gc.PostChecker("gerrit-linter-test", "commitmsg", false)
-		if err != nil {
-			t.Fatalf("create PostChecker: %v", err)
-		}
+
+	msgChecker := createUpdateChecker(t, gc, "commitmsg")
+	footerLang := "commitfooter-User-Visible"
+	footerChecker := createUpdateChecker(t, gc, footerLang)
+
+	changeInput := ChangeInput{
+		Project: "gerrit-linter-test",
+		Subject: "my linter test change.",
+		Branch:  "master",
 	}
-	content, err := g.PostPath("a/changes/",
+	var change ChangeInfo
+	if err := g.PostPathJSON("a/changes/",
 		"application/json",
-		[]byte(`{
-  "project": "gerrit-linter-test",
-  "subject": "my linter test change.",
-  "branch": "master"}
-`))
-	if err != nil {
-		t.Fatal(err)
-	}
-	var change changeInfo
-	if err := gerrit.Unmarshal(content, &change); err != nil {
+		&changeInput, &change); err != nil {
 		t.Fatal(err)
 	}
 	log.Printf("created change %d", change.Number)
+	defer func() {
+		if err := g.PostPathJSON(fmt.Sprintf("a/changes/%d/abandon", change.Number),
+			"application/json", &EditMessageInput{Message: "test succeeded"}, &struct{}{}); err != nil {
+			log.Printf("abandon: %v", err)
+		}
+	}()
 
 	gc.processPendingChecks()
 
@@ -89,23 +121,23 @@
 		t.Fatalf("got %q, want %q", info.State, statusFail)
 	}
 
-	if _, err := g.PutPath(fmt.Sprintf("a/changes/%d/message", change.Number), "application/json", []byte(
-		fmt.Sprintf(`{"message": "New Commit message\n\nChange-Id: %s\n"}`, change.ChangeId))); err != nil {
+	ignored := ""
+	if err := g.PutPathJSON(fmt.Sprintf("a/changes/%d/message", change.Number), "application/json",
+		&EditMessageInput{Message: fmt.Sprintf("New Commit message\n\nChange-Id: %s\n", change.ChangeId)},
+		&ignored); err != nil {
 		t.Fatalf("edit message: %v", err)
 	}
 	gc.processPendingChecks()
 
-	info, err = g.GetCheck(strconv.Itoa(change.Number), 2, msgChecker.UUID)
-	if err != nil {
+	if info, err = g.GetCheck(strconv.Itoa(change.Number), 2, msgChecker.UUID); err != nil {
 		t.Fatalf("2nd GetCheck: %v", err)
-	}
-
-	if info.State != statusSuccessful.String() {
+	} else if info.State != statusSuccessful.String() {
 		t.Fatalf("got %q, want %q", info.State, statusSuccessful)
 	}
 
-	if _, err := g.PostPath(fmt.Sprintf("a/changes/%d/abandon", change.Number),
-		"application/json", []byte(`{"message": "test succeeded"}`)); err != nil {
-		t.Fatalf("abandon: %v", err)
+	if info, err = g.GetCheck(strconv.Itoa(change.Number), 2, footerChecker.UUID); err != nil {
+		t.Fatalf("2nd GetCheck: %v", err)
+	} else if info.State != statusSuccessful.String() {
+		t.Fatalf("got %q, want %q", info.State, statusSuccessful)
 	}
 }
diff --git a/gerrit/server.go b/gerrit/server.go
index c7e1c53..359b7fc 100644
--- a/gerrit/server.go
+++ b/gerrit/server.go
@@ -20,6 +20,7 @@
 	"encoding/json"
 	"fmt"
 	"io/ioutil"
+	"log"
 	"net/http"
 	"net/url"
 	"path"
@@ -89,6 +90,14 @@
 	return g.Get(&u)
 }
 
+func (g *Server) GetPathJSON(p string, data interface{}) error {
+	content, err := g.GetPath(p)
+	if err != nil {
+		return err
+	}
+	return Unmarshal(content, data)
+}
+
 // Do runs a HTTP request against the remote server.
 func (g *Server) Do(req *http.Request) (*http.Response, error) {
 	req.Header.Set("User-Agent", g.UserAgent)
@@ -136,6 +145,32 @@
 	return g.putPostPath("POST", path, contentType, content)
 }
 
+// PostPathJSON does a JSON based REST RPC.
+func (g *Server) PostPathJSON(path, contentType string, input, output interface{}) error {
+	return g.putPostPathJSON("POST", path, contentType, input, output)
+}
+
+// PutPathJSON does a JSON based REST RPC.
+func (g *Server) PutPathJSON(path, contentType string, input, output interface{}) error {
+	return g.putPostPathJSON("PUT", path, contentType, input, output)
+}
+
+// putPostPathJSON does a JSON based REST RPC.
+func (g *Server) putPostPathJSON(method, path, contentType string, input, output interface{}) error {
+	content, err := json.Marshal(input)
+	if err != nil {
+		return err
+	}
+
+	resp, err := g.putPostPath(method, path, contentType, content)
+	if err != nil {
+		log.Println(err, string(content))
+		return err
+	}
+
+	return Unmarshal(resp, output)
+}
+
 func (g *Server) putPostPath(method string, pth string, contentType string, content []byte) ([]byte, error) {
 	u := g.URL
 	u.Path = path.Join(u.Path, pth)
@@ -182,15 +217,10 @@
 
 // GetChange returns the Change (including file contents) for a given change.
 func (g *Server) GetChange(changeID string, revID string) (*Change, error) {
-	content, err := g.GetPath(fmt.Sprintf("changes/%s/revisions/%s/files/",
-		url.PathEscape(changeID), revID))
-	if err != nil {
-		return nil, err
-	}
-	content = bytes.TrimPrefix(content, jsonPrefix)
-
 	files := map[string]*File{}
-	if err := json.Unmarshal(content, &files); err != nil {
+	err := g.GetPathJSON(fmt.Sprintf("changes/%s/revisions/%s/files/",
+		url.PathEscape(changeID), revID), &files)
+	if err != nil {
 		return nil, err
 	}
 
@@ -254,37 +284,21 @@
 
 // PostCheck posts a single check result onto a change.
 func (s *Server) PostCheck(changeID string, psID int, input *CheckInput) (*CheckInfo, error) {
-	body, err := json.Marshal(input)
+	var output CheckInfo
+	err := s.PostPathJSON(fmt.Sprintf("a/changes/%s/revisions/%d/checks/", changeID, psID),
+		"application/json", input, &output)
 	if err != nil {
 		return nil, err
 	}
 
-	res, err := s.PostPath(fmt.Sprintf("a/changes/%s/revisions/%d/checks/", changeID, psID),
-		"application/json", body)
-	if err != nil {
-		return nil, err
-	}
-
-	var out CheckInfo
-	if err := Unmarshal(res, &out); err != nil {
-		return nil, err
-	}
-
-	return &out, nil
+	return &output, nil
 }
 
+// GetCheck returns info for a single check.
 func (s *Server) GetCheck(changeID string, psID int, uuid string) (*CheckInfo, error) {
-	u := s.URL
-	u.Path = path.Join(u.Path, fmt.Sprintf("changes/%s/revisions/%d/checks/%s", changeID, psID, uuid))
-	content, err := s.Get(&u)
-	if err != nil {
-		return nil, err
-	}
-
 	var out CheckInfo
-	if err := Unmarshal(content, &out); err != nil {
-		return nil, err
-	}
+	err := s.GetPathJSON(fmt.Sprintf("changes/%s/revisions/%d/checks/%s", changeID, psID, uuid),
+		&out)
 
-	return &out, nil
+	return &out, err
 }
diff --git a/server.go b/server.go
index f73678f..30f4089 100644
--- a/server.go
+++ b/server.go
@@ -47,8 +47,8 @@
 	Formatter Formatter
 }
 
-// Formatters holds all the formatters supported
-var Formatters = map[string]*FormatterConfig{
+// formatters holds all the formatters supported
+var formatters = map[string]*FormatterConfig{
 	"commitmsg": {
 		Regex:     regexp.MustCompile(`^/COMMIT_MSG$`),
 		Formatter: &commitMsgFormatter{},
@@ -63,7 +63,7 @@
 
 	gjf, err := exec.LookPath("google-java-format.jar")
 	if err == nil {
-		Formatters["java"] = &FormatterConfig{
+		formatters["java"] = &FormatterConfig{
 			Regex: regexp.MustCompile(`\.java$`),
 			Query: "ext:java",
 			Formatter: &toolFormatter{
@@ -77,7 +77,7 @@
 
 	bzl, err := exec.LookPath("buildifier")
 	if err == nil {
-		Formatters["bzl"] = &FormatterConfig{
+		formatters["bzl"] = &FormatterConfig{
 			Regex: regexp.MustCompile(`(\.bzl|/BUILD|^BUILD)$`),
 			Query: "(ext:bzl OR file:BUILD OR file:WORKSPACE)",
 			Formatter: &toolFormatter{
@@ -91,7 +91,7 @@
 
 	gofmt, err := exec.LookPath("gofmt")
 	if err == nil {
-		Formatters["go"] = &FormatterConfig{
+		formatters["go"] = &FormatterConfig{
 			Regex: regexp.MustCompile(`\.go$`),
 			Query: "ext:go",
 			Formatter: &toolFormatter{
@@ -104,16 +104,31 @@
 	}
 }
 
+func GetFormatter(lang string) (*FormatterConfig, bool) {
+	footerPrefix := "commitfooter-"
+	if strings.HasPrefix(lang, footerPrefix) {
+		return &FormatterConfig{
+			Regex: regexp.MustCompile(`^/COMMIT_MSG$`),
+			Formatter: &commitFooterFormatter{
+				Footer: lang[len(footerPrefix):],
+			},
+		}, true
+	}
+
+	cfg, ok := formatters[lang]
+	return cfg, ok
+}
+
 // IsSupported returns if the given language is supported.
 func IsSupported(lang string) bool {
-	_, ok := Formatters[lang]
+	_, ok := GetFormatter(lang)
 	return ok
 }
 
 // SupportedLanguages returns a list of languages.
 func SupportedLanguages() []string {
 	var r []string
-	for l := range Formatters {
+	for l := range formatters {
 		r = append(r, l)
 	}
 	sort.Strings(r)
@@ -138,8 +153,10 @@
 
 	for language, fs := range splitByLang(req.Files) {
 		var buf bytes.Buffer
-		entry := Formatters[language]
-
+		entry, ok := GetFormatter(language)
+		if !ok {
+			return fmt.Errorf("linter: no formatter for %q", language)
+		}
 		out, err := entry.Formatter.Format(fs, &buf)
 		if err != nil {
 			return err
@@ -189,6 +206,57 @@
 	return ""
 }
 
+type commitFooterFormatter struct {
+	Footer string
+}
+
+func (f *commitFooterFormatter) Format(in []File, outSink io.Writer) (out []FormattedFile, err error) {
+	complaint := checkCommitFooter(string(in[0].Content), f.Footer)
+	ff := FormattedFile{}
+	ff.Name = in[0].Name
+	if complaint != "" {
+		ff.Message = complaint
+	} else {
+		ff.Content = in[0].Content
+	}
+	out = append(out, ff)
+	return out, nil
+}
+
+func checkCommitFooter(message, footer string) string {
+	if len(footer) == 0 {
+		return "required footer should be non-empty"
+	}
+
+	blocks := strings.Split(message, "\n\n")
+	if len(blocks) < 2 {
+		return "gerrit changes must have two paragraphs."
+	}
+
+	footerBlock := blocks[len(blocks)-1]
+	lines := strings.Split(footerBlock, "\n")
+	for _, l := range lines {
+		fields := strings.SplitN(l, ":", 2)
+		if len(fields) < 2 {
+			continue
+		}
+
+		if fields[0] != footer {
+			continue
+		}
+
+		value := fields[1]
+		if !strings.HasPrefix(value, " ") {
+			return fmt.Sprintf("footer %q should have space after ':'", fields[1])
+		}
+
+		// length limit?
+		return ""
+	}
+
+	return fmt.Sprintf("footer %q not found", footer)
+}
+
 type toolFormatter struct {
 	bin  string
 	args []string
diff --git a/server_test.go b/server_test.go
index 4ae99a2..08b83df 100644
--- a/server_test.go
+++ b/server_test.go
@@ -42,3 +42,28 @@
 		}
 	}
 }
+
+func TestCommitFooter(t *testing.T) {
+	for in, want := range map[string]string{
+		`abc`: "two paragraphs",
+		`abc
+
+def
+`: "not found",
+		`abc.
+
+myfooter:abc`: "space after",
+		`abc
+
+Change-Id: Iabc123
+myfooter: value!`: "",
+	} {
+		got := checkCommitFooter(in, "myfooter")
+
+		if want == "" && got != "" {
+			t.Errorf("want empty, got %s", got)
+		} else if !strings.Contains(got, want) {
+			t.Errorf("got %s, want substring %s", got, want)
+		}
+	}
+}