Update FAR tool versions #179
Conversation
|
Skipping CI for Draft Pull Request. |
WalkthroughBumps Go to 1.24.x and Kubernetes libraries to v0.33.x, updates tool versions, and refreshes CRD generator annotations/descriptions. Vendored dependencies are extensively updated: fsnotify gains buffered/new APIs across backends, json-patch replaces encoding/json with an internal JSON stack and new options, xxhash adds seeded APIs, cbor v2 is newly vendored, and multiple legacy golang/protobuf v1 files are removed. Changes
Sequence Diagram(s)sequenceDiagram
participant App
participant FSNotify as fsnotify.Watcher
participant Backend as OS Backend
App->>FSNotify: NewBufferedWatcher(sz)
FSNotify->>Backend: init backend (inotify/kqueue/windows/fen)
App->>FSNotify: AddWith(path, opts...)
FSNotify->>Backend: start watching path
loop event loop
Backend-->>FSNotify: events/errors
FSNotify-->>App: Events / Errors channel send
end
App->>FSNotify: Close()
FSNotify->>Backend: stop + cleanup
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
razo7
changed the title
razo7
force-pushed
the
update-tools-apis-25.8
branch
from
c1bd5ea to
1521336
Compare
|
/test 4.20-openshift-e2e |
|
Regardless of this PR we should address the operator-sdk, kustomize update and the migration to the project layout of go/v4 (ECOPROJECT-1945) soon. IMHO, this issue has been left out for too long. |
go.mod
Outdated
| k8s.io/utils v0.0.0-20230726121419-3b25d923346b | ||
| sigs.k8s.io/controller-runtime v0.16.3 | ||
| go.uber.org/zap v1.27.0 | ||
| k8s.io/api v0.31.0 |
There was a problem hiding this comment.
any reason for this specific version? Why not the latest y version 1.33.4, or the latest z version 1.31.12 at least?
There was a problem hiding this comment.
any reason for this specific version? Why not the latest y version 1.33.4,
We can, and we will include that. I thought of not updating it because I wanted to be aligned with the modules controller-runtime uses, and it has breaking changes from the v0.20.0 version (webhook: Remove deprecated Defaulter and Validator (kubernetes-sigs/controller-runtime#2877 kubernetes-sigs/controller-runtime#2945)).
I wanted to address these breaking changes later on with the "migration to the project layout of go/v4 (ECOPROJECT-1945)" changes.
There was a problem hiding this comment.
ack, but please use 1.31.12 then
There was a problem hiding this comment.
Oh, you updated to 1.33.4 already. Let's see how it goes. As long as we don't get conflicts in go.mod it should work
|
/test 4.19-openshift-e2e |
|
|
/test 4.19-openshift-e2e |
razo7
force-pushed
the
update-tools-apis-25.8
branch
2 times, most recently
from
7df83e8 to
b3fadf8
Compare
|
/test 4.19-openshift-e2e |
razo7
changed the title
controller_gen, goImports, opm
…N and Ginkgo Update Golang, K8s, controller-runtime and Ginkgo with newer versions. Updating ENVTEST_VERSION using go get -u sigs.k8s.io/controller-runtime/tools/[email protected] command
Update config and bundle directories
Update module depedencices
Use K8s 1.33.4 version modules and a newer controller-runtime version
Update module depedencices
Malformed arguments - detected a flag after the package liste
razo7
force-pushed
the
update-tools-apis-25.8
branch
from
b3fadf8 to
1eff0c4
Compare
|
/test 4.19-openshift-e2e |
|
/test 4.20-openshift-e2e |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: razo7, slintes The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🔭 Outside diff range comments (3)
vendor/github.com/fsnotify/fsnotify/backend_windows.go (1)
690-701: BUG: Using unsafe.Sizeof on a slice returns header size, not buffer lengthOn ERROR_MORE_DATA you set n = uint32(unsafe.Sizeof(watch.buf)), which yields the size of the slice header (e.g., 24/32 bytes), not the backing buffer length. This will corrupt parsing and likely drop events. Use len(watch.buf).
Apply this fix:
- n = uint32(unsafe.Sizeof(watch.buf)) + n = uint32(len(watch.buf))vendor/github.com/go-openapi/swag/yaml.go (1)
276-286: *Bug: Passing pointer to interface leads to “unhandled type: interface{}” in json2yaml.
json2yaml(&val[i].Value)changes the dynamic type to*interface{}, which is not handled by the type switch and will trigger the default error path. Pass the value, not its address.Apply this fix:
- childNode, err := json2yaml(&val[i].Value) + childNode, err := json2yaml(val[i].Value)vendor/github.com/evanphx/json-patch/v5/merge.go (1)
123-177: Verify HTML escaping options are properly propagated.The refactored code introduces
ApplyOptionswithEscapeHTMLfield throughout the merge operations. However, thepruneDocNullsfunction (Line 82) passes a new&ApplyOptions{}instead of the providedoptions, which may lose the HTML escaping preference.func pruneDocNulls(doc *partialDoc, options *ApplyOptions) *partialDoc { for k, v := range doc.obj { if v == nil { - _ = doc.remove(k, &ApplyOptions{}) + _ = doc.remove(k, options) } else { pruneNulls(v, options) } } return doc }
🧹 Nitpick comments (43)
vendor/github.com/fxamacker/cbor/v2/CONTRIBUTING.md (3)
28-30: Unify unordered list markers for consistency (dash vs asterisk)The list under “Describe your issue” uses asterisks while the rest of the doc uses dashes. Align to a single style to satisfy common markdownlinters (MD004).
-* If it's a bug, please provide: **version of this library** and **Go** (`go version`), **unmodified error message**, and describe **how to reproduce it**. Also state **what you expected to happen** instead of the error. -* If you propose a change or addition, try to give an example how the improved code could look like or how to use it. -* If you found a compilation error, please confirm you're using a supported version of Go. If you are, then provide the output of `go version` first, followed by the complete error message. +- If it's a bug, please provide: **version of this library** and **Go** (`go version`), **unmodified error message**, and describe **how to reproduce it**. Also state **what you expected to happen** instead of the error. +- If you propose a change or addition, try to give an example of how the improved code could look or how to use it. +- If you found a compilation error, please confirm you're using a supported version of Go. If you are, then provide the output of `go version` first, followed by the complete error message.
11-11: Avoid repeating the same PII restriction twiceLines 11 and 34 convey the same PII restriction. Consider consolidating to one concise statement to reduce duplication while keeping the 1024-byte CBOR data guidance.
-Please do not send data that might contain personally identifiable information, even if you think you have permission. That type of support requires payment and a signed contract where I'm indemnified, held harmless, and defended by you for any data you send to me. +Please do not send data that might contain personally identifiable information, even if you think you have permission. That type of support requires payment and a signed contract where I'm indemnified, held harmless, and defended by you for any data you send to me. @@ -Please don't send data containing personally identifiable information, even if you think you have permission. That type of support requires payment and a contract where I'm indemnified, held harmless, and defended for any data you send to me. +<!-- Duplicate of PII restriction above removed to reduce repetition. -->Also applies to: 34-36
40-41: Minor grammar/flow improvements in CreditsPolish phrasing for readability.
-- This guide used nlohmann/json contribution guidelines for inspiration as suggested in issue #22. -- Special thanks to @lukseven for pointing out the contribution guidelines didn't mention signing requirements. +- This guide was inspired by the nlohmann/json contribution guidelines, as suggested in issue #22. +- Special thanks to @lukseven for pointing out that the contribution guidelines didn't mention signing requirements.vendor/github.com/go-logr/zapr/README.md (4)
5-6: Grammar: make the opening sentence complete“Can also be used…” lacks a subject and article. Suggest rewording for clarity.
-[Zap](https://github.com/uber-go/zap). Can also be used as -[slog](https://pkg.go.dev/log/slog) handler. +[Zap](https://github.com/uber-go/zap). It can also be used as a +[slog](https://pkg.go.dev/log/slog) handler.
39-39: Add a language identifier to the fenced code blockMarkdown linters prefer specifying the language for syntax highlighting.
-``` +```go
43-60: Replace tabs with spaces in code exampleThere are hard tabs in the example (MD010). Replace with spaces for consistency with the surrounding examples and typical Markdown style.
-import ( - "fmt" - "log/slog" - - "github.com/go-logr/logr/slogr" - "github.com/go-logr/zapr" - "go.uber.org/zap" -) +import ( + "fmt" + "log/slog" + + "github.com/go-logr/logr/slogr" + "github.com/go-logr/zapr" + "go.uber.org/zap" +) @@ -func main() { - var log *slog.Logger +func main() { + var log *slog.Logger @@ - zapLog, err := zap.NewDevelopment() - if err != nil { - panic(fmt.Sprintf("who watches the watchmen (%v)?", err)) - } - log = slog.New(slogr.NewSlogHandler(zapr.NewLogger(zapLog))) + zapLog, err := zap.NewDevelopment() + if err != nil { + panic(fmt.Sprintf("who watches the watchmen (%v)?", err)) + } + log = slog.New(slogr.NewSlogHandler(zapr.NewLogger(zapLog))) @@ - log.Info("Logr in action!", "the answer", 42) + log.Info("Logr in action!", "the answer", 42)
104-107: Capitalize Zapr at sentence startMinor polish; improves readability.
-The zapr `logr.LogSink` implementation also implements `logr.SlogHandler`. That +The Zapr `logr.LogSink` implementation also implements `logr.SlogHandler`. That enables `slogr.NewSlogHandler` to provide a `slog.Handler` which just passes parameters through to zapr. zapr handles special slog values (Group, LogValuer), regardless of which front-end API is used.Optionally also capitalize the second “zapr” in that paragraph for consistency:
-parameters through to zapr. zapr handles special slog values (Group, +parameters through to Zapr. Zapr handles special slog values (Group,vendor/github.com/fsnotify/fsnotify/mkdoc.zsh (4)
121-124: Fix typo in generated documentation: “to to destination” → “to the destination”.Small grammatical nit in the Add() docs here-doc that propagates into generated Go comments.
Apply this diff in the here-doc so future doc generation is clean:
-// will write to a temporary file which is then moved to to destination, +// will write to a temporary file which is then moved to the destination,
200-202: Pluralization nit: “Write event” → “Write events”.Minor grammar fix in the Events docs.
-// Some systems may send Write event for directories +// Some systems may send Write events for directories
65-69: Likely typo: “events for all times” → “events for all files”.The current sentence reads oddly; given surrounding context, “files” seems intended.
-// Sometimes it will send events for all times, sometimes it will send no +// Sometimes it will send events for all files, sometimes it will send no
243-247: Avoid hardcoded temp file path; use mktemp for safety and to prevent races.Using a fixed /tmp/x can collide across runs and users. mktemp is safer.
- head -n $(( start - 1 )) $file >/tmp/x - print -r -- $cmt >>/tmp/x - tail -n+$(( end + 1 )) $file >>/tmp/x - mv /tmp/x $file + local tmpfile=$(mktemp -t mkdoc.zsh.XXXXXX) + head -n $(( start - 1 )) "$file" >"$tmpfile" + print -r -- $cmt >>"$tmpfile" + tail -n+$(( end + 1 )) "$file" >>"$tmpfile" + mv "$tmpfile" "$file"vendor/github.com/fsnotify/fsnotify/backend_inotify.go (2)
483-491: Prefer errors.Is over Unwrap for os.ErrClosed detectionUsing errors.Is(err, os.ErrClosed) handles arbitrarily wrapped errors (e.g., *os.PathError). The current errors.Unwrap(...) == os.ErrClosed works only one layer deep.
Apply this small change:
- switch { - case errors.Unwrap(err) == os.ErrClosed: + switch { + case errors.Is(err, os.ErrClosed): return
494-503: Dead/unreachable branch and unused errno variablen < unix.SizeofInotifyEvent is already handled; the n < 0 branch will never be hit in Go, and errno is never set. Consider simplifying to reduce noise.
- if n < unix.SizeofInotifyEvent { - var err error - if n == 0 { - err = io.EOF // If EOF is received. This should really never happen. - } else if n < 0 { - err = errno // If an error occurred while reading. - } else { - err = errors.New("notify: short read in readEvents()") // Read was too short. - } + if n < unix.SizeofInotifyEvent { + var err error + if n == 0 { + err = io.EOF // This should really never happen. + } else { + err = errors.New("notify: short read in readEvents()") + }vendor/github.com/fsnotify/fsnotify/README.md (2)
129-139: Typo: “to to destination”Minor grammar nit.
- as many programs (especially editors) update files atomically: it will write to - a temporary file which is then moved to to destination, overwriting the original + as many programs (especially editors) update files atomically: it will write to + a temporary file which is then moved to the destination, overwriting the originalNote: since this is vendored upstream documentation, consider keeping local changes minimal and, ideally, sending a small upstream PR to avoid divergence.
83-87: Wording nit: “for all times” likely meant “for all files”Very minor clarity tweak.
- // Sometimes it will send events for all times, sometimes it will send no + // Sometimes it will send events for all files, sometimes it will send noAs this is vendored content, prefer fixing upstream.
vendor/github.com/fsnotify/fsnotify/backend_kqueue.go (2)
583-591: Handle error from sendDirectoryChangeEvents to avoid silent failuresRight now, errors from sendDirectoryChangeEvents are ignored in the Write-on-directory path. For consistency with the Remove path below, propagate via sendError and stop when closed.
- if path.isDir && event.Has(Write) && !event.Has(Remove) { - w.sendDirectoryChangeEvents(event.Name) + if path.isDir && event.Has(Write) && !event.Has(Remove) { + if err := w.sendDirectoryChangeEvents(event.Name); err != nil { + if !w.sendError(err) { + closed = true + continue + } + }
689-716: Consider continuing on per-file EACCES/EPERM when scanning a directorysendDirectoryChangeEvents returns nil as soon as it encounters an unreadable file (EACCES/EPERM), which can short-circuit processing of the rest of the directory and miss events. Prefer skipping that entry and continuing the loop.
- if err != nil { - // Don't need to send an error if this file isn't readable. - if errors.Is(err, unix.EACCES) || errors.Is(err, unix.EPERM) { - return nil - } - return fmt.Errorf("fsnotify.sendDirectoryChangeEvents: %w", err) - } + if err != nil { + // Skip unreadable entries, but keep scanning the rest. + if errors.Is(err, unix.EACCES) || errors.Is(err, unix.EPERM) { + continue + } + return fmt.Errorf("fsnotify.sendDirectoryChangeEvents: %w", err) + }vendor/github.com/fsnotify/fsnotify/backend_fen.go (4)
261-299: Idempotent Add and explicit symlink resolution — good; options noop is fineAddWith correctly:
- Returns ErrClosed if closed.
- Avoids duplicate watches via PathIsWatched.
- Resolves symlinks only for explicitly watched paths; tracks dirs vs. explicit file watches.
Minor: getOptions(opts...) result is intentionally discarded on non-Windows backends. Consider a short comment to preempt future “unused”/”noop” questions.
311-346: Gracefully handle Remove on already-deleted pathsIf the filesystem entry is gone but the event port still considers the path watched, Remove currently returns the os.Stat error. Prefer best-effort dissociation and a nil return to match typical Remove semantics.
Proposed change:
- stat, err := os.Stat(name) - if err != nil { - return err - } + stat, err := os.Stat(name) + if err != nil { + // Path may have been removed from the filesystem while still associated. + // Best-effort dissociate and return nil to honor Remove semantics. + if errors.Is(err, os.ErrNotExist) { + _ = w.port.DissociatePath(name) + return nil + } + return err + }
421-428: Defensive type assertion for event.Cookie to avoid panicsIf event.Cookie is ever not an os.FileMode (unexpected, but the assertion will panic), prefer a guarded assertion with error reporting + dissociation to prevent a tight loop.
Proposed change:
- var ( - events = event.Events - path = event.Path - fmode = event.Cookie.(os.FileMode) - reRegister = true - ) + var ( + events = event.Events + path = event.Path + reRegister = true + ) + fmode, ok := event.Cookie.(os.FileMode) + if !ok { + if !w.sendError(fmt.Errorf("unexpected cookie type %T for %q", event.Cookie, path)) { + return nil + } + _ = w.port.DissociatePath(path) + return nil + }
484-496: Typo in comment: “sudirectory” → “subdirectory”- // get here, the sudirectory is already gone. Clearly we were watching + // get here, the subdirectory is already gone. Clearly we were watchingvendor/github.com/go-openapi/swag/string_bytes.go (1)
5-8: Clarify read-only contract in hackStringBytes docWe confirmed
hackStringBytesis only used for read-only access (e.g., insplit.go), so to prevent undefined behavior if someone modifies the returned slice, let’s make the docstring explicit:
- File:
vendor/github.com/go-openapi/swag/string_bytes.go, lines 5–8-// hackStringBytes returns the (unsafe) underlying bytes slice of a string. +// hackStringBytes returns a read-only (unsafe) view of the underlying bytes of a string. // Do NOT modify the returned slice; mutating it results in undefined behavior. func hackStringBytes(str string) []byte { return unsafe.Slice(unsafe.StringData(str), len(str)) }vendor/github.com/go-openapi/swag/BENCHMARK.md (1)
11-22: Optional: specify fence languages and avoid hard tabs to keep markdownlint happy.Not blocking. If you want markdownlint to pass in strict environments:
- Add a language to fenced blocks (e.g.,
text).- Replace hard tabs with spaces in benchmark lines.
Example:
-``` +```textAlso applies to: 28-39, 41-52
vendor/github.com/go-openapi/swag/yaml.go (1)
351-353: Consider handling additional scalar types to reduce “unhandled type” errors.Depending on inputs, plain
int,uint, andfloat32may occur. Handling them avoids surprising failures.Proposed additions within the switch (before default):
case string: return &yaml.Node{ Kind: yaml.ScalarNode, Tag: yamlStringScalar, Value: val, }, nil +case int: + return &yaml.Node{ + Kind: yaml.ScalarNode, + Tag: yamlIntScalar, + Value: strconv.Itoa(val), + }, nil +case uint: + return &yaml.Node{ + Kind: yaml.ScalarNode, + Tag: yamlIntScalar, + Value: strconv.FormatUint(uint64(val), 10), + }, nil +case float32: + return &yaml.Node{ + Kind: yaml.ScalarNode, + Tag: yamlFloatScalar, + Value: strconv.FormatFloat(float64(val), 'f', -1, 32), + }, nil default: return nil, fmt.Errorf("unhandled type: %T", val)Makefile (1)
521-521: E2E: Corrected ginkgo flag ordering prevents the v2 CLI errorPlacing -coverprofile before ./test/e2e resolves “Malformed arguments - detected a flag after the package list.” Good fix.
For explicitness and future-proofing, consider making the subcommand explicit and using the equals form for coverprofile:
- $(GINKGO) -r --keep-going --require-suite --vv -coverprofile cover.out ./test/e2e + $(GINKGO) run -r --keep-going --require-suite --vv --coverprofile=cover.out ./test/e2eThis mirrors the examples in ginkgo v2 docs and avoids any ambiguity if defaults change.
vendor/github.com/fxamacker/cbor/v2/diagnose.go (2)
166-175: Default diag mode error ignored is acceptable but could be made explicitUsing DiagOptions{} with defaults should always succeed, so ignoring the error is fine. To aid future maintainers, consider a brief comment stating why the error is safely ignored.
-var defaultDiagMode, _ = DiagOptions{}.diagMode() +// Defaults produce a valid diagMode (DecOptions applies safe defaults), +// so the error can be safely ignored. +var defaultDiagMode, _ = DiagOptions{}.diagMode()
476-487: Unify buffer growth/encode patterns for readabilityTwo different patterns are used to grow and fill the buffer (Bytes()[Len():] with reslice/append vs. Bytes()[Len():Len()+n]). Both are correct, but the mixed styles make the code harder to scan. Consider standardizing on one approach (e.g., the Len(): open-ended + reslice pattern) for consistency.
Example for the no-whitespace base16 path:
- sz := hex.EncodedLen(len(val)) - di.w.Grow(sz) - dst := di.w.Bytes()[di.w.Len() : di.w.Len()+sz] - hex.Encode(dst, val) - di.w.Write(dst) + sz := hex.EncodedLen(len(val)) + di.w.Grow(sz) + dst := di.w.Bytes()[di.w.Len():] // zero-len slice with available cap + dst = dst[:sz] + hex.Encode(dst, val) + di.w.Write(dst)Same idea applies to the writeU16 helper and the base32/base32hex/base64 branches.
Also applies to: 530-535, 542-547, 552-557, 562-567
vendor/github.com/evanphx/json-patch/v5/internal/json/fuzz.go (1)
13-42: Focused fuzz harness guarded by build tag; safe for normal builds.The gofuzz-only entry exercises Marshal/Unmarshal roundtrips across any, map[string]any, and []any. Panics on mismatches are appropriate for fuzzing. No production impact due to build constraint.
If you plan to adopt “go test -fuzz” in CI later, consider adding a separate testing fuzz target (FuzzXxx(*testing.F)) outside vendor to avoid mixing fuzz strategies.
vendor/github.com/fxamacker/cbor/v2/encode_map.go (2)
19-30: Safe lifetime and cleanup of pooled reflect.ValuesGet/Put with SetZero in defers ensures the pooled Values don’t retain references across calls. One nit: consider guarding against a nil Get result defensively, but given the Pool has New initializers (Lines 78-89), it’s not a practical issue.
31-44: Happy-path streaming encode is straightforwardThe kvs == nil fast-path cleanly streams k,v without offset bookkeeping. Minor nit: the i loop var is unused and can be dropped for readability, but not worth deviating from upstream vendored code.
Please keep this vendor file aligned with upstream. If you regenerated vendor via “go mod vendor”, no action needed.
vendor/github.com/go-openapi/swag/initialism_index.go (3)
131-141: Keep allocation heuristic in sync after AddInitialismsAfter adding new initialisms,
maxAllocMatchesremains stale. Recompute it to keep allocation hints accurate for downstream code.Apply this diff:
func AddInitialisms(words ...string) { for _, word := range words { // commonInitialisms[upper(word)] = true commonInitialisms.add(upper(word)) } // sort again initialisms = commonInitialisms.sorted() initialismsRunes = asRunes(initialisms) initialismsUpperCased = asUpperCased(initialisms) + maxAllocMatches = maxAllocHeuristic(initialismsRunes) }
133-136: Defensive: skip empty/whitespace inputs in AddInitialismsAdding an empty string would create an empty initialism entry, which can lead to subtle issues (e.g., assumptions on first rune elsewhere). Ignore such inputs.
Apply this diff:
func AddInitialisms(words ...string) { for _, word := range words { + word = strings.TrimSpace(word) + if word == "" { + continue + } // commonInitialisms[upper(word)] = true commonInitialisms.add(upper(word)) }
27-37: Potential data race on global slices (optional)
initialisms,initialismsRunes, andinitialismsUpperCasedare reassigned without synchronization. If other goroutines read these concurrently (e.g., during name generation), this may race. Consider guarding updates with a RWMutex or publishing an immutable struct via atomic pointer swap.If you want, I can sketch a minimal atomic “snapshot” struct to publish these three slices safely.
Also applies to: 85-94, 131-141
vendor/github.com/fxamacker/cbor/v2/common.go (1)
149-156: Minor readability: use AI constants instead of raw byte rangeReadability nit: instead of checking
contentHeadbetween 0xf9 and 0xfb, derive additional information and compare to named constants for float encodings.Apply this diff:
- if t != cborTypePositiveInt && t != cborTypeNegativeInt && (contentHead < 0xf9 || contentHead > 0xfb) { + ai := getAdditionalInformation(contentHead) + if t != cborTypePositiveInt && t != cborTypeNegativeInt && + (ai < additionalInformationAsFloat16 || ai > additionalInformationAsFloat64) {vendor/github.com/go-openapi/swag/name_lexem.go (1)
62-72: Simplify first-rune extraction with utf8.DecodeRuneInStringCurrent loop breaks after the second rune to derive
rest. Usingutf8.DecodeRuneInStringis simpler and avoids a loop.Apply this diff:
- for i, orig := range l.original { - if i == 0 { - first = orig - continue - } - - if i > 0 { - rest = l.original[i:] - break - } - } + first, size := utf8.DecodeRuneInString(l.original) + if size < len(l.original) { + rest = l.original[size:] + }Also applies to: 74-82
vendor/github.com/evanphx/json-patch/v5/internal/json/scanner.go (1)
367-373: Consider consolidating Unicode escape validationThe four sequential state functions (stateInStringEscU, stateInStringEscU1, stateInStringEscU12, stateInStringEscU123) have identical validation logic. Consider refactoring to reduce code duplication.
+// isHexDigit returns true if c is a valid hexadecimal digit +func isHexDigit(c byte) bool { + return '0' <= c && c <= '9' || 'a' <= c && c <= 'f' || 'A' <= c && c <= 'F' +} + // stateInStringEscU is the state after reading `"\u` during a quoted string. func stateInStringEscU(s *scanner, c byte) int { - if '0' <= c && c <= '9' || 'a' <= c && c <= 'f' || 'A' <= c && c <= 'F' { + if isHexDigit(c) { s.step = stateInStringEscU1 return scanContinue } // numbers return s.error(c, "in \\u hexadecimal character escape") }Also applies to: 376-383, 386-393, 396-403
vendor/github.com/fxamacker/cbor/v2/structfields.go (1)
96-97: Consider preallocating the vTypes mapThe
vTypesmap starts with only one entry but could grow significantly for deeply nested structs. Consider preallocating with a reasonable initial capacity.- vTypes := map[reflect.Type]bool{t: true} // visited field types at less nested levels + vTypes := make(map[reflect.Type]bool, 8) // visited field types at less nested levels + vTypes[t] = truevendor/github.com/fxamacker/cbor/v2/cache.go (1)
96-108: Simple multierror implementation for backward compatibilityThe custom multierror implementation is a reasonable approach for maintaining Go 1.17 compatibility. Consider adding a TODO to migrate to errors.Join when the minimum Go version is updated.
// The stdlib errors.Join was introduced in Go 1.20, and we still support Go 1.17, so instead, // here's a very basic implementation of an aggregated error. +// TODO: Replace with errors.Join when minimum Go version >= 1.20 type multierror []errorvendor/github.com/go-openapi/swag/util.go (1)
34-40: Consider validating the GoNamePrefixFunc output.The
prefixFuncfunction assumes GoNamePrefixFunc returns a string starting with an uppercase letter, but doesn't validate this. Consider adding a check or using the default "X" prefix if the returned string doesn't meet the requirements.func prefixFunc(name, in string) string { if GoNamePrefixFunc == nil { return "X" + in } - return GoNamePrefixFunc(name) + in + prefix := GoNamePrefixFunc(name) + // Ensure prefix starts with an uppercase letter + if prefix == "" || !unicode.IsUpper(rune(prefix[0])) { + return "X" + in + } + return prefix + in }vendor/github.com/evanphx/json-patch/v5/merge.go (1)
275-277: Simplify redundant unmarshal wrapper.The
unmarshalfunction is a simple wrapper that just callsjson.UnmarshalValid. Consider removing this indirection and callingjson.UnmarshalValiddirectly throughout the code for better clarity.-func unmarshal(data []byte, into interface{}) error { - return json.UnmarshalValid(data, into) -} - // createArrayMergePatch will return an array of merge-patch documents capable // of converting the original document to the modified document for each // pair of JSON documents provided in the arrays. // Arrays of mismatched sizes will result in an error. func createArrayMergePatch(originalJSON, modifiedJSON []byte) ([]byte, error) { originalDocs := []json.RawMessage{} modifiedDocs := []json.RawMessage{} - err := unmarshal(originalJSON, &originalDocs) + err := json.UnmarshalValid(originalJSON, &originalDocs) if err != nil { return nil, errBadJSONDoc } - err = unmarshal(modifiedJSON, &modifiedDocs) + err = json.UnmarshalValid(modifiedJSON, &modifiedDocs) if err != nil { return nil, errBadJSONDoc }vendor/github.com/evanphx/json-patch/v5/patch.go (1)
228-236: Improve efficiency by using TrimSpace once.The
nextBytemethod trims whitespace character by character in a loop, which could be inefficient for strings with many leading spaces. Consider usingbytes.TrimSpaceonce instead.func (n *lazyNode) nextByte() byte { s := []byte(*n.raw) - - for unicode.IsSpace(rune(s[0])) { - s = s[1:] - } - - return s[0] + trimmed := bytes.TrimSpace(s) + if len(trimmed) == 0 { + return 0 // or handle empty case appropriately + } + return trimmed[0] }vendor/github.com/fxamacker/cbor/v2/stream.go (1)
222-226: Docstring nit: “array” → “map”.StartIndefiniteMap’s comment says “starts array encoding,” but it should say “starts map encoding.”
Apply:
-// StartIndefiniteMap starts array encoding of indefinite length. +// StartIndefiniteMap starts map encoding of indefinite length.vendor/github.com/fxamacker/cbor/v2/encode.go (1)
1444-1530: “Fast shuffle” is a deterministic deck cut; confirm that’s intended.For SortFastShuffle, using a single random cut (rand.Intn) produces a rotation, not a full shuffle, and given the global PRNG’s default seed, it’s deterministic across runs unless seeded elsewhere. That may be fine per docs (“not unbiased”), but confirm this meets your non-determinism or reproducibility expectations.
If a less predictable order is desired while keeping O(n), consider seeding math/rand at program init or using a cheap, local xorshift PRNG seeded per process. If deterministic-but-varied order is desired, incorporating a stable hash of the struct type as the “start” could also work.
| // Handle all children of the directory. | ||
| for _, entry := range files { | ||
| finfo, err := entry.Info() | ||
| if err != nil { | ||
| return err | ||
| } | ||
| err = handler(filepath.Join(path, finfo.Name()), finfo, false) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| } | ||
|
|
||
| // And finally handle the directory itself. | ||
| return handler(path, stat, follow) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Don’t follow symlinks for directory children; use Lstat and avoid name shadowing
handleDirectory intends to avoid following symlinks for non-explicit entries (follow=false), but entry.Info() follows symlinks. This can cause cookie/mode mismatches and misclassification (e.g., treating a symlink-to-dir as a dir). Use os.Lstat on the joined path.
Also, adopt a childPath variable to avoid path shadowing.
Proposed change:
- for _, entry := range files {
- finfo, err := entry.Info()
- if err != nil {
- return err
- }
- err = handler(filepath.Join(path, finfo.Name()), finfo, false)
- if err != nil {
- return err
- }
- }
+ for _, entry := range files {
+ childPath := filepath.Join(path, entry.Name())
+ finfo, err := os.Lstat(childPath)
+ if err != nil {
+ return err
+ }
+ if err := handler(childPath, finfo, false); err != nil {
+ return err
+ }
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Handle all children of the directory. | |
| for _, entry := range files { | |
| finfo, err := entry.Info() | |
| if err != nil { | |
| return err | |
| } | |
| err = handler(filepath.Join(path, finfo.Name()), finfo, false) | |
| if err != nil { | |
| return err | |
| } | |
| } | |
| // And finally handle the directory itself. | |
| return handler(path, stat, follow) | |
| } | |
| // Handle all children of the directory. | |
| for _, entry := range files { | |
| childPath := filepath.Join(path, entry.Name()) | |
| finfo, err := os.Lstat(childPath) | |
| if err != nil { | |
| return err | |
| } | |
| if err := handler(childPath, finfo, false); err != nil { | |
| return err | |
| } | |
| } | |
| // And finally handle the directory itself. | |
| return handler(path, stat, follow) | |
| } |
There was a problem hiding this comment.
ignore vendored code please on all PRs in this organization
There was a problem hiding this comment.
@slintes, understood! I'll ignore vendored code in my reviews for this organization going forward.
✏️ Learnings added
Learnt from: slintes
PR: medik8s/fence-agents-remediation#179
File: vendor/github.com/fsnotify/fsnotify/backend_fen.go:401-415
Timestamp: 2025-08-18T09:05:25.253Z
Learning: For PRs in the medik8s/fence-agents-remediation organization, ignore vendored code and do not provide suggestions or comments on files in vendor/ directories.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
| func (w *Watcher) updateDirectory(path string) error { | ||
| // The directory was modified, so we must find unwatched entities and watch | ||
| // them. If something was removed from the directory, nothing will happen, | ||
| // as everything else should still be watched. | ||
| files, err := os.ReadDir(path) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| for _, entry := range files { | ||
| path := filepath.Join(path, entry.Name()) | ||
| if w.port.PathIsWatched(path) { | ||
| continue | ||
| } | ||
|
|
||
| finfo, err := entry.Info() | ||
| if err != nil { | ||
| return err | ||
| } | ||
| err = w.associateFile(path, finfo, false) | ||
| if err != nil { | ||
| if !w.sendError(err) { | ||
| return nil | ||
| } | ||
| } | ||
| if !w.sendEvent(path, Create) { | ||
| return nil | ||
| } | ||
| } | ||
| return nil | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid following symlinks and variable shadowing in updateDirectory
- entry.Info() follows symlinks; use os.Lstat to align with FILE_NOFOLLOW for non-explicit entries.
- Avoid reassigning path within the loop for readability.
Proposed change:
- for _, entry := range files {
- path := filepath.Join(path, entry.Name())
- if w.port.PathIsWatched(path) {
- continue
- }
-
- finfo, err := entry.Info()
- if err != nil {
- return err
- }
- err = w.associateFile(path, finfo, false)
- if err != nil {
- if !w.sendError(err) {
- return nil
- }
- }
- if !w.sendEvent(path, Create) {
- return nil
- }
- }
+ for _, entry := range files {
+ childPath := filepath.Join(path, entry.Name())
+ if w.port.PathIsWatched(childPath) {
+ continue
+ }
+
+ finfo, err := os.Lstat(childPath)
+ if err != nil {
+ return err
+ }
+ if err := w.associateFile(childPath, finfo, false); err != nil {
+ if !w.sendError(err) {
+ return nil
+ }
+ }
+ if !w.sendEvent(childPath, Create) {
+ return nil
+ }
+ }| if flds[i].keyAsInt { | ||
| nameAsInt, numErr := strconv.Atoi(flds[i].name) | ||
| if numErr != nil { | ||
| errs = append(errs, errors.New("cbor: failed to parse field name \""+flds[i].name+"\" to int ("+numErr.Error()+")")) | ||
| break | ||
| } | ||
| flds[i].nameAsInt = int64(nameAsInt) | ||
| } |
There was a problem hiding this comment.
Potential integer overflow in field name parsing
The code parses field names to int using strconv.Atoi and then converts to int64. On 32-bit systems, this could cause issues if the field name represents a number outside the int32 range.
- nameAsInt, numErr := strconv.Atoi(flds[i].name)
+ nameAsInt, numErr := strconv.ParseInt(flds[i].name, 10, 64)
if numErr != nil {
errs = append(errs, errors.New("cbor: failed to parse field name \""+flds[i].name+"\" to int ("+numErr.Error()+")"))
break
}
- flds[i].nameAsInt = int64(nameAsInt)
+ flds[i].nameAsInt = nameAsInt📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if flds[i].keyAsInt { | |
| nameAsInt, numErr := strconv.Atoi(flds[i].name) | |
| if numErr != nil { | |
| errs = append(errs, errors.New("cbor: failed to parse field name \""+flds[i].name+"\" to int ("+numErr.Error()+")")) | |
| break | |
| } | |
| flds[i].nameAsInt = int64(nameAsInt) | |
| } | |
| if flds[i].keyAsInt { | |
| nameAsInt, numErr := strconv.ParseInt(flds[i].name, 10, 64) | |
| if numErr != nil { | |
| errs = append(errs, errors.New("cbor: failed to parse field name \""+flds[i].name+"\" to int ("+numErr.Error()+")")) | |
| break | |
| } | |
| flds[i].nameAsInt = nameAsInt | |
| } |
🤖 Prompt for AI Agents
In vendor/github.com/fxamacker/cbor/v2/cache.go around lines 120 to 127, the
code uses strconv.Atoi then converts the result to int64 which can overflow on
32-bit platforms; replace strconv.Atoi with strconv.ParseInt(flds[i].name, 10,
64) to directly parse a 64-bit integer, handle the returned error similarly, and
assign the parsed int64 result to flds[i].nameAsInt (no intermediate int
conversion) so parsing supports values outside 32-bit range.
| if flds[i].keyAsInt { | ||
| nameAsInt, numErr := strconv.Atoi(flds[i].name) | ||
| if numErr != nil { | ||
| err = errors.New("cbor: failed to parse field name \"" + flds[i].name + "\" to int (" + numErr.Error() + ")") | ||
| break | ||
| } | ||
| flds[i].nameAsInt = int64(nameAsInt) |
There was a problem hiding this comment.
Duplicate integer overflow issue in encoding path
Same issue as in the decoding path - using strconv.Atoi for parsing field names that will be stored as int64.
- nameAsInt, numErr := strconv.Atoi(flds[i].name)
+ nameAsInt, numErr := strconv.ParseInt(flds[i].name, 10, 64)
if numErr != nil {
err = errors.New("cbor: failed to parse field name \"" + flds[i].name + "\" to int (" + numErr.Error() + ")")
break
}
- flds[i].nameAsInt = int64(nameAsInt)
+ flds[i].nameAsInt = nameAsInt📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if flds[i].keyAsInt { | |
| nameAsInt, numErr := strconv.Atoi(flds[i].name) | |
| if numErr != nil { | |
| err = errors.New("cbor: failed to parse field name \"" + flds[i].name + "\" to int (" + numErr.Error() + ")") | |
| break | |
| } | |
| flds[i].nameAsInt = int64(nameAsInt) | |
| if flds[i].keyAsInt { | |
| nameAsInt, numErr := strconv.ParseInt(flds[i].name, 10, 64) | |
| if numErr != nil { | |
| err = errors.New("cbor: failed to parse field name \"" + flds[i].name + "\" to int (" + numErr.Error() + ")") | |
| break | |
| } | |
| flds[i].nameAsInt = nameAsInt | |
| } |
🤖 Prompt for AI Agents
In vendor/github.com/fxamacker/cbor/v2/cache.go around lines 247 to 253, the
code uses strconv.Atoi to parse field names to integers before assigning to an
int64, which can silently overflow on large values; replace strconv.Atoi with
strconv.ParseInt(flds[i].name, 10, 64) (or ParseUint if unsigned expected),
check and propagate parse errors, and assign the resulting int64 directly to
flds[i].nameAsInt so parsing respects 64-bit range and avoids overflow.
| // Use the capacity at the tail of the encode buffer as a staging area to rearrange the | ||
| // encoded pairs into sorted order. | ||
| e.Grow(kvTotalLen) | ||
| tmp := e.Bytes()[e.Len() : e.Len()+kvTotalLen] // Can use e.AvailableBuffer() in Go 1.21+. | ||
| dst := e.Bytes()[kvBeginOffset:] | ||
|
|
||
| if em.sort == SortBytewiseLexical { | ||
| sort.Sort(&bytewiseKeyValueSorter{kvs: kvs, data: dst}) | ||
| } else { | ||
| sort.Sort(&lengthFirstKeyValueSorter{kvs: kvs, data: dst}) | ||
| } | ||
|
|
||
| // This is where the encoded bytes are actually rearranged in the output buffer to reflect |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Bug: slicing beyond buffer length after Grow will panic.
After e.Grow(kvTotalLen), the buffer capacity increases but length stays the same. Slicing with two indices beyond len (tmp := e.Bytes()[e.Len():e.Len()+kvTotalLen]) is invalid and will panic. Use a full slice to capacity or AvailableBuffer (Go 1.21+) for the scratch area.
Apply this fix to safely use the tail capacity:
// Use the capacity at the tail of the encode buffer as a staging area to rearrange the
// encoded pairs into sorted order.
e.Grow(kvTotalLen)
- tmp := e.Bytes()[e.Len() : e.Len()+kvTotalLen] // Can use e.AvailableBuffer() in Go 1.21+.
- dst := e.Bytes()[kvBeginOffset:]
+ // Re-slice to capacity to safely address the unused tail.
+ bufBytes := e.Bytes()
+ tmp := bufBytes[:cap(bufBytes)][e.Len():e.Len()+kvTotalLen]
+ dst := bufBytes[kvBeginOffset:]Alternatively, since this repo targets Go 1.24.x, use AvailableBuffer for clarity:
e.Grow(kvTotalLen)
- tmp := e.Bytes()[e.Len() : e.Len()+kvTotalLen]
- dst := e.Bytes()[kvBeginOffset:]
+ tmp := e.AvailableBuffer()[:kvTotalLen]
+ bufBytes := e.Bytes()
+ dst := bufBytes[kvBeginOffset:]Both avoid panics and keep the zero-copy staging intent.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In vendor/github.com/fxamacker/cbor/v2/encode.go around lines 1303 to 1315, the
temporary slice tmp is created using a two-index slice on e.Bytes() after
calling e.Grow(kvTotalLen), which can panic because len has not changed; replace
that unsafe slice with a slice that uses the buffer capacity or AvailableBuffer:
after Grow, obtain the tail region as either e.AvailableBuffer() (since this
repo targets Go 1.24) or by slicing from e.Len() to cap(e.Bytes())
(e.Bytes()[e.Len():cap(e.Bytes())]) so you get the full available capacity for
staging without touching beyond len; update code to use that tmp and keep the
zero-copy staging intent.
| // GoNamePrefixFunc sets an optional rule to prefix go names | ||
| // which do not start with a letter. | ||
| // | ||
| // The prefix function is assumed to return a string that starts with an upper case letter. | ||
| // | ||
| // e.g. to help convert "123" into "{prefix}123" | ||
| // | ||
| // The default is to prefix with "X" | ||
| var GoNamePrefixFunc func(string) string |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Document the GoNamePrefixFunc parameter expectations.
The comment states "The prefix function is assumed to return a string that starts with an upper case letter" (Line 27), but there's no enforcement of this requirement. If the caller provides a function that doesn't meet this assumption, the resulting name might not be a valid Go identifier.
🏁 Script executed:
#!/bin/bash
# Check if there are any existing usages of GoNamePrefixFunc in the codebase
rg -n "GoNamePrefixFunc\s*=" --type goLength of output: 115
🏁 Script executed:
#!/bin/bash
sed -n '1,60p' vendor/github.com/go-openapi/swag/util.goLength of output: 1712
Document GoNamePrefixFunc return‐value requirement
The existing comment notes that the prefix function “is assumed to return a string that starts with an upper case letter,” but neither the code nor the docs enforce or clearly state this requirement. Update the documentation (and optionally add a runtime check) so callers know they must return a string whose first rune is an uppercase Unicode letter.
• File: vendor/github.com/go-openapi/swag/util.go (around line 24)
• Suggested diff:
// The default is to prefix with "X"
-var GoNamePrefixFunc func(string) string
+// GoNamePrefixFunc sets a rule to prefix Go names which do not start with a letter.
+// The provided function MUST return a string whose first rune is an uppercase Unicode letter,
+// or the resulting name may not be a valid Go identifier.
+var GoNamePrefixFunc func(string) string| if c := firstPart[0]; c < utf8.RuneSelf { | ||
| // ASCII | ||
| switch { | ||
| case 'A' <= c && c <= 'Z': | ||
| result.WriteString(firstPart) | ||
| case 'a' <= c && c <= 'z': | ||
| result.WriteByte(c - 'a' + 'A') | ||
| result.WriteString(firstPart[1:]) | ||
| default: | ||
| result.WriteString(prefixFunc(name, firstPart)) | ||
| // NOTE: no longer check if prefixFunc returns a string that starts with uppercase: | ||
| // assume this is always the case | ||
| } | ||
| } else { | ||
| // unicode | ||
| firstRune, _ := utf8.DecodeRuneInString(firstPart) | ||
| switch { | ||
| case !unicode.IsLetter(firstRune): | ||
| result.WriteString(prefixFunc(name, firstPart)) | ||
| case !unicode.IsUpper(firstRune): | ||
| result.WriteString(prefixFunc(name, firstPart)) | ||
| /* | ||
| result.WriteRune(unicode.ToUpper(firstRune)) | ||
| result.WriteString(firstPart[offset:]) | ||
| */ |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Potential issue with commented-out code and redundant prefixing.
Lines 275-278 contain commented-out code that appears to be an alternative implementation for handling non-uppercase unicode characters. Additionally, when a unicode character is not uppercase (Line 273-274), the code applies prefixFunc which may result in double-prefixing if the character is already a letter but just not uppercase.
case !unicode.IsLetter(firstRune):
result.WriteString(prefixFunc(name, firstPart))
case !unicode.IsUpper(firstRune):
- result.WriteString(prefixFunc(name, firstPart))
- /*
- result.WriteRune(unicode.ToUpper(firstRune))
- result.WriteString(firstPart[offset:])
- */
+ // For lowercase letters, just uppercase the first rune
+ result.WriteRune(unicode.ToUpper(firstRune))
+ _, offset := utf8.DecodeRuneInString(firstPart)
+ result.WriteString(firstPart[offset:])
default:
result.WriteString(firstPart)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if c := firstPart[0]; c < utf8.RuneSelf { | |
| // ASCII | |
| switch { | |
| case 'A' <= c && c <= 'Z': | |
| result.WriteString(firstPart) | |
| case 'a' <= c && c <= 'z': | |
| result.WriteByte(c - 'a' + 'A') | |
| result.WriteString(firstPart[1:]) | |
| default: | |
| result.WriteString(prefixFunc(name, firstPart)) | |
| // NOTE: no longer check if prefixFunc returns a string that starts with uppercase: | |
| // assume this is always the case | |
| } | |
| } else { | |
| // unicode | |
| firstRune, _ := utf8.DecodeRuneInString(firstPart) | |
| switch { | |
| case !unicode.IsLetter(firstRune): | |
| result.WriteString(prefixFunc(name, firstPart)) | |
| case !unicode.IsUpper(firstRune): | |
| result.WriteString(prefixFunc(name, firstPart)) | |
| /* | |
| result.WriteRune(unicode.ToUpper(firstRune)) | |
| result.WriteString(firstPart[offset:]) | |
| */ | |
| if c := firstPart[0]; c < utf8.RuneSelf { | |
| // ASCII | |
| switch { | |
| case 'A' <= c && c <= 'Z': | |
| result.WriteString(firstPart) | |
| case 'a' <= c && c <= 'z': | |
| result.WriteByte(c - 'a' + 'A') | |
| result.WriteString(firstPart[1:]) | |
| default: | |
| result.WriteString(prefixFunc(name, firstPart)) | |
| // NOTE: no longer check if prefixFunc returns a string that starts with uppercase: | |
| // assume this is always the case | |
| } | |
| } else { | |
| // unicode | |
| firstRune, _ := utf8.DecodeRuneInString(firstPart) | |
| switch { | |
| case !unicode.IsLetter(firstRune): | |
| result.WriteString(prefixFunc(name, firstPart)) | |
| case !unicode.IsUpper(firstRune): | |
| // For lowercase letters, just uppercase the first rune | |
| result.WriteRune(unicode.ToUpper(firstRune)) | |
| _, offset := utf8.DecodeRuneInString(firstPart) | |
| result.WriteString(firstPart[offset:]) | |
| default: | |
| result.WriteString(firstPart) | |
| } | |
| } |
🤖 Prompt for AI Agents
In vendor/github.com/go-openapi/swag/util.go around lines 254 to 278, the
unicode branch both leaves commented-out alternative logic and unconditionally
calls prefixFunc when the first rune is a letter but not uppercase, which can
cause double-prefixing; remove the dead commented code and change the branch so
that: if the first rune is not a letter call prefixFunc(name, firstPart),
otherwise when it is a letter but not uppercase convert the first rune to its
uppercase form and write that plus the remainder of firstPart (instead of
calling prefixFunc) to avoid adding a prefix twice.
Why we need this PR
Update FAR tools and versions
Similar to #163
Changes made
go.mod, and Ginkgo v2.22.0 ->2.23.4make go-verifyWhich issue(s) this PR fixes
Test plan
Summary by CodeRabbit