Upgrade to golangci-lint 1.60.1 #548
Conversation
Here, take the project to golangci-lint 1.60.1, which is now widely available on Homebrew and elsewhere. It comes with a number of breaking changes, so I figure it's not the worst idea to keep us on latest so that future upgrades are less painful. All breakages are related to overflow checking in `gosec` getting more strict. The strictness is kind of useful in that it reminds us to check bounds with something like `min(limit, math.MaxInt32)`, but it can't detect that a bounds check is done with `min(...)`, which is kind of annoying because it means you need a `nolint` tag on all of these.
| @@ -327,7 +327,7 @@ func (p *JobListParams) First(count int) *JobListParams { | |||
| panic("count must be <= 10000") | |||
| } | |||
| paramsCopy := p.copy() | |||
| paramsCopy.paginationCount = int32(count) | |||
| paramsCopy.paginationCount = int32(count) //nolint:gosec | |||
There was a problem hiding this comment.
Bounds are checked a little higher up in this function.
| @@ -35,6 +35,6 @@ func (p *QueueListParams) First(count int) *QueueListParams { | |||
| panic("count must be <= 10000") | |||
| } | |||
| result := p.copy() | |||
| result.paginationCount = int32(count) | |||
| result.paginationCount = int32(count) //nolint:gosec | |||
There was a problem hiding this comment.
Bounds are checked a little higher up in this function.
|
A lot of nolint you add are caused by the gosec G115 that was added recently and that are currently being reviewed as it causes a lot of problems securego/gosec#1149 (comment) I would wait a few days if I was you Side remark,I thought the gosec G115 was only added in golangci-lint 1.60.2 |
|
Ah interesting. I'm glad they're reconsidering. There's some overflow check logic that's clearly just wrong (e.g. |
This one was a bug Length 2, so 8 was not caught |
|
@bgentry Actually, can we move forward with this? Even if some of the gosec stuff is retooled, it's not like golangci-lint 1.60.2 is going to re-released. If they do anything, they'll roll forward to a new 1.60.3, at which point we can upgrade and remove some |
|
ty. Yeah I checked the other projects and a couple fixes will be required, but they're similar to these ones and should only take a couple min. |
Here, take the project to golangci-lint 1.60.1, which is now widely
available on Homebrew and elsewhere. It comes with a number of breaking
changes, so I figure it's not the worst idea to keep us on latest so
that future upgrades are less painful.
All breakages are related to overflow checking in
gosecgetting morestrict. The strictness is kind of useful in that it reminds us to check
bounds with something like
min(limit, math.MaxInt32), but it can'tdetect that a bounds check is done with
min(...), which is kind ofannoying because it means you need a
nolinttag on all of these.