[chore] Upgrade golangci-lint, ignore existing int overflow warnings #3420
Conversation
There is a new lint for unchecked int casts. Integer overflows are bad, but the old code that triggers this lint seems to be perfectly fine. Instead of disabling the lint entirely for new code as well, grandfather in existing code.
|
oops, this is a duplicate of #3258 -- though that one is stuck. can port over the nolint annotations from there. |
|
Porting over the nolint annotations and math.MaxWhatever checks is probably a good idea 👍 |
|
having this linter is a good idea, though replacing api model ints with uints i'm not sure about. the reason that go returns signed integer types for lengths, silces, capacities etc is that a signed integer overflow specifically causes a panic as a negative length is invalid. but an unsigned integer just silently overflows to a valid result which makes hunting down bugs much harder. |
| if avatar.Size > int64(maxsz) { | ||
| text := fmt.Sprintf("media exceeds configured max size: %s", maxsz) | ||
| if avatar.Size > maxsz { | ||
| text := fmt.Sprintf("media exceeds configured max size: %d", maxsz) |
There was a problem hiding this comment.
bytesize.Size specifically implements String() for nicer log output, this will only make error responses harder to read by returning a number bytes in the order of millions vs simplified size output.
leaving it as it was ensures we keep that nicer log output both here and below.
There was a problem hiding this comment.
I'll revert those things then. I suggest to change go-bytesize to use the same type as file sizes in the rest of std. this conversion shouldn't be necessary at all if everybody just agrees on a type.
|
just realized these changes are from the go1.23 PR, but i hadn't gotten around to reading that one yet 😅 given these weren't your changes either myself or @tsmethurst can make them (though totally i'm happy to), if so just let me know :) |
|
np im doing it |
|
this is also ready to review IMO |
|
Hell yeah nice, thanks! I'll take a look tomorrow :) |
|
Squerged, thank you very much! |
There is a new lint for unchecked int casts. Integer overflows are bad,
but the old code that triggers this lint seems to be perfectly fine.
Instead of disabling the lint entirely for new code as well, grandfather
in existing code. Maybe new code can be written in such a way to avoid
excessive casting.
Also fix a broken link in CONTRIBUTING