RFC: Update route syntax (#10)

* Init RFC

* WIP new wildcard delimiter

* Updating tests suite

* Fix test for named and wildcard parameters routes

* Fix benchmark

* Small improvement

* Fix benchmark

* Use IndexByte instead of Index for string

* Update README with the new route definition

* Add tests for remove

* Remove initial change on README.d

* Mark unreachable old validation condition as to delete

* Removing unnecessary validation checks

* Add golangci-lint action

* Configure linter

* Fix linter

* Fix linter

* Update README.md

.github/workflows/tests.yaml

@@ -12,9 +12,10 @@ jobs:
         go: [ '>=1.19' ]
     steps:
       - name: Set up Go
-        uses: actions/setup-go@v3
+        uses: actions/setup-go@v4
         with:
           go-version: ${{ matrix.go }}
+          cache: false
 
       - name: Check out code
         uses: actions/checkout@v3
@@ -28,4 +29,22 @@ jobs:
       - name: Upload coverage to Codecov
         uses: codecov/codecov-action@v3
         with:
-          flags: coverage.txt
+          flags: coverage.txt
+  lint:
+    name: Lint Fox
+    runs-on: ubuntu-latest
+    strategy:
+      matrix:
+        go: [ '>=1.19' ]
+    steps:
+      - name: Set up Go
+        uses: actions/setup-go@v4
+        with:
+          go-version: ${{ matrix.go }}
+          cache: false
+
+      - name: Check out code
+        uses: actions/checkout@v3
+
+      - name: Run linter
+        uses: golangci/golangci-lint-action@v3

.golangci.yml

@@ -0,0 +1,28 @@
+linters-settings:
+  govet:
+    check-shadowing: true
+
+linters:
+  disable-all: true
+  enable:
+    - errcheck
+    - gosimple
+    - ineffassign
+    - staticcheck
+    - typecheck
+    - unused
+    - govet
+    - gosec
+
+issues:
+  exclude-rules:
+    - path: _test\.go
+      linters:
+        - errcheck
+        - gosimple
+        - ineffassign
+        - staticcheck
+        - typecheck
+        - unused
+        - govet
+        - gosec

README.md

@@ -71,7 +71,7 @@ func main() {
 	r := fox.New()
 
 	Must(r.Handler(http.MethodGet, "/", WelcomeHandler))
-	Must(r.Handler(http.MethodGet, "/hello/:name", new(HelloHandler)))
+	Must(r.Handler(http.MethodGet, "/hello/{name}", new(HelloHandler)))
 
 	log.Fatalln(http.ListenAndServe(":8080", r))
 }
@@ -102,31 +102,38 @@ if errors.Is(err, fox.ErrRouteConflict) {
 ```
 
 #### Named parameters
-A route can be defined using placeholder (e.g `:name`). The values are accessible via `fox.Params`, which is just a slice of `fox.Param`.
+A route can be defined using placeholder (e.g `{name}`). The values are accessible via `fox.Params`, which is just a slice of `fox.Param`.
 The `Get` method is a helper to retrieve the value using the placeholder name.
 
 ```
-Pattern /avengers/:name
+Pattern /avengers/{name}
 
 /avengers/ironman       match
 /avengers/thor          match
 /avengers/hulk/angry    no match
 /avengers/              no match
 
-Pattern /users/uuid_:id
+Pattern /users/uuid:{id}
 
-/users/uuid_xyz         match
-/users/uuid             no match
+/users/uuid:123             match
+/users/uuid                 no match
 ```
 
 ### Catch all parameter
-Catch-all parameters can be used to match everything at the end of a route. The placeholder start with `*` followed by a name.
+Catch-all parameters can be used to match everything at the end of a route. The placeholder start with `*` followed by a regular
+named parameter (e.g. `*{name}`).
 ```
-Pattern /src/*filepath
+Pattern /src/*{filepath}
 
-/src/                   match
-/src/conf.txt           match
-/src/dir/config.txt     match
+/src/                       match
+/src/conf.txt               match
+/src/dir/config.txt         match
+
+Patter /src/file=*{path}
+
+/src/file=                  match
+/src/file=config.txt        match
+/src/file=/dir/config.txt   match
 ```
 
 #### Warning about params slice
@@ -168,7 +175,7 @@ As such threads that route requests should never encounter latency due to ongoin
 
 ### Managing routes a runtime
 #### Routing mutation
-In this example, the handler for `routes/:action` allow to dynamically register, update and remove handler for the
+In this example, the handler for `routes/{action}` allow to dynamically register, update and remove handler for the
 given route and method. Thanks to Fox's design, those actions are perfectly safe and may be executed concurrently.
 
 ```go
@@ -230,7 +237,7 @@ func (h *ActionHandler) ServeHTTP(w http.ResponseWriter, r *http.Request, params
 
 func main() {
 	r := fox.New()
-	Must(r.Handler(http.MethodPost, "/routes/:action", &ActionHandler{fox: r}))
+	Must(r.Handler(http.MethodPost, "/routes/{action}", &ActionHandler{fox: r}))
 	log.Fatalln(http.ListenAndServe(":8080", r))
 }
 
@@ -349,7 +356,7 @@ Fox itself implements the `http.Handler` interface which make easy to chain any
 provides convenient `fox.WrapF` and `fox.WrapH` adapter to be use with `http.Handler`. Named and catch all parameters are forwarded via the
 request context
 ```go
-_ = r.Handler(http.MethodGet, "/users/:id", fox.WrapF(func(w http.ResponseWriter, r *http.Request) {
+_ = r.Handler(http.MethodGet, "/users/{id}", fox.WrapF(func(w http.ResponseWriter, r *http.Request) {
     params := fox.ParamsFromContext(r.Context())
     _, _ = fmt.Fprintf(w, "user id: %s\n", params.Get("id"))
 }))
@@ -501,8 +508,9 @@ BenchmarkPat_GithubAll                                       550           21177
 ```
 
 ## Road to v1
-- [Update route syntax](https://github.com/tigerwill90/fox/pull/10#issue-1643728309)
-- [Route overlapping](https://github.com/tigerwill90/fox/pull/9#issue-1642887919)
+- [x] [Update route syntax](https://github.com/tigerwill90/fox/pull/10#issue-1643728309) @v0.6.0
+- [ ] [Route overlapping](https://github.com/tigerwill90/fox/pull/9#issue-1642887919) @v0.7.0
+- [ ] Collect feedback and polishing
 
 ## Contributions
 This project aims to provide a lightweight, high performance and easy to use http router. It purposely has a limited set of features and exposes a relatively low-level api.

iter_test.go

@@ -8,7 +8,7 @@ import (
 	"testing"
 )
 
-var routesCases = []string{"/fox/router", "/foo/bar/:baz", "/foo/bar/:baz/:name", "/john/doe/*args", "/john/doe"}
+var routesCases = []string{"/fox/router", "/foo/bar/{baz}", "/foo/bar/{baz}/{name}", "/john/doe/*{args}", "/john/doe"}
 
 func TestIterator_Rewind(t *testing.T) {
 	tree := New().Tree()
@@ -59,7 +59,7 @@ func TestIterator_SeekPrefix(t *testing.T) {
 		require.NoError(t, tree.Handler(http.MethodHead, rte, emptyHandler))
 	}
 
-	want := []string{"/foo/bar/:baz", "/foo/bar/:baz/:name"}
+	want := []string{"/foo/bar/{baz}", "/foo/bar/{baz}/{name}"}
 	results := make(map[string][]string)
 
 	it := NewIterator(tree)
@@ -81,7 +81,7 @@ func TestIterator_SeekMethodPrefix(t *testing.T) {
 		require.NoError(t, tree.Handler(http.MethodHead, rte, emptyHandler))
 	}
 
-	want := []string{"/foo/bar/:baz", "/foo/bar/:baz/:name"}
+	want := []string{"/foo/bar/{baz}", "/foo/bar/{baz}/{name}"}
 	results := make(map[string][]string)
 
 	it := NewIterator(tree)

node.go

@@ -63,7 +63,7 @@ func newNodeFromRef(key string, handler Handler, children []atomic.Pointer[node]
 	}
 	// TODO find a better way
 	if catchAllKey != "" {
-		suffix := "*" + catchAllKey
+		suffix := "*{" + catchAllKey + "}"
 		if !strings.HasSuffix(path, suffix) {
 			n.path += suffix
 		}

router.go

@@ -426,64 +426,87 @@ func findRootNode(method string, nodes []*node) int {
 	return -1
 }
 
+const (
+	stateDefault uint8 = iota
+	stateParam
+	stateCatchAll
+)
+
+// parseRoute parse and validate the route in a single pass.
 func parseRoute(path string) (string, string, int, error) {
+
 	if !strings.HasPrefix(path, "/") {
 		return "", "", -1, fmt.Errorf("path must start with '/': %w", ErrInvalidRoute)
 	}
 
-	routeType := func(key byte) string {
-		if key == '*' {
-			return "catch all"
-		}
-		return "param"
-	}
-
-	var n int
-	p := []byte(path)
-	for i, c := range p {
-		if c != '*' && c != ':' {
-			continue
-		}
-		n++
-
-		// /foo*
-		if p[i-1] != '/' && p[i] == '*' {
-			return "", "", -1, fmt.Errorf("missing '/' before catch all route segment: %w", ErrInvalidRoute)
-		}
-
-		// /foo/:
-		if i == len(p)-1 {
-			return "", "", -1, fmt.Errorf("missing argument name after %s operator: %w", routeType(c), ErrInvalidRoute)
-		}
-
-		// /foo/:/
-		if p[i+1] == '/' {
-			return "", "", -1, fmt.Errorf("missing argument name after %s operator: %w", routeType(c), ErrInvalidRoute)
-		}
-
-		if c == ':' {
-			for k := i + 1; k < len(path); k++ {
-				if path[k] == '/' {
-					break
+	state := stateDefault
+	previous := stateDefault
+	startCatchAll := 0
+	paramCnt := 0
+	inParam := false
+
+	i := 0
+	for i < len(path) {
+		switch state {
+		case stateParam:
+			if path[i] == '}' {
+				if !inParam {
+					return "", "", -1, fmt.Errorf("%w: missing parameter name between '{}'", ErrInvalidRoute)
 				}
-				// /foo/:abc:xyz
-				if path[k] == ':' {
-					return "", "", -1, fmt.Errorf("only one param per path segment is allowed: %w", ErrInvalidRoute)
+				inParam = false
+				if previous != stateCatchAll {
+					if i+1 < len(path) && path[i+1] != '/' {
+						return "", "", -1, fmt.Errorf("%w: unexpected character after '{param}'", ErrInvalidRoute)
+					}
+				} else {
+					if i+1 != len(path) {
+						return "", "", -1, fmt.Errorf("%w: catch-all '*{params}' are allowed only at the end of a route", ErrInvalidRoute)
+					}
 				}
+				state = stateDefault
+				i++
+				continue
 			}
-		}
 
-		if c == '*' {
-			for k := i + 1; k < len(path); k++ {
-				// /foo/*args/
-				if path[k] == '/' || path[k] == ':' {
-					return "", "", -1, fmt.Errorf("catch all are allowed only at the end of a route: %w", ErrInvalidRoute)
-				}
+			if path[i] == '/' || path[i] == '*' || path[i] == '{' {
+				return "", "", -1, fmt.Errorf("%w: unexpected character in '{params}'", ErrInvalidRoute)
+			}
+			inParam = true
+			i++
+
+		case stateCatchAll:
+			if path[i] != '{' {
+				return "", "", -1, fmt.Errorf("%w: unexpected character after '*' catch-all delimiter", ErrInvalidRoute)
 			}
-			return path[:i], path[i+1:], n, nil
+			startCatchAll = i
+			previous = state
+			state = stateParam
+			i++
+
+		default:
+			if path[i] == '{' {
+				state = stateParam
+				paramCnt++
+			} else if path[i] == '*' {
+				state = stateCatchAll
+				paramCnt++
+			}
+			i++
 		}
 	}
-	return path, "", n, nil
+
+	if state == stateParam {
+		return "", "", -1, fmt.Errorf("%w: unclosed '{params}'", ErrInvalidRoute)
+	}
+	if state == stateCatchAll {
+		return "", "", -1, fmt.Errorf("%w: missing '{params}' after '*' catch-all delimiter", ErrInvalidRoute)
+	}
+
+	if startCatchAll > 0 {
+		return path[:startCatchAll-1], path[startCatchAll+1 : len(path)-1], paramCnt, nil
+	}
+
+	return path, "", paramCnt, nil
 }
 
 func getRouteConflict(n *node) []string {