Skip to content

Commit cbab7cf

Browse files
dunglasclaude
andauthored
refactor!: rename error vars to ErrFoo and expand lint enforcement (#11)
* refactor!: rename exported error vars from FooError to ErrFoo BREAKING CHANGE: every exported error identifier changes name to match the staticcheck ST1012 convention that the Go standard library uses. Consumers referencing these variables must update their imports: NoBaseURLError -> ErrNoBaseURL UnexpectedEmptyStringError -> ErrUnexpectedEmptyString NonEmptySuffixError -> ErrNonEmptySuffix BadParserIndexError -> ErrBadParserIndex DuplicatePartNameError -> ErrDuplicatePartName RequiredTokenError -> ErrRequiredToken InvalidIPv6HostnameError -> ErrInvalidIPv6Hostname InvalidPortError -> ErrInvalidPort EmptyPartNameError -> ErrEmptyPartName InvalidModifierError -> ErrInvalidModifier InvalidPrefixOrSuffix -> ErrInvalidPrefixOrSuffix InvalidPartNameError -> ErrInvalidPartName TypeError -> ErrType Also drops the ST1012 exclusion from .golangci.yml since the codebase now satisfies it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore: address remaining golangci-lint findings and enforce them Expand the enabled linter set in .golangci.yml (err113, goconst, gocritic, intrange, modernize, perfsprint, thelper, unconvert, unparam, wastedassign, whitespace) and address every new finding: - wastedassign: collapse ExecInit's 8 dead var = "" initializations and one in parser.go where regexpValue was assigned and immediately reassigned. - gocritic (assignOp / ifElseChain / typeSwitchVar): use x += y, rewrite if-else chains to switch, switch on typed values. - intrange: for range b.N in benchmarks, for i := range len(portValue) in the port prefix check; keep escape.go as-is (i needed after loop) with a //nolint rationale. - modernize: replace interface{} with any in tests, use slices.Contains. - perfsprint: fmt.Sprintf("%d", i) -> strconv.Itoa(i) in test names. - unconvert: drop redundant int() around p.componentStart. - unparam: consumeRequiredToken no longer returns its *token since every caller discarded it. - whitespace: trim a stray trailing newline in buildExpected and a leading one in newExpectedResult. - err113: hoist "invalid hostname" to errInvalidHostname at package level, add two test-only sentinels for the WPT harness. - goconst: extract "pattern"/"url" into initTypePattern / initTypeURL. - thelper: t.Helper() in the three WPT test helpers. - Drop the dead commented //p.result.Hash/Search lines in the constructor parser and refactor the surrounding if/else into a switch. All 365 WPT cases + the regression tests still pass; golangci-lint reports 0 issues with the widened configuration. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 0d8189f commit cbab7cf

9 files changed

Lines changed: 156 additions & 153 deletions

.golangci.yml

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,22 @@
11
version: "2"
22

33
linters:
4+
enable:
5+
- err113
6+
- goconst
7+
- gocritic
8+
- intrange
9+
- modernize
10+
- perfsprint
11+
- thelper
12+
- unconvert
13+
- unparam
14+
- wastedassign
15+
- whitespace
416
settings:
517
staticcheck:
618
checks:
719
- "all"
8-
# ST1012: public API exports FooError variables (NoBaseURLError,
9-
# InvalidPortError, ...). Renaming to ErrFoo would break consumers.
10-
- "-ST1012"
1120
# ST1020/ST1021: exported symbols are documented with their
1221
# corresponding WHATWG spec URL, not a "Foo does..." sentence.
1322
# The URL is the most useful pointer for maintainers of a

benchmark_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ func BenchmarkNew(b *testing.B) {
4040
b.ReportAllocs()
4141
var p *urlpattern.URLPattern
4242
var err error
43-
for i := 0; i < b.N; i++ {
43+
for range b.N {
4444
p, err = urlpattern.New(bc.pattern, bc.baseURL, nil)
4545
if err != nil {
4646
b.Fatal(err)
@@ -60,7 +60,7 @@ func BenchmarkTest(b *testing.B) {
6060
b.Run(bc.name, func(b *testing.B) {
6161
b.ReportAllocs()
6262
var ok bool
63-
for i := 0; i < b.N; i++ {
63+
for range b.N {
6464
ok = p.Test(bc.input, "")
6565
}
6666
benchBoolSink = ok
@@ -77,7 +77,7 @@ func BenchmarkExec(b *testing.B) {
7777
b.Run(bc.name, func(b *testing.B) {
7878
b.ReportAllocs()
7979
var r *urlpattern.URLPatternResult
80-
for i := 0; i < b.N; i++ {
80+
for range b.N {
8181
r = p.Exec(bc.input, "")
8282
}
8383
benchResultSink = r

constructor_type_parser.go

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -68,15 +68,13 @@ func parseConstructorString(input string) (*URLPatternInit, error) {
6868
if p.state == stateInit {
6969
p.rewind()
7070

71-
if p.isHashPrefix() {
71+
switch {
72+
case p.isHashPrefix():
7273
p.changeState(stateHash, 1)
73-
} else if p.isSearchPrefix() {
74+
case p.isSearchPrefix():
7475
p.changeState(stateSearch, 1)
75-
//p.result.Hash = ""
76-
} else {
76+
default:
7777
p.changeState(statePathname, 0)
78-
//p.result.Hash = ""
79-
//p.result.Search = ""
8078
}
8179

8280
p.tokenIndex += p.tokenIncrement
@@ -160,26 +158,28 @@ func parseConstructorString(input string) (*URLPatternInit, error) {
160158
}
161159

162160
case stateHostname:
163-
if p.isIPV6Open() {
161+
switch {
162+
case p.isIPV6Open():
164163
p.hostnameIPv6BracketDepth++
165-
} else if p.isIPV6Close() {
164+
case p.isIPV6Close():
166165
p.hostnameIPv6BracketDepth--
167-
} else if p.isPortPrefix() && p.hostnameIPv6BracketDepth == 0 {
166+
case p.isPortPrefix() && p.hostnameIPv6BracketDepth == 0:
168167
p.changeState(statePort, 1)
169-
} else if p.isPathnameStart() {
168+
case p.isPathnameStart():
170169
p.changeState(statePathname, 0)
171-
} else if p.isSearchPrefix() {
170+
case p.isSearchPrefix():
172171
p.changeState(stateSearch, 1)
173-
} else if p.isHashPrefix() {
172+
case p.isHashPrefix():
174173
p.changeState(stateHash, 1)
175174
}
176175

177176
case statePort:
178-
if p.isPathnameStart() {
177+
switch {
178+
case p.isPathnameStart():
179179
p.changeState(statePathname, 0)
180-
} else if p.isSearchPrefix() {
180+
case p.isSearchPrefix():
181181
p.changeState(stateSearch, 1)
182-
} else if p.isHashPrefix() {
182+
case p.isHashPrefix():
183183
p.changeState(stateHash, 1)
184184
}
185185

@@ -351,15 +351,15 @@ func (p *constructorTypeParser) changeState(newState state, skip int) {
351351
}
352352

353353
p.state = newState
354-
p.tokenIndex = p.tokenIndex + skip
354+
p.tokenIndex += skip
355355
p.componentStart = p.tokenIndex
356356
p.tokenIncrement = 0
357357
}
358358

359359
// https://urlpattern.spec.whatwg.org/#make-a-component-string
360360
func (p *constructorTypeParser) makeComponentString() string {
361361
token := p.tokenList[p.tokenIndex]
362-
componentStartToken := p.getSafeToken(int(p.componentStart))
362+
componentStartToken := p.getSafeToken(p.componentStart)
363363
componentStartInputIndex := componentStartToken.index
364364
endIndex := token.index
365365

escape.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ func init() {
3535
func escapeRegexpString(s string) string {
3636
// A byte loop is correct because all metacharacters are ASCII.
3737
var i int
38-
for i = 0; i < len(s); i++ {
38+
for i = 0; i < len(s); i++ { //nolint:intrange // i is reused below
3939
if specialRegexp(s[i]) {
4040
break
4141
}
@@ -63,7 +63,7 @@ func escapeRegexpString(s string) string {
6363
func escapePatternString(s string) string {
6464
// A byte loop is correct because all metacharacters are ASCII.
6565
var i int
66-
for i = 0; i < len(s); i++ {
66+
for i = 0; i < len(s); i++ { //nolint:intrange // i is reused below
6767
if specialPattern(s[i]) {
6868
break
6969
}

parser.go

Lines changed: 30 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -39,14 +39,16 @@ var urlParser = url.NewParser()
3939
var hostnameParser = canonicalizer.New(canonicalizer.WithDefaultScheme("http"))
4040

4141
var (
42-
NonEmptySuffixError = errors.New("suffix must be the empty string")
43-
BadParserIndexError = errors.New("parser's index must be less than parser's token list size")
44-
DuplicatePartNameError = errors.New("duplicate name")
45-
RequiredTokenError = errors.New("missing required token")
46-
InvalidIPv6HostnameError = errors.New("invalid IPv6 hostname")
47-
InvalidPortError = errors.New("invalid port")
42+
ErrNonEmptySuffix = errors.New("suffix must be the empty string")
43+
ErrBadParserIndex = errors.New("parser's index must be less than parser's token list size")
44+
ErrDuplicatePartName = errors.New("duplicate name")
45+
ErrRequiredToken = errors.New("missing required token")
46+
ErrInvalidIPv6Hostname = errors.New("invalid IPv6 hostname")
47+
ErrInvalidPort = errors.New("invalid port")
4848
)
4949

50+
var errInvalidHostname = errors.New("invalid hostname")
51+
5052
// https://urlpattern.spec.whatwg.org/#encoding-callback
5153
type encodingCallback func(string) (string, error)
5254

@@ -88,7 +90,7 @@ func parsePatternString(input string, options options, encodingCallback encoding
8890
}
8991

9092
if prefix != "" && prefix != string(options.prefixCodePoint) {
91-
p.pendingFixedValue = p.pendingFixedValue + prefix
93+
p.pendingFixedValue += prefix
9294
prefix = ""
9395
}
9496

@@ -115,7 +117,7 @@ func parsePatternString(input string, options options, encodingCallback encoding
115117
}
116118
}
117119
if fixedToken != nil {
118-
p.pendingFixedValue = p.pendingFixedValue + fixedToken.value
120+
p.pendingFixedValue += fixedToken.value
119121

120122
continue
121123
}
@@ -146,7 +148,7 @@ func parsePatternString(input string, options options, encodingCallback encoding
146148
return nil, err
147149
}
148150

149-
if _, err := p.consumeRequiredToken(tokenClose); err != nil {
151+
if err := p.consumeRequiredToken(tokenClose); err != nil {
150152
return nil, fmt.Errorf("missing close token: %w", err)
151153
}
152154

@@ -166,7 +168,7 @@ func parsePatternString(input string, options options, encodingCallback encoding
166168
return nil, err
167169
}
168170

169-
if _, err := p.consumeRequiredToken(tokenEnd); err != nil {
171+
if err := p.consumeRequiredToken(tokenEnd); err != nil {
170172
return nil, fmt.Errorf("missing end token: %w", err)
171173
}
172174
}
@@ -189,7 +191,7 @@ type patternParser struct {
189191
func (p *patternParser) tryConsumeToken(tokenType tokenType) (*token, error) {
190192
// Assert: parser’s index is less than parser’s token list size.
191193
if p.index >= len(p.tokenList) {
192-
return nil, BadParserIndexError
194+
return nil, ErrBadParserIndex
193195
}
194196

195197
nextToken := p.tokenList[p.index]
@@ -265,7 +267,7 @@ func (p *patternParser) addPart(prefix string, nameToken *token, regexpOrWildcar
265267
}
266268

267269
if nameToken == nil && regexpOrWildcardToken == nil && modifier == partModifierNone {
268-
p.pendingFixedValue = p.pendingFixedValue + prefix
270+
p.pendingFixedValue += prefix
269271

270272
return nil
271273
}
@@ -277,7 +279,7 @@ func (p *patternParser) addPart(prefix string, nameToken *token, regexpOrWildcar
277279
if nameToken == nil && regexpOrWildcardToken == nil {
278280
// Assert: suffix is the empty string.
279281
if suffix != "" {
280-
return NonEmptySuffixError
282+
return ErrNonEmptySuffix
281283
}
282284

283285
if prefix == "" {
@@ -295,12 +297,13 @@ func (p *patternParser) addPart(prefix string, nameToken *token, regexpOrWildcar
295297
return nil
296298
}
297299

298-
regexpValue := ""
299-
if regexpOrWildcardToken == nil {
300+
var regexpValue string
301+
switch {
302+
case regexpOrWildcardToken == nil:
300303
regexpValue = p.segmentWildcardRegexp
301-
} else if regexpOrWildcardToken.tType == tokenAsterisk {
304+
case regexpOrWildcardToken.tType == tokenAsterisk:
302305
regexpValue = fullWildcardRegexpValue
303-
} else {
306+
default:
304307
regexpValue = regexpOrWildcardToken.value
305308
}
306309

@@ -312,7 +315,6 @@ func (p *patternParser) addPart(prefix string, nameToken *token, regexpOrWildcar
312315
case fullWildcardRegexpValue:
313316
pType = partFullWildcard
314317
regexpValue = ""
315-
316318
}
317319

318320
name := ""
@@ -325,7 +327,7 @@ func (p *patternParser) addPart(prefix string, nameToken *token, regexpOrWildcar
325327

326328
// https://urlpattern.spec.whatwg.org/#is-a-duplicate-name
327329
if _, seen := p.seenNames[name]; seen {
328-
return DuplicatePartNameError
330+
return ErrDuplicatePartName
329331
}
330332

331333
encodedPrefix, err := p.encodingCallback(prefix)
@@ -373,16 +375,16 @@ func (p *patternParser) consumeText() (string, error) {
373375
}
374376

375377
// https://urlpattern.spec.whatwg.org/#consume-a-required-token
376-
func (p *patternParser) consumeRequiredToken(tokenType tokenType) (*token, error) {
378+
func (p *patternParser) consumeRequiredToken(tokenType tokenType) error {
377379
result, err := p.tryConsumeToken(tokenType)
378380
if err != nil {
379-
return nil, err
381+
return err
380382
}
381383
if result == nil {
382-
return nil, RequiredTokenError
384+
return ErrRequiredToken
383385
}
384386

385-
return result, nil
387+
return nil
386388
}
387389

388390
// https://urlpattern.spec.whatwg.org/#generate-a-segment-wildcard-regexp
@@ -435,7 +437,7 @@ func canonicalizeHostname(hostnameValue, protocolValue string) (string, error) {
435437
if hostnameValue[0] != '[' {
436438
for _, c := range hostnameValue {
437439
if c == ':' {
438-
return "", errors.New("invalid hostname")
440+
return "", errInvalidHostname
439441
}
440442
}
441443
}
@@ -478,16 +480,17 @@ func canonicalizePort(portValue, protocolValue string) (string, error) {
478480
// an ASCII digit (e.g. "invalid80"). Without this the URL library returns
479481
// an empty port instead of failing.
480482
firstDigit := false
481-
for i := 0; i < len(portValue); i++ {
483+
for i := range len(portValue) {
482484
c := portValue[i]
483485
if c == '\t' || c == '\n' || c == '\r' {
484486
continue
485487
}
486488
firstDigit = c >= '0' && c <= '9'
489+
487490
break
488491
}
489492
if !firstDigit {
490-
return "", InvalidPortError
493+
return "", ErrInvalidPort
491494
}
492495

493496
scheme := protocolValue
@@ -595,7 +598,7 @@ func canonicalizeIPv6Hostname(value string) (string, error) {
595598

596599
for _, c := range value {
597600
if c != '[' && c != ']' && c != ':' && !unicode.Is(unicode.ASCII_Hex_Digit, c) {
598-
return "", InvalidIPv6HostnameError
601+
return "", ErrInvalidIPv6Hostname
599602
}
600603

601604
result.WriteRune(unicode.ToLower(c))

parts.go

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,10 @@ const (
2020
)
2121

2222
var (
23-
EmptyPartNameError = errors.New("part's name must not be empty string")
24-
InvalidModifierError = errors.New(`part's modifier must be "zero-or-more" or "one-or-more"`)
25-
InvalidPrefixOrSuffix = errors.New("part's prefix is not the empty string or part's suffix is not the empty string")
26-
InvalidPartNameError = errors.New("part's name is not the empty string or null")
23+
ErrEmptyPartName = errors.New("part's name must not be empty string")
24+
ErrInvalidModifier = errors.New(`part's modifier must be "zero-or-more" or "one-or-more"`)
25+
ErrInvalidPrefixOrSuffix = errors.New("part's prefix is not the empty string or part's suffix is not the empty string")
26+
ErrInvalidPartName = errors.New("part's name is not the empty string or null")
2727
)
2828

2929
type partModifier uint8
@@ -81,7 +81,7 @@ func (pl partList) generateRegularExpressionAndNameList(options options) (string
8181

8282
// Assert: part's name is not the empty string.
8383
if p.name == "" {
84-
return "", nil, EmptyPartNameError
84+
return "", nil, ErrEmptyPartName
8585
}
8686

8787
nameList = append(nameList, p.name)
@@ -140,12 +140,12 @@ func (pl partList) generateRegularExpressionAndNameList(options options) (string
140140

141141
// Assert: part’s modifier is "zero-or-more" or "one-or-more".
142142
if p.modifier != partModifierZeroOrMore && p.modifier != partModifierOneOrMore {
143-
return "", nil, InvalidModifierError
143+
return "", nil, ErrInvalidModifier
144144
}
145145

146146
// Assert: part’s prefix is not the empty string or part’s suffix is not the empty string.
147147
if p.prefix == "" && p.suffix == "" {
148-
return "", nil, InvalidPrefixOrSuffix
148+
return "", nil, ErrInvalidPrefixOrSuffix
149149
}
150150

151151
result.WriteString("(?:")
@@ -233,7 +233,7 @@ func (pl partList) generatePatternString(options options) (string, error) {
233233

234234
// Assert: part’s name is not the empty string or null.
235235
if part.name == "" {
236-
return "", InvalidPartNameError
236+
return "", ErrInvalidPartName
237237
}
238238

239239
if needGrouping {

tokenizer.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import (
88
"golang.org/x/exp/utf8string"
99
)
1010

11-
var TypeError = errors.New("type error")
11+
var ErrType = errors.New("type error")
1212

1313
// https://wicg.github.io/urlpattern/#tokenizing
1414
type tokenizePolicy bool
@@ -241,7 +241,7 @@ func (t *tokenizer) addTokenWithDefaultPositionAndLength(tType tokenType) {
241241

242242
func (t *tokenizer) processTokenizingError(nextPosition, valuePosition int) error {
243243
if t.policy == tokenizePolicyStrict {
244-
return fmt.Errorf("%w: %#v", TypeError, t)
244+
return fmt.Errorf("%w: %#v", ErrType, t)
245245
}
246246

247247
t.addTokenWithDefaultLength(tokenInvalidChar, nextPosition, valuePosition)

0 commit comments

Comments
 (0)