Repository: gorilla/mux
Pull request #526 (2019-10-24 10:25:19)

Fixed golint warnings
Fixed:
1. mux.go:366:5: error var SkipRouter should have name of the form ErrFoo
2. regexp.go:184:9: if block ends with a return statement, so drop this else and outdent its block



Patch:
diff --git a/context.go b/context.go
deleted file mode 100644
index 665940a..0000000
--- a/context.go
+++ /dev/null
@@ -1,18 +0,0 @@
-package mux
-
-import (
-	"context"
-	"net/http"
-)
-
-func contextGet(r *http.Request, key interface{}) interface{} {
-	return r.Context().Value(key)
-}
-
-func contextSet(r *http.Request, key, val interface{}) *http.Request {
-	if val == nil {
-		return r
-	}
-
-	return r.WithContext(context.WithValue(r.Context(), key, val))
-}
diff --git a/middleware.go b/middleware.go
index cf2b26d..cb51c56 100644
--- a/middleware.go
+++ b/middleware.go
@@ -58,22 +58,17 @@ func CORSMethodMiddleware(r *Router) MiddlewareFunc {
 func getAllMethodsForRoute(r *Router, req *http.Request) ([]string, error) {
 	var allMethods []string
 
-	err := r.Walk(func(route *Route, _ *Router, _ []*Route) error {
-		for _, m := range route.matchers {
-			if _, ok := m.(*routeRegexp); ok {
-				if m.Match(req, &RouteMatch{}) {
-					methods, err := route.GetMethods()
-					if err != nil {
-						return err
-					}
-
-					allMethods = append(allMethods, methods...)
-				}
-				break
+	for _, route := range r.routes {
+		var match RouteMatch
+		if route.Match(req, &match) || match.MatchErr == ErrMethodMismatch {
+			methods, err := route.GetMethods()
+			if err != nil {
+				return nil, err
 			}
+
+			allMethods = append(allMethods, methods...)
 		}
-		return nil
-	})
+	}
 
-	return allMethods, err
+	return allMethods, nil
 }
diff --git a/mux.go b/mux.go
index 26f9582..c9ba647 100644
--- a/mux.go
+++ b/mux.go
@@ -5,6 +5,7 @@
 package mux
 
 import (
+	"context"
 	"errors"
 	"fmt"
 	"net/http"
@@ -58,8 +59,7 @@ type Router struct {
 
 	// If true, do not clear the request context after handling the request.
 	//
-	// Deprecated: No effect when go1.7+ is used, since the context is stored
-	// on the request itself.
+	// Deprecated: No effect, since the context is stored on the request itself.
 	KeepContext bool
 
 	// Slice of middlewares to be called after a match is found
@@ -195,8 +195,8 @@ func (r *Router) ServeHTTP(w http.ResponseWriter, req *http.Request) {
 	var handler http.Handler
 	if r.Match(req, &match) {
 		handler = match.Handler
-		req = setVars(req, match.Vars)
-		req = setCurrentRoute(req, match.Route)
+		req = requestWithVars(req, match.Vars)
+		req = requestWithRoute(req, match.Route)
 	}
 
 	if handler == nil && match.MatchErr == ErrMethodMismatch {
@@ -426,7 +426,7 @@ const (
 
 // Vars returns the route variables for the current request, if any.
 func Vars(r *http.Request) map[string]string {
-	if rv := contextGet(r, varsKey); rv != nil {
+	if rv := r.Context().Value(varsKey); rv != nil {
 		return rv.(map[string]string)
 	}
 	return nil
@@ -438,18 +438,20 @@ func Vars(r *http.Request) map[string]string {
 // after the handler returns, unless the KeepContext option is set on the
 // Router.
 func CurrentRoute(r *http.Request) *Route {
-	if rv := contextGet(r, routeKey); rv != nil {
+	if rv := r.Context().Value(routeKey); rv != nil {
 		return rv.(*Route)
 	}
 	return nil
 }
 
-func setVars(r *http.Request, val interface{}) *http.Request {
-	return contextSet(r, varsKey, val)
+func requestWithVars(r *http.Request, vars map[string]string) *http.Request {
+	ctx := context.WithValue(r.Context(), varsKey, vars)
+	return r.WithContext(ctx)
 }
 
-func setCurrentRoute(r *http.Request, val interface{}) *http.Request {
-	return contextSet(r, routeKey, val)
+func requestWithRoute(r *http.Request, route *Route) *http.Request {
+	ctx := context.WithValue(r.Context(), routeKey, route)
+	return r.WithContext(ctx)
 }
 
 // ----------------------------------------------------------------------------
diff --git a/regexp.go b/regexp.go
index 0293a92..b5a15ed 100644
--- a/regexp.go
+++ b/regexp.go
@@ -181,16 +181,16 @@ func (r *routeRegexp) Match(req *http.Request, match *RouteMatch) bool {
 			}
 		}
 		return r.regexp.MatchString(host)
-	} else {
-		if r.regexpType == regexpTypeQuery {
-			return r.matchQueryString(req)
-		}
-		path := req.URL.Path
-		if r.options.useEncodedPath {
-			path = req.URL.EscapedPath()
-		}
-		return r.regexp.MatchString(path)
 	}
+
+	if r.regexpType == regexpTypeQuery {
+		return r.matchQueryString(req)
+	}
+	path := req.URL.Path
+	if r.options.useEncodedPath {
+		path = req.URL.EscapedPath()
+	}
+	return r.regexp.MatchString(path)
 }
 
 // url builds a URL part using the given values.


Test patch:
diff --git a/context_test.go b/context_test.go
deleted file mode 100644
index d8a56b4..0000000
--- a/context_test.go
+++ /dev/null
@@ -1,30 +0,0 @@
-package mux
-
-import (
-	"context"
-	"net/http"
-	"testing"
-	"time"
-)
-
-func TestNativeContextMiddleware(t *testing.T) {
-	withTimeout := func(h http.Handler) http.Handler {
-		return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-			ctx, cancel := context.WithTimeout(r.Context(), time.Minute)
-			defer cancel()
-			h.ServeHTTP(w, r.WithContext(ctx))
-		})
-	}
-
-	r := NewRouter()
-	r.Handle("/path/{foo}", withTimeout(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		vars := Vars(r)
-		if vars["foo"] != "bar" {
-			t.Fatal("Expected foo var to be set")
-		}
-	})))
-
-	rec := NewRecorder()
-	req := newRequest("GET", "/path/bar")
-	r.ServeHTTP(rec, req)
-}
diff --git a/middleware_test.go b/middleware_test.go
index 27647af..e9f0ef5 100644
--- a/middleware_test.go
+++ b/middleware_test.go
@@ -478,6 +478,26 @@ func TestCORSMethodMiddleware(t *testing.T) {
 	}
 }
 
+func TestCORSMethodMiddlewareSubrouter(t *testing.T) {
+	router := NewRouter().StrictSlash(true)
+
+	subrouter := router.PathPrefix("/test").Subrouter()
+	subrouter.HandleFunc("/hello", stringHandler("a")).Methods(http.MethodGet, http.MethodOptions, http.MethodPost)
+	subrouter.HandleFunc("/hello/{name}", stringHandler("b")).Methods(http.MethodGet, http.MethodOptions)
+
+	subrouter.Use(CORSMethodMiddleware(subrouter))
+
+	rw := NewRecorder()
+	req := newRequest("GET", "/test/hello/asdf")
+	router.ServeHTTP(rw, req)
+
+	actualMethods := rw.Header().Get("Access-Control-Allow-Methods")
+	expectedMethods := "GET,OPTIONS"
+	if actualMethods != expectedMethods {
+		t.Fatalf("expected methods %q but got: %q", expectedMethods, actualMethods)
+	}
+}
+
 func TestMiddlewareOnMultiSubrouter(t *testing.T) {
 	first := "first"
 	second := "second"
diff --git a/mux_test.go b/mux_test.go
index 9a740bb..2d8d2b3 100644
--- a/mux_test.go
+++ b/mux_test.go
@@ -7,6 +7,7 @@ package mux
 import (
 	"bufio"
 	"bytes"
+	"context"
 	"errors"
 	"fmt"
 	"io/ioutil"
@@ -16,6 +17,7 @@ import (
 	"reflect"
 	"strings"
 	"testing"
+	"time"
 )
 
 func (r *Route) GoString() string {
@@ -682,8 +684,8 @@ func TestHeaders(t *testing.T) {
 		},
 		{
 			title:       "Headers route, regex header values to match",
-			route:       new(Route).Headers("foo", "ba[zr]"),
-			request:     newRequestHeaders("GET", "http://localhost", map[string]string{"foo": "bar"}),
+			route:       new(Route).HeadersRegexp("foo", "ba[zr]"),
+			request:     newRequestHeaders("GET", "http://localhost", map[string]string{"foo": "baw"}),
 			vars:        map[string]string{},
 			host:        "",
 			path:        "",
@@ -2804,6 +2806,28 @@ func TestSubrouterNotFound(t *testing.T) {
 	}
 }
 
+func TestContextMiddleware(t *testing.T) {
+	withTimeout := func(h http.Handler) http.Handler {
+		return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			ctx, cancel := context.WithTimeout(r.Context(), time.Minute)
+			defer cancel()
+			h.ServeHTTP(w, r.WithContext(ctx))
+		})
+	}
+
+	r := NewRouter()
+	r.Handle("/path/{foo}", withTimeout(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		vars := Vars(r)
+		if vars["foo"] != "bar" {
+			t.Fatal("Expected foo var to be set")
+		}
+	})))
+
+	rec := NewRecorder()
+	req := newRequest("GET", "/path/bar")
+	r.ServeHTTP(rec, req)
+}
+
 // mapToPairs converts a string map to a slice of string pairs
 func mapToPairs(m map[string]string) []string {
 	var i int
diff --git a/test_helpers.go b/test_helpers.go
index 32ecffd..5f5c496 100644
--- a/test_helpers.go
+++ b/test_helpers.go
@@ -15,5 +15,5 @@ import "net/http"
 // can be set by making a route that captures the required variables,
 // starting a server and sending the request to that server.
 func SetURLVars(r *http.Request, val map[string]string) *http.Request {
-	return setVars(r, val)
+	return requestWithVars(r, val)
 }