feat(#1726): add literal colon support for gin, echo, fiber#1879
Conversation
# Conflicts: # internal/test/parameters/path_edge_cases/path_edge_cases.gen.go
Builds on the original fix by escaping literal colons in the Fiber URI
converter as well, and corrects the escaping to match the register
templates' current behaviour.
The register templates render the converted path through toGoString
(strconv.Quote), so the converter must emit a single backslash (`\:`); the
quoting turns it into `\\:` in the generated source, which is the value the
router sees at runtime. The prior double-backslash escape predated the
templates' switch to toGoString and would emit `\\:` to the router, which
Echo rejects (Not Found) and Gin panics on. A shared escapeLiteralPathColons
helper now feeds SwaggerUriToEchoUri, SwaggerUriToGinUri, and
SwaggerUriToFiberUri.
The Fiber v3 handler template interpolated the path raw inside quotes; a
backslash would produce invalid Go, so it now uses toGoString like every
other server template. Iris and the `{}`-style routers (Chi, Gorilla,
net/http) are unaffected: Iris already routes literal colons correctly and
escaping breaks it, and the others do not treat `:` specially.
Adds a paths/ regression fixture that registers two colon paths sharing a
prefix (/pets:validate, /pets:generate) on Echo, Gin, Fiber v2, and Fiber v3
and asserts each dispatches to its own handler. Moves the existing
path_edge_cases fixture under paths/ and regenerates it with the escaped
route.
|
I've basically rewritten this PR as an additional commit. Thanks for submitting this ages ago, and sorry for such a long delay. |
Greptile SummaryThis PR fixes issue #1726 by teaching the code generator to escape literal
Confidence Score: 4/5The core fix is well-scoped and correct — existing paths without literal colons are unaffected, and paths with colons were already broken before this change, so there is no regression risk. The The
|
| Filename | Overview |
|---|---|
| pkg/codegen/utils.go | Adds escapeLiteralPathColons helper and calls it in SwaggerUriToEchoUri, SwaggerUriToFiberUri, and SwaggerUriToGinUri before path-param substitution — logic is correct and order-safe. Iris uses {} not : for params, so its converter correctly needs no change. |
| pkg/codegen/utils_test.go | Adds literal-colon test assertions for Echo, Gin, and Fiber URI converters, covering both standalone colons and colons following a path parameter. |
| pkg/codegen/templates/fiber-v3/fiber-handler.tmpl | Switches from embedding the raw path string directly in a Go string literal to passing it through toGoString (strconv.Quote), fixing a pre-existing gap where backslash escapes from swaggerUriToFiberUri would not have been properly quoted in the generated source. |
| internal/test/paths/colon_paths_test.go | Adds per-backend integration tests (Echo, Gin, Fiber v2, Fiber v3) verifying that /pets:validate and /pets:generate dispatch to distinct handlers. Directory placement creates a new top-level paths/ category not in the established structure. |
| internal/test/paths/path_edge_cases/path_edge_cases.gen.go | Package renamed from parameterspathedgecases to pathedgecases (directory moved from parameters/). Generated router registration for /pets:validate now correctly emits \\: escape in Go source. |
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
internal/test/paths/doc.go:1-10
**New `paths/` category not in established test structure**
The repository organises `internal/test/` into a fixed set of top-level categories (e.g. `parameters/`, `servers/`, `schemas/`, …); `paths/` does not appear in that list. The new colon-routing tests are about server route registration, so `internal/test/servers/` is the most natural home. A sibling directory there (e.g. `servers/colon_paths/`) with per-backend config files would fit the established structure without requiring a new top-level category.
The same concern applies to the relocation of `parameters/path_edge_cases/` into this new directory. Moving an already-established leaf out of `parameters/` into an unofficial category quietly breaks the taxonomy that the rules are meant to preserve.
Reviews (1): Last reviewed commit: "Complete literal-colon path support for ..." | Re-trigger Greptile
Closes #1726
This MR supersedes #1815