From 42a4cd3392db543a8e6296d49535aedc5f9be326 Mon Sep 17 00:00:00 2001
From: Robert Griesemer <gri@golang.org>
Date: Mon, 7 Apr 2014 12:54:28 -0700
Subject: [PATCH] go.tools/godoc: sanitize function signatures for object index

Fixes golang/go#7703.

LGTM=bgarcia
R=bgarcia
CC=golang-codereviews
https://golang.org/cl/84410045
---
 godoc/godoc.go            | 48 ++++++++++++++++++++++++++++++++++++++-
 godoc/godoc_test.go       | 42 ++++++++++++++++++++++++++--------
 godoc/static/package.html |  6 ++---
 godoc/static/static.go    |  6 ++---
 4 files changed, 85 insertions(+), 17 deletions(-)

diff --git a/godoc/godoc.go b/godoc/godoc.go
index 3cb1f21713..5b9f95f7b6 100644
--- a/godoc/godoc.go
+++ b/godoc/godoc.go
@@ -76,6 +76,7 @@ func (p *Presentation) initFuncMap() {
 		"node_html":    p.node_htmlFunc,
 		"comment_html": comment_htmlFunc,
 		"comment_text": comment_textFunc,
+		"sanitize":     sanitizeFunc,
 
 		// support for URL attributes
 		"pkgLink":     pkgLinkFunc,
@@ -228,6 +229,50 @@ func comment_textFunc(comment, indent, preIndent string) string {
 	return buf.String()
 }
 
+// sanitizeFunc sanitizes the argument src by replacing newlines with
+// blanks, removing extra blanks, and by removing trailing whitespace
+// and commas before closing parentheses.
+func sanitizeFunc(src string) string {
+	buf := make([]byte, len(src))
+	j := 0      // buf index
+	comma := -1 // comma index if >= 0
+	for i := 0; i < len(src); i++ {
+		ch := src[i]
+		switch ch {
+		case '\t', '\n', ' ':
+			// ignore whitespace at the beginning, after a blank, or after opening parentheses
+			if j == 0 {
+				continue
+			}
+			if p := buf[j-1]; p == ' ' || p == '(' || p == '{' || p == '[' {
+				continue
+			}
+			// replace all whitespace with blanks
+			ch = ' '
+		case ',':
+			comma = j
+		case ')', '}', ']':
+			// remove any trailing comma
+			if comma >= 0 {
+				j = comma
+			}
+			// remove any trailing whitespace
+			if j > 0 && buf[j-1] == ' ' {
+				j--
+			}
+		default:
+			comma = -1
+		}
+		buf[j] = ch
+		j++
+	}
+	// remove trailing blank, if any
+	if j > 0 && buf[j-1] == ' ' {
+		j--
+	}
+	return string(buf[:j])
+}
+
 type PageInfo struct {
 	Dirname string // directory containing the package
 	Err     error  // error or nil
@@ -576,7 +621,8 @@ func (p *Presentation) writeNode(w io.Writer, fset *token.FileSet, x interface{}
 	}
 }
 
-// WriteNote writes x to w.
+// WriteNode writes x to w.
+// TODO(bgarcia) Is this method needed? It's just a wrapper for p.writeNode.
 func (p *Presentation) WriteNode(w io.Writer, fset *token.FileSet, x interface{}) {
 	p.writeNode(w, fset, x)
 }
diff --git a/godoc/godoc_test.go b/godoc/godoc_test.go
index 955f2f873f..76ce946a36 100644
--- a/godoc/godoc_test.go
+++ b/godoc/godoc_test.go
@@ -16,8 +16,8 @@ func TestPkgLinkFunc(t *testing.T) {
 		{"/src/pkg/fmt", "pkg/fmt"},
 		{"/fmt", "pkg/fmt"},
 	} {
-		if got, want := pkgLinkFunc(tc.path), tc.want; got != want {
-			t.Errorf("pkgLinkFunc(%v) = %v; want %v", tc.path, got, want)
+		if got := pkgLinkFunc(tc.path); got != tc.want {
+			t.Errorf("pkgLinkFunc(%v) = %v; want %v", tc.path, got, tc.want)
 		}
 	}
 }
@@ -38,8 +38,8 @@ func TestSrcPosLinkFunc(t *testing.T) {
 		{"fmt/print.go", 0, 0, 0, "/src/pkg/fmt/print.go"},
 		{"fmt/print.go", 0, 1, 5, "/src/pkg/fmt/print.go?s=1:5#L1"},
 	} {
-		if got, want := srcPosLinkFunc(tc.src, tc.line, tc.low, tc.high), tc.want; got != want {
-			t.Errorf("srcLinkFunc(%v, %v, %v, %v) = %v; want %v", tc.src, tc.line, tc.low, tc.high, got, want)
+		if got := srcPosLinkFunc(tc.src, tc.line, tc.low, tc.high); got != tc.want {
+			t.Errorf("srcLinkFunc(%v, %v, %v, %v) = %v; want %v", tc.src, tc.line, tc.low, tc.high, got, tc.want)
 		}
 	}
 }
@@ -54,8 +54,8 @@ func TestSrcLinkFunc(t *testing.T) {
 		{"/fmt/print.go", "/src/pkg/fmt/print.go"},
 		{"fmt/print.go", "/src/pkg/fmt/print.go"},
 	} {
-		if got, want := srcLinkFunc(tc.src), tc.want; got != want {
-			t.Errorf("srcLinkFunc(%v) = %v; want %v", tc.src, got, want)
+		if got := srcLinkFunc(tc.src); got != tc.want {
+			t.Errorf("srcLinkFunc(%v) = %v; want %v", tc.src, got, tc.want)
 		}
 	}
 }
@@ -72,8 +72,8 @@ func TestQueryLinkFunc(t *testing.T) {
 		{"src/pkg/fmt/print.go", "EOF", 33, "/src/pkg/fmt/print.go?h=EOF#L33"},
 		{"src/pkg/fmt/print.go", "a%3f+%26b", 1, "/src/pkg/fmt/print.go?h=a%3f+%26b#L1"},
 	} {
-		if got, want := queryLinkFunc(tc.src, tc.query, tc.line), tc.want; got != want {
-			t.Errorf("queryLinkFunc(%v, %v, %v) = %v; want %v", tc.src, tc.query, tc.line, got, want)
+		if got := queryLinkFunc(tc.src, tc.query, tc.line); got != tc.want {
+			t.Errorf("queryLinkFunc(%v, %v, %v) = %v; want %v", tc.src, tc.query, tc.line, got, tc.want)
 		}
 	}
 }
@@ -87,8 +87,30 @@ func TestDocLinkFunc(t *testing.T) {
 		{"/src/pkg/fmt", "Sprintf", "/pkg/fmt/#Sprintf"},
 		{"/src/pkg/fmt", "EOF", "/pkg/fmt/#EOF"},
 	} {
-		if got, want := docLinkFunc(tc.src, tc.ident), tc.want; got != want {
-			t.Errorf("docLinkFunc(%v, %v) = %v; want %v", tc.src, tc.ident, got, want)
+		if got := docLinkFunc(tc.src, tc.ident); got != tc.want {
+			t.Errorf("docLinkFunc(%v, %v) = %v; want %v", tc.src, tc.ident, got, tc.want)
+		}
+	}
+}
+
+func TestSanitizeFunc(t *testing.T) {
+	for _, tc := range []struct {
+		src  string
+		want string
+	}{
+		{},
+		{"foo", "foo"},
+		{"func   f()", "func f()"},
+		{"func f(a int,)", "func f(a int)"},
+		{"func f(a int,\n)", "func f(a int)"},
+		{"func f(\n\ta int,\n\tb int,\n\tc int,\n)", "func f(a int, b int, c int)"},
+		{"  (   a,   b,  c  )  ", "(a, b, c)"},
+		{"(  a,  b, c    int, foo   bar  ,  )", "(a, b, c int, foo bar)"},
+		{"{   a,   b}", "{a, b}"},
+		{"[   a,   b]", "[a, b]"},
+	} {
+		if got := sanitizeFunc(tc.src); got != tc.want {
+			t.Errorf("sanitizeFunc(%v) = %v; want %v", tc.src, got, tc.want)
 		}
 	}
 }
diff --git a/godoc/static/package.html b/godoc/static/package.html
index 875ebbcee1..897fa32ac7 100644
--- a/godoc/static/package.html
+++ b/godoc/static/package.html
@@ -65,18 +65,18 @@
 			{{end}}
 			{{range .Funcs}}
 				{{$name_html := html .Name}}
-				<dd><a href="#{{$name_html}}">{{node_html $ .Decl false}}</a></dd>
+				<dd><a href="#{{$name_html}}">{{node_html $ .Decl false | sanitize}}</a></dd>
 			{{end}}
 			{{range .Types}}
 				{{$tname_html := html .Name}}
 				<dd><a href="#{{$tname_html}}">type {{$tname_html}}</a></dd>
 				{{range .Funcs}}
 					{{$name_html := html .Name}}
-					<dd>&nbsp; &nbsp; <a href="#{{$name_html}}">{{node_html $ .Decl false}}</a></dd>
+					<dd>&nbsp; &nbsp; <a href="#{{$name_html}}">{{node_html $ .Decl false | sanitize}}</a></dd>
 				{{end}}
 				{{range .Methods}}
 					{{$name_html := html .Name}}
-					<dd>&nbsp; &nbsp; <a href="#{{$tname_html}}.{{$name_html}}">{{node_html $ .Decl false}}</a></dd>
+					<dd>&nbsp; &nbsp; <a href="#{{$tname_html}}.{{$name_html}}">{{node_html $ .Decl false | sanitize}}</a></dd>
 				{{end}}
 			{{end}}
 			{{if $.Notes}}
diff --git a/godoc/static/static.go b/godoc/static/static.go
index 96dd87fb7f..0151eee43e 100644
--- a/godoc/static/static.go
+++ b/godoc/static/static.go
@@ -1292,18 +1292,18 @@ function cgAddChild(tree, ul, cgn) {
 			{{end}}
 			{{range .Funcs}}
 				{{$name_html := html .Name}}
-				<dd><a href="#{{$name_html}}">{{node_html $ .Decl false}}</a></dd>
+				<dd><a href="#{{$name_html}}">{{node_html $ .Decl false | sanitize}}</a></dd>
 			{{end}}
 			{{range .Types}}
 				{{$tname_html := html .Name}}
 				<dd><a href="#{{$tname_html}}">type {{$tname_html}}</a></dd>
 				{{range .Funcs}}
 					{{$name_html := html .Name}}
-					<dd>&nbsp; &nbsp; <a href="#{{$name_html}}">{{node_html $ .Decl false}}</a></dd>
+					<dd>&nbsp; &nbsp; <a href="#{{$name_html}}">{{node_html $ .Decl false | sanitize}}</a></dd>
 				{{end}}
 				{{range .Methods}}
 					{{$name_html := html .Name}}
-					<dd>&nbsp; &nbsp; <a href="#{{$tname_html}}.{{$name_html}}">{{node_html $ .Decl false}}</a></dd>
+					<dd>&nbsp; &nbsp; <a href="#{{$tname_html}}.{{$name_html}}">{{node_html $ .Decl false | sanitize}}</a></dd>
 				{{end}}
 			{{end}}
 			{{if $.Notes}}