test issue 3#150
Conversation
🤖 Augment PR SummarySummary: Adds a new 🤖 Was this summary useful? React with 👍 or 👎 |
![augmentcode[bot]](https://avatars.githubusercontent.com/in/1027498?s=60&v=4){kind=small}
| } | ||
|
|
||
| func (bs badStruct) add( i int) { | ||
| bs.ar = append(bs.ar, i) |
There was a problem hiding this comment.
add has a value receiver, so updating bs.ar won’t update the caller’s slice header (the appended element may be lost or only show up via subtle backing-array mutation). Consider making the receiver a pointer (or otherwise returning the updated value) if the intent is to mutate the original badStruct.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
There was a problem hiding this comment.
Deep Reviewer summary
Findings on this diff (1 file, +8/-0):
- 1 × correctness (high): value-receiver
appendis a no-op for callers. - 1 × formatting (low): stray space inside parameter list —
gofmtviolation. - 1 × dead code (medium): unused type/method will fail
unused/revive/staticcheckper.golangci.yml.
Overall verdict: request changes recommended before merge. CI (golangci-lint) will fail on the unused symbols and on the gofmt issue, and the method as written is silently broken even if it ever gets called.
| type badStruct struct { | ||
| ar []int | ||
| } | ||
|
|
||
| func (bs badStruct) add( i int) { | ||
| bs.ar = append(bs.ar, i) | ||
| } |
There was a problem hiding this comment.
Dead code — will fail CI (unused / revive / staticcheck).
badStruct and its method add are not referenced anywhere in the package (this file is the package main entrypoint and only calls cmdproc.ExecRootCmd). The repository's .golangci.yml enables unused, revive, and staticcheck (with all checks), all of which will flag this as U1000/unused-parameter/etc.
If this was added for demonstration/testing purposes, please remove it before merging; otherwise the lint job will block the PR. If it is meant to be used, please add the call site in the same PR so the symbols stop being dead.
| func (bs badStruct) add( i int) { | ||
| bs.ar = append(bs.ar, i) |
There was a problem hiding this comment.
Correctness bug: value receiver makes append invisible to the caller.
add has a value receiver (bs badStruct), so bs is a copy of the caller's struct. append may return a slice with a new length (and potentially a new backing array); assigning it to bs.ar only mutates the local copy. The caller's badStruct.ar is never updated — the method is a silent no-op from the caller's perspective.
Also, add( i int) has a stray space after (, which gofmt will reject (and the repo's formatter check enforces this).
Minimal fix — switch to a pointer receiver and remove the extra space:
| func (bs badStruct) add( i int) { | |
| bs.ar = append(bs.ar, i) | |
| func (bs *badStruct) add(i int) { | |
| bs.ar = append(bs.ar, i) |
(If the type is removed per the other comment, this one becomes moot.)
|
explain more |
1 participant
test issue 3