url: new package

This is just moving the URL code from package http into its own package,
which has been planned for a while.
Besides clarity, this also breaks a nascent dependency cycle the new template
package was about to introduce.

Add a gofix module, url, and use it to generate changes outside http and url.

Sadness about the churn, gladness about some of the naming improvements.

R=dsymonds, bradfitz, rsc, gustavo, r
CC=golang-dev
https://golang.org/cl/4893043

Author: robpike

misc/dashboard/builder/http.go

@@ -12,6 +12,7 @@ import (
 	"log"
 	"os"
 	"strconv"
+	"url"
 )
 
 type param map[string]string
@@ -26,7 +27,7 @@ func dash(meth, cmd string, resp interface{}, args param) os.Error {
 		log.Println("dash", cmd, args)
 	}
 	cmd = "http://" + *dashboard + "/" + cmd
-	vals := make(http.Values)
+	vals := make(url.Values)
 	for k, v := range args {
 		vals.Add(k, v)
 	}

src/cmd/godoc/main.go

@@ -44,6 +44,7 @@ import (
 	"runtime"
 	"strings"
 	"time"
+	"url"
 )
 
 const defaultAddr = ":6060" // default webserver address
@@ -160,7 +161,7 @@ func loggingHandler(h http.Handler) http.Handler {
 }
 
 func remoteSearch(query string) (res *http.Response, err os.Error) {
-	search := "/search?f=text&q=" + http.URLEscape(query)
+	search := "/search?f=text&q=" + url.QueryEscape(query)
 
 	// list of addresses to try
 	var addrs []string

src/cmd/gofix/Makefile

@@ -23,6 +23,7 @@ GOFILES=\
 	sortslice.go\
 	stringssplit.go\
 	typecheck.go\
+	url.go\
 
 include ../../Make.cmd
 

src/cmd/gofix/url.go

@@ -0,0 +1,116 @@
+// Copyright 2011 The Go Authors.  All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package main
+
+import (
+	"fmt"
+	"os"
+	"go/ast"
+)
+
+var _ fmt.Stringer
+var _ os.Error
+
+var urlFix = fix{
+	"url",
+	url,
+	`Move the URL pieces of package http into a new package, url.
+
+http://codereview.appspot.com/4893043
+`,
+}
+
+func init() {
+	register(urlFix)
+}
+
+var urlRenames = []struct{ in, out string }{
+	{"ParseURL", "Parse"},
+	{"ParseURLReference", "ParseWithReference"},
+	{"ParseQuery", "ParseQuery"},
+	{"Values", "Values"},
+	{"URLEscape", "QueryEscape"},
+	{"URLUnescape", "QueryUnescape"},
+	{"URLError", "Error"},
+	{"URLEscapeError", "EscapeError"},
+}
+
+func url(f *ast.File) bool {
+	if imports(f, "url") || !imports(f, "http") {
+		return false
+	}
+
+	fixed := false
+
+	// Update URL code.
+	urlWalk := func(n interface{}) {
+		// Is it an identifier?
+		if ident, ok := n.(*ast.Ident); ok && ident.Name == "url" {
+			ident.Name = "url_"
+			return
+		}
+		// Find declared identifiers called url that might be confused.
+		// TODO: Why does gofix not walk the Names in a ValueSpec?
+		// TODO: Just a bug; fix later as it will have consequences.
+		if valSpec, ok := n.(*ast.ValueSpec); ok {
+			for _, ident := range valSpec.Names {
+				if ident.Name == "url" {
+					ident.Name = "url_"
+				}
+			}
+		}
+		// Parameter and result names.
+		if fn, ok := n.(*ast.FuncType); ok {
+			fixed = urlDoFields(fn.Params) || fixed
+			fixed = urlDoFields(fn.Results) || fixed
+		}
+	}
+
+	// Fix up URL code and add import, at most once.
+	fix := func() {
+		if fixed {
+			return
+		}
+		walk(f, urlWalk)
+		addImport(f, "url")
+		fixed = true
+	}
+
+	walk(f, func(n interface{}) {
+		// Rename functions and methods.
+		if expr, ok := n.(ast.Expr); ok {
+			for _, s := range urlRenames {
+				if isPkgDot(expr, "http", s.in) {
+					fix()
+					expr.(*ast.SelectorExpr).X.(*ast.Ident).Name = "url"
+					expr.(*ast.SelectorExpr).Sel.Name = s.out
+					return
+				}
+			}
+		}
+	})
+
+	// Remove the http import if no longer needed.
+	if fixed && !usesImport(f, "http") {
+		deleteImport(f, "http")
+	}
+
+	return fixed
+}
+
+func urlDoFields(list *ast.FieldList) (fixed bool) {
+	if list == nil {
+		return
+	}
+	for _, field := range list.List {
+		for _, ident := range field.Names {
+			if ident.Name == "url" {
+				fixed = true
+				ident.Name = "url_"
+			}
+		}
+	}
+	return
+}

src/cmd/gofix/url_test.go

@@ -0,0 +1,147 @@
+// Copyright 2011 The Go Authors.  All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package main
+
+func init() {
+	addTestCases(urlTests)
+}
+
+var urlTests = []testCase{
+	{
+		Name: "url.0",
+		In: `package main
+
+import (
+	"http"
+)
+
+func f() {
+	http.ParseURL(a)
+	http.ParseURLReference(a)
+	http.ParseQuery(a)
+	m := http.Values{a: b}
+	http.URLEscape(a)
+	http.URLUnescape(a)
+	var x http.URLError
+	var y http.URLEscapeError
+}
+`,
+		Out: `package main
+
+import "url"
+
+func f() {
+	url.Parse(a)
+	url.ParseWithReference(a)
+	url.ParseQuery(a)
+	m := url.Values{a: b}
+	url.QueryEscape(a)
+	url.QueryUnescape(a)
+	var x url.Error
+	var y url.EscapeError
+}
+`,
+	},
+	{
+		Name: "url.1",
+		In: `package main
+
+import (
+	"http"
+)
+
+func f() {
+	http.ParseURL(a)
+	var x http.Request
+}
+`,
+		Out: `package main
+
+import (
+	"http"
+	"url"
+)
+
+func f() {
+	url.Parse(a)
+	var x http.Request
+}
+`,
+	},
+	{
+		Name: "url.2",
+		In: `package main
+
+import (
+	"http"
+)
+
+func f() {
+	http.ParseURL(a)
+	var url = 23
+	url, x := 45, y
+}
+
+func g(url string) string {
+	return url
+}
+
+func h() (url string) {
+	return url
+}
+`,
+		Out: `package main
+
+import "url"
+
+func f() {
+	url.Parse(a)
+	var url_ = 23
+	url_, x := 45, y
+}
+
+func g(url_ string) string {
+	return url_
+}
+
+func h() (url_ string) {
+	return url_
+}
+`,
+	},
+	{
+		Name: "url.3",
+		In: `package main
+
+import "http"
+
+type U struct{ url string }
+
+func f() {
+	var u U
+	u.url = "x"
+}
+
+func (url *T) m() string {
+	return url
+}
+`,
+		Out: `package main
+
+import "http"
+
+type U struct{ url string }
+
+func f() {
+	var u U
+	u.url = "x"
+}
+
+func (url *T) m() string {
+	return url
+}
+`,
+	},
+}

src/pkg/Makefile

@@ -164,6 +164,7 @@ DIRS=\
 	time\
 	try\
 	unicode\
+	url\
 	utf16\
 	utf8\
 	websocket\

src/pkg/exp/template/funcs.go

@@ -7,12 +7,12 @@ package template
 import (
 	"bytes"
 	"fmt"
-	"http"
 	"io"
 	"os"
 	"reflect"
 	"strings"
 	"unicode"
+	"url"
 	"utf8"
 )
 
@@ -364,5 +364,5 @@ func URLQueryEscaper(args ...interface{}) string {
 	if !ok {
 		s = fmt.Sprint(args...)
 	}
-	return http.URLEscape(s)
+	return url.QueryEscape(s)
 }

src/pkg/http/Makefile

@@ -22,6 +22,5 @@ GOFILES=\
 	status.go\
 	transfer.go\
 	transport.go\
-	url.go\
 
 include ../../Make.pkg

src/pkg/http/cgi/child.go

@@ -18,6 +18,7 @@ import (
 	"os"
 	"strconv"
 	"strings"
+	"url"
 )
 
 // Request returns the HTTP request as represented in the current
@@ -93,7 +94,7 @@ func RequestFromMap(params map[string]string) (*http.Request, os.Error) {
 		// Hostname is provided, so we can reasonably construct a URL,
 		// even if we have to assume 'http' for the scheme.
 		r.RawURL = "http://" + r.Host + params["REQUEST_URI"]
-		url, err := http.ParseURL(r.RawURL)
+		url, err := url.Parse(r.RawURL)
 		if err != nil {
 			return nil, os.NewError("cgi: failed to parse host and REQUEST_URI into a URL: " + r.RawURL)
 		}
@@ -103,7 +104,7 @@ func RequestFromMap(params map[string]string) (*http.Request, os.Error) {
 	// failed to parse
 	if r.URL == nil {
 		r.RawURL = params["REQUEST_URI"]
-		url, err := http.ParseURL(r.RawURL)
+		url, err := url.Parse(r.RawURL)
 		if err != nil {
 			return nil, os.NewError("cgi: failed to parse REQUEST_URI into a URL: " + r.RawURL)
 		}

src/pkg/http/cgi/host.go

@@ -276,7 +276,7 @@ func (h *Handler) printf(format string, v ...interface{}) {
 }
 
 func (h *Handler) handleInternalRedirect(rw http.ResponseWriter, req *http.Request, path string) {
-	url, err := req.URL.ParseURL(path)
+	url, err := req.URL.Parse(path)
 	if err != nil {
 		rw.WriteHeader(http.StatusInternalServerError)
 		h.printf("cgi: error resolving local URI path %q: %v", path, err)