From 425215ac751ec3c05cfc6c46aa2645e330506a71 Mon Sep 17 00:00:00 2001 From: Koichi Shiraishi Date: Sat, 27 Jun 2026 20:35:27 +0900 Subject: [PATCH 1/2] cleanup: keep URI maintenance surface focused Remove stale lint exclusions for files outside this module and trim redundant test scaffolding so future changes fail with clearer evidence. Keep production cleanup limited to path helper internals already covered by the existing URI path tests. --- .golangci.yaml | 48 ------------------------------------------------ change_test.go | 5 ----- fspath_test.go | 17 ++++++----------- posixpath.go | 18 ++++++++---------- vector_test.go | 16 +++++++--------- 5 files changed, 21 insertions(+), 83 deletions(-) diff --git a/.golangci.yaml b/.golangci.yaml index b1f3b5e..360747d 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -12,11 +12,6 @@ linters: # removed-in-v2 linters (deadcode, structcheck, varcheck, golint, scopelint, # maligned, exhaustivestruct, ifshort) are dropped; goerr113 -> err113, # gomnd -> mnd, exportloopref -> copyloopvar. - # - # exhaustive is intentionally not enabled: several switches over the wire - # enum types (MessageViewKind, fieldKind, idKind) deliberately fold the - # unhandled cases into a single default arm, and an explicit case-per-value - # would be dead duplication rather than added safety. enable: - asciicheck - bodyclose @@ -89,10 +84,6 @@ linters: enable-all: true disable: - fieldalignment - # The frame writers shadow the response error param with a scoped - # ctx.Err() check that returns before the param is reused, so the - # shadow is intentional and harmless. - - shadow lll: line-length: 150 tab-width: 1 @@ -135,45 +126,6 @@ linters: - testpackage - thelper - tparallel - # The server-serving surface deliberately ignores Close/Remove errors on - # teardown paths where there is nothing actionable to do with the error, - # and ListenAndServe's documented contract is to call net.Listen directly. - - path: serve\.go - linters: - - errcheck - - noctx - - gocognit - - gocyclo - - funlen - # The hand-written scanner and string un-escaper are deliberately dense, - # single-pass state machines whose perf depends on keeping the branch logic - # inline; their complexity is reviewed in the scanner gap-review comment in - # scan_test.go rather than spread across helper functions. - - path: scan\.go - linters: - - gocognit - - gocyclo - - gocritic - - path: jsonstr\.go - linters: - - gocyclo - # The framer compares against the io.EOF and bufio.ErrBufferFull sentinels, - # which the bufio reader returns unwrapped, so a direct comparison is - # correct here; and its ReadFrame results are documented by the interface - # contract rather than by named returns. - - path: framer\.go - linters: - - errorlint - - gocritic - # The ID writer repeats the JSON null literal across the kind switches, so - # goconst's duplicate-string heuristic fires on intentional wire constants. - - path: id\.go - linters: - - goconst - # The dispatch goroutine intentionally fires the handler without checking - # its returned (connection-level) error inline. - - path: handler\.go - text: "Error return value of `handler` is not checked" formatters: enable: diff --git a/change_test.go b/change_test.go index 2b1cf5d..eb952ed 100644 --- a/change_test.go +++ b/change_test.go @@ -47,11 +47,6 @@ func TestWith(t *testing.T) { change: Change{Authority: str("")}, want: "scheme:/path", }, - "success: remove authority with clear pointer": { - base: "scheme://authority/path", - change: Change{Authority: str("")}, - want: "scheme:/path", - }, "success: clear path leaves authority": { base: "scheme:/path", change: Change{Authority: str("authority"), Path: str("")}, diff --git a/fspath_test.go b/fspath_test.go index 669e9ca..4d63e80 100644 --- a/fspath_test.go +++ b/fspath_test.go @@ -169,10 +169,11 @@ func TestFsPathFor(t *testing.T) { func TestFileAndFsPathAllocationGates(t *testing.T) { tests := map[string]struct { - name string - alloc func() float64 + alloc func() float64 + maxAllocs float64 }{ "FileFor clean absolute uses at most one allocation": { + maxAllocs: 1, alloc: func() float64 { return testing.AllocsPerRun(1000, func() { u := FileFor(PlatformPOSIX, "/abs/clean/path.go") @@ -183,6 +184,7 @@ func TestFileAndFsPathAllocationGates(t *testing.T) { }, }, "FsPathFor clean posix file is zero allocation": { + maxAllocs: 0, alloc: func() float64 { u := MustParse("file:///home/user/x.go") return testing.AllocsPerRun(1000, func() { @@ -197,15 +199,8 @@ func TestFileAndFsPathAllocationGates(t *testing.T) { for name, tt := range tests { t.Run(name, func(t *testing.T) { allocs := tt.alloc() - switch name { - case "FileFor clean absolute uses at most one allocation": - if allocs > 1 { - t.Fatalf("allocs = %v, want <= 1", allocs) - } - case "FsPathFor clean posix file is zero allocation": - if allocs != 0 { - t.Fatalf("allocs = %v, want 0", allocs) - } + if allocs > tt.maxAllocs { + t.Fatalf("allocs = %v, want <= %v", allocs, tt.maxAllocs) } }) } diff --git a/posixpath.go b/posixpath.go index a993019..de80f5c 100644 --- a/posixpath.go +++ b/posixpath.go @@ -12,9 +12,8 @@ func posixNormalize(p string) string { } absolute := p[0] == '/' trailing := p[len(p)-1] == '/' - parts := strings.Split(p, "/") - stack := make([]string, 0, len(parts)) - for _, part := range parts { + stack := make([]string, 0, strings.Count(p, "/")+1) + for part := range strings.SplitSeq(p, "/") { switch part { case "", ".": continue @@ -46,21 +45,20 @@ func posixNormalize(p string) string { } func posixJoin(paths ...string) string { - var joined string + var joined strings.Builder for _, p := range paths { if p == "" { continue } - if joined == "" { - joined = p - } else { - joined += "/" + p + if joined.Len() > 0 { + joined.WriteByte('/') } + joined.WriteString(p) } - if joined == "" { + if joined.Len() == 0 { return "." } - return posixNormalize(joined) + return posixNormalize(joined.String()) } func posixResolve(paths ...string) string { diff --git a/vector_test.go b/vector_test.go index 1bffb2b..f5e350e 100644 --- a/vector_test.go +++ b/vector_test.go @@ -63,10 +63,10 @@ func TestVectors(t *testing.T) { if vectors.Contract != "go-comparable-canonical-uri" { t.Fatalf("vectors contract = %q, want go-comparable-canonical-uri", vectors.Contract) } - if !containsString(vectors.ReferenceGenerated, "parse.components.fromCanonicalReparse") { + if !slices.Contains(vectors.ReferenceGenerated, "parse.components.fromCanonicalReparse") { t.Fatalf("referenceGenerated = %v, want parse.components.fromCanonicalReparse", vectors.ReferenceGenerated) } - if !containsString(vectors.Curated, "errors") { + if !slices.Contains(vectors.Curated, "errors") { t.Fatalf("curated = %v, want errors", vectors.Curated) } for _, v := range vectors.Parse { @@ -110,7 +110,7 @@ func TestVectors(t *testing.T) { if err == nil { t.Fatal("parse succeeded, want error") } - if !errors.Is(err, sentinelForVectorError(v.Error)) { + if !errors.Is(err, sentinelForVectorError(t, v.Error)) { t.Fatalf("parse error = %v, want %q", err, v.Error) } }) @@ -156,10 +156,6 @@ func TestVectors(t *testing.T) { } } -func containsString(values []string, want string) bool { - return slices.Contains(values, want) -} - func readVectors(t *testing.T) vectorFile { t.Helper() data, err := os.ReadFile("testdata/vectors.json") @@ -173,7 +169,8 @@ func readVectors(t *testing.T) vectorFile { return vectors } -func sentinelForVectorError(s string) error { +func sentinelForVectorError(t *testing.T, s string) error { + t.Helper() switch s { case ErrMissingScheme.Error(): return ErrMissingScheme @@ -184,6 +181,7 @@ func sentinelForVectorError(s string) error { case ErrPathAuthority.Error(): return ErrPathAuthority default: - return errors.New(s) + t.Fatalf("unknown vector error %q", s) + return nil } } From 3b145b12272a0f3322cc9ac8bcc9af5eb3a3c684 Mon Sep 17 00:00:00 2001 From: Koichi Shiraishi Date: Sat, 27 Jun 2026 21:01:47 +0900 Subject: [PATCH 2/2] path: avoid avoidable posixJoin allocations Handle zero and single non-empty joins before constructing a builder, and grow the builder exactly for multi-part joins. This addresses the Gemini review thread on PR #5 without changing URI path semantics. --- posixpath.go | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/posixpath.go b/posixpath.go index de80f5c..d5391d9 100644 --- a/posixpath.go +++ b/posixpath.go @@ -45,7 +45,26 @@ func posixNormalize(p string) string { } func posixJoin(paths ...string) string { + totalLen := 0 + nonEmptyCount := 0 + onlyNonEmpty := "" + for _, p := range paths { + if p == "" { + continue + } + totalLen += len(p) + nonEmptyCount++ + onlyNonEmpty = p + } + if nonEmptyCount == 0 { + return "." + } + if nonEmptyCount == 1 { + return posixNormalize(onlyNonEmpty) + } + var joined strings.Builder + joined.Grow(totalLen + nonEmptyCount - 1) for _, p := range paths { if p == "" { continue @@ -55,9 +74,6 @@ func posixJoin(paths ...string) string { } joined.WriteString(p) } - if joined.Len() == 0 { - return "." - } return posixNormalize(joined.String()) }